Files
HologramAgent-DSH-Memory-Port/test/loop.test.js
jianf 377e02c943 fix(memory): make the loop actually promote real recall into system
`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).
2026-11-04 18:24:58 +08:00

443 lines
20 KiB
JavaScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* Behavioural contract for the loop that promotes snapshot sections.
*
* The loop only earns its keep if a promoted recall block reaches the SYSTEM
* prompt and does NOT reach history. These tests pin exactly that, plus the
* cases where it must keep its hands off.
*
* P1 a pure snapshot carrier message moves into `assembly.sections`.
* 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.
* 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.
* P8 a decision with no `assembly` passes through untouched.
* P9 everything that is not promoted survives in `messages`, in order.
* P10 the class does not re-register the factory (no constructor / inject).
*
* @module dsh-memory/test/loop
*/
import assert from 'node:assert/strict'
import test from 'node:test'
import { HologramAgentLoop, promoteSnapshotSections } from '../lib/loop.js'
/** Build a user message shaped like a snapshot-style injector emits. */
function snapshotMessage(text, kind = 'memory') {
return {
role: 'user',
content: [],
source: { kind, form: 'snapshot', sections: [{ name: kind, text }] },
}
}
/** Build a plain user message with real content. */
function plainMessage(text) {
return { role: 'user', content: [{ type: 'text', text }], source: { kind: 'user' } }
}
/** A minimal decision carrying an assembly with one existing section. */
function decisionWith(messages, sections = [{ name: 'base', text: 'base prompt' }]) {
return { kind: 'enter', messages, assembly: { sections, variables: {}, tools: [] } }
}
test('P1: a pure snapshot carrier is promoted into assembly.sections', () => {
const decision = decisionWith([plainMessage('what did I decide?'), snapshotMessage('note alpha')])
const out = promoteSnapshotSections(decision)
assert.equal(out.messages.length, 1)
assert.deepEqual(out.messages[0].content, [{ type: 'text', text: 'what did I decide?' }])
assert.equal(out.assembly.sections.length, 2)
const promoted = out.assembly.sections[1]
assert.equal(promoted.text, 'note alpha')
assert.ok(promoted.name.startsWith('memory'))
})
test('P2: the input decision is not mutated', () => {
const original = decisionWith([snapshotMessage('note alpha')])
const before = structuredClone(original)
promoteSnapshotSections(original)
assert.deepEqual(original, before)
})
test('P3: the promoted section opts out of interpolation', () => {
const out = promoteSnapshotSections(decisionWith([snapshotMessage('literal {{not-a-var}} here')]))
assert.equal(out.assembly.sections[1].interpolate, false)
})
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([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', () => {
const runtime = {
role: 'user',
content: [],
source: { kind: 'runtime-context', form: 'inline', sections: [{ name: 'x', text: 'y' }] },
}
const out = promoteSnapshotSections(decisionWith([runtime]))
assert.equal(out.messages.length, 1)
assert.equal(out.assembly.sections.length, 1)
})
test('P6: a name collision is suffixed, never dropped', () => {
const out = promoteSnapshotSections(
decisionWith([snapshotMessage('note', 'base')], [{ name: 'base', text: 'base prompt' }]),
)
assert.equal(out.assembly.sections.length, 2)
const names = out.assembly.sections.map((section) => section.name)
assert.equal(new Set(names).size, names.length, 'section names must stay unique')
assert.equal(out.assembly.sections[1].text, 'note')
})
test('P7: a reject decision passes through untouched', () => {
const reject = { kind: 'reject', reason: 'nope' }
assert.equal(promoteSnapshotSections(reject), reject)
})
test('P8: a decision without an assembly passes through untouched', () => {
const bare = { kind: 'enter', messages: [snapshotMessage('note')] }
assert.equal(promoteSnapshotSections(bare), bare)
})
test('P9: non-promoted messages keep their order', () => {
const out = promoteSnapshotSections(
decisionWith([plainMessage('first'), snapshotMessage('note'), plainMessage('second')]),
)
assert.deepEqual(
out.messages.map((message) => message.content[0].text),
['first', 'second'],
)
})
test('P10: the subclass does not re-declare the factory or its inject list', () => {
// A second `setFactory` registration throws at startup, and re-declaring
// `inject` would pull a duplicate AgentLoop into the container. Both are
// avoided by not defining a constructor or static inject at all.
assert.equal(Object.prototype.hasOwnProperty.call(HologramAgentLoop, 'inject'), false)
const source = HologramAgentLoop.prototype.constructor.toString()
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)}`,
)
})