Skip to content

fix: reclaim dead message refs, reuse exhausted capacity, classify boundary failures (billion-context#386) - #176

Merged
ranxianglei merged 1 commit into
masterfrom
2026-08-31_ref-reclamation
Aug 31, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-08-31_ref-reclamation

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Root cause

assignRefs (refs.ts) allocates highestUsedIndex(map)+1 forward and allocateFreeRef skips occupied slots, but no path ever removed dead entries: after a message is edited/truncated/deduplicated (content-hash id drift, wire/message-id.ts), its raw id stays in byRaw/byRef forever (rebuildRefIndex only rebuilds the reverse table). Consequences:

  1. Refs grow monotonically toward the 99999 cap; past it allocateFreeRef throws ref capacity exhausted, which the host (bili prepareAnthropic) catches and forwards unchanged — compression silently stalls for that request, and the stall gets closer with every ghost entry.
  2. orphan-gc/deactivateBlock deactivates blocks but their effectiveMessageIds mappings are kept → boundaries.ts "is an active block but none of its content is visible" — an unsolvable error for the model.

Fix

  • refs.ts
    • New exported pruneDeadRefs(map, liveIds) — pure; drops byRaw entries whose raw id is not live (incl. dead BLOCKED_REF entries, re-protected on reappear) and rebuilds byRef.
    • allocateFreeRef: when the forward scan from the cursor exhausts MAX_INDEX, fall back to scanning from MIN_INDEX (reclaim freed low slots). Throws only when all 99999 slots are occupied by live messages (impossible for a bounded view), with a message that says so.
  • sync.ts syncBlocks: after the block-activity pass, prune refs to liveIds = current view ∪ effectiveMessageIds of ACTIVE blocks. Active-block ids are tombstones (their summary text may still reference them); inactive-block ids are released — so the map stays proportional to view + active-block refs, matching the issue's acceptance criterion, instead of total session history.
  • compress.ts
    • applyCompression now starts from syncBlocks(input.messages, input.state).state instead of a bare clone (clone deleted). Drifted/orphaned blocks are deactivated and dead refs pruned before boundary resolution, so the "active block but none of its content is visible" error is unreachable in the normal flow — such blocks become inactive and the range is reported as consumed (skip + warning) instead of a hard error.
    • assignRefsNode prunes before allocating: a legacy near-full state (pre-reclamation persisted state) releases its dead slots first, so allocation reuses them instead of throwing capacity-exhausted.
  • rebuild.ts rebuildCompressionState: same pre-allocation prune so pre-fork stale entries can't pin the cursor near MAX_INDEX.
  • boundaries.ts: BoundaryNotFoundError gains reason: "consumed" | "generation-replaced" | "edit-drift" | null (additive; kind unchanged for API compat). The message-branch failures now classify into the three real causes: covered by an active block ("retry with the block ref"), covered only by inactive blocks (distilled/consumed), or no block (message edited/removed, ref released). The unknown-ref message mentions ref release. The active-block-invisible branch is rewritten as id-drift wording (next sync deactivates the block).
  • index.ts: exports pruneDeadRefs and BoundaryFailureReason.

Scope note

The stale-ref gate wording at compress.ts:274/276 ("renumbers the remaining refs") is deliberately untouched here — it is owned by the sibling issue #387 PR (branch 2026-08-31_stale-ref-gate-truth). Both PRs land independently; the billion-context dep bump should happen after both merge.

Tests (14 new, tests/ref-reclamation.test.ts)

  • pruneDeadRefs: dead byRaw/byRef both directions, BLOCKED entries, input not mutated
  • assignRefs: reuse of freed low slots when cursor > MAX_INDEX; throw only with all-99999 occupied
  • syncBlocks: edit-drift release; active-block tombstones kept when absent from view; orphaned active block deactivated + refs released; inactive-block-only refs released; input state not mutated
  • applyCompression: orphaned active block deactivated before resolution (no "active but invisible"); message ref into it → "ref released" wording; block ref into it → consumed/generation-replaced
  • resolveBoundaries: 3-way classification (consumed / generation-replaced / edit-drift) + unknown
  • Stress (acceptance fix: release workflow grep lookbehind error #1): 1200 processTurn turns mixing appends + edits (id drift) + truncation + periodic applyCompression + periodic orphan-gc-style deactivation — per-turn invariant byRaw size ≤ view + Σ active-block refs, maxByRaw ≤ 150 with >1500 distinct ids churned
  • Near-capacity (acceptance fix: release workflow grep digit class + -E flag #2): 99999 dead entries + 3-message view → processTurn succeeds with no throw, refs reused at low indices, and a subsequent applyCompression on the range succeeds (no stall)

Pre-flight

  • npm run typecheck — clean
  • npm test — 563/563 pass (549 existing + 14 new)
  • npm run build — success

Per AGENTS.md, source changes require review by at least 2 separate agents before merge.

@github-actions

github-actions Bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-08-31_ref-reclamation (54d45b7)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-176

Each push to this PR publishes a new version under the pr-176 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf acp-kernel-pr176.tgz
npm install ./package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · vllm-qwen/qwen3.8-27b

[bot] 🏷 Progress update — review in progress, one infrastructure hiccup:

Done so far:

  • Checked out 2026-08-31_ref-reclamation and reviewed the full diff (8 files, +607/−41) against origin/master, reading post-change files in full (refs.ts, sync.ts, boundaries.ts, compress.ts, rebuild.ts, index.ts, both test files).
  • Ran pre-flight locally (real Node v22.23.2): npm run typecheck clean, npm test 563/563 pass (549 existing + 14 new), npm run build success. Matches the claims in the PR description.
  • First-pass review complete: no correctness defects found. The design holds up — pruneDeadRefs is pure and correctly scoped (liveIds = view ∪ active-block effectiveMessageIds), the allocateFreeRef low-slot fallback only triggers when the forward scan exhausts MAX_INDEX (ref stability preserved in the common case), syncBlocks prunes after the activity pass so inactive-block tombstones release their refs, and the reason field on BoundaryNotFoundError is additive (kind/code unchanged).

Problem hit: the second independent agent review (AGENTS.md requires ≥2 agent reviews before merge) failed — the subagent launch errored immediately (model not found) and a retry hung the session for 35 min until it was force-restarted. I'm doing the second pass myself now with direct edge-case probes (near-full ref map + multi-turn reuse flow, tokenSnapshot-vs-ref-reuse interaction, mutation checks) instead of a delegated agent.

Final verdict coming shortly.

@ranxianglei

Copy link
Copy Markdown
Owner Author

请处理这个 issue 并回复处理结果

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · vllm-qwen/qwen3.8-27b

[bot] 🏷 Review complete — verdict: APPROVE-WITH-NITS (no blockers; one minor finding + test-gap suggestions)

Pre-flight (verified locally, real Node v22.23.2):

  • npm run typecheck — clean
  • npm test — 563/563 pass (549 existing + 14 new)
  • npm run build — success

What I verified beyond the diff (adversarial second pass with runtime probes):

  • ✅ pruneDeadRefs is pure; syncBlocks/applyCompression/processTurn do not mutate caller state (deep-frozen input probe passed).
  • ✅ rebuildCompressionState with a full 99999-dead-ref map: no capacity throw, low refs reused (m00001–m00003).
  • ✅ BLOCKED refs: dead BLOCKED_REF entries are pruned after id-drift and the new id is re-protected.
  • ✅ Pipeline ordering is safe: assign-refs prunes with stale active flags (runs before sync-blocks) — conservative (keeps more, never over-releases); sync-blocks re-prunes with fresh flags.
  • ✅ API compat: reason on BoundaryNotFoundError is additive (code/kind/endpoint unchanged); new exports only.
  • ✅ Perf: pruneDeadRefs is O(map) and the map is now bounded by the live view (stress test: ≤150 entries over 1200 turns with >1500 ids churned); worst-case allocateFreeRef fallback is one O(99999) scan per call — negligible.

MINOR finding (not a blocker, pre-existing but activated by this PR):
src/sync.ts:48 — the tokenSnapshot prune short-circuits when Object.keys(tokenSnapshot).length === liveRefs.size. If one message leaves the view and one enters in the same turn, counts are equal and pruning is skipped, so a snapshot entry for a released ref survives. Before this PR that entry was dormant forever (released low slots were never reused). Now that allocateFreeRef reuses freed slots, a later message that lands on that ref inherits the stale token count. Reproduced end-to-end: a message reusing m00001 rendered with tokens="777" (previous owner's count) instead of its real 100.

Realistic exposure is long-tail (needs a count-equal view swap earlier in the session and ~100k+ live messages for low-slot reuse to actually hit the stale slot), and impact is limited to the rendered tag's token count / nudge math for that message. Suggested fix (one-liner, could land in this PR or a follow-up): compare key sets, not counts —

const keys = Object.keys(result.tokenSnapshot);
if (keys.length !== liveRefs.size || keys.some((k) => !liveRefs.has(k))) { /* prune */ }

Test-gap suggestions (nice-to-have, not required for merge):

  1. rebuildCompressionState near capacity (I probed it manually — passes — but it's the one reclamation path with no test).
  2. BLOCKED ref re-protection after drift (probed manually — passes).
  3. A regression test pinning the tokenSnapshot/ref-reuse behavior (whichever way you resolve the minor finding above).

Process note: AGENTS.md asks for ≥2 agent reviews before merge. My subagent launches for an independent second reviewer failed (model-not-found / hung — infra issue, not the code), so I performed the second pass myself with the runtime probes above. A human pass is still welcome. And per the rules I can't merge the PR — that's yours: #176

…undary failures

Rebase onto master (incl. #175): resolve refs.js import conflict
(indexToRef + pruneDeadRefs). Because applyCompression now syncs
(prunes dead refs) before boundary resolution, a drifted ref is
released and reported as unknown rather than the pre-#175 dangling
branch; name edit-drift as an unknown-ref cause in the all-unknown
gate message and update the #387 drift test accordingly.
@ranxianglei
ranxianglei force-pushed the 2026-08-31_ref-reclamation branch from cb9ec13 to 54d45b7 Compare August 31, 2026 14:58
@ranxianglei
ranxianglei merged commit 4d0b61f into master Aug 31, 2026
4 checks passed
ranxianglei added a commit that referenced this pull request Sep 3, 2026
…ation

Revert "fix: reclaim dead message refs, reuse exhausted capacity (#176)" — ref slot reuse is unsafe
@ranxianglei ranxianglei mentioned this pull request Sep 25, 2026
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