[feat]: thread currentTokenCount into ContextEngine.assemble#81079
[feat]: thread currentTokenCount into ContextEngine.assemble#81079DatPham-6996 wants to merge 2 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 8, 2026, 8:21 AM ET / 12:21 UTC. Summary PR surface: Source +85, Tests +180, Docs +37. Total +302 across 13 files. Reproducibility: not applicable. this is a new optional context-engine/plugin SDK capability rather than a reported current-main bug reproduction. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Land a maintainer-approved assemble headroom contract only after the count is derived from the exact messages visible to Do we have a high-confidence way to reproduce the issue? Not applicable: this is a new optional context-engine/plugin SDK capability rather than a reported current-main bug reproduction. Is this the best way to solve the issue? No: the narrow API is plausible, but this patch still misaligns Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4bf70be01a21. Label changesLabel justifications:
Evidence reviewedPR surface: Source +85, Tests +180, Docs +37. Total +302 across 13 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (3 earlier review cycles)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Hi @jalehman could you please help me review this PR ? |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Addressed [P2] in The production loop-hook install site at
Test: new test in Tally: 194/194 tests pass on the touched files. PR description updated to reflect the new bullet, target test file, and counts. Real behavior proof from a live long-session run is still pending and will be posted before merge. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
This pull request has been automatically marked as stale due to inactivity. |
|
ClawSweeper PR egg: 🎁 locked until real behavior proof passes. Details
|
tokenBudget passed to ContextEngine.assemble now means the same thing on every harness path: the budget available for assembled messages, computed by the shared computeContextEngineMessageBudget helper (context window minus compaction reserve minus rendered system/user prompt pressure). currentTokenCount is now a messages-only estimate (estimateTranscriptTokenPressure), so engines size systemPromptAddition as tokenBudget - currentTokenCount with no double-counting. - PI main assemble: budget block now uses the shared helper; count is messages-only - PI tool loop: new assembleTokenBudget hook param carries the loop budget; afterTurn keeps the raw window per its contract; the hook resolves getRuntimeContext once per iteration instead of twice - Codex harness: derives the budget with reserve 0 (native compaction owns the reserve concept there) - plugin SDK re-exports both helpers instead of the removed estimatePrePromptTokens alias; docs and interface comments updated
b7aa56b to
65deda8
Compare
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
|
Closing due to inactivity. |
Summary
ContextEngine.assemblereceives onlytokenBudget(the full model context window) and has no signal for how many of those tokens are already consumed bymessages + systemPrompt + prompt. Engines that prepend asystemPromptAdditionhave no way to size it against actual remaining headroom.preemptive-compaction(which has full visibility) runs before assemble, so it does not catch overflow caused by the engine's injection — the next LLM call simply fails with a context-limit error.currentTokenCount?: numberfield toContextEngine.assembleparams, plumbed it throughassembleHarnessContextEngine, and wired computation at every production caller: the PI runner main assemble (runEmbeddedAttempt), the PI runner tool-loop hook (installContextEngineLoopHookvia thegetRuntimeContextcallback), and the Codex harness (extensions/codex/src/app-server/run-attempt.ts).estimatePrePromptTokensis re-exported fromopenclaw/plugin-sdk/agent-harness-runtimeso extension harnesses use the same estimator the PI runner uses — defining the semantics once. Docs updated.tokenBudget.preemptive-compactionlogic. No public type was renamed or removed. No new dependency was added.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
systemPromptAdditionagainst the model's remaining headroom becauseassemblereceives onlytokenBudget, notcurrentTokenCount. After this change, engines can computeremaining = tokenBudget - currentTokenCount - <reserve>and skip injection when the curated content would not fit.pnpm exec vitest runon the touched test files. Not yet tested against a live model on a long session — verification so far is at the unit/integration-test layer. A follow-up real-session repro on a Gemini 1M-context channel where pre-this-PR overflow was observed is planned before merge.runEmbeddedAttemptcomputespreassemblyCurrentTokenCountviaestimatePrePromptTokensfrom{ activeSession.messages, systemPromptText, params.prompt }and passes it throughassembleAttemptContextEngine.installContextEngineLoopHookreadscurrentTokenCountfrom the existinggetRuntimeContext()callback. The production install site atattempt.ts:1996now computes the estimate from the loop messages andsystemPromptText(with emptyprompt— the continuation is driven by tool results already inmessages), so engines see a real value on this path too.extensions/codex/src/app-server/run-attempt.ts:624computes the same estimate from{ historyMessages, developerInstructions, params.prompt }and passes it throughassembleHarnessContextEngine.Root Cause (if applicable)
N/A — this is a capability addition, not a bug fix in OpenClaw. The existing behavior (passing only
tokenBudget) is by design.systemPromptAdditiondiscovered they had no way to size injection safely. TheContextEngineRuntimeContexttype already definedcurrentTokenCountfor other lifecycle hooks (afterTurn,maintain,compact) —assemblewas the only one that didn't receive it.Regression Test Plan (if applicable)
src/agents/harness/context-engine-lifecycle.test.ts— wrapper pass-through (present + absent)src/agents/pi-embedded-runner/tool-result-context-guard.test.ts— tool-loop hook propagation (from runtimeContext, missing-from-context, no-callback)src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-injection.test.ts— attempt-level alias plumbs the fieldsrc/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts— captures loop-hook params and assertsgetRuntimeContext()returns a positivecurrentTokenCountin productionextensions/codex/src/app-server/run-attempt.context-engine.test.ts— Codex harness path now asserts a positivecurrentTokenCountreaches the enginecurrentTokenCountreachesengine.assemblefrom every production harness path; field is omitted (not undefined-valued) when no estimate is available, so engines can detect older runtimes.preemptive-compaction.test.tsalready covers the underlyingestimatePrePromptTokenshelper that produces the value.User-visible / Behavior Changes
None for OpenClaw end users. Plugin authors implementing
ContextEngineget a new optionalcurrentTokenCount?: numberparam onassemble. Plugins that do not read it see unchanged behavior. Extension authors gain a new re-exportestimatePrePromptTokensonopenclaw/plugin-sdk/agent-harness-runtimefor use when wiring custom harnesses.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: N/A.Repro + Verification
Environment
ContextEngine)tsconfig.json+ workspacepnpm-workspace.yamlSteps
git checkout feat/context-engine-current-token-countpnpm installpnpm exec vitest run src/agents/harness/context-engine-lifecycle.test.ts src/agents/pi-embedded-runner/tool-result-context-guard.test.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-injection.test.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts extensions/codex/src/app-server/run-attempt.context-engine.test.tsExpected
git diff --stat origin/main..HEADshows only the touched source, test, and docs files.Actual
Evidence
estimatePrePromptTokensis now invoked at each main assemble call site (PI main, Codex) and once per tool-loop iteration (PI loop hook). The helper already runs at the preemptive-compaction precheck on the same turn (PI main path), so the marginal cost there is one extra encode ofmessages + systemPrompt + prompt. Loop-hook iterations add one encode per iteration. Not measured separately.Human Verification (required)
assembleHarnessContextEnginepassescurrentTokenCountto the engine when supplied.assembleHarnessContextEngineomits the field when caller does not supply it (so engines can detect older runtimes viaparams.currentTokenCount === undefined).installContextEngineLoopHookreadscurrentTokenCountfromgetRuntimeContext()when the callback returns it; omits otherwise; omits when no callback is wired.attempt.ts:1996computescurrentTokenCountand feeds it into thegetRuntimeContextruntime context (asserted by capturing the callback inattempt.spawn-workspace.context-engine.test.ts).assembleAttemptContextEnginere-export still plumbs the field.extensions/codex/src/app-server/run-attempt.ts:624computes a positivecurrentTokenCountand passes it through (asserted inrun-attempt.context-engine.test.ts).getRuntimeContextcallback in the loop hook → field omitted.getRuntimeContextreturns an object withoutcurrentTokenCount→ field omitted.currentTokenCount→ unchanged behavior.estimatePrePromptTokenscalls under stress (large transcripts, fast tool loops).Review Conversations
Compatibility / Migration
params.currentTokenCountand gate behavior on whether it is present.Risks and Mitigations
estimatePrePromptTokenscall at each main assemble call site adds one extra token-estimation pass per turn, plus one per tool-loop iteration on the PI loop-hook path.undefinedor0when not supplied (see the...(typeof x === "number" ? { x } : {})spread idiom in the wrappers). Engines that follow the documented contract (params.currentTokenCount === undefined⇒ no headroom info available) behave correctly.estimatePrePromptTokensfromopenclaw/plugin-sdk/agent-harness-runtimewidens the public extension API by one function and commits us to maintaining its signature.