fix(graph-context): honest wording for the unconfigured-embedder failure
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).
This commit is contained in:
@@ -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)");
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in New Issue
Block a user