Phase 2.6.Q — models.dev as the model-metadata source (ADR-0071) + DB-backed catalog (ADR-0072)#76
Open
cemililik wants to merge 63 commits into
Open
Phase 2.6.Q — models.dev as the model-metadata source (ADR-0071) + DB-backed catalog (ADR-0072)#76cemililik wants to merge 63 commits into
cemililik wants to merge 63 commits into
Conversation
…db coupling it exposed Marks 2.6.C done in current.md, the phase plan, and CLAUDE.md (which still called Phase 2.6 "next up" while two of its workstreams were already on `main`). Files one finding that had nowhere to live. `regression.e2e.test.ts` drives the real CLI shell without a `--db`, so it resolves the DEFAULT path and opens — and MIGRATES — the developer's real `~/.relavium/history.db`. Verified rather than assumed: running that file with HOME pointed at an empty directory creates `$HOME/.relavium/history.db` with all 11 migrations applied. The sibling test at :315 already does it correctly (mkdtemp + explicit dbPath), so the isolation exists; this path just does not use it. It is worth fixing because it already bit, in this very workstream: a migration re-cut during development changes its journal timestamp, drizzle replays such a migration, and `CREATE TABLE session_costs` ran against the table it had itself created. CI could not see it — a fresh runner has no history.db, so nothing had been applied and nothing could conflict. The failure surfaced only against the maintainer's 3.3 MB database of real sessions. A suite that writes to real user data both damages it and blinds CI to the damage. Also records the cross-turn tool-call memory spin-off in current.md's open obligations, with the three already-live bugs that gate it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he hand-typed registry is retired
The 12-row `MODEL_PRICING` table cannot say what is true, and that is not an inconvenience — it is three live
failures with one root:
• It prices 12 of ~97 reachable models, so ADR-0028's cost cap — a SAFETY control — is silently skipped for the
rest. `gpt-5.4-pro`, the model the maintainer actually hit, is not in it.
• Reasoning capability is a `boolean`, and the truth is not. `gpt-5.4-pro` accepts {medium, high, xhigh} and
REJECTS `low`; `gpt-5-pro` accepts only {high}. A boolean cannot say that, so the picker offers all five tiers
and the provider 400s.
• Worst: the control's SHAPE is per-model, and we get it wrong TODAY. ADR-0066 assumed one native shape per
ADAPTER ("Gemini → thinking-level") and filed the token-budget shape as "legacy". Google's docs for the
generateContent API we call: "Gemini 2.5 series models don't support thinkingLevel; use thinkingBudget
instead" — and gemini-2.5-pro cannot disable thinking at all. Those two models are our ONLY Gemini rows. The
shape ADR-0066 called legacy is the shape of 100% of the Gemini models we ship.
models.dev was verified, not trusted: all 12 of our hand-checked prices match it EXACTLY (including 0.435/0.87 and
0.75/4.5), and its gemini-2.5-pro budget range {min:128, max:32768} matches Google's own documentation — on the very
model where our code is broken.
The design, in one line each: a GENERATED snapshot (not hand-typed, not runtime-only) ships in the binary, so the
cap works offline on day 0 — a cap that needs a third-party host reachable is not a safety control. The refresh is
opt-in DEFAULT-OFF (ADR-0068's convention), additive-only, and can never lower the floor the binary shipped with.
Accepted tiers are COMPUTED from the wire values, never copied — a literal read drops `off` from every Claude model.
And one `CATALOG_PROVIDER_KEYS` table drives everything, so adding a provider tomorrow is one line, not a rewrite.
Folds a review pass that was right about the substance:
• Recorded the fifth outbound path in security-review.md's closed inventory — and drew the distinction it exists
for: the four SSRF paths are dangerous because SOMETHING ELSE picks the URL. This one's host is a compile-time
constant. Made non-constant, it moves into the four and needs the primitive.
• Named what the retirement BREAKS up front (`contextWindowForModel`, ADR-0062's compaction path) rather than
discovering it mid-implementation.
• Narrowed ADR-0065's precedence to `user > catalog`, justified by ADR-0065's OWN custom base_url feature: on a
proxy the public list price is simply wrong. Its real intent ("not SILENTLY") is kept by making the divergence
visible, not by overruling the user.
• Dated amendment notes on ADR-0064 (two clauses superseded), ADR-0065 (precedence), ADR-0066 (a FALSE premise,
quoted), plus the missing ADR-0070 back-reference and the decisions/README index row.
Updated for the next provider: the add-llm-adapter skill's "hand-write a pricing entry" step becomes "add one line
to CATALOG_PROVIDER_KEYS and review the generated diff", and add-a-provider.md gains the two-axis model
(availability from your key, metadata from the snapshot) and the two-axis `models refresh`.
Refs: ADR-0071, ADR-0064, ADR-0065, ADR-0066, ADR-0070, ADR-0028
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… the max_completion_tokens rule
Auditing the plan against the Accepted ADR before writing code found a real gap, and it was mine: rewriting the
ADR to fold the review, I DROPPED its "what the catalog does NOT own" section. So the plan committed to fixing
`max_tokens` vs `max_completion_tokens` — the second half of the maintainer's "max tokens errors" report — while
the ADR said nothing about it at all.
Restored as §10, and it forced a decision I had earlier got wrong. I had proposed DISCOVERING the dialect at
runtime: send the modern field, and on a 400 that names the parameter, retry once with the legacy field and cache
the result per endpoint. That is worse than a rule, and rejecting it is now recorded:
• it burns a real user turn to learn what a constant already tells us;
• it depends on string-matching a provider's 400 message, which is not a contract; and
• it introduces mutable per-endpoint state that must be persisted, invalidated, and somehow made deterministic
for tests.
The rule instead: the official OpenAI endpoint gets `max_completion_tokens` (the current field — `max_tokens` is
the deprecated one that reasoning models reject); DeepSeek and every custom `base_url` keep `max_tokens`, which is
the field every OpenAI-compatible server implements. A per-provider config key covers an exotic endpoint that
wants the modern one. Knowable, testable, and it costs the user nothing.
§10 also pins the four other things the catalog does not own, so none of them surfaces as a mid-build surprise:
media output rates (`cost: null` on every image model — nothing lost, no model carries one today), deprecation
DATES (it has a flag, not a date — adopting it would LOSE information we already have), `cache_read` when absent
(`undefined`, never `0` — `0` means "no discount" and would price cached input at zero), and reachability with
your key (that is `listModels`' job; a catalog cannot know your account).
The plan doc is rewritten as an implementation plan and cross-checked against the ADR line by line. It had drifted
badly: it still said "decisions pending", still called for the cancelled Step-0 probe, still carried an archived
section contradicting its own §3, still said "zero egress" (the refresh is opt-in default-OFF, not absent), and
still said to cut D3 from scope — which I had already retracted, because D3 IS the maintainer's finding. Its
consumer list was also short: it named 15 files; the real count is 18, and the one it missed —
`session-view-model.ts` — is the 2.6.C context-fullness indicator.
Refs: ADR-0071, ADR-0065, ADR-0062
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ith the reasoning control per MODEL
Step 1 of ADR-0071: the catalog data and its generator, entirely ADDITIVE (nothing consumes it yet, so nothing
can break). What it produces is the thing a 12-row hand-typed table could never say.
The three rows this whole workstream exists for, straight out of the snapshot:
gemini-2.5-pro reasoning={"budgetTokens":{"min":128,"max":32768}} ← NO effort axis. And min:128 says,
in one field, that `off` is IMPOSSIBLE
("N/A: Cannot disable thinking").
claude-haiku-4-5 reasoning={"budgetTokens":{"min":1024}} ← NO effort axis either — yet our
adapter sends `output_config.effort`.
gpt-5.4-pro reasoning={"effortValues":["medium","high","xhigh"]} ← the maintainer's bug report, verbatim:
it REJECTS `low`, which we offer.
deepseek-reasoner reasoning={} ← reasons, but has NO controllable tier.
Distinct from "does not reason".
All 12 of our hand-verified prices survive the transform BYTE-EXACT (checked, including 0.435/0.87 and 0.75/4.5).
Two real bugs were found by building it, and both are the kind that ship silently:
**The first draft validated the whole 166-provider payload with one strict schema — and died on the first run.**
`requesty`, a provider we have no adapter for and can never call, publishes a `budget_tokens` option with no
`min`. One malformed row in data we never read killed the entire sync. Validating a third-party payload wholesale
makes you hostage to 162 providers' data quality. Now: parse only what we consume, one model at a time, so a bad
row is a dropped model — reported, never silent — instead of a dead sync.
**The CI guard was born dead.** `renderSnapshot` emitted `JSON.stringify`'s double quotes; prettier rewrote them
to single ones; `--check` then reported the snapshot STALE *even when it was current*. A weekly guard that is red
no matter what is not a guard — everyone learns to ignore it, and the price-change protection it exists to give
quietly evaporates. The generator now formats with prettier, so the comparison is apples-to-apples. Break-verified:
corrupt the snapshot and `--check` fails; leave it alone and it says "current".
Design points worth naming, because each is a money decision:
• ABSENT ≠ 0 for cache-read (19 of 97 models, incl. gpt-5.4-pro). `0` means "no discount"; writing it for an
absent rate bills cached input as FREE — a silent undercharge in the mechanism this work is hardening.
• An unpriceable model (upstream `cost: null` on image models) is DROPPED, not written at $0. A $0 row is worse
than absence: it *passes* the cost cap, where an unpriced model is flagged.
• `google-vertex` is ignored — it republishes the same Gemini ids at DIFFERENT prices, and a naive flatten would
price the user's Gemini traffic at Vertex rates.
• A dropped model is reported by its OWN id, never the record key: the sync's shipped-model guard compares that
id against the committed snapshot, so a key/id divergence would wave a real regression straight through.
• A cross-provider id collision THROWS. A generator that quietly halves its own output is exactly the failure
mode this workstream exists to end.
Provider-extensible by construction: `CATALOG_PROVIDER_KEYS` is a `Record<ProviderId, string>`, so a new provider
is a COMPILE ERROR until it is mapped, and the generator iterates `LLM_PROVIDERS` rather than a literal list.
Adding one tomorrow is a single line.
Refs: ADR-0071, ADR-0066, ADR-0028
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…an embedding was on its way to the picker An adversarial review found 23 defects, and the two worst were mine in the same place: the guards that exist to stop a third-party file from silently weakening the ADR-0028 cost cap could not, in fact, stop it. **The guard was blind to `o1` and `o3`.** It regex-matched the generated snapshot's TEXT and required a single-quoted key. Prettier's default `quoteProps: 'as-needed'` unquotes any key that is a valid JS identifier, so those two were emitted bare — the regex saw 88 of 90 models, and they had NO BASELINE AT ALL. A halved `o1` price passed in silence; the fatal "a shipped model was dropped" check could not fire for them either. This is the SECOND time the same root bit me. I had already "fixed" a prettier mismatch by formatting the generator's output — and then left a guard that reads those bytes back with assumptions prettier does not honour. The generated file's exact shape is prettier's decision, not the tool's. Any guard that reads it as text is one formatting default away from going quietly blind. So the guard now diffs the DATA: it imports the built snapshot module and compares structurally. The key's byte shape is now irrelevant, which is the only way this stays fixed. **The guard compared only the flat input/output pair.** Cache-read, cache-write and every context tier were unguarded — and the tier is the worst possible field to miss, because the pre-egress estimate takes the HIGHEST applicable tier, so on a long-context turn the TIER rate is the number that sizes the cap. Twelve models are tiered. Both are break-verified against the real tool: halve o1's flat rate (unquoted key) → guard FIRES (was silent) halve ONLY gemini-2.5-pro's >200k tier → guard FIRES (was invisible) **A shipped model that VANISHES was removed in silence.** The old check could only see models the normalizer explicitly dropped; one that simply disappeared from the payload — upstream deleted it, a provider-key edit erased its provider — appeared in no list. Now fatal, with `--accept-removals` for a deliberate one (a provider really does retire models; a guard with no escape hatch is a tool that eventually cannot run). **An embedding was on its way into the model picker.** The catalog imported `text-embedding-3-large` and 9 other non-chat models. That is not cosmetic: `keepOpenAiModelId` short-circuits on `pricedIds.has(id)` — a PRICED id bypasses the live list's deny-list entirely — so once the catalog becomes the priced set, every non-chat model in it would be RESCUED past that filter and offered to the user as something to chat with. The filter now has ONE canonical home (`model-kind.ts`), shared by the live list and the catalog. Two filters that can disagree is a cascade; one filter cannot. 90 models → 80 chat models. Also folded: the outer payload schema no longer validates all 166 providers wholesale (one malformed stub among providers we never read could still kill the sync); the SHA is now of OUR catalog, not the 3.17 MB upstream body (any byte moving in the 162 providers we discard made `--check` report STALE when nothing we ship had changed — a weekly guard that is always red is not a guard); the fetch has a timeout and surfaces `error.cause` instead of Node's bare "fetch failed"; and `tools/**` is now linted in CI, which it never was — it had 5 live errors, including a `no-restricted-syntax` hit on a computed dynamic import that the ADR-0011 seam fence is right to forbid. Fixed with a literal specifier, not a disable. Refs: ADR-0071, ADR-0064, ADR-0028, ADR-0011 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he catalog, never copied
Step 2 of ADR-0071 (§6). The catalog's `effortValues` are PROVIDER-WIRE strings; Relavium's tiers are a different
vocabulary. Reading one as the other is not a rounding error — it is actively wrong in both directions:
a literal read DROPS `off` from every Claude model (there, `off` is `thinking:{disabled}` — not an effort value)
a literal read DROPS `off` and `max` from gpt-5.5 (its wire spellings are 'none' and 'xhigh')
a literal read OFFERS `low` on gpt-5.4-pro (which rejects it — the maintainer's 400)
So `acceptedTiers(provider, controls)` composes the wire map with the model's published values. The wire maps
moved out of three adapters into one home (`reasoning-wire.ts`), because two copies of "what we send a provider"
are two chances to disagree about it.
The asymmetry that makes this necessary: `off` is an effort value on ONE of our four providers.
anthropic / deepseek `thinking: {type:'disabled'}` — an independent switch; always available
openai `reasoning_effort: 'none'` — only if 'none' is among the model's values
gemini `thinkingConfig.thinkingBudget: 0` — only if the model's budget floor IS zero
That last line is the live bug in one sentence. Google's docs say gemini-2.5-pro "cannot disable thinking"; the
catalog says `budgetTokens.min = 128`. Same fact, one field — and our adapter maps off→MINIMAL and sends it
anyway, which neither disables thinking nor is a value the model takes.
Two bugs found while writing it, both mine:
• The empty descriptor (`deepseek-reasoner` — reasons, but publishes NO control) was being offered `off`, on
DeepSeek's *general* ability to disable. That is a guess about a model whose capability upstream declined to
describe — and a guess is precisely what put a rejected value on the wire in the first place. It now offers
nothing, and the field is withheld.
• Gemini's `ThinkingLevel` enum is the UPPERCASE form of the catalog's tokens. Two maps would have drifted;
there is one map and an explicit `toGeminiThinkingLevel` at the wire.
Tested against the REAL shipped catalog, not fixtures — a bridge that is right about a fixture and wrong about
gemini-2.5-pro is worth nothing. Two invariants hold across all 80 models: no model is offered a tier whose wire
value it does not publish, and no model that cannot be disabled is ever offered `off`.
Refs: ADR-0071, ADR-0066
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…all four adapters
Step 3 of ADR-0071 (§6). This is the bug the maintainer hit, and the one the ADR-0066 correction note names: the
native reasoning shape was a property of the ADAPTER, and it is a property of the MODEL.
**Gemini.** The adapter sent `thinkingConfig.thinkingLevel` to every reasoning model. Google's docs for the
`generateContent` API we call: "Gemini 2.5 series models don't support thinkingLevel; use thinkingBudget instead."
`gemini-2.5-flash` and `gemini-2.5-pro` are the ONLY two Gemini rows we ship. So `/effort` on Gemini has been
sending a parameter those models do not take, for as long as the feature has existed.
And `off` was worse than wrong. It mapped to `MINIMAL` — which is the LOWEST thinking level, not an off switch. A
model set to MINIMAL still thinks, and still bills for it. A user who turned reasoning OFF was being charged for
reasoning. Disabling on Gemini is `thinkingBudget: 0`, a different field, and only a model whose budget floor IS
zero can express it: `gemini-2.5-pro` has `min: 128` ("N/A: Cannot disable thinking"), so `off` is not offered for
it at all, and if one arrives anyway the field is WITHHELD rather than substituted.
**Anthropic.** `claude-haiku-4-5` publishes a token budget and NO effort axis — the maintainer confirmed this
against Anthropic independently. The adapter sent `output_config.effort` to it anyway. ADR-0066 filed the budget
shape as "legacy", and it is, for the industry; it is not legacy for one of the four Claude models we ship. It now
gets `thinking.budget_tokens`, with the ceiling taken from the REQUEST's own `max_tokens` — Anthropic requires
`budget_tokens < max_tokens`, which is exactly why haiku publishing no `max` is not a gap the catalog can close
alone.
The old Gemini test asserted the bug — and its own comment said "a Pro model rejects budget 0", so the author
already suspected the shape was not universal. It is rewritten to assert what is true, per model:
gemini-2.5-flash (budget, min 0) → thinkingBudget: N, and `off` → thinkingBudget: 0
gemini-3.5-flash (effort) → thinkingLevel: 'HIGH'
gemini-2.5-pro (budget, min 128)→ `off` is WITHHELD, never downgraded to MINIMAL
claude-haiku-4-5 (budget) → thinking.budget_tokens, and output_config is NEVER sent
an unknown model (custom endpoint)→ no reasoning field at all
That last row is the discipline the whole change turns on: when the catalog cannot describe a model, the field is
withheld. A guess is what put a rejected value on the wire in the first place.
The four wire maps also moved out of three adapters into one home (`reasoning-wire.ts`) — `acceptedTiers` has to
compose them with the catalog's per-model values, and two copies of "what we send a provider" are two chances to
disagree about it.
Refs: ADR-0071, ADR-0066
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…her the tier was IN it The Opus review of step 3 found the fix carried the original bug's shape one level down. Four defects, all of which put a value the model rejects on the wire, or bill the user for reasoning they turned off: - MEMBERSHIP, not presence. Every adapter tested that the model published *an* effort axis, then sent its own tier name regardless. `claude-opus-4-5` publishes ['low','medium','high'] — no `max` — so `/effort max` on it was a 400 that only the failover chain hid. `acceptedWireValue` now returns the wire value ONLY if the model actually lists it, and the four adapters ask it instead of testing for `!== undefined`. - Gemini's off switch diverged from the picker's. `gemini-2.5-flash-lite` publishes a toggle AND `budgetTokens.min: 512`; `acceptedTiers` offered `off`, the adapter looked only at `min === 0` and silently withheld the field. The user switched reasoning off, was billed for it anyway, and nothing said so. Both sides now ask one predicate, `canDisableReasoning`. - `max` spent the ENTIRE output cap on thoughts, leaving zero tokens to answer with — a request the API accepts and that comes back empty. `thinkingCeiling` reserves 20% (THINKING_BUDGET_SHARE) for the reply, and a budget that no longer fits under the cap is withheld rather than sent to be rejected. - `fallback-chain.ts` still gated on the boolean `modelSupportsReasoning`, so a failover entry re-sent the tier its own model had just rejected. It re-gates on `acceptedTiers`. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…h tiers it takes The five tiers were a fiction the UI told the user. `/effort` listed all five for every model and `gateReasoningEffort` passed each one straight through, so a tier the model never published reached the adapter and came back a 400 — or, worse, was quietly downgraded to something that still thinks. - `gateReasoningEffort` takes a `resolveEffortTiers(model)` and returns a discriminated result — `send` / `unset` / `uncontrollable` / `rejected` — instead of a bare tier, so a caller cannot forget the case where the model has no reasoning control at all. - `/effort` and the model picker's effort sub-step build their list from `effortTiersFor(model)`; the sub-step does not open at all for a model with no control. `gpt-5-pro` publishes exactly one effort level and now offers exactly one row, rather than five that mostly 400. `deepseek-reasoner` publishes none and offers nothing. - The CLI session host and the workflow engine builder inject the resolver from the one catalog (`catalogModel` + `acceptedTiers`), so the picker and the wire read the same source of truth. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…shes two
The Sonnet round on step 3 found two more paths where the picker and the wire answer
the same question differently — the bug class this work exists to close, one level in.
- `acceptedTiers` treated `effortValues` and `budgetTokens` as an either/or. `claude-opus-4-5`
publishes BOTH (ladder ['low','medium','high'], budget {min:1024}), and its adapter already
serves `max` from the budget — so the exclusive branch hid, from the picker and from the
failover re-gate, a tier the model serves perfectly well. The rescue turn ran with no
reasoning at all. The axes now UNION.
Not a blind union, though: only Anthropic and Gemini have a budget field to put it in.
OpenAI's `reasoning_effort` and DeepSeek's `thinking` are effort-shaped, so a budget in the
catalog buys those models nothing, and offering every tier for one would be the same
divergence wearing a different hat (`honorsBudget`).
- The DeepSeek arm sent `thinking` the moment the model had ANY descriptor. `deepseek-reasoner`
ships `reasoning: {}` — it reasons, and upstream describes no knob — so `{}` is not
`undefined`, the gate opened, and every tier including `off` went on the wire for a model
that never said it takes one. Both arms of the shared OpenAI-compatible adapter now ask
`acceptedTiers`, the same function the picker asks.
`canDisableReasoning` was the enabling half of that: it answered `true` for anthropic and
deepseek on the PROVIDER's general ability to disable, regardless of the model. An empty
descriptor has nothing to turn.
Plus `effortTiersFor(modelId)` — one exported predicate, so the four surfaces that currently
hand-write `catalogModel(m) + acceptedTiers(...)` cannot drift apart.
Refs: ADR-0071
Co-Authored-By: Claude <noreply@anthropic.com>
…fort tier" — now it has one The Opus round on step 4 found two blockers, and both are the same shape: step 4 migrated the pickers and the engine's gate to the catalog and left the other half of the CLI on the old boolean. The two then disagreed, and every disagreement is a bill the user cannot see. - `chat.ts` still asked `modelSupportsReasoning` — an id heuristic over the hand-typed pricing table — for the footer indicator and for `/effort`. It contradicts the catalog on SIXTEEN shipped models. On `claude-sonnet-4-5` (absent from `MODEL_PRICING` entirely) the picker offered five tiers, the engine sent the chosen one, and this line decided the model was not capable and blanked the footer: the user paid for extended thinking with no indicator it was on, and `/effort` could never tick the tier it had just bound. - The typed `/effort <tier>` never went through the picker. It validated against the fixed five and applied unconditionally, so on twenty-four models it accepted a tier the gate then silently dropped — `/effort low` on `gpt-5.4-pro` answered "applies to your next message", showed `low` in the footer, and sent nothing. It now refuses, and names what the model does take. Both now ask `effortTiersFor`, which moves to `@relavium/llm` as ONE exported predicate. The three hand-written `catalogModel(m) + acceptedTiers(...)` copies (two hosts + the picker) are gone; the CLI keeps only an ordering projection of it, because a Set has no order a UI can use. And the gate no longer withholds in silence. It replaced a loud provider 400 with a quiet no-op, which on an authored workflow is the worse of the two — the run succeeds, the knob does nothing, and the bill lands at the provider's default tier. `onEffortWithheld` hands the host the gate's own verdict (carrying the tiers the model WOULD take); the chat surface puts it in the transcript, `relavium run` writes it to stderr — never stdout, which `--json` owns. Also: a stale `reasoning_effort` in config survives a model-only default write, so switching the default model onto `gemini-2.5-pro` — which cannot disable thinking at all — left `off` bound, inert, and displayed. The session seed now says so instead. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…laces that build a session The Sonnet round on step 4 found the previous commit had built the channel and then mostly not connected it. `onBudgetWarning` — the same shape, the same purpose — is threaded through every one of these; `onEffortWithheld` reached only the two straight-line chat builds and `relavium run`. Silent everywhere else: `/clear`, the `/models` reseat, both bare-Home chat builds, `relavium agent run`, and `relavium gate`. On `agent run` and `gate` the miss is total — there is no picker, no footer, no client-side check of any kind — so an authored `reasoning_effort: low` on `gpt-5.4-pro` ran, was dropped, and billed at the provider's default with nothing said anywhere. That is the exact failure the previous commit's message describes as the reason the channel exists. Three more from the same round: - The notices moved out of `render/tui/effort-picker.ts` into `chat/effort-notice.ts`. The engine host and the session host were reaching into the TUI layer to build a sentence. - The model id is SANITIZED. It comes from an authored YAML — `nonEmptyString`, no charset restriction — and two of the sinks write the note raw to a terminal, so an escape sequence in a crafted workflow's model id reached the terminal uninterpreted. `models.ts` already guards the same input for the same reason. - `onceEffortNotice`: the gate runs on EVERY turn and every agent-node execution, and a withheld tier is a standing condition, not an event. A stale `off` bound on `gemini-2.5-pro` (which cannot disable thinking at all) grew a fresh transcript line every turn; an agent inside a `loop` printed the same stderr warning per iteration. And the fourth answer is gone for good: `ModelCatalogEntry.supportsReasoning` and the `modelSupportsReasoning` id heuristic behind it are DELETED, not just unused. Nothing read them any more, but they sat on the public seam of `@relavium/llm` — a boolean that disagreed with the catalog on sixteen shipped models, one import away from any surface that asked the wrong question again. `effortTiersFor` is the answer. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…ld name was wrong on half the population The other half of the maintainer's "max tokens errors" (ADR-0071 §7/§10a). THE CLAMP. Nothing compared an authored `max_tokens` against the model's real output limit, because nothing KNEW the limit: `MODEL_PRICING` carried a context window and no output ceiling at all. So `max_tokens: 200000` on a 64 000-token model was a 400 on every single turn, and the workflow it sat in never ran. The catalog carries both numbers, so `cappedMaxTokens` holds the cap at or below the model's ceiling — DOWN, never up. A cap BELOW the ceiling is the author's deliberate budget (a cost control, a latency bound), and raising it to the maximum would spend their money for them. Anthropic's thinking budget is derived FROM that cap, and the provider requires `budget_tokens < max_tokens` — so clamping the cap and not the derivation would have traded one 400 for another. The value is computed once and both uses read it. THE DIALECT. OpenAI's official Chat Completions deprecated `max_tokens` in favour of `max_completion_tokens`, and its reasoning models reject the old field outright. But this same adapter serves every custom OpenAI-compatible `base_url` — LM Studio, Ollama, vLLM, LiteLLM, an enterprise gateway — most of which implement only the legacy field, so switching globally would have traded one broken population for another. The rule is by ENDPOINT, not by provider: OpenAI's own API gets the modern field; DeepSeek's own API and every custom base URL keep `max_tokens`. `official` is not a synonym for `openai`: DeepSeek's `api.deepseek.com` is OUR default, not a caller's override, so it is official — and it still takes the legacy field. The same endpoint flag governs the clamp. A custom `base_url` may serve something entirely different under a familiar model id, and silently LOWERING a number the user typed on a model we cannot describe is a behaviour change we have no right to make. That it lands the opposite way from the reasoning field — which we withhold on an unknown model — is deliberate: withholding a field we cannot justify is safe, lowering a number the user chose is not. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…h — so BOTH went out
The Opus round on step 5 found a regression the dialect rule introduced, and a money bug it
exposed.
- **Both cap fields on the wire.** The escape-hatch merge is `{...providerOptions, ...body}`,
and `body` winning was automatic only while the mapped key and the escape-hatch key were
the SAME STRING. Renaming the mapped key to `max_completion_tokens` meant the old one no
longer shadowed anything, so a caller's `providerOptions.max_tokens` rode alongside it —
and OpenAI rejects a request carrying both. A 400 on exactly the population §10a rescues.
The adapter now guarantees EXACTLY ONE cap field: whichever we map wins outright, the other
key is dropped, and an un-mapped escape-hatch cap stands untouched (which is the override
ADR §10a promised — no config key exists, and none is needed).
- **"Official" was decided by "did a caller pass a string".** The CLI stores a `--base-url`
verbatim, so registering OpenAI with its own endpoint spelled out —
`https://api.openai.com/v1/`, one trailing slash — was classified CUSTOM: the deprecated
field, and no clamp, on the official API. That is this very bug, restored by a typo. The
endpoint is decided by resolved HOST now.
- **The budget governor pre-authorized spend on the UNCLAMPED cap.** Every hop from
`agent-runner` through the fallback chain to `checkPreEgress` carried the raw authored cap
while only the adapter clamped. `gemini-2.5-pro` with `max_tokens: 200000` (ceiling 65 536)
was estimated at three times the spend the model can physically produce — `on_exceed: fail`
killed the run over money that could never be spent, `pause_for_approval` gated a human on
the same phantom. Before the clamp landed the request just 400'd, so the gap was invisible.
`estimateMaxNextCost` now prices the cap the wire will actually carry.
Plus: `wasCapClamped` is deleted rather than shipped with no caller; the Gemini clamp gets the
tests it never had (including the clamped-cap → `thinkingBudget` coupling); the seam doc records
that `maxTokens` is clamped and that the OpenAI wire field is endpoint-dependent; and the
Anthropic SDK's refusal of a large non-streaming `max_tokens` — which the clamp cannot fix,
because every Anthropic ceiling sits above the SDK's threshold — is filed in deferred-tasks
rather than left as a comment in a test.
Refs: ADR-0071
Co-Authored-By: Claude <noreply@anthropic.com>
…-authorizing real spend The Sonnet round on step 5 found the clamp fix's own mirror image, and a table drift that is the argument for this whole ADR. - **The estimate clamped a CUSTOM endpoint.** `estimateMaxNextCost` always assumed the official rule, but the adapter deliberately does NOT clamp a custom `base_url` — a gateway may serve anything under a familiar id. So for a LiteLLM/vLLM endpoint the estimate landed BELOW what the wire can spend: `gpt-5.5` with `max_tokens: 500000` was pre-authorized at the 128 000 ceiling, and the governor waved through a call it exists to stop. Over-estimating kills a valid run; under-estimating spends the user's money, and this was the second kind. The endpoint is now injected — `ProviderResolver.endpointKind` → `BudgetGovernor` → the estimate — so the number the governor reasons about is the number the adapter will send. The engine still knows nothing of base URLs; the host answers, exactly as it does for price. - **The two tables disagreed, and nothing compared them.** `claude-sonnet-4-6` carried `maxOutputTokens: 64_000` in the hand-typed registry and `128_000` in the generated catalog; the clamp reads the catalog, the CLI's model list reads the registry, so the user was shown one ceiling while the wire enforced another. A guard added to catch it found a SECOND drift the review had missed — `gpt-5.5`'s context window, 1_000_000 by hand against 1_050_000 generated, which the context-fullness footer has been dividing by. Both corrected to the catalog, and the guard stands until Step 6 deletes the registry. - **A trailing-dot FQDN is the same host.** `https://api.openai.com./v1` is a legitimate spelling that DNS resolves identically, the CLI stores a `--base-url` verbatim, and the adjacent SSRF check already normalizes it — so leaving it unnormalized classified the REAL official endpoint as custom: deprecated field, no clamp. The bug, reinstated by a dot. Normalized in the adapter and in the CLI's own custom-host predicate, which must agree. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…e source, and the USER outranks it THE BIG SWAP (ADR-0071 §1). `MODEL_PRICING` is gone: twelve models, hand-typed from four pricing pages, re-verified whenever someone remembered. `pricing.ts` drops from 290 lines to 96 and keeps only what a price IS — the contract the CostTracker bills against, the pre-egress governor estimates against, and a `models pricing` row instantiates. The numbers come from the generated catalog now (eighty models, synced from models.dev with money-guards that refuse a silent price change), and from the user, who OUTRANKS it. **The precedence flipped, and that is the point.** Registry-first protected the user from mispricing a model *we* had verified by hand — a protection that made sense while the table was ours. It is not ours any more, and by the time it was retired it had drifted from the catalog on two numbers without anyone noticing. The user has a negotiated rate, an enterprise discount, or simply a price our snapshot has not caught up with, and they are the one holding the invoice. `priceModel` resolves user → catalog → throw. So `models pricing <a-model-the-catalog-knows>` no longer REFUSES. It used to, honestly: the table always won, so the override would have been a silent no-op. Now it takes effect, which is the entire reason to type the command. Also retired, because they answered the wrong question: - `isCanonicalModelId` / `KNOWN_MODEL_IDS` / `CanonicalModelId` — "is this one of OUR models". The honest question is "can we price this one", and a user-priced model is perfectly billable while never having been canonical. - `ModelPricing.reasoning` (a boolean) — a price is not the place to answer a capability question, and "does this model reason" is not what the wire asks anyway. - The static `deprecatedAt`. models.dev does not publish a retirement date and no source we have does; it comes from the live provider list (and the user) alone. The provider is the only one who knows when the provider is retiring something — a date typed into a file is a date that goes stale in a file. BREAKING CHANGE: `MODEL_PRICING`, `KNOWN_MODEL_IDS`, `isCanonicalModelId` and `CanonicalModelId` are removed from `@relavium/llm`'s public surface. `PricingSource` reports `'catalog'` where it reported `'registry'`. A user pricing row now overrides a catalog price instead of being refused. Refs: ADR-0071, ADR-0064, ADR-0065 Co-Authored-By: Claude <noreply@anthropic.com>
…ken — all of them money The Opus round on step 6 found the ADR's own clauses violated by the code that implements it. **Cached reads billed at ZERO.** `catalogPricing` wrote `?? 0`, and `cost()` computes `cacheReadTokens × rate` — so 0 does not mean "charge the normal rate", it means charge nothing. Eleven catalog models publish no cache rate, OpenAI auto-caches, and on `o1-pro` ($150/MTok in) the cached fraction of every prompt billed at $0.00. ADR-0071 §10 exists to forbid exactly this line — "absent ⇒ fall back to the full input rate" — and the comment I wrote above it claimed that was what 0 did. The test that "checked" it compared the projection to `x ?? 0`, the very expression it was testing, so it certified the bug. It now asserts the behaviour: a 100k cached read must cost the input rate, not zero. **The loud divergence never got built.** §5 makes the removal of "a user can never misprice a shipped model" conditional on the divergence being VISIBLE. It wasn't: `models pricing` printed its confirmation and said nothing about the catalog price it had just overruled. It now names both, in prose and in `--json` (`overriddenCatalogPrice`). The user gets what they asked for; they simply cannot do it in silence. **The merge and the cost path disagreed about which user rows apply.** `mergeModelCatalog` drops a row whose provider contradicts the catalog's; `priceModel` read the overlay unconditionally and billed it. So `models pricing gpt-5.5 --provider anthropic --input 0.00000001` zeroed a shipped OpenAI model's cost — realized fold and pre-egress estimate alike — while the picker went on displaying $5/MTok. Not merely silent: actively wrong. The guard now lives in the shared overlay builder, and the command refuses the write. Three more from the same round: - **A partial override was not partial.** The DB's columns are NOT NULL DEFAULT 0, so a user who types only `--input`/`--output` arrived here with zeroes for the window, the ceiling and the cache rate — harmless when a user row could only describe an unknown model, destructive under user-first precedence. The picker showed a 0-token context window for GPT-5.5, and its cache reads billed at nothing. Each unstated dimension now inherits the catalog's. - **Deprecation went permanently `false` for every model in the product.** I deleted the two DeepSeek dates arguing they should come from the live list — but no adapter populates `ModelListing.deprecatedAt` (the OpenAI list is id-only), so `deepseek-chat` was set to stop working in eleven days with nothing to say so. ADR-0071 §10 always specified a Relavium-owned overlay for precisely this; I removed the data instead of building it. - **The context tiers were dead data.** Parsed, guarded, exported — and read by no pricing code, so a >200k `gemini-2.5-pro` turn billed at the cheap tier. A tolerable gap while those models threw `UnknownModelError`; a silent 2× under-bill the moment the catalog started pricing them. The realized fold prices the tier the prompt landed in; the estimate takes the highest, because a cap that over-estimates refuses an affordable turn while one that under-estimates lets money out. Only one of those is recoverable. Refs: ADR-0071, ADR-0064, ADR-0065 Co-Authored-By: Claude <noreply@anthropic.com>
…ser to run a command I never wrote The Sonnet round on step 6 found six, and the most embarrassing is the simplest: the "loud divergence" message tells the user to run `models pricing <model> --clear` to revert. There was no `--clear`. There was no way back AT ALL — an override could be corrected but never removed, so a user who mispriced a catalog model by mistake was stuck with their own number for good. It exists now: a soft-deactivate (`model_catalog.id` is an FK target from five tables, so never a DELETE), and the model falls back to the catalog's price. **A cache HIT could cost five times a cache MISS.** A partial override inherited the catalog's ABSOLUTE cache rate — so a user who negotiated $0.10/MTok input on `gpt-5.5` inherited its $0.50 cache rate, and paid five times more for a cached token than a fresh one. A cache read is a DISCOUNT off input; the discount is what a partial override inherits, and it can never exceed the input rate it is a discount off. **`--cached 0` could not be said.** The money column is NOT NULL DEFAULT 0, so it cannot tell "the user typed zero" — a genuinely free cache on a self-hosted endpoint — from "the user never mentioned cache reads". Reading the zero as free bills a whole class of tokens at nothing; reading it as unset discards an explicit instruction. One column cannot hold both meanings, so the FACT of the statement gets its own (migration 0011, additive; verified on a copy of the maintainer's real 3.5 MB history.db — 84 catalog rows and 52 sessions intact). **A user-priced tiered model lost its tiers**, which quietly reopened the same silent 2× under-bill the previous commit closed — just on the overlay path. A tiered model is tiered whoever is paying: the tiers are inherited as MULTIPLIERS, so the user's own rate still doubles above 200k. The catalog's $2.50 is a fact about a price they no longer pay; that it DOUBLES is a fact about the model. Plus the tests that should have caught all of it: `model-catalog-view.test.ts` priced only models the catalog has never heard of, so nothing exercised an override of a model it KNOWS — which is the entire point of the flip, and where every one of these defects lived. One gap is documented rather than guessed at: cache WRITES are billed at the flat rate, never a tier's, because models.dev's tier schema publishes `cache_read` and no `cache_write`. Scaling one from the input tier's multiple would be a fabrication on a money path. Filed. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…ays default-OFF `relavium models refresh` re-fetches BOTH axes now (ADR-0071 §4a), because "refresh what I know about models" is one user intent, not two: the provider lists say which models a key can REACH, and the catalog says what they COST and which reasoning tiers they take. `--providers` and `--catalog` address one each; neither flag means both. The catalog half needs **no provider key**, which is the form that makes the default livable: a user with nothing configured yet, who wants to see what things cost, types one command and gets them. A `models refresh` with no key but a delivered catalog is no longer an error — telling someone their command failed while half of it worked is a lie. **Default OFF, and the default is the design.** `[catalog] auto_refresh` is `false`. Relavium SHIPS the catalog: a fresh install with no network prices every model it carries, offline, and contacts nobody. A local-first tool that reaches out to a third party by default violates its own spirit even when the payload is innocuous (rule 6), so the standing egress is opt-in — while an explicit `models refresh` fetches regardless, because a command the user typed IS consent. ADR-0068's convention: ship a risk-bearing surface default-OFF, validate, then decide. **Additive only, and the shipped snapshot is the FLOOR.** A refresh may add models and enrich thin ones; it may never leave one less described than it shipped. An unreachable models.dev, a 5xx, a truncated body, a row that arrives without a price — every one of them is a NO-OP, and the snapshot answers. The cost cap is a safety control, and it does not get to lapse because a third party had a bad deploy. The posture (§8): one compile-time-constant destination, HTTPS, `redirect: 'manual'` with an off-host response refused, no key, no cookie, no user data, Zod at the boundary — and the same `normalizeCatalog` the offline generator uses, so a refreshed row and a shipped row cannot end up shaped differently. The cache holds the RAW payload, not the normalized rows. Caching the normalized ones would freeze today's normalizer into the file, so a release that FIXED a normalization bug would go on serving the old wrong rows to everyone who had ever refreshed. Re-normalizing on load means an upgrade repairs the cache for free. Refs: ADR-0071, ADR-0064, ADR-0068 Co-Authored-By: Claude <noreply@anthropic.com>
…ded $14.50 as $0.00 The Opus round on step 7 found the one thing that mattered: `installCatalogRefresh`'s "additive only, the snapshot is the FLOOR" guard enforced no floor. It read `if (shipped === undefined || model.output > 0)`, and the line above had already dropped every `output <= 0` — so the second clause was ALWAYS true, the whole guard was `if (true)`, and a refreshed row replaced its shipped row wholesale. A moved price, a dropped context tier, a shrunk reasoning ladder all sailed through. Executed end to end: a payload that passes Zod (input 0 is legal, output 1 microcent is `> 0`) downgraded `gpt-5.5` from the shipped $30/Mtok output to ~free, and a 1M-in/100k-out turn that truly costs $14.50 recorded as $0.00 — so a `max_cost_microcents` cap of any value stopped tripping. It does not take a hostile models.dev; an upstream typo (a dropped decimal) or a preview model published at `input: 0` reproduces it. The safety control this whole ADR exists to build, defeated by the code that builds it. The fix is the reading §4.2 and §9 require together: the runtime refresh is **purely additive for shipped models** — it never writes a model the snapshot already pins, because a human-verified price is a release-time decision, not a runtime one (§9). It adds the long tail and nothing else, so it CANNOT make a known model cheaper — it does not write one. The floor test lives in `@relavium/llm`'s `lookup.test.ts`, source-direct, because the CLI test imports across the package boundary from `dist` and a break there would not fail until a rebuild — the stale-artifact trap that makes a break-verification lie. Verified: with the skip deleted, the source-direct test fails; the CLI test keeps the integration leg. Four more from the same round: - The cache stored the FULL 166-provider ~2 MB `api.json`, re-read and re-parsed by `seedCatalog` on every invocation including `--help`. It now caches only the four mapped providers. - The cache write is temp-then-rename now, so a concurrent reader never sees a torn file. - `seedCatalog`'s dead `io` parameter is gone. - ADR-0071 §4.2/§9 amended to record the additive-only reconciliation. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
… cap measured too late The Sonnet round on step 7 confirmed the floor holds (it reverted the fix and watched the source-direct test fail), and found three reporting/robustness gaps on the same money+egress path. - **`added` overcounted.** The host counted new ids against the snapshot by hand, while `installCatalogRefresh` separately dropped any new-but-unpriced id — so a payload with one priced and one unpriced new model reported `added: 2` while only one landed. The count now comes from `installCatalogRefresh` itself (it returns how many it admitted), so the number cannot drift from the floor that produced it. - **The byte cap measured after buffering the whole body.** `response.text()` read everything into memory and only then checked the length — so on a fast link a misbehaving host could deliver many times the 16 MB cap inside the timeout before the check fired. The comment claimed it "refuses an endless body"; it did not. `readCapped` streams the reader, counts as it goes, and aborts the transfer the moment the running total crosses the ceiling. - **Config was resolved twice per invocation.** `seedCatalog` ran a full `loadResolvedConfig` — a project-dir walk plus up to three TOML parses — at boot on EVERY command, including bare Home and `--help`, and `withModelsDeps` then resolved it again. The seed needs only the home dir, which is `homedir()` and needs no config parse: a new `resolveHomeDir` gives it that cheaply, and the boot path stops paying for work it throws away. Plus the tests those gaps hid: a mixed priced/unpriced payload asserting `added: 1`, an oversized `ReadableStream` asserting the reader is aborted early rather than drained, and the `[catalog] auto_refresh` behaviour on both settings (on ⇒ a bare `models` refreshes before it lists; off, the default ⇒ no network at all). Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…_cost_cap refuses it
A cost cap on a model we cannot price is a false sense of safety (ADR-0071 §K7). The governor
catches `UnknownModelError` and degrades to `allow` — the right trade for a self-hosted model
with ~no metered cost — but it did it in SILENCE. The user who set `max_cost_microcents` never
learned that the one turn it mattered for slipped past the cap entirely.
- The governor now returns a `{ kind: 'unpriced', model }` result and fires an `onUnpriced`
sink, once per model (a `loop` over an unpriced model must not repeat it every turn). The
chat surface routes it to the transcript, `relavium run` to stderr — never stdout, which
`--json` owns. The turn still runs; it is just no longer silent about the hole.
- `strict_cost_cap` (default false) inverts the trade for anyone who wants it: a workflow
`budget.strict_cost_cap` or `[chat] strict_cost_cap`, and an unpriced model is REFUSED
pre-egress instead of waved through. For a user who set a cap SPECIFICALLY to bound an
untrusted model, "if you cannot price it, do not run it" is the safer default — and now it
is available. The refusal names the model and how to fix it (`models pricing <model>`, or
turn the flag off).
`BudgetExceededError` gains an optional message so the strict refusal reads as what it is —
"cannot be enforced" — rather than a spend-exceeded projection.
Documented in both canonical homes (config-spec.md `[chat]`, workflow-yaml-spec.md `budget`),
and the governor's break-verify confirms the block: with `strict_cost_cap` ignored, the
BLOCKS test fails.
Refs: ADR-0071, ADR-0028
Co-Authored-By: Claude <noreply@anthropic.com>
… other surfaces The Opus round on step 8 found the same enumeration gap step 4 taught: `onUnpriced` was wired at some session/engine builds and not all, and it defaults to silent — so the "say it out loud" half of the commit type-checked its way out of the flagship surface. - **The in-Home chat** (`drive-home.tsx`, fresh + `/models` reseat) wired `onBudgetWarning` and `onEffortWithheld` but not `onUnpriced`. It is the primary interactive surface in Phase 2.6, and there an unpriced turn slipped past the cap in exactly the silence this commit exists to end. Both builds route to the transcript now. - **`relavium agent run`** and the resumed **`relavium gate`** built a governor without the sink too — both to stderr now (never stdout, which `--json` owns). Two more from the same round: - The notice text lived inline in four places, in two formats — one printed USD, two printed raw micro-cents, and the USD one rendered a tiny cap as `$1e-8`. One `unpricedModelNote` home now, with a `capUsd` formatter that reads `$0.00`, so all five surfaces say it the same way. - `strict_cost_cap` is inert without a positive `max_cost_microcents` (no cap, nothing to enforce) — documented in config-spec.md rather than left as a silent surprise. Plus the tests: `buildGovernorWiring` now asserts an unpriced model degrades-and-notifies (deduped per model) and that `strict_cost_cap` hard-fails regardless of `on_exceed`; and the governor's own "OFF degrades with a notice" test actually drives `checkPreEgress` now instead of discarding the captured sink. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…ropped them on the floor The Sonnet round on step 8 found the bug two commits missed — including the fold whose whole job was to close the enumeration gap. `WorkflowEngine`'s constructor stored only `#resolvePrice` and forwarded only that to each `RunExecution`. `deps.onUnpriced` and `deps.resolveEndpoint` reached the class and DIED there — never a field, never forwarded, in `start()` or `resumeFromCheckpoint()`. So on the two workflow surfaces — `relavium run` (fresh) and `relavium gate` (resumed) — both of these were inert: - **The unpriced notice (§K7)** was dead. A user relying on it to learn their cost cap silently did not apply to a self-hosted model saw nothing, on exactly the batch/automation surfaces where an unattended run is the highest-risk place for the cap to lapse. - **The endpoint resolver (§7)**, added in step 5's own fold and never wired here, was ALSO dead — a pre-existing spend-safety bug the same code-reading surfaced. Without it the pre-egress estimate assumed `official` for every model, so a custom `base_url` under a familiar id was estimated at the clamped cap while the wire sends the un-clamped one: the governor under-authorizes and waves through a call it exists to stop. Both are now stored and forwarded exactly like `#resolvePrice`, in both `start()` and `resumeFromCheckpoint()`. It went uncaught because NO test exercised these deps at the `WorkflowEngine` boundary — every passing test built `BudgetGovernor` or the session governor directly, which are the surfaces that already worked. A regression test at the engine boundary now fails without the forward (break-verified: delete it and the "reaches the sink THROUGH WorkflowEngine" test goes red), plus a `strict_cost_cap`-blocks-through-the-engine case. Three more from the round: - `unpricedModelNote` takes the surface's own strict-setting spelling now — a chat user reads `[chat] strict_cost_cap`, a workflow user `budget.strict_cost_cap`, instead of a generic name that makes each guess which file to edit. - `capUsd` gets direct tests (`$0.05`, `$10.00`, `$0.00` — the `$1e-8` bug pinned). - The reseat's three notice sinks all route through one `noteToStore` indirection now, so none can TDZ on the store declared after the build — `onBudgetWarning` was the only one guarded, and the asymmetry was an accident waiting for a refactor. Refs: ADR-0071, ADR-0028 Co-Authored-By: Claude <noreply@anthropic.com>
…nightly effort probe A third party (models.dev) now feeds a safety control (the cost cap) and a wire parameter (the reasoning tier). Two guards keep a stale or moved snapshot from silently breaking either — both scheduled, neither gating a PR (`.github/workflows/models-catalog.yml`). - **weekly-catalog-check** runs `pnpm sync:models:check`: it writes nothing and fails red when the committed snapshot has drifted — a new priced model, or (the one that matters) a moved or vanished price on a model we already ship. `sync.mjs`'s own money guards then refuse the moved-price / vanished-model commit without `--accept-price-changes` / `--accept-removals`, so taking it is a human decision in a reviewable diff, never a silent bot commit. The automatic PR for the purely-additive case is deferred — it needs a create-pull-request action pinned to a verified SHA, and inventing one unseen is the supply-chain risk rule 3 forbids; a red weekly check already surfaces the drift. Filed. - **nightly-effort-conformance** is `effort.conformance.test.ts`: for a representative shipped reasoning model per provider it sends EVERY tier the catalog claims it accepts and asserts the real API does not reject it. This is the ONLY mechanism that catches a stale catalog claiming a tier a model has stopped accepting — the exact Gemini bug this whole ADR fixed, which no offline test can see (fixtures replay what we recorded; a unit test proves internal consistency, not that the provider agrees). Key-gated per provider, so it skips a missing secret and never gates a PR. An OFFLINE assertion pins the four probe ids to real shipped reasoning models, so a rename cannot quietly turn the live lane into a silent no-op. ADR-0071 §9 amended to record both guards as wired. Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…vider the bug came from The Opus round on step 9 found the make-or-break flaw. The effort conformance test's Gemini probe was `gemini-2.5-flash`, which is BUDGET-shaped — so the adapter sends `thinkingBudget` (a number the API always accepts) and NEVER `thinkingLevel`, the one wire value that can 400 when a model stops accepting a tier. The lane passed no matter how the catalog drifted, on the exact provider whose `thinkingLevel` acceptance was the original bug this whole ADR fixed. The probe is `gemini-3-flash-preview` now — effort-shaped, so the adapter sends `thinkingLevel` and the real provider is the independent arbiter, exactly as the anthropic/openai/deepseek lanes already are. The offline gate confirms it has a graded ladder. Two more from the round: - **The weekly check went red on ANY drift, not just a moved price.** `sync:models:check` fails the moment models.dev adds any model in our four (fast-moving) providers — and a weekly check that is red every week trains the maintainer to ignore it, which is precisely the erosion sync.mjs's own comments warn against, defeating the moved-price protection it exists to give. The weekly job runs `pnpm sync:models` now, whose money guards red ONLY on a moved or vanished shipped-model price (the human decision) and pass on benign additive drift. New models still "merge automatically" via the deferred auto-PR. - The nightly conformance step is scoped to `src/conformance`, so a flaky live case cannot widen the red surface to the whole package's unit suite. ADR §9 amendment and the deferred-task corrected to match (moved-only red, effort-shaped probe). Refs: ADR-0071 Co-Authored-By: Claude <noreply@anthropic.com>
…eck for one
The Sonnet round on step 9 found the offline gate — whose whole job is to keep a future edit
from silently reintroducing the budget-shaped tautology the Opus round just removed — asserting
only that the probe has a graded ladder. But a budget-shaped model ALSO reports a full ladder
(the budget axis fills every tier), so `gemini-2.5-flash` still passed the gate. The one CI
signal meant to prevent the tautology could not see it.
The gate now pins each probe as EFFORT-shaped (`effortValues` defined) — the property the live
guard actually needs, since it only bites when the adapter sends an effort wire value the
provider can reject. Break-verified: revert the probe to `gemini-2.5-flash` and the gate goes
red, naming it as budget-shaped.
Three more from the round:
- The Gemini probe is `gemini-3.5-flash` now — a GA effort-shaped model, not the `-preview` id
the fold picked. A preview id is the one most likely to be renamed or pulled by the vendor
independent of any drift, which would red the nightly lane for the wrong reason — the exact
erosion the weekly-check scoping was careful to avoid.
- The live loop sends one call per distinct WIRE value, not per tier. DeepSeek's low/medium/high
all map to `reasoning_effort: 'high'` and Gemini's `max` onto `HIGH`, so probing every tier
fired real, billed calls proving the identical fact. The test title still names the full
claimed ladder; only the sends are deduped.
- A garbled word ("Reding") in the ADR §9 amendment.
And the scope: the prior fold's `fix(llm,ci)` used `ci` as a scope, but commit-style.md's scope
list has no `ci` (it is a commit TYPE) — the workflow file is root tooling, which the standard
names `repo`. Corrected here.
Refs: ADR-0071
Co-Authored-By: Claude <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces the hand-maintained MODEL_PRICING table with a generated models.dev-backed catalog (snapshot, normalization, refresh, DB overlay), adds per-model reasoning-effort tier gating and strict cost-cap governance across the core engine and adapters, and updates the CLI's config, chat, and TUI surfaces to consume catalog-derived pricing, reasoning tiers, and provider defaults. ChangesCatalog and Sync Infrastructure
Estimated code review effort: 5 (Critical) | ~150 minutes Runtime Governance and CLI Integration
Estimated code review effort: 5 (Critical) | ~180 minutes Documentation
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Adapter as LLM Adapter
participant ReasoningWire as reasoning-wire.ts
participant Catalog as catalogModel()
participant Engine as AgentSession/AgentRunner
participant Governor as BudgetGovernor
Engine->>Catalog: resolveEffortTiers(model)
Catalog-->>Engine: acceptedTiers(provider, reasoning)
Engine->>ReasoningWire: gateReasoningEffort(requested, tiers)
ReasoningWire-->>Engine: EffortGateResult (send/rejected/capped/uncontrollable)
Engine->>Adapter: effortToSend(result)
Adapter->>Catalog: modelAccepts(param) / cappedMaxTokens(model)
Adapter-->>Engine: request with withheld/adjusted fields
Engine->>Governor: checkPreEgress(model, provider)
Governor->>Catalog: catalogPricing(model)
alt priced
Governor-->>Engine: allow/warn/fail
else unpriced
Governor-->>Engine: onUnpriced(model, cap) note
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…rice cannot resurrect them (review M3) `clearUserPricing` soft-deactivates the `source='user'` row (never DELETEs — the row id is an FK target from five history tables) but left the pricing columns intact. `upsert` looks a row up by `(provider, model)` with `deletedAt IS NULL` — it ignores `isActive` — so a later PARTIAL re-price (`models pricing --input X --output Y`, no `--cached`) reuses that same FK-stable row and PRESERVES every column it omits. The cleared `cachedInputCostPerMtokMicrocents` / `cachedInputStated=true` therefore came back, silently billing cache-read tokens at the rate the user had explicitly cleared, marked as if freshly user-stated. Zero `inputCostPerMtokMicrocents`, `outputCostPerMtokMicrocents`, `cachedInputCostPerMtokMicrocents` and set `cachedInputStated=false` alongside `isActive=false`, so a reused row starts from a clean baseline: an omitted cache rate on the re-price now derives from the catalog discount, not the stale value. Regression test: price with a stated cache rate → clear → partial re-price (input/output only) → the cache rate is 0 and no longer user-stated. Refs: ADR-0065 §1, ADR-0071 §10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ore reseat notices (review M5 + bot) Two independent defects in drive-home's session builders: (a) startChat read the fresh default_model and default_provider INDEPENDENTLY, each `?? config.chat.*`. A user who hand-edits the global config mid-session to a different-family model (e.g. claude-sonnet-4-6) without a provider line got the fresh Claude model paired with the STALE startup `default_provider` (openai) — dialing the OpenAI adapter with a Claude id → 404 on the first turn. Read the effective [chat] ONCE and couple the pair: when the fresh read supplies a model, take ITS provider (undefined ⇒ inferred from the model id, never back-filled from startup); only with no fresh model fall back to the startup pair. Drops the stale `?? config.chat.defaultProvider` fallback (and the now-unused readEffectiveProvider). (b) reseatChat's noteToStore fell back to `writeErr` when a governor/effort callback fired DURING the build (before the seeded store exists). On the alt-screen renderer the next frame erases stderr, and onceEffortNotice had already marked the note delivered — so a mid-build effort-withheld/unpriced notice was silently lost. Buffer such notices and flush them through store.notice once the store is wired. Regression tests: a mid-session config rewrite to a provider-less Claude model builds with defaultProvider undefined (not stale openai); and an onUnpriced note fired during the reseat build reaches the transcript, not stderr. Refs: ADR-0059, ADR-0071 §6 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… gpt-5.6's `max` (review M1) `OPENAI_WIRE.max` was the fixed alias `'xhigh'`, and the adapter indexed it directly. But the shipped catalog carries a distinct `'max'` ABOVE `'xhigh'` for the gpt-5.6 family (`effortValues: [...,'xhigh','max']`), so the picker offered a "max" row, the gate accepted it, and the adapter sent `reasoning_effort: 'xhigh'` — one tier below the model's actual maximum, unreachable from any surface. gpt-5.4 -pro / gpt-5.5 (top `'xhigh'`) were correct; only a `'max'`-publishing model lost its top. New `openAiWireValue(tier, controls)` is the ONE home for OpenAI's per-model wire: normalized `max` → the model's own `'max'` when published, else `'xhigh'`; every other tier is the static map. `wireValueFor` (hence `acceptedTiers` / `acceptedWireValue`) and the adapter all read it, so the accept decision and the wire value can never disagree. The picker's dedup/projection path passes no controls and keeps the default `'xhigh'` — which never collides with another tier, so dedup is unchanged. The adapter assignment carries one documented cast: `'max'` is a valid models.dev wire value (the ADR-0071 source of truth) that the pinned OpenAI SDK's `ReasoningEffort` union has not yet published (it tops at `'xhigh'`). Narrowed to the field's own type; the value is always one `acceptedTiers` proved the model takes. Also folds a review-bot nitpick in the same file: `toGeminiThinkingLevel`'s `.toUpperCase() as` becomes an exhaustive typed Record (no unsafe assertion). Regression tests: openAiWireValue/wireValueFor/acceptedTiers/acceptedWireValue for gpt-5.6 (max→max) vs gpt-5.5 (max→xhigh); the adapter sends `'max'` for gpt-5.6 and `'xhigh'` for gpt-5.5. Refs: ADR-0066, ADR-0071 §6 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…apped` gate verdict (review M6)
A budget-shaped model (`claude-haiku-4-5`, `budgetTokens {min:1024}`, no ladder)
accepts every tier, so the effort gate said "send medium" — but the adapter then
computed `reasoningBudgetFor(medium, {min:1024}, thinkingCeiling(500)) = undefined`
and dropped `thinking` entirely. The turn ran at 500 output tokens with NO
reasoning despite the author selecting it, and nothing — transcript, stderr, run
event — said so. The gate's decision and the adapter's behavior disagreed in
silence.
Make the gate cap-aware so the two agree and the drop is voiced:
- `reasoningWithheldByCap(provider, controls, tier, maxTokens)` (llm) answers the
same question the adapter acts on — true only on the budget path, when the
model's floor cannot fit under `thinkingCeiling(maxTokens)`. It wraps the SAME
`reasoningBudgetFor` the adapters call, so gate and wire cannot drift.
- `gateReasoningEffort` takes an optional `{ maxTokens, withheldByCap }` and, for
an accepted non-`off` tier the cap withholds, returns a new `capped` verdict
(carrying the offending `max_tokens`) instead of `send`. `effortToSend` omits
the field, exactly as the adapter would.
- Both hosts inject the catalog-backed `reasoningWithheldByCapFor` and pass the
turn/node `max_tokens`; `agent-session` and `agent-runner` fire `onEffortWithheld`
for `capped` too. `effortCappedNote` names `max_tokens` as the fix ("reasoning on
needs a larger max_tokens than 500 …"), routed through the one `effortWithheldNote`.
Regression tests: `reasoningWithheldByCap` (budget vs ladder vs effort-only
provider); the gate returns `capped`/`send` and never cap-checks `off` or a
rejected tier; `effortCappedNote` + `reasoningWithheldByCapFor`.
Refs: ADR-0066, ADR-0071 §6
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nge cannot evict a priced model (review M7) `ModelSchema` validated `reasoning_options` as a whole-array `discriminatedUnion` and `reasoning`/`limit` non-nullably — as strictly as the money fields. So an upstream change to a field we only ENRICH with — a new `reasoning_options.type`, a `reasoning: null` — failed `ModelSchema.safeParse` and DROPPED a fully-priced model. A dropped already-shipped model reads to `diffCatalog` as a VANISHED price, which throws the ADR-0071 §9 money guard red for an entirely non-money reason; the tempting `--accept-removals` then deletes the model's price and silently disables the cost cap for anyone on it. Decouple enrichment from the money gate: - `reasoning_options` is now `z.array(z.unknown())`; `toReasoningControls` validates each entry INDEPENDENTLY with `ReasoningOptionSchema.safeParse` and skips an unrecognized/malformed one — an unknown control shape thins the descriptor instead of evicting the model. - `reasoning` tolerates `null` (→ treated as "no reasoning"). - `limit` tolerates `null` (→ treated as absent, a clean unpriceable-shape drop that is reported, not a fatal parse of the whole row). `cost`/`limit` values stay authoritative — they ARE the money surface. Regression tests: an unknown reasoning_options shape (and a malformed known one) is skipped with the model kept + priced; `reasoning: null` admits an unreasoning priced model; `limit: null` drops cleanly and is reported. Refs: ADR-0071 §9, §11 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… never gets a param it rejects (capability gating)
The reported bug: `gpt-5.6-luna` has `temperature: false` on models.dev, and a
turn that sets any temperature 400s immediately. The pre-existing `CapabilityFlags`
is a per-PROVIDER struct — it cannot say that one model in a provider rejects a
parameter its siblings accept. This is the same "a table cannot say what is true"
class ADR-0071 already fixed for pricing and reasoning shape, one more axis:
temperature / tool_call / structured_output / attachment all vary per model.
Mechanism (ADR-0071 §12 amendment):
- `CatalogModel.requestCapabilities` carries the per-model booleans, parsed from
models.dev and normalized like `reasoning` — NOT forced into the seam's
`CapabilityFlags`. Enrichment, parsed leniently (M7 rule), stores ONLY a `false`;
absent ⇒ accepted (the safe default — never withhold on missing metadata).
- `modelAccepts(model, param)` is the one predicate the adapters ask. All three
adapters WITHHOLD `temperature` and `structured_output` when the model rejects
them — a 400 becomes a served turn at the model's default, the same "withhold,
never guess" posture §6 set for reasoning.
- Anthropic additionally withholds a caller `temperature` whenever it enabled
extended thinking for the turn: `thinking:{enabled|adaptive}` pins temperature to
1, so any other value 400s (review M4).
Scope, honest: `temperature`/`structured_output` are safe silent param-withholds;
`tool_call`/`attachment` change request MEANING (agent can't act / image ignored),
so their catalog data is carried but gating them is deferred to a louder signal
(ADR §12). A user-facing "your temperature was dropped" notice is a follow-up. The
snapshot is GENERATED (§3) and NOT hand-edited: these fields land on the next
`pnpm sync:models`, so the mechanism is inert-but-correct until then.
Tests (via the additive `installCatalogRefresh`, no snapshot edit): schema stores
only rejected params; `modelAccepts` defaults to accepted for absent/uncatalogued;
each adapter withholds temperature/structured_output when rejected and sends when
accepted; Anthropic withholds temperature under enabled thinking (M4).
Refs: ADR-0071 §6, §11, §12
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… the /models picker Anthropic ships BOTH a rolling alias (`claude-opus-4-1`) and its dated pin (`claude-opus-4-1-20250805`) as byte-identical priced catalog rows, so the picker offered two selectable rows for one model. This is the DISPLAY half of the same alias↔dated-pin relationship whose AVAILABILITY half ADR-0064's 2026-07-14 clarification already handled. `collapseAliasDatedPinPairs` (@relavium/llm, reusing the existing `datedPinBase`) drops the dated pin at the display boundary — in `buildMergedCatalog`'s host projection, NOT in `mergeModelCatalog` (whose full-fidelity output other consumers keep). It collapses ONLY when both members are present as catalog rows — a lone dated pin with no alias sibling stays visible, or the model would vanish from the picker with no substitute (the same "both present" gate `hasLiveSibling` uses). Anthropic-only by construction: the `-YYYYMMDD` regex does not match OpenAI's `gpt-4o-2024-05-13` (`YYYY-MM-DD`) shape. This is IDENTITY dedup, not an availability change — deliberately distinct from the picker's "never hide a dimmed/deprecated model" rule (documented at the function and in the ADR). The ✓ marker normalizes a persisted DATED-PIN default onto the surviving alias row rather than vanishing (the same model resolves either id server-side, so the marker moves, the stored choice does not). No "(latest)" suffix exists to drop — the surviving alias already reads as a plain name. Tests: collapse drops the dated pin keyed on the alias; keeps a lone dated pin; keeps a non-anthropic dated variant; does not cross providers on a shared base id; buildMergedCatalog seeds the full registry minus the collapsed pairs (snapshot- derived count, asserted non-zero). Refs: ADR-0064 §6 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eading /effort affordance Six small but real fixes surfaced by the 2.6.Q review: - MONEY: the new-model refresh floor guarded only the OUTPUT price, so a model with `input: 0` was admitted and billed the input (and derived cached-input) side FREE — a silent undercharge in the exact mechanism the floor hardens. Guard both sides. - MONEY: `models pricing --clear --json` reported `source: cleared ? 'catalog'`, so a cleared model the catalog does NOT price reported 'catalog' while the human line said the opposite. Decide `source` from `catalogPricing()`, matching the message. - SECURITY: `unpricedModelNote` interpolated the provider-controlled model id into a terminal line UNSANITIZED (twice), while every sibling effort notice sanitizes it — a terminal-control-injection surface. Sanitize it. - SECURITY: the build/CI `sync.mjs` read the upstream body UNBOUNDED, while its runtime sibling caps + aborts. Bound it (declared + actual length) so a misbehaving host cannot OOM the sync or write a nonsense catalog. - ROBUSTNESS: `writeCache`'s temp-then-rename left an orphan pid-suffixed temp file on a write/rename fault. Best-effort unlink on failure (a hard kill can still orphan — noted). - UX: the bare `/effort` list advertises an "on" row (budget models), but typed `/effort on` was rejected as an unknown tier. Parse the typed arg against the SAME labels the discovery list printed, so the affordance it shows actually works. Also folds a review-bot nitpick: `readCatalogBody` narrows each stream chunk with `instanceof Uint8Array` (annotating the read result `unknown`) instead of asserting the whole body as `ReadableStream<Uint8Array>`. Regression tests: input-0 floor rejection; `--clear --json` source (catalog vs null); `unpricedModelNote` sanitization. Refs: ADR-0071 §5, §9, §K7 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… §K7, and stale JSDoc/DDL The review found the docs drifted from the shipped code, concentrated in the money-precedence area (which rule 8 makes authoritative, so it is a defect class): - H2 (commands.md): the `models pricing` reference still described the pre-ADR-0071 precedence — a "canonical id is refused / the shipped price always wins / a user can never misprice a shipped model". All false since §5 (USER outranks catalog). Rewritten to the shipped rule, with `--clear` documented; the `--json refresh` schema now documents the catalog record it emits alongside the per-provider ones. - The phantom `ADR-0071 §K7`: 45 citations (config spec, workflow spec, engine) pointed at a section that did not exist — `K7` was an implementation milestone label that leaked before `strict_cost_cap` was written up. Recorded now as ADR-0071 §K7 (append-only), so every citation resolves rather than renumbering 45. - config-spec `[chat].default_provider`: rewritten to the COUPLED resolution (same layer as the effective default_model, never independently — the M5/Bug-3 rule), dropping the "falls back to global [preferences]" wording that implied the decoupling that 404s. - Stale references: the `mergeModelCatalog` JSDoc said `price ← registry ?? user` (pre-§1 flip; now `user ?? catalog`); `installCatalogRefresh`'s doc claimed it "enriches thin shipped models" when it never touches a shipped row; the `model_catalog` DDL omitted the shipped `cached_input_stated` column (migration 0011); the ADR-0059 link in ADR-0063 pointed at the wrong filename. Deferred (tracked in deferred-tasks.md): the capability `tool_call`/`attachment` gating + a withheld-parameter notice (ADR-0071 §12), and surfacing a user↔catalog price divergence on the picker/`/cost`, not only at set-time (§5). Refs: ADR-0071 §1, §5, §K7, §12 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sage, and a false-green nightly guard Three consistency/observability nits from the review: - Migration 0011 spelled the new boolean's default `DEFAULT false`, diverging from the `DEFAULT 0`/`1` integer idiom every other migration, the drizzle snapshot (which already records `"0"`), and the runtime use. Fixed to `DEFAULT 0` — identical to SQLite, consistent with the idiom, and db:generate stays clean (0011 is part of this unmerged PR, so the SQL is safe to correct). - The catalog refresh reported a `redirect: 'manual'`-caught redirect (an OPAQUE response: `type: 'opaqueredirect'`, `status: 0`) via the generic status branch as "models.dev returned 0" — misleading, when the dedicated "redirected off-host" message exists for exactly this ADR-0071 §8 case. Detect the opaque redirect before the status check. - The nightly effort-conformance job's live cases are `it.skipIf(<key> === '')`, so with NO provider secret every probe SKIPS and the suite passes — a green that proved nothing, indistinguishable from "all tiers verified". A guard step now fails loudly when no secret is configured, so a green nightly means probes ran. Regression test: a manual-caught opaque redirect (status 0) is named "off-host". Refs: ADR-0071 §8, §9, §10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…versarial-review fold of M7) The M7 leniency fix (8278057) made each `reasoning_options` ELEMENT tolerant (safe-parsed one at a time) and named `reasoning: null` as a case it survives — but left the CONTAINER strict: `z.array(z.unknown()).optional()` still FAILS on `null`, `{}`, a string, or a number. So a `reasoning_options: null` from upstream (models.dev already emits explicit `null` for reasoning/cost/limit), or an array→object change, failed the whole `ModelSchema.safeParse` and DROPPED a fully-priced model — the exact §9 vanished-price red CI M7 set out to prevent, one field over. Give the container the same tolerance as its siblings: `.nullish().catch(() => undefined)` — `null` is symmetric with `reasoning`, and a wrong TYPE degrades to "no controls" (`raw.reasoning_options ?? []` already handles the undefined) instead of failing the row. Regression test: `reasoning_options` = null / {} / string / number each keeps the priced model with an empty reasoning descriptor, never dropped. Refs: ADR-0071 §9, §11 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/llm/src/adapters/openai.test.ts (1)
94-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
catModelto a shared test-fixtures module.Identical
catModelhelper is duplicated verbatim inanthropic.test.ts,gemini.test.ts, andmodel-catalog.test.ts. A futureCatalogModelfield change needs to be replicated in all 4 files or they drift.♻️ Proposed refactor
-/** A minimal catalog row (additive refresh) carrying only what a capability test needs. */ -const catModel = (over: Partial<CatalogModel> & Pick<CatalogModel, 'modelId'>): CatalogModel => ({ - provider: 'openai', - displayName: over.modelId, - contextWindowTokens: 100_000, - maxOutputTokens: 10_000, - inputPerMtokMicrocents: 1_000_000, - outputPerMtokMicrocents: 2_000_000, - ...over, -}); +import { catModel } from '../test-fixtures/catalog-model.js';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/openai.test.ts` around lines 94 - 103, Move the shared catModel helper into the existing test-fixtures module, preserving its defaults and Partial<CatalogModel> override behavior. Update anthropic.test.ts, gemini.test.ts, model-catalog.test.ts, and openai.test.ts to import and reuse that fixture, removing their local catModel definitions.packages/llm/src/model-catalog.ts (1)
149-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify with optional chaining.
Flagged by SonarCloud:
alias !== undefined && alias.provider === 'anthropic'can be simplified.♻️ Proposed simplification
- return !(alias !== undefined && alias.provider === 'anthropic'); + return alias?.provider !== 'anthropic';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/model-catalog.ts` at line 149, Update the return condition in the model-catalog filtering logic to use optional chaining when checking alias.provider, replacing the explicit alias undefined guard while preserving the existing boolean behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/decisions/0063-cli-config-write-contract.md`:
- Around line 9-10: The ADR’s later writer contract is outdated and omits
defaultProvider and reasoningEffort. Update the §3 contract to document
writeGlobalPreferences as the partial-merge API persisting defaultModel,
defaultProvider, and reasoningEffort, and remove or revise the
setDefaultModel-only wording.
In `@docs/reference/cli/commands.md`:
- Around line 238-240: Update the catalog record description under `relavium
models refresh --json` to document the complete schema: `source` must be
`'catalog'`, `status` is `'refreshed'` or `'failed'`, `models` and `added` are
numeric with `0` on failure, and an optional secret-free `reason` may be
included.
---
Nitpick comments:
In `@packages/llm/src/adapters/openai.test.ts`:
- Around line 94-103: Move the shared catModel helper into the existing
test-fixtures module, preserving its defaults and Partial<CatalogModel> override
behavior. Update anthropic.test.ts, gemini.test.ts, model-catalog.test.ts, and
openai.test.ts to import and reuse that fixture, removing their local catModel
definitions.
In `@packages/llm/src/model-catalog.ts`:
- Line 149: Update the return condition in the model-catalog filtering logic to
use optional chaining when checking alias.provider, replacing the explicit alias
undefined guard while preserving the existing boolean behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44877372-d13b-42c9-8c66-52f42a027043
📒 Files selected for processing (55)
.github/workflows/models-catalog.ymlapps/cli/src/chat/effort-notice.test.tsapps/cli/src/chat/effort-notice.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/models-pricing.test.tsapps/cli/src/commands/models-pricing.tsapps/cli/src/engine/build-engine.tsapps/cli/src/engine/catalog-refresh.test.tsapps/cli/src/engine/catalog-refresh.tsapps/cli/src/engine/model-catalog-view.test.tsapps/cli/src/engine/model-catalog-view.tsapps/cli/src/home/drive-home.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/render/tui/model-picker-view.tsxdocs/decisions/0063-cli-config-write-contract.mddocs/decisions/0064-live-model-catalog.mddocs/decisions/0071-models-dev-as-the-model-metadata-source.mddocs/reference/cli/commands.mddocs/reference/contracts/config-spec.mddocs/reference/desktop/database-schema.mddocs/roadmap/deferred-tasks.mddocs/roadmap/phases/phase-2.6-conversational-authoring.mdpackage.jsonpackages/core/src/engine/agent-runner.tspackages/core/src/engine/agent-session.tspackages/core/src/engine/agent-turn.tspackages/core/src/engine/budget-governor.test.tspackages/core/src/engine/budget-governor.tspackages/core/src/engine/engine.tspackages/core/src/engine/reasoning-effort.test.tspackages/core/src/engine/reasoning-effort.tspackages/core/src/index.tspackages/db/drizzle/0011_odd_thunderbolt.sqlpackages/db/src/model-catalog-store.test.tspackages/db/src/model-catalog-store.tspackages/llm/src/adapters/anthropic.test.tspackages/llm/src/adapters/anthropic.tspackages/llm/src/adapters/gemini.test.tspackages/llm/src/adapters/gemini.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.tspackages/llm/src/catalog/catalog-model.tspackages/llm/src/catalog/lookup.test.tspackages/llm/src/catalog/lookup.tspackages/llm/src/catalog/models-dev-schema.test.tspackages/llm/src/catalog/models-dev-schema.tspackages/llm/src/fallback-chain.test.tspackages/llm/src/fallback-chain.tspackages/llm/src/index.tspackages/llm/src/model-catalog.test.tspackages/llm/src/model-catalog.tspackages/llm/src/reasoning-wire.test.tspackages/llm/src/reasoning-wire.tstools/sync-models-dev/sync.mjs
🚧 Files skipped from review as they are similar to previous changes (30)
- packages/core/src/index.ts
- packages/llm/src/catalog/lookup.ts
- package.json
- apps/cli/src/chat/effort-notice.test.ts
- packages/db/drizzle/0011_odd_thunderbolt.sql
- apps/cli/src/engine/model-catalog-view.ts
- .github/workflows/models-catalog.yml
- apps/cli/src/commands/models-pricing.test.ts
- packages/core/src/engine/agent-runner.ts
- apps/cli/src/engine/build-engine.ts
- packages/db/src/model-catalog-store.ts
- packages/llm/src/adapters/gemini.ts
- packages/llm/src/fallback-chain.ts
- packages/llm/src/adapters/anthropic.ts
- packages/core/src/engine/agent-session.ts
- packages/llm/src/index.ts
- apps/cli/src/engine/catalog-refresh.test.ts
- packages/llm/src/catalog/models-dev-schema.ts
- docs/reference/contracts/config-spec.md
- apps/cli/src/engine/catalog-refresh.ts
- apps/cli/src/chat/session-host.ts
- packages/llm/src/model-catalog.test.ts
- tools/sync-models-dev/sync.mjs
- apps/cli/src/chat/effort-notice.ts
- packages/core/src/engine/budget-governor.ts
- packages/llm/src/reasoning-wire.test.ts
- apps/cli/src/commands/models-pricing.ts
- packages/llm/src/reasoning-wire.ts
- apps/cli/src/commands/chat.ts
- packages/llm/src/adapters/openai.ts
…alog test fixture, and the --json catalog record
Verified each finding against current code; fixed the still-valid ones.
FIXED:
- Sonar (model-catalog.ts:149) + inline: `!(alias !== undefined && alias.provider
=== 'anthropic')` → `byId.get(base)?.provider !== 'anthropic'`. An absent alias
short-circuits to `undefined !== 'anthropic'` ⇒ keep, exactly as the explicit
guard did.
- Sonar (reasoning-effort.ts:86): the cap guard becomes an optional chain,
`cap?.withheldByCap(model, effort, cap.maxTokens) === true` (TS still narrows
`cap` in the body).
- Sonar (reasoning-wire.ts:75): the thrice-repeated `'low' | 'medium' | 'high'`
union becomes the `GeminiWireLevel` type alias (GEMINI_WIRE, the thinking-level
Record, and toGeminiThinkingLevel all read it).
- Sonar (gemini.ts) cognitive complexity 16→under budget: the capability gates
pushed `buildGeminiRequest` over, so the nested reasoning block is extracted to
`applyThinkingConfig`.
- Sonar (sync.mjs) cognitive complexity 16→under budget: the bounded-body checks
pushed `main()` over, so fetch + status + size-bound is extracted to
`fetchUpstreamBody` (with `MAX_BODY_BYTES` hoisted).
- commands.md: the `models refresh --json` CATALOG record now documents the whole
schema — `source` is always `'catalog'`, `status` ∈ refreshed|failed, `models`
and `added` are numeric (`0` on failure), and `reason` is present only on
`failed` and is secret-free. Matches what models.ts actually emits.
- The four duplicated `catModel` test helpers collapse to ONE
`catalogModelFixture`, placed under `src/conformance/` — the path
`tsconfig.build.json` EXCLUDES, so a test-only helper does not ship in dist.
SKIPPED:
- ADR-0063 §3 "typed setter (setDefaultModel)": that is the original DECISION body,
and CLAUDE.md rule 9 makes ADRs append-only ("never rewrite history"). The two
dated notes (2026-07-07, 2026-07-13) already record the generalization to
`writeGlobalPreferences({ defaultModel?, defaultProvider?, reasoningEffort? })`.
Rewriting the body would violate the rule.
Validated: lint + typecheck + test green across all 6 workspaces (23/23 tasks,
4,656 tests), lint:tools + typecheck:tools green, prettier clean, build 6/6, and
the new fixture confirmed absent from the published dist.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… close two index gaps The doc was filed under reference/desktop/ despite documenting a schema three surfaces share (desktop SQLCipher, CLI better-sqlite3 per ADR-0050, VS Code wasm) — moved it to reference/shared-core/ and repointed the ~49 references across docs and JSDoc comments that pointed at the old path (plus one pre-existing, unrelated depth bug in packages/db/src/index.ts's own doc links, fixed while touching the same lines). Also added the idx_runs_created / idx_agent_sessions_updated indexes the 2.5.B perf pass shipped but the doc never recorded, and a full Mermaid ER diagram of all 14 tables so the relationships are visible at a glance instead of only per-table prose. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ffline snapshot floor Records the DB-backed model-catalog decision after a four-bot adversarial review of the draft. Folds every finding that survived grounding against the code and the amended ADRs: - Snapshot-membership exclusion keys on compile-time CATALOG_SNAPSHOT, never the mutable DB `origin` column (Bot-4 HIGH): a promoted long-tail id must not be shadowed by a stale refreshed row. Load-bearing; explicitly tested. - `admitRefreshedModels` pinned to @relavium/llm, typed over CatalogModel, so the dependency direction stays apps/cli -> @relavium/llm (engine purity). - Money invariants made concrete: finite-and-positive base price (NaN fails the finite check), NULL cache_read -> undefined never 0 (ADR-0071 §10), shipped-row enrichment restricted to an allow-list, single authoritative overlay backing. - Long-tail price-drop gap and the ADR-0065 id-reuse collision addressed as conscious-acceptance (no stateful magnitude guard, no re-key — both would break a stronger invariant); `description` kept per the maintainer's explicit request. - `catalog_meta` (SHA + TTL + schema-version) named in the Decision; egress axis clarified (picker-open runs availability only); `visible` never-clobber reworded to the real write paths; citations fixed (arch-principles §3, ADR-0071 §4). - Reclassified as a PARTIAL SUPERSESSION of ADR-0064 §4's seed-prohibition (a prohibition -> scoped permission is a reversal) + AMENDMENTS of ADR-0071 §3/§4/§9. Append-only stamps (bodies untouched, Status stays Accepted): - ADR-0064: corrects the stale "five tables" FK count -> six (ADR-0070 added session_costs); records the §4 partial supersession. - ADR-0071: records the §3/§4/§9 amendments (floor now sits under the DB). Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…catalog.visible (ADR-0072)
The DB-backed model-catalog schema behind the offline snapshot floor. Three additive,
reversible changes in migration 0012:
- `model_metadata` — the models.dev mirror keyed by `model_id` alone (the CATALOG_SNAPSHOT
key space), a leaf with NO FK to model_catalog and no referrers. Money columns: base
input/output NOT NULL; cache_read/cache_write NULLABLE (NULL != 0, ADR-0071 §10). A
scoped CHECK requires a REFRESHED row priced > 0 on both sides while a SHIPPED row (pinned
to the reviewed snapshot) may be free. `origin` CHECK IN ('shipped','refreshed').
- `catalog_meta` — the singleton cursor (id = 1 CHECK + PK) holding the seed SHA, the
normalizer schema version, and the two per-axis TTL cursors.
- `model_catalog.visible` — additive ALTER ADD DEFAULT 1, orthogonal to is_active, for the
future /settings > /models picker toggle.
`CATALOG_METADATA_ORIGINS` closed set added to @relavium/shared. Schema tests prove the
money CHECKs at rest (free refreshed row rejected, free shipped row admitted, negative
rejected, NULL cache round-trips as null, both singleton halves, visible default). Review
fold (Opus + Sonnet): documented both tables + the visible column in database-schema.md
(the DDL canonical home), corrected its stale "five tables"/"pricing home" prose to six +
the ADR-0072 mirror, and extended the client.test.ts table/index smoke lists (also closing
a pre-existing session_costs gap).
Refs: ADR-0072
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nrichment (ADR-0072) Extracts the additive-admission floor from installCatalogRefresh into a single pure `admitRefreshedModels(models)` — exported from the pure @relavium/llm index so the host DB writer applies the IDENTICAL rule (dependency direction stays apps/cli -> @relavium/llm). The gate hardens the money invariants: - snapshot-membership exclusion keys on compile-time CATALOG_SNAPSHOT, not a mutable DB flag (ADR-0072 point 3a) — a spoofed price + enrichment riding along cannot re-admit a shipped id; - finite-AND-positive base price, finite check FIRST (NaN <= 0 is false, so a torn/NaN money value would slip a bare <= 0 guard and propagate NaN into the governor). installCatalogRefresh now delegates to the gate (behaviour byte-identical bar the added finite check — the old <= 0-only guard actually admitted a NaN price). The read seam `catalogModel = refreshed[id] ?? CATALOG_SNAPSHOT[id]` is byte-unchanged. CatalogModel gains pure-enrichment fields (input/output modalities, knowledge cutoff, description) parsed leniently from models.dev — a shape change THINS the descriptor, never EVICTS a priced model (the review-M7 class). Enrichment is invisible to moneyFingerprint / diffCatalog, so the §9 money guard stays money-only. NOTE: normalizeCatalog now emits enrichment the committed snapshot lacks. The snapshot is deliberately NOT regenerated here — a live sync surfaces unrelated gemini-flash price INCREASES the §9 guard correctly refuses; those are a separate maintainer money decision (run `pnpm sync:models --accept-price-changes` when ready). Enrichment reaches the DB via the live refresh path regardless. The nightly models-catalog.yml is not a PR gate. Review fold (Opus + Sonnet): Readonly return type on the boundary-crossing gate; the parity test renamed to "installCatalogRefresh hasn't drifted from the gate" (the real cross-implementation parity test lands with the P4 DB writer); added the spoofed-shipped-id exclusion + bad-element-modalities cases; fixed a detached group doc-comment. Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`createModelMetadataStore` — the `@relavium/db`-pure backing for the DB catalog mirror that replaces the ~/.relavium file cache. Depends only on @relavium/shared: it reads and writes raw ModelMetadataRow records; the row<->CatalogModel projection and the admitRefreshedModels money gate stay the HOST's job (P4), keeping the engine portable. - `upsert` — full-row upsert (a boot re-seed of shipped rows, or long-tail rows the host's gate already admitted); fullUpsertSet overwrites money+wire+enrichment while preserving created_at, and preserves a NULL cache rate as NULL (never 0, ADR-0071 §10). - `updateEnrichment` — writes ONLY the four pure-enrichment columns, so a catalog refresh can enrich a shipped row without ever moving its pinned, human-reviewed price/reasoning (ADR-0072 point 5). - `readAll` / `readMeta` / `upsertMeta` — the singleton catalog_meta cursor (id=1) with partial-patch semantics so the two per-axis TTL cursors move independently. - `coerceCatalogMetadataOrigin` — a foreign value degrades to 'refreshed', never 'shipped' (the price-CHECK-exempt origin); wired into the host projection in P4. Writes use BEGIN IMMEDIATE + withBusyRetry (ADR-0064 2.5.I). Review fold (Opus + Sonnet): `?? now` over `|| now` for the epoch-0 edge; re-seed test now asserts wire+enrichment overwrite (not just money); NULL-cache test now covers the conflict/update path; added no-op cases (updateEnrichment on a missing id, empty upsertMeta patch). Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The host half of the DB catalog: a new `catalog-metadata.ts` is the ONE place @relavium/db and @relavium/llm meet (mirroring pricing-overlay.ts), projecting rows <-> CatalogModel, seeding shipped rows (SHA-gated) from the snapshot, installing the long-tail overlay from the DB, and mirroring a models.dev refresh into model_metadata through the shared admitRefreshedModels gate. @relavium/core and pure @relavium/llm stay db-free. Wiring (M4-safe — only priced surfaces open the DB, never --help/--version): - readUserPricingOverlay/loadUserPricingOverlay call syncCatalogFromDb (best-effort); - refreshCatalog gains an injected `persist` callback (keeps catalog-refresh.ts db-free); - the `models` command seeds+installs and wires persist over its own db. CATALOG_SCHEMA_VERSION + CATALOG_SHA256 exported from @relavium/llm gate the seed/read. Review fold (Opus + Sonnet), money-safety first: - HIGH: installCatalogFromDb installed unconditionally, so an empty/shipped-only DB WIPED a good boot file-cache overlay (wholesale replace) — unpricing its long tail. Now gated on a non-empty admitted set (ADR-0072 point 7 single-authoritative-backing), + a regression test. - HIGH: seedShippedCatalog's reseed blanked shipped ENRICHMENT on every binary release (snapshot carries none). Now carries it forward (`?? prior`), + a test. - HIGH: parseJson used `JSON.parse as T` (unsafe as, CLAUDE.md rule 1) with no shape check. Now Zod-validates each JSON column; a corrupt money field drops to the flat rate (cap still engages) rather than unpricing the row, + a test. - installCatalogFromDb filters origin='refreshed' (skips parsing shipped rows every priced command; cleanly resolves the de-shipped-row edge); round-trip test now covers contextTiers + cacheWrite; models command routed through best-effort syncCatalogFromDb; accepted-drift + invariant notes. NOTE: the snapshot is deliberately NOT regenerated (a live sync surfaces unrelated gemini-flash price increases the §9 guard refuses — a separate maintainer money decision); enrichment reaches the DB via the refresh path regardless. Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Makes the P1 `model_catalog.visible` column live on the read side: `ModelCatalogListing` carries `visible`, `toListing` maps it, and `buildMergedCatalog` HARD-filters hidden models out of the `/models` picker — distinct from §6's "dim, never hide" availability rule (a dimmed model is one you cannot USE; a hidden one is one you chose not to SEE). The write path (a setter + the /settings > /models toggle UI) is deliberately deferred — the maintainer framed it as future — so the filter is forward-compatible infrastructure that defaults to visible until a toggle ships. The never-clobber holds by construction: replaceProviderModels' UPDATE and upsert's `shared` both omit `visible`, so a routine availability refresh never un-hides a hidden model. Review fold (Opus + Sonnet): - HIGH: the hidden set keyed on modelId ALONE, but `visible` is per (provider_id, model_id) and an id can be reused across providers (ADR-0065 §6) — hiding one provider's model could drop another's same-id model. Now anchored on (provider, modelId), mirroring buildUserPricing's cross-provider guard. + a provider-anchored test. - MEDIUM: documented that `relavium models` / `--json` is DELIBERATELY unfiltered (visibility is a picker preference, not a catalog deletion — a --json consumer must see hidden models). - tests: tier-independence (static tier), never-clobber for upsert + the reactivation (vanished-then-reappeared) branch, a guarded snapshot helper (no bare `!`), prettier. Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cemililik
changed the title
The root `//#format:check` (`prettier --check .`) is a SEPARATE turbo task from `lint`, so `turbo run lint typecheck test build` did not catch these — seven P1–P4 files carried over-width lines. Cosmetic re-wrapping only; no logic change (db/llm/cli suites unchanged: 242 / 689 / 2239 green). Full-repo `prettier --check .` is now clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/decisions/0064-live-model-catalog.md (1)
77-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the alias-collapse contract with the actual gate.
collapseAliasDatedPinPairsonly checks that the base id exists in the merged entries and is Anthropic; it does not require both rows to be shipped catalog rows. Revise the ADR wording, or tighten the helper if that shipped-row constraint is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/decisions/0064-live-model-catalog.md` around lines 77 - 102, Align collapseAliasDatedPinPairs with the documented shipped-row gate: require both the rolling alias and its Anthropic dated-pin sibling to exist as catalog rows before collapsing the dated pin. Otherwise preserve the dated row, including when only one member is present, and keep mergeModelCatalog’s full-fidelity output unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/engine/catalog-metadata.ts`:
- Around line 42-60: Tighten the numeric validation in PriceTiersSchema so
aboveContextTokens, inputPerMtokMicrocents, and outputPerMtokMicrocents accept
only finite positive integers, while cachedInputPerMtokMicrocents accepts finite
non-negative integers. Update parseJson to return z.infer<typeof schema> rather
than casting to the generic T, preserving the existing schema-driven parsing
flow.
In `@docs/analysis/_archive/README.md`:
- Line 57: Revert the change to the archive provenance entry in README.md and
restore its prior target exactly. Do not modify any files under the frozen
archive; if the referenced document was removed, resolve link compatibility
outside the archive while preserving the historical snapshot.
In `@docs/decisions/0064-live-model-catalog.md`:
- Line 5: Update the canonical-homes sentence in ADR-0064 to remove pricing.ts
as the active static registry, since MODEL_PRICING is retired by ADR-0071. Point
the pricing/catalog reference to the current generated catalog snapshot home, or
explicitly label the pricing.ts reference as historical.
In `@docs/roadmap/phases/phase-4-vscode.md`:
- Line 376: Update the roadmap statement around relavium.exportChatSession to
remove the claim that CLI, desktop, and VS Code share one encrypted physical
history.db. Describe the cross-host behavior as a shared canonical schema with
surface-specific database implementations and at-rest encryption handling,
preserving the existing wasm SQLite detail for the extension host.
In `@packages/db/src/schema.ts`:
- Around line 703-720: Document catalog_meta as allowing at most one row unless
an explicit bootstrap or migration inserts id = 1. Update
packages/db/src/schema.ts lines 703-720,
packages/db/src/model-metadata-schema.test.ts lines 122-129, and
docs/reference/shared-core/database-schema.md lines 293-305 consistently; if
choosing bootstrap seeding instead, make that insertion explicit in the relevant
initialization path and preserve the singleton constraint.
---
Outside diff comments:
In `@docs/decisions/0064-live-model-catalog.md`:
- Around line 77-102: Align collapseAliasDatedPinPairs with the documented
shipped-row gate: require both the rolling alias and its Anthropic dated-pin
sibling to exist as catalog rows before collapsing the dated pin. Otherwise
preserve the dated row, including when only one member is present, and keep
mergeModelCatalog’s full-fidelity output unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dcc5bb9a-95ad-4eaf-ae59-af28c88a12b1
📒 Files selected for processing (83)
apps/cli/src/commands/dispatch.tsapps/cli/src/commands/models.test.tsapps/cli/src/commands/models.tsapps/cli/src/engine/catalog-metadata.test.tsapps/cli/src/engine/catalog-metadata.tsapps/cli/src/engine/catalog-refresh.tsapps/cli/src/engine/model-catalog-view.test.tsapps/cli/src/engine/model-catalog-view.tsapps/cli/src/engine/pricing-overlay.tsapps/cli/src/render/tui/format.tsdocs/analysis/_archive/README.mddocs/analysis/models-dev-dynamic-catalog-enrichment-2026-07-11.mddocs/architecture/agent-sessions.mddocs/architecture/cloud-phase-2.mddocs/architecture/desktop-architecture.mddocs/architecture/execution-model.mddocs/architecture/local-first-and-security.mddocs/architecture/managed-inference.mddocs/architecture/multi-llm-providers.mddocs/architecture/overview.mddocs/architecture/shared-core-engine.mddocs/decisions/0005-sqlite-drizzle-local-postgres-cloud.mddocs/decisions/0022-run-references-workflow-by-uuid.mddocs/decisions/0024-agent-first-entry-point-agentsession.mddocs/decisions/0042-engine-media-storage-substrate-mediastore-deinline-retention.mddocs/decisions/0044-media-access-governance-read-media-save-to-cost.mddocs/decisions/0045-async-media-job-loop-poll-checkpoint-resume-cancel.mddocs/decisions/0050-cli-history-db-at-rest-posture.mddocs/decisions/0064-live-model-catalog.mddocs/decisions/0065-provider-economics-and-extensibility.mddocs/decisions/0070-durable-per-model-session-cost-attribution.mddocs/decisions/0071-models-dev-as-the-model-metadata-source.mddocs/decisions/0072-model-metadata-in-the-db-behind-a-generated-offline-floor.mddocs/decisions/README.mddocs/project-structure.mddocs/reference/README.mddocs/reference/cli/regression-harness.mddocs/reference/contracts/agent-session-spec.mddocs/reference/contracts/config-spec.mddocs/reference/contracts/ipc-contract.mddocs/reference/contracts/sse-event-schema.mddocs/reference/desktop/README.mddocs/reference/desktop/keychain-and-secrets.mddocs/reference/desktop/routes-and-screens.mddocs/reference/desktop/tauri-plugins.mddocs/reference/portal/api-reference.mddocs/reference/shared-core/database-schema.mddocs/reference/shared-core/llm-provider-seam.mddocs/reference/shared-core/store-shapes.mddocs/reference/vscode/extension-api.mddocs/roadmap/README.mddocs/roadmap/deferred-tasks.mddocs/roadmap/phases/phase-0-foundations.mddocs/roadmap/phases/phase-1-engine-and-llm.mddocs/roadmap/phases/phase-2-cli.mddocs/roadmap/phases/phase-2.6-conversational-authoring.mddocs/roadmap/phases/phase-3-desktop.mddocs/roadmap/phases/phase-4-vscode.mddocs/roadmap/phases/phase-5-managed-inference.mddocs/roadmap/phases/phase-6-cloud-execution-portal.mddocs/standards/documentation-style.mddocs/tech-stack.mddocs/tutorials/desktop/build-your-first-workflow.mdpackages/db/README.mdpackages/db/drizzle/0012_greedy_glorian.sqlpackages/db/drizzle/meta/0012_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/client.test.tspackages/db/src/index.tspackages/db/src/metadata-store.test.tspackages/db/src/metadata-store.tspackages/db/src/model-catalog-store.test.tspackages/db/src/model-catalog-store.tspackages/db/src/model-metadata-schema.test.tspackages/db/src/schema.tspackages/llm/src/catalog/catalog-model.tspackages/llm/src/catalog/lookup.test.tspackages/llm/src/catalog/lookup.tspackages/llm/src/catalog/models-dev-schema.test.tspackages/llm/src/catalog/models-dev-schema.tspackages/llm/src/index.tspackages/shared/src/constants.tspackages/shared/src/run.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/decisions/0065-provider-economics-and-extensibility.md
- docs/reference/shared-core/llm-provider-seam.md
- docs/roadmap/phases/phase-2.6-conversational-authoring.md
- docs/reference/contracts/config-spec.md
- packages/llm/src/catalog/models-dev-schema.test.ts
- packages/db/src/model-catalog-store.ts
- apps/cli/src/commands/models.test.ts
- docs/roadmap/deferred-tasks.md
- packages/llm/src/catalog/models-dev-schema.ts
- apps/cli/src/commands/models.ts
| | Frozen section | Seeded living doc(s) | | ||
| |----------------|----------------------| | ||
| | `database` (13-table schema, `workflowDefinitionJSONSchema`, SQLite-vs-Postgres differences, partitioning DDL) | [reference/desktop/database-schema.md](../../reference/desktop/database-schema.md), [reference/contracts/workflow-yaml-spec.md](../../reference/contracts/workflow-yaml-spec.md), [architecture/cloud-phase-2.md](../../architecture/cloud-phase-2.md) | | ||
| | `database` (13-table schema, `workflowDefinitionJSONSchema`, SQLite-vs-Postgres differences, partitioning DDL) | [reference/shared-core/database-schema.md](../../reference/shared-core/database-schema.md), [reference/contracts/workflow-yaml-spec.md](../../reference/contracts/workflow-yaml-spec.md), [architecture/cloud-phase-2.md](../../architecture/cloud-phase-2.md) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not modify the frozen archive provenance map.
This file explicitly declares the archive frozen and says never to edit files in this folder. Revert this change; if the old target is intentionally removed, preserve the historical snapshot and handle link compatibility outside the archive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/analysis/_archive/README.md` at line 57, Revert the change to the
archive provenance entry in README.md and restore its prior target exactly. Do
not modify any files under the frozen archive; if the referenced document was
removed, resolve link compatibility outside the archive while preserving the
historical snapshot.
| - **Status**: Accepted | ||
| - **Date**: 2026-07-05 | ||
| - **Related**: [ADR-0011](0011-internal-llm-abstraction.md) + [ADR-0030](0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md) + [ADR-0031](0031-llm-seam-shape-amendment-multimodal-io.md) (**this ADR amends the `LLMProvider` seam shape — additively**; append-only top-notes added there) · [ADR-0038](0038-agentrunner-llm-call-boundary.md) (host-injected provider resolution — the refresh service reuses it) · [ADR-0050](0050-cli-history-db-at-rest-posture.md) (the cache shares `history.db`; a model list is non-secret) · [ADR-0044](0044-media-access-governance-read-media-save-to-cost.md) + [ADR-0045](0045-async-media-job-loop-poll-checkpoint-resume-cancel.md) (the `model_catalog` **media-routing** consumer this must not regress) · [ADR-0056](0056-cli-in-app-slash-command-system-and-manifest.md) (the `/models` REPL command + `models refresh` shell command) · [ADR-0049](0049-cli-machine-output-contract.md) (`--json`) · [ADR-0059](0059-cli-mid-session-model-reseat.md) (the **other** `/models` — mid-chat reseat, Phase 2.6 — disambiguated below) · [ADR-0063](0063-cli-config-write-contract.md) (the `/models` selection persists the next session's default via its config-write primitive) · [ADR-0065](0065-provider-economics-and-extensibility.md) (**extends** this ADR's merge with a user-pricing tier + custom providers). Canonical homes: the seam signature → [llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md); the `model_catalog` DDL → [database-schema.md](../reference/desktop/database-schema.md); the commands → [commands.md](../reference/cli/commands.md); the static registry → [pricing.ts](../../packages/llm/src/pricing.ts). | ||
| - **Related**: [ADR-0011](0011-internal-llm-abstraction.md) + [ADR-0030](0030-llm-seam-shape-amendment-reasoning-response-format-provider-executed.md) + [ADR-0031](0031-llm-seam-shape-amendment-multimodal-io.md) (**this ADR amends the `LLMProvider` seam shape — additively**; append-only top-notes added there) · [ADR-0038](0038-agentrunner-llm-call-boundary.md) (host-injected provider resolution — the refresh service reuses it) · [ADR-0050](0050-cli-history-db-at-rest-posture.md) (the cache shares `history.db`; a model list is non-secret) · [ADR-0044](0044-media-access-governance-read-media-save-to-cost.md) + [ADR-0045](0045-async-media-job-loop-poll-checkpoint-resume-cancel.md) (the `model_catalog` **media-routing** consumer this must not regress) · [ADR-0056](0056-cli-in-app-slash-command-system-and-manifest.md) (the `/models` REPL command + `models refresh` shell command) · [ADR-0049](0049-cli-machine-output-contract.md) (`--json`) · [ADR-0059](0059-cli-mid-session-model-reseat.md) (the **other** `/models` — mid-chat reseat, Phase 2.6 — disambiguated below) · [ADR-0063](0063-cli-config-write-contract.md) (the `/models` selection persists the next session's default via its config-write primitive) · [ADR-0065](0065-provider-economics-and-extensibility.md) (**extends** this ADR's merge with a user-pricing tier + custom providers). Canonical homes: the seam signature → [llm-provider-seam.md](../reference/shared-core/llm-provider-seam.md); the `model_catalog` DDL → [database-schema.md](../reference/shared-core/database-schema.md); the commands → [commands.md](../reference/cli/commands.md); the static registry → [pricing.ts](../../packages/llm/src/pricing.ts). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the retired pricing registry from the canonical-home list.
This still presents packages/llm/src/pricing.ts as the “static registry,” but ADR-0071 retires MODEL_PRICING in favor of the generated catalog snapshot. Point this entry to the current catalog home or explicitly mark it historical.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/decisions/0064-live-model-catalog.md` at line 5, Update the
canonical-homes sentence in ADR-0064 to remove pricing.ts as the active static
registry, since MODEL_PRICING is retired by ADR-0071. Point the pricing/catalog
reference to the current generated catalog snapshot home, or explicitly label
the pricing.ts reference as historical.
| ### 4.M — Chat export + session persistence | ||
|
|
||
| `relavium.exportChatSession` (session → `.relavium.yaml` scaffold, [ADR-0026](../../decisions/0026-session-export-to-workflow.md)) plus session persistence to the **shared encrypted `history.db`** — the same store as CLI/desktop, so a session started anywhere resumes here. There is **no** extension-host session store; the extension host opens the SQLCipher DB via a **wasm SQLite** build (no native module — respects [ADR-0003](../../decisions/0003-pure-ts-engine-not-langgraph-python.md)). See the cross-host note in [database-schema.md](../../reference/desktop/database-schema.md). | ||
| `relavium.exportChatSession` (session → `.relavium.yaml` scaffold, [ADR-0026](../../decisions/0026-session-export-to-workflow.md)) plus session persistence to the **shared encrypted `history.db`** — the same store as CLI/desktop, so a session started anywhere resumes here. There is **no** extension-host session store; the extension host opens the SQLCipher DB via a **wasm SQLite** build (no native module — respects [ADR-0003](../../decisions/0003-pure-ts-engine-not-langgraph-python.md)). See the cross-host note in [database-schema.md](../../reference/shared-core/database-schema.md). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the cross-host database claim.
This line says CLI, desktop, and VS Code share one encrypted physical history.db, but the canonical schema states that desktop uses SQLCipher while CLI uses unencrypted better-sqlite3; those surfaces cannot share one file. Describe this as a shared schema with surface-specific storage/at-rest handling instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/roadmap/phases/phase-4-vscode.md` at line 376, Update the roadmap
statement around relavium.exportChatSession to remove the claim that CLI,
desktop, and VS Code share one encrypted physical history.db. Describe the
cross-host behavior as a shared canonical schema with surface-specific database
implementations and at-rest encryption handling, preserving the existing wasm
SQLite detail for the extension host.
…alog_meta cardinality) Two still-valid findings from the PR review; the other four were verified and skipped (reasons below). - catalog-metadata.ts (finding 1): tighten PriceTiersSchema — aboveContextTokens and the input/output tier rates must be finite POSITIVE integers, the cache rate a finite NON-NEGATIVE integer, so a corrupt/zero/negative money tier is caught and dropped to the flat rate (cap still engages) instead of trusted. And make `parseJson` schema-derived: `<Out>(text, schema: z.ZodType<Out>)` infers Out FROM the schema — no free generic `T`, no `as` cast, no `any`. The remaining reconciliation is 3 STRUCTURALLY-CHECKED casts at the consumer (Zod's `field?: X | undefined` vs the interfaces' exactOptionalPropertyTypes), which TS verifies — unlike the old unchecked free-`T` cast. - catalog_meta (finding 5): the singleton is "AT MOST one row" (0-or-1, created lazily by the host's first upsertMeta — no bootstrap seed), not "exactly one". Corrected the schema.ts comment, database-schema.md, and the test comment consistently. Skipped (verified against current code): - collapseAliasDatedPinPairs already requires BOTH alias + dated-pin present before collapsing (model-catalog.ts:149) — matches the finding. - ADR-0064 line 5's pricing.ts pointer is already redirected by ADR-0071's existing dated note (lines 54-55); editing the frozen Related line would violate the append-only rule. - docs/analysis/_archive/README.md + phase-4-vscode.md: pre-existing files the ADR-0072 work never touched (the archive edit is from a prior 2.6.Q commit) — out of scope. Refs: ADR-0072 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
…g `validation` Fixes the reported gpt-5.6-luna bug: at high reasoning effort, the turn hard-failed `error: validation`. Root cause is a models.dev data-vs-API mismatch — the catalog claims gpt-5.6-luna accepts `reasoning_effort: ['none','low','medium','high','xhigh','max']`, but the live OpenAI API 400s on high/medium. The capability gate trusts the catalog, so a wrong "accepted" datum reaches the wire and `bad_request` → `validation` hard-fails the chat. The OpenAI-compatible adapter (openai/deepseek/every custom base_url gateway) now SELF-HEALS: on a 400 that names a DROPPABLE tuning param (`reasoning_effort`/`temperature`), it strips that param, retries once, and LEARNS the rejection so `buildCommonBody` withholds it on every later turn — one extra round-trip per (endpoint, model, param), not one per turn. Only pure-tuning params are droppable; `tools`/`messages`/`model`/`response_format` are never auto-stripped (losing them would silently change the request — they stay a loud, honest error). The retry loop is provably bounded (learned-guard + droppable-count cap). Covers both `generate` and the streamed create. Review fold (Opus + Sonnet): - HIGH: the learned-rejection store is module-wide across all adapter instances, so keying on the bare model id let one endpoint poison another's same-id model (a custom gateway serving `gpt-4o` vs real OpenAI). Now keyed by (provider, endpoint, model, param) — endpoints are isolated; the same endpoint correctly shares. - MEDIUM: an authoritative non-droppable `err.param` no longer falls through to the message scan; the message-scan fallback matches the QUOTED param name (not a bare substring) so an unrelated 400 mentioning "temperature" can't learn a spurious permanent rejection. - the `providerOptions` escape hatch is documented out of scope (an explicit verbatim override stays a loud error); removed a needless `as` on the typed `err.param`; tests cover the stream path, temperature, the message-scan fallback, and the bounded-loop guarantee; a file-level afterEach resets the module state between tests. NOTE: this covers the OpenAI-compatible family (the reported case + openai/deepseek/custom). Anthropic/Gemini use nested, provider-specific reasoning params with different error shapes; the same pattern extends there if they ever exhibit the mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/llm/src/adapters/openai.ts (2)
841-877: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStrip learned parameters from
providerOptionstoo.The learned gates only omit mapped fields. If
providerOptionscontainstemperatureorreasoning_effort,escapeadds the rejected parameter back, so both the retry and subsequent calls still fail.Proposed fix
const escape = { ...req.providerOptions }; + for (const param of DROPPABLE_PARAMS) { + if (hasLearnedRejection(provider, endpoint, req.model, param)) { + delete escape[param]; + } + } if (maxTokens !== undefined) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/openai.ts` around lines 841 - 877, Update the providerOptions escape-hatch reconciliation in the request-body builder so learned rejections for temperature and reasoning_effort also remove those keys from escape before returning { ...escape, ...body }. Reuse the existing provider, endpoint, model, and hasLearnedRejection gates, while preserving explicitly supported mapped fields and the existing max-token reconciliation.
946-953: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winGive
OpenAiCompatibleBodyan explicitreasoning_effortwire union.
openAiWireValue()can emit'max', but the inherited OpenAI SDK field is still narrower, so thisas Exclude<...>is just a cast. Override the field locally with the full wire union instead of asserting into the vendor type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/llm/src/adapters/openai.ts` around lines 946 - 953, Update OpenAiCompatibleBody so its reasoning_effort field explicitly uses the complete wire-value union, including values emitted by openAiWireValue such as 'max'. Then remove the Exclude<OpenAiCompatibleBody['reasoning_effort'], undefined> assertion from the assignment and rely on the locally overridden field type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/llm/src/adapters/openai.ts`:
- Around line 538-550: Update the retry loop around createOnce so each
invocation tracks its own rejected parameters separately from global learned
state. Use that per-invocation tracking to prevent retrying the same parameter
within one request, while allowing concurrent identical requests to
independently rebuild without a rejected parameter instead of throwing solely
because hasLearnedRejection reports it.
- Around line 459-475: Update rejectionKey and its callers to scope learned
parameter rejections by the full endpoint, including a normalized custom baseURL
or host rather than only EndpointKind. Ensure distinct custom gateways serving
the same model cannot share rejection state, while preserving shared state for
the same normalized endpoint; review the normalization for security-sensitive
URL handling.
---
Outside diff comments:
In `@packages/llm/src/adapters/openai.ts`:
- Around line 841-877: Update the providerOptions escape-hatch reconciliation in
the request-body builder so learned rejections for temperature and
reasoning_effort also remove those keys from escape before returning {
...escape, ...body }. Reuse the existing provider, endpoint, model, and
hasLearnedRejection gates, while preserving explicitly supported mapped fields
and the existing max-token reconciliation.
- Around line 946-953: Update OpenAiCompatibleBody so its reasoning_effort field
explicitly uses the complete wire-value union, including values emitted by
openAiWireValue such as 'max'. Then remove the
Exclude<OpenAiCompatibleBody['reasoning_effort'], undefined> assertion from the
assignment and rely on the locally overridden field type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e6552c0-0d36-43dd-b419-7ea9c44ec461
📒 Files selected for processing (6)
apps/cli/src/engine/catalog-metadata.tsdocs/reference/shared-core/database-schema.mdpackages/db/src/model-metadata-schema.test.tspackages/db/src/schema.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/db/src/schema.ts
- packages/db/src/model-metadata-schema.test.ts
- docs/reference/shared-core/database-schema.md
- packages/llm/src/adapters/openai.test.ts
Comment on lines
459
to
+475
| /** | ||
| * Id SEGMENTS that are NOT chat-completions text models — DENIED from the OpenAI/DeepSeek live list | ||
| * (ADR-0064 §3). The OpenAI `/v1/models` list is id-only (no capability metadata), so the filter is an | ||
| * id-family heuristic: deny wins over allow, so `gpt-image-1` / `gpt-4o-audio-preview` / `omni-moderation` | ||
| * are dropped even though they match a `gpt`/`o` allow-family. Each token is matched on a `-`/`_` SEGMENT | ||
| * boundary (not a bare substring), so `search` denies `gpt-4o-search-preview` but NOT `o3-deep-research` | ||
| * (re**search**), and `dall-e`'s internal `-` is a literal segment. The tail entries | ||
| * (`instruct`/`ocr`/`davinci`/`babbage`) drop non-chat completion families that otherwise pass the | ||
| * gpt/deepseek allow-family; all are priced-rescue-safe (the `pricedIds.has(id)` short-circuit wins first). | ||
| * `(provider, endpoint, model, param)` tuples the live API has rejected this process. The key is scoped to the | ||
| * ENDPOINT (provider + official-vs-custom `base_url`), NOT the bare model id: this store is module-wide across every | ||
| * adapter instance, and a custom gateway can serve a familiar id (`gpt-4o`) that 400s on a param the real OpenAI | ||
| * `gpt-4o` accepts — keying on the model alone would let one endpoint silently poison another's same-id model. | ||
| * | ||
| * Learning is PARAM-level, not (param, tier)-level: a 400 on `reasoning_effort: high` withholds reasoning_effort | ||
| * ENTIRELY for that model, even a `low`/`medium` it might accept — a deliberate degrade (the catalog datum that | ||
| * claimed the tier was already proven unreliable, and the goal is to never hard-fail again). | ||
| */ | ||
| const OPENAI_DENY_SUBSTRINGS = [ | ||
| 'embedding', | ||
| 'tts', | ||
| 'whisper', | ||
| 'image', | ||
| 'moderation', | ||
| 'realtime', | ||
| 'audio', | ||
| 'dall-e', | ||
| 'transcribe', | ||
| 'search', | ||
| 'instruct', | ||
| 'ocr', | ||
| 'davinci', | ||
| 'babbage', | ||
| ] as const; | ||
|
|
||
| /** Escape a literal string for embedding inside a `RegExp`. The deny tokens carry no metacharacters today | ||
| * (`dall-e`'s `-` is literal outside a character class), but this keeps the boundary match safe if one is | ||
| * ever added. */ | ||
| function escapeRegExp(text: string): string { | ||
| // `String.raw` avoids the doubled backslash of `'\\$&'` — the replacement is a literal `\` + the `$&` match ref. | ||
| return text.replace(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`); | ||
| const learnedParamRejections = new Set<string>(); | ||
| const rejectionKey = ( | ||
| provider: ProviderId, | ||
| endpoint: EndpointKind, | ||
| model: string, | ||
| param: string, | ||
| ): string => `${provider} ${endpoint} ${model} ${param}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect relevant symbols around the reported lines.
ast-grep outline packages/llm/src/adapters/openai.ts --view expanded > /tmp/openai_outline.txt || true
wc -l packages/llm/src/adapters/openai.ts
sed -n '430,520p' packages/llm/src/adapters/openai.ts
printf '\n--- OUTLINE ---\n'
sed -n '1,220p' /tmp/openai_outline.txtRepository: HodeTech/Relavium
Length of output: 10290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '430,520p' packages/llm/src/adapters/openai.tsRepository: HodeTech/Relavium
Length of output: 4998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('packages/llm/src/adapters/openai.ts')
text = p.read_text()
for start,end in [(430,520),(900,980),(1260,1320)]:
print(f"\n===== {start}-{end} =====")
lines = text.splitlines()
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4}: {lines[i]}")
PYRepository: HodeTech/Relavium
Length of output: 14998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('packages/llm/src/adapters/openai.ts')
lines = p.read_text().splitlines()
for start,end in [(430,520),(900,980),(1260,1320)]:
print(f"\n===== {start}-{end} =====")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4}: {lines[i]}")
PYRepository: HodeTech/Relavium
Length of output: 14998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "type EndpointKind|enum EndpointKind|EndpointKind|learnedParamRejections|rejectionKey|base_url|baseURL" packages/llm/src/adapters/openai.ts packages/llm/src -g '!**/dist/**'Repository: HodeTech/Relavium
Length of output: 11677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '90,110p' packages/llm/src/adapters/openai.ts
sed -n '20,40p' packages/llm/src/output-cap.tsRepository: HodeTech/Relavium
Length of output: 2584
Scope learned rejections by the full endpoint, not just official vs custom. EndpointKind collapses every custom baseURL into the same bucket, so one gateway can poison retries for another gateway serving the same model id. Include a normalized baseURL/host in the key, or move this cache off the module scope. This touches custom provider base URLs and needs explicit security review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/llm/src/adapters/openai.ts` around lines 459 - 475, Update
rejectionKey and its callers to scope learned parameter rejections by the full
endpoint, including a normalized custom baseURL or host rather than only
EndpointKind. Ensure distinct custom gateways serving the same model cannot
share rejection state, while preserving shared state for the same normalized
endpoint; review the normalization for security-sensitive URL handling.
Source: Coding guidelines
Comment on lines
+538
to
+550
| for (let stripped = 0; ; stripped++) { | ||
| try { | ||
| return await createOnce(); | ||
| } catch (err) { | ||
| const param = rejectedDroppableParam(err); | ||
| if ( | ||
| param === undefined || | ||
| hasLearnedRejection(provider, endpoint, model, param) || | ||
| stripped >= DROPPABLE_PARAMS.size | ||
| ) { | ||
| throw err; | ||
| } | ||
| learnParamRejection(provider, endpoint, model, param); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let concurrent requests suppress each other’s fallback.
If two identical requests fail concurrently, the first learns the rejection; the second then sees hasLearnedRejection(...) and throws instead of rebuilding without the parameter. Track parameters retried by this invocation separately from global learned state.
Proposed fix
+ const retriedParams = new Set<string>();
for (let stripped = 0; ; stripped++) {
try {
return await createOnce();
} catch (err) {
const param = rejectedDroppableParam(err);
if (
param === undefined ||
- hasLearnedRejection(provider, endpoint, model, param) ||
+ retriedParams.has(param) ||
stripped >= DROPPABLE_PARAMS.size
) {
throw err;
}
+ retriedParams.add(param);
learnParamRejection(provider, endpoint, model, param);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (let stripped = 0; ; stripped++) { | |
| try { | |
| return await createOnce(); | |
| } catch (err) { | |
| const param = rejectedDroppableParam(err); | |
| if ( | |
| param === undefined || | |
| hasLearnedRejection(provider, endpoint, model, param) || | |
| stripped >= DROPPABLE_PARAMS.size | |
| ) { | |
| throw err; | |
| } | |
| learnParamRejection(provider, endpoint, model, param); | |
| const retriedParams = new Set<string>(); | |
| for (let stripped = 0; ; stripped++) { | |
| try { | |
| return await createOnce(); | |
| } catch (err) { | |
| const param = rejectedDroppableParam(err); | |
| if ( | |
| param === undefined || | |
| retriedParams.has(param) || | |
| stripped >= DROPPABLE_PARAMS.size | |
| ) { | |
| throw err; | |
| } | |
| retriedParams.add(param); | |
| learnParamRejection(provider, endpoint, model, param); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/llm/src/adapters/openai.ts` around lines 538 - 550, Update the retry
loop around createOnce so each invocation tracks its own rejected parameters
separately from global learned state. Use that per-invocation tracking to
prevent retrying the same parameter within one request, while allowing
concurrent identical requests to independently rebuild without a rejected
parameter instead of throwing solely because hasLearnedRejection reports it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2.6.Q — models.dev becomes the model-metadata source; the hand-typed registry is retired
Implements ADR-0071. The 12-row hand-typed
MODEL_PRICINGtable is gone; price, context window, output ceiling, and the per-model reasoning control now come from a generated models.dev snapshot that ships in the binary and answers offline. The reasoning control's shape is per-model data, not a per-adapter assumption — which fixes a live bug (/efforton Gemini 2.5 was sendingthinkingLevel, a parameter Google's docs say those models do not take).Everything downstream of a price is a safety control (the ADR-0028 cost cap) or a wire parameter (the reasoning tier), so this touched a lot of money paths. Each step was landed under the same discipline: implement → commit → Opus review → fold → commit → Sonnet review → fold → commit. The adversarial rounds earned their keep — they caught, among others, a downgrade that recorded $14.50 of real spend as $0.00, an unpriced-model notice wired at 2 of 9 surfaces, and a
WorkflowEnginethat took two injected deps and dropped them on the floor.The nine steps
acceptedTiersbridge;reasoning-wire.tsas the one home for tier→wire394acb779e0d952d7b051344bc831395b9afbb83cabda0b10edaffac466445emax_completion_tokensdialect rule (official vs custom endpoint)903033994636a1ce6cfbfMODEL_PRICINGdeleted, priced from the catalog, precedence flipped to user → catalog5216c430b0b9b2e9c4367models refresh --providers/--catalog+[catalog] auto_refresh(default OFF), additive-only, shipped snapshot is the floordddb0013f3d08477c88acstrict_cost_capto refuse it785468d70f086e3945c0dec0b2ee21943b495a5880Notable decisions
user → catalogprecedence (a deliberate reversal). The catalog is a snapshot of a third-party aggregator; the user holds the invoice. Amodels pricingrow now overrides a catalog price instead of being refused — and the divergence is loud (the command names the catalog price it replaces;--clearreverts it).models refreshnever touches a shipped model's price — that stays a human, release-time decision (§9). It adds the long tail and can never make a known model cheaper, because it does not write one.strict_cost_capturns the silent degrade-to-allow on an unpriced model into a hard pre-egress refusal, for a user who set a cap specifically to bound an untrusted model.Guarantees held
ProviderIdstays the closed enum (four adapters); adding a provider is one entry + oneCATALOG_PROVIDER_KEYSline, anddocs/runbooks/add-a-provider.mdsays exactly that.@relavium/llmseam;packages/corestays platform-pure (the refresh fetch lives host-side inapps/cli/src/engine/).fetch, existing Zod. models.dev is fetched from a compile-time-constant host, HTTPS-only, no credentials, Zod-validated at the boundary.cached_input_statedflag) is additive and verified on a copy of the real 3.5 MBhistory.db— 84 catalog rows and 52 sessions intact.Toolchain
pnpm turbo run lint typecheck test build— 24/24 green. Test counts:@relavium/llm642 (+11 skipped live/effort),@relavium/core1001,apps/cli2187, plus shared/db/mcp.Follow-ups filed (docs/roadmap/deferred-tasks.md)
create-pull-requestaction pinned to a verified SHA — not invented unseen, per rule 3).cache_readand nocache_write; scaling one would be a fabrication on a money path).generate()ceiling (the SDK refuses a largemax_tokens; the clamp cannot reach under it).🤖 Generated with Claude Code
Summary by CodeRabbit
models refresh(--providers/--catalog) and improvedmodels pricing --clear.Follow-on: ADR-0072 — model metadata in the DB behind the offline snapshot floor
Builds directly on the ADR-0071 work above. The maintainer's ask: stop keeping models.dev data only in the binary — mirror it into
history.db, refresh it on a schedule / onCtrl+R, carry the full field set (modalities,tool_call, knowledge cutoff, description,structured_output, …), and add a per-model visibility flag for a future/settings > /modelstoggle.The load-bearing constraint: the generated snapshot is not incidental static data — it is the offline floor of a safety control (the ADR-0028 cost cap must price every model offline, first-run, no network). A wrong price still engages the cap; a missing price does not. So the design keeps the snapshot as a money-and-wire floor UNDER a DB-backed catalog — the DB is the live, refreshed source; the binary snapshot is the terminal fallback. "No static-in-binary" is reframed as retire the hand-typed
MODEL_PRICING(done in ADR-0071) and the~/.relaviumfile cache — not the generated safety floor. Full rationale in ADR-0072 (accepted after a four-bot adversarial review of the draft; it partially supersedes ADR-0064 §4 and amends ADR-0071 §3/§4/§9, recorded append-only).The phases (same discipline: implement → commit → Opus review → fold → Sonnet review → fold → commit)
951fe5fmodel_metadata+catalog_metatables +model_catalog.visible(migration 0012); money CHECKs at rest772f843admitRefreshedModelsgate extracted to pure@relavium/llm+CatalogModelenrichment fields109633f@relavium/db-puremodel_metadatastore (row CRUD + cursor + enrichment-only write)27b9d68fb2b57avisiblepicker read-side (hard filter, provider-anchored, never-clobber)3ceaa7fMoney-safety invariants (the point of the whole thing)
catalogModel = refreshed[id] ?? CATALOG_SNAPSHOT[id]is byte-unchanged; a shipped id always resolves to the snapshot (the gate excludes it by compile-time snapshot membership, never a mutable DBorigin). A fresh/torn/older-schema DB degrades to the snapshot, offline.admitRefreshedModels(finite-and-positive price —NaNcaught by the finite check first; snapshot-membership exclusion). A parity test asserts they admit the identical set.NULLcache rate →undefined, never0across the DB round-trip (a0would price a cached read free).@relavium/coreand pure@relavium/llmstay@relavium/db-free —catalog-metadata.ts(apps/cli) is the one host join; the engine receives the overlay through the pureinstallCatalogRefreshseam.The adversarial rounds earned their keep here too — they caught a wholesale-replace bug that would have wiped a good file-cache overlay and unpriced its long tail, an enrichment-reset-on-every-release regression, an unsafe
JSON.parse as Ton a money column, and a cross-provider visibility collision (hiding one provider's model dropping another's same-id model, ADR-0065 §6).Two deliberate calls for review
pnpm sync:modelssurfaces unrelated gemini-flash price increases (gemini-flash-latestinput 30M→150M, output 250M→900M µ¢/Mtok) that the §9 money guard correctly refuses to take silently — those are a separate maintainer money decision (pnpm sync:models --accept-price-changeswhen ready). Enrichment reaches the DB via the refresh path regardless; the nightlymodels-catalog.ymlis not a PR gate.~/.relaviumfile cache) is deferred. ADR-0072 point 7 keeps the file cache "until the DB path is proven" — removing the transitional fallback in the same PR that introduces the DB path would strip the safety net exactly when it's most needed. The/settings > /modelstoggle UI (the write path forvisible) is likewise future, as framed; P5 ships the live read-side so it's a small addition later.