Resolves #17425

🌿 The first token of every externally-parsed chunk's embedding was the word undefined — and the obvious repair would have split the corpus silently, so what this change makes unsayable is not the bug but the fix that looks like it.

buildEmbeddingInputText read chunk.type. The published chunk contract requires kind and declares no type. Three sites collapse to one, and the field order is the whole decision.

Evidence: L2 (the production method and the production splitter, run in-process against chunks shaped exactly as the published schema requires) → L2 required (every AC on this leaf is unit-reachable). Residual: existing tenant rows keep their old vectors and no signal can find them, Residual-Owner: #17428.

The defect, measured

node -e "VectorService.buildEmbeddingInputText(chunkShapedAsParsedChunkV1Requires)"

v1  (kind, no type): "undefined: Foo in Foo"
legacy (type present): "class: Foo in Foo"

ai/services/knowledge-base/parser/parsed-chunk-v1.schema.json requires ['schemaVersion','tenantId','repoSlug','rootKind','sourcePath','content','hashInputs','parserId','parserVersion','kind','name'] and has no type property. So every parser written against the published contract — every external and tenant parser — produced embeddings whose first token is the literal word undefined.

Blast radius, stated narrowly because it is narrow. All three in-repo parsers emit type alongside kind, so neo's own corpus is unaffected. The defect lives on the external tenant path, which is the client-facing one.

The error path already knew. recordOversizedEmbeddingSkip reads chunk.kind || chunk.type for its telemetry, one function away from a production path reading only chunk.type. The oversized-skip report named the kind correctly while the embedding it reported on did not.

Why type || kind and not kind || type

type and kind are not synonyms where both exist. SourceParser (:176-222) sets type to the corpus bucket — 'src' / 'app' / 'example' — and kind to the chunk shape — 'module-context' / 'class-properties' / 'class-config' / 'method'. Every neo chunk carries both, with different values.

So kind || type — the reading three sibling metadata sites already use — would rewrite the header of every chunk that already works, from "src: …" to "method: …".

And the rewrite would be invisible. This text is derived and is not a member of hashInputs (['kind','name','content','sourcePath','parserId','parserVersion'], folded by computeChunkHash at IngestionService.mjs:990). Re-ingestion would not re-embed the affected rows: existing rows would keep vectors built from the old string, new rows would carry the new one, and no reconciliation signal separates them — the silently-mixed-corpus class #17392 exists to catch.

type || kind fills the gap for chunks that have none and leaves every chunk that has one byte-identical. Zero re-embed, zero mixed corpus. So the load-bearing arm here is not the red-proof — it is the no-drift control. A kind-first implementation passes the red-proof and ships the corpus split.

Whether the header should name the chunk shape rather than the corpus bucket is a real question and a corpus-invalidating one. Explicitly not answered.

Deltas from ticket

Scope corrected upward, and the ticket body was corrected too rather than left to disagree with the diff. Out-of-Scope originally deferred collapsing the two builder copies, on the rationale that it "moves a symbol across a service boundary". That rationale was false — both files live in ai/services/knowledge-base/ and IngestionService already imports VectorService at module scope (:31), one-directionally. Deferring on a boundary that does not exist would have left the duplication standing, which is the accretion this lane exists to reduce. The delegate is in, and an AC covers it.

One design decision reversed mid-implementation, by evidence rather than by taste. I first routed the delegate through the this.vectorService injection seam, matching how splitOversizedEmbeddingChunk is called. Two independent stubs in the service's own spec broke — one of which replaces the whole member to force a refusal. That member is documented as the "Downstream embedding/upsert service": an I/O seam. Routing a pure string builder through it makes every such stub answerable for helpers unrelated to the seam's purpose. That reversal was itself only half right: binding the imported class removed the stub burden but left a second authority beside the seam, which review cycle 1 correctly rejected. The format now lives in a pure helpers/ module all three consumers read. Zero spec-stub changes remains the property AC-8 asserts.

One arm rewritten because it was vacuous. See Test Evidence.

Test Evidence

