Repository navigation
fix(jev): preserve applied effort transitions in combo logs - #6663
Conversation
Preserve the caller-to-JEV effort transition in request and attempt requestedEffort labels for the Responses child. Native Chat children keep the caller effort label since they do not apply the JEV override. Telemetry-only change; outbound effort, routing and retry policy are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughCombo effort labels now show a differing JEV-selected effort for applicable Responses children and preserve later child transitions. Native Chat children retain caller effort and native-child transitions. Tests and documentation cover these behaviors, including fallback attempts. ChangesJEV Combo Effort Labels
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The reviewed changes preserve the reported effort transitions in request and attempt labels, including native and fallback behavior. This review found no behavior-based reason to block merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. UI screenshot waived by a maintainer comment. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Maintainer lane note (compat hardening sweep, lane B).
Ready for maintainer integration. |
|
Lane B refresh after #6662 landed (as requested by the coordinator).
Ready for maintainer integration. |
|
Maintainer integration into
|
Summary
requestedEffortlabels when the selected effort is applied to the Responses child. A caller requestinghighwith JEV selectingxhighrecordshigh->xhigh, including later adjustments such ashigh->xhigh->medium.Verification
[executed] Commands below are relative to the repository root unless a subshell changes directory. The test wrapper used Bun 1.4.0; direct
bun testand documentation commands used Bun 1.3.14. This PR targetsdev; focused validation passed, while broad validation is not green. The full suite was not run locally: concurrent test-suite contention made a full run disproportionately expensive. The documented test-scope exception applies, with focused commands and results, broad-run comparisons, and remaining CI coverage recorded below. No GUI files changed, so no UI screenshot is required.[code read] Reviewed the complete branch diff against
origin/devfor secrets, authentication changes, and unsafe defaults; found no real credentials, auth-boundary changes, or unsafe-default changes (the added decision-service key is a mocked test fixture).Focused validation and repository guards
Native-path baseline evidence
[code read / executed] Both
HEADand the localorigin/devref weref92482065217bae891a88e27fd6c00012f212042. The native body builder is unchanged from that revision. Export the baseline production code and copy only the updated regression file into it; this is not an assertion that the new test existed ondev.[executed] Reproduction setup, retaining dependency symlinks but no production patch:
mkdir -p .tmp/effort-label-baseline git archive origin/dev | tar -x -C .tmp/effort-label-baseline ln -s ../../node_modules .tmp/effort-label-baseline/node_modules ln -s ../../../gui/node_modules .tmp/effort-label-baseline/gui/node_modules ln -s ../../../docs-site/node_modules .tmp/effort-label-baseline/docs-site/node_modules cp tests/responses/chat-native-combo.test.ts .tmp/effort-label-baseline/tests/responses/chat-native-combo.test.ts[executed] Setup commands exited 0. To reproduce the same baseline after
origin/devmoves, use the recorded commit in thegit archivecommand instead. The same native-only selection ran on the changed tree and the exported baseline:[executed] Compared case names (all under
native Chat candidates in a combo):JEV wire effort: lane=native, selected=low, pin=unset, label=highJEV wire effort: lane=native, selected=xhigh, pin=unset, label=highJEV wire effort: lane=native, selected=high, pin=unset, label=highJEV wire effort: lane=native, selected=low, pin=medium, label=high->medium[executed] On both runtimes, native children send
reasoning_effort=highfor unpinned callerhigh, even when JEV selectsloworxhigh. With amediummodel pin, both sendmediumand labelhigh->medium. This establishes the native outbound-effort gap predates this label patch; fixing that gap is separate follow-up work.Import-connected validation and broad-failure caveat
[executed] Failures were in
tests/claude-integration/claude-models-discovery.test.ts(12 failures: one timeout followed by 11SPEND_LEDGER_OWNER_HOME_CONFLICTfailures) andtests/claude-integration/claude-picker-recovery.test.ts(two timeouts and one unhandled assertion error). The picker timeout names werea replacement process retries the recorded predecessor before rotating and armingandreplacement defers a pending certificate still published by a live owner.[executed] The same two files pass in isolation on both trees, using the baseline export above:
[executed] The two files above were re-run with the same command on a clean detached worktree of the recorded
origin/devcommit and on this branch, sequentially, each with its ownTMPDIR. Both trees passed 16/16 (0 fail), so the 14 failures and one error from the first import-connected run did not reproduce at file level on either tree.[executed] Broad comparison. Because the failures did not reproduce in isolation, the exact 510 test files that the import-connected selection covered (listed from a JUnit report of a second
test:changedrun) were run through the same wrapper command on a cleandevworktree and on this branch, sequentially. Setup and commands, with$BASELINE_WORKTREEa detached worktree of the recordedorigin/devcommit that hasnode_modulessymlinked from the main checkout:[executed] The dev run covers 509 files because
tests/server/combo-requested-effort.test.tsis new in this branch and absent fromdev. Failure-name comparison ((fail)lines with timing suffixes stripped; 141 distinct names on dev, 139 on the branch): 139 names fail on both trees; 0 fail only on the branch; 2 fail only on dev (a store-enabled turn leaves the native lane, so its reset is not replacedandthe replacement send's request copy is released before the replacement stream is relayed, both intests/responses/chat-native-spend.test.ts). The single unhandled error is the same/api/system/memory500-versus-200 assertion on both trees. The selection run also had a second unhandled error whiletests/cli/cli-agent-settings.test.tswas running:ReferenceError: Cannot access 'chatgptDesktopSchema' before initializationinsrc/config/schema/config-schema.ts, a module-initialization-order error. This patch does not touch the config schema, and the error did not recur in either comparison run. No failure is unique to this branch, so none is attributable to this patch from this evidence.[executed] The failing set is not stable and tracks host load. The host load average was roughly 170 to 360 during these runs because other test suites were running, and the first run (load not recorded) failed a different set (14 failures plus 1 error, all in the two
tests/claude-integrationfiles above) than the later runs (about 145 failures concentrated intests/server/server-management-auth.test.ts, WebSocket steering and injection tests undertests/responses,tests/codex-integration/doctor.test.ts, and others). None of the first run's 14 names failed in either later run. Treat the broad-suite failures as load-sensitive and pre-existing ondevfor the 139 shared names; this does not show the suite is green on a quiet host.[not checked] A re-run on an idle host, and full-suite plus exact-head CI validation, remain outstanding. This PR touches only the combo label path, and the files that failed are unrelated to it (management auth, WebSocket steering, doctor, Claude discovery and picker recovery).
Documentation build
[executed] The normal documentation build is blocked by the same unsupported runtime on changed and baseline trees; the explicit Bun-runtime build succeeds. This does not claim the required normal build passed.
[not checked] Full-suite and exact-head CI validation remain outstanding. Exact-head repository CI is left to maintainers because fork contributors cannot start it; required checks must pass before merge. Live decision-service/provider integration and browser rendering were not tested.
Checklist
Notes
protocols.rollout.nativeChatCombos=true, JEV can select a different effort while the native Chat child still sends the caller/configured effort. This PR corrects the label only and deliberately does not fix that separate outbound-effort gap. Decision metadata continues to report what JEV selected, not a claim that the native child applied it.🤖 Generated with Claude Code
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit