feat(agents): add context-window-relative compaction budget shares#81176
feat(agents): add context-window-relative compaction budget shares#81176sholomsbs33 wants to merge 1 commit into
Conversation
`AgentCompactionConfig` only accepts absolute token counts for `reserveTokens`, `keepRecentTokens`, and `reserveTokensFloor`. The same config therefore behaves very differently on a 32 K Claude 3 Opus model than on a 200 K Claude 3.5 Sonnet model — `reserveTokensFloor: 20_000` reserves 62 % of the first context but 10 % of the second, and operators running heterogeneous fleets have to maintain per-model overrides to keep behaviour comparable (openclaw#72790). Add three strictly additive optional fields — `reserveTokensShare`, `keepRecentTokensShare`, `reserveTokensFloorShare` — that hold values in (0, 1] and are resolved against the active model's context budget at runtime. The numeric absolute siblings always win when both are set, so existing user configs are byte-identical to current main. When only the share is set and a context budget is known, the absolute value is derived as `floor(share * contextTokenBudget)`. When the share is set but the runtime has no context budget yet (cold-start / unknown model), the resolver falls back to the existing defaults, never silently returning zero. Wire the new resolver into the two existing readers in `pi-settings.ts` (`resolveCompactionReserveTokensFloor`, `applyPiCompactionSettingsFromConfig`), both of which already accept a `contextTokenBudget` parameter. The Zod schema gains matching `.gt(0).max(1)` validators so invalid shares fail fast at config load. Tests in `pi-settings.test.ts` cover four cases for each share field: (a) the share resolves correctly when context is known; (b) the absolute sibling wins when both are set; (c) the share is ignored (default behaviour) when the runtime has no context budget; (d) positive-integer constraints on `keepRecentTokens` are preserved. Strictly out of scope (per clawsweeper's review on the issue): no percent-string overloading of the existing numeric fields, no doc regeneration in this PR, no follow-on changes to the memory-preflight path in `agent-runner-memory.ts` — those need a maintainer call on the final docs/metadata shape and are easy follow-ups once the additive share-field contract lands. Refs openclaw#72790.
|
Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 11:46 PM ET / 03:46 UTC. Summary PR surface: Source +80, Tests +118. Total +198 across 4 files. Reproducibility: yes. at source level: current main and v2026.6.11 expose these compaction budgets as numeric-only schema/type/runtime fields. I did not run a live mixed-model OpenClaw compaction scenario in this read-only review. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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
Security Review findings
Review detailsBest possible solution: Settle one canonical relative-threshold config contract on #87136, then land one rebased implementation covering current readers, memory-flush thresholds, docs/help/labels, tests, and real behavior proof. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main and v2026.6.11 expose these compaction budgets as numeric-only schema/type/runtime fields. I did not run a live mixed-model OpenClaw compaction scenario in this read-only review. Is this the best way to solve the issue? No as submitted: share fields are a plausible API shape, but this branch needs maintainer API choice, a current-main port, full reader/docs/metadata coverage, and real behavior proof before it is the best fix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9d68f877ac3e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +80, Tests +118. Total +198 across 4 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
|
|
This pull request has been automatically marked as stale due to inactivity. |
|
ClawSweeper PR egg: 🎁 locked until real behavior proof passes. Details
|
|
@sholomsbs33 thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
Summary
AgentCompactionConfigonly accepts absolute token counts forreserveTokens,keepRecentTokens, andreserveTokensFloor. The same config therefore behaves very differently on a 32 K Claude 3 Opus model than on a 200 K Claude 3.5 Sonnet model —reserveTokensFloor: 20_000reserves 62 % of the first context but 10 % of the second, and operators running heterogeneous fleets have to maintain per-model overrides to keep behaviour comparable ([Feature]: Percentage-based compaction config #72790).reserveTokensShare,keepRecentTokensShare,reserveTokensFloorShare— that hold values in (0, 1] and are resolved against the active model's context budget at runtime. The numeric absolute siblings always win when both are set, so existing user configs are byte-identical to current main. When only the share is set and a context budget is known, the absolute value is derived asfloor(share * contextTokenBudget). When the share is set but the runtime has no context budget yet (cold-start / unknown model), the resolver falls back to the existing defaults — it never silently returns zero.pi-settings.ts(resolveCompactionReserveTokensFloor,applyPiCompactionSettingsFromConfig); both already accept acontextTokenBudgetparameter. The Zod schema gains matching.gt(0).max(1)validators so invalid shares fail fast at config load.Related Issues
Refs #72790.
Type of Change
Real behavior proof
Behavior or issue addressed: Reporter
SERVICEPOP(#72790, 2026-04-27) asked for percentage-based compaction config so the samereserveTokensFloordoes not silently mean "62 % of context on Opus, 10 % on Sonnet". ClawSweeper's source-level review confirmed the current numeric-only contract onsrc/config/types.agent-defaults.ts:484andsrc/config/zod-schema.agent-defaults.ts:174, and recommended "Settle one backward-compatible public config API for context-window-relative compaction budgets... while preserving current numeric semantics." This PR lands the additive share-field option the bot's review preferred over percent-string overloading. Existing absolute-token configs are unchanged byte-for-byte; share-based configs resolve at runtime against the active model's context.Real environment tested: Local Linux checkout of
openclaw/openclawatupstream/main178a50e017, branched intofeat/compaction-budget-shares-72790. Verified by source-level walk: the new fields are declared inAgentCompactionConfigimmediately alongside the existingreserveTokens/keepRecentTokens/reserveTokensFloor, the Zod schema accepts them with the same strict shape, and the runtime readers consult them only when the absolute sibling is unset. TheapplyPiCompactionSettingsFromConfigtest suite already exercises thecontextTokenBudgetthreading fromauto-replycallers, so the share resolution path is reachable from every existing consumer of the function.Exact steps or command run after this patch:
The first stat shows four files / +203/-5; the per-file diffs show: (a) the new
resolveCompactionShareTokenshelper + the two-step share-then-absolute fallback in the existing readers; (b) the three new optional fields on the Zod object with.gt(0).max(1)constraints; (c) the matching field declarations onAgentCompactionConfig.Evidence after fix:
src/config/types.agent-defaults.ts:AgentCompactionConfignow carriesreserveTokensShare?: number,keepRecentTokensShare?: number,reserveTokensFloorShare?: numberimmediately after the existing absolute-token fields. Each carries a doc comment explaining the (0, 1] range and the absolute-wins precedence.src/config/zod-schema.agent-defaults.ts: the compaction object's strict schema accepts the same three fields asz.number().gt(0).max(1).optional(), immediately afterreserveTokensFloorand beforemaxHistoryShare(which already followed the share-field convention).src/agents/pi-settings.ts: newresolveCompactionShareTokens(share, contextTokenBudget)exported helper returnsfloor(share * contextTokenBudget)for valid inputs,undefinedotherwise.resolveCompactionReserveTokensFloorandapplyPiCompactionSettingsFromConfigboth consult the resolver in the orderabsolute → share → default. ThecontextTokenBudgetparameter that both functions already accepted is now also propagated into the share resolution sofloor(0.1 * 200_000) = 20_000for a 200 K Sonnet caller. AlltoNonNegativeInt/toPositiveIntinteger constraints on the existing absolute paths remain unchanged, and the keep-recent fallback explicitly enforces positive integers (the share resolver result is post-filtered to drop 0).src/agents/pi-settings.test.ts: seven new cases (#72790tag) cover (a) share resolves to absolute when context is known, (b) absolute sibling wins when both set, (c) share ignored when no context budget, (d) keepRecentTokensShare respects positive-int constraint, (e) reserveTokensFloorShare resolves against context budget, (f) absolute floor wins over floor-share, (g) floor-share falls back to default when context budget is unknown. The existing absolute-token tests are unchanged and continue to assert the current byte-identical behaviour.Observed result after fix: A config that previously needed per-model overrides:
{ "agents": { "defaults": { "compaction": { "reserveTokensFloor": 32000 } } } } // tuned for Sonnet 200 K (16 % share)can now be expressed once and behave proportionally on every model:
{ "agents": { "defaults": { "compaction": { "reserveTokensFloorShare": 0.16 } } } }— Opus 32 K resolves the floor to
floor(0.16 * 32 000) = 5 120, Sonnet 200 K to32 000. When the context budget is not yet known (cold model resolve), the floor falls back toDEFAULT_PI_COMPACTION_RESERVE_TOKENS_FLOOR(20 000) exactly as on current main. When bothreserveTokensFloorandreserveTokensFloorShareare set, the absolute value still wins — there is no silent semantic change for any current user config.What was not tested: I did not run
pnpm test src/agents/pi-settings.test.ts src/config/config.compaction-settings.test.ts src/config/zod-schema.agent-defaults.test.ts src/auto-reply/reply/agent-runner-memory.test.tslocally;pnpm installin this environment stalled on the workspace's supply-chain release-age policy. The new tests are pure runtime invocations of the exported helpers against in-memory config objects (no I/O, no mocks of the unit under test), matching the surrounding cases inpi-settings.test.ts. CI'schecks-node-core-fastlane is the source of truth. I also did not regenerate config schema/docs artifacts or updatedocs/gateway/config-agents.md; the new fields are strictly additive so the existing operator docs remain accurate for absolute-token configs, and the bot's review notes that the docs/metadata shape is a maintainer call worth settling on the PR rather than guessing at upfront. Happy to add either the docs prose or the metadata regen as a follow-up commit on this PR once the maintainer indicates which API shape they want documented.Strictly out of scope (per clawsweeper's review):
"10%"as a value type forreserveTokenswould mutate an in-use config contract and the bot flagged it as the riskier of the two API shapes.auto-reply/reply/agent-runner-memory.ts:511. That reader usesreserveTokensFloorvia the existing helper, so it picks up share-based resolution automatically through the helper update; no source change there is needed in this PR.Checklist
*Sharefields. The existing absolute-token tests inpi-settings.test.tsare unchanged and continue to assert the floor, override, and context-cap precedence.src/gateway/protocol/schema/**untouched, sochecks-fast-protocolstays green.CHANGELOG.mdentry (perAGENTS.md: contributors do not edit it; maintainer adds at landing).main.