diff --git a/.changeset/fts-plain-text.md b/.changeset/fts-plain-text.md new file mode 100644 index 0000000000..54d71fcb81 --- /dev/null +++ b/.changeset/fts-plain-text.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fixes full-text search matching Portable Text's internal JSON instead of just prose. Searches for structural tokens like "normal", "span", or "block" no longer match documents whose visible text doesn't contain them, and search snippets show prose instead of JSON fragments. Existing search indexes are rebuilt automatically by a migration on upgrade — no manual reindex needed. diff --git a/packages/core/src/database/migrations/055_fts_plain_text.ts b/packages/core/src/database/migrations/055_fts_plain_text.ts new file mode 100644 index 0000000000..c2c0ce600e --- /dev/null +++ b/packages/core/src/database/migrations/055_fts_plain_text.ts @@ -0,0 +1,211 @@ +import type { Kysely } from "kysely"; +import { sql } from "kysely"; + +import { isSqlite } from "../dialect-helpers.js"; +import { validateIdentifier } from "../validate.js"; + +/** + * Migration: Rebuild FTS5 indexes as self-contained tables indexing + * extracted Portable Text prose + * + * Background: FTS tables were external-content (`content='ec_'`), + * which forces the index to mirror the raw column values — and Portable + * Text fields store JSON, so the index was polluted with structural tokens + * (`_type`, `span`, style values like `normal`, `_key` ULIDs). Searches for + * those tokens matched nearly every document and snippets showed JSON + * fragments. + * + * The fix rebuilds each search-enabled collection's FTS table as a + * self-contained FTS5 table (no `content=` option) whose Portable Text + * columns hold extracted prose — every JSON string under a `text`, `alt`, + * `caption`, or `code` key — with sync triggers computing the same + * extraction in SQL. Self-contained tables also retire the external-content + * `'delete'` choreography and its corruption modes (see migration 039). + * + * The SQL emitted here MUST stay in lock-step with + * `FTSManager.createTriggers` / `createFtsTable` / `populateFromContent` in + * `src/search/fts-manager.ts`. If those change again, add a new migration + * rather than editing this one — migrations are forward-only. + * + * Postgres: no-op. FTS5 is SQLite-only. + * + * D1: idempotent at the granularity we care about (drop-then-create + + * repopulate with `INSERT OR REPLACE`, so concurrent migrators converge). + * A partial apply that drops the FTS table without recreating it is healed + * by the next `verifyAndRepairIndex` call at runtime. + */ + +interface CollectionRow { + slug: string; + search_config: string | null; +} + +interface FieldRow { + slug: string; + type: string; +} + +export async function up(db: Kysely): Promise { + if (!isSqlite(db)) return; + + const collections = await sql` + SELECT slug, search_config FROM _emdash_collections + WHERE search_config IS NOT NULL + `.execute(db); + + for (const collection of collections.rows) { + if (!isSearchEnabled(collection.search_config)) continue; + + // Defensive re-validation before raw SQL interpolation, mirroring 039. + try { + validateIdentifier(collection.slug, "collection slug"); + } catch (error) { + console.warn( + `[migration 055] skipping FTS rebuild for collection "${collection.slug}": ${ + error instanceof Error ? error.message : String(error) + }`, + ); + continue; + } + + const fields = await getSearchableFields(db, collection.slug); + if (fields.length === 0) continue; + + await rebuildIndex(db, collection.slug, fields); + } +} + +/** + * Forward-only. Down is a no-op: the FTS tables are managed by FTSManager + * at runtime and the self-contained shape remains fully functional for + * older code paths that only MATCH and join on id. + */ +export async function down(_db: Kysely): Promise { + // no-op +} + +function isSearchEnabled(searchConfig: string | null): boolean { + if (!searchConfig) return false; + try { + const parsed: unknown = JSON.parse(searchConfig); + return ( + typeof parsed === "object" && + parsed !== null && + "enabled" in parsed && + parsed.enabled === true + ); + } catch { + return false; + } +} + +async function getSearchableFields( + db: Kysely, + collectionSlug: string, +): Promise { + const rows = await sql` + SELECT f.slug, f.type FROM _emdash_fields f + INNER JOIN _emdash_collections c ON c.id = f.collection_id + WHERE c.slug = ${collectionSlug} AND f.searchable = 1 + `.execute(db); + + const out: FieldRow[] = []; + for (const row of rows.rows) { + try { + validateIdentifier(row.slug, "searchable field name"); + out.push(row); + } catch { + console.warn( + `[migration 055] skipping invalid searchable field "${row.slug}" on collection "${collectionSlug}"`, + ); + } + } + return out; +} + +/** Indexed-value expression for one field; lock-step with FTSManager.searchValueExpr. */ +function searchValueExpr(ref: string, fieldType: string): string { + if (fieldType !== "portableText") return ref; + return ( + `CASE WHEN ${ref} IS NULL THEN NULL ` + + `WHEN json_valid(${ref}) THEN (` + + `SELECT group_concat(j.value, ' ') FROM json_tree(${ref}) AS j ` + + `WHERE j.key IN ('text', 'alt', 'caption', 'code') AND j.type = 'text') ` + + `ELSE ${ref} END` + ); +} + +async function rebuildIndex( + db: Kysely, + collectionSlug: string, + fields: FieldRow[], +): Promise { + const ftsTable = `_emdash_fts_${collectionSlug}`; + const contentTable = `ec_${collectionSlug}`; + const slugs = fields.map((f) => f.slug); + const columnList = ["id UNINDEXED", "locale UNINDEXED", ...slugs].join(", "); + const fieldList = slugs.join(", "); + const newValueList = fields.map((f) => searchValueExpr(`NEW.${f.slug}`, f.type)).join(", "); + // Table-qualified: a bare column reference inside the json_tree extraction + // subquery binds to json_tree's own key/value/type/... columns. + const selectValueList = fields + .map((f) => searchValueExpr(`"${contentTable}"."${f.slug}"`, f.type)) + .join(", "); + + await sql.raw(`DROP TRIGGER IF EXISTS "${ftsTable}_insert"`).execute(db); + await sql.raw(`DROP TRIGGER IF EXISTS "${ftsTable}_update"`).execute(db); + await sql.raw(`DROP TRIGGER IF EXISTS "${ftsTable}_delete"`).execute(db); + await sql.raw(`DROP TABLE IF EXISTS "${ftsTable}"`).execute(db); + + await sql + .raw(` + CREATE VIRTUAL TABLE IF NOT EXISTS "${ftsTable}" USING fts5( + ${columnList}, + tokenize='porter unicode61' + ) + `) + .execute(db); + + await sql + .raw(` + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_insert" + AFTER INSERT ON "${contentTable}" + WHEN NEW.deleted_at IS NULL + BEGIN + INSERT OR REPLACE INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + VALUES (NEW.rowid, NEW.id, NEW.locale, ${newValueList}); + END + `) + .execute(db); + + await sql + .raw(` + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_update" + AFTER UPDATE ON "${contentTable}" + BEGIN + DELETE FROM "${ftsTable}" WHERE rowid = OLD.rowid; + INSERT INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + SELECT NEW.rowid, NEW.id, NEW.locale, ${newValueList} + WHERE NEW.deleted_at IS NULL; + END + `) + .execute(db); + + await sql + .raw(` + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_delete" + AFTER DELETE ON "${contentTable}" + BEGIN + DELETE FROM "${ftsTable}" WHERE rowid = OLD.rowid; + END + `) + .execute(db); + + await sql + .raw(` + INSERT OR REPLACE INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + SELECT rowid, id, locale, ${selectValueList} FROM "${contentTable}" + WHERE deleted_at IS NULL + `) + .execute(db); +} diff --git a/packages/core/src/database/migrations/runner.ts b/packages/core/src/database/migrations/runner.ts index 5c1c7764ff..fccfca46d2 100644 --- a/packages/core/src/database/migrations/runner.ts +++ b/packages/core/src/database/migrations/runner.ts @@ -57,6 +57,7 @@ import * as m051 from "./051_content_taxonomies_denorm.js"; import * as m052 from "./052_media_usage_read_index.js"; import * as m053 from "./053_plugin_mcp_tools.js"; import * as m054 from "./054_media_upload_attempts.js"; +import * as m055 from "./055_fts_plain_text.js"; const MIGRATIONS: Readonly> = Object.freeze({ "001_initial": m001, @@ -112,6 +113,7 @@ const MIGRATIONS: Readonly> = Object.freeze({ "052_media_usage_read_index": m052, "053_plugin_mcp_tools": m053, "054_media_upload_attempts": m054, + "055_fts_plain_text": m055, }); /** Total number of registered migrations. Exported for use in tests. */ diff --git a/packages/core/src/search/fts-manager.ts b/packages/core/src/search/fts-manager.ts index c200b74125..ea8764b829 100644 --- a/packages/core/src/search/fts-manager.ts +++ b/packages/core/src/search/fts-manager.ts @@ -75,26 +75,24 @@ export class FTSManager { if (!isSqlite(this.db)) return; this.validateInputs(collectionSlug, searchableFields); const ftsTable = this.getFtsTableName(collectionSlug); - const contentTable = this.getContentTableName(collectionSlug); // Build the column list for FTS5 // id and locale are UNINDEXED (used for joining/filtering, not searched) const columns = ["id UNINDEXED", "locale UNINDEXED", ...searchableFields].join(", "); - // Create the FTS5 virtual table. - // `content=''` makes this an *external content* FTS5 table: - // the inverted index lives in the FTS shadow tables, but the actual - // row data lives in the backing content table. The triggers in - // `createTriggers` keep the index in sync; they MUST use the - // external-content-safe `'delete'` command (see notes there) to - // avoid `SQLITE_CORRUPT_VTAB` on UPDATE/DELETE. + // Create the FTS5 virtual table. The table stores its own copy of the + // indexed values (no `content=` option): Portable Text fields are + // indexed as extracted plain text — see searchValueExpr — which cannot + // mirror the raw JSON in the ec_* column, and external-content FTS5 + // requires the index to exactly mirror the backing table's values + // (snippet() reads them, and the 'delete' command must be fed the + // inserted values or the index corrupts — see migration 039's history). + // Storing the extracted text also makes snippet() return prose. // tokenize='porter unicode61' enables stemming (run matches running, ran, etc.) await sql .raw(` CREATE VIRTUAL TABLE IF NOT EXISTS "${ftsTable}" USING fts5( ${columns}, - content='${contentTable}', - content_rowid='rowid', tokenize='porter unicode61' ) `) @@ -104,6 +102,51 @@ export class FTSManager { await this.createTriggers(collectionSlug, searchableFields); } + /** + * SQL expression producing the indexed value for one searchable field. + * + * Portable Text fields are stored as JSON; indexing the raw JSON pollutes + * the index with structural tokens (`_type`, style values like `normal`, + * `_key` ULIDs) and makes snippets show JSON fragments. Extract the prose + * instead: every JSON string under a `text`, `alt`, `caption`, or `code` + * key (span text, image alt/caption, code blocks — mirroring + * `extractPlainText` in text-extraction.ts). `json_valid` guards legacy + * rows holding a bare string, which is indexed as-is; extraction must live + * in SQL because the sync triggers cannot call into JS. + * + * `ref` must be a validated column reference (`NEW.x`, `OLD.x`, `"x"`). + */ + private searchValueExpr(ref: string, fieldType: string | undefined): string { + if (fieldType !== "portableText") return ref; + return ( + `CASE WHEN ${ref} IS NULL THEN NULL ` + + `WHEN json_valid(${ref}) THEN (` + + `SELECT group_concat(j.value, ' ') FROM json_tree(${ref}) AS j ` + + `WHERE j.key IN ('text', 'alt', 'caption', 'code') AND j.type = 'text') ` + + `ELSE ${ref} END` + ); + } + + /** + * Field type per slug for a collection, for choosing the indexed-value + * expression. Fields missing from the schema fall back to raw indexing. + */ + private async getFieldTypes(collectionSlug: string): Promise> { + const collection = await this.db + .selectFrom("_emdash_collections") + .select("id") + .where("slug", "=", collectionSlug) + .executeTakeFirst(); + if (!collection) return new Map(); + + const rows = await this.db + .selectFrom("_emdash_fields") + .select(["slug", "type"]) + .where("collection_id", "=", collection.id) + .execute(); + return new Map(rows.map((r) => [r.slug, r.type])); + } + /** * Create triggers to keep FTS table in sync with content table. * @@ -112,31 +155,20 @@ export class FTSManager { * search index and ensures the FTS row count matches the non-deleted * content count (which `verifyAndRepairIndex` relies on). * - * IMPORTANT: The FTS5 virtual table is created with `content='ec_'` - * which makes it an *external content* FTS5 table. For external-content - * tables, removing a row must use the documented `'delete'` command and - * supply the OLD column values explicitly, e.g.: + * The FTS table stores its own values (no `content=` option), so removal + * is a plain `DELETE FROM fts WHERE rowid = OLD.rowid` — a harmless no-op + * for rows that were never indexed (soft-deleted content). The + * external-content `'delete'`-command choreography and its corruption + * modes (migration 039) do not apply to self-contained tables. * - * INSERT INTO fts(fts, rowid, col1, col2) - * VALUES('delete', OLD.rowid, OLD.col1, OLD.col2); + * `INSERT OR REPLACE` keeps the insert path idempotent: re-running a + * populate (D1 has no migration lock, so two isolates can race) converges + * on one index row per content row instead of failing on the rowid + * constraint. * - * Using `DELETE FROM fts WHERE rowid = OLD.rowid` is the correct form - * for *contentless* tables but is unsafe for external-content tables: - * FTS5 then reads column values from the backing content table, which - * in an AFTER UPDATE trigger already holds the NEW values. The wrong - * tokens get removed and the inverted index drifts out of sync until - * SQLite raises `SQLITE_CORRUPT_VTAB` on the next mutation. See - * https://www.sqlite.org/fts5.html#external_content_tables. - * - * The UPDATE and DELETE triggers gate the `'delete'` on - * `OLD.deleted_at IS NULL` because the INSERT trigger never indexed - * rows that were already soft-deleted. Issuing `'delete'` for a rowid - * that was never inserted into the FTS index is itself a corruption - * trigger -- FTS5's `'delete'` is not a no-op on missing rowids and - * raises `SQLITE_CORRUPT_VTAB`. Affected paths include restore-from- - * trash (UPDATE where `OLD.deleted_at IS NOT NULL`), permanent-delete - * from trash (DELETE on a soft-deleted row), and any edit on a row - * that's currently in the trash. + * The trigger SQL emitted here MUST stay in lock-step with migration + * `055_fts_plain_text.ts`. If this changes again, add a new migration + * rather than editing that one — migrations are forward-only. */ private async createTriggers(collectionSlug: string, searchableFields: string[]): Promise { this.validateInputs(collectionSlug, searchableFields); @@ -148,64 +180,48 @@ export class FTSManager { } const ftsTable = this.getFtsTableName(collectionSlug); const contentTable = this.getContentTableName(collectionSlug); + const fieldTypes = await this.getFieldTypes(collectionSlug); const fieldList = searchableFields.join(", "); - const newFieldList = searchableFields.map((f) => `NEW.${f}`).join(", "); - // `'delete'` takes the FTS5 virtual table name as the first column, - // then the rowid being removed, then the OLD value of every column - // declared on the FTS5 table (in declaration order: id, locale, - // then each searchable field). - const oldFieldList = searchableFields.map((f) => `OLD.${f}`).join(", "); + const newValueList = searchableFields + .map((f) => this.searchValueExpr(`NEW.${f}`, fieldTypes.get(f))) + .join(", "); // Insert trigger - only index non-deleted content await sql .raw(` - CREATE TRIGGER IF NOT EXISTS "${ftsTable}_insert" - AFTER INSERT ON "${contentTable}" + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_insert" + AFTER INSERT ON "${contentTable}" WHEN NEW.deleted_at IS NULL BEGIN - INSERT INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) - VALUES (NEW.rowid, NEW.id, NEW.locale, ${newFieldList}); + INSERT OR REPLACE INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + VALUES (NEW.rowid, NEW.id, NEW.locale, ${newValueList}); END `) .execute(this.db); - // Update trigger - remove the old row from the FTS index using the - // external-content-safe `'delete'` command (which uses OLD column - // values, captured before the row was modified), then re-insert - // the new values when the row is still visible. - // - // `'delete'` is gated on `OLD.deleted_at IS NULL` because rows that - // were soft-deleted are not in the FTS index (the INSERT trigger - // skips them). Issuing `'delete'` for a missing rowid raises - // `SQLITE_CORRUPT_VTAB`, which would break restore-from-trash and - // edits to soft-deleted rows. + // Update trigger - drop the old index row, re-insert when the row is + // still visible. Trash (deleted_at set) ends at DELETE only; restore + // ends at DELETE (no-op) + re-insert. await sql .raw(` - CREATE TRIGGER IF NOT EXISTS "${ftsTable}_update" - AFTER UPDATE ON "${contentTable}" + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_update" + AFTER UPDATE ON "${contentTable}" BEGIN - INSERT INTO "${ftsTable}"("${ftsTable}", rowid, id, locale, ${fieldList}) - SELECT 'delete', OLD.rowid, OLD.id, OLD.locale, ${oldFieldList} - WHERE OLD.deleted_at IS NULL; + DELETE FROM "${ftsTable}" WHERE rowid = OLD.rowid; INSERT INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) - SELECT NEW.rowid, NEW.id, NEW.locale, ${newFieldList} + SELECT NEW.rowid, NEW.id, NEW.locale, ${newValueList} WHERE NEW.deleted_at IS NULL; END `) .execute(this.db); - // Delete trigger - same external-content-safe `'delete'` form, - // gated on `OLD.deleted_at IS NULL` for the same reason as the - // UPDATE trigger: permanent-delete from trash hits a row whose - // `deleted_at` is already set and which was never indexed. + // Delete trigger await sql .raw(` - CREATE TRIGGER IF NOT EXISTS "${ftsTable}_delete" - AFTER DELETE ON "${contentTable}" + CREATE TRIGGER IF NOT EXISTS "${ftsTable}_delete" + AFTER DELETE ON "${contentTable}" BEGIN - INSERT INTO "${ftsTable}"("${ftsTable}", rowid, id, locale, ${fieldList}) - SELECT 'delete', OLD.rowid, OLD.id, OLD.locale, ${oldFieldList} - WHERE OLD.deleted_at IS NULL; + DELETE FROM "${ftsTable}" WHERE rowid = OLD.rowid; END `) .execute(this.db); @@ -260,20 +276,30 @@ export class FTSManager { } /** - * Populate the FTS table from existing content + * Populate the FTS table from existing content. + * + * `INSERT OR REPLACE` so a concurrent double-populate (D1 has no + * migration lock) converges instead of failing on the rowid constraint. */ async populateFromContent(collectionSlug: string, searchableFields: string[]): Promise { if (!isSqlite(this.db)) return; this.validateInputs(collectionSlug, searchableFields); const ftsTable = this.getFtsTableName(collectionSlug); const contentTable = this.getContentTableName(collectionSlug); + const fieldTypes = await this.getFieldTypes(collectionSlug); const fieldList = searchableFields.join(", "); + // Table-qualified references: json_tree exposes columns named + // key/value/type/path/..., and inside the extraction subquery a bare + // column reference binds to those instead of the ec_* column. + const valueList = searchableFields + .map((f) => this.searchValueExpr(`"${contentTable}"."${f}"`, fieldTypes.get(f))) + .join(", "); // Insert all existing content into FTS table await sql .raw(` - INSERT INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) - SELECT rowid, id, locale, ${fieldList} FROM "${contentTable}" + INSERT OR REPLACE INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + SELECT rowid, id, locale, ${valueList} FROM "${contentTable}" WHERE deleted_at IS NULL `) .execute(this.db); @@ -494,10 +520,8 @@ export class FTSManager { return true; } - // Row count parity check. For external-content FTS tables, COUNT(*) - // on the virtual table is answered from the backing content table - // (including soft-deleted rows), so we use the docsize shadow table - // which tracks rows actually present in the full-text index. + // Row count parity check against the docsize shadow table, which + // tracks rows actually present in the full-text index. const contentCount = await sql<{ count: number }>` SELECT COUNT(*) as count FROM ${sql.ref(contentTable)} WHERE deleted_at IS NULL diff --git a/packages/core/tests/integration/database/migrations.test.ts b/packages/core/tests/integration/database/migrations.test.ts index ce2e6de4c3..6bb3a66664 100644 --- a/packages/core/tests/integration/database/migrations.test.ts +++ b/packages/core/tests/integration/database/migrations.test.ts @@ -141,6 +141,7 @@ describe("Database Migrations (Integration)", () => { "052_media_usage_read_index", "053_plugin_mcp_tools", "054_media_upload_attempts", + "055_fts_plain_text", ]; await db.deleteFrom("_emdash_migrations").where("name", "in", trailing).execute(); diff --git a/packages/core/tests/integration/search/portable-text-indexing.test.ts b/packages/core/tests/integration/search/portable-text-indexing.test.ts new file mode 100644 index 0000000000..c3bb82cf49 --- /dev/null +++ b/packages/core/tests/integration/search/portable-text-indexing.test.ts @@ -0,0 +1,165 @@ +/** + * FTS indexes Portable Text prose, not its JSON structure. + * + * Portable Text fields are stored as JSON in the content table. Feeding that + * raw JSON to FTS5 pollutes the index with structural tokens — every post + * matches searches for "normal" (a style value), "span", or "markDefs", and + * snippets show JSON fragments instead of prose. The index must contain only + * extracted text: span text, image alt/caption, code content. + */ + +import type { Kysely } from "kysely"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { ContentRepository } from "../../../src/database/repositories/content.js"; +import type { Database } from "../../../src/database/types.js"; +import { SchemaRegistry } from "../../../src/schema/registry.js"; +import { FTSManager } from "../../../src/search/fts-manager.js"; +import { searchWithDb } from "../../../src/search/query.js"; +import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js"; + +describe("Portable Text FTS indexing", () => { + let db: Kysely; + let registry: SchemaRegistry; + let repo: ContentRepository; + let ftsManager: FTSManager; + + beforeEach(async () => { + db = await setupTestDatabase(); + registry = new SchemaRegistry(db); + repo = new ContentRepository(db); + ftsManager = new FTSManager(db); + + await registry.createCollection({ + slug: "pages", + label: "Pages", + labelSingular: "Page", + supports: ["drafts", "revisions", "search"], + }); + // content first: searchSingleCollection snippets the first searchable + // field (FTS column 2), and these tests assert content snippets. + await registry.createField("pages", { + slug: "content", + label: "Content", + type: "portableText", + searchable: true, + }); + await registry.createField("pages", { + slug: "title", + label: "Title", + type: "string", + required: true, + searchable: true, + }); + + await ftsManager.enableSearch("pages"); + + await repo.create({ + type: "pages", + slug: "haunted-cinema", + status: "published", + data: { + title: "Opening Night", + content: [ + { + _type: "block", + _key: "b1", + style: "normal", + markDefs: [], + children: [ + { _type: "span", _key: "s1", text: "The haunted cinema screens forbidden films." }, + ], + }, + { _type: "image", _key: "b2", alt: "festival poster", caption: "official artwork" }, + { _type: "code", _key: "b3", code: "SELECT midnight FROM screenings" }, + ], + }, + }); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + it("does not match Portable Text structural tokens", async () => { + for (const structural of ["normal", "span", "markDefs", "block"]) { + const { items } = await searchWithDb(db, structural, { collections: ["pages"] }); + expect(items, `"${structural}" must not match`).toEqual([]); + } + }); + + it("matches prose inside spans", async () => { + const { items } = await searchWithDb(db, "haunted", { collections: ["pages"] }); + expect(items).toHaveLength(1); + expect(items[0]!.slug).toBe("haunted-cinema"); + }); + + it("matches image alt text, captions, and code content", async () => { + for (const term of ["poster", "artwork", "midnight"]) { + const { items } = await searchWithDb(db, term, { collections: ["pages"] }); + expect(items, `"${term}" must match`).toHaveLength(1); + } + }); + + it("returns prose snippets, not JSON fragments", async () => { + const { items } = await searchWithDb(db, "forbidden", { collections: ["pages"] }); + expect(items).toHaveLength(1); + const snippet = items[0]!.snippet ?? ""; + expect(snippet).toContain("forbidden"); + expect(snippet).not.toContain("_type"); + expect(snippet).not.toContain("{"); + }); +}); + +describe("Portable Text FTS indexing — json_tree column-name collisions", () => { + let db: Kysely; + + beforeEach(async () => { + db = await setupTestDatabase(); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + it("populates fields whose slug collides with a json_tree output column", async () => { + // json_tree exposes columns named key/value/type/path/...; an + // unqualified column reference inside the extraction subquery binds to + // those instead of the ec_* column, silently indexing NULL. + const registry = new SchemaRegistry(db); + await registry.createCollection({ + slug: "notes", + label: "Notes", + labelSingular: "Note", + supports: ["search"], + }); + await registry.createField("notes", { + slug: "value", + label: "Value", + type: "portableText", + searchable: true, + }); + + // Create before enabling search so the row flows through + // populateFromContent (the bare-reference path), not the triggers. + await new ContentRepository(db).create({ + type: "notes", + slug: "n1", + status: "published", + data: { + value: [ + { + _type: "block", + _key: "b1", + style: "normal", + children: [{ _type: "span", _key: "s1", text: "A spectral apparition." }], + }, + ], + }, + }); + await new FTSManager(db).enableSearch("notes"); + + const { items } = await searchWithDb(db, "spectral", { collections: ["notes"] }); + expect(items).toHaveLength(1); + }); +}); diff --git a/packages/core/tests/unit/database/migrations/039_fix_fts5_triggers.test.ts b/packages/core/tests/unit/database/migrations/039_fix_fts5_triggers.test.ts index 3496bb69a2..6d08f71537 100644 --- a/packages/core/tests/unit/database/migrations/039_fix_fts5_triggers.test.ts +++ b/packages/core/tests/unit/database/migrations/039_fix_fts5_triggers.test.ts @@ -74,6 +74,42 @@ describe("migration 039: rebuild FTS5 triggers", () => { .execute(db); } + /** + * Rebuild the collection's FTS table in the pre-fix *external-content* + * shape (`content='ec_'`) that every site running a pre-fix EmDash + * version actually had. The current FTSManager builds self-contained FTS + * tables — on those, the "broken" contentless-style triggers sync + * correctly, so the historical corruption can only be reproduced against + * the historical table shape. + */ + async function installExternalContentFts( + collectionSlug: string, + fields: string[], + ): Promise { + const ftsTable = `_emdash_fts_${collectionSlug}`; + const contentTable = `ec_${collectionSlug}`; + const fieldList = fields.join(", "); + + await sql.raw(`DROP TABLE IF EXISTS "${ftsTable}"`).execute(db); + await sql + .raw(` + CREATE VIRTUAL TABLE "${ftsTable}" USING fts5( + id UNINDEXED, locale UNINDEXED, ${fieldList}, + content='${contentTable}', + content_rowid='rowid', + tokenize='porter unicode61' + ) + `) + .execute(db); + await sql + .raw(` + INSERT INTO "${ftsTable}"(rowid, id, locale, ${fieldList}) + SELECT rowid, id, locale, ${fieldList} FROM "${contentTable}" + WHERE deleted_at IS NULL + `) + .execute(db); + } + async function setupSearchEnabledPages(): Promise { await registry.createCollection({ slug: "pages", @@ -137,11 +173,12 @@ describe("migration 039: rebuild FTS5 triggers", () => { data: { title: "About", body: "Some searchable body text." }, }); - // Simulate the pre-fix state: broken triggers + a published row. - // The legacy triggers are functional on INSERT (the contentless and - // external-content forms agree there), so the row is in the index - // at this point. The migration must replace the triggers without - // losing that row. + // Simulate the pre-fix state: external-content FTS table + broken + // triggers + a published row. The legacy triggers are functional on + // INSERT (the contentless and external-content forms agree there), + // so the row is in the index at this point. The migration must + // replace the triggers without losing that row. + await installExternalContentFts("pages", ["title", "body"]); await installPreFixTriggers("pages", ["title", "body"]); await runMigration039(); @@ -187,16 +224,17 @@ describe("migration 039: rebuild FTS5 triggers", () => { data: { title: "Corrupt me", body: "Original aardvark body." }, }); - // Install the broken triggers and then *fire them* by issuing the - // kind of UPDATE the publish path does. The broken trigger's - // `DELETE FROM fts WHERE rowid = OLD.rowid` on an external-content - // table reads NEW values from the content table when removing - // tokens, so the OLD tokens are left behind in the inverted index - // even though the content table no longer holds them. The result - // is a stale-token leak: searches for words from the OLD body - // keep matching the (now updated) row, and segment metadata - // drifts out of sync until SQLite eventually surfaces it as - // SQLITE_CORRUPT_VTAB. + // Install the historical external-content table and broken triggers, + // then *fire them* by issuing the kind of UPDATE the publish path + // does. The broken trigger's `DELETE FROM fts WHERE rowid = OLD.rowid` + // on an external-content table reads NEW values from the content + // table when removing tokens, so the OLD tokens are left behind in + // the inverted index even though the content table no longer holds + // them. The result is a stale-token leak: searches for words from + // the OLD body keep matching the (now updated) row, and segment + // metadata drifts out of sync until SQLite eventually surfaces it + // as SQLITE_CORRUPT_VTAB. + await installExternalContentFts("pages", ["title", "body"]); await installPreFixTriggers("pages", ["title", "body"]); // Sanity check: the OLD content's unique token is indexed before diff --git a/packages/core/tests/unit/database/migrations/055_fts_plain_text.test.ts b/packages/core/tests/unit/database/migrations/055_fts_plain_text.test.ts new file mode 100644 index 0000000000..a35918b7a9 --- /dev/null +++ b/packages/core/tests/unit/database/migrations/055_fts_plain_text.test.ts @@ -0,0 +1,146 @@ +/** + * Migration 055 rebuilds FTS indexes as self-contained tables indexing + * extracted Portable Text prose. These tests exercise the migration against + * the pre-fix state a real upgrade hits: an external-content FTS table whose + * index holds raw Portable Text JSON, so structural tokens ("normal", + * "span") match documents whose prose never contains them. + */ + +import type { Kysely } from "kysely"; +import { sql } from "kysely"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { ContentRepository } from "../../../../src/database/repositories/content.js"; +import type { Database } from "../../../../src/database/types.js"; +import { SchemaRegistry } from "../../../../src/schema/registry.js"; +import { FTSManager } from "../../../../src/search/fts-manager.js"; +import { setupTestDatabase, teardownTestDatabase } from "../../../utils/test-db.js"; + +describe("migration 055: FTS indexes extracted Portable Text prose", () => { + let db: Kysely; + let registry: SchemaRegistry; + let repo: ContentRepository; + + beforeEach(async () => { + db = await setupTestDatabase(); + registry = new SchemaRegistry(db); + repo = new ContentRepository(db); + + await registry.createCollection({ + slug: "pages", + label: "Pages", + labelSingular: "Page", + supports: ["search"], + }); + await registry.createField("pages", { + slug: "content", + label: "Content", + type: "portableText", + searchable: true, + }); + await new FTSManager(db).enableSearch("pages"); + + await repo.create({ + type: "pages", + slug: "haunted", + status: "published", + data: { + content: [ + { + _type: "block", + _key: "b1", + style: "normal", + children: [{ _type: "span", _key: "s1", text: "The haunted cinema." }], + }, + ], + }, + }); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + /** + * Rebuild the pages FTS table in the pre-055 shape: external-content + * FTS5 indexing the raw column values (Portable Text JSON included). + */ + async function installPreFixFts(): Promise { + await sql.raw(`DROP TRIGGER IF EXISTS "_emdash_fts_pages_insert"`).execute(db); + await sql.raw(`DROP TRIGGER IF EXISTS "_emdash_fts_pages_update"`).execute(db); + await sql.raw(`DROP TRIGGER IF EXISTS "_emdash_fts_pages_delete"`).execute(db); + await sql.raw(`DROP TABLE IF EXISTS "_emdash_fts_pages"`).execute(db); + await sql + .raw(` + CREATE VIRTUAL TABLE "_emdash_fts_pages" USING fts5( + id UNINDEXED, locale UNINDEXED, content, + content='ec_pages', + content_rowid='rowid', + tokenize='porter unicode61' + ) + `) + .execute(db); + await sql + .raw(` + INSERT INTO "_emdash_fts_pages"(rowid, id, locale, content) + SELECT rowid, id, locale, content FROM "ec_pages" + WHERE deleted_at IS NULL + `) + .execute(db); + } + + async function matches(term: string): Promise { + const result = await sql<{ count: number }>` + SELECT COUNT(*) as count FROM "_emdash_fts_pages" + WHERE "_emdash_fts_pages" MATCH ${term} + `.execute(db); + return Number(result.rows[0]?.count ?? 0); + } + + async function runMigration055(): Promise { + const { up } = await import("../../../../src/database/migrations/055_fts_plain_text.js"); + await up(db as unknown as Kysely); + } + + it("replaces the JSON-polluted index with extracted prose", async () => { + await installPreFixFts(); + + // Pre-migration: structural tokens match — the pollution being fixed. + expect(await matches("normal")).toBe(1); + expect(await matches("span")).toBe(1); + + await runMigration055(); + + expect(await matches("normal")).toBe(0); + expect(await matches("span")).toBe(0); + expect(await matches("haunted")).toBe(1); + }); + + it("installs working sync triggers alongside the rebuilt index", async () => { + await installPreFixFts(); + await runMigration055(); + + const rows = await sql<{ id: string }>`SELECT id FROM ec_pages`.execute(db); + await repo.update("pages", rows.rows[0]!.id, { + data: { + content: [ + { + _type: "block", + _key: "b1", + style: "normal", + children: [{ _type: "span", _key: "s1", text: "A midnight screening." }], + }, + ], + }, + }); + + expect(await matches("midnight")).toBe(1); + expect(await matches("haunted")).toBe(0); + expect(await matches("normal")).toBe(0); + }); + + it("is a no-op on databases with no search-enabled collections", async () => { + await new FTSManager(db).disableSearch("pages"); + await expect(runMigration055()).resolves.toBeUndefined(); + }); +});