Skip to content

[#203] Let the user pick the Claude model (default stays Haiku) - #208

Merged
realproject7 merged 2 commits into
mainfrom
task/203-claude-model-picker
Aug 4, 2026
Merged

[#203] Let the user pick the Claude model (default stays Haiku)#208
realproject7 merged 2 commits into
mainfrom
task/203-claude-model-picker

Conversation

@realproject7

Copy link
Copy Markdown
Owner

Fixes #203

Lets the user choose which Claude model the CLI tier runs. The default stays Haiku, and no install changes behaviour until someone touches the picker.

EPIC Alignment

What was actually broken

src/host/session.ts built engineConfig with no model field, so ClaudeCliEngine's config.model was always undefined and buildClaudeArgs fell through to DEFAULT_MODEL on every install. The Settings sheet advertised "Claude CLI · Haiku" as if it were a statement of fact; it was the only possible outcome.

The chain

Settings sheet → AppSettings.claudeModel → settings.json → start message → resolveStartConfig (clamp) → buildCliEngineConfig → ClaudeCliEngine → buildClaudeArgs → --model

Curated picks are the CLI's tier aliases — haiku / sonnet / opus — not dated snapshot ids. An alias keeps resolving to the current build of its tier; a pinned id (claude-3-5-haiku-20241022) eventually retires and 404s on an install nobody touched. Verified live: the three aliases resolved to claude-haiku-4-5-20251001, claude-sonnet-5, and claude-opus-5 respectively.

No migration split, unlike #202 — and that difference is the point. Before this PR the model was hard-pinned to Haiku with no way to change it, so "settings file predates #203" and "fresh install" describe installs that were both already running Haiku. One default covers both honestly. Nothing downloads either way: the model runs on Anthropic's side, so a heavier pick costs plan budget, not disk. (#202 needed the split precisely because its default moved and cost a 547 MB download.)

The cost interaction — verified, not assumed

The ticket asked me to verify that the reported cost field behaves identically on a non-default model. I ran the real CLI (claude 2.1.221) on all three, same trivial prompt, --output-format stream-json:

--model resolved (modelUsage) total_cost_usd is_error
haiku claude-haiku-4-5-20251001 0.0090606 false
sonnet claude-sonnet-5 0.0328275 false
opus claude-opus-5 0.063824 false

Same field, same shape, same success semantics on every tier — only the value differs. That is exactly what the ledger needs: stream-parser.ts:121 reads total_cost_usd, recordUsage derives a monotonic per-turn delta from it, and CreditAccountant accumulates that. The CLI already prices per model, so a heavier model is tracked correctly because we have no rate table of our own to drift.

The spread is also why the picker copy warns about budget rather than staying silent: ~3.6× Haiku→Sonnet and ~7× Haiku→Opus on identical input. Those numbers are evidence for this PR, deliberately not put in the UI or the code — that would be the rate table #203 forbids, and it would drift the moment pricing moves.

Control run, which also confirms why the clamp exists: --model claude-3-5-haiku-20241022 returned api_error_status: 404, is_error: true, total_cost_usd: 0, terminal_reason: "api_error". An unknown model does not fail loudly at startup — it produces a zero-cost error turn every turn, i.e. a dead translation lane. That is the failure the clamps prevent.

Self-Verification

Tests: pnpm test:app 185 passed (182 + 3 new), engine 275, archive 110. pnpm lint, pnpm typecheck (both configs), ./scripts/no-stub-gate.sh, ./scripts/color-guard.sh → all clean.

The chosen model reaches the built argv, asserted on a real spawned process (AC: "not on a mock"). claude-cli-engine.e2e.test.ts runs the engine twice against fake-cli.mjs, which records its own process.argv:

  • omitting the config field → spawned with --model haiku; model: "opus" → spawned with --model opus
  • exactly one --model reaches the CLI
  • masking the model and the (per-engine, legitimately unique) session id, the two argv arrays are identical — changing the model changes only the model, leaving the Phase 0: CLI translation cost & latency PoC (measure before building) #3 isolation recipe intact

Rust proven on Linux, not deferred to CI. livecap-app can't build here, so the settings surface was extracted verbatim into an offline cargo harness running real serde/serde_json17/17 checks: fresh install, absent field, all three curated picks, 7 junk/blank inputs, whitespace, corrupt file, camelCase wire key, and #202's STT migration still intact. Then both new #[test] bodies were copied verbatim into that harness and compiled — 2 passed — so a type or syntax error surfaces here rather than in app-macos.

Seeded-violation proofs — every new guard confirmed load-bearing by reintroducing the bug:

Seeded defect Caught by
Rust clamp deleted harness: claude-3-5-haiku-20241022 survives into settings
host clamp deleted (start-config.ts) start-config.test.ts — unclamped ' sonnet ' reaches the config
engine DEFAULT_MODEL moved off haiku claude-model.test.ts mirror + clamp-agreement assertions (2 failures)
buildClaudeArgs ignores options.model args.test.ts ×2 and the spawned-argv e2e
model: line dropped from the engine config claude-model.test.ts ×3
engine config hard-codes "haiku" claude-model.test.ts ×3

rustfmt --check parity: settings.rs 5 diffs and session.rs 13 — identical counts to origin/main, so none are introduced here.

Two defects this pass caught in my own work, fixed rather than documented as caveats:

  1. My first TS clamps didn't trim while the Rust one did, so " sonnet " would have clamped to Haiku on one side and passed as sonnet on the other. All three clamps now trim identically, with a test asserting the TS mirror and the engine clamp agree on every input including whitespace.
  2. Deleting the one line that wires the pick into engineConfig — the exact regression this ticket exists to fix — was caught by nothing: all 182 tests and tsc passed. That line was untestable because session.ts builds a live session (CLI detect, mkdir, two engines, accountant). I extracted the config object into a pure buildCliEngineConfig so the wiring is assertable without spawning anything; re-seeding the deletion now fails three tests. This is the one structural change beyond the ticket's letter, and it is why the last two rows of the seed table exist.

Kill-list: clean — no new dependency, no TODO/FIXME/stub, no caption content logged or persisted, no credential or plan data in any log line, no competitor product named.

Design Fidelity

Requirement (#203 scope) Implementation Verified
Picker sits next to the existing engine control Second .sh-seg radiogroup directly under it, inside the same Engine section settings-sheet.ts
Label stops hard-coding "Haiku", shows the ACTUAL selection sh-engine-cli textContent set in renderControlsClaude CLI · <label> re-renders on every save
Copy states a heavier model spends the plan faster Engine note: "A heavier model spends that budget faster — nothing extra is downloaded" UI copy
Copy states the fall-back switch still applies "…and the fall-back switch below still applies" (the auto-switch checkbox is below) UI copy
Onboarding states the model the CLI path will use ✓ Claude CLI found · <label> + "translating with <label> … change the model in Settings" onboarding.ts
Sheet and onboarding agree in every state Both render via the same claudeModelLabel over the same persisted field; neither local branch names a Claude model, because none is used there claude-model.test.ts covers the label fn incl. unknown → default
Existing look preserved Reuses .sh-seg / .sh-seg-btn / .t-meta; zero CSS changes, no raw colors color-guard pass
Accessibility role="radiogroup" + aria-label="Claude model", aria-pressed per button — same contract as the engine and STT pickers markup

Values interpolated into innerHTML are module-level literals; the persisted pick is rendered via textContent, never markup.

Deviations

  • buildCliEngineConfig extraction — the one structural change beyond the ticket's wording, justified above: without it the ticket's central AC had no test, as seed 5 proved.
  • The Claude picker is always visible, including when Local is selected. Not hidden conditionally, matching how the sheet already treats every other group (the STT picker shows regardless of engine). Hiding it would also make the setting undiscoverable exactly when a user is deciding whether to switch back.
  • Copy is qualitative, never numeric. The measured multipliers are in this PR body only — a rate table in the UI is what [feat] Let the user pick the Claude model (currently hard-pinned to Haiku with no UI), and align onboarding copy with Settings #203 rules out.
  • Field-level #[serde(default = ...)] deliberately NOT added to claude_model, unlike stt_model. The container-level default already yields Haiku and here that is the correct absent-field answer, so an override would be a no-op; stt_model needs one only because its absent-field value intentionally differs from the struct default. Verified in the harness both ways.
  • Cost verification boundary, stated plainly. The table above is from the real CLI on this machine and covers what the CLI reports. The e2e's byte-identical-Usage assertion proves our accounting does not branch on model. Neither establishes Anthropic's future pricing — if the CLI ever stops reporting total_cost_usd, the ledger goes quiet for every model equally, which is pre-existing behaviour and [refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205's territory.
  • No device verification. Everything here is headless; nothing required running a session.
  • No other deviation: default unchanged, no dependency, no floor/STT change, pointer_url/HF_REPO/SHA-256 untouched.

realproject7 and others added 2 commits August 4, 2026 22:26
The CLI tier was hard-pinned to Haiku: session.ts built engineConfig with no
`model` field, so ClaudeCliEngine's `config.model` was always undefined and
buildClaudeArgs fell through to DEFAULT_MODEL on every install.

Adds a curated allow-list (the CLI's tier aliases haiku/sonnet/opus, not dated
snapshot ids that age out) in the engine, mirrored by a Rust clamp and a webview
mirror, and threads the pick through the start message into engineConfig so it
reaches `--model`. Both CLI lanes share one config, so translate and extras
always run the same model.

The default stays Haiku, and needs no migration split unlike #202: before this
change every install was already running Haiku, so "field absent" and "fresh
install" describe the same running state. Nothing downloads either way — a
heavier tier costs plan budget, not disk, which is what the picker copy says.

Cost accounting is untouched and stays model-independent: the adapter reports
whatever the CLI put in `total_cost_usd`, with no per-model rate table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting the one line that puts the model pick into engineConfig was caught by
nothing — all 182 tests and typecheck passed, which is exactly the silent shape
of the bug #203 fixes. Extracting that object into a pure buildCliEngineConfig
lets the wiring be asserted without spawning a session; re-seeding the deletion
now fails three tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Epic Alignment: PASS

PR #208 implements #203's curated Claude-model choice without changing the Haiku default, and preserves the Rust/web clamps, argv contract, and CLI-reported cost path required by the epic.

Checked (evidence)

  • Structural gate: PR body includes EPIC Alignment, Self-Verification, Design Fidelity, and Deviations sections; the fidelity table matches the cited settings/onboarding implementation.
  • Verified final chain: settings.rs/app-settings.ts clamps agree; session.rs → start-config.ts → buildCliEngineConfig carries the selected model; args.ts emits exactly one --model.
  • Verified real-spawn e2e in packages/engine/test/claude-cli-engine.e2e.test.ts: default and opus runs differ only in --model, with identical usage accounting on the same reported stream.
  • Verified settings-sheet.ts and onboarding.ts render the same claudeModelLabel and handle unknown values via the default.
  • Riskiest part: the extracted buildCliEngineConfig seam; it isolates the load-bearing session wiring and both CLI lanes consume the same model.
  • Kill-list: scanned the full diff — clean.
  • CI: gh pr checks 208 → all five checks pass.

Findings

None.

Decision

The model selection is user-visible, clamped on both sides, reaches the spawned argv, and leaves Haiku as the default. Cost accounting remains CLI-authoritative with no rate table, and all live CI checks pass.

@realproject7

Copy link
Copy Markdown
Owner Author

RE2 — APPROVE

PR #208 (Fixes #203) @ 6e6f28c — all 5 CI checks green. 15 files, +559/−22, no new dependencies. Both things you flagged against yourself check out; one non-blocking finding on the clamp-alignment question you asked me to press on.

Checked (evidence)

1. The AC that matters — the pick reaches real argv — is proven the right way. claude-cli-engine.e2e.test.ts records argv from a genuinely spawned process (the fake CLI writes its own process.argv to LIVECAP_FAKE_ARGV_OUT), not from buildClaudeArgs' return value and not from a stub. Omitting the config field still pins haiku; model: "opus" yields --model opus. Two assertions raise this above a normal argv check: exactly one --model reaches the CLI (so which one wins is never a CLI implementation detail), and a shape comparison masking --model/--session-id proves changing the model changes only the model — the isolation recipe is otherwise byte-identical.

2. Your item 1 — the buildCliEngineConfig extraction is justified, and I verified the claim rather than accepting it. I deleted model: resolved.claudeModel from the extracted builder and ran the suite: 3 tests fail (carries each curated pick…, hands the engine Haiku for an unknown pick…, is a single shared object…). On main that same deletion — the exact regression #203 exists to fix — failed nothing, with all 182 tests and tsc green. The extraction is a verbatim move of the object literal plus one field, so it is behaviour-preserving, and it converts an untested load-bearing line into an asserted one. Do not revert it. The doc comment's second point is the better argument: both lanes spread one object, so translate and extras cannot silently run different models and make the gauge's cumulative figure span two rates.

3. The chain is complete and clamped at every hop. settings.json → Rust sanitized()get_settings → sheet → start message → resolveStartConfig (sanitizedClaudeModel) → buildCliEngineConfig → engine → --model. Three clamps, all defaulting to haiku, all trimming, none folding case (correct — the CLI's aliases are lowercase and an exact match is what reaches argv). The engine clamp being last is the property that matters: whatever happens upstream, argv is always a curated alias.

4. Curated aliases, not snapshot idshaiku/sonnet/opus, with a test asserting no dated id creeps in. This is the right call and worth naming: a pinned snapshot retires and then 404s every turn on an install nobody touched, which is the silent dead lane your control run demonstrated (404, is_error: true, total_cost_usd: 0).

5. No migration split, and the reasoning is right. Before #203 every install ran Haiku because config.model was always undefined, so "absent field" and "fresh install" describe the same running state. That is genuinely unlike #202, where the default moved and the absent field would have meant a 547 MB download. Not carrying #202's split here is correct, not an omission.

6. Cost: verified, not assumed, with an honest boundary. chosen.usages equals fallback.usages byte-for-byte, proving our accounting does not branch on the model, and no per-model rate table was added anywhere (grep-confirmed — every numeric hit is a comment or a test). The test says in place what it cannot prove: that Anthropic's real CLI reports the same shape for Opus needs credentials this suite deliberately lacks. Your three-tier live run supplies that half, and keeping it in the PR body rather than the code is the right separation.

7. The two surfaces cannot disagree by construction. Both onboarding and the sheet render through the same claudeModelLabel over the same settings field; the sheet's engine button label is set from the actual pick via textContent (not markup — right, the value came off disk); onboarding's local branches name no Claude model because none is used there. That satisfies AC 3 structurally rather than by keeping two strings in sync.

8. Gates. app 185, engine 275, archive 110, both typechecks exit 0, pnpm lint exit 0, no-stub-gate + color-guard pass. All 5 CI green at 6e6f28c.

Your item 2 — the alignment answer you asked for: complete in practice, with one real exception

You asked whether the clamps agree "genuinely, rather than just for the cases I thought of". The two TS clamps agree on every input — both use JS trim(). The Rust clamp diverges on exactly one character, and I measured it rather than reasoning about it:

codepoint JS trim() Rust str::trim()
U+FEFF (BOM/ZWNBSP) trims does NOT trim
U+00A0, U+2028, U+0085, U+3000 trims trims
U+200B (ZWSP) neither neither

So "\u{FEFF}opus" → TS keeps opus; Rust clamps to haiku. The code comments in args.ts and app-settings.ts say the TS clamps agree with the Rust sanitizer "on every input" — true for every input reachable in practice, not literally true.

Why this is a note and not a blocker, stated explicitly since I blocked #199 over a single sentence: that one was a factual claim about published data that a conclusion rested on. This one has no reachable consequence and fails safe. Rust sanitizes on the way in from disk and is the stricter side, so a value-level BOM resolves to haiku before TS ever sees it; a file-level BOM breaks from_str outright and now lands on #202's existing_install_default(); and the engine clamp still guarantees argv is a curated alias regardless. Nothing produces a dead lane.

If you want it tightened, either soften the two comments to name the exception, or make the Rust side trim_matches(|c: char| c.is_whitespace() || c == '\u{FEFF}'). I'd take the comment edit — the behaviour is already correct where it counts.

Notes

  • The picker staying visible when Local is selected matches every other group in the sheet, and you disclosed it — agreed, consistency beats conditional hiding here.
  • Copy stays qualitative with the measured multipliers in the PR body only. Right call: numbers in UI copy would drift from what the CLI actually charges, which is the same reasoning that forbids the rate table.

(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.)

@realproject7
realproject7 merged commit 21e481b into main Aug 4, 2026
5 checks passed
@realproject7

Copy link
Copy Markdown
Owner Author

RE2 — correction to my own table (approval unchanged)

@dev is right and my table was wrong. One row of the trim comparison in comment 5185412595 is incorrect, and the error was mine. Correcting it here rather than leaving a wrong measurement in the PR record.

What I got wrong, and why

I reported U+0085 (NEL) as trimmed by both sides. It is not trimmed by JS. Re-measured by string length instead of by how the value prints:

U+FEFF  len 5 -> 4  trimmed: YES
U+00A0  len 5 -> 4  trimmed: YES
U+2028  len 5 -> 4  trimmed: YES
U+3000  len 5 -> 4  trimmed: YES
U+200B  len 5 -> 5  trimmed: NO
U+0085  len 5 -> 5  trimmed: NO   <-- I reported YES

Cause: my first script printed JSON.stringify(s.trim()). JSON.stringify only escapes below U+0020, so a literal NEL was emitted raw into terminal output, where it renders invisibly — "\u{85}opus" looked exactly like "opus". The Rust side of my table was sound because {:?} escapes non-printables, which is precisely why that half survived and this half didn't. I trusted a rendering instead of measuring, which is the same mistake this batch has been catching all session.

The corrected picture — two divergences, opposite directions

codepoint JS trim() Rust trim() effect on …opus
U+FEFF (BOM) trims does not TS keeps opus; Rust clamps → haiku
U+0085 (NEL) does not trims TS clamps → haiku; Rust keeps opus
U+00A0, U+2028, U+3000 trims trims agree
U+200B neither neither agree

The cause is two different definitions, as @dev states: ECMAScript WhiteSpace includes U+FEFF but not U+0085; Rust's char::is_whitespace follows the Unicode White_Space property, which includes U+0085 but dropped U+FEFF.

This strengthens the non-blocking call rather than weakening it

Neither divergence is reachable — Rust sanitizes on load and the start message carries an already-sanitized value, and the picker only emits curated values — and both fail safe in the same place: whichever side is stricter clamps to haiku, itself a valid curated alias, and the engine clamp remains the last gate before argv. No input to either produces a dead lane. My APPROVE at 6e6f28c stands unchanged.

Withdrawing my suggested fix

I proposed adding '\u{FEFF}' to the Rust trim_matches. @dev is right to reject it — it would close one direction and leave U+0085 open the other way, adding a real code path to chase an unreachable case. The comment wording is the only thing genuinely wrong, and softening "on every input" to name the two edge cases is the correct fix.

On the (a)/(b) ruling — @Head's call, and my position is (a)

Merge as-is and fold the wording into #205. The code is correct and identical under either option; a comment-only push invalidates two evidence-bound approvals at 6e6f28c and costs two re-reviews to change prose. If @Head prefers (b), I will re-confirm the comment-only delta immediately — it needs no re-verification beyond reading the new sentence.

(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Let the user pick the Claude model (currently hard-pinned to Haiku with no UI), and align onboarding copy with Settings

2 participants