Skip to content

fix(compaction): bound stale transcript usage#81916

Closed
jbetala7 wants to merge 3 commits into
openclaw:mainfrom
jbetala7:fix/81178-bound-stale-transcript-usage
Closed

fix(compaction): bound stale transcript usage#81916
jbetala7 wants to merge 3 commits into
openclaw:mainfrom
jbetala7:fix/81178-bound-stale-transcript-usage

Conversation

@jbetala7

@jbetala7 jbetala7 commented May 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #81178

Summary

  • bound stale raw transcript prompt-usage snapshots against the recent active replay estimate
  • keep post-usage tail pressure additive for fresh usage records so interrupted tool output after the latest usage record remains conservative
  • when a post-usage compaction marker proves the latest usage snapshot is stale, count only the post-marker tail bytes instead of stale pre-compaction tail bytes
  • drop marker-proven stale output pressure even when the stale prompt usage is moderate and does not trip the prompt-disparity heuristic
  • add preflight compaction regressions for both giant stale usage and moderate stale prompt usage with large stale output/pre-marker tail

Real behavior proof

Behavior addressed: stale pre-compaction transcript usage, stale pre-marker tail bytes, and stale output tokens should not force another preflight compaction when a post-usage compaction marker proves that the active post-compaction replay is small and no explicit transcript-byte policy is exceeded.

Real environment tested: local OpenClaw source runtime from PR head 74ba2e34ec on macOS, using a real temporary JSONL session transcript and the actual runPreflightCompactionIfNeeded path. The compact dependency was instrumented only to count whether the runtime attempted compaction.

Exact steps or command run after this patch: from the PR checkout at 74ba2e34ec, ran a node --import tsx source-runtime proof that wrote a transcript with stale assistant usage input=9000, stale assistant usage output=80000, a 450000-byte stale pre-marker tool result, a compaction marker, and a small post-compaction replay; then it invoked runPreflightCompactionIfNeeded with a 100000-token context window.

Evidence after fix: copied terminal output from the direct source-runtime proof:

{
  "head": "74ba2e34ec",
  "behavior": "post-usage compaction marker drops stale output and pre-marker tail pressure",
  "staleUsageInputTokens": 9000,
  "staleOutputTokens": 80000,
  "preMarkerTailBytes": 450000,
  "contextWindowTokens": 100000,
  "compactCalls": 0,
  "phaseCalls": [],
  "returnedOriginalEntry": true
}

Observed result after fix: the source-runtime preflight call returned the original session entry, never entered the preflight_compacting phase, and made zero compaction calls despite stale output plus stale pre-marker tail pressure that would otherwise exceed the configured 100k context window.

What was not tested: I did not run a live Discord/Pi provider session because the failure is in the local preflight estimator and the source-runtime proof exercises that estimator path directly.

Verification

  • node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts (25 passed)
  • node_modules/.bin/oxfmt --check --threads=1 src/auto-reply/reply/agent-runner-memory.ts src/auto-reply/reply/agent-runner-memory.test.ts
  • git diff --check
  • git diff --cached --check
  • git merge-tree --write-tree HEAD origin/main
  • node --import tsx -e ... source-runtime proof copied above

Re-verified at exact branch head 74ba2e34ec on 2026-06-03 (the earlier proof referenced b90c31584d, the head before the Dependency Guard rebase; the patch is unchanged — two commits on top of current main ed4c4afc0f).

Before fix (origin/main estimator, same standalone source-runtime harness and transcript): runPreflightCompactionIfNeeded throws Preflight compaction required but failed: not_compacted — i.e. main forces preflight compaction for the stale-after-compaction transcript (the bug).

After fix (PR head 74ba2e34ec), both regression scenarios via the real source-runtime path (node --import tsx, compaction action instrumented only to count attempts):

