feat(#53): real local neural embedder via node sidecar (supersedes onnxruntime-web approach) - #69
Merged
Conversation
recall = local now runs genuine offline semantic recall. Root cause of the prior blocker (PR #54): forcing onnxruntime-web in-process gives Bun an empty execution-provider list ("Unsupported device: cpu"), and the transformers web build has no fs under Bun. Only the transformers *node* build + onnxruntime-node under real Node produces vectors — so the Bun CLI shells out to a `node` sidecar (embed-worker.mjs) for embeddings and stays on the fast path. - embed-worker.mjs: node sidecar, stdin texts -> stdout MiniLM vectors, model cached under the guard home, offline after first pull. - recall-local.ts: rewritten to embed via the sidecar; rips out the broken onnxruntime-web override. Degrades to [] / no-embedding on any failure. - Per-backend warn threshold: cosine on short commands lands ~0.3, so the Jaccard-calibrated 0.5 never fires. localRecallWarnThreshold=0.25 (calibration knob documented). runtime uses adapter.warnThreshold ?? global. - Hook wired to async recallAdvisory on the allow path for local/embed, so a paraphrase-level warning fires live where the sync lexical hot path can't. - transformers moved to optionalDependencies; onnxruntime-web dep dropped. - Tests: recall-local.test.ts ported to the Embedder injection API (11 pass); new recall-local-realmodel.test.ts drives the REAL model — a zero-shared-token paraphrase recalls the incident above threshold where lexical scores 0 (2 pass), skips honestly if the model can't load. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Testing the real CLI hook surfaced two issues on the allow path: 1. Double-close crash: close() was called in both the try and the catch, and IncidentStore.close(true) throws on a locked handle — the second throw (from the catch) escaped and killed the hook with exit 1. Enforcement must never crash. Now closes exactly once in `finally` with a swallowed error. 2. Per-hook cost: every allow-path hook opened the incident store before checking the mode. Now gates on the config mode first (one existsSync + small TOML read); lexical/off/no-config returns before any SQLite I/O. Verified end to end through `bun src/cli.ts hook`: local mode + a seeded incident + a zero-shared-token paraphrase prints the recall advisory (exit 0); unrelated commands and the default lexical path stay silent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
PR #69 code remediation complete at Verified:
Merge blocker is external: GitHub Actions jobs never start because account payments failed or Actions spending limit must increase. Latest check annotation says exactly that. PR merge state is |
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
1 participant
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.
Closes #53. Makes
recall = locala real offline semantic embedder — verified against the live model, not a stub.Root cause of the prior blocker (PR #54)
PR #54 forced
onnxruntime-webin-process. Under Bun on Windows that registers zero execution providers (Unsupported device: "cpu"), and the transformers web build has no filesystem under Bun. The only combination that actually produces vectors is the transformers node build + onnxruntime-node under real Node. Reproduced both failures directly.Approach
The Bun CLI shells out to a short-lived
nodesidecar (src/ingest/embed-worker.mjs) for embeddings and stays on its fast path.nodeis present wherevernpxinstalls. Any failure degrades to no-recall — never throws into enforcement.recall-local.tsrewritten to embed via the sidecar; the broken onnxruntime-web override is gone. InjectableEmbedderfor tests.localRecallWarnThreshold = 0.25(documented calibration knob). Runtime usesadapter.warnThreshold ?? global.recallAdvisoryon the allow path for local/embed, gated on config mode before any store I/O — the default lexical path opens no SQLite. Close is crash-proofed (a locked handle can't take down the hook).@huggingface/transformers→optionalDependencies; redundantonnxruntime-webdep dropped.Verification
tests/recall-local.test.tsported to the Embedder API — 11 pass.tests/recall-local-realmodel.test.ts(new) drives the REAL MiniLM: a genuinely reworded command with zero shared tokens recalls the incident above threshold where lexical scores exactly 0; stays silent on unrelated commands. Skips honestly (not green-stub) if the model can't load. 2 pass.bun src/cli.ts hook: local mode + seeded incident + paraphrase prints the advisory (exit 0); unrelated + default-lexical stay silent.Note on overlap
Supersedes the onnxruntime-web core of #54 /
local-embedder-fixes; keeps their storeembeddingcolumn, onboarding gate, and dims/degrade contract. Recommend landing this and closing #54.🤖 Generated with Claude Code