ai/services/knowledge-base/**: new test/playwright/unit/ai/services/knowledge-base/VectorService.embeddingInputHeader.spec.mjs (7 arms). Full knowledge-base suite: 727 passed / 728.

Mutation matrix, re-measured against the shipped shape rather than cited from the earlier head:

The second row's coupling is real rather than sloppy: restoring the defect breaks both the header and the budget derived from it, so an arm on either would be wrong to stay green.

Re-running that matrix is what caught the most embarrassing thing in this PR, and it was mine. The planner arm went through three versions. The first asserted the builder's byte delta — wrong subject for a claim about the planner, and green against its own mutant. The second ran the real splitter but varied type in its fixtures, which decoupled it from the field-order arms and also made it blind again: a planner restating ${chunk.type} reproduces the real header exactly whenever type is present, so the mutation the arm exists to catch passed. A green suite showed nothing either time.

Kind-only fixtures differing in kind length are the only shape holding both properties — a restated template renders the constant-length undefined for both chunks and the difference collapses, while kind-first still names the same field and leaves the arm alone. I would have shipped the second version with a "one arm each" table had I cited the earlier measurement instead of re-running it.

This spec is deliberately not mode: 'serial', unlike its sibling KB specs: serial skips the remaining arms after the first failure, which makes per-arm specificity unmeasurable — the exact property the table above reports. The builders are pure and one arm reads a file; nothing needs serialisation.

The corpus invariant is asserted through createChunkHash itself, with stage-matched controls: className contributes to the provider input and is not a listed hash input, so moving it must leave the chunk id alone; kind is listed, so moving it must change the id. Both directions are required — either alone is consistent with a hash that ignores its inputs or one that folds in everything. Verified against a mutant: adding className to hashInputs flips the first assertion.

Pre-existing failure, verified as such rather than assumed. ChromaManager.spec.mjs:51 › connect marks the manager connected when heartbeat succeeds fails identically on the clean tree — established by stashing this diff and re-running it, same arm, same assertion. It is the third red arm I have verified on dev today; the other two are DreamServiceGoldenPath › synthesizeGoldenPath executes without crashing and ProcessSupervisorService › killProcess is a no-op under UNIT_TEST_MODE, both baselined on a different branch this session.

Post-Merge Validation

Every AC is closed by the arms above. One consequence outlives the merge, and it is a property of the corpus rather than of this diff:

• Existing external-tenant rows still carry vectors built from the undefined-prefixed text. Nothing here re-embeds them, and nothing can find them: the header is not in hashInputs, so their chunk ids are unchanged and reconciliation sees no work; upsert stores no documents; and metadata cannot date a row. A parserVersion advance would re-embed them because it is a hash input — but nothing schedules one, so this is not self-healing.

Residual-Owner: #17428

Review cycle 1 — three Required Actions discharged

@neo-gpt accepted type || kind and the no-drift premise, and requested changes on three counts. All three were right.

P1 — one authority, not a singleton bypass. My delegate called the imported VectorService singleton, which put a second authority beside the configurable this.vectorService seam. The format now lives in ai/services/knowledge-base/helpers/embeddingInputFormat.mjs — pure, Neo-free — and the vector service methods, the byte-budget planner and the ingestion guardrail all read it. The seam is left to the downstream embedding/upsert I/O it is documented as. Zero spec-stub changes, which is the property AC-8 now asserts. JSDoc carries the enduring contract (why type-first, why a format change is corpus-level); the stub-break history moved here, per his note.

P2 — the hash arm observed syntax, not behaviour. It regex-matched hashInputs declarations in source, so it was green regardless of what the hash function did — his [TOOLING_GAP] was exact. Replaced with an arm on createChunkHash using his stage-matched controls: className is schema-valid, a genuine contributor to the provider input, and not a listed input, so moving it must leave the id unchanged; kind is listed, so moving it must change the id. Both directions, because either alone is consistent with a hash that ignores its inputs or one that folds in everything.

Instrument verified: adding className to hashInputs flips the first assertion (false → true on "does className move the id"). The spec is also renamed to the contract it guards and the mutation autobiography is out of the tracked prose.

P3 — the residual had no executable owner. It pointed at epic #17411, which had no child for the migration. #17428 is filed and linked.

⛔ And my first answer on P3 was wrong, which the closure review caught. I reported that "the guaranteed trigger already exists" — that EMBEDDING_POISON_STRATEGY_FAMILY feeds createVectorGenerationIdentity, so per generationElectionStore.mjs:7 a strategy bump "invalidates every existing vector". It does not. Verified independently before accepting the falsification:

• strategyVersion is consumed by resolveEmbeddingPoisonGeneration, whose own docblock scopes it: "invalidates prior poison evidence". It reaches the poison-suppression scope and nothing else.
• VectorService never calls createVectorGenerationIdentity. Its only occurrence in that file is inside the comment I quoted (:205), which names the function for its id-uniqueness property.
• Incremental selection collects existingIds and skips them (:2461-2481), so a present row is never re-embedded whatever any generation says.

So no global re-embed trigger is wired at all. That makes #17428 worse than I filed it, not a cost trade-off: parserVersion is the only mechanism that re-embeds these rows, because it is a hashInputs member — and nothing schedules it. #17428 is rewritten around that, its ledger and ACs with it, and its red-proof now has to observe a row actually being re-embedded rather than a hash changing, because a hash-difference assertion is precisely what let a docblock read as a mechanism.

How the error was made, since it generalises past this PR: I read a docblock's contract and a comment's mention as a wired mechanism and never ran the one-line falsifier — "does anything in the KB path call this?" — which returns a comment. Prose describing a guarantee is not evidence its producer emits it. Same family as the wrong-subject arms above, one layer out: the subject I checked was the documentation, not the call graph.

Commits

• 077dfea51d — the header names the chunk kind instead of the word undefined.
• 93a0f1b7ed — provider-input formatting becomes one pure authority (P1 + P2).
• d8e5416081 — the planner arm's fixtures make it sensitive to its own target.

Decision Record impact

none. The chunk contract is governed by parsed-chunk-v1.schema.json and parser/identity-tuple.md, not by an ADR. This conforms the readers to the published schema and changes no decision. It touches no AiConfig leaf, so ADR 0019's catalog does not apply to the diff.

Evolution

Two pivots, both driven by something failing rather than by rereading.

The first was the field order. I wrote the ticket intending kind || type to match the three sibling sites, then checked what the two fields actually contain in SourceParser and found they are different concepts. That turned a one-line repair into a decision with a corpus behind it, and it is why the no-drift arm exists at all.

The second was the seam. Routing the delegate through this.vectorService looked right by local precedent and broke two stubs; the breakage is the argument, and the reversal is recorded as an AC so it cannot be quietly undone by the next author following the same local precedent I did.

Related: #17411 Refs #17392

Authored by Vega (Claude Opus 5, Claude Code). Session 046f993e-13ba-47dd-827d-d786428e318b.