{ "head": "74ba2e34ec", "behavior": "post-usage compaction marker drops stale output and pre-marker tail pressure",
  "staleUsageInputTokens": 9000, "staleOutputTokens": 80000, "preMarkerTailBytes": 450000,
  "contextWindowTokens": 100000, "compactCalls": 0, "phaseCalls": [], "returnedOriginalEntry": true }
{ "head": "74ba2e34ec", "behavior": "giant stale pre-compaction usage dropped after compaction marker",
  "staleUsageInputTokens": 240000, "staleOutputTokens": 120000, "preMarkerTailBytes": 450000,
  "contextWindowTokens": 100000, "compactCalls": 0, "phaseCalls": [], "returnedOriginalEntry": true }
PROOF_RESULT: PASS

Focused suite at this head: node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts → 42 passed.


Update 2026-06-03 — addressed ClawSweeper P1 (head 26d1293708)

ClawSweeper re-review (proof judged diamond lobster) raised a P1 patch-quality
risk: transcriptLineHasPostUsageCompactionMarker also matched the post-compaction
refresh phrases ([Post-compaction context refresh], Session was just compacted.)
in free message text, so ordinary user/tool content echoing them could masquerade
as a marker and wrongly drop stale-usage pressure.

Fix: detect only structured compaction records (type/payload.type ===
"compaction" | "session.compacted" — the records the runtime actually persists
via transcript-file-state.ts / session-manager.ts). Dropped the free-text
fallback and the now-unused collectTranscriptText helper (net -? prod LOC).
The refresh phrases come from post-compaction-context.ts and are prompt-injected
context, not persisted transcript markers, so structured detection fully covers the
real signal.

Real behavior proof at head 26d1293708 (node --import tsx, real
runPreflightCompactionIfNeeded, compaction instrumented to count attempts):

# structured {type:"compaction"} markers still suppress compaction (feature preserved)
moderate stale (in=9000 out=80000, 450k tail): compactCalls=0, returnedOriginalEntry=true
giant   stale (in=240000 out=120000, 450k tail): compactCalls=0, returnedOriginalEntry=true
PROOF_RESULT: PASS

# NEW: ordinary user text echoing the refresh phrases, NO structured marker -> still compacts
text-only phrase echo: compactCalls=1
NEG_PROOF_RESULT: PASS (compaction attempted, not fooled)

Focused suite: node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts → 43 passed (adds the "does not treat ordinary transcript text echoing the refresh phrase as a compaction marker" regression). oxfmt --check clean.

@clawsweeper

clawsweeper Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed June 30, 2026, 9:18 PM ET / July 1, 2026, 01:18 UTC.

Summary
The PR changes preflight transcript usage estimation to treat structured post-usage compaction records as stale-usage boundaries and adds focused regression tests.

PR surface: Source +103, Tests +210. Total +313 across 2 files.

Reproducibility: yes. Source inspection shows current main still lets stale transcript usage plus trailing bytes drive the preflight gate after compaction, and the PR-specific blocker is source-reproducible from the marker byte slice versus the model-visible compaction summary path.

Review metrics: 1 noteworthy metric.

  • Persistent Transcript Interpretation: 1 estimator path changed, 0 config/schema surfaces added. Existing JSONL compaction records now affect preflight token-pressure accounting, so upgrade behavior matters before merge.

Stored data model
Persistent data-model change detected: serialized state: src/auto-reply/reply/agent-runner-memory.test.ts, serialized state: src/auto-reply/reply/agent-runner-memory.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #81178
Summary: This PR is the focused candidate fix for the open stale transcript-usage compaction issue; merged adjacent PRs cover other preflight token-accounting legs rather than superseding this marker-aware estimator change.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦞 diamond lobster
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

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

Rank-up moves:

  • [P2] Count the structured compaction summary in the marker-aware estimate and add a large-summary regression.
  • Rerun node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts.
  • Refresh the source-runtime proof at the repaired head, including stale-marker and large-summary cases.

Risk before merge

  • [P1] Merging as-is can undercount active post-compaction context because the persisted compaction summary is replayed to the model but excluded from the marker-aware token estimate.
  • [P1] The change reinterprets existing JSONL compaction records for preflight token pressure, so upgrade behavior around existing transcripts needs focused proof before landing.

