Skip to content

feat(compaction): support percentage strings for token thresholds#87932

Open
tanshanshan wants to merge 5 commits into
openclaw:mainfrom
tanshanshan:fix/compaction-percentage-thresholds-87136
Open

feat(compaction): support percentage strings for token thresholds#87932
tanshanshan wants to merge 5 commits into
openclaw:mainfrom
tanshanshan:fix/compaction-percentage-thresholds-87136

Conversation

@tanshanshan

@tanshanshan tanshanshan commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Four compaction parameters (reserveTokens, keepRecentTokens, reserveTokensFloor, softThresholdTokens) use absolute token counts that don't scale with the model's context window. Switching from a 1M-token model to a 200K-token model causes softThresholdTokens=400000 to exceed the entire context window, triggering memory flush on every turn.
  • Solution: Allow percentage strings (e.g. "40%") for these four parameters, resolved at runtime against the active model's context window. Int values keep the current behavior.
  • What changed: 5 files — zod-schema.agent-defaults.ts (schema), types.agent-defaults.ts (types), agent-runner-memory.ts (apply resolveTokenThreshold), schema.help.ts (help text), token-threshold.ts (resolver).
  • What did NOT change: No runtime behavior change beyond percentage resolution. No config migration needed. Int values backward compatible.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Motivation

When switching models with different context windows (e.g. DeepSeek 1M → GLM-5.1 200K), absolute token thresholds don't scale. contextThreshold (0.75) and maxHistoryShare (0.8) already use ratios correctly — these four parameters should too. The current workaround (manual JSON edit + gateway restart per model switch) is error-prone and unsustainable.

Real behavior proof (required for external PRs)

Behavior addressed: Percentage strings (e.g. "40%") for compaction token thresholds resolve at runtime against the active model's context window. Int values keep current behavior.

Real environment tested: Linux (x86_64), Node.js 22.22, OpenClaw local checkout on fix/compaction-percentage-thresholds-87136 branch.

Exact steps or command run after this patch:

node --import tsx scripts/compaction-percentage-proof.mjs

Evidence after fix:

=== Compaction Percentage Token Threshold Proof ===

1. Percentage string resolution against context window
✓ PASS: "40%" with 1M context → 400000
✓ PASS: "40%" with 200K context → 80000 (correctly scaled)
✓ PASS: "10%" with 128K context → 12800
✓ PASS: "75%" with 256K context → 192000

2. Numeric pass-through (backward compatible)
✓ PASS: 400000 int with 1M context → 400000 (unchanged)
✓ PASS: 80000 int with 200K context → 80000 (unchanged)
✓ PASS: 0 int → 0 (edge case)

3. Undefined falls back to default
✓ PASS: undefined with default 4000 → 4000
✓ PASS: undefined with default 20000 → 20000

4. Invalid strings fall back to default
✓ PASS: invalid string 'not-a-percent' → default 4000
✓ PASS: invalid string '40.5%' (decimal) → default 4000
✓ PASS: invalid string 'abc' → default 4000
✓ PASS: empty string → default 4000

5. Model-switch scaling scenario
✓ PASS: DeepSeek 1M + "40%" → 400000 (40% of 1M)
✓ PASS: GLM 200K + "40%" → 80000 (40% of 200K)
✓ PASS: DeepSeek threshold = 5x GLM threshold for same percentage

=== All checks complete ===

Observed result after fix:

  • "40%" with 1M context window → 400000 tokens ✓
  • "40%" with 200K context window → 80000 tokens ✓
  • Integer 400000 passes through unchanged ✓
  • undefined falls back to default (4000 / 20000) ✓
  • Invalid strings ("not-a-percent", "40.5%", "abc", "") fall back to default ✓
  • Same "40%" config scales correctly across model switches (1M vs 200K) ✓

What was not tested: Live compaction with percentage config and model switching on a real gateway. The proof exercises resolveTokenThreshold through real production code, which is the same resolver agent-runner-memory.ts calls during memory preflight/flush planning.

