From d755ae82b7bf4d0aa6edc5460d5e203069677865 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 23 Sep 2026 04:01:48 +0300 Subject: [PATCH] fix(graph-context): honest wording for the unconfigured-embedder failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave-6 review (Teacher stream): the injection runner's null-embedder branch failed with 'graph.embeddingModel is null', but the resolver collapses BOTH an unset model AND a set model whose provider key vanished into null — a key race mid-session would have reported a false 'null' setting. The message now names both causes honestly. Pinned by a new failure-mode test (null resolver → honest error, never a phantom query). Review verdicts (no changes needed): the session-deletion guard sits at every chat-merge boundary (per batch, fresh lookup, rides the existing AbortError path — nothing merged, nothing checkpointed, registry cleaned in the finally); persistRunUsage's deleted-chat skip composes with appendSourceUsage's vanished-source no-op so no usage record dangles; notes sources have no deletion surface so the notes path needs no guard. The embedding tail's non-fatal contract holds (failure → recordEmbedFailure → no stage, extractor/auditor usage still persists through the catch). --- src/agent/subagents/graph-context.ts | 4 +++- tests/graph-context-job.test.ts | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/agent/subagents/graph-context.ts b/src/agent/subagents/graph-context.ts index d3cef03..c3a21ca 100644 --- a/src/agent/subagents/graph-context.ts +++ b/src/agent/subagents/graph-context.ts @@ -145,9 +145,11 @@ export async function runGraphContextJob(deps: GraphContextDeps, jobId: string): // BYOK (wave 6): fresh resolution per run; null = honestly off (the // dispatch gate already skips unconfigured sessions — this only fires // on a config change racing a respawn, and the error says exactly why). + // The resolver returns null for BOTH an unset model and a set model + // whose key vanished, so the message names both, never a false "null". const embedder = deps.embedder(); if (!embedder || embedder.model === null) { - return fail("no embedding model configured — injection skipped (graph.embeddingModel is null)"); + return fail("no embedding model available — injection skipped (graph.embeddingModel unset, or its provider key is missing)"); } const topic = topicOf(chat.messages); if (topic === null) return fail("no injectable topic (session past its start)"); diff --git a/tests/graph-context-job.test.ts b/tests/graph-context-job.test.ts index ec99a4e..a11857e 100644 --- a/tests/graph-context-job.test.ts +++ b/tests/graph-context-job.test.ts @@ -216,6 +216,21 @@ describe("runGraphContextJob — failure modes are honest errors, never phantom assert.match(got.error ?? "", /stale/); }); + it("BYOK: a null embedder (unset model or vanished key) fails honestly, never a phantom query", async () => { + const { id: chatId } = newTopicChat("injection unconfigured"); + const job = jobStore.createJob({ kind: "graph-context", chatId, toolCallId: `ctx_${chatId}` }); + trackJob(job.id); + // Wave 6: the resolver collapses BOTH unset-model and missing-key + // into null — the error must name the two honestly, never a false + // "graph.embeddingModel is null" for a key race. + const d = { ...deps(fakeGraphStore({ vectors: { n1: unit(0) } }), fakeEmbedder().embedder, async () => {}), embedder: () => null }; + await runGraphContextJob(d, job.id); + const got = jobStore.getJob(job.id)!; + assert.equal(got.status, "error"); + assert.match(got.error ?? "", /no embedding model available/); + assert.match(got.error ?? "", /unset, or its provider key is missing/); + }); + it("nothing fitting the budget errors rather than delivering an empty block", async () => { // Budget below the framing overhead: nothing can fit, ever. const { id: chatId } = newTopicChat("injection empty selection", { graphContextTokens: 10 });