Skip to content

fix: search resilience to foreign and corrupt embedding vectors#2

Merged
AmadeusDelfino merged 10 commits into
feat/multilingual-per-section-embeddingsfrom
fix/dimension-mismatch-resilience
Jul 13, 2026
Merged

fix: search resilience to foreign and corrupt embedding vectors#2
AmadeusDelfino merged 10 commits into
feat/multilingual-per-section-embeddingsfrom
fix/dimension-mismatch-resilience

Conversation

@AmadeusDelfino

Copy link
Copy Markdown
Owner

Closes review finding #1 on PR #1 — and the worse silent variant it implied.

Problem

A single .embedding produced by a different model breaks search in two ways:

  • Noisy: a different-dimension vector makes cosineSimilarity throw Vectors must have same length, rejecting the entire search() — good entries disappear and no file is named.
  • Silent (worse): a same-dimension vector from a different model scores as garbage with no error at all (e.g. the old 384-dim default vs the new 384-dim multilingual default).

The startup re-index that should heal this couldn't observe its own failures: the swallowing regen path made count++ fire on silent failures, one bad file aborted the rest of a directory, and a cold-cache model swap always timed out (30s query default vs a ~465MB download).

What changed

  • EmbeddingService.isCompatible(entry) — one shared predicate (version + model) used by both the search guard and the scan's staleness check, so batch item Convert to esm package obra/private-journal-mcp#4 (quantized/prefix in the key) extends both in lockstep.
  • Search, model-identity ceiling: only isCompatible rows are ranked. Foreign/v1 rows are skipped and surfaced once per search: search_journal: skipped N entries (incompatible embedding model); run a re-index. Examples: <on-disk paths>. listRecent stays provenance-blind (chronological browse still shows everything).
  • Search, dimension floor: corrupt vectors (wrong length, null, null section elements, non-array sectionEmbeddings container) are skipped per-vector, never thrown past. The throw in cosineSimilarity stays — it's guarded at the call site.
  • Migration hardening: throwing regen core (regenerateEmbedding) so the scan observes failures, while writeThoughts keeps its non-fatal wrapper; truthful count (only real writes); per-file failure isolation; structurally-corrupt vectors (whole-entry or per-section) trigger regen; the orphan .embedding of an empty source is deleted (unrecoverable state); single generous 120s init attempt with abort-on-failure — no retry, because transformers.js v2.17.2 neither resumes nor dedupes downloads (verified in hub.js).

Design spec and plan are included in the diff under docs/superpowers/{specs,plans}/2026-07-13-dimension-mismatch-*, including the accepted residuals (same-model quantized/prefix divergence stays for item obra#4; same-model wrong-length vectors are floor-skipped but not scan-detectable).

Verification

  • 66/66 tests (14 new), tsc --noEmit clean, eslint clean. TDD red→green per task; suite output pristine (all intentional logs asserted via the global spy).
  • Live end-to-end against the real built server and the real multilingual model:
    • First startup on an unindexed journal: 3 entries embedded, honest count.
    • Half-migrated state (hand-crafted 768-dim e5 entry with its .md made unreadable, a same-model corrupt vector, an empty source with a stale orphan): scan healed the corrupt entry, deleted the orphan, isolated the unreadable file with one Failed to migrate line, reported Generated embeddings for 1 (previously would have claimed 3); a real MCP search_journal call returned the 2 healthy entries (top score 0.898) and logged exactly one skip line naming the foreign file's on-disk path.
    • Cold cache + no network: exactly one embedding model unavailable — aborting re-index line, zero per-file spam, server stayed up.
    • Not exercised with real bits: the 120s timeout against an actual ~1GB cold download (unit-pinned instead).

Design for review-batch item #1 (dimension-mismatch crash). Covers both
the noisy dimension-crash and the silent same-dimension-different-model
mode via a shared model-identity compatibility predicate, a read-only
skip-and-defer search guard, a dimension floor as corruption backstop,
and a hardened startup migration (observable throwing regen, truthful
count, generous init timeout).

Hardened through two adversarial read-only review rounds; residual holes
(quantized/prefix in the key = item obra#4, large-journal startup latency,
concurrent writers) recorded as out-of-scope.
4 TDD tasks: shared isCompatible predicate, dimension floor (never crash),
model-identity ceiling + disk-path skip log, hardened migration scan
(observable throwing regen, truthful count, per-file isolation, generous
init timeout). Derived from the reviewed spec (5c81036).
…/spec

- add spec-required listRecent provenance-blindness test to Task 3
- add corrupt-vector regen test; extend needsRegen to section vectors
- empty-source stale .embedding is now deleted (unrecoverable state)
- fix Task 4 edit boundary (duplicate return risk; no-unreachable is off)
- fix Task 2 import instruction (EmbeddingService lives in src/embeddings)
- restore the fs.readFile spy (jest config has no restoreMocks)
- record branch/PR decision: new branch, PR based on the feat branch
- spec: honest residuals (wrong-length vectors; per-boot empty-file log)
…n/v1 vectors from ranking

Replaces the v1 whole-entry-fallback test added by aa585a2: per decision (ii)
in the design spec, v1 files are foreign by provenance and are skipped from
ranking; the startup scan converts v1 -> v2.
…lation, generous init timeout in migration scan
@AmadeusDelfino
AmadeusDelfino merged commit 506681e into feat/multilingual-per-section-embeddings Jul 13, 2026
AmadeusDelfino added a commit that referenced this pull request Jul 13, 2026
…lience

fix: search resilience to foreign and corrupt embedding vectors
@AmadeusDelfino
AmadeusDelfino deleted the fix/dimension-mismatch-resilience branch July 13, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant