Skip to content

memory-router: dedicated MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS for the semanticSearch path - #100

Merged
LanNguyenSi merged 2 commits into
masterfrom
task/b1bbbf68-hook-embed-timeout-knob
Aug 16, 2026
Merged

memory-router: dedicated MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS for the semanticSearch path#100
LanNguyenSi merged 2 commits into
masterfrom
task/b1bbbf68-hook-embed-timeout-knob

Conversation

@LanNguyenSi

@LanNguyenSi LanNguyenSi commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • New MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS decouples the query-embedding timeout from the shared MEMORY_ROUTER_EMBED_TIMEOUT_MS: precedence hook knob > shared knob > 5000ms default, same integer guard semantics as resolveEmbedTimeoutMs (behavior-preservation verified by the reviewer over 72 old-vs-new cases, zero mismatches).
  • semanticSearch passes the resolved value explicitly; rebuildIndex keeps its 60s index budget and ignores the new knob. src/lint/conflicts.ts untouched (own task qw8-fu-05).
  • Fix round after independent review: the two pre-existing shared-knob tests are now hermetic against an ambient hook knob (the README's own recipe used to turn the suite red locally), plus a hermeticity guard test; README states the real scope (the knob governs every semanticSearch caller: prompt hook, MCP search tool, eval runner; only rebuildIndex excluded) and no longer implies the old coupling was removed; upper-boundary acceptance test for 2147483647.

Verification

  • Suite 648/648, typecheck clean, coverage 98.14/89.79/92.42 vs gate 90/80/80; provider.ts 100/100/100.
  • Hermeticity proof: with MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS=3000 exported the suite was 644 pass / 2 fail before the fix and is 648/648 after (also green with =5000 and unset).
  • Mutation probes: removing the precedence line (3 red), dropping the middle tier (14 red), removing the indexer pass-through (2 red), removing the hermeticity neutralization under an ambient knob (guard red).
  • Independent reviewer subagent pass (accept_with_notes); both MEDIUM notes fixed in the fix-round commit.

Refs: b1bbbf68-a154-4038-bb67-223af80756c1

Lan Nguyen Si and others added 2 commits August 16, 2026 12:59
Lan Nguyen Si @claude
memory-router: hook-only embed-timeout knob, decoupled from the index…
507907e 
… budget

Add MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS: precedence hook knob > shared
MEMORY_ROUTER_EMBED_TIMEOUT_MS knob > 5s default. Only semanticSearch (the
UserPromptSubmit hook path) consults it; rebuildIndex (index-rebuild path)
is unchanged. Fixes the coupling PR #96 left behind, where a persistent
shared-knob export for index headroom also raised the hook's per-prompt
budget.

Refs: b1bbbf68-a154-4038-bb67-223af80756c1
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
Lan Nguyen Si @claude
memory-router: fix hook-embed-timeout knob review findings
efc2e12 
Fix reviewer-verified findings on the b1bbbf68 hook-embed-timeout knob:

- Hermeticity regression: the two pre-existing semanticSearch tests in
  embed-timeout-budget.test.ts (no-override, shared-knob-overrides-both)
  did not neutralize the new MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS knob, so
  an ambient export of that var (which the README's own shell-profile
  recipe recommends) flipped their assertions. Wrap both in
  withHookEmbedTimeoutEnv(undefined, ...) and add a hermeticity guard test
  that simulates an ambient hook-knob value directly and re-checks both
  behaviors still hold once neutralized.
- Docs accuracy: README described the new knob as a "hook-path-only"
  override that "decouples the hook path". It actually governs every
  semanticSearch caller (UserPromptSubmit hook, MCP memory-search tool,
  eval runner, the public semanticSearch export); only rebuildIndex is
  excluded. Corrected both README spots.
- Docs tense: the "since both paths shared that one knob" clause implied
  the coupling no longer applies; reworded to "as long as the hook knob
  below is unset".
- Claim hygiene: verified the CHANGELOG's evidence prose cites no test
  counts that a re-run would contradict; no change needed there.

Also adds a boundary test pinning that 2147483647 (the upper bound) is
accepted by the hook knob, mirroring the already-covered rejected
neighbor 3000000000.

Full suite (648 tests) and coverage green, including with
MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS=3000/5000 exported ambiently.

Refs: b1bbbf68-a154-4038-bb67-223af80756c1
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
@LanNguyenSi
LanNguyenSi merged commit 4c2f886 into master Aug 16, 2026
3 of 4 checks passed
@LanNguyenSi
LanNguyenSi deleted the task/b1bbbf68-hook-embed-timeout-knob branch August 16, 2026 11:26
LanNguyenSi pushed a commit that referenced this pull request Aug 16, 2026
Lan Nguyen Si @claude
Merge origin/master into task/fe9c61bc-lint-semantic-budget
7ba70c2 
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
LanNguyenSi pushed a commit that referenced this pull request Aug 16, 2026
Lan Nguyen Si @claude
memory-router: README/CHANGELOG catch up lint --semantic as a shared-…
7c91238 
…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 LanNguyenSi mentioned this pull request Aug 16, 2026
Merged
LanNguyenSi added a commit that referenced this pull request Aug 16, 2026
@LanNguyenSi Lan Nguyen Si @claude
memory-router: lint --semantic uses the index embed budget and chunks…
2964467 
… at 64 (#103)

* memory-router: lint --semantic uses index embed budget + 64-batch chunking

Refs: fe9c61bc-e430-4f72-9c1f-8a027c5aba66
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2

* memory-router: fix lint-conflicts-embed-budget test hermeticity, alignment, 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

* memory-router: README/CHANGELOG catch up lint --semantic as a shared-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

---------

Co-authored-by: Lan Nguyen Si <contact@lan-nguyen-si.de>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LanNguyenSi pushed a commit that referenced this pull request Aug 16, 2026
Lan Nguyen Si @claude
memory-router: fix floor-drop-hint review findings (openai misfire, u…
21667b6 
…npinned guard, model-name sanitization)

Fix reviewer-verified findings on the d33f968c floor-drop-hint branch:

- MEDIUM: openai resolved through the same 'fallback' provenance as an
  un-calibrated Ollama model, so a healthy openai run (floor 0.5, one
  candidate below it) misfired the hint with misleading calibration
  advice. resolveDefaultMinSemanticScoreDetail's openai/no-config branch
  now returns source 'provider' instead of 'fallback' (0.5 is openai's
  own deliberate default, not a calibration gap); the router gate stays
  `=== 'fallback'` unchanged, so it now naturally excludes openai.
  Updated the openai provenance test, added a router-level test proving
  openai stays silent, and corrected the README paragraph that implied
  the hint was scoped to Ollama fallback models.
- MEDIUM: deleting `semanticCandidateCount > 0 &&` from the guard left
  the suite green. Added two cases (empty semantic result, caught
  search error) proving the guard needs that conjunct, placed before
  the case that trips the once-per-process flag so each proves
  independently.
- MEDIUM: documented the once-per-process guard's practical consequence
  in the README paragraph: the hook is a fresh process per prompt, so
  an affected corpus sees the line every prompt until calibrated or
  overridden; the MCP server/eval runner emit it once per lifetime.
- LOW: sanitize the interpolated model name (trim, strip control
  characters, keep any :tag suffix) before writing to stderr. Pinned
  the exact message for a tag-suffixed, whitespace/control-byte-dirty
  model name.
- LOW: reworded the resolveDefaultMinSemanticScore comment: its only
  remaining callers are the pinned tests in tests/confidence.test.ts,
  no production callsite.
- LOW: moved the CHANGELOG paragraph from Changed to Added, matching
  PR #100's structurally similar addition.

Full suite (631 tests), typecheck, and coverage all pass (98.26% lines
/ 89.82% branches / 92.46% functions, gate 90/80/80).

Refs: d33f968c-f02a-441a-b870-5c844342bf97
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
LanNguyenSi pushed a commit that referenced this pull request Aug 16, 2026
Lan Nguyen Si @claude
Merge origin/master into task/d33f968c-floor-drop-hint
96a5162 
Resolves the CHANGELOG Unreleased/Added conflict by keeping both new
bullets (floor-drop hint from this branch, hook embed-timeout knob from
PR #100). Merged tree verified: typecheck clean, 667/667 tests pass.

Refs: d33f968c-f02a-441a-b870-5c844342bf97
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
@LanNguyenSi Lan Nguyen Si @claude
memory-router: once-per-process stderr hint when an uncalibrated fall…
6b4efda 
…back floor drops all semantic candidates (#104)

* memory-router: once-per-process stderr hint when an uncalibrated floor drops every semantic candidate

PR #97 made the un-overridden MEMORY_ROUTER_BLEND_MIN_SEMANTIC default
model/provider-conditional, but only bge-m3 has a specifically-calibrated
entry. Every other Ollama model falls through to the generic 0.78
provider fallback, calibrated against bge-m3's own cosine band, not
theirs - a model with a systematically lower band can silently lose its
entire semantic path with no signal the operator ever sees.

resolveBlended (src/router.ts) now prints one stderr line, once per
process, when the floor is fallback-sourced AND every semantic candidate
this run scored below it. Never fires on a calibrated map hit (bge-m3),
an explicit MEMORY_ROUTER_BLEND_MIN_SEMANTIC override, or a run where at
least one candidate still passed. loadBlendWeights() gains
minSemanticScoreSource ('env' | 'map' | 'fallback') and
minSemanticScoreModel to carry the provenance; resolveDefaultMinSemanticScore
now delegates to the new resolveDefaultMinSemanticScoreDetail().

Refs: d33f968c-f02a-441a-b870-5c844342bf97
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2

* memory-router: reorder floor-drop-hint tests so each mutation-proves independently

Case 4 ("at least one candidate passes" -> no hint) has to run before
case 1 ("all candidates below a fallback floor" -> exactly one hint,
then none) within this test file: once case 1 trips the module-level
floorDropHintEmitted flag, any later test in the same process can no
longer independently prove its own guard condition even under mutation,
since the already-tripped flag masks it. Verified: reverting
semanticHits.length === 0 to >= 0 now turns both case 4 and case 1 red
(previously, with case 1 first, only case 1 caught it).

Refs: d33f968c-f02a-441a-b870-5c844342bf97
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2

* memory-router: fix floor-drop-hint review findings (openai misfire, unpinned guard, model-name sanitization)

Fix reviewer-verified findings on the d33f968c floor-drop-hint branch:

- MEDIUM: openai resolved through the same 'fallback' provenance as an
  un-calibrated Ollama model, so a healthy openai run (floor 0.5, one
  candidate below it) misfired the hint with misleading calibration
  advice. resolveDefaultMinSemanticScoreDetail's openai/no-config branch
  now returns source 'provider' instead of 'fallback' (0.5 is openai's
  own deliberate default, not a calibration gap); the router gate stays
  `=== 'fallback'` unchanged, so it now naturally excludes openai.
  Updated the openai provenance test, added a router-level test proving
  openai stays silent, and corrected the README paragraph that implied
  the hint was scoped to Ollama fallback models.
- MEDIUM: deleting `semanticCandidateCount > 0 &&` from the guard left
  the suite green. Added two cases (empty semantic result, caught
  search error) proving the guard needs that conjunct, placed before
  the case that trips the once-per-process flag so each proves
  independently.
- MEDIUM: documented the once-per-process guard's practical consequence
  in the README paragraph: the hook is a fresh process per prompt, so
  an affected corpus sees the line every prompt until calibrated or
  overridden; the MCP server/eval runner emit it once per lifetime.
- LOW: sanitize the interpolated model name (trim, strip control
  characters, keep any :tag suffix) before writing to stderr. Pinned
  the exact message for a tag-suffixed, whitespace/control-byte-dirty
  model name.
- LOW: reworded the resolveDefaultMinSemanticScore comment: its only
  remaining callers are the pinned tests in tests/confidence.test.ts,
  no production callsite.
- LOW: moved the CHANGELOG paragraph from Changed to Added, matching
  PR #100's structurally similar addition.

Full suite (631 tests), typecheck, and coverage all pass (98.26% lines
/ 89.82% branches / 92.46% functions, gate 90/80/80).

Refs: d33f968c-f02a-441a-b870-5c844342bf97
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 LanNguyenSi mentioned this pull request Aug 16, 2026
Merged
@LanNguyenSi LanNguyenSi added review:tests-pass merge-approval gate prerequisite review:checklist-complete merge-approval gate prerequisite review:comments-resolved merge-approval gate prerequisite review:scope-matches-task merge-approval gate prerequisite review:evidence-logged merge-approval gate prerequisite labels Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

review:checklist-complete merge-approval gate prerequisite review:comments-resolved merge-approval gate prerequisite review:evidence-logged merge-approval gate prerequisite review:scope-matches-task merge-approval gate prerequisite review:tests-pass merge-approval gate prerequisite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@LanNguyenSi