Diagram

Before (broken):
  model: deepseek-v4-pro (1M context)
    softThresholdTokens = 400000 → 40% ✓
  switch to zai/glm-5.1 (200K context)
    softThresholdTokens = 400000 → 200% ✗ (exceeds context window!)

After (fixed):
  model: deepseek-v4-pro (1M context)
    softThresholdTokens = "40%" → 400000 ✓
  switch to zai/glm-5.1 (200K context)
    softThresholdTokens = "40%" → 80000 ✓ (correctly scaled)

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Linux x86_64
  • Runtime/container: Node 22.22
  • Model/provider: N/A
  • Integration/channel: N/A
  • Relevant config (redacted): N/A

Steps

  1. Checkout fix/compaction-percentage-thresholds-87136 branch
  2. Run node --import tsx scripts/compaction-percentage-proof.mjs
  3. Verify all 18 assertions pass

Expected

  • Percentage strings resolve to Math.floor((contextWindow × pct) / 100)
  • Int values pass through unchanged
  • Invalid/undefined values fall back to defaults

Actual

All 18 proof checks pass.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets (proof script output)
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: Percentage resolution, int pass-through, default fallback, invalid input handling, model-switch scaling
  • Edge cases checked: 0%, 75%, empty string, decimal percentage, non-percent string
  • What you did NOT verify: Live compaction with percentage config on real gateway

Compatibility / Migration

  • Backward compatible? Yes (int values unchanged)
  • Config/env changes? Optional (new percentage format)
  • Migration needed? No

Risks and Mitigations

None — additive feature, existing int configs unchanged. Percentage strings are a new opt-in format.

Fixes #87136

@openclaw-barnacle openclaw-barnacle Bot added channel: zalouser Channel integration: zalouser size: S proof: supplied External PR includes structured after-fix real behavior proof. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 9:07 PM ET / 01:07 UTC.

Summary
The PR widens four compaction threshold config fields to accept percentage strings, adds resolveTokenThreshold, wires it into agent-runner-memory, updates help/types/schema, and adds a standalone proof script.

PR surface: Source +43, Other +92. Total +135 across 6 files.

Reproducibility: yes. at source level: PR head accepts percentage strings in the schema, while current settings and memory-core readers still treat these fields as numeric-only. I did not run a live mixed-model gateway compaction scenario in this read-only review.

Review metrics: 1 noteworthy metric.

  • Compaction config value types: 4 widened, 0 added keys, 0 removed. The PR changes public semantics for existing config fields, so reader coverage and upgrade behavior need review before merge.

Stored data model
Persistent data-model change detected: unknown-data-model-change: src/config/schema.help.ts. Confirm migration or upgrade compatibility proof 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, with a competing open API-shape PR and adjacent compaction-threshold reports.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
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:

  • Choose the percent-string API with maintainers or align with the competing share-field PR.
  • Resolve percentage values in all current compaction readers, including agent settings and memory-core flush planning.
  • [P1] Add redacted OpenClaw terminal output or logs showing percentage config loaded and affecting compaction or memory-flush thresholds.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body provides resolver terminal output, but it does not show percentage config loaded by OpenClaw and affecting a real compaction or memory-flush run; contributor should add redacted terminal logs, live output, or a linked artifact, then update the PR body for re-review.

Risk before merge

  • [P1] The PR documents and accepts percentage strings for existing public config fields while current settings and memory-core readers can still ignore or replace those strings with defaults.
  • [P1] The branch competes with an open additive share-field implementation, so merging either PR before maintainer API direction could create public config churn.
  • [P1] The supplied proof is resolver-only terminal output; it does not show OpenClaw loading percentage config and affecting a real compaction or memory-flush threshold.

Maintainer options:

  1. Complete One Config Contract (recommended)
    Choose percent strings or additive share fields, then require the selected API to cover every current compaction reader and public config surface before merge.
  2. Pause For API Direction
    Keep this PR open as design input until maintainers decide whether existing numeric fields should accept strings or new share fields should be added.
  3. Accept Percent Strings Deliberately
    Maintainers can sponsor percent strings, but should require reader coverage, docs/help alignment, tests, and real OpenClaw proof first.

