Skip to content

feat: transient compression holds for provisional messages (for AF #159) - #120

Open
antra-tess wants to merge 2 commits into
mainfrom
feat/compression-hold
Open

antra-tess wants to merge 2 commits into
mainfrom
feat/compression-hold

Conversation

@antra-tess

Copy link
Copy Markdown
Contributor

Unblocks anima-research/agent-framework#159 (tool-result guard), review finding #3: the guard stages a tool_result as a ~20-token placeholder via addMessage and later swaps in the real output via editMessage. Edits never reach derived entries or the strategy, and Autobiographical dedupes by message id, so if the placeholder is summarized while pending, the accepted output never reaches compressed memory. The baseline test in this PR reproduces that.

Design

The smallest correct fix is a transient compression hold:

  • addMessage(..., { holdCompression: true }) places the hold before onNewMessage fires, so the ingress rebuild can't chunk the message.
  • holdCompression(ids), releaseCompression(ids) (idempotent; releasing a sharded add releases every shard), and getCompressionHolds().
  • The strategy view gains an optional live isCompressionHeld(id) predicate. Autobiographical's getRecentWindowStart is clamped to the earliest held message, stepping back one so the tool_use stays paired with its tool_result. The held message, everything after it, and its tool_use therefore stay in the raw protected tail and never enter a chunk. Knowledge inherits this through getCompressibleMessages.
  • A chunk that closed before a late holdCompression call, and that reaches the hold boundary, is not queued or compressed until release. This also covers chunks after the hold whose lead-in would carry the placeholder. It also covers the kv-stable/kv-unified demand path (enqueueL1ForRange → tick).
  • In-memory only. After a restart AF records a still-pending batch as withheld, so its placeholder is the final content and should compress normally. Persisting holds would risk a permanent stall; not persisting them can't cause one.

Edits: I did not add an onMessageEdited hook. The hold ensures nothing derived exists for a held message, so an edit made while held, followed by release, is correct. Invalidating summaries after an edit would mean re-minting and re-merging, which carries much more risk. editMessage is now documented as "edit while held, then release".

Strategies: Autobiographical and Knowledge honor the hold. Passthrough and windowed-passthrough don't compress, so they're unaffected. The folding solvers (kv-stable, kv-unified, flat-profile, oldest-first) only fold existing summaries, and none can cover a held message. Merges are unaffected for the same reason.

Invariants: With no holds the predicate returns false everywhere, so chunking, requests, cache markers and compiled context are unchanged (the existing suite passes untouched). The tool_use/tool_result pairing is kept by the clamp. While a hold is active, the raw tail simply starts earlier; after release it goes back to the token boundary.

Tests

test/compression-hold.test.ts runs a real AutobiographicalStrategy against a recording membrane:

  • baseline: without a hold, the placeholder is summarized and the real output never reaches memory. This reproduces the bug.
  • held on add: the placeholder is never summarized, and neither the held message nor its tool_use enters a summary while held. After edit + release, the accepted output reaches a compression request and the resulting L1 covers both the tool_use and the tool_result.
  • late holdCompression: the same outcome after chunks had already closed.
  • transient: no holds after reopen, and the placeholder compresses normally.
  • cleanup: removing a held message drops its hold; unknown ids are a no-op.

These were red before the fix (type errors first, then 3/6 behavioral failures with a stub API). Result: npm test 868 pass / 0 fail / 0 skipped.

Open questions

  • An unreleased hold stalls compression of everything after it by design, so AF must release on every settle path. Should CM add a safety valve, such as a max hold age or a warning?
  • A hold made with holdCompression() after the fact can't un-chunk a persisted chunk record. It only defers that chunk's compression, which is enough for correctness. Callers should prefer the addMessage option.

🤖 Generated with Claude Code

Adds holdCompression/releaseCompression/getCompressionHolds and an
addMessage `holdCompression` option. Autobiographical/Knowledge stop the
compressible region before the earliest held message (keeping its tool_use
paired) and skip already-closed chunks that reach the hold boundary, so a
placeholder later replaced via editMessage is never summarized and the
final content compresses after release. In-memory only; unused = unchanged.

Needed by anima-research/agent-framework#159 (tool-result guard, review
finding #3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antra-tess

Copy link
Copy Markdown
Contributor Author

Reviewed head 027a9265a1b2c76d9cb74a9b38ca80a698a4fa7d alongside agent-framework PR #159 at 647f08197d405933987a2a556288a69f56d7d6fa. Two findings remain in this PR:

  1. [P2] Exclude held messages from compression head context as well as the target chunk. The hold check in chunkHasHeldMessage only examines the chunk's messages, but compressChunkHierarchical emits the configured head window independently (head emission). After resetHeadWindow, a held tool result inside the new head can therefore enter compression prompts for older chunks that precede the hold boundary. I reproduced this with a real ContextManager and AutobiographicalStrategy: seed 12 old messages, append a tool use, reset the head to that tool use, append its result with { holdCompression: true }, append four later messages, then compile and drain. With headWindowTokens: 200, recentWindowTokens: 0, targetChunkTokens: 50, and l1HoldbackChunks: 0, all 3 compression requests contained the held placeholder, while its hold remained active. This permits provisional content to influence compressed memory despite the hold. Apply the hold boundary to auxiliary prompt context too, preserving tool pairing, and add a regression with a moved head window.

  2. [P2] Avoid a full-history hold scan for every uncompressed chunk. rebuildChunks calls chunkHasHeldMessage per uncompressed record-backed chunk (call site); that helper scans the entire timeline to find the hold boundary (helper). CM installs isCompressionHeld even when the hold set is empty, so the early return does not avoid this work for ordinary callers. Instrumenting the real rebuildChunks with a synthetic store containing no holds produced 4,004,000 predicate checks for 4,000 messages / 1,000 uncompressed chunks, and 16,008,000 for 8,000 / 2,000. Compute the boundary/blocked membership once per rebuild and provide a no-holds fast path, while retaining live hold checks for asynchronous drains.

Validation: the six compression-hold tests pass; npm run typecheck and git diff --check origin/main...HEAD pass. The reproductions above cover gaps outside those tests.

Integration note: AF #159 does not yet acquire or release these holds. Running its current ToolResultGuard against this CM head still summarized the placeholder and never sent the accepted output to compression. That finding is also being posted on AF #159.

— Review by Codex.

Review (Codex) on #120:
- After resetHeadWindow the head window can sit past the hold boundary and
  compressChunkHierarchical/executeMerge/generateTransitionSummary emitted it
  as prompt context, leaking the held placeholder. Head context now ends at
  the hold boundary (stepped back onto the paired tool_use).
- rebuildChunks rescanned the timeline per uncompressed chunk. The boundary is
  computed once per pass (holdBlockedIds), and the view's hasCompressionHolds()
  fast path skips scanning entirely when nothing is held. Live checks remain
  for async drains (tick).

Regressions: reviewer's moved-head repro; scan counter (0 with no holds,
bounded per compile with holds).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant