Phase 2.6.Q — models.dev as the model-metadata source (ADR-0071)#76
Open
cemililik wants to merge 52 commits into
Open
Phase 2.6.Q — models.dev as the model-metadata source (ADR-0071)#76cemililik wants to merge 52 commits into
cemililik wants to merge 52 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 hand-maintained model metadata with a generated models.dev catalog, adds guarded refresh and override flows, introduces per-model reasoning controls, and wires endpoint-aware pricing, strict unpriced-model governance, provider-aware defaults, CLI/TUI notices, persistence, tests, and documentation. ChangesCatalog foundation
Runtime behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cli/src/commands/chat.ts (1)
47-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge duplicate
effort-notice.jsimports.SonarCloud flags
'../chat/effort-notice.js'as imported multiple times — the line-range details show a separate import ofeffortRejectedNote/effortUnavailableNote/onceEffortNoticefrom the same module elsewhere alongside thiseffortTiersForimport. Consolidate into a single import statement.♻️ Proposed fix
-import { effortRejectedNote, effortUnavailableNote, onceEffortNotice } from '../chat/effort-notice.js'; -... -import { effortTiersFor } from '../chat/effort-notice.js'; +import { + effortRejectedNote, + effortTiersFor, + effortUnavailableNote, + onceEffortNotice, +} from '../chat/effort-notice.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 `@apps/cli/src/commands/chat.ts` around lines 47 - 48, Consolidate the duplicate imports from ../chat/effort-notice.js in chat.ts into one import declaration containing effortRejectedNote, effortUnavailableNote, onceEffortNotice, and effortTiersFor; preserve all existing usages and remove the separate declaration.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 `@apps/cli/src/chat/effort-notice.ts`:
- Around line 1-3: Update the workspace dependency resolution for apps/cli so
`@relavium/core` and `@relavium/shared` resolve to their workspace source or
generated declarations before typechecking; use the existing project references
or path configuration rather than changing effort-notice.ts. Ensure
seamEffortTiersFor and EffortGateResult are available to eliminate the
downstream no-unsafe errors.
---
Nitpick comments:
In `@apps/cli/src/commands/chat.ts`:
- Around line 47-48: Consolidate the duplicate imports from
../chat/effort-notice.js in chat.ts into one import declaration containing
effortRejectedNote, effortUnavailableNote, onceEffortNotice, and effortTiersFor;
preserve all existing usages and remove the separate declaration.
🪄 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: 8922c721-7616-4854-b614-774c8275b482
📒 Files selected for processing (31)
.claude/skills/add-llm-adapter/SKILL.mdapps/cli/src/chat/effort-notice.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/models-pricing.tsapps/cli/src/commands/models.tsapps/cli/src/commands/specs-forwarding.test.tsapps/cli/src/commands/specs.tsapps/cli/src/config/load.tsapps/cli/src/engine/build-engine.tsapps/cli/src/engine/catalog-refresh.tsapps/cli/src/engine/providers.tsapps/cli/src/harness/regression.e2e.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/render/sanitize.tsapps/cli/src/render/tui/chat-projection.tsapps/cli/src/render/tui/effort-picker.test.tsapps/cli/src/render/tui/effort-picker.tsapps/cli/src/render/tui/model-picker.tsdocs/decisions/0065-provider-economics-and-extensibility.mddocs/roadmap/deferred-tasks.mdpackages/db/drizzle/meta/0011_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/model-catalog-store.tspackages/llm/src/adapters/anthropic.tspackages/llm/src/adapters/openai.tspackages/llm/src/catalog/models-dev-schema.tspackages/llm/src/fallback-chain.tspackages/llm/src/model-kind.tspackages/llm/src/pricing.test.tstools/sync-models-dev/sync.mjs
💤 Files with no reviewable changes (1)
- apps/cli/src/home/drive-home.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/cli/src/config/load.ts
- packages/db/drizzle/meta/_journal.json
- docs/decisions/0065-provider-economics-and-extensibility.md
- tools/sync-models-dev/sync.mjs
- apps/cli/src/commands/specs.ts
- packages/llm/src/fallback-chain.ts
- apps/cli/src/engine/build-engine.ts
- apps/cli/src/render/tui/effort-picker.test.ts
- apps/cli/src/commands/models.ts
- apps/cli/src/engine/catalog-refresh.ts
- apps/cli/src/render/tui/model-picker.ts
- apps/cli/src/chat/session-host.ts
- packages/db/src/model-catalog-store.ts
Comment on lines
+1
to
+3
| import type { EffortGateResult } from '@relavium/core'; | ||
| import { catalogModel, effortTiersFor as seamEffortTiersFor } from '@relavium/llm'; | ||
| import { REASONING_EFFORTS, type ReasoningEffort } from '@relavium/shared'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== `@relavium/llm` effortTiersFor export ==="
rg -n "export (function|const) effortTiersFor" packages/llm/src
echo "=== `@relavium/core` EffortGateResult export ==="
rg -n "EffortGateResult" packages/core/src -g '*.ts' -C2
echo "=== package.json 'main'/'exports' + built dist presence ==="
fd -a 'package.json' packages/llm packages/core --exec cat -n {} \;Repository: HodeTech/Relavium
Length of output: 7408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== apps/cli/src/chat/effort-notice.ts ==="
cat -n apps/cli/src/chat/effort-notice.ts
echo
echo "=== workspace tsconfig / eslint resolver hints ==="
fd -a 'tsconfig*.json' . | sed -n '1,40p'
echo
rg -n '"paths"|"references"|projectReferences|typescript-eslint|import/resolver|preserveSymlinks' \
-g 'tsconfig*.json' -g '.eslintrc*' -g 'eslint.config.*' -g 'package.json' .
echo
echo "=== apps/cli package metadata ==="
fd -a 'package.json' apps/cli --exec cat -n {} \;Repository: HodeTech/Relavium
Length of output: 13066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== apps/cli/tsconfig.json ==="
cat -n apps/cli/tsconfig.json
echo
echo "=== packages/core/tsconfig.json ==="
cat -n packages/core/tsconfig.json
echo
echo "=== packages/llm/tsconfig.json ==="
cat -n packages/llm/tsconfig.json
echo
echo "=== root package/tsconfig references ==="
cat -n tsconfig.json 2>/dev/null || true
cat -n package.json | sed -n '1,120p'Repository: HodeTech/Relavium
Length of output: 5206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== dist presence for workspace packages ==="
for d in packages/core/dist packages/llm/dist packages/shared/dist apps/cli/dist; do
if [ -d "$d" ]; then
echo "-- $d"
find "$d" -maxdepth 2 -type f | sed -n '1,40p'
else
echo "-- $d (missing)"
fi
done
echo
echo "=== root tsconfig.base.json ==="
cat -n tsconfig.base.json
echo
echo "=== workspace package import resolution clues ==="
rg -n '"moduleResolution"|"paths"|"baseUrl"|"composite"|"references"|"declaration"' \
tsconfig.base.json packages/core/tsconfig.json packages/llm/tsconfig.json apps/cli/tsconfig.jsonRepository: HodeTech/Relavium
Length of output: 2231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tsc availability ==="
command -v tsc || true
tsc --version || true
echo
echo "=== resolution probe for apps/cli imports ==="
tsc -p apps/cli/tsconfig.json --noEmit --traceResolution 2>&1 \
| rg -n "`@relavium/`(core|llm)|effort-notice|dist/index\\.d\\.ts|Module name|resolved to|was not resolved" \
-C 2Repository: HodeTech/Relavium
Length of output: 50374
Build the workspace deps before apps/cli typecheck
@relavium/core and @relavium/shared resolve to dist/index.d.ts here, but those files are missing in this workspace layout. That leaves seamEffortTiersFor(model) and EffortGateResult unresolved and triggers the no-unsafe-* errors on 75-78 and 87-96. Either point apps/cli at the workspace source via project references/paths or ensure the packages are built first.
🤖 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 `@apps/cli/src/chat/effort-notice.ts` around lines 1 - 3, Update the workspace
dependency resolution for apps/cli so `@relavium/core` and `@relavium/shared`
resolve to their workspace source or generated declarations before typechecking;
use the existing project references or path configuration rather than changing
effort-notice.ts. Ensure seamEffortTiersFor and EffortGateResult are available
to eliminate the downstream no-unsafe errors.
Source: Linters/SAST tools
…uplicate import The linear-vs-quadratic wrap-shape ratio is GC-dominated on a shared CI runner: the 8x-larger case allocates 8x as much and pays GC the small case dodges, so even a ratio of minimums is unbounded. It read 11.47 then 22.17 on two successive CI runs for PROVABLY-LINEAR code — the second above the ~16.6 the injected quadratic produces, i.e. a linear reading indistinguishable from the bug it exists to catch. A test that is red regardless of the code is not a guard, so skip it under CI (`it.skipIf(IN_CI)`): it runs on a warm local box where the ratio measures the algorithm (real ~4.8, injected O(n^2) ~16.6) and belongs on a nightly perf lane, not the per-PR gate. The CI-side regression signal for this same path stays the latency budget test (`< 100ms`, 3–4x headroom, green on both CI runs). Also fold a Sonar nit from the effort-notice repoint: chat.ts imported ../chat/effort-notice.js twice — merge into one declaration. (The CodeRabbit "@relavium/core/@relavium/shared don't resolve → no-unsafe" finding is a false positive: typecheck + lint are clean; the imports resolve via workspace project references — the tool analyzed the file without the built deps.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/cli/src/render/tui/chat-projection.test.ts (1)
1091-1095: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep the complexity guard runnable in an explicit performance lane.
it.skipIf(IN_CI)skips this test in every CI environment, including the dedicated nightly performance lane described in the comment. That removes the only automated check for the O(n²) regression; the latency-budget test is explicitly insufficient for that case. Keep it out of the per-PR gate, but add an opt-in override such asRUN_WRAP_SCALE_GUARD=1for nightly CI, or document that this guard is manual-only.Also applies to: 1151-1164
🤖 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 `@apps/cli/src/render/tui/chat-projection.test.ts` around lines 1091 - 1095, Update the complexity-guard test’s skip condition to continue excluding normal CI while allowing an explicit performance override such as RUN_WRAP_SCALE_GUARD=1. Apply the same condition to the related guard block around the additional referenced lines, preserving local execution and keeping the latency-budget test behavior 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.
Nitpick comments:
In `@apps/cli/src/render/tui/chat-projection.test.ts`:
- Around line 1091-1095: Update the complexity-guard test’s skip condition to
continue excluding normal CI while allowing an explicit performance override
such as RUN_WRAP_SCALE_GUARD=1. Apply the same condition to the related guard
block around the additional referenced lines, preserving local execution and
keeping the latency-budget test behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f4d9172-f3d0-4045-b184-10401f824bab
📒 Files selected for processing (2)
apps/cli/src/commands/chat.tsapps/cli/src/render/tui/chat-projection.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cli/src/commands/chat.ts
…f-guard env override CI `format:check` (a gate separate from lint/typecheck/test, which is why the earlier folds passed while this went red) flagged 27 files of prettier drift — most predating this work, some from the review folds. Run the repo's own `prettier --write` so the committed tree matches the pinned prettier; changes are whitespace-only. Also fold a reviewer nitpick on the reseat-carry SHAPE guard: gate its CI-skip on `RUN_WRAP_SCALE_GUARD !== '1'` so a dedicated perf lane can still FORCE the (locally stable, GC-flaky-on-shared-CI) ratio on a runner it controls, while the shared per-PR gate keeps skipping it. The latency-budget test is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/chat/effort-notice.test.ts (1)
40-50: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winStrengthen the malicious-ID sanitization assertions.
The fixture does not contain a BEL character, so the BEL assertion is vacuous. Also,
toContain('model')can pass from static notice text even if the supplied model ID is dropped entirely. Include BEL in the input and assert preservation of user-provided fragments such asevilandsecond-line.🤖 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 `@apps/cli/src/chat/effort-notice.test.ts` around lines 40 - 50, The sanitization test for effortRejectedNote must exercise BEL removal and verify that model-ID content is preserved. Update the malicious model ID fixture to include a BEL character, and replace the ambiguous model assertion with checks that the supplied fragments “evil” and “second-line” remain in the sanitized notice while existing ESC and newline exclusions stay intact.
🧹 Nitpick comments (1)
apps/cli/src/engine/model-catalog-view.test.ts (1)
246-249: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep unpriced models distinct from zero-priced models.
catalogPricing(id)returnsundefinedfor unknown models, but?? 0converts that into a free price. Make this helper fail for unknown IDs or returnundefinedso the new unpriced-model tests cannot pass with a false zero expectation.Suggested fix
-): number => catalogPricing(id)?.[field] ?? 0; +): number => { + const pricing = catalogPricing(id); + if (pricing === undefined) { + throw new Error(`Missing catalog pricing for ${id}`); + } + return pricing[field]; +};As per PR objectives, unpriced models must remain distinguishable from zero-priced models.
🤖 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 `@apps/cli/src/engine/model-catalog-view.test.ts` around lines 246 - 249, Update the catalogRate helper to preserve the distinction between unknown and zero-priced models: return undefined when catalogPricing(id) is undefined instead of coercing it to 0, while retaining actual zero values for explicitly free models. Adjust the helper’s return type and any dependent assertions or expectations accordingly.
🤖 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.
Outside diff comments:
In `@apps/cli/src/chat/effort-notice.test.ts`:
- Around line 40-50: The sanitization test for effortRejectedNote must exercise
BEL removal and verify that model-ID content is preserved. Update the malicious
model ID fixture to include a BEL character, and replace the ambiguous model
assertion with checks that the supplied fragments “evil” and “second-line”
remain in the sanitized notice while existing ESC and newline exclusions stay
intact.
---
Nitpick comments:
In `@apps/cli/src/engine/model-catalog-view.test.ts`:
- Around line 246-249: Update the catalogRate helper to preserve the distinction
between unknown and zero-priced models: return undefined when catalogPricing(id)
is undefined instead of coercing it to 0, while retaining actual zero values for
explicitly free models. Adjust the helper’s return type and any dependent
assertions or expectations accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2509ac56-252e-4967-b975-f4b7e927e567
📒 Files selected for processing (20)
apps/cli/src/chat/effort-notice.test.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/models-dispatch.test.tsapps/cli/src/commands/specs-forwarding.test.tsapps/cli/src/commands/specs.tsapps/cli/src/engine/catalog-refresh.tsapps/cli/src/engine/model-catalog-view.test.tsapps/cli/src/render/tui/chat-projection.test.tsapps/cli/src/render/tui/effort-picker.test.tsapps/cli/src/render/tui/model-picker.tspackages/core/src/engine/budget-governor.test.tspackages/llm/src/adapters/gemini.test.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.tspackages/llm/src/catalog/lookup.test.tspackages/llm/src/conformance/effort.conformance.test.tspackages/llm/src/cost-tracker.test.tspackages/llm/src/model-catalog.test.tspackages/llm/src/output-cap.test.tspackages/llm/src/reasoning-wire.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/llm/src/output-cap.test.ts
- apps/cli/src/commands/specs-forwarding.test.ts
- packages/llm/src/catalog/lookup.test.ts
- packages/core/src/engine/budget-governor.test.ts
- apps/cli/src/render/tui/chat-projection.test.ts
- apps/cli/src/commands/specs.ts
- packages/llm/src/adapters/gemini.test.ts
- packages/llm/src/cost-tracker.test.ts
- packages/llm/src/reasoning-wire.test.ts
- apps/cli/src/commands/models-dispatch.test.ts
- apps/cli/src/render/tui/effort-picker.test.ts
- packages/llm/src/conformance/effort.conformance.test.ts
- apps/cli/src/engine/catalog-refresh.ts
- packages/llm/src/adapters/openai.test.ts
- apps/cli/src/render/tui/model-picker.ts
- packages/llm/src/model-catalog.test.ts
- packages/llm/src/adapters/openai.ts
- apps/cli/src/commands/chat.ts
…ferrable id starts Bug 3 of the live model-picker reports. After a provider list-refresh, an OpenAI id `keepOpenAiModelId` admits by substring (`chat-latest` / `chatgpt-4o-latest`) appears in the picker; selecting it and starting a chat crashed "cannot infer a provider for chat model 'chat-latest'". Root cause: the bare-Home default-model WRITE path dropped the provider — known authoritatively at pick time (the live `/models` list is per-provider) — so the next chat re-inferred it from the opaque id, and the prefix map places no `chat*`. Fix (ADR-0059 "the provider is authoritative, never re-inferred", extended to the config path): add an optional `[preferences]/[chat].default_provider` (a closed ProviderId enum) to the config schema; persist it alongside `default_model` in the `/models` picker (home-controller `writeDefault`) and the onboarding wizard, in one atomic `writeGlobalPreferences` write; resolve it (config/resolve) with the same precedence as `default_model`; and pass it to `buildDefaultChatAgent` (agent-source → session-host), which uses it VERBATIM and skips inference. As a fallback hardening, `inferProviderFromModel` is now catalog-first (a catalog-known id resolves regardless of prefix). `keepOpenAiModelId` is deliberately NOT tightened — `chatgpt-4o-latest` is a real chat model, and ADR-0064 §6 is "dim, never hide". Backward-compatible: an old config with no `default_provider` still loads and falls back to inference. Docs: config-spec.md (both blocks + the write-driver note) and an append-only ADR-0063 note. Tests: buildDefaultChatAgent uses a persisted provider for an unprefixed id; the picker + wizard persist the provider; config round-trips + resolves default_provider. Toolchain green (shared 459, cli 2196). Refs: ADR-0059, ADR-0063 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g-3 review fold)
Folds the adversarial review of the Bug-3 provenance fix (6 confirmed findings) + two
test nitpicks.
Correctness (the one real defect the review caught)
- config/resolve: `default_model` + `default_provider` were resolved with INDEPENDENT
per-layer `??` fallbacks. A project pinning only `[chat] default_model` (a claude id —
the common case, since the field is brand new and claude ids were always inferable)
paired with a global `[preferences] default_provider = "openai"` (written by the wizard)
would build an OpenAI agent over a Claude id → egress 404, breaking a previously-working
config. Resolve the pair from the SAME layer that supplies the model (absent ⇒ inference),
the exact coupling `allowed_commands`/`allowed_command_globs` already use. Regression test
pins the no-leak.
Test coverage (the fix's decisive hops were only pinned at the innermost fn)
- agent-source: resolveChatAgent passes a persisted provider verbatim (unplaceable id
builds; absent provider still throws).
- session-host: buildChatSession carries chat.defaultProvider into the default agent.
Docs / comments
- config-spec: the `[chat]` resolution paragraph now documents default_provider as a
second coupled group. write.ts: "two-key" → three-key setter; the defaultProvider
no-op comment corrected (a provider-only write is a real partial write). default-agent
test: soften the catalog-first comment (all shipped ids also match a prefix, so it
cannot prove first-ness).
Two test nitpicks (first-fold files)
- effort-notice: the BEL-removal assertion was vacuous (no BEL in the fixture) — add one,
and assert the legible content ('evil'/'second-line') survives sanitization.
- model-catalog-view: `catalogRate` coerced an unknown rate to 0 (conflating unknown with
free) — return `number | undefined` and assert real rates at the call site.
Toolchain: format:check + lint + typecheck + test green (shared 459, cli 2199).
Refs: ADR-0059, ADR-0063
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bug 2 of the live model-picker reports. Two Haiku rows showed — "Claude Haiku 4.5" (selectable) and "Claude Haiku 4.5 (latest)" (dimmed "unavailable on your key") — despite an Anthropic key. Root cause: availability is decided by EXACT model-id membership in the provider's live list, and Anthropic's `models.list()` returns only the dated pin (`claude-haiku-4-5-20251001`), not the rolling alias (`claude-haiku-4-5`), while the catalog ships BOTH as priced rows — so the alias dimmed as `not-on-key` even though the SAME key calls it (both resolve server-side to the same model). Fix (pure merge, benefits CLI + desktop + VS Code alike): before dimming a live-omitted model, `resolveAvailability` now consults `hasLiveSibling` — a live-omitted model is available when its alias↔dated-pin sibling (`base` ↔ `base-YYYYMMDD`) IS in the live list AND is itself a shipped catalog row of the same provider. The catalog-row gate stops this fabricating availability for an arbitrary unpriced id; a model whose provider genuinely does not serve it still dims. Scoped to Anthropic this round (the maintainer's call — the OpenAI gpt-4o dated family is out of scope). Corrected the anthropic conformance fixture to return the dated pin (real behavior; it had the reverse), so the suite exercises the rescued-alias case. Refines ADR-0064 §6 (append-only note); does not reverse it. Tests: the alias goes available when its dated pin is live and vice versa; a non-catalog sibling id cannot fabricate availability. Toolchain green (llm 644, cli 2199). Refs: ADR-0064, ADR-0071 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/cli/src/chat/default-agent.ts (1)
50-52: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor: catalog lookup is case-sensitive, prefix fallback is not.
catalogModel(model)does a direct object-key lookup while the fallback lower-cases the id first (Line 53m = model.toLowerCase()). A catalog-known id passed in non-canonical casing silently skips the "authoritative" catalog branch and falls to the heuristic — usually harmless since both agree for shipped ids, but not guaranteed if they ever diverge.♻️ Optional normalization
- const cataloged = catalogModel(model)?.provider; + const cataloged = catalogModel(model.toLowerCase())?.provider ?? catalogModel(model)?.provider;🤖 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 `@apps/cli/src/chat/default-agent.ts` around lines 50 - 52, Update inferProviderFromModel to normalize the model identifier before the catalogModel lookup, matching the lowercase normalization already used by the prefix fallback. Preserve the catalog result as authoritative and only apply the fallback when the normalized lookup returns no provider.
🤖 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/chat/session-host.ts`:
- Around line 615-618: Update the resolveEndpoint callback in the session host
configuration to use the provider bound to the selected model rather than
catalogModel(model)?.provider with an OpenAI fallback. Thread that bound
provider through the surrounding model/governor setup so Anthropic and custom
providers resolve to the correct endpoint kind and retain proper clamping.
In `@apps/cli/src/config/resolve.ts`:
- Around line 169-190: Update the global fallback in the
defaultModel/defaultProvider resolution block so
global.preferences.default_provider is only returned when
global.preferences.default_model is defined. Preserve the existing project and
workspace coupling behavior, and otherwise leave defaultProvider undefined when
no global model is configured.
---
Nitpick comments:
In `@apps/cli/src/chat/default-agent.ts`:
- Around line 50-52: Update inferProviderFromModel to normalize the model
identifier before the catalogModel lookup, matching the lowercase normalization
already used by the prefix fallback. Preserve the catalog result as
authoritative and only apply the fallback when the normalized lookup returns no
provider.
🪄 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: 66ec1ec1-2d0a-4dc8-9a6f-df7fdfc83aed
📒 Files selected for processing (28)
apps/cli/src/chat/agent-source.test.tsapps/cli/src/chat/agent-source.tsapps/cli/src/chat/default-agent.test.tsapps/cli/src/chat/default-agent.tsapps/cli/src/chat/effort-notice.test.tsapps/cli/src/chat/persister.test.tsapps/cli/src/chat/session-host.test.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/chat.test.tsapps/cli/src/config/resolve.test.tsapps/cli/src/config/resolve.tsapps/cli/src/config/write.test.tsapps/cli/src/config/write.tsapps/cli/src/engine/media-wiring.test.tsapps/cli/src/engine/model-catalog-view.test.tsapps/cli/src/harness/session-chain.e2e.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/onboarding/wizard.test.tsapps/cli/src/onboarding/wizard.tsapps/cli/src/render/tui/home-controller.test.tsapps/cli/src/render/tui/home-controller.tsdocs/decisions/0063-cli-config-write-contract.mddocs/decisions/0064-live-model-catalog.mddocs/reference/contracts/config-spec.mdpackages/llm/src/conformance/fixtures/anthropic.tspackages/llm/src/model-catalog.test.tspackages/llm/src/model-catalog.tspackages/shared/src/config.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/cli/src/engine/media-wiring.test.ts
- apps/cli/src/commands/chat.test.ts
- apps/cli/src/chat/persister.test.ts
- docs/decisions/0064-live-model-catalog.md
- apps/cli/src/chat/effort-notice.test.ts
…with negative tests Folds the Bug-2 adversarial review (2 confirmed test-coverage findings; both no-op guards today, but a deliberate decision with no regression net). - The `provider !== 'anthropic'` scoping gate had no negative test: install a synthetic OpenAI alias+dated-pin pair (via installCatalogRefresh) and assert the alias is NOT rescued, so a future broadening — or a new non-Anthropic dated family — cannot pass silently. - The catalog-row anti-fabrication gate was negatively tested in only the alias branch (the `99999999` case). Add the symmetric dated-pin case: a live base that is NOT a catalog row must not rescue its dated pin, exercising the line-146 conjunct in isolation. llm 26 model-catalog tests green. Refs: ADR-0064 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bug 1 of the live model-picker reports, generalized (not haiku- or deepseek-specific).
The picker offered five effort rows to models that expose fewer real choices: several
normalized tiers can collapse onto one provider wire value (DeepSeek's low/medium/high
all send `high`; Gemini's `max` coarsens onto `high`), and a continuous token BUDGET has
no discrete rungs at all (claude-haiku-4-5 — the report: a 5-tier ladder where the model
is really on/off, à la Claude Code).
The seam (`acceptedTiers`, the wire truth the gate / failover / adapters read) is
UNCHANGED. A new presentation helper `reasoningControlShape` → graded | budget | none and
`CANONICAL_ON_TIER` (= medium) drive a CLI projection (`effortTiersFor`, `effortRowLabel`):
- GRADED → deduped by distinct wire value (representative reads the name matching the
wire): deepseek-v4-pro → off/high/max; gemini-3.5-flash → low/medium/high; all-distinct
ladders (opus, gpt-5.4-pro) unchanged.
- BUDGET → a two-row off/on ("on" = medium, a real accepted tier so the accept sends a
value the gate accepts); a model that can't be turned off (gemini-2.5-pro) has nothing
to toggle → no overlay.
- NONE → nothing.
Every emitted row is still a valid accepted tier. `effortRowLabel` makes a budget model's
`medium` read "on" everywhere — the picker, the footer, and every effort notice — so the
surfaces cannot disagree.
Tests: reasoningControlShape (seam) + the full projection matrix (budget on/off, dedup,
gemini-2.5-pro empty, unchanged ladders). ADR-0066 §6 append-only presentation note.
Toolchain green (llm 649, cli 2203).
Refs: ADR-0066, ADR-0071
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…der-inference lookup Folds a review pass on the Bug-3 provenance work (2 fixed, 1 skipped). - config/resolve: the coupled (default_model, default_provider) resolution took the provider from the GLOBAL layer even when global set no MODEL, so a stray `[preferences].default_provider` (a hand-edited config — the picker/wizard always write the pair) would pair with the built-in default model → the wrong-adapter/404 the Bug-3 fold closed, reintroduced at the global-model-absent boundary. Now global's provider is taken only when global also sets the model; else both stay undefined (inference). Regression test added. - default-agent: `inferProviderFromModel` now lowercases ONCE before the catalog lookup (catalog ids are lowercase, and the prefix fallback already lowercased), so a mixed-case id resolves via the authoritative catalog, not only the prefix heuristic. Skipped (functionally inert, not a bug): the session-host / build-engine `resolveEndpoint` `?? 'openai'` fallback. It fires ONLY for an uncatalogued model, and `cappedMaxTokens` returns the request UNCLAMPED for any model with no catalog ceiling regardless of endpoint kind — so the endpoint never changes the estimate; a catalogued model already resolves its correct provider from the catalog. Refs: ADR-0059 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ls onto surviving rows The general effort-presentation rule (d509f6b) collapses graded/budget models to one row per distinct outcome, so `medium` is no longer guaranteed to be a picker row. Five sites still assumed the fixed five tiers: - HIGH — the opening highlight. `initialEffortPickerState` and the `/models` effort sub-step used `tiers.indexOf('medium')`, which returns -1 for a collapsed graded model (deepseek `[off,high,max]`) and clamps to 0 = `off`. A fresh picker opened on reasoning-DISABLED and an immediate Enter silently WROTE `off` on a model the user opened to turn reasoning up. New `projectEffortToRow(model, rows, tier)` folds a tier onto its surviving representative (graded → the row with the same wire value; budget → the canonical-on row), and every highlight/✓ now routes through it. - the ✓ marker (effort-tier-list), the standalone-picker accept note (chat-ink), and the typed `/effort` bare-list + set confirmation (chat.ts) all printed the raw tier; they now read through `effortRowLabel`, so a budget model's `medium` reads "on". Finding 5 (effortRejectedNote's raw tier) skipped: only OpenAI all-distinct-effort models reject a tier, and their rows are unrelabelled, so the note is already accurate. Regression tests: `projectEffortToRow` matrix; the picker opens on a reasoning-ON row (never `off`) for both a collapsed graded model and a budget model; and a bare-Home immediate Enter on deepseek-v4-pro writes `high`, never `off`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
apps/cli/src/engine/catalog-refresh.ts (1)
165-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
as ReadableStream<Uint8Array>asserts a concrete element type the DOM lib does not guarantee. The rationale (DOM typesbodyasReadableStream<any>) is fair, but the coding guidelines discourageas. Consider narrowing the reader'svalueat the point of use (it is what actually needs the type) so the byte arithmetic stays type-checked without asserting the whole stream's element type:♻️ Alternative without a concrete stream-generic assertion
- const reader = (body as ReadableStream<Uint8Array>).getReader(); + const reader = body.getReader(); for (;;) { const { done, value } = await reader.read(); if (done || value === undefined) break; - bytes += value.byteLength; + const chunk = value as Uint8Array; // narrowed only where read; still a cast, but scoped + bytes += chunk.byteLength; if (bytes > MAX_BYTES) { controller.abort(); return undefined; } - out += decoder.decode(value, { stream: true }); + out += decoder.decode(chunk, { stream: true }); }Non-blocking — if a scoped cast is no cleaner than the current one, leaving it is reasonable.
🤖 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 `@apps/cli/src/engine/catalog-refresh.ts` around lines 165 - 167, In the catalog refresh reader flow, avoid asserting the entire body as ReadableStream<Uint8Array>. Keep getReader() on the original body and narrow or validate the returned value at the byte-count arithmetic use site, preserving type-safe byte counting without a concrete stream-generic cast.Source: Coding guidelines
packages/llm/src/reasoning-wire.ts (1)
71-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a lookup map over the
asassertion.The upper-casing narrows via an unsafe assertion. A keyed
Recordyields the same result with no assertion and keeps the mapping exhaustive at the type level.As per coding guidelines: "do not use
anyor unsafe type assertions such asas, and prefer type guards."♻️ Assertion-free mapping
-export const toGeminiThinkingLevel = (wire: 'low' | 'medium' | 'high'): 'LOW' | 'MEDIUM' | 'HIGH' => - wire.toUpperCase() as 'LOW' | 'MEDIUM' | 'HIGH'; +const GEMINI_THINKING_LEVEL: Record<'low' | 'medium' | 'high', 'LOW' | 'MEDIUM' | 'HIGH'> = { + low: 'LOW', + medium: 'MEDIUM', + high: 'HIGH', +}; +export const toGeminiThinkingLevel = (wire: 'low' | 'medium' | 'high'): 'LOW' | 'MEDIUM' | 'HIGH' => + GEMINI_THINKING_LEVEL[wire];🤖 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/reasoning-wire.ts` around lines 71 - 72, Replace the unsafe uppercase assertion in toGeminiThinkingLevel with an exhaustive typed lookup Record mapping 'low' | 'medium' | 'high' to 'LOW' | 'MEDIUM' | 'HIGH', and return the mapped value directly while preserving the function’s existing input and output types.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 @.github/workflows/models-catalog.yml:
- Around line 40-46: Update the pinned actions in the workflow’s checkout, pnpm
setup, and Node setup steps—including the corresponding steps around the later
referenced block—from their v4 releases to the current Node 24-compatible major
releases, and replace each SHA with the matching verified commit for that major
while preserving the existing pinning and version comments.
In `@apps/cli/src/commands/models-pricing.ts`:
- Around line 229-244: Update the JSON branch of runClearPricing to determine
the source using the catalog’s actual pricing for args.model, matching
clearedPricingMessage: report source as "catalog" only when
catalogPricing(args.model) is defined, otherwise report null. Preserve the
existing cleared behavior and JSON response shape for models priced by the
catalog.
In `@apps/cli/src/home/drive-home.tsx`:
- Around line 556-567: Update noteToStore and the onceEffortNotice flow to
buffer notices emitted before storeRef.current is assigned instead of marking
them as permanently delivered through stderr. After assigning storeRef.current,
flush the queued notices through ChatStoreController.notice, preserving
deduplication while ensuring withheld effort notices reach the transcript.
- Line 507: Update the provider resolution around readEffectiveProvider so it
returns an explicit success/failure state instead of using undefined for both
cases. Preserve an intentional missing provider to allow model-ID inference, and
use config.chat.defaultProvider only when the fresh provider read fails; remove
the nullish-coalescing fallback that can restore stale configuration.
In `@docs/decisions/0066-normalized-reasoning-effort-control.md`:
- Line 40: Update the ADR blockquote in
0066-normalized-reasoning-effort-control.md so the blank line at the indicated
location remains part of the continuous blockquote by adding the blockquote
marker, or remove the blank line.
In `@docs/reference/cli/commands.md`:
- Around line 229-236: Update the canonical --json field reference for relavium
models refresh to document both record shapes: the existing per-provider fields
and the catalog record with source, status, models, and added. Keep the
reference consistent with the two-axis behavior described above.
In `@docs/reference/contracts/config-spec.md`:
- Line 154: Update the inline comment for [chat].default_provider in the
configuration example to state that it is resolved from the same layer as the
effective [chat].default_model, with provider inference from the model ID when
no provider is present at any layer. Remove the conflicting
fallback-to-global-[preferences].default_provider wording.
---
Nitpick comments:
In `@apps/cli/src/engine/catalog-refresh.ts`:
- Around line 165-167: In the catalog refresh reader flow, avoid asserting the
entire body as ReadableStream<Uint8Array>. Keep getReader() on the original body
and narrow or validate the returned value at the byte-count arithmetic use site,
preserving type-safe byte counting without a concrete stream-generic cast.
In `@packages/llm/src/reasoning-wire.ts`:
- Around line 71-72: Replace the unsafe uppercase assertion in
toGeminiThinkingLevel with an exhaustive typed lookup Record mapping 'low' |
'medium' | 'high' to 'LOW' | 'MEDIUM' | 'HIGH', and return the mapped value
directly while preserving the function’s existing input and output types.
🪄 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: f7716c69-155f-4890-b48e-6786c6c30f79
📒 Files selected for processing (133)
.claude/skills/add-llm-adapter/SKILL.md.github/workflows/models-catalog.ymlCLAUDE.mdapps/cli/src/chat/agent-source.test.tsapps/cli/src/chat/agent-source.tsapps/cli/src/chat/default-agent.test.tsapps/cli/src/chat/default-agent.tsapps/cli/src/chat/effort-notice.test.tsapps/cli/src/chat/effort-notice.tsapps/cli/src/chat/persister.test.tsapps/cli/src/chat/session-host.test.tsapps/cli/src/chat/session-host.tsapps/cli/src/commands/agent-run.tsapps/cli/src/commands/chat.test.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/dispatch.tsapps/cli/src/commands/gate.tsapps/cli/src/commands/manifest.tsapps/cli/src/commands/models-dispatch.test.tsapps/cli/src/commands/models-pricing.test.tsapps/cli/src/commands/models-pricing.tsapps/cli/src/commands/models.test.tsapps/cli/src/commands/models.tsapps/cli/src/commands/run.tsapps/cli/src/commands/specs-forwarding.test.tsapps/cli/src/commands/specs.tsapps/cli/src/config/load.tsapps/cli/src/config/resolve.test.tsapps/cli/src/config/resolve.tsapps/cli/src/config/write.test.tsapps/cli/src/config/write.tsapps/cli/src/engine/build-engine.tsapps/cli/src/engine/catalog-refresh.test.tsapps/cli/src/engine/catalog-refresh.tsapps/cli/src/engine/media-wiring.test.tsapps/cli/src/engine/model-catalog-view.test.tsapps/cli/src/engine/model-catalog-view.tsapps/cli/src/engine/pricing-overlay.tsapps/cli/src/engine/providers.tsapps/cli/src/harness/regression.e2e.test.tsapps/cli/src/harness/session-chain.e2e.test.tsapps/cli/src/home/drive-home.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/onboarding/wizard.test.tsapps/cli/src/onboarding/wizard.tsapps/cli/src/render/sanitize.tsapps/cli/src/render/tui/chat-ink.tsxapps/cli/src/render/tui/chat-projection.test.tsapps/cli/src/render/tui/chat-projection.tsapps/cli/src/render/tui/effort-picker.test.tsapps/cli/src/render/tui/effort-picker.tsapps/cli/src/render/tui/effort-tier-list.tsxapps/cli/src/render/tui/home-app.test.tsxapps/cli/src/render/tui/home-app.tsxapps/cli/src/render/tui/home-controller.test.tsapps/cli/src/render/tui/home-controller.tsapps/cli/src/render/tui/model-picker-view.tsxapps/cli/src/render/tui/model-picker.test.tsapps/cli/src/render/tui/model-picker.tsapps/cli/src/run.tsdocs/decisions/0063-cli-config-write-contract.mddocs/decisions/0064-live-model-catalog.mddocs/decisions/0065-provider-economics-and-extensibility.mddocs/decisions/0066-normalized-reasoning-effort-control.mddocs/decisions/0071-models-dev-as-the-model-metadata-source.mddocs/decisions/README.mddocs/reference/cli/commands.mddocs/reference/contracts/config-spec.mddocs/reference/contracts/workflow-yaml-spec.mddocs/reference/desktop/database-schema.mddocs/reference/shared-core/llm-provider-seam.mddocs/roadmap/current.mddocs/roadmap/deferred-tasks.mddocs/roadmap/phases/phase-2.6-conversational-authoring.mddocs/runbooks/add-a-provider.mddocs/standards/security-review.mdeslint.config.mjspackage.jsonpackages/core/src/engine/agent-runner.e2e.test.tspackages/core/src/engine/agent-runner.test.tspackages/core/src/engine/agent-runner.tspackages/core/src/engine/agent-session.test.tspackages/core/src/engine/agent-session.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/drizzle/meta/0011_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/model-catalog-store.tspackages/db/src/schema.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/adapters/shared.tspackages/llm/src/budget-estimator.test.tspackages/llm/src/budget-estimator.tspackages/llm/src/catalog/catalog-model.tspackages/llm/src/catalog/catalog-providers.tspackages/llm/src/catalog/deprecations.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/catalog/pricing.tspackages/llm/src/catalog/snapshot-guard.test.tspackages/llm/src/catalog/snapshot-guard.tspackages/llm/src/catalog/snapshot.tspackages/llm/src/conformance/effort.conformance.test.tspackages/llm/src/conformance/fixtures/anthropic.tspackages/llm/src/cost-tracker.test.tspackages/llm/src/cost-tracker.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/model-kind.tspackages/llm/src/output-cap.test.tspackages/llm/src/output-cap.tspackages/llm/src/pricing.test.tspackages/llm/src/pricing.tspackages/llm/src/reasoning-wire.test.tspackages/llm/src/reasoning-wire.tspackages/shared/src/config.tspackages/shared/src/workflow.tstools/sync-models-dev/sync.mjs
Comment on lines
+40
to
+46
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 | ||
| with: | ||
| persist-credentials: false | ||
| - name: Set up pnpm | ||
| uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 | ||
| - name: Set up Node | ||
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve current major-version tags to their commit SHAs and compare against the pinned ones.
gh api repos/actions/checkout/commits/v4 --jq '.sha'
gh api repos/actions/setup-node/commits/v4 --jq '.sha'
gh api repos/pnpm/action-setup/commits/v4 --jq '.sha'
gh api repos/actions/checkout/tags --paginate --jq '.[] | select(.name | startswith("v5") or startswith("v6")) | {name, sha: .commit.sha}'Repository: HodeTech/Relavium
Length of output: 874
🌐 Web query:
GitHub Actions Node 20 deprecation actions/checkout v4 setup-node v4 pnpm action-setup v4 Node 24 runners June 2 2026 September 16 2026
💡 Result:
GitHub Actions has deprecated Node.js 20 in favor of Node.js 24 as part of the runner runtime migration [1][2]. The transition involves two key dates in 2026: 1. June 16, 2026: Node.js 24 became the default runtime for GitHub Actions [3][4]. Actions that rely on Node.js 20 will trigger deprecation warnings if they have not been updated to support Node.js 24 [5]. 2. September 16, 2026: Node.js 20 is scheduled to be removed from GitHub Actions runners [5]. To address deprecation warnings and ensure compatibility, maintainers must update their workflows to use versions of actions that support Node.js 24 (often v5 or higher for core actions like actions/checkout, actions/setup-node, and pnpm/action-setup) [6]. Migration details: - Environment Variables: During the transition, users could use FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true to force Node.js 24 usage or ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true to temporarily continue using Node.js 20 after it was deprecated [2][5]. - Action Compatibility: Standard actions such as actions/checkout@v4 and pnpm/action-setup@v4 may trigger warnings if they are not updated to a version explicitly configured for the Node.js 24 runtime environment [7][6][5]. Users are encouraged to bump these to the latest available major versions [6].
Citations:
- 1: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- 2: Node 20 -> Node 24 migration feature flagging, opt-in and opt-out environment variables actions/runner#3948
- 3: https://github.com/actions/runner/releases
- 4: Update Node 24 default date to June 16th, 2026 actions/runner#4462
- 5: https://github.com/johanviberg/proxycheck-sdk/actions/runs/24346248537
- 6: Bump GitHub Actions versions before Node 20 deprecation (June 2026 → September 2026) baabakk/llm-ports#26
- 7: Handle Node.js 20 actions deprecation on github pnpm/action-setup#209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow locations mentioned in the comment.
sed -n '1,120p' .github/workflows/models-catalog.yml
# Check whether the pinned SHAs exist and which major tags currently point to them.
gh api repos/actions/checkout/commits/34e114876b0b11c390a56381ad16ebd13914f8d5 --jq '.sha'
gh api repos/actions/setup-node/commits/49933ea5288caeca8642d1e84afbd3f7d6820020 --jq '.sha'
gh api repos/pnpm/action-setup/commits/b906affcce14559ad1aafd4ab0e942779e9f58b1 --jq '.sha'
# Pull the action metadata from tags where available.
for repo in actions/checkout actions/setup-node pnpm/action-setup; do
echo "== $repo =="
gh api "repos/$repo/tags" --paginate --jq '.[] | select(.name=="v4" or .name=="v5" or .name=="v6") | {name, sha: .commit.sha}'
doneRepository: HodeTech/Relavium
Length of output: 5753
🌐 Web query:
actions/checkout v5 node24 runtime release notes actions/setup-node v5 node24 runtime release notes pnpm/action-setup v5 node24 runtime release notes GitHub Actions Node 20 deprecation warning current runners 2026
💡 Result:
As of July 13, 2026, the migration from Node.js 20 to Node.js 24 for GitHub Actions is in effect. Node.js 20 reached end-of-life in April 2026 [1][2][3], and GitHub has deprecated Node.js 20 support on GitHub Actions runners, with an enforcement date of June 2, 2026 [4][5]. Key Details: - Official Actions Support: The major versions of standard actions, including actions/checkout (v5.0.0+) [6][7] and actions/setup-node (v5.0.0+) [8][9], have been updated to use the Node.js 24 runtime. - Runner Requirement: To use these v5 versions, your GitHub Actions runner must be updated to at least version v2.327.1 [10][8][11]. - Deprecation Status: Node.js 20-based actions will trigger a deprecation warning annotation at the end of job runs [5]. Users are expected to migrate their workflows to use actions that support the Node.js 24 runtime [1][5]. - pnpm/action-setup: While specific v5 release notes for pnpm/action-setup were not explicitly surfaced in the same format as official actions/ repositories, the broader migration requires that all actions running on GitHub-hosted runners be compatible with the Node.js 24 environment [5]. Users should check the repository's latest release for v5+ updates to ensure Node.js 24 compatibility.
Citations:
- 1: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
- 2: https://github.com/orgs/community/discussions/189324
- 3: https://github.com/nodejs/release/blob/main/README.md
- 4: Update Node.js 20 deprecation date to June 2nd, 2026 actions/runner#4258
- 5: Add Node.js 20 deprecation warning annotation (Phase 1) actions/runner#4242
- 6: https://github.com/actions/checkout/releases/tag/v5.0.0
- 7: https://github.com/actions/checkout/blob/main/CHANGELOG.md
- 8: https://github.com/actions/setup-node/releases/tag/v5.0.0
- 9: actions/setup-node@v4.0.2...v5.0.0
- 10: https://github.com/actions/checkout
- 11: https://github.com/actions/setup-node
Upgrade these pinned GitHub Actions off the Node 20 line .github/workflows/models-catalog.yml:40-46,70-76 — actions/checkout, actions/setup-node, and pnpm/action-setup are still on v4 releases; move them to the current Node 24-compatible majors to avoid deprecation warnings and future runner failures.
🤖 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 @.github/workflows/models-catalog.yml around lines 40 - 46, Update the pinned
actions in the workflow’s checkout, pnpm setup, and Node setup steps—including
the corresponding steps around the later referenced block—from their v4 releases
to the current Node 24-compatible major releases, and replace each SHA with the
matching verified commit for that major while preserving the existing pinning
and version comments.
| > canonical-wins-over-`providerOptions` precedence, the `/effort` overlay, and the host-gated design. What | ||
| > changes is only that a model now *declares* which tiers it accepts and which native shape carries them, | ||
| > instead of the adapter assuming both. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the ADR blockquote continuous.
The blank line at Line 40 is inside the > block and triggers MD028. Add > to that blank line or remove it.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 40-40: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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/0066-normalized-reasoning-effort-control.md` at line 40,
Update the ADR blockquote in 0066-normalized-reasoning-effort-control.md so the
blank line at the indicated location remains part of the continuous blockquote
by adding the blockquote marker, or remove the blank line.
Source: Linters/SAST tools
…so they actually run (review H1) Both ADR-0071 §9 scheduled guards were red on every run for an infrastructure reason, not a real signal: - The weekly shipped-price-change guard runs `pnpm sync:models`, whose build step was `pnpm --filter @relavium/llm build` — a bare per-package `tsc` that does NOT build @relavium/llm's `@relavium/shared` dependency. `@relavium/*` exports resolve only to a gitignored `dist/`, so a fresh runner dies on TS2307 before the money guard is ever evaluated. sync.mjs then imports `packages/llm/dist/**`, which also needs the build. Route the script's build through turbo so `^build` pulls in @relavium/shared first (also fixes a fresh local `pnpm sync:models`). - The nightly live effort-conformance job ran vitest with no build at all, so it could not resolve @relavium/shared and every conformance file errored out before a single live probe. Add a `pnpm turbo run build --filter=@relavium/llm` step. A guard that is red no matter what trains the maintainer to ignore it — exactly the erosion the workflow's own header warns against. Now both guards reach the thing they protect: the shipped-price money guard and the live tier-acceptance probe. Refs: ADR-0071 §9 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r, not the model catalog (review M2) The pre-egress cost estimate clamps an authored `max_tokens` to the model's catalog ceiling on an OFFICIAL endpoint and — correctly — not on a CUSTOM one. But it decided official-vs-custom from `catalogModel(model).provider`, the model's fixed catalog provider, rather than the provider actually routing the call. A custom `base_url` gateway (OpenRouter/LiteLLM) registered as `openai` and serving a `deepseek-*`/`anthropic-*` model id is `custom` at the wire yet `official` by catalog: the estimate clamped to the catalog ceiling while the adapter sent the full request, so the governor under-authorized and waved through spend it exists to stop — the exact under-authorization `resolveEndpoint` (ce6cfbf) was added to prevent. Thread the routing provider through the pre-egress seam so the estimate keys on the same provider the adapter dials, per attempt: - `FallbackChain.PreAttemptHook` info gains `provider` (the entry's provider.id); a failover moves it, so each attempt reports its own routing provider. - `PreEgressHook` info gains an optional `provider`; agent-turn threads the active provider (updated with `activeModel` in `onAttempt`) on both the primary-model pre-check and the per-attempt chain hook. A media-only gate (`maxTokens: 0`) omits it harmlessly — the token estimate is 0 regardless. - `BudgetGovernor.evaluatePreEgress`/`checkPreEgress` accept the provider and key the endpoint on it; `resolveEndpoint` is now `(provider) => EndpointKind`. - The two CLI closures collapse to `resolveEndpoint: endpointKind` — the provider IS the key, so the catalog-provider derivation (and its `?? 'openai'` guess) is gone. Regression tests: the governor routes the endpoint by the provider argument (same model + maxTokens, opposite clamp, cap set between the two costs); a media-only gate defaults to official harmlessly; and the chain's preAttempt carries the primary then the failover provider across a failover. Refs: ADR-0071 §7 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…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>
|
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 refreshsplit into separate provider-availability vs catalog-metadata axes (--providers/--catalog).models pricing --clearto remove user pricing overrides.strict_cost_capbehavior.