Next step before merge

  • [P1] Human review is needed because the remaining blockers are public config API direction, complete reader coverage, and contributor-side real behavior proof rather than a narrow safe automation repair.

Security
Cleared: The diff adds config/type/runtime helper changes and a local proof script; no concrete security or supply-chain regression was found.

Review findings

  • [P2] Resolve accepted threshold strings in every reader — src/config/zod-schema.agent-defaults.ts:161-169
  • [P2] Preserve memory-flush strings before planning — src/config/zod-schema.agent-defaults.ts:198-200
  • [P2] Keep keepRecentTokens validation positive — src/config/zod-schema.agent-defaults.ts:164-166
Review details

Best possible solution:

Choose one backward-compatible relative-threshold config contract, then update schema, types, help/docs, agent settings, memory-core flush planning, manual compaction behavior, focused tests, and real-run proof together.

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

Yes at source level: PR head accepts percentage strings in the schema, while current settings and memory-core readers still treat these fields as numeric-only. I did not run a live mixed-model gateway compaction scenario in this read-only review.

Is this the best way to solve the issue?

No as submitted: percentage strings are a plausible API, but accepting them at the schema boundary before every consumer resolves them is not the narrowest maintainable fix. The safer path is one maintainer-chosen API with complete reader, docs, tests, and proof coverage.

Full review comments:

  • [P2] Resolve accepted threshold strings in every reader — src/config/zod-schema.agent-defaults.ts:161-169
    This schema now validates percentage strings for reserveTokens, keepRecentTokens, and reserveTokensFloor, but the active settings path still reads those fields through number-only guards in src/agents/agent-settings.ts. A value like reserveTokens: "4%" would look valid and documented while being ignored or replaced by defaults in embedded agent settings, so either resolve strings in all readers or do not accept them here yet.
    Confidence: 0.93
  • [P2] Preserve memory-flush strings before planning — src/config/zod-schema.agent-defaults.ts:198-200
    memoryFlush.softThresholdTokens is accepted as a string here, but extensions/memory-core/src/flush-plan.ts normalizes it with a number-only helper before agent-runner-memory can call resolveTokenThreshold. Configuring "40%" therefore falls back to the default plan value instead of driving the post-turn memory-flush gate.
    Confidence: 0.9
  • [P2] Keep keepRecentTokens validation positive — src/config/zod-schema.agent-defaults.ts:164-166
    Current main rejects keepRecentTokens: 0, and the runtime still treats zero as invalid through toPositiveInt. This union changes the numeric branch to nonnegative, so an invalid keep-recent config would validate and then be silently ignored by the current reader.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 54b09580f61b.

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 widens existing public config value types while current readers still expect numeric values in several paths.
  • 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 🦪 silver shellfish 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 resolver terminal output, but it does not show percentage config loaded by OpenClaw and affecting a real compaction or memory-flush run; contributor should add redacted terminal logs, live output, or a linked artifact, then update the PR body for re-review.
Evidence reviewed

PR surface:

Source +43, Other +92. Total +135 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 5 65 22 +43
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 92 0 +92
Total 6 157 22 +135

