From 377e02c943a6d507833c45ef71dad2d8d2f6bcf6 Mon Sep 17 00:00:00 2001 From: jianf <2198972886@qq.com> Date: Wed, 4 Nov 2026 18:24:58 +0800 Subject: [PATCH] fix(memory): make the loop actually promote real recall into system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `promoteSnapshotSections` accepted only ONE carrier spelling: `content: []`. The plugin never emits that shape. `lib/index.js:932` always attaches exactly one `{ type: 'text', text: injected }` block whose text IS `sections[0].text`, so `promotedSection`'s `if (blocks.length > 0) return null` matched on every real recall and the promotion was a no-op in production. Recall stayed in user/message history — the exact defect 6d9ad62 claimed to fix. 6d9ad62's "surface recall as a system section instead of history" was never true: the promotion it relies on was blocked by that content-non-empty check. The unit tests could not see it because their `snapshotMessage()` helper built `content: []` — a shape no producer emits — and the old P4 asserted "real content blocks are NOT promoted", which is the opposite of reality. It was confirmed today, from `lib/index.js:932`'s constant one-text-block injection, and is fixed here. The rule is now "move the text, don't rewrite it": a snapshot carrier with content is promoted only when the rendered blocks equal the sections' text under one of the two canonical joins (`''` glued, `'\n\n'` spaced), and the promoted section text is the rendered content, so relocation is assertable. Non-text blocks, mismatched text, and the stock runtime-context snapshot (whose content carries a preamble the sections do not) pass through untouched. Tests: S1-S7 plus real multi-block S2c/S2d pin the acceptance surface; every negative case carries a positive control in the same decision, so the old "content non-empty => reject" rule cannot satisfy it. P4 is split into P4a/b (content == sections promotes; content != sections does not) with P4c proving the split is decided by text, not by presence of content. `test/loop-system-e2e.test.js` mounts the real HologramAgentLoop on real dsh services, runs one turn, and asserts on the request the adapter received: the recall is in the system prompt (E1) and in no non-system message body (E2), carried by exactly one message (E2b). --- lib/loop.js | 49 +++++- package.json | 2 +- test/loop-system-e2e.test.js | 194 +++++++++++++++++++++ test/loop.test.js | 325 ++++++++++++++++++++++++++++++++++- 4 files changed, 555 insertions(+), 15 deletions(-) create mode 100644 test/loop-system-e2e.test.js diff --git a/lib/loop.js b/lib/loop.js index 15f924e..52a2bdf 100644 --- a/lib/loop.js +++ b/lib/loop.js @@ -33,10 +33,29 @@ const PROMOTED_SUFFIX = ':system' /** * Split one message's `source.sections` out of the message itself. * - * A message is promoted only when it is a pure carrier: snapshot form, a - * non-empty sections array, and no content blocks of its own. A message that - * carries both text and sections is the plugin saying "this is a user-visible - * turn" — touching it would delete something the plugin meant to keep. + * A snapshot carrier is recognised by ONE of two canonical spellings, both of + * which real producers emit: + * + * 1. `content: []` — a bare carrier. The sections are the whole payload. + * 2. `content: [{ type: 'text', text }]` — what this plugin itself emits + * (`lib/index.js:932`): one text block whose text is the sections' text + * joined by `\n\n` (or, from other producers, concatenated with no + * separator). Both conventions are accepted because the joining choice + * belongs to the producer, not to this loop. + * + * ★ The invariant is "move the text, do not rewrite it". So when content is + * present, it is promoted ONLY if the rendered blocks equal the sections' + * text under one of those two conventions, and the promoted section text is + * the *rendered content*, not a re-join of the sections. That makes + * "relocation" assertable in a test (`promoted.text === original content + * text`) and leaves no room for a silent edit. Anything else — a differing + * text, a non-text block, a runtime-context snapshot whose content carries a + * fixed preamble the sections do not — stays in history untouched. + * + * History: the previous rule was "content non-empty ⇒ reject", derived from an + * assumption ("content means the plugin wants the user to see it") that is + * false for this plugin's own output, so on the real path the promotion could + * never fire. See `test/loop.test.js` S1–S7. * * @param {object} message a user message from a pre-step decision * @returns {{text: string, name: string} | null} @@ -46,8 +65,6 @@ function promotedSection(message) { if (source === null || typeof source !== 'object') return null if (source.form !== 'snapshot') return null if (!Array.isArray(source.sections) || source.sections.length === 0) return null - const blocks = Array.isArray(message.content) ? message.content : [] - if (blocks.length > 0) return null const parts = [] for (const section of source.sections) { if (section !== null && typeof section === 'object' && typeof section.text === 'string' && section.text !== '') { @@ -57,7 +74,25 @@ function promotedSection(message) { } if (parts.length === 0) return null const name = typeof source.kind === 'string' && source.kind !== '' ? source.kind : 'memory' - return { text: parts.join('\n\n'), name } + + const blocks = Array.isArray(message.content) ? message.content : [] + if (blocks.length === 0) return { text: parts.join('\n\n'), name } + + // Any non-text block means this is not a carrier of prompt text: an image + // (or a block whose text is not a string) would be silently destroyed by a + // text-only move, so refuse the whole message. + for (const block of blocks) { + if (block === null || typeof block !== 'object') return null + if (block.type !== 'text' || typeof block.text !== 'string') return null + } + + // Both canonical joins are accepted; the promoted text is the RENDERED + // content, which is what the model would have read, byte for byte. + const rendered = blocks.map((block) => block.text).join('') + const glued = parts.join('') + const spaced = parts.join('\n\n') + if (rendered !== glued && rendered !== spaced) return null + return { text: rendered, name } } /** diff --git a/package.json b/package.json index 2fa3334..85621c9 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "node": ">=22.5.0" }, "scripts": { - "test": "node --test test/store.test.js test/plugin.test.js test/loader.test.js test/phase-b-a.test.js test/phase-b-g.test.js test/phase-b-g-static.test.js test/phase-b-h.test.js test/memory-log.test.js test/loop.test.js test/loop-ports.test.js", + "test": "node --test test/store.test.js test/plugin.test.js test/loader.test.js test/phase-b-a.test.js test/phase-b-g.test.js test/phase-b-g-static.test.js test/phase-b-h.test.js test/memory-log.test.js test/loop.test.js test/loop-ports.test.js test/loop-system-e2e.test.js", "check": "node scripts/checkpoints.js" }, "peerDependencies": { diff --git a/test/loop-system-e2e.test.js b/test/loop-system-e2e.test.js new file mode 100644 index 0000000..c228a48 --- /dev/null +++ b/test/loop-system-e2e.test.js @@ -0,0 +1,194 @@ +/** + * End-to-end proof: a recall block produced by the REAL injection shape reaches + * the SYSTEM prompt and does NOT reach any user/message content. + * + * Why this file exists next to `loop.test.js` + * ------------------------------------------- + * `loop.test.js` asserts the pure decision rewrite (`promoteSnapshotSections`). + * That is necessary but not sufficient: the bug it fixes was that the rewrite + * never fired on the real injection shape, and a unit test can only catch that + * if it also encodes the real shape. This file closes the loop the other way — + * it mounts the actual `HologramAgentLoop` on real dsh services (`SessionStore`, + * `AgentRegistry`, `LlmRuntime`, `SystemPrompt`, `ToolRuntime`, + * `SessionProjectionRegistry`), drives one real turn (pre-step → request build → + * LLM call), and asserts against **the request object the adapter actually + * received**, not an intermediate decision. + * + * Two directions, both required: + * E1 positive — the recall text appears in the system prompt the adapter got. + * E2 negative — the recall text appears in NO message body the adapter got + * (every message, every block; not just the last one). + * + * Only E1 fails on the broken commit; E2 is already true there because the old + * code left recall in history. E2 is what this fix is *for*: it must stay true + * after the move, otherwise the promotion would duplicate recall rather than + * relocate it. + * + * No network: the "adapter" is a local `LlmAdapter` subclass that records the + * request and yields one text chunk. + * + * @module dsh-memory/test/loop-system-e2e + */ + +import assert from 'node:assert/strict' +import { createRequire } from 'node:module' +import { pathToFileURL } from 'node:url' +import test from 'node:test' +import { Context } from '@deepseek-ai/cordis' +import { AgentRegistry } from '@deepseek-ai/dsh-agent' +import { LlmAdapter, LlmRuntime, createUserMessage } from '@deepseek-ai/dsh-llm' +import { SessionStore } from '@deepseek-ai/dsh-session' +import { SystemPrompt } from '@deepseek-ai/dsh-system-prompt' +import { ToolRuntime } from '@deepseek-ai/dsh-tools' +import { HologramAgentLoop } from '../lib/loop.js' + +/** + * Load `SessionProjectionRegistry` the way the harness does. + * + * It is a peer of `dsh-agent-loop`, not of this package, so a bare specifier + * only resolves when the repo's `node_modules` happens to link it. Resolving + * from `dsh-agent-loop`'s own location is exactly what a real dsh process does; + * if neither route works there is no harness to run against, and the test skips + * loudly instead of asserting on a half-mounted app. + * @returns {Promise} + */ +async function loadProjectionRegistry() { + try { + return (await import('@deepseek-ai/dsh-session-projection')).SessionProjectionRegistry + } catch { + try { + const peerRequire = createRequire(import.meta.resolve('@deepseek-ai/dsh-agent-loop')) + return (await import(pathToFileURL(peerRequire.resolve('@deepseek-ai/dsh-session-projection')).href)) + .SessionProjectionRegistry + } catch { + return null + } + } +} + +/** Every text the model would read out of a message body. */ +function bodyText(message) { + if (!Array.isArray(message?.content)) return '' + return message.content.map((block) => (typeof block?.text === 'string' ? block.text : '')).join('') +} + +/** The system prompt text as the adapter received it. */ +function systemText(request) { + return request.messages + .filter((message) => message.role === 'system') + .map((message) => bodyText(message)) + .join('\n') +} + +/** Every non-system message body, flattened, for the negative direction. */ +function historyBlocks(request) { + return request.messages + .filter((message) => message.role !== 'system') + .map((message, index) => ({ index, role: message.role, kind: message.source?.kind, text: bodyText(message) })) +} + +test('E1/E2: recall reaches the system prompt and no message body', async (t) => { + const SessionProjectionRegistry = await loadProjectionRegistry() + if (SessionProjectionRegistry === null) { + t.skip('@deepseek-ai/dsh-session-projection not resolvable from this install (no harness to mount)') + return + } + + const RECALL = 'RECALLED-NOTE-4471: user prefers evidence over summaries.' + + /** Records the request it is called with; yields one stop-finish text chunk. */ + const captured = [] + class CaptureAdapter extends LlmAdapter { + async *stream(request) { + captured.push(request) + yield { type: 'block-start', index: 0, blockType: 'text' } + yield { type: 'text-delta', index: 0, text: 'acknowledged' } + yield { type: 'block-end', index: 0, block: { type: 'text', text: 'acknowledged' } } + yield { type: 'finish', reason: 'stop' } + } + } + + const ctx = new Context() + try { + // Cordis defers service publication by a microtask; each `new` below needs + // the previous one visible, so yield between mounts. + new SessionStore(ctx) + await new Promise((resolve) => setTimeout(resolve, 0)) + new AgentRegistry(ctx) + new SystemPrompt(ctx, {}) + new ToolRuntime(ctx) + new LlmRuntime(ctx) + new SessionProjectionRegistry(ctx) + await new Promise((resolve) => setTimeout(resolve, 0)) + + // The loop under test. Mount before any agent exists: `setFactory` runs in + // the constructor and the class refuses a second registration. + new HologramAgentLoop(ctx, { maxParallelToolCalls: 4, agents: [] }) + await new Promise((resolve) => setTimeout(resolve, 0)) + ctx.llm.registerAdapter(['capture'], new CaptureAdapter()) + + // The REAL injection shape, reproduced byte for byte from `lib/index.js:932`: + // one text block whose text IS the section text, `form: 'snapshot'`. + ctx.on( + 'agent/pre-step', + async (payload, next) => { + const decision = await next() + if (decision.kind === 'reject' || payload.signal.aborted) return decision + return { + ...decision, + messages: [ + ...decision.messages, + createUserMessage({ + content: [{ type: 'text', text: RECALL }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: RECALL }] }, + }), + ], + } + }, + { prepend: true }, + ) + + const handle = await ctx.agents.create({ + sessionId: 'loop-system-e2e', + agentOptions: { provider: 'capture', model: 'capture-1' }, + }) + const agent = handle.agent + agent.followup( + createUserMessage({ content: [{ type: 'text', text: 'what did we decide?' }], source: { kind: 'user' } }), + ) + await agent.whenIdle() + + assert.equal(captured.length, 1, `the adapter must be called exactly once; got ${captured.length} call(s)`) + const request = captured[0] + + // ── E1 · positive: the recall is in the system prompt ──────────────── + const system = systemText(request) + assert.ok( + system.includes(RECALL), + `E1: recall must appear in the system prompt the adapter received. system prompt was ${JSON.stringify(system)}`, + ) + + // ── E2 · negative: the recall is in NO message body ────────────────── + const history = historyBlocks(request) + const leaking = history.filter((entry) => entry.text.includes(RECALL)) + assert.deepEqual( + leaking, + [], + `E2: recall must not appear in ANY non-system message; leaking messages: ${JSON.stringify(leaking)}`, + ) + + // E2b · the loop must not duplicate it either: exactly one system message + // may carry it, so a "promote but keep" bug cannot hide behind E1. + const carriers = request.messages.filter((message) => bodyText(message).includes(RECALL)) + assert.equal( + carriers.length, + 1, + `E2b: exactly one message may carry the recall (a move, not a copy); got ${carriers.length}: ${JSON.stringify( + carriers.map((message) => message.role), + )}`, + ) + assert.equal(carriers[0].role, 'system', `E2b: the sole carrier must be the system message; got ${carriers[0].role}`) + } finally { + await ctx.fiber.dispose() + } +}) diff --git a/test/loop.test.js b/test/loop.test.js index 5a20650..0512e47 100644 --- a/test/loop.test.js +++ b/test/loop.test.js @@ -9,8 +9,10 @@ * P2 the returned decision is a NEW object; the input is not mutated. * P3 the promoted section carries `interpolate: false` — free text with * `{{...}}` must not reach the strict interpolator. - * P4 a message with real content blocks is NOT promoted (plugin said - * "user-visible"). + * P4a a carrier whose content text equals its sections IS promoted (the real + * path: `lib/index.js:932` always attaches one text block); P4b the same + * message with differing content is NOT; P4c proves the split is decided + * by text equality, not by the presence of content. * P5 a non-snapshot message is NOT promoted. * P6 a name collision suffixes instead of overwriting or dropping. * P7 `reject` decisions pass through untouched. @@ -68,15 +70,63 @@ test('P3: the promoted section opts out of interpolation', () => { assert.equal(out.assembly.sections[1].interpolate, false) }) -test('P4: a message carrying real content blocks is not promoted', () => { - const mixed = { +test('P4a: a carrier whose content equals its sections IS promoted', () => { + // The old P4 asserted the opposite for every message carrying content, on the + // assumption that "content blocks present" means "the plugin wants the user to + // see this". That assumption is FALSE for the only producer that matters here: + // `lib/index.js:932` always injects exactly ONE text block whose text is + // `sections[0].text` verbatim — content and sections are the same string. So + // the old rule (`content non-empty ⇒ reject`) made `promoteSnapshotSections` a + // permanent no-op on the real recall path and left recall in user/message + // history, which is the bug this pair of tests exists to pin. + const aligned = { + role: 'user', + content: [{ type: 'text', text: 'recalled note' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'recalled note' }] }, + } + const out = promoteSnapshotSections(decisionWith([aligned])) + assert.equal(out.messages.length, 0, `content == sections must be promoted; got ${out.messages.length} message(s)`) + assert.equal(out.assembly.sections.length, 2, `expected base + promoted; got ${out.assembly.sections.length}`) + assert.equal( + out.assembly.sections[1].text, + 'recalled note', + `promoted text must be the content text unchanged; got ${JSON.stringify(out.assembly.sections[1].text)}`, + ) +}) + +test('P4b: a carrier whose content differs from its sections is NOT promoted', () => { + // The old P4 used 'visible' vs 'hidden' — content != sections — which is the + // side that must keep its hands off. That is exactly why it was green on the + // broken commit: it asserted the correct behaviour for the wrong reason and + // never covered the case that actually occurs (`content == sections`). It is + // kept here, correctly named, as one half of the pair. + const drifted = { role: 'user', content: [{ type: 'text', text: 'visible' }], source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'hidden' }] }, } - const out = promoteSnapshotSections(decisionWith([mixed])) - assert.equal(out.messages.length, 1) - assert.equal(out.assembly.sections.length, 1) + const out = promoteSnapshotSections(decisionWith([drifted])) + assert.equal(out.messages.length, 1, `content != sections must stay in history; got ${out.messages.length}`) + assert.equal(out.assembly.sections.length, 1, `no section may be added; got ${out.assembly.sections.length}`) +}) + +test('P4c: P4a and P4b are decided by TEXT, not by "has content at all"', () => { + // One decision holding both carriers: if the implementation matched on the + // mere presence of content, both would land on the same side and the + // messages/sections counts below could not both hold. + const aligned = { + role: 'user', + content: [{ type: 'text', text: 'recalled note' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'recalled note' }] }, + } + const drifted = { + role: 'user', + content: [{ type: 'text', text: 'visible' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'hidden' }] }, + } + const out = promoteSnapshotSections(decisionWith([aligned, drifted])) + assert.deepEqual(out.messages, [drifted], 'only the mismatching carrier may survive') + assert.equal(out.assembly.sections.length, 2, `only the matching carrier may be promoted; got ${out.assembly.sections.length}`) }) test('P5: a non-snapshot message is not promoted', () => { @@ -129,3 +179,264 @@ test('P10: the subclass does not re-declare the factory or its inject list', () assert.equal(source.includes('constructor'), false, 'subclass must not define a constructor') assert.equal(typeof HologramAgentLoop.prototype.prepare, 'function') }) + +// ───────────────────────────────────────────────────────────────────────── +// S1–S7 · the two canonical spellings, and the ones that must not be touched +// ───────────────────────────────────────────────────────────────────────── +// +// Why this block exists: every test above builds its snapshot carrier with +// `content: []`. That is a shape NO real producer emits — `lib/index.js:932` +// always attaches exactly one `{ type: 'text' }` block whose text is +// `sections[0].text` verbatim. So the whole suite was green over a code path +// that never runs in production, and `promoteSnapshotSections` was a no-op on +// every real recall. The rule under test is three-way: +// +// content == sections (either joining convention) → PROMOTE, moving the text +// content != sections → leave it in history +// +// The invariant is "move, don't rewrite": the promoted section text must be +// *identical* to the text the model would otherwise have read out of the +// message content, so these tests can tell a relocation from a silent rewrite. +// +// ★ Every negative case below carries a POSITIVE CONTROL in the same decision: +// a carrier of the same shape that *must* be promoted. Without the control a +// negative assertion is satisfied by the old "content non-empty ⇒ reject" rule +// alone, so it would be green on the broken commit and prove nothing. With it, +// the assertion fails unless the implementation is comparing the two texts. + +/** A user message whose content and sections are the SAME single text. */ +function carrier(text, kind = 'memory') { + return { + role: 'user', + content: [{ type: 'text', text }], + source: { kind, form: 'snapshot', sections: [{ name: kind, text }] }, + } +} + +/** + * A carrier whose N sections are rendered into ONE content block. + * + * ★ Note the shape: this is "single content block carrying N sections", NOT + * "N content blocks". It models the two producers' joining conventions + * ('\n\n' and '') on a single block — which is what `lib/index.js:932` + * actually emits. True multi-block carriers are S2c/S2d below. + * @param {string[]} texts one entry per section + * @param {'glued'|'spaced'} convention how the sections were concatenated + */ +function multiCarrier(texts, convention = 'spaced', kind = 'memory') { + const joined = convention === 'glued' ? texts.join('') : texts.join('\n\n') + return { + role: 'user', + content: [{ type: 'text', text: joined }], + source: { kind, form: 'snapshot', sections: texts.map((text, i) => ({ name: `${kind}-${i}`, text })) }, + } +} + +/** Readable "expected vs actual" rendering for the assertion messages. */ +const show = (value) => JSON.stringify(value) + +test('S1: single block == single section is promoted, text byte-identical', () => { + const out = promoteSnapshotSections(decisionWith([plainMessage('what did I decide?'), carrier('note alpha')])) + assert.equal( + out.messages.length, + 1, + `the carrier must leave messages (expected 1 remaining, got ${out.messages.length})`, + ) + assert.equal( + out.assembly.sections.length, + 2, + `expected base + promoted section = 2, got ${out.assembly.sections.length}`, + ) + const promoted = out.assembly.sections[1] + assert.equal( + promoted.text, + 'note alpha', + `promoted text must be the carrier text unchanged; expected ${show('note alpha')}, got ${show(promoted.text)}`, + ) +}) + +test('S2a: single content block carrying N sections ("\\n\\n" convention) is promoted', () => { + const out = promoteSnapshotSections(decisionWith([multiCarrier(['a', 'b'], 'spaced')])) + assert.equal( + out.messages.length, + 0, + `the spaced carrier must be promoted (expected 0 messages, got ${out.messages.length})`, + ) + assert.equal( + out.assembly.sections.length, + 2, + `expected base + promoted = 2 sections, got ${out.assembly.sections.length}`, + ) + assert.equal( + out.assembly.sections[1].text, + 'a\n\nb', + `promoted text must equal the content text exactly (move, not rewrite); expected ${show('a\n\nb')}, got ${show(out.assembly.sections[1].text)}`, + ) +}) + +test('S2b: single content block carrying N sections ("" glued convention) is promoted', () => { + const out = promoteSnapshotSections(decisionWith([multiCarrier(['a', 'b'], 'glued')])) + assert.equal( + out.messages.length, + 0, + `the glued carrier must be promoted (expected 0 messages, got ${out.messages.length})`, + ) + assert.equal( + out.assembly.sections.length, + 2, + `expected base + promoted = 2 sections, got ${out.assembly.sections.length}`, + ) + assert.equal( + out.assembly.sections[1].text, + 'ab', + `promoted text must equal the content text exactly; expected ${show('ab')}, got ${show(out.assembly.sections[1].text)}`, + ) +}) + +test('S2c: two content blocks == two sections (glued) is promoted', () => { + const msg = { + role: 'user', + content: [{ type: 'text', text: 'a' }, { type: 'text', text: 'b' }], // 2 blocks, rendered='ab' + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'm0', text: 'a' }, { name: 'm1', text: 'b' }] }, + } + const out = promoteSnapshotSections(decisionWith([msg])) + assert.equal(out.messages.length, 0, `two blocks == two sections must be promoted; got ${out.messages.length} message(s)`) + assert.equal(out.assembly.sections[1].text, 'ab', `promoted text must be the rendered content; got ${JSON.stringify(out.assembly.sections[1].text)}`) +}) + +test('S2d: two content blocks != sections is NOT promoted (control: the match is)', () => { + const drifted = { + role: 'user', + content: [{ type: 'text', text: 'a' }, { type: 'text', text: 'b' }], // rendered='ab' + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'm0', text: 'a\n\nb' }] }, // glued/spaced='a\n\nb' + } + const control = { + role: 'user', + content: [{ type: 'text', text: 'a' }, { type: 'text', text: 'b' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'm0', text: 'a' }, { name: 'm1', text: 'b' }] }, + } + const out = promoteSnapshotSections(decisionWith([control, drifted])) + assert.equal(out.messages.length, 1, `only the drifted one may stay; got ${out.messages.length}`) + assert.equal(out.messages[0], drifted, 'the surviving message must be the drifted one') +}) + +test('S3: multi block != multi section is NOT promoted (control: the match is)', () => { + // Sections say "a"+"b", the content block says "zzz": editing the text would + // not be a move, so this one must stay in history — while the matching + // carrier beside it must go. The two must be told apart by CONTENT, not by + // "does it carry content at all". + const drifted = { + role: 'user', + content: [{ type: 'text', text: 'zzz' }], + source: { + kind: 'memory', + form: 'snapshot', + sections: [{ name: 'memory-0', text: 'a' }, { name: 'memory-1', text: 'b' }], + }, + } + const control = multiCarrier(['a', 'b'], 'spaced') + const out = promoteSnapshotSections(decisionWith([control, drifted])) + assert.equal( + out.messages.length, + 1, + `only the drifted carrier may stay (expected 1 message, got ${out.messages.length})`, + ) + assert.equal(out.messages[0], drifted, 'the surviving message must be the drifted carrier itself') + assert.equal( + out.assembly.sections.length, + 2, + `only the control may be promoted (expected 2 sections, got ${out.assembly.sections.length})`, + ) + assert.equal( + out.assembly.sections[1].text, + 'a\n\nb', + `the promoted text must come from the control; expected ${show('a\n\nb')}, got ${show(out.assembly.sections[1].text)}`, + ) +}) + +test('S4: single block != single section is NOT promoted (control: the match is)', () => { + const drifted = { + role: 'user', + content: [{ type: 'text', text: 'content says this' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'sections say that' }] }, + } + const control = carrier('note alpha') + const out = promoteSnapshotSections(decisionWith([control, drifted])) + assert.equal(out.messages.length, 1, `only the drifted carrier may stay; got ${out.messages.length} messages`) + assert.equal(out.messages[0], drifted, 'the surviving message must be the drifted carrier itself') + assert.equal(out.assembly.sections.length, 2, `only the control may be promoted; got ${out.assembly.sections.length} sections`) + assert.equal( + out.assembly.sections[1].text, + 'note alpha', + `promoted text must be the control's; expected ${show('note alpha')}, got ${show(out.assembly.sections[1].text)}`, + ) +}) + +test('S5: empty content (legacy carrier) is promoted — regression, with its real-world twin', () => { + // Two carriers: the legacy `content: []` shape (which the old code did handle) + // and the shape every real producer emits (which it did not). Both must be + // promoted, so this doubles as the regression pin for the old path AND as the + // minimal reproduction of the production no-op. + const out = promoteSnapshotSections(decisionWith([snapshotMessage('note alpha'), carrier('note beta')])) + assert.equal( + out.messages.length, + 0, + `both carriers must be promoted (expected 0 messages, got ${out.messages.length})`, + ) + assert.deepEqual( + out.assembly.sections.map((section) => section.text), + ['base prompt', 'note alpha', 'note beta'], + `expected the legacy text first, then the real-world text; got ${show(out.assembly.sections.map((s) => s.text))}`, + ) +}) + +test('S6: runtime-context shape (prefix in content, absent from sections) is NOT promoted', () => { + // The stock loop prepends a fixed sentence to the content and keeps `sections` + // as the raw contributions, so the two can never be equal. Promoting it would + // delete that preamble, so it must pass through — the control carrier in the + // same decision proves the comparison is on text, not on "has content". + const runtime = { + role: 'user', + content: [ + { + type: 'text', + text: 'Current runtime context. This snapshot supersedes earlier runtime-context snapshots.\n\ncwd /tmp', + }, + ], + source: { kind: 'runtime-context', form: 'snapshot', sections: [{ name: 'runtime-context', text: 'cwd /tmp' }] }, + } + const control = carrier('cwd /tmp', 'runtime-context') + const out = promoteSnapshotSections(decisionWith([control, runtime])) + assert.equal(out.messages.length, 1, `runtime-context must stay in history; got ${out.messages.length} messages`) + assert.equal(out.messages[0], runtime, 'the surviving message must be the runtime-context snapshot itself') + assert.equal(out.assembly.sections.length, 2, `only the control may be promoted; got ${out.assembly.sections.length} sections`) + assert.equal( + out.assembly.sections[1].text, + 'cwd /tmp', + `promoted text must be the control's; expected ${show('cwd /tmp')}, got ${show(out.assembly.sections[1].text)}`, + ) +}) + +test('S7: a non-text block means NOT promoted (control: the text-only one is)', () => { + const imageCarrier = { + role: 'user', + content: [{ type: 'image', text: 'note alpha' }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: 'note alpha' }] }, + } + const badText = { + role: 'user', + content: [{ type: 'text', text: 42 }], + source: { kind: 'memory', form: 'snapshot', sections: [{ name: 'memory', text: '42' }] }, + } + const control = carrier('note alpha') + const out = promoteSnapshotSections(decisionWith([control, imageCarrier, badText])) + assert.equal(out.messages.length, 2, `only the text-only control may be promoted; got ${out.messages.length} messages`) + assert.equal(out.messages[0], imageCarrier, 'the image carrier must survive') + assert.equal(out.messages[1], badText, 'the non-string-text carrier must survive') + assert.equal(out.assembly.sections.length, 2, `only the control may be promoted; got ${out.assembly.sections.length} sections`) + assert.equal( + out.assembly.sections[1].text, + 'note alpha', + `promoted text must be the control's; expected ${show('note alpha')}, got ${show(out.assembly.sections[1].text)}`, + ) +})