From 09fedcf564d65570563d7469cba533aa3cdb2e2c Mon Sep 17 00:00:00 2001 From: Artificium Date: Tue, 28 Jul 2026 01:22:41 +0000 Subject: [PATCH] =?UTF-8?q?perf:=20fix=20keyword=20search=20returning=20?= =?UTF-8?q?=E2=89=A41=20result=20and=20Redis-absent=20stalls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found by running the corrected benchmark, both real bugs rather than tuning knobs. 1. Keyword search matched almost nothing. queryKeyword used plainto_tsquery, which ANDs every lexeme: "what kind of exercise did I say I do" becomes 'kind' & 'exercis' & 'say', and a single conversation session rarely contains all of them. Measured on the LongMemEval corpus the AND form matched 0 rows where OR matched 16 — keyword mode returned a median of ONE result for any natural-language question, and because hybrid fuses the two arms, hybrid was effectively running semantic-only. Rewriting the operators to OR restores graded matching, with ts_rank_cd doing the discriminating: rows matching more query lexemes, closer together, rank above weak single-term hits. Precision comes from ranking plus LIMIT rather than from refusing to match. The rewrite goes through plainto_tsquery's own output, so stemming, stop-words and escaping remain Postgres's job and no user input reaches query syntax. Stratified 42-question run, Recall@k over sessions: keyword R@3 60.0% → 83.3%, R@5 60.0% → 92.9%, R@10 60.0% → 95.2% Latency cost p50 5ms → 9ms. 2. An absent Redis stalled every cached read. A failed connect costs ~7.5s (connectTimeout plus the reconnect ladder) and the attempt left no memory of itself, so /query and /stats re-paid it on every single request — while the docs advertised Redis as optional with graceful degradation. Adds a circuit breaker: after a failure, skip Redis until a cooldown expires, then allow one probe. Measured on the benchmark server, consecutive queries went 7.6s → 0.016s → 0.007s. Behaviour with Redis present is unchanged. Benchmark methodology fixes found along the way: - LongMemEval-S stores instances in contiguous blocks by question type, so BENCHMARK_LIMIT=50 sampled 50 single-session-user questions and nothing else. Every limited run was category-biased, and tuning retrieval against one would have optimised for whichever category sat at the offset. Adds deterministic stratified sampling (default for partial runs; BENCHMARK_SAMPLE=sequential restores the old slice). - The report generator overwrites RESULTS.md in place, so a 20-question smoke test silently replaced the published page — formatted exactly like an official result. Partial runs now carry a prominent not-an-official-result banner naming the sample size and strategy. Tests: tests/cache-degradation.test.ts covers the no-Redis path, which had none because the existing cache suite exits early when Redis is missing — that gap is why this shipped. No test relaxed or removed; full suite green (integration 24, http 43, security 29, epistemic 33, causal 39, abstractions 39, bootstrap 22, contested 9, metrics 17). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011xCqQo49d3CEbn6oEvb3Ru --- benchmarks/RESULTS.md | 133 ++++++++++++++++--------------- benchmarks/lib/sample.ts | 79 ++++++++++++++++++ benchmarks/longmemeval/ingest.ts | 15 +++- benchmarks/longmemeval/report.ts | 16 +++- package.json | 3 +- src/cache.ts | 37 ++++++++- src/memory-manager.ts | 33 ++++++-- tests/cache-degradation.test.ts | 91 +++++++++++++++++++++ 8 files changed, 330 insertions(+), 77 deletions(-) create mode 100644 benchmarks/lib/sample.ts create mode 100644 tests/cache-degradation.test.ts diff --git a/benchmarks/RESULTS.md b/benchmarks/RESULTS.md index ae99dd3..cac5163 100644 --- a/benchmarks/RESULTS.md +++ b/benchmarks/RESULTS.md @@ -1,103 +1,108 @@ # MemForge Benchmark Results -> # ⚠️ RETRACTED — do not cite the figures below -> -> **Every Recall@k number on this page is invalid.** The scorer that produced -> them called `recallAtK(ids, answers, ids.length)` — passing the candidate -> list's own length as `k`, so the internal `slice(0, k)` never truncated and -> R@1, R@3, R@5 and R@10 were all computed over the *entire* retrieved set. -> -> The inflation is not marginal. Consolidation packs many sessions into each -> warm-tier row, so "the top 5 rows" could hold hundreds of sessions. What was -> published as "93.2% R@5" actually means *"a gold session appeared anywhere -> among all sessions inside the top 5 rows."* That is not LongMemEval's R@5. -> -> Fixed in `benchmarks/lib/metrics.ts`, which now computes two clearly named -> metrics — `recallAtKSessions` (LongMemEval's definition, the comparable one) -> and `recallAtKRows` (MemForge's native row-level behaviour) — plus the -> sessions-per-row packing factor that explains the gap. Regression tests live -> in `tests/benchmark-metrics.test.ts`; the scorer previously had none, which -> is how this shipped. -> -> A corrected full re-run is in progress. Numbers below are retained only as a -> record of what was previously claimed. - -Generated: 2026-04-09 (superseded) +Generated: 2026-07-28 -> **Also note:** these were **retrieval-only** scores, not LongMemEval's -> official end-to-end QA accuracy (retrieve → generate → judge), which is -> typically 20–30 points lower. The QA harness now lives in -> `benchmarks/longmemeval-qa/`. +## LongMemEval — hybrid mode -## [RETRACTED] LongMemEval-S — hybrid mode (retrieval R@5) +> ## ⚠️ Partial run — not an official result +> +> 42 of 500 questions (stratified sample). +> Reported for development iteration only. Publishable figures require a +> full 500-question run; cite nothing from this page until then. -- Questions evaluated: 500 +- Questions evaluated: 42 of 500 (stratified sample) - Consolidation mode: concat -- Timestamp: 2026-04-09T08:05:52.269Z +- Timestamp: 2026-07-28T01:21:09.050Z ### Retrieval Quality -| Metric | Score | -|--------|-------| -| Recall@1 | 81.0% | -| Recall@3 | 90.8% | -| Recall@5 | 93.2% | -| Recall@10 | 96.4% | +> **Recall@k (sessions)** is the comparable metric: a hit means a gold +> session is among the first k distinct sessions by rank — LongMemEval's +> definition. **Recall@k (rows)** counts a hit anywhere inside the top-k +> retrieved rows; because consolidation packs multiple sessions per row it +> is strictly more generous and is NOT comparable to published figures. -**Baselines:** Hippo 74.0% R@5 (BM25 keyword), Zep +18.5% over full-context +| Metric | Sessions (comparable) | Rows (native) | +|--------|----------------------|---------------| +| Recall@1 | 69.0% | 69.0% | +| Recall@3 | 85.7% | 85.7% | +| Recall@5 | 92.9% | 92.9% | +| Recall@10 | 92.9% | 92.9% | + +Sessions packed per retrieved row: **1.0** +(1.0 means rows and sessions are 1:1 and the two columns converge.) + +**Baselines (compare against the Sessions column only):** Hippo 74.0% R@5 +(BM25 keyword), Zep +18.5% over full-context ### Per-Category Breakdown -| Category | Count | R@1 | R@3 | R@5 | R@10 | +| Category | Count | R@1 (sessions) | R@3 (sessions) | R@5 (sessions) | R@10 (sessions) | |----------|-------|------|------|------|------| -| knowledge-update | 78 | 93.6% | 97.4% | 97.4% | 100.0% | -| multi-session | 133 | 86.5% | 94.0% | 96.2% | 98.5% | -| single-session-assistant | 56 | 92.9% | 98.2% | 100.0% | 100.0% | -| single-session-preference | 30 | 43.3% | 66.7% | 80.0% | 93.3% | -| single-session-user | 70 | 74.3% | 84.3% | 87.1% | 90.0% | -| temporal-reasoning | 133 | 75.2% | 89.5% | 91.0% | 94.7% | +| knowledge-update | 7 | 85.7% | 100.0% | 100.0% | 100.0% | +| multi-session | 7 | 85.7% | 85.7% | 100.0% | 100.0% | +| single-session-assistant | 7 | 85.7% | 100.0% | 100.0% | 100.0% | +| single-session-preference | 7 | 42.9% | 71.4% | 85.7% | 85.7% | +| single-session-user | 7 | 57.1% | 71.4% | 85.7% | 85.7% | +| temporal-reasoning | 7 | 57.1% | 85.7% | 85.7% | 85.7% | ### Latency | Operation | p50 | p95 | Mean | |-----------|-----|-----|------| -| Query | 45ms | 77ms | 48ms | -| Ingest (per question) | 23.1s | 35.8s | 23.1s | +| Query | 27ms | 52ms | 31ms | +| Ingest (per question) | 5.2s | 10.7s | 5.4s | ## LongMemEval — keyword mode -- Questions evaluated: 500 +> ## ⚠️ Partial run — not an official result +> +> 42 of 500 questions (stratified sample). +> Reported for development iteration only. Publishable figures require a +> full 500-question run; cite nothing from this page until then. + +- Questions evaluated: 42 of 500 (stratified sample) - Consolidation mode: concat -- Timestamp: 2026-04-09T08:05:52.272Z +- Timestamp: 2026-07-28T01:21:09.051Z ### Retrieval Quality -| Metric | Score | -|--------|-------| -| Recall@1 | 33.4% | -| Recall@3 | 34.6% | -| Recall@5 | 35.0% | -| Recall@10 | 35.0% | +> **Recall@k (sessions)** is the comparable metric: a hit means a gold +> session is among the first k distinct sessions by rank — LongMemEval's +> definition. **Recall@k (rows)** counts a hit anywhere inside the top-k +> retrieved rows; because consolidation packs multiple sessions per row it +> is strictly more generous and is NOT comparable to published figures. + +| Metric | Sessions (comparable) | Rows (native) | +|--------|----------------------|---------------| +| Recall@1 | 69.0% | 69.0% | +| Recall@3 | 83.3% | 83.3% | +| Recall@5 | 92.9% | 92.9% | +| Recall@10 | 95.2% | 95.2% | + +Sessions packed per retrieved row: **1.0** +(1.0 means rows and sessions are 1:1 and the two columns converge.) -**Baselines:** Hippo 74.0% R@5 (BM25 keyword), Zep +18.5% over full-context +**Baselines (compare against the Sessions column only):** Hippo 74.0% R@5 +(BM25 keyword), Zep +18.5% over full-context ### Per-Category Breakdown -| Category | Count | R@1 | R@3 | R@5 | R@10 | +| Category | Count | R@1 (sessions) | R@3 (sessions) | R@5 (sessions) | R@10 (sessions) | |----------|-------|------|------|------|------| -| knowledge-update | 78 | 56.4% | 56.4% | 56.4% | 56.4% | -| multi-session | 133 | 29.3% | 30.1% | 30.1% | 30.1% | -| single-session-assistant | 56 | 19.6% | 23.2% | 25.0% | 25.0% | -| single-session-preference | 30 | 6.7% | 10.0% | 10.0% | 10.0% | -| single-session-user | 70 | 57.1% | 58.6% | 58.6% | 58.6% | -| temporal-reasoning | 133 | 23.3% | 24.1% | 24.8% | 24.8% | +| knowledge-update | 7 | 85.7% | 100.0% | 100.0% | 100.0% | +| multi-session | 7 | 85.7% | 85.7% | 100.0% | 100.0% | +| single-session-assistant | 7 | 57.1% | 100.0% | 100.0% | 100.0% | +| single-session-preference | 7 | 42.9% | 42.9% | 71.4% | 85.7% | +| single-session-user | 7 | 71.4% | 85.7% | 100.0% | 100.0% | +| temporal-reasoning | 7 | 71.4% | 85.7% | 85.7% | 85.7% | ### Latency | Operation | p50 | p95 | Mean | |-----------|-----|-----|------| -| Query | 14ms | 26ms | 14ms | -| Ingest (per question) | 23.1s | 35.8s | 23.1s | +| Query | 9ms | 40ms | 12ms | +| Ingest (per question) | 5.2s | 10.7s | 5.4s | --- diff --git a/benchmarks/lib/sample.ts b/benchmarks/lib/sample.ts new file mode 100644 index 0000000..d551a20 --- /dev/null +++ b/benchmarks/lib/sample.ts @@ -0,0 +1,79 @@ +// Subset selection for limited benchmark runs. +// +// LongMemEval-S stores its 500 instances in contiguous blocks by question +// type: single-session-user ×70, multi-session ×62, single-session-preference +// ×30, multi-session ×71, temporal-reasoning ×133, knowledge-update ×78, +// single-session-assistant ×56. +// +// A plain `slice(offset, offset + limit)` therefore samples one or two +// categories and nothing else — `BENCHMARK_LIMIT=50` returns 50 +// single-session-user questions. Any number produced that way describes one +// question type while looking like an overall score, and tuning retrieval +// against it optimises for whichever category happens to sit at the offset. +// +// Stratified selection round-robins across the types present, so a 50-question +// run covers every category in proportion. It is deterministic — no RNG, and +// original order is preserved within each type — so runs stay comparable +// across code changes, which is the whole point of a regression benchmark. + +export type SampleStrategy = 'stratified' | 'sequential'; + +export function resolveStrategy(raw: string | undefined): SampleStrategy { + return raw === 'sequential' ? 'sequential' : 'stratified'; +} + +/** + * Pick `limit` instances starting at `offset`. + * + * `sequential` reproduces the original slice — use it to re-measure an exact + * historical run, or when evaluating the full dataset where the distinction + * does not apply. + * + * `stratified` (default) takes instances round-robin by `question_type`, which + * for limit >= number-of-types yields a representative mix. Returned items + * carry their original dataset index so results stay traceable. + */ +export function selectSubset( + dataset: T[], + limit: number, + offset: number, + strategy: SampleStrategy, +): Array<{ instance: T; datasetIndex: number }> { + const indexed = dataset.map((instance, datasetIndex) => ({ instance, datasetIndex })); + + if (strategy === 'sequential' || limit >= dataset.length) { + return indexed.slice(offset, offset + limit); + } + + // Group by type, preserving dataset order within each group. + const byType = new Map>(); + for (const entry of indexed) { + const type = entry.instance.question_type ?? 'unknown'; + const bucket = byType.get(type) ?? []; + bucket.push(entry); + byType.set(type, bucket); + } + + // Offset advances the starting cursor within every bucket, so successive + // offsets walk disjoint questions rather than re-drawing the same ones. + const types = [...byType.keys()].sort(); + const cursors = new Map(types.map((t) => [t, offset])); + + const picked: Array<{ instance: T; datasetIndex: number }> = []; + let exhausted = false; + while (picked.length < limit && !exhausted) { + exhausted = true; + for (const type of types) { + if (picked.length >= limit) break; + const bucket = byType.get(type)!; + const cursor = cursors.get(type)!; + if (cursor < bucket.length) { + picked.push(bucket[cursor]!); + cursors.set(type, cursor + 1); + exhausted = false; + } + } + } + + return picked; +} diff --git a/benchmarks/longmemeval/ingest.ts b/benchmarks/longmemeval/ingest.ts index 582c249..8d221d9 100644 --- a/benchmarks/longmemeval/ingest.ts +++ b/benchmarks/longmemeval/ingest.ts @@ -8,6 +8,7 @@ import { join } from 'node:path'; import { performance } from 'node:perf_hooks'; import { loadConfig, type BenchmarkConfig } from '../lib/config.js'; import { createLimiter } from '../lib/concurrency.js'; +import { selectSubset, resolveStrategy } from '../lib/sample.js'; import type { LongMemEvalInstance, IngestManifest } from './types.js'; // Dynamic import to avoid module-level side effects @@ -88,8 +89,16 @@ export async function main(configOverride?: BenchmarkConfig): Promise s.instance); + const typeCounts = questions.reduce>((acc, q) => { + const t = q.question_type ?? 'unknown'; + acc[t] = (acc[t] ?? 0) + 1; + return acc; + }, {}); + console.log(`Loaded ${dataset.length} instances, processing ${questions.length} (sampling: ${strategy})`); + console.log(` type mix: ${Object.entries(typeCounts).map(([t, n]) => `${t}=${n}`).join(', ')}`); // Health check const client = await createClient(config); @@ -107,7 +116,7 @@ export async function main(configOverride?: BenchmarkConfig): Promise { - const questionIndex = config.questionOffset + i; + const questionIndex = selected[i]!.datasetIndex; return limit(async () => { // Retry up to 2 times on transient failures (timeout, connection reset) for (let attempt = 0; attempt < 3; attempt++) { diff --git a/benchmarks/longmemeval/report.ts b/benchmarks/longmemeval/report.ts index cb854e6..fecb464 100644 --- a/benchmarks/longmemeval/report.ts +++ b/benchmarks/longmemeval/report.ts @@ -33,7 +33,21 @@ function generateMarkdown(reports: BenchmarkReport[]): string { for (const report of reports) { lines.push(`## LongMemEval — ${report.queryMode} mode`); lines.push(''); - lines.push(`- Questions evaluated: ${report.questionsEvaluated}`); + // A partial run must never read like the official measurement. The + // generator overwrites RESULTS.md in place, so without this a 20-question + // smoke test silently replaces the published page. + const FULL_DATASET = 500; + const isFullRun = report.questionsEvaluated >= FULL_DATASET; + const sampling = process.env['BENCHMARK_SAMPLE'] === 'sequential' ? 'sequential' : 'stratified'; + if (!isFullRun) { + lines.push(`> ## ⚠️ Partial run — not an official result`); + lines.push('>'); + lines.push(`> ${report.questionsEvaluated} of ${FULL_DATASET} questions (${sampling} sample).`); + lines.push('> Reported for development iteration only. Publishable figures require a'); + lines.push('> full 500-question run; cite nothing from this page until then.'); + lines.push(''); + } + lines.push(`- Questions evaluated: ${report.questionsEvaluated}${isFullRun ? '' : ` of ${FULL_DATASET} (${sampling} sample)`}`); lines.push(`- Consolidation mode: ${report.consolidationMode}`); lines.push(`- Timestamp: ${report.timestamp}`); lines.push(''); diff --git a/package.json b/package.json index 3ae2503..0f786e5 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "test:dreams-compat": "node --import tsx/esm --test tests/dreams-compat.test.ts", "test:dreams-anthropic": "node --import tsx/esm --test tests/dreams-anthropic.test.ts", "test:dreams-bridge": "node --import tsx/esm --test tests/dreams-bridge.test.ts", - "test": "node --import tsx/esm --test --test-concurrency=1 tests/integration.test.ts tests/llm-paths.test.ts tests/http-api.test.ts tests/cache.test.ts tests/embedding-migration.test.ts tests/outcome-revision.test.ts tests/reflection-revision.test.ts tests/selective-forgetting.test.ts tests/multi-device.test.ts tests/dream-runs.test.ts tests/dreams-compat.test.ts tests/dreams-anthropic.test.ts tests/dreams-bridge.test.ts tests/sentiment-tagging.test.ts tests/adaptive-sleep.test.ts tests/epistemic-confidence.test.ts tests/explainable-memory.test.ts tests/causal-graph.test.ts tests/abstractions.test.ts tests/bootstrap.test.ts tests/contested-conflicts.test.ts tests/benchmark-metrics.test.ts", + "test": "node --import tsx/esm --test --test-concurrency=1 tests/integration.test.ts tests/llm-paths.test.ts tests/http-api.test.ts tests/cache.test.ts tests/embedding-migration.test.ts tests/outcome-revision.test.ts tests/reflection-revision.test.ts tests/selective-forgetting.test.ts tests/multi-device.test.ts tests/dream-runs.test.ts tests/dreams-compat.test.ts tests/dreams-anthropic.test.ts tests/dreams-bridge.test.ts tests/sentiment-tagging.test.ts tests/adaptive-sleep.test.ts tests/epistemic-confidence.test.ts tests/explainable-memory.test.ts tests/causal-graph.test.ts tests/abstractions.test.ts tests/bootstrap.test.ts tests/contested-conflicts.test.ts tests/benchmark-metrics.test.ts tests/cache-degradation.test.ts", "test:multi-device": "node --import tsx/esm --test tests/multi-device.test.ts", "test:sentiment-tagging": "node --import tsx/esm --test tests/sentiment-tagging.test.ts", "test:adaptive-sleep": "node --import tsx/esm --test tests/adaptive-sleep.test.ts", @@ -56,6 +56,7 @@ "test:bootstrap": "node --import tsx/esm --test tests/bootstrap.test.ts", "test:contested-conflicts": "node --import tsx/esm --test tests/contested-conflicts.test.ts", "test:benchmark-metrics": "node --import tsx/esm --test tests/benchmark-metrics.test.ts", + "test:cache-degradation": "node --import tsx/esm --test tests/cache-degradation.test.ts", "benchmark:longmemeval": "node --import tsx/esm benchmarks/longmemeval/run.ts", "benchmark:download": "node --import tsx/esm benchmarks/longmemeval/download.ts", "benchmark:ingest": "node --import tsx/esm benchmarks/longmemeval/ingest.ts", diff --git a/src/cache.ts b/src/cache.ts index 3ad77cd..e908104 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -62,11 +62,41 @@ const counters: CacheCounters = { hits: 0, misses: 0, sets: 0, invalidations: 0, let redisClient: RedisClientType | null = null; let connectionPromise: Promise | null = null; +/** + * Circuit breaker for an unreachable Redis. + * + * A failed connect costs ~7.5s: connectTimeout plus the reconnect backoff + * ladder before the client gives up. Without this, that price was paid on + * *every* cache read, because the failed attempt left no memory of itself — + * so a deployment with Redis simply absent (a documented, supported + * configuration) had multi-second latency on every cached endpoint while + * appearing to "degrade gracefully". + * + * After a failure we skip Redis outright until the cooldown expires, then + * allow exactly one probe. Reads fall through to the database, which is the + * intended degraded behaviour — just immediately instead of eventually. + */ +const CONNECT_FAILURE_COOLDOWN_MS = parseInt( + process.env['REDIS_RETRY_COOLDOWN_MS'] ?? '30000', + 10, +); +let connectFailedAt = 0; + +/** Test seam: forget the breaker state so a suite can exercise both paths. */ +export function resetRedisCircuitBreaker(): void { + connectFailedAt = 0; +} + // ─── Connection ─────────────────────────────────────────────────────────────── export async function getRedis(): Promise { if (redisClient?.isOpen) return redisClient; + // Breaker open — fail fast rather than re-paying the connect timeout. + if (connectFailedAt !== 0 && Date.now() - connectFailedAt < CONNECT_FAILURE_COOLDOWN_MS) { + return null; + } + // Coalesce concurrent connection attempts if (connectionPromise) return connectionPromise; @@ -103,9 +133,14 @@ export async function getRedis(): Promise { const safeUrl = url.replace(/:\/\/[^@]*@/, '://*:*@'); log.info({ url: safeUrl }, 'Redis connected'); redisClient = client; + connectFailedAt = 0; // recovered — close the breaker return client; } catch (err) { - log.error({ err }, 'Redis connection failed — operating without cache'); + connectFailedAt = Date.now(); + log.error( + { err, cooldownMs: CONNECT_FAILURE_COOLDOWN_MS }, + 'Redis connection failed — operating without cache; suppressing retries for the cooldown', + ); return null; } finally { connectionPromise = null; diff --git a/src/memory-manager.ts b/src/memory-manager.ts index c2d42b7..06d095e 100644 --- a/src/memory-manager.ts +++ b/src/memory-manager.ts @@ -966,14 +966,33 @@ Ranking (numbers only):`; params.push(limit); const limitIdx = params.length; + // plainto_tsquery ANDs every lexeme, which is the wrong semantics for a + // natural-language question: "what kind of exercise did I do" becomes + // 'kind' & 'exercis' & 'say', and a short memory almost never contains + // all of them. Measured on the LongMemEval corpus, the AND form matched + // 0 rows where the OR form matched 16 — keyword search was returning at + // most one result per query, and hybrid's keyword arm contributed almost + // nothing to the fusion. + // + // Rewriting the operators to OR restores graded matching; ts_rank_cd then + // does the discriminating, ranking rows that match more query lexemes (and + // match them closer together) above weak single-term hits. Precision is + // preserved by ranking plus LIMIT rather than by refusing to match. + // + // The rewrite goes through plainto_tsquery rather than string-building a + // tsquery, so stemming, stop-word removal and escaping stay Postgres's + // job — the input is never interpolated into query syntax. const { rows } = await this.pool.query( - `SELECT id, content, summary, metadata, consolidated_at, time_start, time_end, context_signals, - epistemic_status, evidence_count, - ts_rank_cd(content_tsv, plainto_tsquery('english', $2)) * (0.5 + 0.5 * importance) AS rank - FROM warm_tier - WHERE agent_id = $1 - AND namespace = $3 - AND content_tsv @@ plainto_tsquery('english', $2) + `WITH q AS ( + SELECT replace(plainto_tsquery('english', $2)::text, '&', '|')::tsquery AS tsq + ) + SELECT w.id, w.content, w.summary, w.metadata, w.consolidated_at, w.time_start, w.time_end, + w.context_signals, w.epistemic_status, w.evidence_count, + ts_rank_cd(w.content_tsv, q.tsq) * (0.5 + 0.5 * w.importance) AS rank + FROM warm_tier w, q + WHERE w.agent_id = $1 + AND w.namespace = $3 + AND w.content_tsv @@ q.tsq ${timeFilter} ORDER BY rank DESC LIMIT $${limitIdx}`, diff --git a/tests/cache-degradation.test.ts b/tests/cache-degradation.test.ts new file mode 100644 index 0000000..379eb53 --- /dev/null +++ b/tests/cache-degradation.test.ts @@ -0,0 +1,91 @@ +// MemForge — cache degradation tests (Redis absent) +// +// The existing cache suite exits early when Redis is unreachable, so the +// no-Redis path — a documented, supported deployment — had no coverage at all. +// That is how this shipped: every cached read re-attempted a connection that +// takes ~7.5s to fail (connectTimeout plus the reconnect backoff ladder), so +// "graceful degradation" meant multi-second latency on every /query and +// /stats rather than an immediate fall-through to Postgres. +// +// These tests run ONLY when Redis is genuinely absent; with a live Redis they +// skip, because the breaker they exercise never opens. +// +// Run: node --import tsx/esm --test tests/cache-degradation.test.ts +// Requires: no database, no Redis. + +import { describe, it, before } from 'node:test'; +import assert from 'node:assert/strict'; + +const { getRedis, cacheGet, cacheSet, resetRedisCircuitBreaker } = await import('../src/cache.js'); + +// Point at a port nothing listens on so the outcome does not depend on +// whether the developer happens to be running Redis locally. +process.env['REDIS_URL'] = 'redis://127.0.0.1:6390'; + +let redisAbsent = false; + +before(async () => { + resetRedisCircuitBreaker(); + const client = await getRedis(); + redisAbsent = client === null; +}); + +describe('cache degradation when Redis is unreachable', () => { + it('reports no client rather than throwing', async (t) => { + if (!redisAbsent) return t.skip('Redis reachable — breaker never opens'); + + assert.equal(await getRedis(), null); + }); + + it('short-circuits subsequent connection attempts', async (t) => { + if (!redisAbsent) return t.skip('Redis reachable — breaker never opens'); + + // The first attempt in before() already failed and opened the breaker. + // Every later call must return immediately instead of re-paying the + // ~7.5s connect timeout. The generous bound still fails loudly against + // the pre-fix behaviour, which took seconds per call. + const start = Date.now(); + for (let i = 0; i < 5; i++) { + assert.equal(await getRedis(), null); + } + const elapsed = Date.now() - start; + + assert.ok(elapsed < 500, `5 calls with the breaker open took ${elapsed}ms — expected well under 500ms`); + }); + + it('cacheGet returns a miss immediately', async (t) => { + if (!redisAbsent) return t.skip('Redis reachable — breaker never opens'); + + const start = Date.now(); + const value = await cacheGet('memforge:degradation-probe:stats'); + const elapsed = Date.now() - start; + + assert.equal(value, null, 'a missing cache must read as a miss, not an error'); + assert.ok(elapsed < 500, `cacheGet took ${elapsed}ms with Redis absent`); + }); + + it('cacheSet is a no-op rather than a failure', async (t) => { + if (!redisAbsent) return t.skip('Redis reachable — breaker never opens'); + + const start = Date.now(); + await cacheSet('memforge:degradation-probe:stats', { hot_count: 1 }, 'hot'); + const elapsed = Date.now() - start; + + assert.ok(elapsed < 500, `cacheSet took ${elapsed}ms with Redis absent`); + }); + + it('re-probes after the breaker is reset', async (t) => { + if (!redisAbsent) return t.skip('Redis reachable — breaker never opens'); + + // Resetting simulates the cooldown elapsing: the next call is allowed to + // attempt a real connection again, so a Redis that comes back is picked + // up rather than being suppressed forever. + resetRedisCircuitBreaker(); + const start = Date.now(); + const client = await getRedis(); + const elapsed = Date.now() - start; + + assert.equal(client, null, 'still unreachable in this environment'); + assert.ok(elapsed > 50, `expected a real connection attempt after reset, took only ${elapsed}ms`); + }); +});