Skip to content

feat(agents): add context-window-relative compaction budget shares#81176

Open
sholomsbs33 wants to merge 1 commit into
openclaw:mainfrom
sholomsbs33:feat/compaction-budget-shares-72790
Open

feat(agents): add context-window-relative compaction budget shares#81176
sholomsbs33 wants to merge 1 commit into
openclaw:mainfrom
sholomsbs33:feat/compaction-budget-shares-72790

Conversation

@sholomsbs33

Copy link
Copy Markdown

Summary

  • 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 ([Feature]: Percentage-based compaction config #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 — it never silently returns zero.
  • Wire the new resolver into the two existing readers in pi-settings.ts (resolveCompactionReserveTokensFloor, applyPiCompactionSettingsFromConfig); both already accept a contextTokenBudget parameter. 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

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Real behavior proof

Behavior or issue addressed: Reporter SERVICEPOP (#72790, 2026-04-27) asked for percentage-based compaction config so the same reserveTokensFloor does not silently mean "62 % of context on Opus, 10 % on Sonnet". ClawSweeper's source-level review confirmed the current numeric-only contract on src/config/types.agent-defaults.ts:484 and src/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/openclaw at upstream/main 178a50e017, branched into feat/compaction-budget-shares-72790. Verified by source-level walk: the new fields are declared in AgentCompactionConfig immediately alongside the existing reserveTokens / 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. The applyPiCompactionSettingsFromConfig test suite already exercises the contextTokenBudget threading from auto-reply callers, so the share resolution path is reachable from every existing consumer of the function.

Exact steps or command run after this patch:

git checkout feat/compaction-budget-shares-72790
git diff upstream/main --stat
git diff upstream/main -- src/agents/pi-settings.ts
git diff upstream/main -- src/config/zod-schema.agent-defaults.ts

The first stat shows four files / +203/-5; the per-file diffs show: (a) the new resolveCompactionShareTokens helper + 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 on AgentCompactionConfig.

Evidence after fix:

  • src/config/types.agent-defaults.ts: AgentCompactionConfig now carries reserveTokensShare?: number, keepRecentTokensShare?: number, reserveTokensFloorShare?: number immediately 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 as z.number().gt(0).max(1).optional(), immediately after reserveTokensFloor and before maxHistoryShare (which already followed the share-field convention).
  • src/agents/pi-settings.ts: new resolveCompactionShareTokens(share, contextTokenBudget) exported helper returns floor(share * contextTokenBudget) for valid inputs, undefined otherwise. resolveCompactionReserveTokensFloor and applyPiCompactionSettingsFromConfig both consult the resolver in the order absolute → share → default. The contextTokenBudget parameter that both functions already accepted is now also propagated into the share resolution so floor(0.1 * 200_000) = 20_000 for a 200 K Sonnet caller. All toNonNegativeInt / toPositiveInt integer 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 (#72790 tag) 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 to 32 000. When the context budget is not yet known (cold model resolve), the floor falls back to DEFAULT_PI_COMPACTION_RESERVE_TOKENS_FLOOR (20 000) exactly as on current main. When both reserveTokensFloor and reserveTokensFloorShare are 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.ts locally; pnpm install in 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 in pi-settings.test.ts. CI's checks-node-core-fast lane is the source of truth. I also did not regenerate config schema/docs artifacts or update docs/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):

  • No percent-string overloading of the existing numeric fields. Adding "10%" as a value type for reserveTokens would mutate an in-use config contract and the bot flagged it as the riskier of the two API shapes.
  • No follow-on changes to the memory-preflight path in auto-reply/reply/agent-runner-memory.ts:511. That reader uses reserveTokensFloor via 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

  • Byte-identical behaviour for configs that do not set any of the new *Share fields. The existing absolute-token tests in pi-settings.test.ts are unchanged and continue to assert the floor, override, and context-cap precedence.
  • Strictly additive Zod schema — three optional fields on the existing strict object, no rename / no widening of existing field types. Configs that omit the new fields validate identically.
  • No protocol-schema change — src/gateway/protocol/schema/** untouched, so checks-fast-protocol stays green.
  • No CHANGELOG.md entry (per AGENTS.md: contributors do not edit it; maintainer adds at landing).
  • Targets main.

`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.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M proof: supplied External PR includes structured after-fix real behavior proof. labels May 12, 2026
@clawsweeper

clawsweeper Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 11:46 PM ET / 03:46 UTC.

Summary
The PR adds optional compaction *Share config fields, schema validation, runtime resolution in pi-settings.ts, and unit tests for share-to-token behavior.

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.

  • Public compaction config keys: 3 added, 0 changed, 0 removed. The PR adds operator-facing config fields, so compatibility, discoverability, and upgrade behavior need maintainer review before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #87136
Summary: This PR is one implementation candidate for the canonical context-window-relative compaction threshold issue; related items are duplicates, competing candidates, or adjacent threshold work.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Rebase and port the implementation from pi-settings.ts onto current src/agents/agent-settings.ts and current callers.
  • Choose the share-field API with maintainers or align with the competing percentage-string API before adding public config surface.
  • [P1] Add redacted OpenClaw terminal output, logs, or a linked artifact showing share-based config loaded and affecting compaction or memory-flush thresholds.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body provides source inspection and diff commands, but no redacted OpenClaw terminal output, logs, or linked artifact showing share-based config loaded and affecting compaction or memory-flush thresholds after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR adds three public config keys before maintainers have chosen between additive share fields and the competing percentage-string contract.
  • [P1] The submitted branch is dirty and wires runtime behavior into retired src/agents/pi-settings.ts instead of current src/agents/agent-settings.ts.
  • [P1] The canonical model-switching problem also includes memoryFlush.softThresholdTokens, which remains absolute in this PR.
  • [P1] The supplied proof is source inspection and diff review, not redacted OpenClaw terminal output, logs, or an artifact showing share-based config loaded and affecting thresholds.

Maintainer options:

  1. Complete One Config Contract (recommended)
    Choose share fields or percentage strings, then require the selected API to cover current runtime readers and public config surfaces before merge.
  2. Pause For API Direction
    Keep this PR open as design input if maintainers are not ready to choose the public compaction config shape.
  3. Accept Share Fields Deliberately
    Maintainers can sponsor the additive share-field API, but should require a rebase, memory-flush/docs/metadata coverage, and real-run proof first.

Next step before merge

  • [P1] Human review is needed because the remaining blockers are public config API direction, branch conflict/rebase, contributor proof, and complete reader/docs coverage rather than a narrow safe automation repair.

Security
Cleared: The diff is limited to config schema/types, compaction setting resolution, and tests; no concrete security or supply-chain regression was found.

Review findings

  • [P2] Port share resolution to the active settings reader — src/agents/pi-settings.ts:115-136
  • [P2] Cover memory-flush thresholds before exposing shares — src/config/zod-schema.agent-defaults.ts:184-186
  • [P2] Document the new public config keys — src/config/zod-schema.agent-defaults.ts:184-186
Review details

Best 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:

  • [P2] Port share resolution to the active settings reader — src/agents/pi-settings.ts:115-136
    This patch wires the new share logic into src/agents/pi-settings.ts, but current main no longer has that runtime file and the live reader is src/agents/agent-settings.ts. As submitted, the branch conflicts and the new runtime behavior would not cover the current compaction settings path.
    Confidence: 0.93
  • [P2] Cover memory-flush thresholds before exposing shares — src/config/zod-schema.agent-defaults.ts:184-186
    The schema adds relative config only for reserve/keep/floor fields, while current memory-flush preflight still subtracts absolute memoryFlush.softThresholdTokens and reserveTokensFloor from the context window. Users switching model windows would still have inconsistent threshold behavior for the canonical problem.
    Confidence: 0.88
  • [P2] Document the new public config keys — src/config/zod-schema.agent-defaults.ts:184-186
    These fields become accepted public config, but the PR does not add matching schema help, labels, or public docs, so config tooling and operators cannot discover the new contract consistently.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9d68f877ac3e.

Label changes

Label justifications:

  • P2: This is a bounded compaction configuration feature with session-behavior impact, not an outage, security emergency, or crash loop.
  • merge-risk: 🚨 compatibility: The PR adds new public config keys while the durable API shape, docs, metadata, and current-reader coverage remain unsettled.
  • merge-risk: 🚨 session-state: Compaction and memory-flush thresholds control how much session context is retained, summarized, or flushed.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body provides source inspection and diff commands, but no redacted OpenClaw terminal output, logs, or linked artifact showing share-based config loaded and affecting compaction or memory-flush thresholds after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +80, Tests +118. Total +198 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 85 5 +80
Tests 1 118 0 +118
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 203 5 +198

What I checked:

Likely related people:

  • Takhoffman: Commit c1ac37a exposed reserveTokens, keepRecentTokens, and reserveTokensFloor across config, schema, and settings paths that this PR extends. (role: introduced public compaction config surface; confidence: high; commits: c1ac37a6410a; files: src/config/types.agent-defaults.ts, src/config/zod-schema.agent-defaults.ts, src/agents/pi-settings.ts)
  • openperf: Commit 4bc46cc added context-window-aware reserve-floor capping, which any relative budget resolver must preserve for small-context models. (role: adjacent runtime contributor; confidence: high; commits: 4bc46ccfedc4; files: src/agents/agent-settings.ts, src/agents/agent-settings.test.ts)
  • steipete: Commit 7dbb21b introduced pre-compaction memory flush and the soft-threshold behavior that overlaps the canonical relative-threshold issue. (role: feature-history contributor; confidence: medium; commits: 7dbb21be8e57; files: src/auto-reply/reply/memory-flush.ts, src/auto-reply/reply/agent-runner-memory.ts, docs/reference/session-management-compaction.md)
  • vincentkoc: Recent refactors touched src/agents/agent-settings.ts, the current active settings reader this PR would need to target after the pi-settings.ts path was retired. (role: recent area contributor; confidence: medium; commits: 4637b65470de, 9e3db6bedd28; files: src/agents/agent-settings.ts, src/agents/agent-settings.test.ts, docs/reference/session-management-compaction.md)
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.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label May 27, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg: 🎁 locked until real behavior proof passes.

Details
  • No creature or rarity is rolled until proof passes.
  • Eggs are collectible flavor only; they do not affect labels, ratings, merge decisions, or automation.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label May 28, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 24, 2026
@clawsweeper

clawsweeper Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant