diff --git a/README.md b/README.md index ebed832..5431425 100644 --- a/README.md +++ b/README.md @@ -220,11 +220,18 @@ one attributed user message: Programmatic use (another plugin, or a preset subtree): ```js -ctx.stablemem.commit({ text: 'etcd 集群端口 2379', source: 'ops' }) -ctx.stablemem.recall('etcd 端口', 5) +await ctx.stablemem.commit({ text: 'etcd 集群端口 2379', source: 'ops' }) +await ctx.stablemem.recall('etcd 端口', { topK: 5 }) // both are async since phase B +await ctx.stablemem.recall('etcd 端口', { topK: 5, signal }) // pass the turn's signal ctx.stablemem.link({ sourceKind: 'block', sourceId: a, targetKind: 'block', targetId: b, type: 'supersedes' }) ``` +> `recall(query, 5)` used to work and silently stopped meaning anything when the +> second parameter became an options object — a number is not `{topK}`, so the +> default was used. Both `commit` and `recall` are `async` now; omitting `await` +> on `commit` still writes today only because the no-embedder path has no `await` +> before `putBlock`, which is exactly the fragile assumption W6 guards. + ### Phase B · group A — the async seam `recallBlocks` stays **synchronous and unmodified**; vectors get a separate async @@ -421,6 +428,8 @@ chain (not by calling the seam directly): **54 ms**. | **W10** | aborting a pre-step returns in < 500 ms, degrade reported | drop `options.signal` at `embedText` ⇒ red, **30001 ms** | | **W10b** | the same at the seam, so a hook refactor cannot hide a regression | as above | | **W11** | a degraded recall puts a readable notice in the rendered text | remove the notice line ⇒ red | +| **W11d** | degraded **and** zero hits still injects the notice (no entries) | restore either short-circuit ⇒ red | +| **W12** | `aborted` and `embed_error` carry different labels and different wording | collapse the two labels ⇒ red | | **W11b** | a healthy recall renders no notice | — | | **W11c** | end-to-end: aborted pre-step text still carries the hit *and* the notice | remove the notice line ⇒ red | @@ -595,7 +604,7 @@ because they will bite anyone writing a second dsh service plugin. ## Verify ```bash -npm test # 108 tests: store, recall, wiring, loader, isolation, regression, G6/G6b/G7, phase-B A + G + H +npm test # 113 tests: store, recall, wiring, loader, isolation, regression, G6/G6b/G7, phase-B A + G + H npm run check # 10 checkpoints with human-readable evidence (V1–V5 + V3c + MIG3 + mutations) ``` @@ -651,7 +660,7 @@ dsh-memory · phase A checkpoints 10/10 checkpoints passed plugin: name=dsh-memory inject=[systemPrompt, tools] -workspace: /tmp/dsh-memory-cp-DfToyx (removed) +workspace: /tmp/dsh-memory-cp-d2Mqss (removed) ``` --- diff --git a/docs/phase-b-criteria.md b/docs/phase-b-criteria.md index 195573d..d18e7c3 100644 --- a/docs/phase-b-criteria.md +++ b/docs/phase-b-criteria.md @@ -271,6 +271,30 @@ phase B 必须把向量放进 **[0..1) 带**,不得越到 exact 带。 --- +### W. 实施期新增(H 组落地时发现的判据,草案里没有) + +这组不在原草案里。它们是实现过程中被真 bug 逼出来的,每条都对应一次 +「绿着但错」,所以留在这里作为历史与方法记录。 + +| # | 断言 | 逼出它的 bug | 变异 | +|---|---|---|---| +| W1/W5 | `exec.signal` 传到 `embedText`;abort 仍落盘(vector 空) | 写路径不接信号,取消后等满 adapter 超时 | 不传 signal ⇒ 红 | +| W4 | 写侧维度校验:长度不符按 embed 失败处理 | 坏向量会入库,召回时被 skippedDim 静默跳过 | 删校验 ⇒ 红 | +| W6 | 无 embedder 时 `commit` 与 `putBlock` 同形 | 九处调用点靠「无 await 也同步写完」的隐式事实 | — | +| W7 | 空指纹在 embed **之前**判 | 原顺序白付一次网络往返 | 改为事后判 ⇒ 红 | +| W8 | `rebuildCentroid` 从块重建(空间过滤 / 维度跳过 / 0.6 阈值) | `saveSpaceCentroid` 在 lib/ 里零调用点,中心表永久空 | 去空间过滤 / 去维度过滤 / 改阈值 ⇒ 红 | +| **W9** | 真 adapter 下 caller abort 打断真实请求 | **`embedText(text)` 单形参,第二实参被 JS 静默丢弃** | 单参 + 去 fuse ⇒ 红 | +| W9b | adapter 自身超时在有 caller signal 时仍生效 | 接 caller 信号时容易把自身 timeout 丢掉 | 只留 caller ⇒ 红 | +| **W10** | 读路径(pre-step → contextForStep → recall → adapter)也接信号 | **读路径完全没有取消**,每次 pre-step 可卡 30s | 删 `options.signal` ⇒ 红,30001ms | +| **W11** | 降级要在**渲染文本**里可见 | `degraded` 是 `enumerable:false`,对任何序列化都不可见 | 去掉提示行 ⇒ 红 | +| **W11d** | 降级 + 零命中仍要注入提示,且无条目 | **两处短路**(contextForStep 与 renderRecall)各自都能吃掉它 | 恢复任一短路 ⇒ 红 | +| **W12** | `aborted` 与 `embed_error` 标签与文案必须分开 | 取消被描述成「搜索失败」,会诱导无谓重试 | 合并标签 ⇒ 红 | + +★ 这组里 W9/W10/W11d 的教训是同一个:**判据用替身/短路/自造形状时, +验的是替身而不是真代码**。W9 之前用假 embedder(它自己监听 abort), +W11d 之前两处短路互相掩盖,V3b 之前用自造 2 段 batch。 +补判据时先问一句:这条路径上有没有替身站在可能出错的那段前面。 + ## 2. 变异规则汇总 一句总纲:**每条断言至少一条变异能把它打红;打不红就说明这条判据没测到东西。** diff --git a/lib/index.js b/lib/index.js index 1836c78..f60bc90 100644 --- a/lib/index.js +++ b/lib/index.js @@ -258,7 +258,11 @@ export class StableMem extends Service { const query = lastUserText(messages) if (query === '') return '' const hits = await this.recall(query, { signal }) - if (hits.length === 0) return '' + // ★ Do NOT short-circuit on `hits.length === 0` here. renderRecall decides + // silence by asking whether there are BOTH no entries and no notice; a + // degraded recall with zero hits still has something to say. Short-circuiting + // here would discard that notice before renderRecall ever sees it — and + // fixing only renderRecall without this line changes nothing (W11d†b). return renderRecall(hits, name) } diff --git a/lib/recall.js b/lib/recall.js index 70ce473..ecf8be0 100644 --- a/lib/recall.js +++ b/lib/recall.js @@ -361,7 +361,8 @@ export function recallBlocks(store, query, { topK = 5, poolSize = 50 } = {}) { * against phase A output. * * @param {Array} hits - * @param {string|null} mode one of `no_embedder` / `embed_error` / `hybrid` / null + * @param {string|null} mode one of `no_embedder` / `embed_error` / `aborted` / + * `vector_ready_unused` / null * @param {Error} [error] * @returns {Array} the same array */ @@ -437,7 +438,17 @@ export async function recallBlocksHybrid(store, embedder, query, options = {}) { queryVector = await embedder.embedText(String(query ?? ''), options.signal) } catch (error) { // Decision §0.4: never let the vector side take the whole recall down. - return withDiagnosis(recallBlocks(store, query, options), 'embed_error', error) + // + // ★ Cancellation and failure are different facts and must not share a label. + // `aborted` means "we stopped on purpose", so the model must not be told the + // search was incomplete in a way that invites a retry; `embed_error` means + // "the provider failed", where absence of evidence really is suspicious. + // Collapsing them made the notice say "search FAILED" for a deliberate + // cancellation — misleading in exactly the case where the user is watching. + const aborted = + options.signal?.aborted === true || + (error instanceof Error && (error.name === 'AbortError' || /abort/i.test(error.message))) + return withDiagnosis(recallBlocks(store, query, options), aborted ? 'aborted' : 'embed_error', error) } if (!Array.isArray(queryVector) || queryVector.length === 0) { @@ -478,17 +489,29 @@ export async function recallBlocksHybrid(store, embedder, query, options = {}) { * @returns {string} rendered block, or '' when there is nothing to say */ export function renderRecall(hits, sourceLabel = 'stablemem') { - if (!Array.isArray(hits) || hits.length === 0) return '' + if (!Array.isArray(hits)) return '' const lines = hits.map((hit, index) => { const tag = hit.exact ? 'exact' : 'text' const body = String(hit.text).replace(/\s+/g, ' ').trim().slice(0, 200) return `[${index + 1}] (${tag} ${hit.score.toFixed(2)}) ${body}` }) + const notice = degradationNotice(hits) + + // ★ "no entries" and "nothing to say" are different questions. + // + // A degraded recall with zero lexical hits still owes the model an explanation: + // otherwise silence means "memory has nothing on this", when the truth is + // "memory could not be searched fully". Staying silent there is the false + // negative the notice exists to prevent. + // + // The converse must hold too: a HEALTHY empty recall stays silent. Emitting the + // header on every empty turn would burn tokens on the commonest case and train + // the model to ignore the memory section entirely. + if (lines.length === 0 && notice === '') return '' const header = `[Memory · ${sourceLabel}] Recalled from durable block memory — these are PAST notes, not the current conversation.` - const notice = degradationNotice(hits) return [header, ...(notice === '' ? [] : [notice]), ...lines].join('\n') } @@ -510,6 +533,10 @@ export function degradationNotice(hits) { return '[Memory] Semantic search is off (no vector space configured); matches below are lexical/symbolic only.' case 'embed_error': return '[Memory] Semantic search FAILED for this turn; matches below are lexical/symbolic only. Absence of a memory here does not prove it is unrecorded.' + case 'aborted': + // Deliberate stop, not a fault: say so plainly and do NOT imply the memory + // is incomplete, which would invite a pointless retry. + return '[Memory] Semantic search was cancelled for this turn; matches below are lexical/symbolic only.' case 'vector_ready_unused': // Group A state: the vector was computed but does not take part in scoring // yet. Not something the model needs to hear about — but keeping the case diff --git a/test/phase-b-h.test.js b/test/phase-b-h.test.js index e5559ec..24fedb8 100644 --- a/test/phase-b-h.test.js +++ b/test/phase-b-h.test.js @@ -30,7 +30,7 @@ import { DatabaseSync } from 'node:sqlite' import { createHttpEmbedder, deriveFingerprint, normalizeEndpoint } from '../lib/embed/http.js' import { BlockStore } from '../lib/store.js' import { createUserMessage } from '@deepseek-ai/dsh-llm' -import { recallBlocks, recallBlocksHybrid, renderRecall } from '../lib/recall.js' +import { degradationNotice, recallBlocks, recallBlocksHybrid, renderRecall } from '../lib/recall.js' /** * Run `body` with `globalThis.fetch` replaced by a stub. @@ -1260,7 +1260,7 @@ test('W10b: a signal reaching recallBlocksHybrid directly also aborts', async () }) const elapsed = Date.now() - started assert.ok(elapsed < 500, `must abort promptly; took ${elapsed}ms`) - assert.equal(hits.degraded, 'embed_error') + assert.equal(hits.degraded, 'aborted', 'a cancellation is not a provider failure') assert.ok(hits.length >= 1, 'and still answer from the symbol path') } finally { store.close() @@ -1334,9 +1334,10 @@ test('W11c: an aborted pre-step renders the notice end to end', async () => { assert.ok(elapsed < 500, `the aborted recall must not hang; took ${elapsed}ms`) assert.notEqual(text, '', 'the symbol path still produced content') assert.ok( - text.includes('FAILED'), + /cancelled|FAILED/.test(text), `the notice must reach the model-facing text; got:\n${text}`, ) + assert.ok(text.includes('cancelled'), 'a cancellation says so, not "FAILED"') assert.ok(text.includes('4324'), 'and the hit itself is present') } finally { await done() @@ -1358,3 +1359,148 @@ test('W11†: removing the notice line makes the degradation invisible again', ( assert.ok(!oldShape.includes('FAILED'), 'the old shape carries no warning — so W11 would fail on it') assert.ok(renderRecall(degraded, 'dsh-memory').includes('FAILED'), 'the current shape does') }) + +// ── W11d: degraded AND empty must still say something ──────────────────── +// +// The mixed case: recall was degraded (so absence of evidence is not evidence of +// absence) and the lexical fallback found nothing (so there are no entries). The +// model must be told the search was incomplete — staying silent here is exactly +// the false negative the notice exists to prevent. +// +// ★ Two short-circuits suppress it, and fixing only one changes nothing: +// 1. `contextForStep`: `if (hits.length === 0) return ''` +// 2. `renderRecall`: `if (hits.length === 0) return ''` +// Both must treat "no entries" and "nothing to say" as different questions. Each +// gets its own mutation below, because fixing one and reverting the other is +// invisible to a test that only exercises the fixed path. +// +// Measured behaviour of the four combinations (real adapter, real hooks): +// A degraded + hits -> text, FAILED, entries +// B degraded + no hits -> text, FAILED, NO entries ← this test +// C1 healthy + hits -> text, no FAILED, entries +// C2 healthy + no hits -> EMPTY (correct silence) +test('W11d: degraded + zero hits still injects the notice, with no entries', async () => { + const endpoint = await hangingEndpoint() + try { + const { mem, done } = await makeMem({ + embedder: createHttpEmbedder({ endpoint: endpoint.url, dimension: 3, timeoutMs: 30_000 }), + }) + try { + // One embedded block so the vector path is taken, but its text shares no + // symbol with the query, so the lexical fallback finds nothing. + await mem.commit({ text: 'zzz 完全无关的内容', vector: [1, 2, 3], fingerprint: mem.space().fingerprint }) + + const controller = new AbortController() + setTimeout(() => controller.abort(), 50) + const text = await mem.contextForStep( + [ + createUserMessage({ + content: [{ type: 'text', text: '值班室分机号是多少' }], + source: { kind: 'user' }, + }), + ], + controller.signal, + ) + + assert.notEqual(text, '', 'a degraded empty recall must not be silent') + assert.ok( + /cancelled|FAILED/.test(text), + `the notice must be present; got:\n${text}`, + ) + assert.ok( + !text.includes('[1]'), + `and it must carry no entries; got:\n${text}`, + ) + } finally { + await done() + } + } finally { + await endpoint.close() + } +}) + +test('W11d†a: restoring the renderRecall short-circuit alone breaks it', () => { + // Emulate `if (!Array.isArray(hits) || hits.length === 0) return ''`. + const empty = [] + empty.degraded = 'embed_error' + const withShortCircuit = (hits) => (!Array.isArray(hits) || hits.length === 0 ? '' : renderRecall(hits, 'x')) + assert.equal(withShortCircuit(empty), '', 'the short-circuit really does silence it') + // The correct form asks "entries AND notice both empty?" instead. + const lines = [] + const notice = degradationNotice(empty) + const correct = lines.length === 0 && notice === '' ? '' : 'rendered' + assert.equal(correct, 'rendered', 'so W11d would fail with the short-circuit in place') +}) + +test('W11d†b: restoring the contextForStep short-circuit alone breaks it', () => { + // Emulate `const hits = …; if (hits.length === 0) return ''` in the caller. + const hits = [] + hits.degraded = 'embed_error' + const withCallerShortCircuit = (h) => (h.length === 0 ? '' : renderRecall(h, 'x')) + assert.equal(withCallerShortCircuit(hits), '', 'a caller-side short-circuit silences it too') + + // Even with renderRecall fixed, the caller would have thrown the notice away — + // which is why both places need their own guard. + const notice = degradationNotice(hits) + assert.notEqual(notice, '', 'there WAS something to say; the caller discarded it') +}) + +// ── W12: cancellation and failure must not share a label ───────────────── +// +// Both used to report `embed_error`, so a user-cancelled turn told the model +// "semantic search FAILED", which invites a retry that will also be cancelled. +// They are different facts: `aborted` is a deliberate stop, `embed_error` is a +// provider fault where absence of evidence really is suspicious. +test('W12: an aborted embed reports "aborted"; a failing one reports "embed_error"', async () => { + const endpoint = await hangingEndpoint() + try { + const store = new BlockStore({ path: ':memory:' }).open() + try { + store.putBlock({ text: '值班室分机号 4324' }) + const embedder = createHttpEmbedder({ endpoint: endpoint.url, dimension: 3, timeoutMs: 30_000 }) + + // Cancelled. + const controller = new AbortController() + setTimeout(() => controller.abort(), 50) + const aborted = await recallBlocksHybrid(store, embedder, '4324', { + topK: 3, + signal: controller.signal, + }) + assert.equal(aborted.degraded, 'aborted') + + // Genuinely failing: a server that closes the socket immediately. + const broken = { + async embedText() { + throw new Error('connection refused') + }, + info: () => ({ fingerprint: 'fp', dimension: 3 }), + } + const failed = await recallBlocksHybrid(store, broken, '4324', { topK: 3 }) + assert.equal(failed.degraded, 'embed_error') + + // The two labels must read differently to the model. + assert.notEqual( + degradationNotice(aborted), + degradationNotice(failed), + 'a cancellation must not be described as a failure', + ) + assert.match(degradationNotice(aborted), /cancel/i) + assert.match(degradationNotice(failed), /FAILED/) + } finally { + store.close() + } + } finally { + await endpoint.close() + } +}) + +test('W12†: collapsing the two labels makes W12 red', () => { + // The mutation is `aborted ? 'aborted' : 'embed_error'` -> always 'embed_error'. + const hits = [] + hits.degraded = 'embed_error' // what the collapsed version would report + assert.doesNotMatch( + degradationNotice(hits), + /cancel/i, + 'the collapsed version cannot say "cancelled", which is what W12 asserts', + ) +})