Skip to content

fix(jev): exclude Claude Code reminders from decision state - #6662

Merged
lidge-jun merged 2 commits into
lidge-jun:devfrom
geunwoojun99:fix/jev-strip-claude-reminders
Oct 6, 2026
Merged

lidge-jun merged 2 commits into
lidge-jun:devfrom
geunwoojun99:fix/jev-strip-claude-reminders

Conversation

@geunwoojun99

@geunwoojun99 geunwoojun99 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Evidence: code read — the production change, regression tests, public guide, and owning structure contract were inspected.

  • Treat Claude Code <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.
  • Use the existing bounded, stack-based envelope filter for the task and assistant/tool-output tails. Reminders are contextual text on all three surfaces; stripping before sampling prevents them from consuming the sample budget. No separate regex replacement or full cleaned-text allocation is needed.
  • Preserve the special Codex active-goal fallback, except that a goal quoted inside a reminder cannot become the task. Reminder-only content supplies no task; nested and unclosed reminders follow existing protected-envelope semantics.
  • Add converter-backed regressions for string and content-block Claude Code requests, two leading reminders including long project guidance, latest-user selection, clipping, nested reminders, assistant/tool tails, unchanged input bodies, and byte-identical reminder-free states. Update the public guide and owning structure contract.

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/dev across 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

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

$ bun test tests/routing/jev-decision.test.ts
 39 pass
 0 fail
 187 expect() calls
Ran 39 tests across 1 file. [404.00ms]
Exit code: 0

$ bun test tests/routing/jev-typesafe-golden.test.ts tests/routing/jev-model-backend.test.ts
 17 pass
 0 fail
 99 expect() calls
Ran 17 tests across 2 files. [1351.00ms]
Exit code: 0

$ bun run test:changed
 32511 pass
 110 skip
 29 fail
 516124 expect() calls
Ran 32650 tests across 1528 files. [497.71s]
[test] suite finished in 497.7s (exit 1).
error: script "test:changed" exited with code 1
Exit code: 1

$ 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

$ bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts
 18 pass
 0 fail
 555 expect() calls
Ran 18 tests across 2 files. [506.00ms]
Exit code: 0

$ bun test tests/ci-workflows/file-size-ratchet.test.ts
 9 pass
 0 fail
 85 expect() calls
Ran 9 tests across 1 file. [527.00ms]
Exit code: 0

$ git diff --check
Exit code: 0

Regression demonstration (executed, retained pre-fix log): before the production fix, bun test tests/routing/jev-decision.test.ts reported 34 pass, 5 fail, and Ran 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 resolved origin/dev and branch HEAD, with shared dependencies; that worktree has been removed. This does not attest to the latest remote tip.

Running bun run test:changed on 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:

$ bun run test tests/cli/cli-help.test.ts tests/codex-integration/codex-shim-overlay-readiness.test.ts tests/codex-integration/codex-shim-path-readiness.test.ts tests/codex-integration/native-codex-toggle.test.ts tests/codex-integration/native-grok-toggle.test.ts tests/config/serving-runtimes.test.ts tests/server/subagent-model-force-api.test.ts tests/service/service-ownership-state.test.ts tests/service/service-sqlite-home.test.ts tests/service/service.test.ts
 392 pass
 1 skip
 13 fail
 1500 expect() calls
Ran 406 tests across 10 files. [11.55s]
[test] suite finished in 11.6s (exit 1).
error: script "test" exited with code 1
Exit code: 1

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 test followed 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.

repo=$(pwd)
verification=$(mktemp -d)
changed_status=0
bun run test:changed > "$verification/changed.log" 2>&1 || changed_status=$?
sed -n 's/^\(tests\/.*\.test\.ts\):$/\1/p' "$verification/changed.log" \
  | LC_ALL=C sort -u > "$verification/selected-files.txt"
printf 'selected file labels: '; wc -l < "$verification/selected-files.txt"
printf 'selection SHA-256: '; sha256sum < "$verification/selected-files.txt"

git worktree add --detach "$verification/dev" f92482065217bae891a88e27fd6c00012f212042
trap 'git -C "$repo" worktree remove --force "$verification/dev"' EXIT
ln -s "$repo/node_modules" "$verification/dev/node_modules"
ln -s "$repo/gui/node_modules" "$verification/dev/gui/node_modules"
ln -s "$repo/docs-site/node_modules" "$verification/dev/docs-site/node_modules"
dev_status=0
(cd "$verification/dev" && xargs -r -a "$verification/selected-files.txt" bun run test) \
  > "$verification/dev-explicit.log" 2>&1 || dev_status=$?
# Reproduce the documentation runtime failure on the same baseline.
dev_docs_status=0
(cd "$verification/dev/docs-site" && bun run build) \
  > "$verification/dev-docs.log" 2>&1 || dev_docs_status=$?
printf 'changed exit: %s; dev explicit exit: %s; dev docs exit: %s\n' \
  "$changed_status" "$dev_status" "$dev_docs_status"

for run in changed dev-explicit; do
  sed -n 's/^(fail) //p' "$verification/$run.log" \
    | sed -E 's/ \[[0-9.]+ms\]$//' \
    | LC_ALL=C sort -u > "$verification/$run.names"