Maintainer options:

  1. Count Summary Before Merge (recommended)
    Adjust the marker-aware estimator to include the structured compaction record's summary text or marker-line bytes and add a focused long-summary regression.
  2. Accept Session-State Risk
    Maintainers may intentionally land the current estimator only if they judge summary undercounting impossible or acceptable for the affected runtimes.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Fix the marker-aware transcript estimator so active structured compaction summaries are included in the post-marker token estimate, add a regression where a large compaction summary pushes the active context over budget, then rerun `node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts` and refresh the source-runtime proof.

Next step before merge

  • [P2] A narrow automated repair can include the structured compaction summary in the estimator, add the missing regression, and refresh proof on the PR branch.

Security
Cleared: The diff changes local transcript-estimation logic and tests; it does not touch dependencies, workflows, secrets, package metadata, or code-execution surfaces.

Review findings

  • [P1] Count the compaction summary in post-marker pressure — src/auto-reply/reply/agent-runner-memory.ts:611
Review details

Best possible solution:

Keep the structured-marker approach, but include the structured compaction summary in the active post-marker estimate and add a large-summary regression before merge.

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

Yes. Source inspection shows current main still lets stale transcript usage plus trailing bytes drive the preflight gate after compaction, and the PR-specific blocker is source-reproducible from the marker byte slice versus the model-visible compaction summary path.

Is this the best way to solve the issue?

No. The structured marker boundary is the right layer, but this implementation is not the best fix until it counts the active compaction summary instead of slicing it out of the post-marker estimate.

Full review comments:

  • [P1] Count the compaction summary in post-marker pressure — src/auto-reply/reply/agent-runner-memory.ts:611
    The new post-marker byte calculation uses trailingLines.slice(markerIndex + 1), which drops the structured compaction record itself. The current harness replays that record's summary into model-visible context, while the estimator's recent-message reader only sees a lightweight divider, so a large summary can be omitted from the preflight estimate and let the next request cross budget.
    Confidence: 0.9

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

Label changes

Label justifications:

  • P2: This is a normal-priority regression fix for repeated preflight compactions with limited blast radius around session continuity.
  • merge-risk: 🚨 session-state: The PR changes token-pressure accounting that decides whether existing sessions compact before the next agent run.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes copied terminal output from real source-runtime invocations at PR head for stale-marker behavior and a no-marker phrase-echo negative case, though it does not cover the large-summary blocker.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output from real source-runtime invocations at PR head for stale-marker behavior and a no-marker phrase-echo negative case, though it does not cover the large-summary blocker.
Evidence reviewed

PR surface:

Source +103, Tests +210. Total +313 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 105 2 +103
Tests 1 210 0 +210
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 315 2 +313

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts.
  • [P1] git diff --check.
  • [P1] Refresh the node --import tsx source-runtime proof at the repaired PR head, including stale-marker, phrase-echo, and large-summary cases.

What I checked:

Likely related people:

  • jared596: Authored the merged PR that added preflight compaction from transcript estimates when usage is stale. (role: introduced related behavior; confidence: high; commits: c6d8318d07f5; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/memory-flush.ts)
  • jalehman: Merged the original stale-usage transcript-estimate PR and authored narrowing commits in that PR series. (role: reviewer and merger of related feature; confidence: medium; commits: c6d8318d07f5, 307f41c986db, 3d84de1a6f6e; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/memory-flush.ts)
  • yetval: Authored recent merged fixes in the same preflight compaction gate for fresh-token projection and stale persisted totalTokens. (role: recent adjacent contributor; confidence: high; commits: caab3434612c, bc4b1b018a3e; files: src/auto-reply/reply/agent-runner-memory.ts, src/auto-reply/reply/agent-runner-memory.preflight-stale-tokens.test.ts)
  • steipete: Recent history includes transcript snapshot consolidation in the same helper file adjacent to the estimator behavior. (role: recent area contributor; confidence: medium; commits: 9e727893ff82; files: src/auto-reply/reply/agent-runner-memory.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.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 14, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 14, 2026
@stielemans

This comment was marked as low quality.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: supplied External PR includes structured after-fix real behavior proof. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 16, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 16, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. labels May 17, 2026
@jbetala7
jbetala7 force-pushed the fix/81178-bound-stale-transcript-usage branch from e7c39ee to 3fa8b5d Compare May 17, 2026 16:12
@openclaw-barnacle openclaw-barnacle Bot removed the cli CLI command changes label May 17, 2026
@jbetala7
jbetala7 force-pushed the fix/81178-bound-stale-transcript-usage branch from 460f423 to 74ba2e3 Compare June 1, 2026 08:10
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 1, 2026
@jbetala7

jbetala7 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

The Real behavior proof and auto-response checks were from the 06-01 Dependency Guard rebase run (auto-response shows A task was canceled), and the prior proof referenced the pre-rebase head b90c31584d. I re-ran the real source-runtime proof at the exact current head 74ba2e34ec and updated the PR body.

Real behavior proof (real runPreflightCompactionIfNeeded source path via node --import tsx; only the terminal compaction action is instrumented to count attempts):

  • Before (origin/main estimator): same harness + transcript -> runPreflightCompactionIfNeeded throws Preflight compaction required but failed: not_compacted (main forces preflight compaction on the stale-after-compaction transcript — the bug).
  • After (head 74ba2e34ec): both scenarios return the original session entry, never enter preflight_compacting, compactCalls: 0:
    • moderate stale usage input=9000 output=80000, 450000-byte pre-marker tail, 100k window -> compactCalls: 0, returnedOriginalEntry: true
    • giant stale usage input=240000 output=120000, same tail/window -> compactCalls: 0, returnedOriginalEntry: true

Focused suite at this head: node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner-memory.test.ts -> 42 passed.

@clawsweeper

clawsweeper Bot commented Jun 3, 2026

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:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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. labels Jun 3, 2026
ClawSweeper re-review flagged a P1: transcriptLineHasPostUsageCompactionMarker
also matched the post-compaction refresh phrases ("[Post-compaction context
refresh]", "Session was just compacted.") in free message text. Those phrases
are prompt-injected context, not persisted markers, so ordinary user/tool
content echoing them could masquerade as a compaction marker and wrongly drop
stale-usage pressure.

Detect only structured compaction records (type "compaction"/"session.compacted",
the records the runtime actually writes via transcript-file-state / session-manager)
and drop the free-text fallback plus the now-unused collectTranscriptText helper.
Add a regression test proving ordinary transcript text echoing the refresh phrase
(with no structured marker) keeps preflight compaction conservative.
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 3, 2026
@jbetala7

jbetala7 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the P1 from the last re-review at head 26d1293708.

transcriptLineHasPostUsageCompactionMarker now keys only on structured compaction records (type/payload.type === compaction | session.compacted). I removed the free-text phrase fallback (and the now-unused collectTranscriptText helper) because [Post-compaction context refresh] / Session was just compacted. originate from post-compaction-context.ts as prompt-injected context, not persisted transcript markers — so ordinary user/tool content echoing them can no longer masquerade as a marker.

Proof (real runPreflightCompactionIfNeeded, compaction counted):

  • structured {type:"compaction"} markers still suppress compaction: moderate + giant stale cases both compactCalls=0, returns original entry (feature preserved).
  • new regression — ordinary user text echoing both phrases with no structured record: compactCalls=1 (still compacts, not fooled).

Focused suite now 43 passed (adds the phrase-echo regression ClawSweeper requested). @clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 23, 2026
@steipete

Copy link
Copy Markdown
Contributor

Thanks for the careful estimator work and follow-up tests. Closing because the affected JSONL tail-scan path is no longer the canonical session and transcript architecture on current main.

#98236 moved sessions and transcripts to per-agent SQLite, canonical entries carry SQLite markers, and preflight usage estimation now skips this file-tail scanner for those sessions. #81178 was closed on the same basis. This replacement is on current main rather than a release tag yet; if the problem reproduces after the SQLite change, please open a fresh issue with current-main or next-release logs showing the SQLite session marker.

@steipete steipete closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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: sufficient ClawSweeper judged the real behavior proof convincing. 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: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: repeated early preflight compactions after compaction due to stale transcript usage

3 participants