fix: provision embedding model in lifecycle + stop rendering RRF scores as percentages (#240)#242
Merged
Merged
Conversation
…es as percentages (#240) Two regressions found while diagnosing a 23-day silent semantic-search outage. 1. Model provisioning (the root cause of #240) The default embedding model swapped nomic-embed-text/768 -> qwen3-embedding:0.6b/1024 in a72fac2 (#107/#160, 2026-06-22), but no lifecycle script ever pulled it. Every install that updated past #160 silently lost semantic search: hybrid degraded to keyword-only, embeddings stayed at 0, and no surface reported a failure. Adds recall_provision_embedding_model() to lib/install-lib.sh, called from install.sh (self-check) and update.sh (step_verify) -- update.sh being the path that actually regressed. The model name is READ FROM src/lib/embeddings.ts, never hardcoded in bash. A duplicated constant is precisely how the last swap drifted out of sync; hardcoding it here would rebuild the same bug for the next swap. Respects the Ed-locked OPTIONAL invariant (src/lib/embedding-marker.ts:10-13): no Ollama -> informational only, exit 0, never gates the install. 2. Score unit bug The `score` field was polymorphic: raw FTS5 bm25 rank on the FTS-only path (negative, unbounded) and RRF on the fused path (0..~0.033). One formatter rendered both as `(score * 100).toFixed(1) + '%'`, producing an observed "-1121.0%" -- and, once semantic recovered, a meaningless "1.6%" against a 3.3% ceiling. - mcp-server.ts: run the FTS-only list through reciprocalRankFusion so `score` carries one unit everywhere. Same ordering, honest unit. - search-fallback.ts: render `rrf=0.0328`, not `3.3%`. RRF is a relative ranking signal, not a confidence. - mcp-server.ts reused formatHybridResults instead of its hand-copied duplicate, so the bug existed in two places and had to be fixed twice (DRY law). Tests: fixture score 0.83 -> 0.0328 (0.83 was never reachable); regression test asserts no '%' is ever rendered and a negative score cannot print "-1121.0". Verified end-to-end on a live install: model pulled, 14,616 embeddings backfilled, vec index rebuilt, semantic backend knn, and the query that returned "No results found" now returns the decision it was looking for. Refs #240, #241, #226, #107, #160.
5 tasks
* origin/main: claude.md is a symlink to agents.md
…g-regressions * origin/main: fix(ci): align the CLAUDE.md contract with the symlink, unbreaking main (#244)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the root cause of #240 and the score-display bug found alongside it.
Found while diagnosing a 23-day silent semantic-search outage on a live install:
embeddings= 0 rows, hybrid search degraded to keyword-only, and a natural-language query returnedNo results foundfor a memory that was sitting in L1 context at every session start.1. Model provisioning — the root cause (#240)
The default embedding model swapped
nomic-embed-text/768 →qwen3-embedding:0.6b/1024 in a72fac2 (#107/#160, 2026-06-22). No lifecycle script ever pulled it.Every install that updated past #160 without manually running
ollama pullsilently lost semantic search. Not a local misconfiguration — a migration that shipped without a migration step.Fix:
recall_provision_embedding_model()inlib/install-lib.sh, called frominstall.sh(self-check) andupdate.sh(step_verify) — update.sh being the path that actually regressed.Two constraints shaped it:
src/lib/embeddings.ts, never hardcoded in bash. A duplicated constant is exactly how the last swap drifted; hardcoding here would rebuild this bug for the next swap. Verified: no model name appears in any lifecycle script outside a historical comment.src/lib/embedding-marker.ts:10-13). No Ollama → informational, exit 0, never gates the install. Both paths exercised:2. The
scorefield was polymorphicscorecarried raw FTS5 bm25 rank on the FTS-only path (negative, unbounded) and RRF on the fused path (0..~0.033). One formatter rendered both as(score * 100).toFixed(1) + '%'.Observed output:
-1121.0%while degraded, then a meaningless1.6%against a 3.3% ceiling once semantic recovered.mcp-server.ts— run the FTS-only list throughreciprocalRankFusionsoscorehas one unit everywhere. Same ordering, honest unit.search-fallback.ts— renderrrf=0.0328, not3.3%. RRF is a relative ranking signal, not a confidence; the%implied a meaning it never had.mcp-server.tsreusedformatHybridResultsinstead of its hand-copied duplicate — the bug existed in two places and had to be fixed twice. One definition now (DRY law).Tests: fixture
score: 0.83→0.0328(0.83 was never a value RRF could produce — the fixture encoded the misconception). New regression test asserts no%is ever rendered and a negative score cannot print-1121.0.Verification
End-to-end on a live install: model pulled → 14,616 embeddings backfilled → vec index rebuilt →
semantic backend: knn. The query that returnedNo results foundnow returns the decision it was looking for.keyword-only: embeddings unavailable; semantic backend: nonehybrid: FTS5 + embeddings; semantic backend: knnbun run lintclean. Full suite: 1236 pass.Not included
repair --executeleaves the vec index empty) — filed separately, needs its own fix.k=60flattening finding added as a comment on feat: query intent classification for hybrid search — auto-derive --bias-type + signal weighting from the query (extends #82) #234. Deliberately not fixed here: it needs a relevance harness, and feat: query intent classification for hybrid search — auto-derive --bias-type + signal weighting from the query (extends #82) #234 is triaged "STEAL LATER".Refs #240, #241, #226, #234, #107, #160.