What I checked:

  • Repository policy applied: Root policy and the scripts scoped guide were read fully; config/default changes and session compaction behavior are compatibility-sensitive review surfaces. (AGENTS.md:25, 54b09580f61b)
  • Current schema is numeric-only: Current main still accepts reserveTokens, keepRecentTokens, reserveTokensFloor, and memoryFlush.softThresholdTokens as numeric values only. (src/config/zod-schema.agent-defaults.ts:162, 54b09580f61b)
  • Current active settings reader drops strings: applyAgentCompactionSettingsFromConfig reads reserveTokens, keepRecentTokens, and reserveTokensFloor through number-only guards, so PR-accepted strings would not affect the embedded agent settings path. (src/agents/agent-settings.ts:60, 54b09580f61b)
  • Current memory flush planning drops strings: buildMemoryFlushPlan normalizes softThresholdTokens and reserveTokensFloor with a number-only helper before the PR's later resolver would see them. (extensions/memory-core/src/flush-plan.ts:111, 54b09580f61b)
  • PR head widens existing config fields: The PR head changes existing compaction schema entries from number-only to number-or-percent-string unions, including keepRecentTokens changing its numeric branch to nonnegative. (src/config/zod-schema.agent-defaults.ts:161, 9ac0d97a8686)
  • PR resolver is wired only after numeric memory plan normalization: The PR resolves threshold values in agent-runner-memory, but the normal memoryFlushPlan values have already been normalized to numbers by memory-core before this point. (src/auto-reply/reply/agent-runner-memory.ts:738, 9ac0d97a8686)

Likely related people:

  • Takhoffman: PR 21568 exposed reserveTokens, keepRecentTokens, and reserveTokensFloor as public compaction tuning values that this PR changes. (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: PR 65671 added the context-window-aware reserve-floor cap in the same compaction settings path, a sibling invariant for this relative-threshold work. (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, including the threshold behavior affected by softThresholdTokens. (role: feature-history contributor; confidence: high; commits: 7dbb21be8e57; files: src/auto-reply/reply/memory-flush.ts, src/auto-reply/reply/agent-runner-memory.ts, extensions/memory-core/src/flush-plan.ts)
  • vincentkoc: Recent commits on src/agents/agent-settings.ts refactored nearby compaction helpers and warnings in the active settings reader. (role: recent area contributor; confidence: medium; commits: 4637b65470de, 9e3db6bedd28; files: src/agents/agent-settings.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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-21T12:25:41.442Z sha 9ac0d97 :: needs real behavior proof before merge. :: [P2] Resolve percentage values before accepting threshold config | [P2] Preserve softThresholdTokens strings before planning | [P2] Keep keepRecentTokens validation positive

tanshanshan added 2 commits May 29, 2026 15:41
@tanshanshan
feat(compaction): support percentage strings for token thresholds
46be820 
Allow percentage strings (e.g. "40%") for:
- reserveTokens
- keepRecentTokens
- reserveTokensFloor
- softThresholdTokens

Percentages resolve at runtime against the active model's context
window. Integer values keep the current behavior (backward compatible).

Fixes openclaw#87136
@tanshanshan
refactor(compaction): extract resolveTokenThreshold as independent he…
4485379 
…lper

Move the inline percentage-parsing function from agent-runner-memory.ts
into src/config/token-threshold.ts for reuse.

Fixes openclaw#87136
@openclaw-barnacle openclaw-barnacle Bot removed the channel: zalouser Channel integration: zalouser label May 29, 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. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 29, 2026
tanshanshan added 2 commits May 29, 2026 15:49
@tanshanshan
fix(lint): add curly braces and use Number.parseInt
efcd503 
Address oxlint errors: Expected { after if condition, use Number.parseInt instead of global parseInt.
@tanshanshan
fix(compaction): use absolute defaults when threshold is undefined
c00c4d8 
ClawSweeper P1: preserve existing default thresholds. When unset,
return the documented absolute default (e.g. 20000, 4000) instead
of a context-window percentage to avoid changing existing behavior.

Fixes openclaw#87136
@tanshanshan

This comment was marked as low quality.

Sign in to view
@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jun 4, 2026
@tanshanshan

This comment was marked as low quality.

Sign in to view
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

This was referenced Jun 14, 2026
Open
Open
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. 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 15, 2026
This was referenced Jun 18, 2026
Closed
Open
Open
@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: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 19, 2026
This was referenced Jun 21, 2026
Open
Closed
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 20, 2026

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 Jul 20, 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

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. scripts Repository scripts size: S stale Marked as stale due to inactivity 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.

compaction: absolute token thresholds break when switching models with different context windows

1 participant

@tanshanshan