Repository navigation
fix(jev): exclude Claude Code reminders from decision state - #6662
Conversation
Treat Claude Code system-reminder blocks as protected envelopes when building JEV decision state. Long injected guidance no longer displaces the real request in the task sample. Adds regression tests and updates the guide and structure contract. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughJEV sampling now removes recognized Codex protected envelopes and Claude Code system reminders from sampled text before clipping. Tests cover converted Claude messages, nested and unclosed reminders, and reminder-free behavior. ChangesJEV Reminder Filtering
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete issue in the reminder-filtering change is established here; normal validation can continue while the PR remains draft. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.)
✨ 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. |
|
Maintainer integration into
|
Summary
Evidence: code read — the production change, regression tests, public guide, and owning structure contract were inspected.
<system-reminder>blocks as protected envelopes when building JEV decision state. Long injected project guidance, context, and attribution notes no longer displace the real request in the 500-character task sample.Verification
Evidence: executed — the retained command logs were inspected; their selected result lines are reproduced verbatim below. This revision changes only the PR description, so passing checks on unchanged code were not rerun. Commands run at the repository root unless a repository-relative working directory is stated. Focused tests and layout/ratchet guards used Bun 1.3.14; the wrapped changed-mode and explicit baseline runs used the pinned Bun 1.4.0, with
--isolate --parallel=4.Evidence: code read — reviewed
git diff origin/devacross the envelope filter, routing regressions, public guide, and owning structure contract for secrets, auth, and unsafe defaults; no introduced secrets, auth changes, or unsafe defaults were found.Focused tests and repository gates
Regression demonstration (executed, retained pre-fix log): before the production fix,
bun test tests/routing/jev-decision.test.tsreported34 pass,5 fail, andRan 39 tests across 1 file. [464.00ms](exit 1). The reminder-free compatibility case was already passing; the five reminder regressions failed.Reproducible clean origin/dev comparison
Evidence: executed — both original logs and the selected-file manifest are retained; the manifest extraction and normalized failure-name comparison were executed again for the earlier description correction. The test runs themselves were not repeated. The recorded baseline was detached at
f92482065217bae891a88e27fd6c00012f212042, the locally resolvedorigin/devand branch HEAD, with shared dependencies; that worktree has been removed. This does not attest to the latest remote tip.Running
bun run test:changedon the baseline selected zero tests (0 pass,0 fail,Ran 0 tests across 0 files. [1313.00ms], exit 0); this is not equivalent coverage. An initial explicit run of only the ten files containing failures reproduced 13 names, not all 29:The successful reproduction used all 401 unique repository-relative
tests/*.test.ts:file labels emitted by the original changed-mode log, sorted in C locale. The retained baseline command's 401 file arguments exactly match that manifest. Bun reported 1,528 files in changed mode but did not print labels for every file: this 401-file set is a reproduction subset, not the complete changed-mode selection or the full suite.The following recipe reconstructs that exact file selection and baseline invocation from a changed-mode log. Run from the repository root with root, GUI, and docs dependencies available. The explicit invocation expands to
bun run testfollowed by every manifest line in order. Temporary paths are variables, and the detached worktree is removed afterwards; the logs remain available for comparison. The recipe is a reproduction procedure, not a claim that it was rerun during the earlier description-only correction.Recorded explicit baseline test results:
Name comparison, rerun from the retained logs (only the trailing duration is removed; the entire test name and describe path are otherwise compared):
The single baseline-only case is
id-only discovery and a warm cache yield management rows accepted by the Combo pickerintests/providers/minimax-combo-image-input.test.ts. These results establish that all 29 changed-mode failure names also occurred without this patch in the same local environment and broad reproduction subset. They do not prove their root causes, that the failures are deterministic, or that changed-mode validation is green. Evidence: code read —scripts/test.tsgives several service/native-toggle files separate processes and homes in filter-less full-suite mode, but not in changed mode or explicit file-list runs.Documentation
Evidence: executed — retained build/install logs, using repository-relative commands:
The baseline reproduction uses
(cd "$verification/dev/docs-site" && bun run build)in the recipe above. The retained branch and clean-baseline logs each contain the identical Node-version error shown above and exit code 1. Building with Bun's runtime succeeded on the branch:Full-suite scope exception and review readiness
Evidence: executed — changed-mode coverage took 497.7 seconds for 32,650 tests across 1,528 files; the broad clean-baseline reproduction took another 315.7 seconds. Together, those runs already used about 13.6 minutes. Running the larger full suite as an additional local pass was disproportionate to this small envelope-filter change within the local verification session, so the documented
AGENTS.mdresource exception was used. No unmeasured full-suite duration or resource-contention claim is made.Evidence: not checked — the filter-less
bun run testfull suite was not run locally; the documented full-suite scope exception above applies. That coverage includes files outside the changed-mode selection, subprocess/read-as-data dependencies not discoverable by the import graph, and the runner's full-suite-only serial lanes. A standard documentation build on Node 22.12+ and non-Linux runtime coverage also remain for CI. Focused validation passed; changed-mode validation still exits 1, with all 29 failure names reproduced on the clean baseline as documented above. Exact-head CI is left to maintainers because fork contributors cannot start repository CI; no CI pass is claimed. The PR is ready for review under the documented test-scope exception, not a claim that the full suite or changed-mode run passed. No GUI file changed, so no screenshot is applicable.Compatibility and remaining limitations
<system-reminder>markup is treated as context regardless of message origin, consistent with existing Codex envelope handling. This is not a general secret scanner or an XML parser; ordinary text may still contain sensitive content.Checklist
Evidence: code read / executed — the diff is limited to the envelope filter, its routing regressions, the guide, and the owning structure contract; the recorded
privacy:scanpasses. The explicit diff security review is documented under Verification. Evidence: not checked — exact-head full-suite CI remains for maintainers; review readiness uses the documented local test-scope exception.🤖 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
Documentation