Skip to content

fix(jev): preserve applied effort transitions in combo logs - #6663

Merged
lidge-jun merged 4 commits into
lidge-jun:devfrom
geunwoojun99:fix/combo-forced-effort-label
Oct 6, 2026
Merged

lidge-jun merged 4 commits into
lidge-jun:devfrom
geunwoojun99:fix/combo-forced-effort-label

Conversation

@geunwoojun99

@geunwoojun99 geunwoojun99 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • [code read] Preserve the caller-to-JEV transition in request and attempt requestedEffort labels when the selected effort is applied to the Responses child. A caller requesting high with JEV selecting xhigh records high->xhigh, including later adjustments such as high->xhigh->medium.
  • [code read] Do not claim an unapplied JEV transition on opt-in native Chat children. That lane builds a separate request body and currently does not apply the initial JEV effort override. Its labels retain the caller effort and native adjustments; fallback attempts do not inherit the initial JEV effort.
  • [code read] Keep this a telemetry-only patch: outbound effort, selection, routing, and retry policy are unchanged. Extract a pure label helper, register its tests and Responses module inventory, and update the public guide and structure contracts. No file-size baseline or budget increase is needed; the combo owner shrinks by two lines.
  • [executed] Add seven wire-level cases driving the real Chat ingress against a loopback upstream: native and bridge lanes, higher/lower/unchanged JEV choices, and subsequent model pins. Assertions inspect the received request body, decision metadata, request label, attempt label, and protocol lane.

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 test and documentation commands used Bun 1.3.14. This PR targets dev; 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/dev for 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

$ bun run typecheck
$ bun x tsc --noEmit
Exit code: 0

$ bun run test -- tests/server/combo-requested-effort.test.ts tests/server/server-jev-combo-e2e.test.ts tests/responses/responses-core-modules.test.ts tests/responses/chat-native-combo.test.ts
 62 pass
 0 fail
 596 expect() calls
Ran 62 tests across 4 files. [3.78s]
[test] suite finished in 3.8s (exit 0).
Exit code: 0

$ bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts
 27 pass
 0 fail
 640 expect() calls
Ran 27 tests across 3 files. [1095.00ms]
Exit code: 0

$ bun run structure:check
$ bun scripts/structure-ssot.ts
structure/ SSOT checks passed
Exit code: 0

$ bun run privacy:scan
$ bun scripts/privacy-scan.ts
Privacy scan passed
Exit code: 0

$ git diff --check HEAD
Exit code: 0

Native-path baseline evidence

[code read / executed] Both HEAD and the local origin/dev ref were f92482065217bae891a88e27fd6c00012f212042. 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 on dev.

[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/dev moves, use the recorded commit in the git archive command instead. The same native-only selection ran on the changed tree and the exported baseline:

$ bun test tests/responses/chat-native-combo.test.ts --test-name-pattern 'JEV wire effort: lane=native'
 4 pass
 12 filtered out
 0 fail
 36 expect() calls
Ran 4 tests across 1 file. [1366.00ms]
Exit code: 0

$ (cd .tmp/effort-label-baseline && bun test tests/responses/chat-native-combo.test.ts --test-name-pattern 'JEV wire effort: lane=native')
 4 pass
 12 filtered out
 0 fail
 36 expect() calls
Ran 4 tests across 1 file. [868.00ms]
Exit code: 0

[executed] Compared case names (all under native Chat candidates in a combo):

  • JEV wire effort: lane=native, selected=low, pin=unset, label=high
  • JEV wire effort: lane=native, selected=xhigh, pin=unset, label=high
  • JEV wire effort: lane=native, selected=high, pin=unset, label=high
  • JEV wire effort: lane=native, selected=low, pin=medium, label=high->medium

[executed] On both runtimes, native children send reasoning_effort=high for unpinned caller high, even when JEV selects low or xhigh. With a medium model pin, both send medium and label high->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

$ bun run test:changed
$ bun scripts/test.ts --changed=dev
[test] changed mode comparison ref: origin/dev; merge base: f92482065217bae891a88e27fd6c00012f212042
bun test v1.4.0 (34cbb9a40) 4x PARALLEL
--changed: 12 changed files, running 511/2090 test files
 12348 pass
 37 skip
 14 fail
 1 error
 125272 expect() calls
Ran 12399 tests across 511 files. [309.67s]
[test] suite finished in 309.7s (exit 1).
error: script "test:changed" exited with code 1
Exit code: 1

[executed] Failures were in tests/claude-integration/claude-models-discovery.test.ts (12 failures: one timeout followed by 11 SPEND_LEDGER_OWNER_HOME_CONFLICT failures) and tests/claude-integration/claude-picker-recovery.test.ts (two timeouts and one unhandled assertion error). The picker timeout names were a replacement process retries the recorded predecessor before rotating and arming and replacement 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:

$ (cd .tmp/effort-label-baseline && bun run test -- tests/claude-integration/claude-models-discovery.test.ts tests/claude-integration/claude-picker-recovery.test.ts)
 16 pass
 0 fail
 182 expect() calls
Ran 16 tests across 2 files. [7.22s]
[test] suite finished in 7.2s (exit 0).
Exit code: 0

$ bun run test -- tests/claude-integration/claude-models-discovery.test.ts tests/claude-integration/claude-picker-recovery.test.ts
 16 pass
 0 fail
 182 expect() calls
Ran 16 tests across 2 files. [6.76s]
[test] suite finished in 6.8s (exit 0).
Exit code: 0

[executed] The two files above were re-run with the same command on a clean detached worktree of the recorded origin/dev commit and on this branch, sequentially, each with its own TMPDIR. 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:changed run) were run through the same wrapper command on a clean dev worktree and on this branch, sequentially. Setup and commands, with $BASELINE_WORKTREE a detached worktree of the recorded origin/dev commit that has node_modules symlinked from the main checkout:

