feat: add gpt-5.6 models and generate the known-models catalog from models.dev - #27146
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| "model": "claude-3-5-haiku-20241022", | ||
| "input_price": 800000, | ||
| "output_price": 4000000, | ||
| "cache_read_price": 80000, | ||
| "cache_write_price": 1000000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-5-haiku-latest", | ||
| "input_price": 800000, | ||
| "output_price": 4000000, | ||
| "cache_read_price": 80000, | ||
| "cache_write_price": 1000000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-5-sonnet-20240620", | ||
| "input_price": 3000000, | ||
| "output_price": 15000000, | ||
| "cache_read_price": 300000, | ||
| "cache_write_price": 3750000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-5-sonnet-20241022", | ||
| "input_price": 3000000, | ||
| "output_price": 15000000, | ||
| "cache_read_price": 300000, | ||
| "cache_write_price": 3750000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-7-sonnet-20250219", | ||
| "input_price": 3000000, | ||
| "output_price": 15000000, | ||
| "cache_read_price": 300000, | ||
| "cache_write_price": 3750000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-haiku-20240307", | ||
| "input_price": 250000, | ||
| "output_price": 1250000, | ||
| "cache_read_price": 30000, | ||
| "cache_write_price": 300000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-opus-20240229", | ||
| "input_price": 15000000, | ||
| "output_price": 75000000, | ||
| "cache_read_price": 1500000, | ||
| "cache_write_price": 18750000 | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model": "claude-3-sonnet-20240229", | ||
| "input_price": 3000000, | ||
| "output_price": 15000000, | ||
| "cache_read_price": 300000, | ||
| "cache_write_price": 300000 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.9.0 | Round 2 | Last posted: Round 2, 12 findings (2 P2, 4 P3, 3 Nit, 3 Note), COMMENT. Review Finding inventoryFindings
Law analysisEffective LOC: +1066 -608 (16 files). Head SHA: 67f983f. Verdict: Don't split. Enforcement: N/A. Contested and acknowledged(none) Round logRound 1Panel (19 reviewers). 3 P3, 1 Note, 4 Nit new. 4 dropped. Reviewed against 9af3d31..67f983f. Round 2 updateChurn guard PROCEED: CRF-1..CRF-7 all Author fixed (db1391). Panel re-review pending. Reviewed against 9af3d31..db13913. Round 2Panel (11 reviewers) + Netero (advisory, no findings). 2 P2, 1 P3, 2 Note new. CRF-1..CRF-7 verified fixed. CRF-13/CRF-14 (tiered pricing) verified against live models.dev by orchestrator. Reviewed against 9af3d31..db13913. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
/coder-agents-review |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Deep review round 2 (11 panel reviewers + Netero).
All seven round-1 findings (CRF-1..CRF-7) are genuinely fixed in db1391, verified against the code and tests, not just the labels: the overrides.jq mythos guard now errors on both drift directions, the golden test exercises the DisplayName-override and thinkingBudgetTokens paths, buildCatalog fails closed on nil upstream limits, the error table gained MissingUpstreamLimit and EmptyUpstreamName, the stale last_updated comment is gone, the two "missing from upstream" cases assert distinct substrings, and the package doc covers both formats. Clean work.
Round-2 findings: 2 P2, 1 P3, 2 Notes across 5 inline comments.
The two P2s are a linked pair about tiered pricing, and I verified both against a live models.dev fetch. Upstream ships context_over_200k / tiers blocks for the OpenAI 1M-context models (gpt-5.6-sol base input 5 / output 30, over-200k input 10 / output 45) and for none of the Anthropic models. So the handling is backwards: the model that gets the context pin (sonnet-4-5) has no tiered pricing and the pin only makes its UI badge understate a 1M window, while the models that do have tiered pricing (the gpt-5.x family) go unpinned and their price seed silently records base-tier only. Both need a human decision, not a silent default.
The P3 (case-sensitive guard vs case-insensitive consumer) converged from two reviewers independently.
Process notes, none blocking: the buildCatalog validation loop reports errors while ranging a map, so with errors under two providers the surfaced message is nondeterministic (Komugi, developer-ergonomics only). The knownModelsGenerated.test.ts it.each assertions omit the model identifier from failure messages, so a failure names the provider but not the model (Chopper). Several applyKnownModelDefaults logic tests read real catalog entries rather than synthetic fixtures, so a future regeneration can break them for reasons unrelated to the logic (Bisky). And the gpt-5.6 alias on gpt-5.6-sol shadows a distinct bare gpt-5.6 model that also exists upstream; benign today because their pricing is identical, but the alias guard only checks curated identifiers, not the upstream model list (Pariston).
"You built a cage of invariants and dared the input to escape. Most of it holds." (Hisoka)
This review was generated by Coder Agents.
scripts/aibridgepricesgen/main.go:181
P2 [CRF-14] The price seed silently drops context-tiered pricing for the OpenAI 1M-context models, so budgets under-record large-context usage. upstreamCost parses only flat input/output/cache_read/cache_write, and convert writes those base rates into prices.json.
Verified against a live models.dev fetch: gpt-5.6-sol, gpt-5.6-terra, gpt-5.6-luna, gpt-5.5, gpt-5.5-pro, and gpt-5.4 all carry a context_over_200k block (and a tiers entry at tier.size: 272000) where prices roughly double above 200k. gpt-5.6-sol is input:5, output:30 at base and input:10, output:45 above 200k, but the seed records the base tier only (input_price: 5000000), while the same model advertises contextLimit: 1050000.
This lands on the exact feature the PR exists to fix: for any request whose context exceeds the tier boundary, the cost-control loader records well under half the real input spend, so budgets run too lax for large-context OpenAI usage. Flat-only pricing is a documented Coder constraint for the catalog display (catalog.go comment), but the price-seed path has no equivalent acknowledgement and no test pins the behavior. A human should decide whether flat-only seed pricing is acceptable for these tiered models and, if so, document it where the seed is generated. (Mafuuu)
🤖
🤖 This review was automatically generated with Coder Agents.
| } | ||
| // An alias resolving to a canonical identifier would make exact-alias | ||
| // lookup and canonical-id lookup disagree. | ||
| for alias := range seenAliases { |
There was a problem hiding this comment.
P3 [CRF-15] The uniqueness and shadow guards compare identifiers and aliases with case-sensitive equality, but the frontend resolves aliases case-insensitively, so the invariant the guard exists to protect can still be violated.
The comment states the invariant: "An alias resolving to a canonical identifier would make exact-alias lookup and canonical-id lookup disagree." But the shadow check (seenIdentifiers[alias]) and the alias dedup (seenAliases[alias]) both key on raw curation strings. The consumers disagree on case: findKnownModelByExactAlias compares alias.toLowerCase() === value.toLowerCase() (index.ts), while findKnownModelByCanonicalId compares modelIdentifier === modelId (case-sensitive), and ModelIdentifierField runs canonical-id first, then falls through to exact-alias.
Verified in index.ts. So an alias "Claude-Sonnet-4-5" on model A alongside a modelIdentifier claude-sonnet-4-5 on model B passes every guard (the strings differ by case), yet at runtime findKnownModelByExactAlias("claude-sonnet-4-5") matches A while findKnownModelByCanonicalId("claude-sonnet-4-5") matches B: exactly the disagreement the guard claims to prevent. The same gap lets two aliases differing only in case survive dedup and resolve ambiguously.
Latent today because all curation entries are lowercase, but the guard's whole purpose is to make the bad state impossible for the next curation author. Lowercasing the keys in seenIdentifiers, seenAliases, and the post-loop cross-check matches the generator's uniqueness domain to the consumer's lookup domain. (Hisoka P3, Meruem P3)
🤖
| if !m.Cost.hasPricing() { | ||
| return nil, xerrors.Errorf("%s/%s: upstream model has no cost block", providerID, c.ModelIdentifier) | ||
| } | ||
| if m.Limit.Context == nil || m.Limit.Output == nil { |
There was a problem hiding this comment.
Note [CRF-16] The new limit check rejects nil context/output but accepts non-positive ones. A present-but-zero upstream limit (pointer to 0) passes, and because catalogEntry's *int64 fields use omitempty (which drops nil, not zero), the generator emits "contextLimit": 0. That value is semantically invalid: formatContextBadge throws on contextLimit <= 0 (verified in index.ts), and knownModelsGenerated.test.ts asserts every limit is > 0. CI would catch a zero-limit artifact before merge, so the consequence is bounded, but the generator that owns the join could enforce the invariant it produces by tightening the check to *m.Limit.Context <= 0 || *m.Limit.Output <= 0. (Meruem)
🤖
| if !ok { | ||
| return nil, xerrors.Errorf("%s/%s: model missing from upstream (patch it in via overrides.jq if intentional)", providerID, c.ModelIdentifier) | ||
| } | ||
| if !m.Cost.hasPricing() { |
There was a problem hiding this comment.
Note [CRF-17] buildCatalog requires cost with OR (hasPricing accepts any one of input/output/cache_read/cache_write) but limits with AND (both context and output must be present). A curated model whose upstream cost block carries only cache_read passes validation and emits a catalog entry with inputCost/outputCost omitted. TestBuildCatalog covers the reverse (input+output, no cache) but nothing covers cost-present-yet-input/output-absent. Blast radius is small: this feeds only the frontend pre-fill, not the prices.json budget seed (separate convert path), and curated models always carry input/output pricing today. If the asymmetry is deliberate (costs are optional pre-fills, limits drive badges), a one-line comment would keep the next reader from "fixing" it; otherwise an error-table row asserting a missing input/output price would close it. (Bisky)
🤖
ssncferreira
left a comment
There was a problem hiding this comment.
LGTM from AI Gateway cost control side of things, just a few suggestions and comment
| # claude-sonnet-4-5: upstream advertises a 1M-token context window, which | ||
| # implies tiered context_over_200k pricing. Coder persists flat pricing only, | ||
| # so pin the context limit to the flat-priced 200k tier. | ||
| if .anthropic.models | has("claude-sonnet-4-5") then |
There was a problem hiding this comment.
I'm slightly confused about this comment, if I run:
> curl -fsSL https://models.dev/api.json | jq '.anthropic.models."claude-sonnet-4-5"'
{
"id": "claude-sonnet-4-5",
"name": "Claude Sonnet 4.5 (latest)",
"description": "Balanced Claude model for coding, analysis, agent workflows, and cost control",
"family": "claude-sonnet",
"attachment": true,
"reasoning": true,
"reasoning_options": [
{
"type": "budget_tokens",
"min": 1024
}
],
"tool_call": true,
"structured_output": true,
"temperature": true,
"knowledge": "2025-07-31",
"release_date": "2025-09-29",
"last_updated": "2025-09-29",
"modalities": {
"input": [
"text",
"image",
"pdf"
],
"output": [
"text"
]
},
"open_weights": false,
"limit": {
"context": 1000000,
"output": 64000
},
"cost": {
"input": 3,
"output": 15,
"cache_read": 0.3,
"cache_write": 3.75
}
}
So, based on this response, I would assume the cost object (what gets stored in ai_model_prices) is for a context limit of up to a 1M-token context window... but for catalog, we are setting a limit of 200k?
Edit: I just realized this is related to the comment below
There was a problem hiding this comment.
Yep, there was an email on May 1st about this but models.dev doesn't seem to have updated.
Follow-up to #27132 (review thread): the gpt-5.6 models routed correctly but had no price rows, so budgets recorded their usage at NULL cost, and no catalog entries, so they never appeared in the add-model UI.
Changes
prices.jsonfrom models.dev. Adds the gpt-5.6 family and other new models. Delisted rows drop out; the seeder is upsert-only, so existing deployments keep them.make gen/aibridge-pricesfetches models.dev once, appliesoverrides.jq(guarded factual corrections:claude-sonnet-4-5context pinned to the flat-priced 200k tier;claude-mythos-5injected as aclaude-fable-5copy since upstream omits it), and emits both artifacts from that snapshot. Editorial choices (selection, order, aliases, reasoning defaults) live incuration.jsonand are validated at generation time. Deletes the hand-writtenopenai.tsandanthropic.ts.Adding a model is now one
curation.jsonentry plusmake gen/aibridge-prices.Note
Upstream changed
gpt-3.5-turbocache_read_price from a bogus 1250000 to 0. The model does not support prompt caching (as far as the resources consulted said), so no cache-read tokens ever accrue and both values are inert.