feat(memory): opt-in hybrid semantic search - #208
Merged
Conversation
getMemorySemanticEnabled() only ever reads the per-project .ai-devkit.json config; the global config type gained a semantic flag that nothing reads or writes. Config surface should match what's actually wired. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rface - loadLocalEmbedder() had modelsRoot/download options and an offline-inspect-only branch, but its only real caller (getDefaultLocalEmbedder) always passed download:true and no modelsRoot, and nothing else invoked it. Collapsed to a single no-argument path: always resolve the default model directory and ensure the pinned files are present. - storeKnowledgeSemantic/updateKnowledgeSemantic/reembedKnowledge each threaded an unused modelsRoot option through to loadLocalEmbedder; no production caller (api.ts) or test ever passed it. Removed the dead branch, keeping only the `embedder` override used by tests. - LocalEmbedder.dispose() was implemented but never called by any caller (the embedder is process-lifetime in the CLI/MCP server). Removed the unused lifecycle method. - SemanticModelUnavailableError was thrown but never distinguished via instanceof anywhere (catches always fall back to `error.message`). Replaced with a plain Error+cause, since the subclass added no behavior. getSemanticStatus/downloadSemanticModel keep their modelsRoot option: it is exercised by a real test to cover the "model not cached" status without touching the real ~/.ai-devkit/models directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
memoryUpdateCommandAsync was imported and mocked but never exercised by any test (flagged by eslint no-unused-vars), leaving the semantic-enabled update path in registerMemoryCommand untested even though its store/search siblings were covered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… structure The single ad hoc docs/ai/2026-09-01-feature-memory-semantic.md note did not follow the dev-requirements/dev-design/dev-planning/dev-implementation/ dev-testing phase-doc convention (docs/ai/<phase>/<date>-<feature>.md), and several required sections (testable acceptance criteria, the MiniLM-vs-BGE model-selection gate results, task-level planning status, fresh test-run evidence) were thin or missing. Replaced it with the five phase documents, grounded in what actually shipped after the simplification pass in this branch: - requirements: goals/non-goals, acceptance criteria traceable to the gates and tests that exist, and the frozen model-selection gate table (MiniLM pass / BGE fail, with the actual nDCG@5/recall@5 numbers). - design: architecture, data model, API surface, and the design decisions/trade-offs actually reflected in the code (fail-open degradation, RRF fusion, brute-force cosine with a corpus cap). - planning: full task breakdown by milestone, including this simplification pass as its own milestone. - implementation: file-by-file structure notes and an explicit record of what the simplification pass changed and why. - testing: gate evidence plus fresh build/test/lint/e2e output captured after the simplification commits. `ai-devkit lint --feature memory-semantic` passes against the new structure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ecedence
Reinstates GlobalDevKitConfig.memory.semantic, which a prior pass on
this branch removed as dead code. Per user review, the field was
unwired rather than unwanted: a developer who wants semantic search on
for every project should be able to set it once in the global config
(~/.ai-devkit/.ai-devkit.json) instead of repeating
`"memory": { "semantic": true }` in every project's .ai-devkit.json.
getMemorySemanticEnabled() is the single site every semantic-aware
command/server path already calls, so the resolution lives there: an
explicit boolean project value (true or false) wins outright; only
when the project value is absent or non-boolean does it fall back to
an explicit `true` in the global config; otherwise it defaults to
false. No new config surface — same boolean, one more file, resolved
at the existing call site.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Updates the lifecycle docs to reflect the reinstated and wired memory.semantic global fallback: - design: adds an explicit Design Decisions entry for the project > global > default(false) precedence and the rationale (avoid repeating the setting per project), including the note that the earlier removal was a misclassification, not a re-add. - implementation: corrects the file-structure note and simplification record for Config.ts to describe the resolution instead of a removal. - planning: reclassifies the removal task as a new Phase 6 (user review correction) with its own tasks, and updates the progress summary. - testing: adds the three precedence test cases and refreshes the fresh-validation test counts (1,091, up from 1,088). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
memory.semantic: trueEval gates
Expanded-100 model gate against current strict-to-broad FTS:
Built CLI on expanded-100 improved the published 0.57.1 baseline from hit@1 81% / hit@3 91% / hit@5 96% / zero 1% to 88% / 97% / 98% / zero 0%. Judged irrelevant top-three increased from 2.9% to 4.7%, which is a recall/noise trade-off to monitor.
Warm in-process search at 1,000 rows (100 searches after five warmups): median 21.85 ms, p95 29.78 ms, max 33.51 ms.
Packaging
@huggingface/transformers@3.8.1: 705 MB installedonnxruntime-web@1.22.0+@huggingface/tokenizers@0.1.3: 100 MB installed, no native/platform-specific packageXenova/all-MiniLM-L6-v2model: 23 MB lazy download, excluded from install footprintPre-merge simplification pass
Audited the diff against the repo's
simplify-implementationdiscipline (every addition needs a current caller; guards/retries need a demonstrated trigger; delete-before-add) before requesting review:loadLocalEmbedder()'s unusedmodelsRoot/downloadoptions and the unreachable offline-inspect-only branch (its only real caller always passeddownload: true, nomodelsRoot); removed the same deadmodelsRootpass-through fromstoreKnowledgeSemantic/updateKnowledgeSemantic/reembedKnowledge.LocalEmbedder.dispose()— implemented but never invoked by any caller.SemanticModelUnavailableErrorwith a plainError+cause— the subclass was never distinguished viainstanceofanywhere; every catch site already falls back to a generic message.memory updatepath, closing a gap lint had flagged (an unused mock import) and matching the existing store/search coverage.Kept:
getSemanticStatus/downloadSemanticModel'smodelsRootoption (genuinely exercised by a test to isolate from the real model cache directory), theembedderDI seam on every semantic handler (used throughout the test suite), and all RRF fusion / degradation / migration behavior — none of it changed.Reverted on user review: an initial pass here also removed
GlobalDevKitConfig.memory.semanticas dead code (nothing read it). That was a misclassification — the field was unwired, not unwanted. It's reinstated and now wired:memory.semanticresolves with project > global > default(false) precedence at the single site every semantic-aware command/server path already calls (ConfigManager#getMemorySemanticEnabled), so a developer can turn semantic search on once in~/.ai-devkit/.ai-devkit.jsoninstead of repeating it in every project's config. An explicit per-project value (either direction) still overrides the global default. No new config surface — same boolean, one more file, one resolution function. Covered by three new tests (project overrides global, project-unset inherits global, both-unset defaults to false).None of the eval-gate numbers above changed as a result of this pass.
Lifecycle docs
Replaced the single ad hoc
docs/ai/2026-09-01-feature-memory-semantic.mdnote with the five phase documents underdocs/ai/<phase>/2026-09-01-feature-memory-semantic.md, each following thedev-requirements/dev-design/dev-planning/dev-implementation/dev-testingskill structure and grounded in the shipped, simplified state:npx ai-devkit@latest lint --feature memory-semanticpasses against the new structure.Validation
Re-run after the simplification and doc commits:
npm run build— all 6 projectsnpm test— 1,091 passed (was 1,087; +1 for the addedmemory updatesemantic test, +3 for the global/project precedence tests)npm run lint— no errors (3 pre-existing, unrelated warnings)npx vitest run --config e2e/vitest.config.ts— 41 passednpx ai-devkit@latest lint --feature memory-semantic— base structure and all five phase docs passedOperational notes
Semantic search defaults off. Missing model/network, corrupt or stale vectors, and corpora above the 5,000-row brute-force ceiling degrade to lexical search. Rollback is
memory.semantic: false; the additive columns remain inert and row deletion requires no vector-index cleanup.