git worktree add --detach "$BASELINE_WORKTREE" <recorded origin/dev commit>
ln -s "$MAIN_CHECKOUT/node_modules" "$BASELINE_WORKTREE/node_modules"
# selection: bun scripts/test.ts --changed=dev --reporter=junit --reporter-outfile="$SELECTION_XML"
#   -> list of test files from the report, stored in $FILES (510 paths)
(cd "$BASELINE_WORKTREE" && TMPDIR="$TMP_DEV" bun scripts/test.ts $FILES)
TMPDIR="$TMP_BRANCH" bun scripts/test.ts $FILES
git worktree remove --force "$BASELINE_WORKTREE" && git worktree prune
selection run on branch (--changed=dev):  12110 pass / 36 skip / 147 fail / 2 errors,  511 files
clean dev,  same 510 files:               12080 pass / 36 skip / 146 fail / 1 error,  509 files
branch,     same 510 files:               12112 pass / 36 skip / 144 fail / 1 error,  510 files

[executed] The dev run covers 509 files because tests/server/combo-requested-effort.test.ts is new in this branch and absent from dev. 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 replaced and the replacement send's request copy is released before the replacement stream is relayed, both in tests/responses/chat-native-spend.test.ts). The single unhandled error is the same /api/system/memory 500-versus-200 assertion on both trees. The selection run also had a second unhandled error while tests/cli/cli-agent-settings.test.ts was running: ReferenceError: Cannot access 'chatgptDesktopSchema' before initialization in src/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-integration files above) than the later runs (about 145 failures concentrated in tests/server/server-management-auth.test.ts, WebSocket steering and injection tests under tests/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 on dev for 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

$ (cd docs-site && bun install --frozen-lockfile)
Checked 379 installs across 484 packages (no changes) [58.00ms]
Exit code: 0

$ (cd docs-site && bun run build)
$ astro build
Node.js v20.20.2 is not supported by Astro!
Please upgrade Node.js to a supported version: ">=22.12.0"
error: script "build" exited with code 1
Exit code: 1

$ (cd .tmp/effort-label-baseline/docs-site && bun run build)
$ astro build
Node.js v20.20.2 is not supported by Astro!
Please upgrade Node.js to a supported version: ">=22.12.0"
error: script "build" exited with code 1
Exit code: 1

$ (cd docs-site && bun --bun run build)
14:45:14 [opencodex-internal-links] checked 78108 internal links across 561 pages
14:45:14 [build] 561 page(s) built in 30.99s
14:45:14 [build] Complete!
Exit code: 0

