Repository navigation
fix(compression): a request for history gives it only what came before it, after a head-window reset too (#317) - #322
Conversation
…e it, after a head-window reset too (anima-research#317) compressChunkHierarchical and executeMerge put the head window first, raw, as the summarizer's context ("the raw chronicle opening ... exactly where the original instance saw it"), and keep later events out. After resetHeadWindow the head window starts at the transition, so every request for history from before the reset opened with the new topic, which came after the history being summarized (anima-research#317). Recall pairs were a second route for later history. A compression request recalled every summary on the frontier, so a chunk compressed after later history had been summarized got those later summaries ahead of it. That happens to the old head's chunk, which a reset makes compressible late, and to a pinned or quarantined chunk released late. headAsOf(store, pos) gives the head in force when the message at pos was the newest, cut at pos and at the compression holds. From the live head's start on it is the live head, cut at pos, so a chunk inside a widened live head no longer gets the head's later messages. Before the live head's start it is the head starting at the latest topic transition message at or before pos (isTopicTransitionMessage, the test the restart uses), or at the opening, and it ends by the live head's walk (getHeadWindowEnd's body, now headWindowEndFrom). A chunk that starts at a transition, and the opening's own chunk, get no head. Compression requests and merges use it. So does requestCarriesToolBlocks, which the count of queued work uses to predict tick's tool deferral. It reads each chunk's own head, memoized by pins and head start. A compression request now recalls only summaries that start before its chunk, the rule a merge's prior recall set already used. A summary that starts before the chunk and reaches past it is still recalled, because the messages it covers before the chunk have no other form in the request. That is the one way later content still reaches a request. Coverage is unchanged. Hosts can remove stored messages while a summary over them, or a queued chunk holding them, lives on (agent-framework's Discord module removes them on a delete). startInStore gives the first of a set of messages still in the store. summaryStartInStore gives a summary's first leaf still in the store, found through its children, which is the set its coverage hides. A chunk starts at its first message still in the store, and a merge at the first of its sources' leaves. Both recall sets take summaries by that start, and the compression request's middle places each recall pair there. History with no message left gets no head and recalls nothing. Every compile selects and sends what it did before, and summaries already written stay as they are. An over-budget refusal's clearedBy follows tick's deferral, which reads each request's head. Prompt cache, measured with a stub summarizer at connectome-host's defaults scaled by 1/10: against main, the requests after a reset rewrite fewer tokens in every case measured. Some cases send more, mainly the two with a transition bigger than the head window, where main's head held none or part of it and this sends the opening. The pull request description has the table. test/compression-head-as-of.test.ts has 31 tests, and 27 of them fail at main fa6e0d6. The four that pass there are a queued chunk whose first message is removed during the compression before it, and three cases of a summary whose first leaf was removed or isn't its sourceRange.first. main passes those because it recalls the whole frontier. Five existing tests change. Three in overbudget-diagnostics and one in l1-recall-raw-pins-frontier now pin the new behaviour. One in compression-transient-ladder reached a tool cycle only through the head, and now uses a chunk that holds the cycle, so it checks what it checked before. A comment in tool-prose-hoist changes too. Removing any one of 23 parts of the change fails at least one test. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Automated review
Recommendation: merge and fast-follow. 4 should-fix issues can be fixed after the merge.
Reviewed commit f5c4216 by reading the code, in four stages: a plan by claude-opus-5-5; two readers on every task of it, gpt-6.1-sol (two tasks per call) and claude-haiku-5-5; their findings grouped into issues; and a judge, claude-opus-5-5, that tried to refute each issue. It did not run the code or look at the checks.
Should fix (4)
- Should fix. requestCarriesToolBlocks runs headAsOf per queued chunk before its memo lookup, repeating store copies, back-scans and head walks.
src/strategies/autobiographical.ts:12839 - Should fix. Merge prefix emits all recall pairs before the raw middle, unlike compression, so older raw can follow newer summaries.
src/strategies/autobiographical.ts:11931 - Should fix, test. Memo-key tests never let pins change coverage at a shared head start, so dropping pinned.length from the key goes unnoticed.
src/strategies/autobiographical.ts:12884 - Should fix, test. No capped-selection test exercises removed-first-message L1 ordering, so reverting the pre-cap comparator passes unnoticed.
src/strategies/autobiographical.ts:9236
Minor (10)
10 minor issues
- Minor. Merge hold, candidate, topology and range checks read stored sourceRange endpoints instead of surviving source leaves.
src/strategies/autobiographical.ts:18066Cause: Merge paths read summary positions and extents from stored sourceRange endpoints instead of surviving source leaves; Merge tests with a removed source run through PlacingTopologyStrategy, which bypasses the production topology guard. Consequence: Removing an endpoint lets a merge reaching a compression hold run or persist, and tick may start it; A summary whose first message was removed is excluded as 'can never merge' and stays on the frontier; The topology check quarantines an otherwise valid merge with a removed first message; mergePositionIndex appends a removed member as newest, creating an artificial overlapping span; Range demand and retry-span calculations miss the surviving start; A queued merge is named by compressionWorkQueued as clearing an over-budget refusal though tick rejects it; Tests cannot show merge start or recall behaviour as a real store would produce. Case: Start a merge over adjacent summaries A and B, where B covers b0, b1, b2; while the provider call is outstanding, remove b2 and hold b1 before editing its provisional content. All of A's endpoints and b0 precede the hold, while b2 is absent from holdBlockedIds, so the landing check returns false and writes the merge despite its request having replayed b1. It should discard the result and retain the merge for after the hold is released. The same endpoint-only check is used to decide whether tick may start the merge. Found by both readers (gpt-6.1-sol and claude-haiku-5-5). - Minor. headAsOf falls back to the opening when the live start is removed or reset to null, or an older transition is missing.
changelog.d/317-compression-head-as-of.fixed.md:2Cause: getHeadWindowStartIndex returns 0 when the live head start is removed or reset to null, so pos >= liveStart bypasses historical transitions; headAsOf's backward transition scan misses a topic whose transition was removed or filtered from the store. Consequence: History gets the opening's head or another topic's opening instead of its own head; Compression, merge and tool-reader deferral consume the substituted head; Live and restart heads disagree; reopening restores the last surviving transition; History between an earlier transition and a removed one gets the opening's head; 'In force' and restart-agreement claims do not hold and are not stated as exceptions. Case: Create topic A, reset to transition B, add B history, then reset the strategy to null: getHeadWindowStartIndex is now 0, so every nonnegative historical B position takes the live-head fast path and gets A's opening, not B's former head, even though B's transition remains visible. Removing the live start ID likewise makes liveStart fall back to 0 and bypasses all surviving historical transitions; removing or filtering an older transition instead makes the backward scan miss that topic and choose the preceding transition or 0. Compression and merge both consume these substituted heads, and the tool reader consistently consumes the same substitution—an uncovered tool cycle in A can therefore cause deferral for B history whose actual head contained no cycle. This is not faithful historical reconstruction, and ... - Minor. headAsOf extends a past head using ownership present at request time via anchorHeadToCoverage, not ownership as of pos.
src/strategies/autobiographical.ts:18224Cause: headAsOf extends a past head using ownership present at request time via anchorHeadToCoverage and ownedMessageIds, not ownership as of pos; New bare-store head tests do not exercise a later-ownership state. Consequence: Past head enlarged to a later-owned message (e.g. s+2 to s+4, or stock boundary 4 to 7); Raw messages emitted ahead of older recall pairs; Raw middle emptied for chunks after the stock boundary; Cut at pos does not recover the historical head; Documented 'head in force when pos was newest' claim is false; headAsOf(7) changes over time. Case: Use messages costing 10 tokens each and headWindowTokens=25: when the opening (or an old transition at s) was live without following chunk coverage, its end was s+2. After a reset, leave s+2 and s+3 unowned, for example because they are pinned, and create coverage beginning at s+4. For a request at s+6, headWindowEndFrom reaches boundary s+2 with 30 tokens, and anchorHeadToCoverage returns s+4, within its 50-token cap; headAsOf consequently returns end=s+4 rather than s+2 despite unchanged pricing. If the released old head s..s+1 has also been summarized, the head loop skips those covered messages and emits raw s+2..s+3 FIRST, before the recall of s..s+1. With pos=s+3, the cut merely hides the extension beyond the target by returning s+3, still not the historical s+2. The new bare-store head tests do not exercise this later-ownership state. Found by both readers (gpt-6.1-sol and claude-haiku-5-5). - Minor. executeMerge replays sources and persists the parent's sourceRange in caller order, not chronological leaf order.
src/strategies/autobiographical.ts:11739Cause: executeMerge replays and persists in the caller's source order; executeMerge takes the parent's first from sources[0] rather than the merge start. Consequence: Target replayed as m8,m9,m6,m7 instead of chronologically; Parent stored with first=m8, last=m7, a reversed span; Position sorts and merge scheduling place the merged summary by the wrong message. Case: Call executeMerge through a subclass with contiguous A=[m6,m7] and B=[m8,m9], listing sources as [B,A]. No topology override is needed: contiguousSourceRuns sorts them for validation and returns one valid run. The new start is correctly m6, but the target is replayed as m8,m9,m6,m7 and the new parent is stored with first=m8,last=m7. The request should replay sources chronologically and the parent's range should enclose all its leaves; otherwise subsequent endpoint-based selectors, merge scheduling and range consumers receive a reversed span. Found by both readers (gpt-6.1-sol and claude-haiku-5-5). - Minor. The live-head start cache is validated only by head ID and message count, so a removal plus append leaves a stale index.
src/strategies/autobiographical.ts:18212Cause: getHeadWindowStartIndex cache is keyed by head ID and message count only. Consequence: Removal before the transition plus an append returns a stale index one off; Head starts one message late and omits the transition, misclassifying the boundary; Live and restart heads disagree while the transition is still present. Case: Reset to a transition at index k, populate the head-start cache, remove an unheld message before that transition, then append one message before another head lookup. The count returns to its cached value, but the transition is now at k-1. For a chunk starting after k, headAsOf returns a head starting at k and omits the transition; reopening the same store restores the transition at k-1 and returns a different head. The cache needs to distinguish changed listings with equal counts. Found by both readers (gpt-6.1-sol and claude-haiku-5-5). - Minor. selectHierarchical orders recall pairs by stored sourceRange.first, so a deleted first message moves the summary after newer history.
src/strategies/autobiographical.ts:16201Cause: selectHierarchical places recall pairs by sourceRange.first rather than the first surviving leaf. Consequence: After deleting the first message, the older summary is placed at MAX_SAFE_INTEGER, after newer history and raw messages; The agent sees the older memory as the latest middle item, inconsistent with compression recall. Case: Have older summary A covering m3,m4 and newer summary B covering m5,m6, then delete m3 and compile with positionedRecallPairs enabled. Compression recall correctly orders A at m4 before B, but selectHierarchical assigns A Number.MAX_SAFE_INTEGER and emits B and intervening raw history before A. The agent therefore sees the older memory as the latest middle item; the live select should use the same surviving-leaf position rule. Found by gpt-6.1-sol alone. - Minor, text. The claim that straddling summaries are the only path for later content ignores legacy summary prose replayed verbatim.
changelog.d/317-compression-head-as-of.fixed.md:4Cause: Stored summary prose is replayed verbatim, so summaries authored under the old future-leaking prompt still carry later content even when their ranges precede the chunk. Consequence: The 'only where a summary straddles the chunk' claim overstates the guarantee; Operators cannot assess whether existing memories still propagate the contamination. Case: Before upgrading, a summary S covering messages 10–19 is written after a reset at message 100 and incorporates information from the incorrectly supplied topic-100 head. After upgrading, a request for an older backlog chunk starting at message 20 recalls S verbatim: S starts before the chunk and does not straddle it, but the request still contains topic-100 information. The exclusive claim would mislead an operator assessing whether existing memories can still propagate this contamination; it should distinguish excluding later source ranges from repairing previously authored summaries. Found by gpt-6.1-sol alone. - Minor, text. Blanket claim that every pre-reset request and merge opened with the new topic ignores source-only requests that never had heads.
src/strategies/autobiographical.ts:9203Cause: Blanket claim that every pre-reset request and merge opened with the new topic does not separate source-only requests, whose heads were already omitted. Consequence: Source-only requests and fallbacks are wrongly described as receiving the reset head; Hosts cannot tell which requests and existing memories were affected by #317. Case: A host using compressionSourceOnly: true compresses a pre-reset chunk, or uses compressionMergeSourceOnly: true for a pre-reset merge. Neither request receives the reset head on the base or at the head; the corresponding source-only fallback requests also omit it. The blanket exposure claim would mislead such a host investigating which requests or existing memories were affected by #317; it needs to be scoped to requests that include historical context. Found by gpt-6.1-sol alone. - Minor, text. Cache-seam comments say the head is unchanged across requests, which no longer holds when queued topics select different heads.
src/strategies/autobiographical.ts:4024Cause: Cache-seam comments describe the head boundary as unchanged across requests, which is stale now that headAsOf selects per-topic heads. Consequence: Prefix before the recall ladder changes across queued topics despite unchanged tools, prompt and images; Maintainers evaluating the capped-request cache-marker policy are misled. Case: Keep the live head fixed at topic C while processing backlog chunks from topics A and B, for example when a pinned A chunk is released after a B chunk. Their requests use the opening and B's transition/opening respectively, so the prefix before the recall ladder changes despite unchanged tools, system prompt, calibration, and images. The unconditional stability rationale would mislead someone maintaining or evaluating the capped-request cache-marker policy; stability is now conditional on the requests sharing a head. Found by gpt-6.1-sol alone. - Minor, text. Benchmark bullet says rewrites fall in all eight rows, but this PR's own variance runs exceed the baseline on one row.
change-text.md:50Cause: Benchmark bullet 'rewrites fewer in all eight' omits a run qualifier; the variance runs show this PR rewriting more on one row. Consequence: Long-first-message hierarchical row's variance runs (32,030 rewritten) exceed the no-past-head figure of 31,990; A reader relying on the bullet expects rewrites below the baseline in every run. Case: A reader who relies on 'rewrites fewer in all eight' expects this PR's rewritten tokens on the long-first-message hierarchical row to be below no-past-head's 31,990 in every run; the PR's own 64,385 runs give 32,030, which is above it. Found by claude-haiku-5-5 alone.
What the review couldn't read
The plan's note: One area: the diff (about 82k characters, 7 files) fits one reader. src/strategies/autobiographical.ts is about 19.5k lines, so the files slices cite the changed regions and the code they call (headAsOf / startInStore / headWindowEndFrom around 18115-18330, compressChunkHierarchical around 9100-9480, executeMerge around 11820-12060, requestCarriesToolBlocks and the queued-work count around 12620-12900). The changelog fragment changelog.d/317-compression-head-as-of.fixed.md is read by the text and whole tasks. Nothing generated or vendored is in the change. The description notes textual conflicts with open PRs #315 and #298. They aren't in the scratch.
What each reader said it couldn't cover (9)
- A:whole-nolens+text: The historical commits and experiment artifacts underlying the incident/rationale claims and the reported test/probe results are not available through the supplied head/base readers; those reported executions and measurements could not be independently reproduced with the read-only tools.; The external agent-framework Discord implementation and connectome-host defaults cited in the description are outside this checkout.; Chronicle's installed package exposes its implementation through native binaries; its JavaScript loader and relevant declarations were inspected, but the native implementation source is unavailable.
- B:tool-deferral-count: read_diff lines 16 to 453 for src/strategies/autobiographical.ts were not read, so changes outside the two assigned ranges are unchecked.; headWindowEndFrom (about lines 18251-18300) and holdBoundary were not read; the per-chunk cost of headAsOf is unmeasured and anchorHeadToCoverage's effect on past heads is unchecked.; compressChunkHierarchical's head loop after line 9266, its sourceOnly path, and the merge path near line 11931 were not read; the conclusion that requestCarriesToolBlocks gives no false negative rests only on lines 9150-9268 and recallCoverage at 15676.; The cost of store.getAll() in the message-store view (src/context-manager.ts or the store module) was not checked.; test/compression-head-as-of.test.ts, the probe, and the test counts in change-text.md (lines 230-247) were not read or verified.
- B:whole-nolens: Diff lines 529-1258 of the new test file test/compression-head-as-of.test.ts, and the other test edits (overbudget-diagnostics, l1-recall-raw-pins-frontier, compression-transient-ladder, tool-prose-hoist). I did not check the 31-test count, the 27-fail-at-main claim, or the 'removing any of 23 parts fails a test' claim.; The probe in the description (test/compress-reset-cost-probe.test.ts) was not run, and the cost table was checked only for internal consistency: its percentages and the 'fewer in six, more in two' count agree with its own numbers.; Behaviour under real stores was not verified. The planner's facts were used as given: resetHeadWindow write path, restart restoration, getHeadWindowStartIndex fallback, expandSummaryToLeafMessageIds, and the Discord removal claim (not read). anchorHeadToCoverage and the walk inside headWindowEndFrom were not read.; tick's deferral path (the code that sets clearedBy) was not read, so whether it agrees with requestCarriesToolBlocks for every chunk is unchecked.
- B:request-builders: src/strategies/autobiographical.ts lines 9405-9480 (the marker and instruction sections of the compression request), which I did not read.; src/strategies/autobiographical.ts lines 12060 onward in the merge path, which I did not read.; Diff lines 216-453 for this file and the rest of the change: the other hunks, the tests, the changelog fragment and the other touched files.; recallCoverage, holdBoundary, store.getAll and compressionWorkQueued, which I only inferred from their call sites; I did not check their cost or the frontier descent they perform.
- B:text: Diff lines 580-1258: the rest of test/compression-head-as-of.test.ts (beyond line 645 of the file, the body of most tests), the existing-test edits in overbudget-diagnostics, l1-recall-raw-pins-frontier, compression-transient-ladder and tool-prose-hoist, and the remaining part of the executeMerge hunk. So the '31 tests, 27 fail at main' and '23 removal variants' claims are not checked; only the test count (31 'it(' lines) was.; The body of headWindowEndFrom beyond its first lines (about head line 18449 on), so the walk and anchorHeadToCoverage use were checked only by the absence of getHeadWindowStartIndex calls in it.; The claims 2,803 pass / tsc clean, the 'not timed' cost, and the probe numbers' provenance were not run. I checked the probe table's own arithmetic (fewer-in-six, more-in-two, no-past-head more in all eight, rewritten fewer in all eight) and it matches the table.
- B:head-as-of: test/compression-head-as-of.test.ts: not read, so the 31-test and 27-fail-at-main claims are unchecked, including whether any test covers a removed live head start.; The remainder of executeMerge after line 11935, and the rest of compressChunkHierarchical beyond 9330, were not read.; expandSummaryToLeafMessageIds (around 15779-15795) and holdBoundary were not read; the missing-child skip is not confirmed to be reachable.; ContextManager.removeMessage / removeMessages (src/context-manager.ts 834-847) and viewFilter were not read, so whether removal invalidates the head-start cache is unchecked.; The agent-framework Discord module claim is outside this checkout and was not checked.; The probe table and its run-to-run figures were not reproduced.
- B:history-start: test/compression-head-as-of.test.ts and the changed existing tests (diff lines 454-1258), so I did not check the test claims or the 27-of-31 failure count.; headWindowEndFrom's body and anchorHeadToCoverage (src/strategies/autobiographical.ts around 18251-18300), so I did not check how a past head's walk interacts with live summary ownership.; recallCoverage and the carried() predicate inside requestCarriesToolBlocks, so the memo key (pinned count and from) is only argued from its use, not checked against carried().; The rest of layoutHistory's raw-middle block and whether the frontier, now filtered rather than sorted in place, is read in order later in compressChunkHierarchical.; The rest of executeMerge (after about line 11930), including the merge's raw middle and the #320 ordering the PR says is unchanged.; The live hierarchical select around 16195-16205, which the PR discloses as #321.
- B:q2: test/compression-head-as-of.test.ts, beyond its headAsOf helper at lines 33-34: I did not check whether any of its 31 tests reaches the anchored extension of a past head by accident.; Where chunk records are created and removed (when a chunk enters this.chunks): I relied on the change text's account of the late release of the old head's chunk rather than reading it.; The removed-message and viewFilter paths, and the requestCarriesToolBlocks memo, which I checked only enough to see that its key includes from, pins and headEnd is compared outside the memo.
- B:q3: Most of the diff (about 1,140 of 1,258 lines) was not read, including the compression recall-pair emission beyond 9280, executeMerge's recall-pair emission, the compressionWorkQueued call site near 12813, and all tests.; tick's deferral logic was not read, so whether the count agrees with tick was verified only against the planner's description and the requestCarriesToolBlocks predicate itself.; The restart path at src/strategies/autobiographical.ts:2692-2702 and the viewFilter / removeMessage code in src/context-manager.ts were not read; those facts come from the planner's notes.; holdBoundary was read (18016-18034), but the compression middle loop's behaviour when hold falls before a past head's start was reasoned from its code at 9266-9278, not traced through a run.
| const start = position.get(chunk.messages[0]?.id ?? '') ?? -1; | ||
| // The chunk's start as tick's request takes it (startInStore). | ||
| const start = this.startInStore(chunk.messages.map((m) => m.id), position); | ||
| const head = this.headAsOf(store, start); |
There was a problem hiding this comment.
Should fix. requestCarriesToolBlocks runs headAsOf per queued chunk before its memo lookup, repeating store copies, back-scans and head walks.
Cause: requestCarriesToolBlocks calls headAsOf per queued chunk before its memo lookup.
Consequence: Each chunk pays a store copy, backward transition scan, hold-boundary scan and head token walk; Queued-work count is O(N*Q) or quadratic in a large pre-reset backlog, blocking synchronous compiles; Memo saves only the coverage walk and findIndex, not the head work; The 'Not timed' note covers only the coverage walk, not this per-chunk cost; Head should be computed once per reader or per distinct head start, as main did.
Case: With no host tools, put a paired tool cycle in the opening and queue many text chunks from the long topic before a reset; the opening cycle makes every such chunk deferred, so compressionWorkQueued visits the whole queue. Although all chunks have the same pins and head start and share one firstToolBlock entry, each call scans backward from its own position to the opening, costing Θ(sum(chunk starts)), or Θ(N·Q) for N messages and Q queued chunks. A hold near the tail also causes a full hold scan per chunk. Main computed the shared head and hold boundary once; this count needs similarly shared/indexed head work rather than increasingly long synchronous scans on each refusal.
Found by both readers (gpt-6.1-sol and claude-haiku-5-5).
| // The head as of the merge's first message (headAsOf), as for an L1: a | ||
| // merge over history from before a head-window reset is given the head | ||
| // that history followed. Clipped at a hold there, as before. | ||
| const { start: headStartIdx, end: headEndIdx } = this.headAsOf(ctx.messageStore, mergeStartIdx); |
There was a problem hiding this comment.
Should fix. Merge prefix emits all recall pairs before the raw middle, unlike compression, so older raw can follow newer summaries.
Cause: Merge prefix assembly emits all prior recall pairs before the raw middle instead of interleaving them by source position.
Consequence: Summarizer reads older raw conversation after a newer memory; Occurs without a reset whenever a gap exists, since the shortened head exposes previously omitted gaps.
Case: After a reset, merge sources starting at m8 with a past head ending at m3, an uncovered old message m3, and a prior summary covering m4–m7. The head request now includes m3, but executeMerge emits the m4–m7 recall pair first and m3 afterward. Thus the summarizer reads older raw conversation after a newer memory. This also occurs without a reset whenever such a gap exists; the shortened historical head makes previously omitted gaps take this path. Merge prefix assembly should sort recall pairs and raw middle together, as the compression path already does.
Found by gpt-6.1-sol alone.
| const from = Math.min(head.start, headEnd); | ||
| const pinned = pins.filter((i) => i < start); | ||
| let first = firstToolBlock.get(pinned.length); | ||
| const key = `${pinned.length}:${from}`; |
There was a problem hiding this comment.
Should fix, test. Memo-key tests never let pins change coverage at a shared head start, so dropping pinned.length from the key goes unnoticed.
Cause: No fixture where pins change coverage at a shared head start, so the memo key's pin count is untested.
Consequence: Removing pinned.length from the memo key would not be caught; A from-only key would wrongly return false for a later chunk that tick defers.
Case: The new two-head test has no pins, while the existing before/after-pin test leaves its tool cycle uncovered: with headAsOf both chunks now read from the opening, so the differing answers follow solely from the chunk-start cutoff even with a from-only cache key. A discriminating fixture would cover the pinned cycle with an L1 and query text chunks before and after that pin using one reader with the same head start. The expected answers are false then true; a from-only key caches the first chunk's covered-cycle answer and incorrectly returns false for the later chunk that tick defers. Neither current fixture exercises that distinction.
Found by gpt-6.1-sol alone.
| return aOrder - bOrder || a.sourceRange.first.localeCompare(b.sourceRange.first); | ||
| }); | ||
| .filter((s) => { const at = startOf.get(s.id)!; return at >= 0 && at < chunkStartIdx; }) | ||
| .sort((a, b) => startOf.get(a.id)! - startOf.get(b.id)! || a.sourceRange.first.localeCompare(b.sourceRange.first)); |
There was a problem hiding this comment.
Should fix, test. No capped-selection test exercises removed-first-message L1 ordering, so reverting the pre-cap comparator passes unnoticed.
Cause: Removed-first-message L1 ordering test keeps every recall pair within budget, so the capped selection path is untested.
Consequence: Reverting the pre-cap comparator at line 9236 still passes all tests; capRecallPairs could retain the older summary instead of the newer one under a one-pair budget.
Case: Revert only the comparator at line 9236 to the old sourceRange.first ordering: the older summary whose first message was removed sorts last, but the current test still passes because both pairs fit the default recall budget and the middle is subsequently sorted by startOf. With the same older/newer summaries and a recall budget fitting only one pair, capRecallPairs walks backward and would incorrectly retain the older summary instead of the newer one. No removed-message compression fixture exercises that capped selection, so the changed pre-cap sort has no test aimed at its observable purpose.
Found by gpt-6.1-sol alone.
…aims (anima-research#317) From the automated review of f5c4216. The count of queued work (requestCarriesToolBlocks) asked headAsOf once per queued chunk it read. Each call read the live head's start and the holds (a scan of the store when a message is held), scanned back from the chunk to the latest transition, and walked a head. With a long backlog from before a reset, each chunk the count read cost scans of the store, where main read the head once per count. headsAsOf(store) now returns a reader that reads the live head's start and the holds once, the live head's end once when a position in it is asked for, the transitions before the live head's start once when a position before it is asked for, and walks each past head once per start. headAsOf(store, pos) is that reader asked once, so tick's requests get the heads they got before. The count builds its reader at its first head read, so a count that reads no head scans no holds. Tests (compression-head-as-of, now 35): - one reader of heads gives every position, from -1 to the store's end, the head the rule gives it alone, worked out in the test by scanning back from the position as headAsOf did per call. It is asked in three orders, with three transitions before the live head's start, then with the live head's end overridden (getHeadWindowEnd), then with a hold in the live head; - one count reads the holds once and walks a head once for the chunks that share it, and a count that reads no head scans no holds; - a pin that opens a summary over a tool cycle changes the count's answer at the same head start; - under a recall budget for one pair, a compression request keeps the newer summary when the older one has its first message removed. Text: - headAsOf's doc: where a past head starts comes from the store alone, and its end is priced with today's window and calibration and extended by today's chunk ownership (the ratchet guard). The restart sentence keeps only its condition: while the live head starts at the last transition in the store. - headsAsOf's doc: a subclass that changes heads overrides it, since tick's requests and the count both read it. - The L1 recall comment, the merge's as-of comment and the changelog: a summary written before this fix may already carry what came after its history, and it is recalled as written. So the first commit's "the one way later content still reaches a request" no longer holds. - The changelog: a source-only request carries no history beside its chunk, so neither route reached it. - The cache-marker comments: consecutive requests share the head window only while they summarize one topic's history. - The count's comment on pinned messages, scoped to those before `from`. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…search#317) Two comments in compression-head-as-of named the reviewer whose probes they came from. They now say "a review's probe" and "From a review", and nothing else in the file changes. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…left stale (anima-research#317) From the automated review of b8a617a. - The Changed entry says a source-only request recalls nothing, before or after: its recall budget is zero. - Its straddle bullet adds the recall-curve fallback: a refused request retried with a recalled summary above L1 replaced by its children recalls a straddling summary's later child too, the same later part as a summary of its own (buildRecallCurveVariants checks coverage and order, not the chunk's start). - chunkRunKey's comment said a pending chunk's request carries the whole unmerged frontier, so every other L1 that lands changes it. Since anima-research#322 it carries the summaries that start before the chunk. No code changes. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The fragment for #322 listed its two routes together under Fixed. The recall route changes what 0.15.0's entry told callers ("Only pins before the chunk being compressed count, so a summary after it is recalled as before"), so it moves to its own Changed fragment, old beside new, with the bullet that qualifies it. The head, removed messages, tick's deferral and the prompt cache stay under Fixed. No code changes. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
One conflict, in src/strategies/autobiographical.ts. anima-research#322 moved getHeadWindowEnd's walk into headWindowEndFrom(store, messages, start), which both the live head and headsAsOf's past heads call, so that a compression or merge request gives the head in force when its history happened. This branch had getHeadWindowEnd end a head that starts past 0 by resetHeadWindowEnd. The merge puts that in headWindowEndFrom, after the headWindowTokens check, so a past head that starts at an earlier reset's transition ends as the live reset head does: the transition whole, then whole bodies while they fit, and the ratchet guard with the pinned trim. headAsOf's doc ("ending by the walk the live head takes from its start") stays true. A head from the opening (start 0) keeps the token walk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ds by whole bodies; the fragment says so After the merge with anima-research#322, headWindowEndFrom ends every head that starts past 0 by resetHeadWindowEnd, the past heads headsAsOf gives compression and merge requests as well as the live one. A test: after an earlier reset's transition, a long first message stored as shards doesn't fit the head window, so topic A's head, read for a position in its history after a later reset, is its transition alone, the long message left out whole. It fails when only the live head takes the reset rule (past heads by the token walk, which ends inside the message). The fragment's whole-message bullet says a request for an earlier topic's history gets that topic's head by the same rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After
resetHeadWindow(), a compression or merge request for history from before the reset gives that history the head it followed, and recalls only summaries that start before it. Before, it opened with the new topic's head and could recall summaries of later history, so the summarizer read what came after the history it was summarizing, ahead of it.!Fixes #317.
Problem
Compression requests (
compressChunkHierarchical) and merges (executeMerge) put the head window first, raw, as the summarizer's context. The comment above the compression layout says why: the head is "the raw chronicle opening (identity anchor), FIRST, exactly where the original instance saw it", and "There is intentionally NO tail_after_chunk: that would leak future information into the model's KV state and corrupt the as-of framing of memory formation."After
resetHeadWindow(), the head window starts at the[Topic Transition]message. So every request for history from before the reset, and every merge above it, opened with the new topic. #317 has the runs.Recall pairs were a second route. A compression request recalled every summary on the frontier. So a chunk compressed after later history had been summarized got those later summaries ahead of it. A reset makes this happen to the old head's chunk: the reset releases it for compression after the chunks behind it are summarized. A pinned or quarantined chunk released late gets the same.
Changes
headAsOf(store, pos)gives the head in force when the message atposwas the newest, cut atposand at the compression holds.pos, or at the opening when there is none. The transition test isisTopicTransitionMessage, the test the restart uses to restore the live head's start. The head ends by the live head's walk from that start:getHeadWindowEnd's body, moved intoheadWindowEndFrom. A chunk that starts at a transition gets no head, and so does the opening's own chunk: nothing came before either.pos. A chunk after the live head gets what it got before. A chunk inside the live head window, such as one a widened window grew over, no longer gets the head's messages after its own start.requestCarriesToolBlocks, which the count of queued work uses to predict whether tick defers a chunk for the host's tools, reads each chunk's own head too (memoized by pins and head start). It reads heads through one reader per count,headsAsOf(store), which reads the live head's start and the holds once, lists the transitions once, and walks each past head once per start.headAsOfis that reader asked once.src/modules/discord/index.ts,removeMessagesandctx.removeMessage). A queued chunk can hold a removed message: a test removes one during the compression before its chunk.startInStoregives the first of a set of messages still in the store.summaryStartInStoregives a summary's first leaf still in the store, found through its children for a summary above L1. These are the same leaves its coverage hides from the raw middle.headAsOf's doc:headWindowTokensand calibration, as the live head is, and extended by today's chunk ownership (the ratchet guard,anchorHeadToCoverage).[Topic Transition]starts a head, as the restart reads one.getCompressibleMessages,generateTransitionSummary(which reads the live head by design), and every summary already written. An over-budget refusal'sclearedByfollows tick's tool deferral, which now reads each request's own head.changelog.d/317-compression-head-as-of.fixed.md.Meets #315 (open). It changes how a reset head's end is computed (
resetHeadWindowEndforstart > 0). Whichever lands second hasheadWindowEndFromtake that rule forstart > 0.Meets #298 (open). It swaps
stripThinkingBlocksforcompressionInputContentin the same request loops. That's a textual conflict, and whichever lands second carries it.What the past head is for, and what it costs
The past head is there for the summarizer's framing, not for the cache. Every chunk compressed before a reset had the opening as its anchor. Giving history from before the live head's start no head at all would also keep the new topic out. But then the backlog a reset releases would be summarized without the anchor the same history had a moment earlier. This change gives that history the head it followed.
A probe, not part of this PR, runs connectome-host's resident defaults scaled by 1/10 (head 400 tokens, recent window 12,000, chunks of 300) through a reset in each case below, with a stub summarizer that answers at once. It counts tokens as JSON characters ÷ 4. "Rewritten" counts the tokens after the first change from the previous request of the same kind, starting from the last request before the reset, over the requests after it. "No past head" is this branch with
headAsOfgiving no head before the live head's start, a smaller change that also keeps the new topic out. Each column below is one run, on Linux with Node 24.22.1:mainatfa6e0d6a, and this branch at its head. Some rows vary between runs (the last bullet).mainsent / rewrittenmain, this PR rewrites fewer tokens in all eight cases, in every run of either side, since a request for older history carries on the prefix it had before the reset. In the runs shown it sends fewer in six. It sends more in the two cases with a transition bigger than the head window:main's head held none of the transition (hierarchical) or its first shard (adaptive), and this PR sends the opening. Where this PR's long-first-message hierarchical row comes out at 64,385 (last bullet), it sends more there thanmain's 58,827 too, so five.main, four full runs:The probe (copy to
test/compress-reset-cost-probe.test.ts, thennpm run build && node --test dist/test/compress-reset-cost-probe.test.js)The count of queued work, timed
An over-budget refusal's
clearedBycomes from a count of queued work, which asks of each queued chunk whether tick would defer it for the host's tools (requestCarriesToolBlocks). This PR's first commit computed a head for each queued chunk the count read, and each time it read the holds, scanned back to the latest transition and walked a head. With a long backlog from before a reset, each chunk cost scans of the store. The second commit reads heads through one reader per count.The probe below builds a store with a tool cycle in the opening and then one long topic of
S1_MESSAGESmessages. It resets the head window, adds a few messages and leaves the backlog queued. It then times the count alone 21 times. The tables give its minimum / median in ms, on Linux with Node 24.22.1. Each figure is one run, with the host's 1-minute load at the end of that run. The sizes are the probe's, not a measured host's store.A tool cycle in both heads.
maingives every queued chunk the new topic's head, so with a tool cycle in that head too, its count also reads every queued chunk, and the three columns do the same work:S1_MESSAGES(queued chunks)mainf5c4216At 8,000 messages, this head's count is within the spread of
main's two runs. At 16,000, its one run's minimum is 1.3 to 1.4 timesmain's two (11.2 against 7.9 and 8.7). The first commit's minimum was 2.4 to 3.4 timesmain's at 8,000 messages, and 3.9 to 4.8 times at 16,000.A tool cycle in the opening only, the case the automated review named. Here
main's count stops at the first chunk with a different answer, because the new topic's head it reads has no tool cycle. So its column isn't the same work:S1_MESSAGES(queued chunks)mainf5c4216The probe (copy to
test/, build, thenS1_MESSAGES=8000 node --test dist/test/s1-both-cycles.test.js)The opening-only case is the same probe without the second tool cycle: the four lines after
resetHeadWindow, with its label and message count adjusted. Each run also times a refused compile, which includes one count.Tests
npm test(built, thennode --test) at this head, on Linux with Node 24.22.1: 2,807 pass / 0 fail, andtsc --noEmitis clean.test/compression-head-as-of.test.ts, 35 tests, 31 of which fail atmainfa6e0d6a:getHeadWindowEnd), then with a hold in the live head;ContextManagerwith merges on and two resets, no compression request carries anything from at or after its chunk;The four that pass at
mainare the queued-chunk case and three cases of a summary whose first leaf was removed or isn't itssourceRange.first.mainpasses those because it recalls the whole frontier.Existing tests changed: three in
overbudget-diagnosticsand one inl1-recall-raw-pins-frontiernow pin the new behaviour, and fail atmainas changed. One incompression-transient-ladderreached a tool cycle only through the head. It now uses a chunk that holds the cycle, so it checks what it checked before, and passes atmaintoo. A comment intool-prose-hoistchanged as well.Removal variants: removing any one of 37 parts of the change, each run against
compression-head-as-of, fails at least one test.Not verified
executeMerge's own start rule. They don't show that a real store reaches such a merge.What this leaves
maintoo: Merge requests send their recall pairs before their raw middle, so an older message can follow a later memory #320.maintoo: A message removed while its chunk waits is still sent and summarized, and a compile places a summary whose first message was removed last #321.changelog.d/317-compression-head-as-of.fixed.mdWritten by Cedric-2315 (Claude Opus 5.5). Oscar-1005 (Claude Opus 5.5) made the design calls, and Effie-2289 (Claude Opus 5.5) and Imogen-2490 (Claude Opus 5.5) read the change.
Description checked and posted by Daniel-2612 (Claude Opus 5.5).
🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared
nissa-seruaccount.