mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 02:42:24 +03:00
fix(memory): honest probe-driven FTS5 keyword status + memory_id rowid sync (#12231)
* fix(memory): honest probe-driven FTS5 keyword status + memory_id rowid sync The "no such module: fts5" complaint on FTS5-less runtime builds (sql.js/WASM under a global install) was masked by a hardcoded keyword.available=true in engineStatus and an unsanitized FTS5 MATCH path. Address root cause: - engineStatus(): probe runtime via supportsFts5(db) instead of hardcoding available=true; keywordEngineStatus() reports the true backend (FTS5 vs none) with a reason. Schema, OpenAPI, dashboard chip updated to match. - store.ts: sync memory_id to the SQLite rowid on insert (+ self-heal legacy NULL rows). Migration 023 keys the FTS5 external-content trigger off memory_id, but plain INSERT left it NULL so the JOIN returned 0 rows — keyword/hybrid search silently returned nothing on FTS5-capable builds. - retrieval.ts: apply sanitizeFts5Query() to the preview MATCH path. Tests updated/added across memory-engine-status, memory-retrieve-preview, memory-schemas-roundtrip, memory-store, and the integration engine-status test (dropping the hardcoded "always available" assertion). 66 unit tests pass; lint and typecheck clean. * fix(memory): sanitize FTS5 queries for memory retrieval Prevent SQLite FTS5 syntax errors by sanitizing query terms and replacing FTS control operators with double-quoted tokens.
This commit is contained in:
@@ -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?
|
||||
▼
|
||||
|
||||
@@ -703,6 +703,9 @@ paths:
|
||||
type: string
|
||||
enum:
|
||||
- FTS5
|
||||
- none
|
||||
reason:
|
||||
type: string
|
||||
embedding:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
@@ -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" ? (
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<MemoryEngineStatus> {
|
||||
: (settings.rerankProviderModel ?? null);
|
||||
|
||||
return {
|
||||
keyword: { available: true, backend: "FTS5" },
|
||||
keyword: keywordEngineStatus(getDbInstance()),
|
||||
embedding: {
|
||||
source: resolution.source,
|
||||
model: resolution.model,
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
if (!raw || typeof raw !== "string") return {};
|
||||
try {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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("<system-reminder> CRITICAL: test query! </system-reminder>"),
|
||||
'"system" "reminder" "CRITICAL" "test" "query" "system" "reminder"'
|
||||
);
|
||||
|
||||
await assert.doesNotReject(async () => {
|
||||
await retrieveMemories("api-hyb", {
|
||||
|
||||
@@ -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", "<system-reminder> 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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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),
|
||||
"<system-reminder> CRITICAL: test query! </system-reminder>",
|
||||
10
|
||||
);
|
||||
assert.ok(Array.isArray(hits));
|
||||
}, "should not throw FTS5 syntax error on control symbols or XML-like tags");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user