Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/memory-router/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

## [Unreleased]

### Added

- New `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS` env var (b1bbbf68), a hook-only embed-timeout override that takes precedence over the shared `MEMORY_ROUTER_EMBED_TIMEOUT_MS` added in 0.6.0 (#96). Reviewer follow-up from that PR: `MEMORY_ROUTER_EMBED_TIMEOUT_MS` overrides both the hook's confidence-gate path (5 s default) and `memory-router index`'s rebuild path (60 s default) at once, so a persistent shell-profile export meant to give `index` more headroom also raises the hook's per-prompt budget, which must never block a prompt for long. Precedence for the hook's embed call (`semanticSearch` in `src/embed/indexer.ts`) is now `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS` > `MEMORY_ROUTER_EMBED_TIMEOUT_MS` > the 5 s default; the index-rebuild path (`rebuildIndex`) is unchanged and never reads the new var. Same guard semantics as the existing var (integer, `> 0`, `<= 2147483647`; an unset, empty, non-numeric, zero, negative, fractional, or out-of-range value falls through to the next tier). `src/embed/provider.ts` gained `resolveHookEmbedTimeoutMs()` and a shared `parseTimeoutOverride()` helper factored out of the existing `resolveEmbedTimeoutMs()` (same external behavior, no test changes needed for the pre-existing function). New tests in `tests/unit/embed-provider.test.ts` (unit-level precedence and the same 11-value invalid table as PR #96, doubled: once falling back to the shared knob, once falling back to the 5 s default) and `tests/embed-timeout-budget.test.ts` (end-to-end through `indexer.ts`: hook override affects only `semanticSearch`, precedence over the shared knob, and both invalid-fallback tiers). README "Timeout budgets" section and the `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS` override entry updated with the new precedence order and a shell-profile-safe recipe for widening `index`'s budget without also widening the hook's.

### Changed

- The un-overridden `minSemanticScore` default is now model/provider-conditional instead of a flat 0.5: Ollama with bge-m3 resolves to the calibrated 0.78, any other Ollama model falls back to 0.78 at the provider level, and OpenAI keeps 0.5. An explicit `MEMORY_ROUTER_BLEND_MIN_SEMANTIC` still wins on every path; setting it to `0.5` reproduces the pre-upgrade flat-default behavior exactly, on every provider, for an operator who needs to roll back this change without reverting the release. Model names are normalized for the lookup (trim, lowercase, tag stripped), so `bge-m3:latest` matches the `bge-m3` entry. Upgrade cost, measured on the bge-m3 reference corpus (295 memories, no env overrides, flat-0.5 baseline vs. the new conditional default): precision 0.300 -> 0.250 and recall 0.578 -> 0.484, and `semanticContributedCount` (how many of the golden set's prompts had the semantic signal actually win a slot) 20/20 -> 14/20 — the floor now also screens out some genuine borderline matches along with the noise it was added for, not a free win. In exchange, negative controls go from 0/4 to 4/4 blocked and MRR barely moves, 0.7313 -> 0.7104. This is an independent measurement from the mm-v1-T008 calibration table under "Calibration" below (295 memories and a differently-sized golden set here vs. 289 memories/16 positive prompts there); its post-upgrade P/R/MRR land close to that table's floor-0.78 row by coincidence of the underlying cosine distribution, not because it is the same run, so do not read the two as corroborating each other.
Expand Down
5 changes: 4 additions & 1 deletion packages/memory-router/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ Overrides:
- `OPENAI_BASE_URL`: OpenAI-compatible proxy base URL (OpenAI path only).
- `MEMORY_ROUTER_OLLAMA_BASE_URL`: Ollama base URL, default `http://localhost:11434`. Ollama is queried through its OpenAI-compatible `/v1/embeddings` endpoint, unauthenticated.
- `MEMORY_ROUTER_EMBED_TIMEOUT_MS`: per-request timeout override, applies to both the hook's confidence-gate path (default `5000`) and `memory-router index`'s rebuild path (default `60000`); an unset, empty, non-numeric, zero, negative, fractional, or larger-than-`2147483647` value falls back to that path's own default rather than erroring (the upper bound is Node's 32-bit timer limit; beyond it `AbortSignal.timeout` either throws or silently degrades to a 1 ms timer).
- `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS`: timeout override for every `semanticSearch` query-embedding call, precedence over `MEMORY_ROUTER_EMBED_TIMEOUT_MS` above. That is every caller of `semanticSearch`: the UserPromptSubmit hook, the MCP `memory-search` tool, the eval runner, and the public `semanticSearch` export from `src/index.ts`. Only `memory-router index`'s rebuild path (`rebuildIndex`) never reads it. Same validation as `MEMORY_ROUTER_EMBED_TIMEOUT_MS` (an invalid value falls back to the next tier, not to an error); see "Timeout budgets" below for why this exists and the full precedence order.

Model-variable precedence:

Expand All @@ -496,7 +497,9 @@ Embedding dimensionality is never hardcoded: it's read off the first real embed

Local Ollama setup: `ollama pull nomic-embed-text`, then run `ollama serve` (or use the app) before `memory-router index`/normal hook usage.

Timeout budgets: the hook's confidence-gate embed call defaults to a tight 5 s (it must never block a prompt for long) while `memory-router index`'s rebuild defaults to a much more generous 60 s per batch, because a real 64-input Ollama batch on the mm-v1-T008 reference corpus measured roughly 3.5-10 s warm and 11-17 s for the first batch after a cold model load (reliably the slowest), which used to blow past the old shared 5 s budget and make `index` unusable. `MEMORY_ROUTER_EMBED_TIMEOUT_MS` overrides both defaults at once. Beware what that coupling means: a value exported persistently (shell profile) to give `index` more headroom also raises the hook's per-prompt budget to the same value, and the hook must never block a prompt for long. Prefer the per-invocation form, `MEMORY_ROUTER_EMBED_TIMEOUT_MS=120000 memory-router index <dir>`, which widens the index budget without touching the hook.
Timeout budgets: every `semanticSearch` query-embedding call (the hook's confidence-gate path, the MCP `memory-search` tool, the eval runner) defaults to a tight 5 s (it must never block a prompt for long) while `memory-router index`'s rebuild defaults to a much more generous 60 s per batch, because a real 64-input Ollama batch on the mm-v1-T008 reference corpus measured roughly 3.5-10 s warm and 11-17 s for the first batch after a cold model load (reliably the slowest), which used to blow past the old shared 5 s budget and make `index` unusable. `MEMORY_ROUTER_EMBED_TIMEOUT_MS` overrides both defaults at once, and a value exported persistently (shell profile) to give `index` more headroom also raises the hook's per-prompt budget to the same value, as long as the hook knob below is unset.

`MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS` decouples that coupling for every `semanticSearch` caller (the hook, the MCP `memory-search` tool, the eval runner, and the public `semanticSearch` export): each such embed call resolves its timeout with precedence `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS` (hook-specific, wins if set and valid) > `MEMORY_ROUTER_EMBED_TIMEOUT_MS` (shared, still applies when the hook knob is unset or invalid) > `5000` (the hook default). `memory-router index`'s rebuild path (`rebuildIndex`) never reads the hook knob at all, so it stays governed by `MEMORY_ROUTER_EMBED_TIMEOUT_MS` (then its own `60000` default) exactly as before. A persistent `MEMORY_ROUTER_EMBED_TIMEOUT_MS=120000` export for `index` headroom, combined with `MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS=5000` to pin every `semanticSearch` caller back to its tight default, is now the safe way to widen `index`'s budget in a shell profile without also widening theirs. The per-invocation form, `MEMORY_ROUTER_EMBED_TIMEOUT_MS=120000 memory-router index <dir>`, remains an option too, since a per-invocation env var never reaches the hook process at all.

#### Query-embedding cache

Expand Down
8 changes: 8 additions & 0 deletions packages/memory-router/src/embed/indexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
embedBatch,
resolveProviderConfig,
resolveEmbedTimeoutMs,
resolveHookEmbedTimeoutMs,
INDEX_DEFAULT_TIMEOUT_MS,
} = require('./provider');
const { openIndex } = require('./index-store');
Expand Down Expand Up @@ -248,6 +249,13 @@ async function semanticSearch(
model: cfg.model,
baseUrl: cfg.baseUrl,
inputs: [prompt],
// Hook-specific budget: MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS, then
// the shared MEMORY_ROUTER_EMBED_TIMEOUT_MS, then the tight 5s
// hook default. Decoupled from the index-rebuild call above so a
// persistent shared-var export for `index` headroom can't also
// widen this call's prompt-blocking budget — see
// resolveHookEmbedTimeoutMs in provider.ts.
timeoutMs: resolveHookEmbedTimeoutMs(),
});
} catch (err) {
throw describeEmbedError(err, cfg);
Expand Down
50 changes: 36 additions & 14 deletions packages/memory-router/src/embed/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,44 @@ const DEFAULT_TIMEOUT_MS = 5000;
// case with margin, not just the typical case.
const INDEX_DEFAULT_TIMEOUT_MS = 60_000;

// Env override for both DEFAULT_TIMEOUT_MS and INDEX_DEFAULT_TIMEOUT_MS.
// Mirrors src/gates/confidence.ts's recencyHalfLifeDays guard (a
// duration-shaped value must be strictly positive to mean anything) rather
// than that file's envFloat (which allows 0 for a weight/boost, a shape
// where 0 is a meaningful "off"). Unset, empty, non-numeric, zero, and
// negative all fall back to `fallback` unchanged. The value must also be an
// integer no larger than 2147483647: AbortSignal.timeout throws RangeError
// on fractional or > uint32 delays, and Node's 32-bit timer silently
// overflows anything above 2^31-1 to an effective 1 ms budget, so those
// values would defeat the guard's whole purpose on the hook path.
function resolveEmbedTimeoutMs(fallback: number): number {
const raw = process.env.MEMORY_ROUTER_EMBED_TIMEOUT_MS;
if (raw === undefined || raw.trim() === '') return fallback;
// Shared guard for every timeout env var below. Mirrors
// src/gates/confidence.ts's recencyHalfLifeDays guard (a duration-shaped
// value must be strictly positive to mean anything) rather than that
// file's envFloat (which allows 0 for a weight/boost, a shape where 0 is a
// meaningful "off"). Unset, empty, non-numeric, zero, and negative all
// resolve to `undefined` (caller decides the fallback). The value must
// also be an integer no larger than 2147483647: AbortSignal.timeout throws
// RangeError on fractional or > uint32 delays, and Node's 32-bit timer
// silently overflows anything above 2^31-1 to an effective 1 ms budget, so
// those values would defeat the guard's whole purpose on the hook path.
function parseTimeoutOverride(raw: string | undefined): number | undefined {
if (raw === undefined || raw.trim() === '') return undefined;
const parsed = Number(raw);
return Number.isInteger(parsed) && parsed > 0 && parsed <= 2_147_483_647
? parsed
: fallback;
: undefined;
}

// Env override for both DEFAULT_TIMEOUT_MS and INDEX_DEFAULT_TIMEOUT_MS.
function resolveEmbedTimeoutMs(fallback: number): number {
return parseTimeoutOverride(process.env.MEMORY_ROUTER_EMBED_TIMEOUT_MS) ?? fallback;
}

// Hook-only env override, precedence over the shared
// MEMORY_ROUTER_EMBED_TIMEOUT_MS knob above. b1bbbf68: a persistent
// MEMORY_ROUTER_EMBED_TIMEOUT_MS export (shell profile) meant to give
// `memory-router index` more headroom otherwise also raised the hook's
// per-prompt budget by the same amount, and the hook (UserPromptSubmit)
// must never block a prompt for long — see README "Timeout budgets" for
// the coupling this decouples. Only src/embed/indexer.ts's semanticSearch
// consults this; rebuildIndex (the index-rebuild path) never reads it.
function resolveHookEmbedTimeoutMs(): number {
// Precedence: hook-specific override, then the shared override, then the
// 5s hook default.
return (
parseTimeoutOverride(process.env.MEMORY_ROUTER_HOOK_EMBED_TIMEOUT_MS) ??
resolveEmbedTimeoutMs(DEFAULT_TIMEOUT_MS)
);
}

async function embedBatch(opts: EmbedOptions): Promise<number[][]> {
Expand Down Expand Up @@ -204,6 +225,7 @@ module.exports = {
embedBatch,
resolveProviderConfig,
resolveEmbedTimeoutMs,
resolveHookEmbedTimeoutMs,
DEFAULT_TIMEOUT_MS,
INDEX_DEFAULT_TIMEOUT_MS,
};
Loading
Loading