[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

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Notes

  • [code read / executed] Follow-up: with 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.
  • [code read] Missing caller effort preserves the child label; absent/null or equal forced effort preserves existing restoration behavior. Only the initial Responses child gets the JEV transition; native and fallback children retain their existing child transitions.
  • [executed] Loopback upstream coverage verifies the actual request received on native and bridge paths, while the decision service is mocked. No live credentials or external provider sends are part of this verification.
  • [not checked] Independent security review, full-suite validation, and exact-head CI are not claimed. Contributor review-readiness checks are completed under the documented test-scope exception; this does not claim merge readiness.

🤖 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

  • Bug Fixes
    • Request and attempt logs now show when a JEV-selected reasoning effort differs from the caller’s requested effort, including subsequent effort changes.
    • Native Chat requests retain their own effort labels and adjustments; they do not show a JEV effort that was not applied.
    • When a selected attempt fails and fallback runs, it uses the caller’s original effort. Targets without effort controls retain the caller’s requested effort in logs.
  • Documentation
    • Updated guidance to explain effort labels, native Chat behavior, and fallback handling.

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>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 9f087437-df9f-4ddb-9556-cff768e23a10
📥 Commits

Reviewing files that changed from the base of the PR and between 9311e70 and 83b2476.

📒 Files selected for processing (2)
  • docs-site/src/content/docs/guides/combos.md
  • structure/providers/jev-decision.md
 _____________________________________________________________
< Return early? Yes. Return *something meaningful*? Also yes. >
 -------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: e4addf6e-6f0d-4466-87f2-2037417e4a57
📥 Commits

Reviewing files that changed from the base of the PR and between 621a613 and 9311e70.

📒 Files selected for processing (2)
  • scripts/test-layout/layout.json
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Combo 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.

Changes

JEV Combo Effort Labels

Layer / File(s) Summary
Build and restore effort labels
src/server/responses/combo-requested-effort.ts, src/server/responses/core-combo.ts
The new helper combines the original requested-effort label with an optional forced effort and child transitions. The combo path passes the initial JEV effort for non-native children and omits it for native Chat children.
Validate and document effort behavior
tests/server/combo-requested-effort.test.ts, tests/server/server-jev-combo-e2e.test.ts, tests/responses/chat-native-combo.test.ts, tests/helpers/responses-core-source.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/guides/combos.md, structure/providers/jev-decision.md, structure/transports/responses.md
Tests cover label combinations, JEV-selected effort, child transitions, fallback attempts, and native and bridge behavior. Documentation describes these behaviors. Test layout and source inventory include the new helper and test.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9311e

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)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. (2 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preserving applied JEV effort transitions in Combo logs.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ 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.

0/4 boxes ticked.

UI screenshot waived by a maintainer comment.
Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

@geunwoojun99

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
github-actions Bot marked this pull request as ready for review October 6, 2026 07:37
@github-actions
github-actions Bot marked this pull request as draft October 6, 2026 12:28
@lidge-jun
lidge-jun marked this pull request as ready for review October 6, 2026 13:34
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer lane note (compat hardening sweep, lane B).

  • Merged origin/dev at f494403 into this branch (clean).
  • Fix on top: the new combo-requested-effort.test.ts registration pushed scripts/test-layout/layout.json from 1,999 to 2,000 lines, which the file-size ratchet reports as NEW_OVERSIZED (the file has no baseline entry). Commit 9311e70783 puts the registration on the same line as the adjacent server-jev-combo-e2e.test.ts entry; both registrations are kept and no cap changed.
  • Independent adversarial review (gpt-6.1-sol): the only blocker was the ratchet above; no correctness, authorization, privacy or scope findings.
  • Local focused checks on the head: chat-native-combo, combo-requested-effort and server-jev-combo-e2e 52 pass / 0 fail; ratchet + both layout guards 27 pass / 0 fail; bun run typecheck and bun run structure:check exit 0. The full local suite was not run (parallel lanes share the machine); coverage is left to hosted CI.
  • Hosted CI on the exact head 9311e70783: Cross-platform CI run 37463469610 completed success, including all nine Windows shards.
  • dev moved by chore(release): open dev at 2.80.0 before releasing 2.79.0 #6676, test(runner): share Bun's transpiler cache across sandboxed homes #6683 and fix(live): join client-created voice calls as the caller that owns them #6678 afterwards; fix(live): join client-created voice calls as the caller that owns them #6678 also edited layout.json. A local merge with current dev is clean, keeps layout.json at 1,999 lines, and passes the ratchet, layout guards, the three combo test files (79 pass / 0 fail), typecheck and structure check.

Ready for maintainer integration.

@github-actions
github-actions Bot marked this pull request as draft October 6, 2026 13:49
@lidge-jun
lidge-jun marked this pull request as ready for review October 6, 2026 14:41
@lidge-jun

Copy link
Copy Markdown
Owner

Lane B refresh after #6662 landed (as requested by the coordinator).

Ready for maintainer integration.

@github-actions
github-actions Bot marked this pull request as draft October 6, 2026 14:44
@lidge-jun
lidge-jun marked this pull request as ready for review October 6, 2026 14:45
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration into dev (MAINTAINERS.md dev-only maintainer integration; not a self-approval or an independent review).

  • Exact head: 83b24765677aa4bd24fbe7642617b19fe3c23302
  • Hosted CI on that head: all 38 latest check runs completed and every applicable one succeeded. Skipped jobs are gated off by their own path/event conditions.
  • dev is seven commits ahead and fix(service): credit Linux desktop Start at Login as restart protection in doctor #6679/fix(lifecycle): retryable restart-drain code and quiet journal no-op recovery #6686 also edited scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json. Without starting another hosted run, the coordinator merged this head onto origin/dev in a scratch worktree: clean auto-merge, layout.json at 1998 lines; layout/tooling/file-size-ratchet guards 27 pass, the JEV/combo test files 91 pass, bun run structure:check and bun x tsc --noEmit pass on that union tree.
  • Review: independent gpt-6.1-sol review in the compat sweep lane (PASS after the fix(jev): exclude Claude Code reminders from decision state #6662 reconciliation); no unresolved review threads, no maintainer change requests.
  • Scope: JEV combo logs keep applied effort transitions; no gui/ paths.
  • Full local suite not run; one Cross-platform CI run on the dev tip after the batch is the remaining check.

@lidge-jun
lidge-jun merged commit 0cd680a into lidge-jun:dev Oct 6, 2026
42 of 44 checks passed
@lidge-jun lidge-jun mentioned this pull request Oct 7, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants