From 1e7c50c0a8833ddc0bc4d473f4afdd652b1dea97 Mon Sep 17 00:00:00 2001 From: Jinserk Baik Date: Sat, 5 Sep 2026 17:13:28 -0400 Subject: [PATCH 1/2] fix: bound embeddings probe by a configurable timeout `memory_status` reported `Embeddings: ? unknown` on healthy setups where qmd, the models, and semantic search all worked correctly. `probeEmbeddings()` raced `qmd vsearch` against a hardcoded 4000ms timer. That probe is not cheap: it runs LLM query expansion plus an embed and rerank pass. Measured latency on a working install (n=12, warm, idle): min 2.35s median 2.91s max 3.55s That leaves only ~0.45s of headroom. Writes schedule a background `qmd update`/`qmd embed`, which competes for CPU and the embedding model, so the probe crosses 4s exactly when a write just happened - which is when users are most likely to run `memory_status`. Reproduced against real qmd with a concurrent re-index: old hardcoded 4s -> unknown 4005ms new default 15s -> ready 6048ms The `catch` maps any failure to "unknown", so a slow probe was indistinguishable from a broken one and the status output implied the embeddings were at fault when they were fine. Also note `PI_MEMORY_QMD_SEARCH_TIMEOUT_MS` did not apply here: real searches honor it, but the probe's literal `4_000` overrode it, so the knob that looked like it should fix this had no effect. Changes: - add DEFAULT_EMBED_PROBE_TIMEOUT_MS (15s) and getEmbedProbeTimeoutMs(), overridable via PI_MEMORY_EMBED_PROBE_TIMEOUT_MS - pass the probe budget to runQmdSearch via an optional timeout override so an abandoned probe cannot leave a 60s LLM query running - surface the effective timeout in the "unknown" hint and in the memory_status configuration block - document the variable in README.md Considered parsing `qmd status` instead, since it needs no LLM and is ~14x faster (0.21s vs 2.91s). Rejected: its `Vectors: N embedded` count is index-global, not per-collection, so it would report "ready" from another collection's embeddings while pi-memory had none. `qmd status` also has no --json mode, making the parse fragile. Correctness over speed. Tests: 8 new cases covering the default floor, env override, invalid values, ready/missing detection, child-process bounding, and a genuine timeout still yielding "unknown". Verified red/green: reverting only the probe change fails 3 of them. bun test test/unit.test.ts 190 pass, 0 fail (was 182 pass) npm run build clean npm run lint clean No change to on-disk memory formats. qmd invocation is unchanged apart from the child timeout value. Signed-off-by: Jinserk Baik --- README.md | 1 + index.ts | 32 +++++++++++++--- test/unit.test.ts | 94 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 819156e..14811c2 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,7 @@ This ensures in-progress context survives compaction and is visible in the next | `PI_MEMORY_SNAPSHOT` | `stable`, `per-turn` | `stable` | `stable` snapshots memory at checkpoints for KV cache stability; `per-turn` rebuilds every turn (legacy behavior) | | `PI_MEMORY_QMD_UPDATE` | `background`, `manual`, `off` | `background` | Controls automatic `qmd update` + `qmd embed` after writes | | `PI_MEMORY_QMD_SEARCH_TIMEOUT_MS` | positive integer (milliseconds) | `60000` | Sets the timeout for explicit `memory_search` qmd queries | +| `PI_MEMORY_EMBED_PROBE_TIMEOUT_MS` | positive integer (milliseconds) | `15000` | Sets the timeout for the `memory_status` embeddings readiness probe. Raise it on slower machines if the probe reports `unknown` | | `PI_MEMORY_NO_SEARCH` | `1` | unset | Disable selective injection in `per-turn` mode (no effect in `stable` mode) | | `PI_MEMORY_SUMMARIZE_TRANSITIONS` | `1`, `true`, `yes`, `on` | unset | Also write exit summaries during lifecycle transitions (`/reload`, `/new`, `/resume`, `/fork`). By default these transitions skip summaries for speed. | | `PI_MEMORY_EXIT_SUMMARY` | `0`, `off`, `false`, `no` to disable | unset (enabled) | Disable the exit summary on real quit (Ctrl+D, `/quit`, session end). Quitting then does no LLM call and no `qmd update`, so it is instant; explicit `memory_write` during sessions is unaffected. | diff --git a/index.ts b/index.ts index 3ad496d..20593e7 100644 --- a/index.ts +++ b/index.ts @@ -955,6 +955,7 @@ let qmdAvailabilityCheckedAt = 0; const QMD_STATUS_CACHE_TTL_MS = 5 * 60 * 1000; const QMD_STATUS_NEGATIVE_CACHE_TTL_MS = 5 * 1000; const DEFAULT_QMD_SEARCH_TIMEOUT_MS = 60_000; +const DEFAULT_EMBED_PROBE_TIMEOUT_MS = 15_000; const qmdCollectionStatusCache = new Map(); function qmdStatusTtl(positive: boolean): number { @@ -965,6 +966,11 @@ export function getQmdSearchTimeoutMs(env: NodeJS.ProcessEnv = process.env): num const configured = Number(env.PI_MEMORY_QMD_SEARCH_TIMEOUT_MS); return Number.isInteger(configured) && configured > 0 ? configured : DEFAULT_QMD_SEARCH_TIMEOUT_MS; } + +export function getEmbedProbeTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { + const configured = Number(env.PI_MEMORY_EMBED_PROBE_TIMEOUT_MS); + return Number.isInteger(configured) && configured > 0 ? configured : DEFAULT_EMBED_PROBE_TIMEOUT_MS; +} let updateTimer: ReturnType | null = null; let exitSummaryReason: ExitSummaryReason | null = null; let terminalInputUnsubscribe: (() => void) | null = null; @@ -1292,10 +1298,11 @@ export function runQmdSearch( mode: "keyword" | "semantic" | "deep", query: string, limit: number, + timeoutOverrideMs?: number, ): Promise<{ results: QmdSearchResult[]; stderr: string }> { const subcommand = mode === "keyword" ? "search" : mode === "semantic" ? "vsearch" : "query"; const args = [subcommand, "--json", "-c", "pi-memory", "-n", String(limit), query]; - const timeoutMs = getQmdSearchTimeoutMs(); + const timeoutMs = timeoutOverrideMs ?? getQmdSearchTimeoutMs(); return new Promise((resolve, reject) => { execFileFn("qmd", args, { timeout: timeoutMs }, (err, stdout, stderr) => { @@ -1326,18 +1333,27 @@ export function runQmdSearch( /** * Best-effort check of whether vector embeddings are ready for semantic/deep - * search. Bounded by a short timeout because the first semantic query can - * trigger a model download. Returns "unknown" rather than blocking on it. + * search. Bounded by a timeout because the first semantic query can trigger a + * model download. Returns "unknown" rather than blocking on it. * "ready" means a probe query ran without qmd's "need embeddings" warning — * it does not prove the index has content. + * + * The bound must stay well clear of normal `qmd vsearch` latency: the probe + * runs an embed + rerank pass (measured ~2.4-3.6s idle, >4s while a background + * re-index competes for CPU and the embedding model). A tighter bound made + * `memory_status` report "unknown" immediately after a write, which is exactly + * when the index is busy. Override with PI_MEMORY_EMBED_PROBE_TIMEOUT_MS. */ export async function probeEmbeddings(): Promise<"ready" | "missing" | "unknown"> { + const probeTimeoutMs = getEmbedProbeTimeoutMs(); let timer: ReturnType | undefined; try { const { stderr } = await Promise.race([ - runQmdSearch("semantic", "memory", 1), + // Bound the child by the same budget so a probe we abandon does not + // leave a long-running LLM query behind. + runQmdSearch("semantic", "memory", 1, probeTimeoutMs), new Promise((_, reject) => { - timer = setTimeout(() => reject(new Error("timeout")), 4_000); + timer = setTimeout(() => reject(new Error("timeout")), probeTimeoutMs); }), ]); return /need embeddings/i.test(stderr ?? "") ? "missing" : "ready"; @@ -2394,7 +2410,10 @@ export default function (pi: ExtensionAPI) { lines.push(" - Run `qmd embed` once to enable semantic/deep search."); } } else if (embeddings === "unknown") { - lines.push(" - Could not verify within the probe timeout; run a semantic search to confirm."); + lines.push( + ` - Could not verify within the ${getEmbedProbeTimeoutMs() / 1000}s probe timeout; run a semantic search to confirm.`, + " - A background re-index can slow the probe. Raise PI_MEMORY_EMBED_PROBE_TIMEOUT_MS if it persists.", + ); } } else { lines.push(" - Run a `memory_search` (auto-creates it) or `qmd collection add` manually."); @@ -2409,6 +2428,7 @@ export default function (pi: ExtensionAPI) { `- PI_MEMORY_SNAPSHOT: ${getSnapshotMode()}`, `- PI_MEMORY_QMD_UPDATE: ${getQmdUpdateMode()}`, `- PI_MEMORY_QMD_SEARCH_TIMEOUT_MS: ${getQmdSearchTimeoutMs()}`, + `- PI_MEMORY_EMBED_PROBE_TIMEOUT_MS: ${getEmbedProbeTimeoutMs()}`, `- PI_MEMORY_DIR: ${process.env.PI_MEMORY_DIR ? "set" : "default"}`, `- PI_MEMORY_EXIT_SUMMARY: ${isExitSummaryEnabled() ? "enabled" : "disabled"}`, `- PI_MEMORY_EXIT_SUMMARY_MODEL: ${process.env.PI_MEMORY_EXIT_SUMMARY_MODEL?.trim() || "session model"}`, diff --git a/test/unit.test.ts b/test/unit.test.ts index 79865a1..baed819 100644 --- a/test/unit.test.ts +++ b/test/unit.test.ts @@ -31,12 +31,14 @@ import { ensureDirs, ensureQmdEmbed, forgetBlocks, + getEmbedProbeTimeoutMs, getExitSummaryTimeoutMs, getQmdSearchTimeoutMs, isExitSummaryEmpty, isExitSummaryEnabled, nowTimestamp, parseScratchpad, + probeEmbeddings, qmdCollectionInstructions, qmdInstallInstructions, readFileSafe, @@ -2398,3 +2400,95 @@ describe("memory_forget tool", () => { expect(secondRestore.content[0].text).toContain("already restored"); }); }); + +// ========================================================================== +// 13. probeEmbeddings timeout behavior +// ========================================================================== + +describe("probeEmbeddings", () => { + const ENV_KEY = "PI_MEMORY_EMBED_PROBE_TIMEOUT_MS"; + let previous: string | undefined; + + beforeEach(() => { + previous = process.env[ENV_KEY]; + delete process.env[ENV_KEY]; + }); + + afterEach(() => { + _resetExecFileForTest(); + if (previous === undefined) delete process.env[ENV_KEY]; + else process.env[ENV_KEY] = previous; + }); + + test("defaults to a probe timeout with headroom over a contended qmd call", () => { + // Measured cold/contended `qmd vsearch` latency exceeds 4s, so the + // default must leave real headroom instead of racing the common case. + expect(getEmbedProbeTimeoutMs()).toBeGreaterThanOrEqual(15_000); + }); + + test("honors PI_MEMORY_EMBED_PROBE_TIMEOUT_MS override", () => { + process.env[ENV_KEY] = "9000"; + expect(getEmbedProbeTimeoutMs()).toBe(9_000); + }); + + test("ignores invalid PI_MEMORY_EMBED_PROBE_TIMEOUT_MS values", () => { + for (const bad of ["0", "-1", "abc", "1.5"]) { + process.env[ENV_KEY] = bad; + expect(getEmbedProbeTimeoutMs()).toBeGreaterThanOrEqual(15_000); + } + }); + + test("reports ready when qmd answers without an embeddings warning", async () => { + _setExecFileForTest(((_file: string, _args: string[], _opts: any, cb: any) => { + cb(null, "[]", ""); + }) as any); + + expect(await probeEmbeddings()).toBe("ready"); + }); + + test("reports missing when qmd warns that embeddings are needed", async () => { + _setExecFileForTest(((_file: string, _args: string[], _opts: any, cb: any) => { + cb(null, "[]", "warning: need embeddings for vector search"); + }) as any); + + expect(await probeEmbeddings()).toBe("missing"); + }); + + test("survives a slow probe that would trip the old hardcoded 4s race", async () => { + // Regression: a background re-index makes the probe take >4s. The old + // implementation raced a hardcoded 4_000ms timer and returned "unknown". + process.env[ENV_KEY] = "20000"; + _setExecFileForTest(((_file: string, _args: string[], _opts: any, cb: any) => { + setTimeout(() => cb(null, "[]", ""), 4_200); + }) as any); + + expect(await probeEmbeddings()).toBe("ready"); + }, 30_000); + + test("bounds the qmd child process by the probe timeout, not the search timeout", async () => { + // The probe must not leave a 60s LLM query running after it gives up. + process.env[ENV_KEY] = "15000"; + process.env.PI_MEMORY_QMD_SEARCH_TIMEOUT_MS = "60000"; + let observedTimeout: number | undefined; + try { + _setExecFileForTest(((_file: string, _args: string[], opts: any, cb: any) => { + observedTimeout = opts.timeout; + cb(null, "[]", ""); + }) as any); + + await probeEmbeddings(); + expect(observedTimeout).toBe(15_000); + } finally { + delete process.env.PI_MEMORY_QMD_SEARCH_TIMEOUT_MS; + } + }); + + test("still reports unknown when the probe genuinely times out", async () => { + process.env[ENV_KEY] = "150"; + _setExecFileForTest(((_file: string, _args: string[], _opts: any, cb: any) => { + setTimeout(() => cb(null, "[]", ""), 2_000); + }) as any); + + expect(await probeEmbeddings()).toBe("unknown"); + }, 10_000); +}); From 9d4aeb0c001d23181da5547a4e4b2dad10d65d7a Mon Sep 17 00:00:00 2001 From: Jay Zeng Date: Sun, 20 Sep 2026 15:41:12 -0700 Subject: [PATCH 2/2] fix: format resolved probe timeout tests --- test/unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit.test.ts b/test/unit.test.ts index b921319..3e4ff28 100644 --- a/test/unit.test.ts +++ b/test/unit.test.ts @@ -2440,7 +2440,6 @@ describe("memory_forget tool", () => { }); }); - // ========================================================================== // 13. probeEmbeddings timeout behavior // ==========================================================================