diff --git a/changelog.d/116-l1-recall-raw-pins.fixed.md b/changelog.d/116-l1-recall-raw-pins.fixed.md new file mode 100644 index 0000000..9a6eb3f --- /dev/null +++ b/changelog.d/116-l1-recall-raw-pins.fixed.md @@ -0,0 +1 @@ +- The canonical L1 compression prompt honors raw (level-0) pins. The live selector already did, but the prompt still recalled the summary covering a pinned span. A frontier summary that covers a force-raw message is now descended rather than recalled: its children take its place, so only the branch containing the pin opens and the raw added is just the pinned L1 chunks (sibling branches stay summarized). Empty-content frontier summaries are descended the same way instead of exposing their whole span raw. Pin precedence matches the kv-stable selector (`isForceRawPinBound`: a fixed `level` wins over `maxLevel`). Merge recall, L3+ merge inputs, transition summaries and the source-only L1 do not consult pins. diff --git a/src/strategies/autobiographical.ts b/src/strategies/autobiographical.ts index 657acab..92905b8 100644 --- a/src/strategies/autobiographical.ts +++ b/src/strategies/autobiographical.ts @@ -2402,6 +2402,18 @@ export class AutobiographicalStrategy implements ResettableStrategy { * a fixed `level` clamps both ends; a `maxLevel` only caps depth. Honored * solely by the KV-stable controller — see `ProtectedRange`. */ + /** + * Whether a pin bound forces a message raw, with the same precedence as the + * kv-stable selector: a fixed `level` wins over `maxLevel`, so + * `{level: 2, maxLevel: 0}` is held at L2 (not raw). A pinned position with + * no level bound is a classic raw pin. + */ + static isForceRawPinBound(bound: { level?: number; maxLevel?: number } | undefined): boolean { + if (bound === undefined) return true; + if (bound.level !== undefined) return bound.level === 0; + return bound.maxLevel === 0; + } + protected pinLevelBounds(messages: StoredMessage[]): Map { const out = new Map(); if (this.pins.length === 0) return out; @@ -5318,12 +5330,55 @@ export class AutobiographicalStrategy implements ResettableStrategy { const messageOrder = new Map( allMessages.map((message, index) => [message.id, index]), ); - const priorSummaries = this.summaries - // Skip empty-content summaries: emitting `{type:'text', text:''}` as a - // recall pair triggers Anthropic 400 "text content blocks must be - // non-empty", which stalls ALL compression (mirrors the render-path guard - // + load-drop). A single empty summary otherwise poisons every compression. - .filter((s) => !s.mergedInto && !!s.content && s.content.trim().length > 0) + // Raw pins in the canonical L1 prompt's recall. The live view shows a + // force-raw message raw; recall here used to include the summary covering + // it anyway. A frontier summary that can't be shown as a recall pair (it + // covers a force-raw message, or its content is empty) is DESCENDED, not + // dropped: its children take its place, so only the branch containing the + // pin opens up, sibling branches stay folded, and the raw added to the + // prompt is just the pinned L1 chunks. This mirrors the live selector + // (kv-control keeps protected raw leaves beside their group's recall). + // Dropping the whole frontier summary instead expanded an entire L3/L4 to + // raw on every L1 while one pin existed (#116 review). + // + // Scope: the canonical L1 prompt. Merge recall, L3+ merge inputs, + // transition summaries and the source-only L1 do not consult pins. + const summariesById = new Map(); + for (const s of this.summaries) summariesById.set(s.id, s); + const pinnedPositionsSet = this.pinnedPositions(allMessages); + const pinBounds = this.pinLevelBounds(allMessages); + const rawPinnedMessageIds = new Set(); + for (let i = 0; i < allMessages.length; i++) { + if (!pinnedPositionsSet.has(i)) continue; + if (AutobiographicalStrategy.isForceRawPinBound(pinBounds.get(i))) { + rawPinnedMessageIds.add(allMessages[i].id); + } + } + const summaryTouchesRawPin = (summary: SummaryEntry): boolean => { + if (rawPinnedMessageIds.size === 0) return false; + const leaves = new Set(); + this.expandSummaryToLeafMessageIds(summary, summariesById, leaves); + for (const id of leaves) if (rawPinnedMessageIds.has(id)) return true; + return false; + }; + // Skip empty-content summaries as recall pairs: emitting `{type:'text', + // text:''}` triggers Anthropic 400 "text content blocks must be + // non-empty", which stalls ALL compression (mirrors the render-path guard + // + load-drop). They are descended like pinned ones, so their children + // still represent the span instead of all of it going raw. + const recallable = (s: SummaryEntry): boolean => + !!s.content && s.content.trim().length > 0 && !summaryTouchesRawPin(s); + const mixedFrontier: SummaryEntry[] = []; + const descend = (s: SummaryEntry): void => { + if (recallable(s)) { mixedFrontier.push(s); return; } + if (s.level <= 1) return; // an L1 that can't be shown: its chunk stays raw + for (const childId of s.sourceIds) { + const child = summariesById.get(childId); + if (child) descend(child); + } + }; + for (const s of this.summaries) if (!s.mergedInto) descend(s); + const priorSummaries = mixedFrontier .sort((a, b) => { const aOrder = messageOrder.get(a.sourceRange.first) ?? Number.MAX_SAFE_INTEGER; const bOrder = messageOrder.get(b.sourceRange.first) ?? Number.MAX_SAFE_INTEGER; @@ -5337,14 +5392,11 @@ export class AutobiographicalStrategy implements ResettableStrategy { // a budget-dropped summary doesn't make its raw messages reappear. // Expand summary sourceIds down to leaf message IDs — an L2's // sourceIds are L1 IDs, not message IDs; a flat walk would miss - // every message it transitively covers (Bug 10). Also expand merged - // L1s as defense in depth. - const summariesById = new Map(); - for (const s of this.summaries) summariesById.set(s.id, s); + // every message it transitively covers (Bug 10). The frontier is the + // authority here: a second pass expanding every merged L1 would hide the + // raw source of a frontier summary excluded by the pin rule above. + // (summariesById is built above, for that pin-aware recall filter.) const priorSummaryMessageIds = new Set(); - for (const s of this.summaries) { - if (s.level === 1) this.expandSummaryToLeafMessageIds(s, summariesById, priorSummaryMessageIds); - } for (const s of priorSummaries) { this.expandSummaryToLeafMessageIds(s, summariesById, priorSummaryMessageIds); } diff --git a/test/l1-recall-raw-pins-frontier.test.ts b/test/l1-recall-raw-pins-frontier.test.ts new file mode 100644 index 0000000..cc45179 --- /dev/null +++ b/test/l1-recall-raw-pins-frontier.test.ts @@ -0,0 +1,73 @@ +/** + * #116 review: a raw pin inside a merged summary must open only the branch + * that contains it. Before the mixed frontier, one pinned message made the + * canonical L1 prompt replay the whole covering L3 raw (reviewer's repro: 187 + * old messages raw vs 9 in the live view), on every L1 while the pin existed. + */ +import { describe, it, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ContextManager, AutobiographicalStrategy } from '../src/index.js'; + +const dir = mkdtempSync(join(tmpdir(), 'l1-recall-pins-')); +after(() => rmSync(dir, { recursive: true, force: true })); + +async function forest(name: string) { + const requests: any[] = []; + let calls = 0; + const membrane = { complete: async (req: unknown) => { requests.push(JSON.parse(JSON.stringify(req))); calls++; + return { stopReason: 'end_turn', content: [{ type: 'text', text: `Summary #${calls}: ordinary events.` }], usage: { inputTokens: 500, outputTokens: 100 } }; } }; + const strategy = new AutobiographicalStrategy({ compressionModel: 'm', targetChunkTokens: 200, headWindowTokens: 0, recentWindowTokens: 0, + autoTickOnNewMessage: false, minChunkCharsForLLM: 0, summaryParticipant: 'Claude', hierarchical: true, mergeThreshold: 6, adaptiveResolution: true }); + const cm = await ContextManager.open({ path: join(dir, name), strategy, membrane: membrane as never }); + const ids: string[] = []; + for (let i = 0; i < 240; i++) ids.push(cm.addMessage(i % 2 ? 'Claude' : 'User', [{ type: 'text', text: `OLD-${i} ${'word '.repeat(30)}` }])); + for (let r = 0; r < 200; r++) { await cm.compile(); await cm.tick(); if (calls > 150) break; } + return { cm, strategy, ids, requests }; +} + +function firstL1After(cm: ContextManager, requests: any[]) { + return async () => { + requests.length = 0; + for (let i = 0; i < 20; i++) cm.addMessage(i % 2 ? 'Claude' : 'User', [{ type: 'text', text: `NEW-${i} ${'word '.repeat(30)}` }]); + await cm.compile(); await cm.tick(); + const l1 = requests[0]; + const texts: string[] = l1.messages.flatMap((m: any) => m.content).filter((b: any) => typeof b.text === 'string').map((b: any) => b.text); + return { oldRaw: texts.filter((t) => t.startsWith('OLD-')).length, recall: texts.filter((t) => t.startsWith('[CM] Recall memory')).length }; + }; +} + +describe('raw pins open only their own branch of the L1 recall frontier', () => { + it('one pinned message deep in a merged summary adds at most its chunk raw', async () => { + const { cm, strategy, ids, requests } = await forest('one-pin'); + const levels: Record = {}; + for (const s of (strategy as unknown as { summaries: Array<{ level: number; mergedInto?: string }> }).summaries) if (!s.mergedInto) levels[s.level] = (levels[s.level] ?? 0) + 1; + assert.ok((levels[2] ?? 0) + (levels[3] ?? 0) > 0, `need an L2+ frontier, got ${JSON.stringify(levels)}`); + + const baseline = await firstL1After(cm, requests)(); + cm.pinRange(ids[5], ids[5], { name: 'one-message' }); + const pinned = await firstL1After(cm, requests)(); + + // The pinned message's L1 chunk (a handful of messages) opens; nothing else. + assert.ok(pinned.oldRaw - baseline.oldRaw <= 12, `raw grew by ${pinned.oldRaw - baseline.oldRaw} (was 180+ before the fix)`); + assert.ok(pinned.oldRaw > baseline.oldRaw, 'the pinned chunk itself is shown raw'); + assert.ok(pinned.recall >= baseline.recall, 'sibling branches stay summarized as recall pairs'); + cm.close(); + }); +}); + +describe('pin precedence matches the kv-stable selector', () => { + const raw = AutobiographicalStrategy.isForceRawPinBound; + const cases: Array<[string, { level?: number; maxLevel?: number } | undefined, boolean]> = [ + ['classic raw pin (no bound)', undefined, true], + ['level: 0', { level: 0 }, true], + ['maxLevel: 0', { maxLevel: 0 }, true], + ['level: 2', { level: 2 }, false], + ['maxLevel: 2', { maxLevel: 2 }, false], + ['overlapping {level: 2, maxLevel: 0}: level wins', { level: 2, maxLevel: 0 }, false], + ['overlapping {level: 0, maxLevel: 3}', { level: 0, maxLevel: 3 }, true], + ]; + for (const [name, bound, expected] of cases) it(name, () => assert.equal(raw(bound), expected)); +}); diff --git a/test/summary-reasoning-roundtrip.test.ts b/test/summary-reasoning-roundtrip.test.ts index f3375d7..f2beabf 100644 --- a/test/summary-reasoning-roundtrip.test.ts +++ b/test/summary-reasoning-roundtrip.test.ts @@ -301,6 +301,79 @@ describe('Summary reasoning round-trip (Fable-5 signed thinking)', () => { await manager.close(); }); + it('compression honors level-0 pins by replacing the covered recall with raw source', async () => { + cleanup(); + + const requests: Array<{ messages: Array<{ participant: string; content: Array> }> }> = []; + let calls = 0; + const membrane = { + complete: async (req: (typeof requests)[number]) => { + requests.push(JSON.parse(JSON.stringify(req))); + calls++; + return { + stopReason: 'end_turn', + content: [{ type: 'text', text: `Summary #${calls}: ordinary events.` }], + usage: { inputTokens: 500, outputTokens: 100 }, + }; + }, + }; + const strategy = new AutobiographicalStrategy({ + ...strategyConfig(), + targetChunkTokens: 200, + mergeThreshold: 1_000, + }); + const manager = await ContextManager.open({ + path: TEST_STORE_PATH, + strategy, + membrane: membrane as never, + }); + + const textById = new Map(); + for (let i = 0; i < 20; i++) { + const text = `PIN-SOURCE-${i} ${filler(30)}`; + const id = manager.addMessage(i % 2 === 0 ? 'User' : 'Claude', [{ type: 'text', text }]); + textById.set(id, text); + } + await manager.compile(); + await manager.tick(); + + const state = strategy as unknown as { summaries: SummaryEntry[] }; + const pinnedSummary = state.summaries.find((entry) => entry.level === 1); + assert.ok(pinnedSummary, 'setup: an L1 exists to pin back to raw'); + manager.pinAtLevel( + pinnedSummary!.sourceRange.first, + pinnedSummary!.sourceRange.last, + 0, + { name: 'compression-raw-pin' }, + ); + + requests.length = 0; + for (let i = 0; i < 20; i++) { + manager.addMessage(i % 2 === 0 ? 'User' : 'Claude', [ + { type: 'text', text: `NEW-CHUNK-${i} ${filler(30)}` }, + ]); + } + await manager.compile(); + await manager.tick(); + + assert.ok(requests.length >= 1, 'a later compression request was issued'); + const requestText = requests + .flatMap((request) => request.messages) + .flatMap((message) => message.content) + .filter((block): block is Record & { text: string } => typeof block.text === 'string') + .map((block) => block.text) + .join('\n'); + assert.ok( + !requestText.includes(`[CM] Recall memory ${pinnedSummary!.id}.`), + 'the summary covering a raw-pinned span is not recalled', + ); + for (const id of pinnedSummary!.sourceIds) { + assert.ok(requestText.includes(textById.get(id)!), `raw source message ${id} is present`); + } + + await manager.close(); + }); + it('leaves responseContent absent for reasoning-free responses (non-thinking models)', async () => { cleanup();