Skip to content

fix(reply): gate preflight compaction fast-path on token threshold (#63892)#64384

Closed
070freebird070-ctrl wants to merge 1 commit into
openclaw:mainfrom
070freebird070-ctrl:fix/reply-proactive-compaction-63892
Closed

fix(reply): gate preflight compaction fast-path on token threshold (#63892)#64384
070freebird070-ctrl wants to merge 1 commit into
openclaw:mainfrom
070freebird070-ctrl:fix/reply-proactive-compaction-63892

Conversation

@070freebird070-ctrl

@070freebird070-ctrl 070freebird070-ctrl commented Apr 10, 2026

Copy link
Copy Markdown

Summary

Fixes #63892. After the first compaction, `entry.totalTokensFresh` is set to `true` by `incrementRunCompactionCount`. On subsequent runs, `runPreflightCompactionIfNeeded` unconditionally short-circuits at `src/auto-reply/reply/agent-runner-memory.ts:379` because `shouldUseTranscriptFallback` is `false`. The stale-tokens path below (which is what runs the actual threshold check) is never reached, so proactive compaction never re-fires. Overflow-retry recovery still worked because it runs through a separate path in `src/auto-reply/reply/pi-embedded-runner/run.ts` that catches `isLikelyContextOverflowError` directly, bypassing the preflight gate entirely.

  • Problem: Proactive compaction never re-fires after the first checkpoint on long-lived sessions. Users only see compaction when a request actually overflows the context window.
  • Why it matters: On 100k+ sessions (reported against GLM-5.1:cloud, observed against Anthropic + Gemini providers in the issue thread) this turns every long conversation into an eventual overflow-and-retry instead of a smooth proactive checkpoint.
  • What changed: The preflight fast-path early return now also requires `freshPersistedTokens < preflightThreshold`. When fresh tokens have grown back above the threshold, the guard falls through and the existing `shouldRunPreflightCompaction` path runs normally. The duplicate `threshold` local was consolidated onto the new `preflightThreshold` that the guard uses, so the `logVerbose` output is unchanged.
  • What did NOT change (scope boundary): No changes to `shouldRunPreflightCompaction`, `incrementRunCompactionCount`, the overflow-retry path, or the stale-tokens transcript-fallback branch. No config/schema/public SDK surface touched.

Change Type

  • Bug fix

Scope

  • Memory / storage
  • Agents / orchestration (preflight compaction)

Verification

  • `pnpm test src/auto-reply/reply/agent-runner-memory.test.ts` — 5/5 passing
  • `pnpm check` — clean (tsgo, lint, all policy gates)
  • `pnpm build` — clean
  • Regression test covers both directions:
    • `totalTokensFresh: true` + tokens above threshold → `compactEmbeddedPiSession` is called (proactive compaction fires — this is the bug fix).
    • `totalTokensFresh: true` + tokens below threshold → fast-path early return still fires unchanged.

I also verified the equivalent gate on the compiled dist of v2026.4.9 on a long-running gateway against GLM-5.1:cloud (`contextWindowTokens` ≈ 200k, `reserveTokensFloor` 40k, `softThresholdTokens` 25k). Before the patch: zero proactive checkpoints after the first, overflow-retry checkpoints only. After the patch: proactive checkpoints fire on every subsequent cycle as expected.

Credit

The root cause analysis, exact file/line pointers, and the fix direction are all @martingarramon's from the issue thread. My earlier hypothesis about `compactionCount` latching was wrong — thanks for the correction. @mjamiv independently confirmed the repro on v2026.4.9, which is what motivated digging past the first hypothesis.

AI-assisted disclosure

This PR was written with AI assistance (Claude). I (the submitter) read and understand the change, verified the fix against the live compiled runtime before writing the source patch, and wrote the regression tests to directly exercise the `totalTokensFresh === true` re-fire case.

Fixes #63892

@070freebird070-ctrl 070freebird070-ctrl mentioned this pull request Apr 10, 2026
Closed
@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where proactive preflight compaction stops firing after the first compaction checkpoint. Once incrementRunCompactionCount marks entry.totalTokensFresh = true, the old early-return guard at agent-runner-memory.ts:388 would unconditionally short-circuit on every subsequent call, bypassing the token-count check entirely. The fix adds a freshPersistedTokensBelowThreshold predicate so the fast-path early return only fires when fresh tokens are actually still below the compaction threshold — letting grown sessions fall through to shouldRunPreflightCompaction as intended. The overflow-retry path (pi-embedded-runner/run.ts) is unaffected. Two regression tests directly exercise the totalTokensFresh === true re-fire and the unchanged below-threshold skip.

Confidence Score: 5/5

Safe to merge — targeted one-line logic fix with correct tests and no surface-area changes outside the preflight gate.

The fix is logically sound: the guard is coherent with shouldUseTranscriptFallback (when totalTokensFresh !== false and tokens are valid, freshPersistedTokens is guaranteed to be a positive number, so the new predicate behaves correctly in all reachable states). Both directions of the condition are covered by regression tests. No config, schema, or public SDK surface was touched. No remaining P0/P1 findings.

No files require special attention.

Reviews (1): Last reviewed commit: "fix(reply): gate preflight compaction fa..." | Re-trigger Greptile

@prtags

prtags Bot commented Apr 23, 2026

Copy link
Copy Markdown

Related work from PRtags group infinite-dodo-7d3e

Title: Preflight compaction skips fresh token totals

Number Title
#63892 Title unavailable
#64384* fix(reply): gate preflight compaction fast-path on token threshold (#63892)
#65600 Title unavailable
#65622 fix(agents): reevaluate preflight compaction on fresh totals
#66520 Title unavailable
#66716 fix: auto-compaction fires on fresh cached token counts (#66520)

* This PR

This was referenced Apr 23, 2026
Closed
Closed
Closed
Closed
@clawsweeper

clawsweeper Bot commented Apr 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and the useful part of this older PR is already implemented there.

Close this stale, conflicting PR because its central useful fix, letting fresh persisted token totals reach the preflight threshold check instead of returning early forever, is already present on current main and shipped in v2026.5.22. The remaining branch value is obsolete changelog/test churn, while the distinct next-input projection refinement is tracked separately.

So I’m closing this older PR as already covered on main rather than keeping a mostly-duplicated branch open.

Review details

Best possible solution:

Keep the shipped current implementation and close this stale branch; continue any distinct fresh-token next-input projection work in #91488.

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

Yes. The linked issue discussion and earlier source review define the old failure path: a non-heartbeat, non-CLI run with fresh persisted tokens over threshold returned before the threshold helper; current released code no longer has that return.

Is this the best way to solve the issue?

No for this PR as a landing branch. The fix direction was sound, but the central behavior shipped in v2026.5.22, the branch is now conflicting, and the remaining changelog entry is release-owned rather than useful to merge.

Security review:

Security review cleared: No concrete security or supply-chain issue was found; the PR only changes runtime compaction logic, adjacent tests, and a changelog line.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • joshavant: Authored the commit that removed the stale fresh-token early return while changing native compaction failure handling. (role: carried current implementation; confidence: high; commits: b8e9ab9385c0; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-memory.test.ts)
  • steipete: Live file history shows repeated recent work on agent-runner-memory.ts, transcript scanning, memory flush, and adjacent tests after the original bug report. (role: recent area contributor; confidence: high; commits: 82e5dd4da74f, 8d5f6c8ae465, 39bc43cb6068; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-memory.test.ts, src/auto-reply/reply/memory-flush.ts)
  • jared596: Earlier history ties jared596 to the merged stale-usage preflight compaction work that introduced the affected runner/helper surface. (role: adjacent behavior introducer; confidence: medium; commits: c6d8318d07f5; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/memory-flush.ts, src/auto-reply/reply/reply-state.test.ts)
  • amknight: Recent compaction/memory commits touched the same runner and test surface around Codex memory flush and compaction behavior. (role: adjacent area contributor; confidence: medium; commits: d0a74dbfbe4d, bf3b99437816; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-memory.test.ts)

Codex review notes: model internal, reasoning high; reviewed against c78f9376d929; fix evidence: release v2026.5.22, commit b8e9ab9385c0.

@clawsweeper clawsweeper Bot mentioned this pull request May 16, 2026
Closed
@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 19, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle

openclaw-barnacle Bot commented Jun 5, 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 Jun 5, 2026
@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 5, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 6, 2026
@yetval yetval mentioned this pull request Jun 8, 2026
Merged
@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. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 14, 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

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proactive compaction scheduler never re-fires after first checkpoint (compactionCount latched at 1)

1 participant

@070freebird070-ctrl