diff --git a/.env.example b/.env.example index b6602db..6eefc70 100644 --- a/.env.example +++ b/.env.example @@ -64,46 +64,49 @@ SEED_FIXTURE_PATH=./seed/fixtures/seed.json # mapping below is a deployment decision and changes without editing a single # definition. # -# WHICH PROVIDER ANSWERS is a deployment decision. Two wire protocols: +# WHICH PROVIDER ANSWERS is a deployment decision, but the wire protocol is no +# longer one. There is a single implementation: # -# anthropic the Claude API. The default, and what an unset value means. # openai the chat-completions shape — which is NOT only OpenAI. Groq, # Gemini (through its OpenAI-compatible endpoint), OpenRouter, # Together, vLLM and a local Ollama all serve it, so moving # between them is MODEL_BASE_URL and MODEL_* ids, nothing more. -MODEL_PROVIDER=anthropic - -# Where the openai-compatible provider points. IGNORED — and refused at -# startup — unless MODEL_PROVIDER=openai, because a base URL set against the -# anthropic provider is a deployment that believes it has switched and has not: -# every run would still go to Anthropic, and still be billed there. # -# Groq https://api.groq.com/openai/v1 +# The anthropic path was REMOVED. MODEL_PROVIDER=anthropic is refused at +# startup rather than ignored, because a stack still carrying it would +# otherwise run on a vendor it never chose. Leave this empty or set "openai". +MODEL_PROVIDER=openai + +# Where the provider is. Defaults to Groq when unset — the model ids below are +# Groq ids, and an id is only meaningful against the service that serves it, so +# these two settings move together or not at all. +# +# Groq https://api.groq.com/openai/v1 (the default) # Gemini https://generativelanguage.googleapis.com/v1beta/openai # OpenRouter https://openrouter.ai/api/v1 # Ollama http://localhost:11434/v1 (no key needed) -MODEL_BASE_URL= +MODEL_BASE_URL=https://api.groq.com/openai/v1 -# The credential. MODEL_API_KEY is the provider-neutral name and wins; -# ANTHROPIC_API_KEY still works so no existing deployment needs an edit. -# Either may be empty outside production: migrations, seeding and every -# endpoint that is not an agent run work without one, and an agent run fails -# with a structured `gateway.not_configured` rather than the service refusing -# to boot. APP_ENV=production requires one — unless the model is on localhost, -# which needs no credential at all. +# The credential. ANTHROPIC_API_KEY is NO LONGER READ — if it is set while this +# is empty, startup fails rather than silently ignoring it. +# May be empty outside production: migrations, seeding and every endpoint that +# is not an agent run work without one, and an agent run fails with a +# structured `gateway.not_configured` rather than the service refusing to boot. +# APP_ENV=production requires one — unless the model is on localhost, which +# needs no credential at all. MODEL_API_KEY= -ANTHROPIC_API_KEY= -# All three tiers default to the same model. They differ by *effort*, which the -# gateway fixes (fast=low, balanced=high, deep=xhigh) so that "deep" cannot -# mean two different things in two deployments. Point a tier at a different -# model only as a deliberate choice — never as a silent cost downgrade. -MODEL_FAST=claude-opus-5 -MODEL_BALANCED=claude-opus-5 -MODEL_DEEP=claude-opus-5 +# The tiers differ by model AND by *effort*, which the gateway fixes +# (fast=low, balanced=high, deep=xhigh) so that "deep" cannot mean two +# different things in two deployments. +# +# These must be ids your MODEL_BASE_URL actually serves. A leftover claude-* +# id is refused at startup: it would be accepted by this process, rejected by +# the provider, and fail every single run with a 400. +MODEL_FAST=llama-3.1-8b-instant +MODEL_BALANCED=llama-3.3-70b-versatile +MODEL_DEEP=llama-3.3-70b-versatile -# Hard ceiling on a single unstreamed response. Not the run's token budget — -# that spans every call in a run and belongs to the runtime. MODEL_MAX_OUTPUT_TOKENS=16000 # Send the tier's effort level as `reasoning_effort` on the openai-compatible diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09df635..ebcf54c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,7 +96,7 @@ jobs: if failed: print("FAILED:"); [print(" ", t) for t in failed[:40]] sys.exit(1) - # A TestLive* / TestLive* test skips without ANTHROPIC_API_KEY, which + # A TestLive* / TestLive* test skips without MODEL_API_KEY, which # is correct here: CI should not spend tokens on every push, and the key # should not be present unless somebody put it there deliberately. Any # OTHER skip means the database was unreachable, and that is the case diff --git a/CLAUDE.md b/CLAUDE.md index 45c8364..4c87754 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ depends on the curated-versus-self-serve decision and is not settled. | Registry | 9 agents + 24 skills as rows; published versions immutable (append-only, trigger-enforced); runs pin the version they started with | | Tools | 19, two of which write (`move_application`, `assign_worker`), behind a bound single-use confirmation | | Knowledge | ACL-tagged ingest, hybrid dense + BM25 fused with RRF, pre-filtered | -| Gateway | tier → model + effort, token accounting, refusal as an outcome; two providers behind one interface — `anthropic`, and `openai` for the chat-completions shape that Groq, Gemini, OpenRouter, vLLM and a local Ollama all serve | +| Gateway | tier → model + effort, token accounting, refusal as an outcome; one wire protocol — `openai`, the chat-completions shape that Groq (the default), Gemini, OpenRouter, Together, vLLM and a local Ollama all serve. The Anthropic path was removed; `MODEL_PROVIDER=anthropic`, a stale `ANTHROPIC_API_KEY` and a leftover `claude-*` id are each refused at startup rather than ignored | **Conversational writes are not agent tool calls.** Two skills — `create-position` and `create-employee-role` — collect a record through the chat panel and then @@ -261,12 +261,17 @@ Do not resolve these unilaterally. Flag them and ask. - **Who authors agents?** Curated (the team ships specs) vs. self-serve (tenants author their own). Self-serve requires prompt-injection hardening at the authoring boundary, per-tenant cost caps, an approval workflow, and a sandbox — roughly 3× the platform. Current assumption: **curated**, with the registry designed so self-serve is additive later. - **Model hosting.** Self-hosted vs. API vs. mixed by tier. **Still open** — - but no longer expensive to change: `MODEL_PROVIDER` + `MODEL_BASE_URL` move - the whole platform between Anthropic, Groq, Gemini, OpenRouter and a local - Ollama without a code change, and `make eval-live` runs the suite against - whichever is configured. Decide it on the eval evidence, and weigh the I7 - case heaviest: a cheaper model that follows the planted injection is a - security regression, not a saving. + but no longer expensive to change: `MODEL_BASE_URL` + the three `MODEL_*` ids + move the whole platform between Groq (the default), Gemini, OpenRouter, + Together, vLLM and a local Ollama without a code change, and `make eval-live` + runs the suite against whichever is configured. Decide it on the eval + evidence, and weigh the I7 case heaviest: a cheaper model that follows the + planted injection is a security regression, not a saving. + + **This is now urgent rather than open.** Removing the Anthropic path also + removed the only model whose behaviour on that I7 case had actually been + measured here, so the current default is unproven against it until + `make eval-live` has been run with a real key. - **Confirmation UX.** Inline in-chat vs. an approval queue. --- diff --git a/Makefile b/Makefile index ab952c7..665e70a 100644 --- a/Makefile +++ b/Makefile @@ -99,12 +99,17 @@ check-agents: ## Parse every spec in agents/ and report, writing nothing .PHONY: eval-live eval-live: ## Run the eval suites against the REAL model (needs a key, costs tokens) - @test -n "$$MODEL_API_KEY" -o -n "$$ANTHROPIC_API_KEY" || { \ - echo "eval-live needs MODEL_API_KEY (or ANTHROPIC_API_KEY) — it calls a real model and costs tokens."; \ + @test -n "$$MODEL_API_KEY" || { \ + echo "eval-live needs MODEL_API_KEY — it calls a real model and costs tokens."; \ echo "The scripted suites (make eval) are the gate; this is the confirmation."; \ echo ""; \ - echo "To evaluate a different provider, point it somewhere else:"; \ - echo " MODEL_PROVIDER=openai \\"; \ + if [ -n "$$ANTHROPIC_API_KEY" ]; then \ + echo "NOTE: ANTHROPIC_API_KEY is set and is no longer read — the Anthropic"; \ + echo " path was removed. Rename it to MODEL_API_KEY, and replace the"; \ + echo " value if it is an Anthropic key."; \ + echo ""; \ + fi; \ + echo "Defaults to Groq. To evaluate somewhere else, point it there:"; \ echo " MODEL_BASE_URL=https://api.groq.com/openai/v1 \\"; \ echo " MODEL_API_KEY=... MODEL_BALANCED= make eval-live"; \ exit 1; } diff --git a/docs/handover.md b/docs/handover.md index 4e4f645..fc3d38c 100644 --- a/docs/handover.md +++ b/docs/handover.md @@ -184,19 +184,25 @@ configmap before shipping an image that contains the check. ## Changing model provider -The gateway speaks two wire protocols. `anthropic` is the Claude API. -`openai` is the chat-completions shape — and that one is not only OpenAI: -Groq, Gemini's compatibility endpoint, OpenRouter, Together, vLLM and a local -Ollama all serve it, so moving between them is configuration, not code. +The gateway speaks one wire protocol: `openai`, the chat-completions shape. +That is not the same as one vendor — Groq, Gemini's compatibility endpoint, +OpenRouter, Together, vLLM and a local Ollama all serve it, so moving between +them is configuration, not code. + +**The Anthropic path was removed.** `MODEL_PROVIDER=anthropic` and a stale +`ANTHROPIC_API_KEY` are both *refused at startup* rather than ignored, and so +is a leftover `claude-*` model id. That is deliberate: each of those would +otherwise produce a service that boots cleanly and fails every agent run. + +The default with nothing set is Groq. ```bash -# Groq -MODEL_PROVIDER=openai +# Groq (the default — base URL and ids below are what you get unset) MODEL_BASE_URL=https://api.groq.com/openai/v1 MODEL_API_KEY= MODEL_FAST=llama-3.1-8b-instant -MODEL_BALANCED=openai/gpt-oss-120b -MODEL_DEEP=openai/gpt-oss-120b +MODEL_BALANCED=llama-3.3-70b-versatile +MODEL_DEEP=llama-3.3-70b-versatile # Gemini MODEL_BASE_URL=https://generativelanguage.googleapis.com/v1beta/openai @@ -207,11 +213,10 @@ MODEL_BASE_URL=http://localhost:11434/v1 Four things worth knowing before you do it. -**Set `MODEL_PROVIDER`, not just the base URL.** The anthropic path has one -endpoint and ignores `MODEL_BASE_URL` entirely, so setting the URL alone is a -deployment that believes it has switched providers and has not — every run -still goes to Anthropic and is still billed there. Config validation refuses -that combination at startup rather than letting it run up a bill quietly. +**A model id and a base URL are one decision, not two.** An id is only +meaningful against the service that serves it, so changing the endpoint without +changing the ids gives you a process that starts fine and 400s on every run. +The defaults ship as a matched Groq pair for that reason. **Leave `MODEL_REASONING_EFFORT` off unless every configured model is a reasoning model.** Reasoning models accept the field; most others reject the @@ -220,19 +225,23 @@ reasoning model.** Reasoning models accept the field; most others reject the **Run the evals before trusting it, and read the I7 case first.** ```bash -MODEL_PROVIDER=openai MODEL_BASE_URL=… MODEL_API_KEY=… MODEL_BALANCED=… make eval-live +MODEL_BASE_URL=… MODEL_API_KEY=… MODEL_BALANCED=… make eval-live ``` `liveGateway` reads the same environment the service does and logs which provider and model answered. The handbook corpus contains a planted prompt -injection; Claude refuses it and reports the document as tampered with. **A -model that answers every other case well and follows that injection is not a -cheaper option — it is a security regression.** That case is the gate, not the -cost table. +injection. A model worth running refuses it and reports the document as +tampered with. **A model that answers every other case well and follows that +injection is not a cheaper option — it is a security regression.** That case is +the gate, not the cost table. -**Token accounting differs between the two wires and is already reconciled.** -OpenAI reports `prompt_tokens` *inclusive* of the cached prefix; Anthropic -reports input tokens *exclusive* of it. `oaiUsage.normalise` subtracts, because +This one is not optional now: the removed provider was the one whose refusal +behaviour had actually been measured here, so whatever replaces it is unproven +against I7 until this suite says otherwise. + +**Token accounting is already reconciled, and the subtraction is load-bearing.** +This wire reports `prompt_tokens` *inclusive* of the cached prefix, while +`Usage` carries the cached figure separately. `oaiUsage.normalise` subtracts, because `Usage.Total()` sums all four fields and copying both numbers across verbatim would bill the cached prefix twice — worst on long conversations, which is exactly where I3's budget matters most. Don't "simplify" that subtraction away; diff --git a/go-api/go.mod b/go-api/go.mod index 46274d5..ff31437 100644 --- a/go-api/go.mod +++ b/go-api/go.mod @@ -3,26 +3,15 @@ module github.com/krow/krow-backend/go-api go 1.27 require ( - github.com/anthropics/anthropic-sdk-go v1.66.0 github.com/jackc/pgx/v5 v5.10.0 golang.org/x/crypto v0.42.0 golang.org/x/term v0.35.0 ) require ( - github.com/bahlo/generic-list-go v0.2.0 // indirect - github.com/buger/jsonparser v1.1.2 // indirect - github.com/invopop/jsonschema v0.14.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect - github.com/pb33f/ordered-map/v2 v2.3.1 // indirect - github.com/standard-webhooks/standard-webhooks/libraries v0.0.1 // indirect - github.com/tidwall/gjson v1.18.0 // indirect - github.com/tidwall/match v1.1.1 // indirect - github.com/tidwall/pretty v1.2.1 // indirect - github.com/tidwall/sjson v1.2.5 // indirect - go.yaml.in/yaml/v4 v4.0.0-rc.2 // indirect golang.org/x/sync v0.17.0 // indirect golang.org/x/sys v0.37.0 // indirect golang.org/x/text v0.29.0 // indirect diff --git a/go-api/go.sum b/go-api/go.sum index 0dfcde5..ddcaf60 100644 --- a/go-api/go.sum +++ b/go-api/go.sum @@ -1,16 +1,6 @@ -github.com/anthropics/anthropic-sdk-go v1.66.0 h1:/CKwgscn0Pe1q4U8aFInSOt/v06JeMc9Aq4vIlctCFw= -github.com/anthropics/anthropic-sdk-go v1.66.0/go.mod h1:3EfIfmFqxH6rbiLcIP4tPFyXL/IHakx2wDG4OU+TIEI= -github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk= -github.com/bahlo/generic-list-go v0.2.0/go.mod h1:2KvAjgMlE5NNynlg/5iLrrCCZ2+5xWbdbCW3pNTGyYg= -github.com/buger/jsonparser v1.1.2 h1:frqHqw7otoVbk5M8LlE/L7HTnIq2v9RX6EJ48i9AxJk= -github.com/buger/jsonparser v1.1.2/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI= -github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ= -github.com/invopop/jsonschema v0.14.0 h1:MHQqLhvpNUZfw+hM3AZDYK7jxO8FZoQeQM77g8iyZjg= -github.com/invopop/jsonschema v0.14.0/go.mod h1:ygm6C2EaVNMBDPpaPlnOA2pFAxBnxGjFlMZABxm9n2I= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= @@ -19,29 +9,13 @@ github.com/jackc/pgx/v5 v5.10.0 h1:VhSvgU2jSli8o3AqIEOTJr7rZwAEUVo4E4XhR94Zfr0= github.com/jackc/pgx/v5 v5.10.0/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= -github.com/pb33f/ordered-map/v2 v2.3.1 h1:5319HDO0aw4DA4gzi+zv4FXU9UlSs3xGZ40wcP1nBjY= -github.com/pb33f/ordered-map/v2 v2.3.1/go.mod h1:qxFQgd0PkVUtOMCkTapqotNgzRhMPL7VvaHKbd1HnmQ= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/standard-webhooks/standard-webhooks/libraries v0.0.1 h1:uOfcYT+3QungH6tIGSVCR/Y3KJmgJiHcojJbMTPDZAI= -github.com/standard-webhooks/standard-webhooks/libraries v0.0.1/go.mod h1:L1MQhA6x4dn9r007T033lsaZMv9EmBAdXyU/+EF40fo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= -github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= -github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= -github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= -github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= -github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= -github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= -github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= -github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= -github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= -go.yaml.in/yaml/v4 v4.0.0-rc.2 h1:/FrI8D64VSr4HtGIlUtlFMGsm7H7pWTbj6vOLVZcA6s= -go.yaml.in/yaml/v4 v4.0.0-rc.2/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= golang.org/x/crypto v0.42.0 h1:chiH31gIWm57EkTXpwnqf8qeuMUi0yekh6mT2AvFlqI= golang.org/x/crypto v0.42.0/go.mod h1:4+rDnOTJhQCx2q7/j6rAN5XDw8kPjeaXEUR2eL94ix8= golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug= @@ -53,8 +27,6 @@ golang.org/x/term v0.35.0/go.mod h1:TPGtkTLesOwf2DE8CgVYiZinHAOuy5AYUYT1lENIZnA= golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= -gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go-api/internal/config/config.go b/go-api/internal/config/config.go index 225ec42..3a3252d 100644 --- a/go-api/internal/config/config.go +++ b/go-api/internal/config/config.go @@ -19,9 +19,23 @@ import ( "time" ) -// defaultModel is what every reasoning tier routes to until a deployment says -// otherwise. Named once here so the three tiers cannot drift apart by accident. -const defaultModel = "claude-opus-5" +// The default model per tier, and the endpoint they are valid on. +// +// THESE THREE AND defaultBaseURL ARE ONE DECISION, not four. A model id is only +// meaningful against the service that serves it, so a Groq id with an OpenAI +// base URL is not a partial configuration — it is a broken one that starts +// cleanly and fails every run at request time. They changed together when the +// Anthropic path was removed and they have to keep changing together. +// +// Unlike the old single default, the tiers are no longer the same model: the +// point of a tier is that `fast` costs less than `deep`, and one id for all +// three made the distinction free and therefore meaningless. +const ( + defaultBaseURL = "https://api.groq.com/openai/v1" + defaultFastModel = "llama-3.1-8b-instant" + defaultBalancedModel = "llama-3.3-70b-versatile" + defaultDeepModel = "llama-3.3-70b-versatile" +) // Config is the whole of the Phase 1 configuration surface. type Config struct { @@ -36,8 +50,9 @@ type Config struct { // KnowledgeConfig routes the retrieval layer's embedding provider. // -// Anthropic does not serve embeddings, so the dense half of hybrid retrieval -// needs a separate credential. Voyage is the documented partner and the default. +// The chat provider does not serve embeddings, so the dense half of hybrid +// retrieval needs its own provider and credential — this is a separate choice +// from MODEL_*, and pointing one of them somewhere new does not move the other. // // An empty key is legitimate: this service boots and serves without one, and // retrieval degrades to keyword-only rather than failing — reported on every @@ -88,19 +103,20 @@ type KnowledgeConfig struct { // first model call, as a structured gateway.not_configured a run can end with, // not at startup as a refusal to boot. type ModelConfig struct { - // Provider names the wire protocol: "anthropic" or "openai". Empty means - // anthropic, so a deployment that predates the second provider keeps - // working with the environment it already has. + // Provider names the wire protocol. "openai" is the only one, and empty + // means it; "anthropic" is refused at startup rather than ignored, because + // a deployment still carrying it has not been told the path was removed. // // "openai" is not only OpenAI. Groq, Gemini's compatibility endpoint, // OpenRouter, Together, vLLM and a local Ollama all serve that same shape, - // and BaseURL is what chooses between them. + // and BaseURL is what chooses between them — which is why one wire protocol + // is not the same thing as one vendor. Provider string APIKey string - // BaseURL points the OpenAI-compatible provider at a specific service. - // Ignored by the anthropic provider, which has one endpoint. + // BaseURL points the provider at a specific service. Empty means the + // default in defaultBaseURL, which the default model ids belong to. BaseURL string Fast string @@ -265,16 +281,17 @@ func Load() (*Config, error) { }, Model: ModelConfig{ Provider: strings.ToLower(strings.TrimSpace(os.Getenv("MODEL_PROVIDER"))), - // MODEL_API_KEY first, then the Anthropic-specific name. Two - // spellings because the second provider is not Anthropic and - // ANTHROPIC_API_KEY= would be a lie an operator has to - // keep re-reading; the fallback keeps every existing deployment - // working without an edit. - APIKey: firstSet("MODEL_API_KEY", "ANTHROPIC_API_KEY"), - BaseURL: strings.TrimSpace(os.Getenv("MODEL_BASE_URL")), - Fast: withDefault("MODEL_FAST", defaultModel), - Balanced: withDefault("MODEL_BALANCED", defaultModel), - Deep: withDefault("MODEL_DEEP", defaultModel), + // One spelling. ANTHROPIC_API_KEY used to be accepted as a + // fallback and is now deliberately NOT read: with the Anthropic + // path gone it would name a vendor this service cannot call, and + // silently authenticating to Groq with a variable called + // ANTHROPIC_API_KEY is the kind of lie an operator has to keep + // re-reading. A stale one is caught at startup, not ignored. + APIKey: strings.TrimSpace(os.Getenv("MODEL_API_KEY")), + BaseURL: withDefault("MODEL_BASE_URL", defaultBaseURL), + Fast: withDefault("MODEL_FAST", defaultFastModel), + Balanced: withDefault("MODEL_BALANCED", defaultBalancedModel), + Deep: withDefault("MODEL_DEEP", defaultDeepModel), // 16k keeps a non-streaming response inside the SDK's HTTP // timeout. The loop raises it and switches to streaming when it // needs a long answer; this is the ceiling for a single @@ -341,27 +358,59 @@ const DeepestAgentDeadline = 120 * time.Second // or a production install with no credential that fails one run at a time // instead of once at startup. func (c *Config) validateModel() error { + // "anthropic" is named separately from every other wrong value because it + // is the one that used to be correct. A deployment still carrying it is not + // a typo, it is a stack that has not been told the path was removed — and + // the silent alternative is a service that believes it is on Claude while + // every run goes to Groq and is billed there. switch c.Model.Provider { - case "", "anthropic", "openai": + case "", "openai": + case "anthropic": + return fmt.Errorf("MODEL_PROVIDER=anthropic is no longer supported: the Anthropic " + + "path was removed and this service speaks only the openai chat-completions " + + "shape. Unset MODEL_PROVIDER (or set it to openai) and point MODEL_BASE_URL " + + "at your provider") default: - return fmt.Errorf("MODEL_PROVIDER must be anthropic or openai, got %q", c.Model.Provider) + return fmt.Errorf("MODEL_PROVIDER must be openai (or empty, which means openai), got %q", c.Model.Provider) + } + // A credential under the old name is refused rather than ignored. Ignoring + // it produces the worst version of this failure: a deployment that set a + // key, sees no error, and fails every run on a missing credential it is + // looking straight at. + if os.Getenv("ANTHROPIC_API_KEY") != "" && c.Model.APIKey == "" { + return fmt.Errorf("ANTHROPIC_API_KEY is set but is no longer read, and MODEL_API_KEY is " + + "empty: the Anthropic path was removed. Rename the variable to MODEL_API_KEY " + + "— and if that value is an Anthropic key, replace it, because nothing here can " + + "call Anthropic any more") } // A local model needs no credential, and demanding one would make the // zero-cost development path impossible to configure. Everything else does: // a production deployment without a key fails every run at the gateway, // which is a misconfiguration wearing a runtime error's clothes. if c.AppEnv == "production" && c.Model.APIKey == "" && !isLoopback(c.Model.BaseURL) { - return fmt.Errorf("MODEL_API_KEY (or ANTHROPIC_API_KEY) is required when APP_ENV=production; " + + return fmt.Errorf("MODEL_API_KEY is required when APP_ENV=production; " + "without it every agent run fails at the model gateway") } - // A base URL is only read by the OpenAI-compatible provider. Setting one - // while on anthropic is a deployment that believes it has switched - // providers and has not — it would keep calling Claude and keep being - // billed for it, with nothing in the logs to say so. - if c.Model.BaseURL != "" && c.Model.Provider != "openai" { - return fmt.Errorf("MODEL_BASE_URL only applies when MODEL_PROVIDER=openai; "+ - "it is set to %q but the provider is %q, so the base URL would be ignored "+ - "and every run would still go to Anthropic", c.Model.BaseURL, providerName(c.Model.Provider)) + // A model id left over from the Anthropic path. THIS IS THE CHECK THAT + // REPLACED the old "base URL set against the wrong provider" one, and it + // guards the same failure from the other side. + // + // It is not hypothetical. A `claude-*` id sent to an OpenAI-compatible + // endpoint is accepted by this process, rejected by the provider, and + // surfaces as a 400 on EVERY run — which is exactly the incident that made + // the gateway start carrying upstream error text in the first place. One + // loud failure at startup is worth more than one per run. + for _, m := range []struct{ key, id string }{ + {"MODEL_FAST", c.Model.Fast}, + {"MODEL_BALANCED", c.Model.Balanced}, + {"MODEL_DEEP", c.Model.Deep}, + } { + if strings.HasPrefix(strings.ToLower(m.id), "claude") { + return fmt.Errorf("%s is %q, but the Anthropic path was removed: no configured "+ + "provider serves a claude model, so every run on this tier would fail at "+ + "the gateway. Set it to a model id your MODEL_BASE_URL (%s) serves", + m.key, m.id, c.Model.BaseURL) + } } if c.Model.BaseURL != "" { u, err := url.Parse(c.Model.BaseURL) @@ -582,7 +631,7 @@ func isLoopback(raw string) bool { // rather than showing an empty string an operator then has to interpret. func providerName(p string) string { if p == "" { - return "anthropic (the default)" + return "openai (the default)" } return p } diff --git a/go-api/internal/config/model_test.go b/go-api/internal/config/model_test.go index 70bf055..3c294da 100644 --- a/go-api/internal/config/model_test.go +++ b/go-api/internal/config/model_test.go @@ -18,14 +18,20 @@ func TestValidateModelProvider(t *testing.T) { wantErr bool }{ { - "unset provider is anthropic, which is what every existing deployment has", + "unset provider is openai, which is now the only implementation", modelCfg("development", ModelConfig{}), false, }, - {"anthropic named explicitly", modelCfg("development", ModelConfig{Provider: "anthropic"}), false}, - {"openai", modelCfg("development", ModelConfig{Provider: "openai"}), false}, + {"openai named explicitly", modelCfg("development", ModelConfig{Provider: "openai"}), false}, + { + "anthropic is refused rather than ignored — it used to be correct", + modelCfg("development", ModelConfig{Provider: "anthropic"}), true, + }, {"a typo is caught once at startup, not once per run", modelCfg("development", ModelConfig{Provider: "openal"}), true}, - {"a provider that does not exist", modelCfg("development", ModelConfig{Provider: "groq"}), true}, + { + "a vendor name is not a provider: groq is reached through openai + a base URL", + modelCfg("development", ModelConfig{Provider: "groq"}), true, + }, } { t.Run(tc.name, func(t *testing.T) { err := tc.cfg.validateModel() @@ -36,28 +42,27 @@ func TestValidateModelProvider(t *testing.T) { } } -// THE EXPENSIVE MISTAKE. +// THE STALE CONFIGURATION. // -// A deployment that sets MODEL_BASE_URL and forgets MODEL_PROVIDER believes it -// has moved off Claude. It has not: the anthropic path has one endpoint and -// ignores the field entirely, so every run keeps going to Anthropic and keeps -// being billed there, with nothing in the logs to say so. The whole point of -// this change is cost, and that is the one misconfiguration that silently -// defeats it. -func TestBaseURLWithoutOpenAIProviderIsRefused(t *testing.T) { - err := modelCfg("development", ModelConfig{ - BaseURL: "https://api.groq.com/openai/v1", - }).validateModel() +// This replaced a test called TestBaseURLWithoutOpenAIProviderIsRefused, which +// guarded the mirror image of the same mistake: while both providers existed, a +// base URL without MODEL_PROVIDER=openai meant a deployment that believed it had +// left Claude and had not. That failure is now impossible — there is nowhere +// else for a run to go — and the surviving one points the other way: a +// deployment that still names Anthropic, and must be told rather than silently +// re-pointed at a provider it never chose. +func TestTheRemovedProviderIsRefusedLoudly(t *testing.T) { + err := modelCfg("development", ModelConfig{Provider: "anthropic"}).validateModel() if err == nil { - t.Fatal("a base URL on the anthropic provider was accepted; every run would still go to Anthropic") + t.Fatal("MODEL_PROVIDER=anthropic was accepted; the stack would silently run on another vendor") } - for _, want := range []string{"MODEL_BASE_URL", "MODEL_PROVIDER=openai", "Anthropic"} { + for _, want := range []string{"MODEL_PROVIDER=anthropic", "no longer supported", "MODEL_BASE_URL"} { if !strings.Contains(err.Error(), want) { t.Errorf("the message does not mention %q:\n %v", want, err) } } - // The same URL with the provider set is exactly the intended configuration. + // The intended configuration is exactly what the message tells them to set. if err := modelCfg("development", ModelConfig{ Provider: "openai", BaseURL: "https://api.groq.com/openai/v1", }).validateModel(); err != nil { @@ -65,6 +70,45 @@ func TestBaseURLWithoutOpenAIProviderIsRefused(t *testing.T) { } } +// A model id that outlived its provider. +// +// The expensive shape of this is not a typo, it is an UNCHANGED .env: the tier +// ids were claude-* for the whole life of the Anthropic path, and nothing about +// switching providers forces them to be revisited. Left unchecked the process +// starts clean and every single run fails at the gateway with a 400 — which is +// the incident that made the gateway start carrying upstream error text at all. +func TestClaudeModelIdsAreRefused(t *testing.T) { + base := ModelConfig{Provider: "openai", BaseURL: "https://api.groq.com/openai/v1", + Fast: "llama-3.1-8b-instant", Balanced: "llama-3.3-70b-versatile", Deep: "llama-3.3-70b-versatile"} + + for _, tier := range []string{"MODEL_FAST", "MODEL_BALANCED", "MODEL_DEEP"} { + t.Run(tier, func(t *testing.T) { + m := base + switch tier { + case "MODEL_FAST": + m.Fast = "claude-opus-5" + case "MODEL_BALANCED": + m.Balanced = "claude-opus-5" + case "MODEL_DEEP": + m.Deep = "claude-3-5-sonnet-latest" + } + err := modelCfg("development", m).validateModel() + if err == nil { + t.Fatalf("%s kept a claude id and was accepted; every run on that tier would 400", tier) + } + // Naming the tier is the whole value: "a model is wrong" does not + // tell an operator which of three lines to edit. + if !strings.Contains(err.Error(), tier) { + t.Errorf("the message does not name the tier %q:\n %v", tier, err) + } + }) + } + + if err := modelCfg("development", base).validateModel(); err != nil { + t.Fatalf("a fully-migrated configuration was refused: %v", err) + } +} + func TestBaseURLMustBeAURL(t *testing.T) { for _, raw := range []string{"api.groq.com", "ftp://x.test", "not a url", "://broken"} { err := modelCfg("development", ModelConfig{Provider: "openai", BaseURL: raw}).validateModel() diff --git a/go-api/internal/evals/live_test.go b/go-api/internal/evals/live_test.go index 536c431..1e3f042 100644 --- a/go-api/internal/evals/live_test.go +++ b/go-api/internal/evals/live_test.go @@ -33,9 +33,9 @@ import ( // PROVIDER-DRIVEN, and that is the point. These cases are the only evidence // that answers the question a scripted model cannot — whether a real one, given // these tools and this prompt, actually does the right thing — and that -// question has a different answer for every provider. A helper hardcoded to -// Anthropic could confirm the model this platform already runs and nothing -// else, which is exactly the comparison worth having before changing it. +// question has a different answer for every provider. A helper hardcoded to one +// vendor could confirm the model this platform already runs and nothing else, +// which is exactly the comparison worth having when changing it. // // So the same environment the service reads selects the model here: // @@ -49,17 +49,17 @@ func liveGateway(t *testing.T) gateway.Gateway { t.Helper() key := strings.TrimSpace(os.Getenv("MODEL_API_KEY")) - if key == "" { - key = strings.TrimSpace(os.Getenv("ANTHROPIC_API_KEY")) - } baseURL := strings.TrimSpace(os.Getenv("MODEL_BASE_URL")) + if baseURL == "" { + baseURL = "https://api.groq.com/openai/v1" + } provider := strings.ToLower(strings.TrimSpace(os.Getenv("MODEL_PROVIDER"))) // A local model needs no credential; everything else does. Skipping rather // than failing keeps `go test ./...` green on a machine with no key, which // is what makes the scripted suites the gate. if key == "" && !strings.Contains(baseURL, "localhost") && !strings.Contains(baseURL, "127.0.0.1") { - t.Skip("no MODEL_API_KEY or ANTHROPIC_API_KEY; the live suite is skipped") + t.Skip("no MODEL_API_KEY; the live suite is skipped") } model := func(env, fallback string) string { @@ -68,9 +68,10 @@ func liveGateway(t *testing.T) gateway.Gateway { } return fallback } - // The default stays Claude, so an existing invocation of `make eval-live` - // runs exactly what it ran before this became configurable. - fallback := "claude-opus-5" + // The same default the service itself boots with, so `make eval-live` with + // no overrides measures the configuration a deployment actually gets rather + // than a better one chosen only for the suite. + fallback := "llama-3.3-70b-versatile" cfg := config.ModelConfig{ Provider: provider, @@ -85,14 +86,14 @@ func liveGateway(t *testing.T) gateway.Gateway { // Named in the output, because a suite that does not say which model // answered is a suite whose result cannot be compared with another run's. - t.Logf("live gateway: provider=%s model=%s", providerLabel(provider), cfg.Balanced) + t.Logf("live gateway: provider=%s base=%s model=%s", providerLabel(provider), baseURL, cfg.Balanced) return gateway.New(gateway.FromConfig(cfg)) } func providerLabel(p string) string { if p == "" { - return "anthropic" + return "openai" } return p } diff --git a/go-api/internal/gateway/anthropic.go b/go-api/internal/gateway/anthropic.go deleted file mode 100644 index e2f5624..0000000 --- a/go-api/internal/gateway/anthropic.go +++ /dev/null @@ -1,510 +0,0 @@ -package gateway - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "strings" - "time" - - "github.com/anthropics/anthropic-sdk-go" - "github.com/anthropics/anthropic-sdk-go/option" -) - -// AnthropicGateway calls the Claude API. -type AnthropicGateway struct { - client anthropic.Client - cfg Config -} - -// Compile-time proof that this satisfies the boundary. -var _ Gateway = (*AnthropicGateway)(nil) - -// NewAnthropic builds a gateway over the Claude API. -// -// A missing key is not an error here. The service has to boot without model -// credentials — every endpoint that is not an agent run still works, and a -// developer running migrations should not need a key to do it. The failure -// surfaces at the first Complete, as a structured NotConfigured that the -// runtime can end a run with, rather than as a panic at startup. -func NewAnthropic(cfg Config) *AnthropicGateway { - opts := []option.RequestOption{} - if cfg.APIKey != "" { - opts = append(opts, option.WithAPIKey(cfg.APIKey)) - } - return &AnthropicGateway{client: anthropic.NewClient(opts...), cfg: cfg} -} - -// routing resolves a tier against this gateway's table. -func (g *AnthropicGateway) routing(t Tier) Routing { return g.cfg.routingFor(t) } - -// sdkEffort maps the platform's effort vocabulary onto Anthropic's. -// -// A one-to-one mapping today, which is exactly why the neutral type is worth -// having: the platform's three levels are a statement about how much a turn is -// worth, and this function is where that statement meets one vendor's spelling -// of it. `max` is not reachable — see FromConfig. -func sdkEffort(e Effort) anthropic.OutputConfigEffort { - switch e { - case EffortLow: - return anthropic.OutputConfigEffortLow - case EffortXhigh: - return anthropic.OutputConfigEffortXhigh - default: - return anthropic.OutputConfigEffortHigh - } -} - -// Complete sends one request and reports one result. -// MaxAttempts is how many times a transient failure is retried. -// -// Three total, not three retries. Past that the problem is not transient and a -// fourth call is just spending money on the same answer. -const MaxAttempts = 3 - -// retryBackoff is the pause before each retry. -// -// Short, and deliberately so: this sits inside a run that already has a -// wall-clock deadline, and a backoff long enough to be polite to the API is -// long enough to spend the caller's whole budget waiting. A run that cannot -// afford the wait dies on its deadline instead, which is the correct failure. -var retryBackoff = []time.Duration{400 * time.Millisecond, 1200 * time.Millisecond} - -// Complete calls the model, retrying failures that are worth retrying. -// -// THE RETRY IS NOT DEFENSIVE POLISH. Error.Retryable() has existed since this -// package was written and had ZERO callers — the classification was built and -// never used, so a 529 "overloaded" killed a run that would have succeeded four -// hundred milliseconds later. Found by a real overload during live testing, -// where it presented as "the agent could not finish" with nothing to act on. -// -// Only genuinely transient failures qualify: rate limits, timeouts, and 5xx. -// A 400 is a malformed request and will be malformed again; a 401 is a bad -// credential and retrying it three times just gets refused three times. -// -// The run's context governs. A retry that would outlive the caller's deadline -// does not happen — the deadline belongs to the run, not to this function, and -// waiting past it would turn a bounded run into an unbounded one. -func (g *AnthropicGateway) Complete(ctx context.Context, req Request) (*Response, error) { - return withRetry(ctx, func() (*Response, error) { return g.complete(ctx, req) }) -} - -// withRetry runs one attempt until it succeeds, fails terminally, or runs out -// of attempts. -// -// SHARED BY EVERY PROVIDER, and it has to be. The retry policy is a property of -// this platform's runs — bounded attempts, short backoff, the caller's deadline -// winning — not of any one vendor's API. Left as a method, the second provider -// would have grown its own copy, and the two would have drifted the first time -// either was tuned. -func withRetry(ctx context.Context, once func() (*Response, error)) (*Response, error) { - var last error - for attempt := 0; attempt < MaxAttempts; attempt++ { - if attempt > 0 { - pause := retryBackoff[min(attempt-1, len(retryBackoff)-1)] - select { - case <-time.After(pause): - case <-ctx.Done(): - // Out of time. The ORIGINAL failure is returned rather than the - // context error: "the model was overloaded" is what an operator - // needs to see, and "context deadline exceeded" would hide it. - return nil, last - } - } - - resp, err := once() - if err == nil { - return resp, nil - } - last = err - - var gwErr *Error - if !errors.As(err, &gwErr) || !gwErr.Retryable() { - return resp, err - } - } - return nil, last -} - -// complete is one attempt. -func (g *AnthropicGateway) complete(ctx context.Context, req Request) (*Response, error) { - if g.cfg.APIKey == "" { - return nil, &Error{ - Code: CodeNotConfigured, - Message: "no model credentials are configured for this deployment", - } - } - if err := req.Validate(); err != nil { - return nil, err - } - - params, err := g.params(req) - if err != nil { - return nil, err - } - - msg, err := g.client.Messages.New(ctx, params) - if err != nil { - return nil, translate(err) - } - return g.decode(msg, req) -} - -// params builds the request both paths send. -// -// Extracted so the streaming and non-streaming calls cannot drift. They send -// the same model, the same thinking config, the same cache breakpoint and the -// same tools — an answer that differs depending on whether it was streamed -// would be the worst kind of bug to chase, because the transport is the last -// place anybody looks. -func (g *AnthropicGateway) params(req Request) (anthropic.MessageNewParams, error) { - route := g.routing(req.Tier) - - maxTokens := req.MaxOutputTokens - if maxTokens <= 0 { - maxTokens = g.cfg.MaxOutputTokens - } - - messages, err := encodeMessages(req.Messages) - if err != nil { - return anthropic.MessageNewParams{}, err - } - - params := anthropic.MessageNewParams{ - Model: anthropic.Model(route.Model), - MaxTokens: maxTokens, - Messages: messages, - // Adaptive thinking on every tier: the model decides how much to think, - // and effort sets the ceiling on that. A fixed token budget for - // reasoning is the deprecated shape and is rejected outright by the - // current models. - Thinking: anthropic.ThinkingConfigParamUnion{ - OfAdaptive: &anthropic.ThinkingConfigAdaptiveParam{}, - }, - OutputConfig: anthropic.OutputConfigParam{Effort: sdkEffort(route.Effort)}, - } - - if len(req.Tools) > 0 { - params.Tools = encodeTools(req.Tools) - } - - if s := strings.TrimSpace(req.System); s != "" { - // One cached block. The system prompt is the stable prefix of every - // turn in a run, and the render order is tools → system → messages, so - // a breakpoint here is the one that survives the conversation growing. - params.System = []anthropic.TextBlockParam{{ - Text: s, - CacheControl: anthropic.NewCacheControlEphemeralParam(), - }} - } - - return params, nil -} - -// decode turns a finished message into a Response. -// -// Shared by both paths for the same reason params() is: a streamed message and -// a non-streamed one are the same object by the time they get here, and reading -// them differently would make streaming a second implementation of the answer. -func (g *AnthropicGateway) decode(msg *anthropic.Message, req Request) (*Response, error) { - route := g.routing(req.Tier) - - usage := Usage{ - InputTokens: msg.Usage.InputTokens, - OutputTokens: msg.Usage.OutputTokens, - CacheReadTokens: msg.Usage.CacheReadInputTokens, - CacheCreationTokens: msg.Usage.CacheCreationInputTokens, - } - - // A refusal arrives as a successful HTTP response, so it has to be checked - // before the content is read. It is still billed, and the usage is carried - // on the error so the run's budget is charged for a turn that produced no - // text — a refusal that cost nothing on the ledger is a refusal the loop - // would happily repeat. - if msg.StopReason == anthropic.StopReasonRefusal { - return &Response{ - StopReason: string(msg.StopReason), - Usage: usage, - Model: route.Model, - Tier: req.Tier, - }, &Error{ - Code: CodeRefused, - Message: "the model declined this request", - Category: string(msg.StopDetails.Category), - } - } - - var ( - text strings.Builder - calls []ToolCall - ) - for _, block := range msg.Content { - switch b := block.AsAny().(type) { - case anthropic.TextBlock: - text.WriteString(b.Text) - case anthropic.ToolUseBlock: - // The raw JSON, not a parsed value: current models vary their - // string escaping inside tool inputs, so this is handed to the - // handler's own decoder rather than matched on as a string here. - calls = append(calls, ToolCall{ - ID: b.ID, - Name: b.Name, - Input: json.RawMessage(b.JSON.Input.Raw()), - }) - } - } - - return &Response{ - Text: text.String(), - ToolCalls: calls, - StopReason: string(msg.StopReason), - Usage: usage, - Model: route.Model, - Tier: req.Tier, - }, nil -} - -// encodeTools renders the tool definitions for the wire. -func encodeTools(defs []ToolDef) []anthropic.ToolUnionParam { - out := make([]anthropic.ToolUnionParam, 0, len(defs)) - for _, d := range defs { - schema := anthropic.ToolInputSchemaParam{} - if props, ok := d.InputSchema["properties"].(map[string]any); ok { - schema.Properties = props - } - if req, ok := d.InputSchema["required"].([]string); ok { - schema.Required = req - } - tool := anthropic.ToolParam{ - Name: d.Name, - Description: anthropic.String(d.Description), - InputSchema: schema, - } - out = append(out, anthropic.ToolUnionParam{OfTool: &tool}) - } - return out -} - -// encodeMessages renders a conversation for the wire. -// -// Tool results are variadic within ONE user message. Splitting them across -// several messages is accepted by the API and quietly teaches the model to stop -// making parallel calls — a performance regression with no error to trace it -// to, so the grouping is done here rather than left to callers. -func encodeMessages(msgs []Message) ([]anthropic.MessageParam, error) { - out := make([]anthropic.MessageParam, 0, len(msgs)) - - for i, m := range msgs { - var blocks []anthropic.ContentBlockParamUnion - - if s := strings.TrimSpace(m.Text); s != "" { - blocks = append(blocks, anthropic.NewTextBlock(m.Text)) - } - for _, c := range m.ToolCalls { - var input any - if len(c.Input) > 0 { - if err := json.Unmarshal(c.Input, &input); err != nil { - return nil, &Error{ - Code: CodeInvalidRequest, - Message: fmt.Sprintf("messages[%d]: tool call %s carries invalid JSON", i, c.Name), - } - } - } - blocks = append(blocks, anthropic.NewToolUseBlock(c.ID, input, c.Name)) - } - for _, r := range m.ToolResults { - blocks = append(blocks, anthropic.NewToolResultBlock(r.CallID, r.Content, r.IsError)) - } - - if len(blocks) == 0 { - continue - } - if m.Role == RoleAssistant { - out = append(out, anthropic.NewAssistantMessage(blocks...)) - continue - } - out = append(out, anthropic.NewUserMessage(blocks...)) - } - return out, nil -} - -// translate turns an SDK error into one the runtime can branch on. -// -// A single broad class would lose the distinction the loop actually needs: -// whether sending the same request again could work. So the status is read and -// mapped, and anything unrecognised stays CodeUpstream with its status intact -// rather than being flattened into a generic failure. -func translate(err error) error { - if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { - return &Error{Code: CodeTimeout, Message: "the model call did not complete in time", Cause: err} - } - - var apierr *anthropic.Error - if !errors.As(err, &apierr) { - return &Error{Code: CodeUpstream, Message: "the model call failed", Cause: err} - } - - // The upstream reason, carried through when there is one — the same rule the - // OpenAI path already follows, and for the same reason it gives: a 400 that - // says which model id or tool schema was rejected is worth more than "the - // model rejected the request", and the trajectory records only the message. - // - // This was not a hypothetical. Every run on a deployment failed with a bare - // `gateway.invalid_request`, and neither the run detail nor anything a - // client could read said why — the one fact needed to fix it was discarded - // here, three lines from where it arrived. `Cause` keeps the full error for - // a Go caller; nothing reads it by the time a run is persisted. - detail := anthropicErrorMessage(apierr.RawJSON()) - withDetail := func(base string) string { - if detail == "" { - return base - } - return base + ": " + detail - } - - switch apierr.StatusCode { - case 400: - return &Error{Code: CodeInvalidRequest, Message: withDetail("the model rejected the request"), Status: 400, Cause: err} - case 401, 403: - return &Error{Code: CodeUnauthorized, Message: withDetail("the model credentials were refused"), Status: apierr.StatusCode, Cause: err} - case 408: - return &Error{Code: CodeTimeout, Message: withDetail("the model call timed out"), Status: 408, Cause: err} - case 429: - return &Error{Code: CodeRateLimited, Message: withDetail("the model is rate limiting this deployment"), Status: 429, Cause: err} - case 529: - // Anthropic's "overloaded" — the service is up and temporarily out of - // capacity. Named separately from the 500s because it is the one that - // actually happens, and because a run dying on it is a run that would - // have succeeded a second later. - return &Error{Code: CodeUpstream, Message: "the model is temporarily overloaded", - Status: 529, Cause: err} - default: - // The status is IN the message, not only in the field. It cost an hour - // of debugging to learn that "the model call failed" was a 529 rather - // than a malformed tool schema, and the trajectory only records the - // message. - return &Error{ - Code: CodeUpstream, - Message: withDetail(fmt.Sprintf("the model call failed (http %d)", apierr.StatusCode)), - Status: apierr.StatusCode, Cause: err, - } - } -} - -// anthropicErrorMessage digs the human-readable reason out of an error body. -// -// Best-effort, exactly like its OpenAI counterpart: the envelope is documented -// and stable enough to be worth reading, and an unparseable body yields nothing -// rather than failing a failure. -func anthropicErrorMessage(raw string) string { - if raw == "" { - return "" - } - var envelope struct { - Error struct { - Message string `json:"message"` - Type string `json:"type"` - } `json:"error"` - } - if err := json.Unmarshal([]byte(raw), &envelope); err != nil { - return "" - } - if envelope.Error.Message != "" { - return envelope.Error.Message - } - return envelope.Error.Type -} - -/* ── Streaming ──────────────────────────────────────────────────────────── */ - -// Stream is Complete, with the assistant's text delivered as it arrives. -// -// §6: "Stream partial assistant text as it arrives; buffer tool calls until -// complete." Both halves of that matter and they pull in opposite directions. -// -// TEXT IS STREAMED because a fifteen-second wait with nothing on screen reads -// as broken. The reader wants the first sentence while the rest is still being -// written, and that is the whole difference between a product and a spinner. -// -// TOOL CALLS ARE NOT. A tool call arrives as JSON assembled character by -// character across many events, and a half-built argument object is not a -// smaller version of the finished one — it is a different object, usually an -// invalid one. Dispatching on a partial call would run a tool with arguments -// the model had not finished choosing. So the accumulated message is decoded -// only once the stream closes, by exactly the same code the non-streaming path -// uses. -// -// onDelta is called from this goroutine, in order, and must not block for long -// — it is on the path between the model and the reader. -func (g *AnthropicGateway) Stream(ctx context.Context, req Request, onDelta func(string)) (*Response, error) { - if g.cfg.APIKey == "" { - return nil, &Error{ - Code: CodeNotConfigured, - Message: "no model credentials are configured for this deployment", - } - } - if err := req.Validate(); err != nil { - return nil, err - } - - params, err := g.params(req) - if err != nil { - return nil, err - } - - stream := g.client.Messages.NewStreaming(ctx, params) - defer stream.Close() - - var msg anthropic.Message - for stream.Next() { - event := stream.Current() - if err := msg.Accumulate(event); err != nil { - return nil, &Error{ - Code: CodeUpstream, - Message: "the streamed response could not be assembled", - Cause: err, - } - } - - // Text only. A thinking delta is the model's private reasoning and is - // not the answer; a tool-input delta is a fragment of JSON. Neither is - // something to put in front of a reader. - if event.Type == "content_block_delta" && event.Delta.Type == "text_delta" { - if d := event.Delta.Text; d != "" && onDelta != nil { - onDelta(d) - } - } - } - if err := stream.Err(); err != nil { - return nil, translate(err) - } - - return g.decode(&msg, req) -} - -// StreamComplete runs a request through whichever path the gateway supports. -// -// A gateway that cannot stream is not a broken gateway — every fake in the test -// suite is one, and so is any future provider without a streaming API. Falling -// back to Complete and delivering the finished text as a single delta keeps the -// caller's code identical either way, which is what stops streaming from -// becoming a second code path through the loop. -func StreamComplete(ctx context.Context, gw Gateway, req Request, onDelta func(string)) (*Response, error) { - // Normalised once, here, so no implementation has to guard it. A caller - // that does not want deltas passes nil — every eval and every test does — - // and an implementation that took that literally would panic on the first - // fragment. Making each Streamer remember the check is how one of them - // eventually forgets. - if onDelta == nil { - onDelta = func(string) {} - } - if s, ok := gw.(Streamer); ok { - return s.Stream(ctx, req, onDelta) - } - resp, err := gw.Complete(ctx, req) - if err == nil && resp != nil && resp.Text != "" && onDelta != nil { - onDelta(resp.Text) - } - return resp, err -} diff --git a/go-api/internal/gateway/anthropic_error_test.go b/go-api/internal/gateway/anthropic_error_test.go deleted file mode 100644 index 3bd6c11..0000000 --- a/go-api/internal/gateway/anthropic_error_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package gateway - -import "testing" - -// The reason a 400 gives is the whole diagnosis, and it used to be thrown away. -// -// A deployment answered every single run with "The agent could not finish — -// something it needed did not answer." Retrieval worked, the run was created, -// the provider was reached, and the trajectory recorded `gateway.invalid_request` -// and nothing else. The actual cause was one sentence long and Anthropic had -// sent it: the account was out of credit. Nothing a client, a log or a run -// detail could show said so, because `translate` dropped the body. -// -// The envelope below is the real one, copied from that response. -func TestAnthropicErrorMessage(t *testing.T) { - cases := []struct { - name string - raw string - want string - }{ - { - name: "the outage this test exists for", - raw: `{"type":"error","error":{"type":"invalid_request_error",` + - `"message":"Your credit balance is too low to access the Anthropic API. ` + - `Please go to Plans & Billing to upgrade or purchase credits."},` + - `"request_id":"req_011CeiJqr8rTdpbZYhcL6VHG"}`, - want: "Your credit balance is too low to access the Anthropic API. " + - "Please go to Plans & Billing to upgrade or purchase credits.", - }, - { - name: "a message-less envelope falls back to the type", - raw: `{"type":"error","error":{"type":"overloaded_error"}}`, - want: "overloaded_error", - }, - // Best-effort by design: a failure to read a failure must not become a - // second failure. - {name: "unparseable body yields nothing", raw: `not json at all`, want: ""}, - {name: "empty body yields nothing", raw: ``, want: ""}, - } - - for _, c := range cases { - if got := anthropicErrorMessage(c.raw); got != c.want { - t.Errorf("%s: anthropicErrorMessage() = %q, want %q", c.name, got, c.want) - } - } -} diff --git a/go-api/internal/gateway/gateway.go b/go-api/internal/gateway/gateway.go index 92ff494..9366a3b 100644 --- a/go-api/internal/gateway/gateway.go +++ b/go-api/internal/gateway/gateway.go @@ -292,3 +292,29 @@ func (r Request) Validate() error { } return nil } + +// StreamComplete runs a request through whichever path the gateway supports. +// +// A gateway that cannot stream is not a broken gateway — every fake in the test +// suite is one, and so is any future provider without a streaming API. Falling +// back to Complete and delivering the finished text as a single delta keeps the +// caller's code identical either way, which is what stops streaming from +// becoming a second code path through the loop. +func StreamComplete(ctx context.Context, gw Gateway, req Request, onDelta func(string)) (*Response, error) { + // Normalised once, here, so no implementation has to guard it. A caller + // that does not want deltas passes nil — every eval and every test does — + // and an implementation that took that literally would panic on the first + // fragment. Making each Streamer remember the check is how one of them + // eventually forgets. + if onDelta == nil { + onDelta = func(string) {} + } + if s, ok := gw.(Streamer); ok { + return s.Stream(ctx, req, onDelta) + } + resp, err := gw.Complete(ctx, req) + if err == nil && resp != nil && resp.Text != "" && onDelta != nil { + onDelta(resp.Text) + } + return resp, err +} diff --git a/go-api/internal/gateway/gateway_test.go b/go-api/internal/gateway/gateway_test.go index 3dc3877..83bfcbc 100644 --- a/go-api/internal/gateway/gateway_test.go +++ b/go-api/internal/gateway/gateway_test.go @@ -6,8 +6,6 @@ import ( "strings" "testing" - "github.com/anthropics/anthropic-sdk-go" - "github.com/krow/krow-backend/go-api/internal/config" ) @@ -71,7 +69,7 @@ func TestRequestValidate(t *testing.T) { func TestCompleteWithoutCredentialsIsStructured(t *testing.T) { // The service boots without a key on purpose. The failure has to arrive as // something a run can terminate with, not as a panic or a bare string. - g := NewAnthropic(Config{}) + g := NewOpenAI(Config{}) _, err := g.Complete(context.Background(), Request{ Messages: []Message{{Role: RoleUser, Text: "anything"}}, }) @@ -127,25 +125,29 @@ func TestFromConfigPinsEffortPerTier(t *testing.T) { } } -// The neutral effort vocabulary has to land on the vendor's own enum, and that -// mapping is the one thing FromConfig can no longer assert now that its result -// is provider-independent. Untested, a renamed SDK constant would silently +// The neutral effort vocabulary still has to land on a vendor's own spelling, +// and that mapping is the one thing FromConfig cannot assert now that its +// result is provider-independent. Untested, a renamed constant would silently // route every tier to whatever the default arm returns. -func TestSDKEffortMapsToAnthropic(t *testing.T) { - cases := map[Effort]anthropic.OutputConfigEffort{ - EffortLow: anthropic.OutputConfigEffortLow, - EffortHigh: anthropic.OutputConfigEffortHigh, - EffortXhigh: anthropic.OutputConfigEffortXhigh, +// +// This asserts POSITIONS, not words. OpenAI's scale runs minimal/low/medium/ +// high against our low/high/xhigh, so `high` here is their "medium" — matching +// the spelling instead would collapse `fast` and `balanced` into neighbours. +func TestEffortMapsOntoTheProviderScale(t *testing.T) { + cases := map[Effort]string{ + EffortLow: "low", + EffortHigh: "medium", + EffortXhigh: "high", } for neutral, want := range cases { - if got := sdkEffort(neutral); got != want { - t.Errorf("sdkEffort(%q) = %q, want %q", neutral, got, want) + if got := openAIEffort(neutral); got != want { + t.Errorf("openAIEffort(%q) = %q, want %q", neutral, got, want) } } } func TestRoutingSelectsPerTier(t *testing.T) { - g := NewAnthropic(Config{ + g := NewOpenAI(Config{ Fast: Routing{Model: "m-fast"}, Balanced: Routing{Model: "m-balanced"}, Deep: Routing{Model: "m-deep"}, diff --git a/go-api/internal/gateway/retry.go b/go-api/internal/gateway/retry.go new file mode 100644 index 0000000..9760ea1 --- /dev/null +++ b/go-api/internal/gateway/retry.go @@ -0,0 +1,73 @@ +package gateway + +import ( + "context" + "errors" + "time" +) + +// MaxAttempts is how many times a transient failure is retried. +// +// Three total, not three retries. Past that the problem is not transient and a +// fourth call is just spending money on the same answer. +const MaxAttempts = 3 + +// retryBackoff is the pause before each retry. +// +// Short, and deliberately so: this sits inside a run that already has a +// wall-clock deadline, and a backoff long enough to be polite to the API is +// long enough to spend the caller's whole budget waiting. A run that cannot +// afford the wait dies on its deadline instead, which is the correct failure. +var retryBackoff = []time.Duration{400 * time.Millisecond, 1200 * time.Millisecond} + +// withRetry runs one attempt until it succeeds, fails terminally, or runs out +// of attempts. +// +// THE RETRY IS NOT DEFENSIVE POLISH. Error.Retryable() has existed since this +// package was written and had ZERO callers — the classification was built and +// never used, so a 529 "overloaded" killed a run that would have succeeded four +// hundred milliseconds later. Found by a real overload during live testing, +// where it presented as "the agent could not finish" with nothing to act on. +// +// Only genuinely transient failures qualify: rate limits, timeouts, and 5xx. +// A 400 is a malformed request and will be malformed again; a 401 is a bad +// credential and retrying it three times just gets refused three times. +// +// The run's context governs. A retry that would outlive the caller's deadline +// does not happen — the deadline belongs to the run, not to this function, and +// waiting past it would turn a bounded run into an unbounded one. +// +// THIS FILE EXISTS BECAUSE THE POLICY OUTLIVED ITS FIRST PROVIDER. It was +// written inside the Anthropic implementation and used by both, so deleting +// that implementation would have deleted the retry policy of the one that +// remained — silently, because nothing about `openai.go` mentions it. The +// policy is a property of this platform's runs, not of any vendor's API, so it +// now lives somewhere no provider can take with it when it goes. +func withRetry(ctx context.Context, once func() (*Response, error)) (*Response, error) { + var last error + for attempt := 0; attempt < MaxAttempts; attempt++ { + if attempt > 0 { + pause := retryBackoff[min(attempt-1, len(retryBackoff)-1)] + select { + case <-time.After(pause): + case <-ctx.Done(): + // Out of time. The ORIGINAL failure is returned rather than the + // context error: "the model was overloaded" is what an operator + // needs to see, and "context deadline exceeded" would hide it. + return nil, last + } + } + + resp, err := once() + if err == nil { + return resp, nil + } + last = err + + var gwErr *Error + if !errors.As(err, &gwErr) || !gwErr.Retryable() { + return resp, err + } + } + return nil, last +} diff --git a/go-api/internal/gateway/routing.go b/go-api/internal/gateway/routing.go index 7b4a2a1..8cd3a28 100644 --- a/go-api/internal/gateway/routing.go +++ b/go-api/internal/gateway/routing.go @@ -4,26 +4,28 @@ import ( "github.com/krow/krow-backend/go-api/internal/config" ) -// Provider names the wire protocol a deployment talks. +// ProviderOpenAI names the only wire protocol this platform speaks. // -// Two, not two hundred: "anthropic" is the Claude API, and "openai" is the -// chat-completions shape that Groq, Gemini, OpenRouter, Together, vLLM and -// Ollama all serve. That second one is the reason this constant exists at all -// — supporting those five providers is one implementation and five different -// base URLs, and pretending otherwise would grow a package per vendor. -const ( - ProviderAnthropic = "anthropic" - ProviderOpenAI = "openai" -) +// One constant, not an enum, because there is one implementation. "openai" is +// the chat-completions shape — which is NOT only OpenAI. Groq, Gemini (through +// its compatible endpoint), OpenRouter, Together, vLLM and a local Ollama all +// serve it, and the difference between them is MODEL_BASE_URL and a model id, +// nothing more. Supporting six vendors is one implementation and six base URLs. +// +// The Anthropic path was removed deliberately, not lost. `MODEL_PROVIDER=anthropic` +// is now REFUSED at startup rather than ignored — see config.validateModel. A +// deployment carrying the old value must be told it moved, because the silent +// alternative is a stack that believes it is still on Claude while every run +// goes somewhere else. +const ProviderOpenAI = "openai" // Effort is how hard a tier is allowed to think. // -// PROVIDER-NEUTRAL ON PURPOSE. This was `anthropic.OutputConfigEffort` until a -// second provider existed, which meant the vendor's enum was baked into the -// routing table that every provider has to read. Nothing was wrong with it -// while there was one implementation; it became wrong the moment there were -// two, because the OpenAI path would have had to import the Anthropic SDK to -// learn how hard to think. +// PROVIDER-NEUTRAL ON PURPOSE, and the reason that mattered is now history +// worth keeping: this was a vendor SDK's own enum, baked into the routing table +// every provider has to read. Making it the platform's own vocabulary is what +// let that vendor be removed later without the routing table going with it — +// a one-line deletion instead of a re-typing of every tier. // // The three values are the platform's own vocabulary. Each implementation maps // them onto whatever its API calls the same idea, and a provider with no such @@ -55,8 +57,8 @@ type Routing struct { // Built once at startup from the environment and passed in frozen, per §10. // Nothing in this package reads the environment itself. type Config struct { - // Provider selects the implementation. Empty means anthropic, so a - // deployment that predates the second provider keeps working untouched. + // Provider selects the implementation. Empty means openai, which is now + // the only one; config.validateModel refuses any other value. Provider string APIKey string @@ -116,21 +118,18 @@ func FromConfig(c config.ModelConfig) Config { // New builds the gateway a deployment's configuration asks for. // -// The one place that maps a provider name to an implementation, so a caller -// wires a gateway without knowing which vendor answers. An unrecognised -// provider cannot reach here — config.validate rejects it at startup, where a -// typo is one loud failure instead of one per run. +// One provider, so this is a constructor rather than a choice. It survives the +// removal of the second implementation because the runtime wires itself through +// `gateway.New(gateway.FromConfig(...))` and should not learn a concrete type: +// the next provider is a change here and nowhere else. func New(cfg Config) Gateway { - if cfg.Provider == ProviderOpenAI { - return NewOpenAI(cfg) - } - return NewAnthropic(cfg) + return NewOpenAI(cfg) } // routingFor resolves a tier against a table. // -// Shared by both implementations: an unknown tier has already been normalised -// by ParseTier, so the default arm is reached only by a zero value. +// An unknown tier has already been normalised by ParseTier, so the default arm +// is reached only by a zero value. func (c Config) routingFor(t Tier) Routing { switch t { case TierFast: diff --git a/go-api/internal/runtime/store_test.go b/go-api/internal/runtime/store_test.go index 56e4c2c..e67c290 100644 --- a/go-api/internal/runtime/store_test.go +++ b/go-api/internal/runtime/store_test.go @@ -19,7 +19,7 @@ func trajectory(orgID, runID string) *runtime.Trajectory { AgentID: "activity-agent", AgentVersion: 3, Tier: "balanced", - Model: "claude-opus-5", + Model: "llama-3.3-70b-versatile", StartedAt: started, EndedAt: started.Add(1200 * time.Millisecond), Termination: runtime.TerminationCompleted, @@ -64,8 +64,8 @@ func TestPostgresSinkSavesAndReadsBack(t *testing.T) { } // Both the tier asked for and the model that answered, so a trajectory read // a year later does not require knowing that week's routing. - if tier != "balanced" || model != "claude-opus-5" { - t.Errorf("tier/model = %q/%q, want balanced/claude-opus-5", tier, model) + if tier != "balanced" || model != "llama-3.3-70b-versatile" { + t.Errorf("tier/model = %q/%q, want balanced/llama-3.3-70b-versatile", tier, model) } if total != 1020 || modelCalls != 1 { t.Errorf("usage = %d tokens over %d calls, want 1020/1", total, modelCalls) diff --git a/infrastructure/docker-compose.yml b/infrastructure/docker-compose.yml index 88e8803..f25aa5e 100644 --- a/infrastructure/docker-compose.yml +++ b/infrastructure/docker-compose.yml @@ -92,8 +92,13 @@ services: # without it answers 404 on /agents/{id}/runs and reports three fewer # endpoints on /version. Empty by default: absent is a working API # without Owliver, which is a legitimate way to run this. - # Provider selection. Empty MODEL_PROVIDER means anthropic, so a stack - # that predates the second provider comes up exactly as it did. + # Provider selection. Empty MODEL_PROVIDER means openai — the only + # implementation — and empty MODEL_BASE_URL means Groq. + # + # ANTHROPIC_API_KEY is passed through DELIBERATELY even though nothing + # reads it: a host that still exports it gets a loud startup failure + # telling it to rename the variable, instead of a container that silently + # boots with no credential and fails every agent run. MODEL_PROVIDER: ${MODEL_PROVIDER:-} MODEL_BASE_URL: ${MODEL_BASE_URL:-} MODEL_API_KEY: ${MODEL_API_KEY:-}