Repository navigation
fix(web): preserve provider defaults for optionless threads - #23528
juliusmarminge wants to merge 7 commits into
Conversation
An existing thread persisted without options shows and sends the New threads default effort, because deriveEffectiveComposerModelState falls back to the project/settings default selection's options. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lt options Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:trusted
juliusmarminge
added
bugfix
GitCafe Production
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — Existing optionless threads will use provider defaults instead of inheriting project/settings options, changing their effective model configuration. Human review is required for this default-policy change and the broadened Knip fixture entry-point configuration. No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Thread option precedence apps/web/src/composerDraftStore.ts, apps/web/src/modelSelection.test.ts, scripts/fixtures/composer-option-fallback.mjs, knip.jsonc |
The resolver uses project options only when there is no thread model selection. The test checks that an optionless thread selection does not inherit project options. The fixture seeds threads with different option configurations, and Knip includes script fixtures as entry files. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related issues
- pingdotgg/t3code#23497 — Reports that an optionless thread selection incorrectly inherits project or settings options.
Possibly related PRs
- pingdotgg/t3code#24619 — Also changes composer model-state resolution and how it handles missing thread options.
- pingdotgg/t3code#23957 — Changes the source of composer model options in
deriveEffectiveComposerModelState. - pingdotgg/t3code#23184 — Also preserves explicitly empty model-option selections in web composer state.
Suggested reviewers: t3dotgg, macroscopeapp
Merge Risk: 
430af
-
No actionable merge-blocking risk was found in the reviewed changes.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving provider defaults for threads without options. Description check The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the bug, fix, linked issue, test results, limitations, and supporting evidence. Linked Issues check [ #23497] requires that a thread saved without options keep provider defaults, not project/settings options. At the reviewed head,deriveEffectiveComposerModelStatein `apps/web/src/composerDraftStor…Out of Scope Changes check All changed code connects to [ #23497]. The product change is limited to the option-fallback expression incomposerDraftStore.ts. The regression test inmodelSelection.test.tsverifies that express…Full details: Linked Issues check
-
Explanation
[
#23497] requires that a thread saved without options keep provider defaults, not project/settings options. At the reviewed head,deriveEffectiveComposerModelStateinapps/web/src/composerDraftStore.tschanges themodelOptionsfallback. Draft options still win. Next, the thread selection supplies options when it exists, even when it has none. Project options apply only whenthreadModelSelectionis absent. This matches the issue's stated cause, where options fell back to project/settings defaults while the model resolved from the thread.apps/web/src/modelSelection.test.tsadds a regression case. It uses an optionless thread selection and a project selection witheffort: high, and assertsstate.modelOptions?.[instanceId]is undefined. Two caveats. The?? nullterminator was removed, somodelOptionscan now beundefinedrather thannullwhen the thread has no options. The test accepts this, and the author reports web typecheck passed. Second, the send-path dispatch is not traced in this review. The author's browser evidence reports that effort is omitted from dispatch after the change. This review relies on that evidence and on the shared resolver. Coding requirements are met at the state-derivation level.
Full details: Out of Scope Changes check
-
Explanation
All changed code connects to [
#23497]. The product change is limited to the option-fallback expression incomposerDraftStore.ts. The regression test inmodelSelection.test.tsverifies that expression.scripts/fixtures/composer-option-fallback.mjsis a manual synthetic fixture. It builds the optionless, explicit-low, and inherited thread states used for the bug's before/after proof.knip.jsoncregistersscripts/fixtures/*.mjsas Knip entries so that the fixture is not reported as unused. These are verification support for the same fallback behavior, not unrelated product, wire, or persistence changes. The PR description reports no wire or persistence changes. The diffs reviewed here do not show any such change.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
vouch:trusted
juliusmarminge
added
pr:ready
|
Closing in favour of #16546, which makes the same change. |
juliusmarminge
removed
the
pr:ready
Problem
With the new-thread default set to Claude Opus 5.5 with High effort, an existing thread saved without options displayed High and sent an effort override when its composer was left untouched. It should keep the provider default, Medium.
Change
Use the thread selection as the source of its options, including when it has none. Fall back to project/settings options only when there is no thread selection. A focused regression test covers the omitted-options case; a synthetic fixture rebuilds the captured states and is registered as a manual entry for Knip.
Scope and approval
Fixes #23497
GitCafe: https://git.cafe/pingdotgg/t3code/issues/23497
Triaged bug via the automated issue flow. This fixes the reproduced composer fallback and preserves the existing new-thread and explicit-option behavior.
Verification
The regression fails on main with
expected [{ id: "effort", value: "high" }] to be undefinedand passes with the fix. Focused model-selection, draft-store and ChatView logic tests: 142/142 passed. Targeted lint and web typecheck passed. Both independent code reviewers returned MERGE. The final scoped tests, targeted lint, and web typecheck also pass after merging main.Independent proof audit: V1–V4 PASS; 84 artifact hashes verified, all 24 diffs recomputed, and fresh V1 R/F/O captures match exactly with zero changed pixels.
Evidence
Full evidence index · Fixture matrix
Rendering environment
Saved thread selections omitted options. Environment default was Claude Opus 5.5 / effort High; project override inherited. The live descriptor declares Medium as default. Before: High selected and dispatched. After: Medium selected; effort omitted from dispatch. Both send implicit
fastMode:false. All captured drafts remained optionless when opening/closing menus.Linux / Chrome 154; DPR 2, zoom 1, loaded system fonts, scroll 0,0, pointer 0,450. CSS viewports exactly 1440×900 and 390×844. Built-in snapshot caps wide PNGs to 1280×800; narrow PNGs are 780×1688. Source recordings are 2880×1800 and 780×1688. Sources are published unchanged. Diffs use equal original dimensions, no post-capture resizing/registration. Browser measurement · Recording metadata · All eight source videos decode successfully
Four affected rows, each with before/after recordings, resting (R), focused (F), and open-menu (O) stills. At 390 wide, the real More composer controls overflow is used.
V1 — 1440×900, light — complete
Steps: start resting; focus and type
Reply with OK.; open the traits/overflow menu without selecting; close it; submit; wait for the successful receipt and completed real Claude run. Actual UTC milestones below align the two recordings.Before recording · After recording · Before dispatch/receipt/projection · After dispatch/receipt/projection
Threads:
proof-V1-clean→proof-V1. Both created optionless through real RPC; exact persisted create events are in the manifest.V2 — 1440×900, dark — complete
Steps: start resting; focus and type
Reply with OK.; open the traits/overflow menu without selecting; close it; submit; wait for the successful receipt and completed real Claude run. Actual UTC milestones below align the two recordings.Before recording · After recording · Before dispatch/receipt/projection · After dispatch/receipt/projection
Threads:
proof-V2-take2→proof-V2-clean. Both created optionless through real RPC; exact persisted create events are in the manifest.V3 — 390×844, light — complete
Steps: start resting; focus and type
Reply with OK.; open the traits/overflow menu without selecting; close it; submit; wait for the successful receipt and completed real Claude run. Actual UTC milestones below align the two recordings.Before recording · After recording · Before dispatch/receipt/projection · After dispatch/receipt/projection
Threads:
proof-V3-clean→proof-V3-clean. Both created optionless through real RPC; exact persisted create events are in the manifest.V4 — 390×844, dark — complete
Steps: start resting; focus and type
Reply with OK.; open the traits/overflow menu without selecting; close it; submit; wait for the successful receipt and completed real Claude run. Actual UTC milestones below align the two recordings.Before recording · After recording · Before dispatch/receipt/projection · After dispatch/receipt/projection
Threads:
proof-V4→proof-V4-clean. Both created optionless through real RPC; exact persisted create events are in the manifest.Other visible differences and controls
High changes to Medium, the selected menu row changes, and neighboring controls shift with the wider label. All changed pixels in the paired stills are confined to those areas. Narrow focused pairs are pixel-identical because the trait control is hidden. Diff0 amplifies every nonzero RGB delta by 8; DiffT does the same for deltas ≥26/255. The index includes raw counts and the full unmasked images.
Explicit Low and draft overrides remain Low; new threads still inherit the default. Omitted options and explicit empty options both preserve provider defaults. All eight captured real Claude runs completed successfully.
Web was tested at 1440×900 and 390×844 in light and dark. Desktop shares this web helper; its Electron shell was not separately run. Native mobile already uses the saved selection directly and is unaffected. No wire or persistence changes.
Captures use the source SHAs shown above. The later fixture-only commit leaves production code and tests byte-identical to the after capture.
Main was merged as 09c06b91ac; the three fix files remain byte-identical to the audited head.
CI initially found the manual fixture as unused. Registering
scripts/fixtures/*.mjsas a Knip entry fixes it:vp run knip:check, scoped formatting/lint, and both generated-file checks pass. Production code, test, and fixture are unchanged.Models: Claude Opus 5.5 and GPT-6.1-Sol, through Claude Code and Codex in T3 Code.