done
printf 'changed failure names: '; wc -l < "$verification/changed.names"
printf 'dev failure names: '; wc -l < "$verification/dev-explicit.names"
printf 'shared failure names: '
LC_ALL=C comm -12 "$verification/changed.names" "$verification/dev-explicit.names" | wc -l
printf 'changed-only failure names: '
LC_ALL=C comm -23 "$verification/changed.names" "$verification/dev-explicit.names" | wc -l
printf 'dev-only failure names: '
LC_ALL=C comm -13 "$verification/changed.names" "$verification/dev-explicit.names" | wc -l
LC_ALL=C comm -13 "$verification/changed.names" "$verification/dev-explicit.names"
git worktree remove --force "$verification/dev"
trap - EXIT

Recorded explicit baseline test results:

selected file labels: 401
selection SHA-256: 490e20a25eef0dcb33626706d43acadde0309a19dde6877a5de78c54814143ed  -

 11314 pass
 9 skip
 30 fail
 270990 expect() calls
Ran 11353 tests across 401 files. [315.66s]
[test] suite finished in 315.7s (exit 1).
error: script "test" exited with code 1
Exit code: 1

Name comparison, rerun from the retained logs (only the trailing duration is removed; the entire test name and describe path are otherwise compared):

changed failure names: 29
dev failure names: 30
shared failure names: 29
changed-only failure names: 0
dev-only failure names: 1

The single baseline-only case is id-only discovery and a warm cache yield management rows accepted by the Combo picker in tests/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.ts gives 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:

$ (cd docs-site && bun install --frozen-lockfile)
377 packages installed [747.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

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:

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

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.md resource exception was used. No unmeasured full-suite duration or resource-contention claim is made.

Evidence: not checked — the filter-less bun run test full 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

  • Code read / executed: reminder-free compatibility is covered by unchanged clipping, trimming, state shape, and serialized-state expectations, plus passing payload golden and model-backend tests. The model request itself is not modified; only the decision-state projection changes.
  • Code read: recognized literal <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.
  • Code read / not checked: tests use synthetic text and the real inbound request converter. No live Claude Code session or live decision service was exercised. No macOS or Windows runtime validation was performed.
  • Code read / executed: no new test file or baseline-cap increase is introduced; regression cases extend the registered routing test file, and the layout and file-size guards pass. The public guide and owning structure contract are updated. Translation parity beyond the English guide is not checked.

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.

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:scan passes. 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

    • Task text now excludes Claude Code system reminders and recognized Codex internal context across sampled messages, assistant responses, and tool results. Tasks inside reminders are not used as fallbacks.
    • Reminder-wrapped messages are handled correctly before text clipping, while reminder-free behavior remains unchanged.
  • Documentation

    • Clarified which protected content is omitted from visible task text and noted that the original model request remains unchanged. Ordinary assistant or tool output may still contain sensitive content.

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

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 834301be-6c36-4581-b0ba-39f9a4df963e
📥 Commits

Reviewing files that changed from the base of the PR and between f924820 and 4671e96.

📒 Files selected for processing (4)
  • docs-site/src/content/docs/guides/combos.md
  • src/combos/jev.ts
  • structure/providers/jev-decision.md
  • tests/routing/jev-decision.test.ts

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


📝 Walkthrough

Walkthrough

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

Changes

JEV Reminder Filtering

Layer / File(s) Summary
Filter protected text before sampling
src/combos/jev.ts:50, 306-307, structure/providers/jev-decision.md:57-61, docs-site/src/content/docs/guides/combos.md:477-481
JEV sampling excludes system reminders and recognized Codex protected envelopes before clipping. A goal inside a reminder cannot provide a fallback task. The documentation describes the filtering and notes that the original model request is unchanged.
Validate converted-message behavior
tests/routing/jev-decision.test.ts:13, 142-268
Tests cover string and block-form user content, clipping, nested and unclosed envelopes, assistant and tool-result text, and reminder-free output. They also check that conversion does not mutate the request body.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 4671e

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 states the main change: excluding Claude Code reminders from JEV decision state.
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ 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:36
@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:10
@lidge-jun

Copy link
Copy Markdown
Owner

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

Ready for maintainer integration.

@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: b9d8db5a0d853991587c7e179edf96842f424eb5
  • Hosted CI on that head: every applicable check completed successfully (ci, test 1-4/4, structure gate, gates, docker smoke, storage policy, api usage, npm-global, keyring, enforce-target, hygiene). Skipped jobs are gated off by their own path/event conditions. The only open item is the CodeRabbit-status resolve-pr wake of enforce-target, whose PR-event run on this head already succeeded.
  • Review: independent gpt-6.1-sol review in the compat sweep lane, folded; no unresolved review threads, no maintainer change requests.
  • Scope: bug fix in src/combos/jev.ts with structure/docs updates and focused test; no gui/ paths.
  • Full local suite not run (parallel lanes share one machine); focused tests and typecheck ran in the lane, remaining coverage is the hosted matrix above.

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