memory-router: lint --semantic uses the index embed budget and chunks at 64 - #103
Merged
Merged
Conversation
…nking Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
…nment, and cleanup
Fixes reviewer findings on the lint --semantic embed-budget tests:
wrap both existing tests in withEmbedTimeoutEnv(undefined, ...) so an
ambient MEMORY_ROUTER_EMBED_TIMEOUT_MS no longer flips them red; make
the fetch stub return a per-pair one-hot vector instead of an identical
constant so a vectors-to-ids misalignment is actually detectable, and
add an outcome test (N=70, crosses the 64-item chunk boundary) that
pins every pair upgrading to high severity; add a malformed-non-first-
chunk test asserting fail-open to the untouched base report with a
chunk-sized (not total-sized) stderr message; pin the exact 64-id
chunk boundary (one request, no empty trailing batch); and replace the
dead pendingBatchSize placeholder in withCapturedEmbedCalls with a
single lastTimeoutMs, pushing each {timeoutMs, batchSize} entry from
the fetch stub directly.
Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
Pulls in PR #100 (MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS) and #101 ahead of the fe9c61bc fix round, so the README and CHANGELOG doc fixes for this task land on top of current master instead of conflicting with it. Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
…timeout-knob consumer After #100 (MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS), the README's MEMORY_ROUTER_EMBED_TIMEOUT_MS and MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS entries only named the hook's confidence-gate path and index's rebuild path as consumers/non-consumers of the two knobs. lint --semantic's missing-pair embed call (this branch) is a third shared-knob consumer and a second path that never reads the hook-only knob; both entries now name it. Also updates the CHANGELOG bullet's semanticSearch clause to name resolveHookEmbedTimeoutMs, the post-#100 mechanism, instead of the stale DEFAULT_TIMEOUT_MS reference. Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
LanNguyenSi
added a commit
that referenced
this pull request
Aug 16, 2026
…context (#106) * memory-router: enrich lint --semantic chunk-embed errors via describeEmbedError The chunked missing-pair embed call in `lint --semantic`'s conflict checker let a raw fetch/HTTP error through unenriched. Live repro (reviewer finding, PR #103): the operator saw exactly "The operation was aborted due to timeout" on stderr, with no indication of which provider/model/endpoint it was even talking to. rebuildIndex already wrapped its own embedBatch calls with describeEmbedError for this same reason; this gives the chunked lint --semantic path the same treatment by re-exporting describeEmbedError from src/embed/indexer.ts instead of duplicating it. The fail-open (missing provider, exit 0) vs fail-closed (embed error, exit 1) asymmetry noted in the reviewer finding is intentional and is now documented in a code comment in src/lint/conflicts.ts and in the README's --semantic section: a missing provider is a chosen configuration state, while an embed call that errors mid-flight signals a real failure in a provider the operator did configure. New test in tests/lint-conflicts-embed-budget.test.ts pins the enriched error text end-to-end through a mocked AbortSignal.timeout fetch abort. Refs: 372ed7ab-6acc-43fa-bfb2-2d65637552c0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2 * memory-router: fix lint --semantic embed-error hermeticity, README example, and seam misattribution Reviewer-verified fixes on top of describeEmbedError enrichment for lint --semantic's chunked missing-pair embed call: - guard MEMORY_ROUTER_EMBED_PROVIDER (plus the ollama base-url/model knobs for depth) in the openai-pinned enrichment test; an ambient =ollama export previously hijacked cfg to the ollama branch and turned the test red regardless of OPENAI_API_KEY. - correct the README's --semantic embed-error example: buildOllamaConfig always resolves a concrete baseUrl, so the documented "baseUrl=default" ollama shape could never be emitted; replaced it with the exact baseUrl-plus-hint string the code produces and pinned it with a new ollama-branch test, closing the gap where only the openai branch had end-to-end coverage. - stop enriching a caller-supplied embedFn's error with a provider config it never contacted: the seam condition now checks !opts.embedFn before attributing cfg's provider/model/baseUrl, with a test pinning that a throwing custom embedFn passes its error through unchanged. - pin describeEmbedError's re-export from embed/indexer.ts next to the existing EMBED_DIMENSIONS export-contract precedent. - drop an em dash from the new test's comment prose. Refs: 372ed7ab-6acc-43fa-bfb2-2d65637552c0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2 --------- Co-authored-by: Lan Nguyen Si <contact@lan-nguyen-si.de> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LanNguyenSi
added
review:tests-pass
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.
Summary
lint --semantic's missing-pair embed call no longer inherits the 5s hook default and no longer sends all inputs in one request: it now uses the index budget (resolveEmbedTimeoutMs(INDEX_DEFAULT_TIMEOUT_MS), 60s) and chunks at 64, mirroringrebuildIndex. The hook path is untouched.MEMORY_ROUTER_EMBED_TIMEOUT_MS; the embedding-to-id alignment across chunk boundaries is now pinned with distinct one-hot marker vectors and an N=70 outcome assertion (a shifted-alignment mutant previously survived the entire suite); malformed-non-first-chunk fail-open and the exact-64 boundary are pinned; README nameslint --semanticas a third consumer of the shared timeout knob (post-memory-router: dedicated MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS for the semanticSearch path #100); dead test-harness placeholder removed. Branch carries a clean merge of current master.Verification
MEMORY_ROUTER_EMBED_TIMEOUT_MS=12345exported.Out of scope, follow-up to be filed: describeEmbedError parity for the chunk call (a raw abort error currently reaches the CLI without provider/model context).
Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66