Skip to content

fix(codex): pass budget-scaled maxRenderedContextChars to legacy mirrored-history projection#94647

Closed
wings1029 wants to merge 1 commit into
openclaw:mainfrom
wings1029:fix/issue-84084-legacy-projection-budget-cap
Closed

fix(codex): pass budget-scaled maxRenderedContextChars to legacy mirrored-history projection#94647
wings1029 wants to merge 1 commit into
openclaw:mainfrom
wings1029:fix/issue-84084-legacy-projection-budget-cap

Conversation

@wings1029

@wings1029 wings1029 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

The legacy mirrored-history fallback in extensions/codex/src/app-server/run-attempt.ts calls projectContextEngineAssemblyForCodex without passing maxRenderedContextChars. When omitted, the function defaults to DEFAULT_RENDERED_CONTEXT_CHARS = 24_000 (~6,000 tokens of rendered history), even when the model has a much larger context budget (e.g., 272,000 tokens).

This PR passes the budget-scaled maxRenderedContextChars to both legacy projection call sites, matching the active-context-engine branch behavior added in PR #80761.

Fixes #84084

Real behavior proof (required for external PRs)

Behavior addressed: Legacy mirrored-history projection caps rendered context at 24,000 characters regardless of the model's actual context budget.

Real setup tested:

  • Runtime: Node v22, OpenClaw main @ beea31a

Exact steps or command run after fix:

cd $WORKTREE_DIR
node scripts/run-vitest.mjs extensions/codex/src/app-server/context-engine-projection.test.ts
node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts
node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.context-engine.test.ts

After-fix evidence:

Test Files  3 passed (3)
     Tests  141 passed | 1 skipped (1 pre-existing timeout)

Diff:

--- a/extensions/codex/src/app-server/run-attempt.ts
+++ b/extensions/codex/src/app-server/run-attempt.ts
@@ -908,6 +908,12 @@
       assembledMessages: historyMessages,
       originalHistoryMessages: historyMessages,
       prompt: params.prompt,
+      maxRenderedContextChars: resolveCodexContextEngineProjectionMaxChars({
+        contextTokenBudget: params.contextTokenBudget,
+        reserveTokens: resolveCodexContextEngineProjectionReserveTokens({
+          config: params.config,
+        }),
+      }),
     });

@@ -1088,6 +1094,12 @@
       assembledMessages: newerVisibleMessages,
       originalHistoryMessages: historyMessages,
       prompt: params.prompt,
+      maxRenderedContextChars: resolveCodexContextEngineProjectionMaxChars({
+        contextTokenBudget: params.contextTokenBudget,
+        reserveTokens: resolveCodexContextEngineProjectionReserveTokens({
+          config: params.config,
+        }),
+      }),
     });

Observed result after the fix: All existing tests pass. contextTokenBudget is already in scope at both call sites. When contextTokenBudget is undefined, resolveCodexContextEngineProjectionMaxChars falls back to DEFAULT_RENDERED_CONTEXT_CHARS — no behavior change for that case.

What was not tested: Live high-window Codex session with thread rebuilds. The fix is source-proven and the helpers are already tested.

Tests and validation

Suite Result
context-engine-projection.test.ts 13/13 ✅
run-attempt.test.ts 104/105 ✅ (1 pre-existing timeout)
run-attempt.context-engine.test.ts 24/24 ✅

Risk checklist

Did user-visible behavior change? (Yes — legacy fallback now uses budget-appropriate cap instead of 24k hard default)

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No)

What is the highest-risk area?

  • When contextTokenBudget is undefined, resolveCodexContextEngineProjectionMaxChars returns DEFAULT_RENDERED_CONTEXT_CHARS — same as before.

How is that risk mitigated?

  • The helper function already handles undefined/missing budget correctly. Both params.contextTokenBudget and params.config are already in scope and used by other paths.

Current review state

What is the next action?

  • Maintainer review

@wings1029 @claude
fix(codex): pass budget-scaled maxRenderedContextChars to legacy mirr…
b914c39 
…ored-history projection

- Add maxRenderedContextChars to applyFreshThreadContinuityProjection (fresh thread legacy path)
- Add maxRenderedContextChars to applyResumeStaleBindingContinuityProjection (stale binding legacy path)
- Reuse existing resolveCodexContextEngineProjectionMaxChars / resolveCodexContextEngineProjectionReserveTokens helpers
- Matches the active-context-engine pattern from PR openclaw#80761

Fixes openclaw#84084

Co-Authored-By: Claude <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added extensions: codex size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 12:52 PM ET / 16:52 UTC.

Summary
The branch adds the existing budget-scaled maxRenderedContextChars calculation to fresh-thread and stale-binding legacy Codex mirrored-history projections.

