diff --git a/docs/frameworks/MEMORY.md b/docs/frameworks/MEMORY.md index 99aed184b3..17e3db7b4c 100644 --- a/docs/frameworks/MEMORY.md +++ b/docs/frameworks/MEMORY.md @@ -59,8 +59,11 @@ infrastructure and settings. Three tiers exist, applied in priority order: ``` ┌─────────────────────────────────────────────────────────────┐ │ TIER 0 — Keyword (FTS5) │ - │ Always available. SQLite FTS5 full-text search over │ - │ content + key. Used when strategy = "exact" or as fallback. │ + │ Probe-driven availability: FTS5 when the SQLite build │ + │ supports it (better-sqlite3 / node:sqlite / bun:sqlite); │ + │ unavailable on FTS5-less builds (e.g. sql.js/WASM — │ + │ "no such module: fts5"). Used when strategy = "exact" or │ + │ as fallback; engine-status keyword reflects the probe. │ └──────────────────────────────────┬──────────────────────────┘ │ strategy = semantic|hybrid? ▼ diff --git a/public/openapi.yaml b/public/openapi.yaml index 8e99fcffb4..7a9f771cb2 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -703,6 +703,9 @@ paths: type: string enum: - FTS5 + - none + reason: + type: string embedding: type: object properties: diff --git a/src/app/(dashboard)/dashboard/memory/components/MemoryEngineStatus.tsx b/src/app/(dashboard)/dashboard/memory/components/MemoryEngineStatus.tsx index ba1b1a052a..9e37cf2aa2 100644 --- a/src/app/(dashboard)/dashboard/memory/components/MemoryEngineStatus.tsx +++ b/src/app/(dashboard)/dashboard/memory/components/MemoryEngineStatus.tsx @@ -59,8 +59,8 @@ export default function MemoryEngineStatus({ status, onConfigure }: Props) { const rows: Array<{ label: string; chip: ChipColor; reason: string; cta?: React.ReactNode }> = [ { label: t("engine.keywordLabel"), - chip: "green", - reason: t("engine.keywordReason"), + chip: status.keyword.available ? "green" : "red", + reason: status.keyword.reason || t("engine.keywordReason"), }, { label: t("engine.embeddingLabel"), @@ -77,12 +77,11 @@ export default function MemoryEngineStatus({ status, onConfigure }: Props) { }, { label: t("engine.vectorStoreLabel"), - chip: - status.vectorStore.available - ? "green" - : status.vectorStore.backend === "none" - ? "gray" - : "red", + chip: status.vectorStore.available + ? "green" + : status.vectorStore.backend === "none" + ? "gray" + : "red", reason: status.vectorStore.reason, cta: status.vectorStore.backend === "none" ? ( diff --git a/src/lib/db/migrationRunner.ts b/src/lib/db/migrationRunner.ts index 2b1b6269e1..498be0f9bc 100644 --- a/src/lib/db/migrationRunner.ts +++ b/src/lib/db/migrationRunner.ts @@ -191,7 +191,7 @@ function isOptionalFts5Migration(migration: { version: string; name: string }): return OPTIONAL_FTS5_MIGRATION_VERSIONS.has(migration.version); } -function supportsFts5(db: SqliteAdapter): boolean { +export function supportsFts5(db: SqliteAdapter): boolean { const cached = fts5SupportCache.get(db); if (cached !== undefined) { return cached; @@ -513,9 +513,8 @@ function isSchemaAlreadyApplied( if (migration.name !== "rename_freepik_to_magnific") return false; if (!hasTable(db, "provider_connections")) return false; return ( - db - .prepare("SELECT 1 FROM provider_connections WHERE provider = 'freepik' LIMIT 1") - .get() == null + db.prepare("SELECT 1 FROM provider_connections WHERE provider = 'freepik' LIMIT 1").get() == + null ); default: return false; diff --git a/src/lib/memory/retrieval.ts b/src/lib/memory/retrieval.ts index b5cf1292c4..a42a591f90 100644 --- a/src/lib/memory/retrieval.ts +++ b/src/lib/memory/retrieval.ts @@ -10,7 +10,15 @@ import { recordMemoryAccess } from "./store"; import { stats as embeddingCacheStats } from "./embedding/cache"; import { getQdrantConfig, checkQdrantHealth, searchSemanticMemory } from "./qdrant"; import type { MemoryEngineStatus } from "@/shared/schemas/memory"; -import { estimateTokens, parseMetadata, rowToMemory, getRelevanceScore } from "./retrieval/scoring"; +import { supportsFts5 } from "../db/migrationRunner"; +import type { SqliteAdapter } from "../db/adapters/types"; +import { + estimateTokens, + parseMetadata, + rowToMemory, + getRelevanceScore, + sanitizeFts5Query, +} from "./retrieval/scoring"; import type { MemoryRow } from "./retrieval/scoring"; const log = logger("MEMORY_RETRIEVAL"); @@ -47,9 +55,7 @@ export interface RetrievePreviewBundle { budgetMaxTokens: number; } -export { estimateTokens } from "./retrieval/scoring"; - -// ──────────────── Helpers ──────────────── +export { estimateTokens, sanitizeFts5Query } from "./retrieval/scoring"; function hasTable(tableName: string): boolean { const db = getDbInstance(); @@ -96,6 +102,8 @@ interface FtsColConfig { */ function buildFtsRows(apiKeyId: string, config: FtsColConfig): MemoryRow[] { if (!config.query) return []; + const safeQuery = sanitizeFts5Query(config.query); + if (!safeQuery) return []; const db = getDbInstance(); const { apiKeyCol, @@ -103,7 +111,6 @@ function buildFtsRows(apiKeyId: string, config: FtsColConfig): MemoryRow[] { createdCol, sessionCol, tableName, - query: q, scope, sessionId, retentionDays, @@ -122,7 +129,7 @@ function buildFtsRows(apiKeyId: string, config: FtsColConfig): MemoryRow[] { } ftsQueryStr += ` ORDER BY f.rank LIMIT 100`; - const ftsParams: unknown[] = [q, apiKeyId]; + const ftsParams: unknown[] = [safeQuery, apiKeyId]; if (scope === "session" && sessionId) ftsParams.push(sessionId); if (retentionDays && retentionDays > 0) { const cutoff = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000).toISOString(); @@ -915,14 +922,17 @@ export async function retrievePreview( // Semantic/hybrid degraded to FTS5 let ftsRows: MemoryRow[] = []; if (query && ftsAvailable) { - const ftsQueryStr = apiKeyId - ? `SELECT m.* FROM ${tableName} m JOIN memory_fts f ON m.memory_id = f.rowid WHERE f.memory_fts MATCH ? AND m.${apiKeyCol} = ? ORDER BY f.rank LIMIT ?` - : `SELECT m.* FROM ${tableName} m JOIN memory_fts f ON m.memory_id = f.rowid WHERE f.memory_fts MATCH ? ORDER BY f.rank LIMIT ?`; - const ftsP: unknown[] = apiKeyId ? [query, apiKeyId, limit] : [query, limit]; - try { - ftsRows = db.prepare(ftsQueryStr).all(...ftsP) as MemoryRow[]; - } catch { - ftsRows = []; + const safeQuery = sanitizeFts5Query(query); + if (safeQuery) { + const ftsQueryStr = apiKeyId + ? `SELECT m.* FROM ${tableName} m JOIN memory_fts f ON m.memory_id = f.rowid WHERE f.memory_fts MATCH ? AND m.${apiKeyCol} = ? ORDER BY f.rank LIMIT ?` + : `SELECT m.* FROM ${tableName} m JOIN memory_fts f ON m.memory_id = f.rowid WHERE f.memory_fts MATCH ? ORDER BY f.rank LIMIT ?`; + const ftsP: unknown[] = apiKeyId ? [safeQuery, apiKeyId, limit] : [safeQuery, limit]; + try { + ftsRows = db.prepare(ftsQueryStr).all(...ftsP) as MemoryRow[]; + } catch { + ftsRows = []; + } } } @@ -958,6 +968,34 @@ export async function retrievePreview( }; } +// ──────────────── keyword engine status (§3.2) ──────────────── + +export interface KeywordEngineStatus { + available: boolean; + backend: "FTS5" | "none"; + reason: string; +} + +/** + * Probe the runtime SQLite build for FTS5 support and report the TRUTH of the + * keyword tier — never a hardcoded claim. `supportsFts5()` reuses the module + * probe already run by migrationRunner (cached per-adapter WeakMap), so an + * FTS5-less build (e.g. sql.js/WASM, "no such module: fts5") surfaces here as + * `available:false` instead of the dashboard claiming FTS5 is always available. + * Any unexpected probe error degrades to unavailable rather than throwing. + */ +export function keywordEngineStatus(db: SqliteAdapter): KeywordEngineStatus { + let available = false; + let reason = "SQLite build lacks FTS5 — keyword search unavailable (fall back to exact scan)"; + try { + available = supportsFts5(db); + if (available) reason = "FTS5 keyword search active"; + } catch (err: unknown) { + reason = `FTS5 probe failed: ${sanitizeErrorMessage(err instanceof Error ? err.message : String(err))}`; + } + return { available, backend: available ? "FTS5" : "none", reason }; +} + // ──────────────── engineStatus (§3.2) ──────────────── /** @@ -1039,7 +1077,7 @@ export async function engineStatus(): Promise { : (settings.rerankProviderModel ?? null); return { - keyword: { available: true, backend: "FTS5" }, + keyword: keywordEngineStatus(getDbInstance()), embedding: { source: resolution.source, model: resolution.model, diff --git a/src/lib/memory/retrieval/scoring.ts b/src/lib/memory/retrieval/scoring.ts index b2a68c2783..4a3ee38d0b 100644 --- a/src/lib/memory/retrieval/scoring.ts +++ b/src/lib/memory/retrieval/scoring.ts @@ -28,6 +28,19 @@ export function estimateTokens(text: string): number { return Math.ceil(text.length / 4); } +/** + * Sanitize query text for SQLite FTS5 MATCH expressions. + * Strips FTS control operators and wraps individual terms in double quotes. + */ +export function sanitizeFts5Query(query?: string): string { + if (!query) return ""; + const cleaned = query.replace(/[^\w\s\u00C0-\u024F\u1E00-\u1EFF]/g, " ").trim(); + if (!cleaned) return ""; + const tokens = cleaned.split(/\s+/).filter(Boolean); + if (tokens.length === 0) return ""; + return tokens.map((t) => `"${t}"`).join(" "); +} + export function parseMetadata(raw: unknown): Record { if (!raw || typeof raw !== "string") return {}; try { diff --git a/src/lib/memory/store.ts b/src/lib/memory/store.ts index 5233c3f5b7..2ddb457d45 100644 --- a/src/lib/memory/store.ts +++ b/src/lib/memory/store.ts @@ -195,6 +195,13 @@ export async function createMemory( existing.id ); + // Self-heal rows created before the insert-time memory_id sync (see the + // INSERT branch below): set memory_id from the rowid when still NULL so the + // FTS JOIN keeps working for legacy rows. No-op for rows already synced. + db.prepare("UPDATE memories SET memory_id = rowid WHERE id = ? AND memory_id IS NULL").run( + existing.id + ); + const updatedMemory: Memory = { id: String(existing.id), apiKeyId: memory.apiKeyId, @@ -269,6 +276,14 @@ export async function createMemory( memory.expiresAt?.toISOString() ?? null ); + // Keep memory_id in sync with the SQLite rowid. Migration 023 made the FTS5 + // external-content trigger key off `memory_id` (JOIN memories.memory_id = + // memory_fts.rowid in retrieval.ts), but a plain INSERT leaves it NULL — the + // trigger then stores an auto-assigned FTS5 rowid and every keyword/hybrid + // search silently returns 0 results. The AFTER UPDATE trigger re-syncs FTS + // when memory_id is set here. + db.prepare("UPDATE memories SET memory_id = rowid WHERE id = ?").run(id); + const createdMemory: Memory = { id, apiKeyId: memory.apiKeyId, diff --git a/src/lib/memory/vectorStore.ts b/src/lib/memory/vectorStore.ts index 6996b84024..6329ef3d48 100644 --- a/src/lib/memory/vectorStore.ts +++ b/src/lib/memory/vectorStore.ts @@ -9,6 +9,7 @@ import { createRequire } from "module"; import type { EmbeddingResolution } from "./embedding/types"; +import { sanitizeFts5Query } from "./retrieval/scoring"; import { getMemoryVecMeta, setMemoryVecMeta, @@ -291,6 +292,14 @@ class VectorStoreImpl implements VectorStore { const k = topK > 0 ? topK : TOP_K_DEFAULT; const rrfK = RRF_K; const q = liveVecQuantization(); + const safeFtsQuery = sanitizeFts5Query(queryText); + const ftsMatchClause = safeFtsQuery ? "fts.memory_fts MATCH ?" : "0 = 1"; + + const params: unknown[] = [encodeVector(vector), { apiKeyId: apiKeyId ?? null }, k]; + if (safeFtsQuery) { + params.push(safeFtsQuery); + } + params.push(k, k); // SQLite does not support FULL OUTER JOIN — use UNION ALL + GROUP BY (RRF recipe). // Reference: https://alexgarcia.xyz/blog/2024/sqlite-vec-hybrid-search/ @@ -312,7 +321,7 @@ class VectorStoreImpl implements VectorStore { fts.rank AS fts_score FROM memory_fts fts JOIN memories m ON m.memory_id = fts.rowid - WHERE fts.memory_fts MATCH ? + WHERE ${ftsMatchClause} AND ($apiKeyId IS NULL OR m.api_key_id = $apiKeyId) LIMIT ? ), @@ -339,7 +348,7 @@ class VectorStoreImpl implements VectorStore { ORDER BY rrf_score DESC LIMIT ?` ) - .all(encodeVector(vector), { apiKeyId: apiKeyId ?? null }, k, queryText, k, k) as Array<{ + .all(...params) as Array<{ memory_id: string; vec_rank: number | null; fts_rank: number | null; diff --git a/src/shared/schemas/memory.ts b/src/shared/schemas/memory.ts index 3302fb1915..d80b34ce10 100644 --- a/src/shared/schemas/memory.ts +++ b/src/shared/schemas/memory.ts @@ -112,8 +112,9 @@ export const EmbeddingProviderListingSchema = z.object({ /** Response shape do GET /api/memory/engine-status (UI Engine tab — D11). */ export const MemoryEngineStatusSchema = z.object({ keyword: z.object({ - available: z.literal(true), - backend: z.literal("FTS5"), + available: z.boolean(), + backend: z.enum(["FTS5", "none"]), + reason: z.string(), }), embedding: z.object({ source: z.enum(["remote", "static", "transformers"]).nullable(), diff --git a/tests/integration/memory-engine-status.test.ts b/tests/integration/memory-engine-status.test.ts index 8bd612a347..2b750b6f67 100644 --- a/tests/integration/memory-engine-status.test.ts +++ b/tests/integration/memory-engine-status.test.ts @@ -60,10 +60,30 @@ test("GET /api/memory/engine-status — 200 + valid MemoryEngineStatusSchema sha const body = await res.json(); - // Validate shape matches MemoryEngineStatusSchema + // Validate shape matches MemoryEngineStatusSchema. Keyword availability is + // probe-driven (runtime FTS5 support), so assert the honest schema contract + // rather than a hardcoded "always available" claim: available is a boolean, + // backend is the FTS5/none enum, and reason is a non-empty string. When the + // probe reports available, the backend must be FTS5. assert.ok(body.keyword, "should have keyword section"); - assert.strictEqual(body.keyword.available, true, "keyword.available should be true"); - assert.strictEqual(body.keyword.backend, "FTS5", "keyword.backend should be FTS5"); + assert.strictEqual( + typeof body.keyword.available, + "boolean", + "keyword.available should be boolean" + ); + assert.ok( + ["FTS5", "none"].includes(body.keyword.backend), + "keyword.backend should be FTS5 or none (probe-driven)" + ); + assert.ok(typeof body.keyword.reason === "string", "keyword.reason should be a string"); + assert.ok(body.keyword.reason.length > 0, "keyword.reason should be non-empty"); + if (body.keyword.available) { + assert.strictEqual( + body.keyword.backend, + "FTS5", + "available keyword tier must report FTS5 backend" + ); + } assert.ok(body.embedding, "should have embedding section"); assert.strictEqual( diff --git a/tests/unit/memory-engine-status.test.ts b/tests/unit/memory-engine-status.test.ts index 936a901e86..5e4d056f36 100644 --- a/tests/unit/memory-engine-status.test.ts +++ b/tests/unit/memory-engine-status.test.ts @@ -21,6 +21,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import Database from "better-sqlite3"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-engine-status-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -61,14 +62,68 @@ test("engineStatus(): output validates against MemoryEngineStatusSchema", async ); }); -test("engineStatus(): keyword section is always available with FTS5 backend", async () => { +test("engineStatus(): keyword section reports FTS5 available on a real build (probe-driven)", async () => { core.getDbInstance(); const { engineStatus } = await import("../../src/lib/memory/retrieval.ts"); const status = await engineStatus(); - assert.equal(status.keyword.available, true, "keyword.available must always be true"); - assert.equal(status.keyword.backend, "FTS5", "keyword.backend must be 'FTS5'"); + // On a real better-sqlite3 build the runtime probe should succeed → keyword available. + assert.equal( + status.keyword.available, + true, + "keyword.available should reflect the runtime FTS5 probe" + ); + assert.equal( + status.keyword.backend, + "FTS5", + "keyword.backend should be FTS5 when the build supports it" + ); + assert.equal(typeof status.keyword.reason, "string", "keyword.reason must be a string"); + assert.ok(status.keyword.reason.length > 0, "keyword.reason must be non-empty"); +}); + +test("keywordEngineStatus(): reports unavailable on an FTS5-less SQLite build (sql.js)", async () => { + // Simulate the sql.js/WASM driver, which is compiled WITHOUT FTS5 — this is + // the exact "no such module: fts5" infra failure that began this investigation. + const raw = new Database(":memory:"); + const sqlJsLike = { + prepare(sql: string) { + return raw.prepare(sql); + }, + exec(sql: string) { + if (/fts5/i.test(sql)) throw new Error("no such module: fts5"); + raw.exec(sql); + }, + pragma(pragmaStr: string) { + return raw.pragma(pragmaStr); + }, + transaction(fn: (...args: unknown[]) => unknown) { + const tx = raw.transaction((...args: unknown[]) => fn(...args)); + return (...args: unknown[]) => tx(...args); + }, + }; + + const { keywordEngineStatus } = await import("../../src/lib/memory/retrieval.ts"); + const status = keywordEngineStatus(sqlJsLike as never); + + assert.equal(status.available, false, "FTS5-less build → keyword tier must report unavailable"); + assert.equal(status.backend, "none", "backend must be 'none' when FTS5 is missing"); + assert.equal(typeof status.reason, "string", "reason must be a string"); + assert.ok( + status.reason.length > 0, + "reason must explain the absence of FTS5 (not a hardcoded lie)" + ); +}); + +test("keywordEngineStatus(): reports available on a real better-sqlite3 build", async () => { + const { keywordEngineStatus } = await import("../../src/lib/memory/retrieval.ts"); + const raw = new Database(":memory:"); + + const status = keywordEngineStatus(raw as never); + + assert.equal(status.available, true, "FTS5-capable build → keyword tier available"); + assert.equal(status.backend, "FTS5", "backend must be 'FTS5' when the probe succeeds"); }); test("engineStatus(): embedding section when no source configured", async () => { diff --git a/tests/unit/memory-retrieval-hybrid.test.ts b/tests/unit/memory-retrieval-hybrid.test.ts index 5614cef312..79accadca9 100644 --- a/tests/unit/memory-retrieval-hybrid.test.ts +++ b/tests/unit/memory-retrieval-hybrid.test.ts @@ -74,7 +74,12 @@ test("retrieveMemories: hybrid strategy with no vec store does NOT throw", async insertMemory(db, "h1", "api-hyb", "Paris is the capital of France."); insertMemory(db, "h2", "api-hyb", "Berlin is the capital of Germany."); - const { retrieveMemories } = await import("../../src/lib/memory/retrieval.ts"); + const { retrieveMemories, sanitizeFts5Query } = await import("../../src/lib/memory/retrieval.ts"); + + assert.equal( + sanitizeFts5Query(" CRITICAL: test query! "), + '"system" "reminder" "CRITICAL" "test" "query" "system" "reminder"' + ); await assert.doesNotReject(async () => { await retrieveMemories("api-hyb", { diff --git a/tests/unit/memory-retrieve-preview.test.ts b/tests/unit/memory-retrieve-preview.test.ts index 14fd82a2ce..54deb2354f 100644 --- a/tests/unit/memory-retrieve-preview.test.ts +++ b/tests/unit/memory-retrieve-preview.test.ts @@ -51,6 +51,11 @@ function insertMemory( `INSERT INTO memories (id, api_key_id, session_id, type, key, content, metadata, created_at, updated_at, expires_at) VALUES (?, ?, ?, 'factual', ?, ?, '{}', datetime('now'), datetime('now'), NULL)` ).run(id, apiKeyId, "", key ?? `key-${id}`, content); + + // Mirror production store.ts: keep memory_id in sync with the SQLite rowid so + // the FTS5 trigger (023) stores a matching rowid and the JOIN used by + // retrieval.ts returns rows. Without this, FTS MATCH finds nothing. + db.prepare("UPDATE memories SET memory_id = rowid WHERE id = ?").run(id); } // ── Tests ───────────────────────────────────────────────────────────────────── @@ -184,6 +189,33 @@ test("retrievePreview: empty DB returns empty items", async () => { assert.equal(bundle.totalTokens, 0); }); +test("retrievePreview: semantic FTS fallback sanitizes adversarial query (no FTS5 syntax error, relevant rows only)", async () => { + const db = core.getDbInstance(); + // Content includes the exact tokens that used to break the raw FTS5 MATCH when + // the preview path bound the user query unsanitized: + // - `<` → `fts5: syntax error near "<"` + // - `CRITICAL` as a bare token → `no such column: CRITICAL` + insertMemory(db, "adv-1", "api-adv", "The server logs CRITICAL errors during a system reminder."); + insertMemory(db, "adv-2", "api-adv", "Unrelated content about baking bread."); + insertMemory(db, "adv-3", "api-adv-b3", "Other tenant memory mentioning CRITICAL systems."); + + const { retrievePreview } = await import("../../src/lib/memory/retrieval.ts"); + + const bundle = await retrievePreview("api-adv", " CRITICAL:", { + strategy: "semantic", + maxTokens: 2000, + limit: 10, + }); + + const ids = bundle.items.map((i) => i.memory.id); + assert.ok( + ids.includes("adv-1"), + "sanitized semantic preview must surface the CRITICAL/system/reminder memory" + ); + assert.ok(!ids.includes("adv-2"), "unrelated memory must not be returned by FTS ranking"); + assert.ok(!ids.includes("adv-3"), "other-tenant memory must not be returned (API-key scoping)"); +}); + test("retrievePreview: totalTokens equals sum of item.tokens", async () => { const db = core.getDbInstance(); insertMemory(db, "tok-1", "api-tok", "Short text."); // ~3 tokens diff --git a/tests/unit/memory-schemas-roundtrip.test.ts b/tests/unit/memory-schemas-roundtrip.test.ts index 29cdfbdb5f..443be72373 100644 --- a/tests/unit/memory-schemas-roundtrip.test.ts +++ b/tests/unit/memory-schemas-roundtrip.test.ts @@ -200,7 +200,7 @@ test("EmbeddingProviderListingSchema: rejects missing required model fields", () test("MemoryEngineStatusSchema: accepts valid fully-populated status", () => { const result = MemoryEngineStatusSchema.safeParse({ - keyword: { available: true, backend: "FTS5" }, + keyword: { available: true, backend: "FTS5", reason: "FTS5 keyword search active" }, embedding: { source: "remote", model: "openai/text-embedding-3-small", @@ -230,7 +230,7 @@ test("MemoryEngineStatusSchema: accepts valid fully-populated status", () => { test("MemoryEngineStatusSchema: rejects wrong literal for keyword.backend", () => { const result = MemoryEngineStatusSchema.safeParse({ - keyword: { available: true, backend: "BM25" }, // wrong backend literal + keyword: { available: true, backend: "BM25", reason: "x" }, // wrong backend literal embedding: { source: null, model: null, @@ -243,12 +243,12 @@ test("MemoryEngineStatusSchema: rejects wrong literal for keyword.backend", () = qdrant: { enabled: false, healthy: null, latencyMs: null, error: null }, rerank: { enabled: false, provider: null, model: null, available: false, reason: "" }, }); - assert.equal(result.success, false, "backend 'BM25' must be rejected (must be literal 'FTS5')"); + assert.equal(result.success, false, "backend 'BM25' must be rejected (must be 'FTS5' or 'none')"); }); test("MemoryEngineStatusSchema: rejects invalid vectorStore backend", () => { const result = MemoryEngineStatusSchema.safeParse({ - keyword: { available: true, backend: "FTS5" }, + keyword: { available: true, backend: "FTS5", reason: "x" }, embedding: { source: null, model: null, diff --git a/tests/unit/memory-store.test.ts b/tests/unit/memory-store.test.ts index b48eb7b3ad..15d1cf74e5 100644 --- a/tests/unit/memory-store.test.ts +++ b/tests/unit/memory-store.test.ts @@ -117,6 +117,40 @@ test("memory store CRUD round-trip persists to the memories table and invalidate assert.equal(await store.deleteMemory(created.id), false); }); +test("createMemory syncs memory_id to the SQLite rowid so FTS keyword search can JOIN (regression: silent 0-result FTS)", async () => { + const created = await store.createMemory({ + apiKeyId: "key-fts", + sessionId: "session-fts", + type: MemoryType.FACTUAL, + key: "incident:server", + content: + "The server logs CRITICAL errors during a system reminder. Compact dashboard preferred.", + }); + + const db = core.getDbInstance(); + const row = db + .prepare("SELECT id, memory_id, rowid FROM memories WHERE id = ?") + .get(created.id) as { id: string; memory_id: number | null; rowid: number } | undefined; + assert.ok(row, "stored memory must exist"); + assert.equal(row.memory_id, row.rowid, "memory_id must be synced to the SQLite rowid on insert"); + + // Sanitize the MATCH query exactly like retrieval.ts does (bare tokens can + // crash FTS5 with syntax errors; quoted tokens are the safe AND form). + const safeQuery = ["server", "CRITICAL", "reminder"].map((t) => `"${t}"`).join(" "); + const hits = db + .prepare( + `SELECT m.id FROM memories m JOIN memory_fts f ON m.memory_id = f.rowid + WHERE f.memory_fts MATCH ? AND m.api_key_id = ? ORDER BY f.rank LIMIT ?` + ) + .all(safeQuery, "key-fts", 10) as Array<{ id: string }>; + assert.ok( + hits.some((h) => h.id === created.id), + "FTS JOIN must return the freshly stored memory (memory_id must match the FTS rowid)" + ); + + assert.equal(await store.deleteMemory(created.id), true); +}); + test("getMemory returns null for invalid identifiers and tolerates malformed metadata rows", async () => { assert.equal(await store.getMemory(""), null); assert.equal(await store.getMemory("missing-id"), null); diff --git a/tests/unit/memory-vectorstore-rrf.test.ts b/tests/unit/memory-vectorstore-rrf.test.ts index 88a3dacd4b..fc08ee83f3 100644 --- a/tests/unit/memory-vectorstore-rrf.test.ts +++ b/tests/unit/memory-vectorstore-rrf.test.ts @@ -250,3 +250,23 @@ test("searchHybrid: apiKeyId filters both vec and FTS results", async (t) => { assert.notEqual(h.memoryId, "mem-key1", "key1 should not appear when filtering for key2"); } }); + +test("searchHybrid: handles control symbols and system reminder tags without FTS5 syntax errors", async (t) => { + const store = getStoreOrSkip(t); + if (!store) return; + + const db = core.getDbInstance(); + await setupTable(store); + + insertMemoryWithFts(db, "mem-tag-1", "key1", "CRITICAL test query memory"); + await store.upsertVector("mem-tag-1", makeVec(1.0, 0.0, 0.0, 0.0)); + + await assert.doesNotReject(async () => { + const hits = await store.searchHybrid( + makeVec(1.0, 0.0, 0.0, 0.0), + " CRITICAL: test query! ", + 10 + ); + assert.ok(Array.isArray(hits)); + }, "should not throw FTS5 syntax error on control symbols or XML-like tags"); +});