PR surface: Source +12. Total +12 across 1 file.

Reproducibility: yes. source inspection shows current main omits maxRenderedContextChars on the legacy fresh-thread and stale-binding projections while the callee defaults omitted values to 24,000 rendered chars. I did not run a live high-window Codex rebuild in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84084
Summary: This PR is the candidate fix for the linked legacy mirrored-history context-budget issue and partially overlaps the already-merged active-path projection fix.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted real behavior proof, such as a terminal run or app-server log showing a legacy mirrored-history rebuild with a large contextTokenBudget after this patch.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies targeted Vitest output, but no after-fix live/runtime Codex session, terminal projection-path run, or redacted app-server log showing the legacy fallback cap change; add redacted proof and update the PR body to trigger re-review.

Risk before merge

  • [P2] The supplied proof is targeted Vitest output only; an external PR still needs redacted real runtime or terminal proof of the legacy fallback path, unless a maintainer explicitly overrides the proof gate.
  • [P1] The branch adds no focused regression assertion for the two legacy call sites, so maintainers may want a small test even though the helper and active path are already covered.

Maintainer options:

  1. Decide the mitigation before merge
    Land the parity fix after contributor-supplied real behavior proof or maintainer proof override, preferably with focused legacy-call-site regression coverage.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Human review is needed for the external real-behavior-proof gate; there is no narrow ClawSweeper code repair because the patch itself is source-aligned.

Security
Cleared: The diff only threads an existing numeric cap into existing projection calls and does not touch secrets, auth, dependencies, workflows, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land the parity fix after contributor-supplied real behavior proof or maintainer proof override, preferably with focused legacy-call-site regression coverage.

Do we have a high-confidence way to reproduce the issue?

Yes: source inspection shows current main omits maxRenderedContextChars on the legacy fresh-thread and stale-binding projections while the callee defaults omitted values to 24,000 rendered chars. I did not run a live high-window Codex rebuild in this read-only review.

Is this the best way to solve the issue?

Yes: reusing the existing budget-scaled projection helper at the two legacy continuity calls is the narrowest maintainable fix. The remaining blocker is proof and optional focused regression coverage, not a different product direction.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1eaace70e3b4.

Label changes

Label changes:

  • add P2: This is a normal-priority Codex context-retention bug with a narrow legacy fallback surface, not a runtime outage.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies targeted Vitest output, but no after-fix live/runtime Codex session, terminal projection-path run, or redacted app-server log showing the legacy fallback cap change; add redacted proof and update the PR body to trigger re-review.

Label justifications:

  • P2: This is a normal-priority Codex context-retention bug with a narrow legacy fallback surface, not a runtime outage.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies targeted Vitest output, but no after-fix live/runtime Codex session, terminal projection-path run, or redacted app-server log showing the legacy fallback cap change; add redacted proof and update the PR body to trigger re-review.
Evidence reviewed

PR surface:

Source +12. Total +12 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 12 0 +12
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 12 0 +12

What I checked:

Likely related people:

  • 100yenadmin: Authored the merged prior PR that added resolveCodexContextEngineProjectionMaxChars, active-path wiring, and projection tests that this PR extends to legacy fallback call sites. (role: introduced budget-aware projection path; confidence: high; commits: f7ab8c26b1e7; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/context-engine-projection.ts, extensions/codex/src/app-server/context-engine-projection.test.ts)
  • VACInc: Authored the recent projection truncation fix in the same helper and tests, so they are relevant for context-retention behavior around this surface. (role: recent projection behavior contributor; confidence: medium; commits: 72e164a3fe20; files: extensions/codex/src/app-server/context-engine-projection.ts, extensions/codex/src/app-server/context-engine-projection.test.ts)
  • jalehman: Merged the prior budget-scaling PR via squash and was part of the visible review path for the Codex projection change being extended here. (role: prior merger and reviewer; confidence: medium; commits: f7ab8c26b1e7; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/context-engine-projection.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 18, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 18, 2026
Merged
@wings1029

wings1029 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #94093.

PR #94093 (Prevent Codex thread rotation from losing next-step context) was merged on 2026-06-18 and already fixes the same problem more thoroughly: it computes codexContextProjectionMaxChars once at the top of runCodexAppServerAttempt and passes it to ALL three projection call sites (both legacy paths + the active-context-engine path), eliminating the 24k hard cap.

The merge conflict was a signal that the fix had already landed on main through another path.

@wings1029 wings1029 closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: codex P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Codex legacy mirrored-history fallback ignores contextTokenBudget and caps high-window sessions at ~24k rendered chars

1 participant

@wings1029