diff --git a/.changeset/fts-snippet-matched-column.md b/.changeset/fts-snippet-matched-column.md new file mode 100644 index 0000000000..a471acd47c --- /dev/null +++ b/.changeset/fts-snippet-matched-column.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fixes search snippets quoting the wrong field. An FTS5 index is laid out as `id UNINDEXED, locale UNINDEXED, ...searchable fields`, and `snippet()` was asked for column 2 — whichever field happens to be searchable first, usually the title. A match anywhere else came back as that first field's text with no highlight, so a hit in the body, an artist name, or a tracklist all rendered as the bare title and told the reader nothing about why the entry matched. Snippets now come from the column FTS5 actually matched, and a title match still quotes the title. diff --git a/packages/core/src/search/query.ts b/packages/core/src/search/query.ts index ed0f090644..e83db23fe0 100644 --- a/packages/core/src/search/query.ts +++ b/packages/core/src/search/query.ts @@ -307,7 +307,10 @@ async function searchSingleCollection( c.slug, c.locale, ${titleExpr} as title, - snippet("${sql.raw(ftsTable)}", 2, '', '', '...', 32) as snippet, + -- Column -1 lets FTS5 pick the column the query actually matched. + -- Hard-coding 2 (the first searchable field) meant a match in any + -- other field returned that first field's text, unhighlighted. + snippet("${sql.raw(ftsTable)}", -1, '', '', '...', 32) as snippet, ${sql.raw(bm25Expr)} as score FROM "${sql.raw(ftsTable)}" f JOIN "${sql.raw(contentTable)}" c ON f.id = c.id @@ -339,10 +342,9 @@ async function searchSingleCollection( slug: row.slug, locale: row.locale, title: row.title ?? undefined, - // SQLite's snippet() returns NULL when the targeted column is - // NULL for that row — even if the row matched via a different - // searchable column. Skip sanitization in that case so we don't - // throw on `null.replace`. The SearchResult.snippet field is + // SQLite's snippet() can still return NULL — for a row whose matched + // column holds no text, for instance. Skip sanitization in that case + // so we don't throw on `null.replace`. The SearchResult.snippet field is // already optional, so omitting it is the documented contract. snippet: row.snippet === null ? undefined : sanitizeSnippet(row.snippet), score: Math.abs(row.score), // bm25 returns negative scores diff --git a/packages/core/tests/integration/search/snippet-column.test.ts b/packages/core/tests/integration/search/snippet-column.test.ts new file mode 100644 index 0000000000..345031787a --- /dev/null +++ b/packages/core/tests/integration/search/snippet-column.test.ts @@ -0,0 +1,87 @@ +import type { Kysely } from "kysely"; +import { describe, it, expect, beforeEach, afterEach } 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 { createPostFixture } from "../../utils/fixtures.js"; +import { setupTestDatabaseWithCollections, teardownTestDatabase } from "../../utils/test-db.js"; + +/** + * An FTS5 index is laid out as `id UNINDEXED, locale UNINDEXED, ...searchable + * fields`, so column 2 is whichever field happens to be searchable first. + * Asking `snippet()` for that fixed column returns the first field's text no + * matter which field the query matched — a hit in the body came back as the + * bare title, with no highlight, and told the reader nothing about why the + * entry matched. + * + * Passing -1 lets FTS5 pick the column that actually matched. These tests pin + * that: a body-only match must quote the body, and a title match must still + * quote the title. + */ +describe("search snippet column selection", () => { + let db: Kysely; + let repo: ContentRepository; + + beforeEach(async () => { + db = await setupTestDatabaseWithCollections(); + repo = new ContentRepository(db); + + const registry = new SchemaRegistry(db); + const ftsManager = new FTSManager(db); + // `title` is registered first, so it owns column 2 — the column the + // snippet used to be pinned to. + await registry.updateField("post", "title", { searchable: true }); + await registry.updateField("post", "content", { searchable: true }); + await ftsManager.enableSearch("post"); + + await repo.create( + createPostFixture({ + slug: "tour-diary", + status: "published", + data: { + title: "Endless Night", + content: [ + { + _type: "block", + style: "normal", + children: [ + { + _type: "span", + text: "The closing track is a cover of Mustang Sally, recorded live.", + }, + ], + }, + ], + }, + }), + ); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + it("quotes the body when the match is in the body, not the title", async () => { + const { items } = await searchWithDb(db, "mustang", { + collections: ["post"], + }); + + expect(items).toHaveLength(1); + // Before the fix this was "Endless Night" — the title, unhighlighted, + // because the snippet was pinned to column 2. + expect(items[0].snippet).toContain("Mustang"); + expect(items[0].snippet).toContain("Sally"); + }); + + it("still quotes the title when the match is in the title", async () => { + const { items } = await searchWithDb(db, "endless", { + collections: ["post"], + }); + + expect(items).toHaveLength(1); + expect(items[0].snippet).toContain("Endless"); + }); +}); diff --git a/scripts/query-counts.queries.d1.json b/scripts/query-counts.queries.d1.json index 93fd65664c..1ea3d847d0 100644 --- a/scripts/query-counts.queries.d1.json +++ b/scripts/query-counts.queries.d1.json @@ -235,7 +235,7 @@ "select * from \"_emdash_migrations\" limit ?": 1, "select * from \"_emdash_redirects\" where \"enabled\" = ?": 1, "SELECT *, (SELECT json_group_array(json_object('id', t.id, 'name', t.name, 'slug', t.slug, 'label', t.label, 'parent_id', t.parent_id, 'locale', t.locale, 'translation_group', t.translation_group)) FROM \"content_taxonomies\" AS ct CROSS JOIN \"taxonomies\" AS t ON t.translation_group = ct.taxonomy_id WHERE ct.collection = ? AND ct.entry_id = \"ec_pages\".translation_group AND t.locale = \"ec_pages\".locale) AS \"_emdash_terms\", (SELECT json_group_array(json_object('roleLabel', cb.role_label, 'sortOrder', cb.sort_order, 'byline', json_object('id', b.id, 'slug', b.slug, 'displayName', b.display_name, 'bio', b.bio, 'avatarMediaId', b.avatar_media_id, 'avatarStorageKey', m.storage_key, 'avatarAlt', m.alt, 'avatarBlurhash', m.blurhash, 'avatarDominantColor', m.dominant_color, 'websiteUrl', b.website_url, 'userId', b.user_id, 'isGuest', b.is_guest, 'createdAt', b.created_at, 'updatedAt', b.updated_at, 'locale', b.locale, 'translationGroup', b.translation_group))) FROM \"_emdash_content_bylines\" AS cb CROSS JOIN \"_emdash_bylines\" AS b ON b.translation_group = cb.byline_id LEFT JOIN \"media\" AS m ON m.id = b.avatar_media_id WHERE cb.collection_slug = ? AND cb.content_id = \"ec_pages\".id AND b.locale = \"ec_pages\".locale) AS \"_emdash_bylines\", (SELECT 1 FROM \"_emdash_bylines\" LIMIT 1) AS \"_emdash_bylines_exist\" FROM \"ec_pages\" WHERE deleted_at IS NULL AND \"status\" = ? ORDER BY \"created_at\" DESC, \"id\" DESC": 1, - "SELECT c.id, c.slug, c.locale, c.title as title, snippet(\"_emdash_fts_posts\", 2, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, + "SELECT c.id, c.slug, c.locale, c.title as title, -- Column -1 lets FTS5 pick the column the query actually matched. -- Hard-coding 2 (the first searchable field) meant a match in any -- other field returned that first field's text, unhighlighted. snippet(\"_emdash_fts_posts\", -1, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, "select count(*) as \"count\" from \"_emdash_collections\"": 1, "SELECT COUNT(*) as count FROM \"_emdash_migrations\"": 1, "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?": 1, @@ -251,7 +251,7 @@ "select * from \"_emdash_menu_items\" where \"menu_id\" = ? order by \"sort_order\" asc": 1, "select * from \"_emdash_menus\" where \"name\" = ? order by \"locale\" asc": 1, "SELECT *, (SELECT json_group_array(json_object('id', t.id, 'name', t.name, 'slug', t.slug, 'label', t.label, 'parent_id', t.parent_id, 'locale', t.locale, 'translation_group', t.translation_group)) FROM \"content_taxonomies\" AS ct CROSS JOIN \"taxonomies\" AS t ON t.translation_group = ct.taxonomy_id WHERE ct.collection = ? AND ct.entry_id = \"ec_pages\".translation_group AND t.locale = \"ec_pages\".locale) AS \"_emdash_terms\", (SELECT json_group_array(json_object('roleLabel', cb.role_label, 'sortOrder', cb.sort_order, 'byline', json_object('id', b.id, 'slug', b.slug, 'displayName', b.display_name, 'bio', b.bio, 'avatarMediaId', b.avatar_media_id, 'avatarStorageKey', m.storage_key, 'avatarAlt', m.alt, 'avatarBlurhash', m.blurhash, 'avatarDominantColor', m.dominant_color, 'websiteUrl', b.website_url, 'userId', b.user_id, 'isGuest', b.is_guest, 'createdAt', b.created_at, 'updatedAt', b.updated_at, 'locale', b.locale, 'translationGroup', b.translation_group))) FROM \"_emdash_content_bylines\" AS cb CROSS JOIN \"_emdash_bylines\" AS b ON b.translation_group = cb.byline_id LEFT JOIN \"media\" AS m ON m.id = b.avatar_media_id WHERE cb.collection_slug = ? AND cb.content_id = \"ec_pages\".id AND b.locale = \"ec_pages\".locale) AS \"_emdash_bylines\", (SELECT 1 FROM \"_emdash_bylines\" LIMIT 1) AS \"_emdash_bylines_exist\" FROM \"ec_pages\" WHERE deleted_at IS NULL AND \"status\" = ? ORDER BY \"created_at\" DESC, \"id\" DESC": 1, - "SELECT c.id, c.slug, c.locale, c.title as title, snippet(\"_emdash_fts_posts\", 2, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, + "SELECT c.id, c.slug, c.locale, c.title as title, -- Column -1 lets FTS5 pick the column the query actually matched. -- Hard-coding 2 (the first searchable field) meant a match in any -- other field returned that first field's text, unhighlighted. snippet(\"_emdash_fts_posts\", -1, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?": 1 }, "GET /tag/webdev (cold)": { diff --git a/scripts/query-counts.queries.sqlite.json b/scripts/query-counts.queries.sqlite.json index c870cb2c38..20aa6416b6 100644 --- a/scripts/query-counts.queries.sqlite.json +++ b/scripts/query-counts.queries.sqlite.json @@ -158,7 +158,7 @@ "select * from \"_emdash_menu_items\" where \"menu_id\" = ? order by \"sort_order\" asc": 1, "select * from \"_emdash_menus\" where \"name\" = ? order by \"locale\" asc": 1, "SELECT *, (SELECT json_group_array(json_object('id', t.id, 'name', t.name, 'slug', t.slug, 'label', t.label, 'parent_id', t.parent_id, 'locale', t.locale, 'translation_group', t.translation_group)) FROM \"content_taxonomies\" AS ct CROSS JOIN \"taxonomies\" AS t ON t.translation_group = ct.taxonomy_id WHERE ct.collection = ? AND ct.entry_id = \"ec_pages\".translation_group AND t.locale = \"ec_pages\".locale) AS \"_emdash_terms\", (SELECT json_group_array(json_object('roleLabel', cb.role_label, 'sortOrder', cb.sort_order, 'byline', json_object('id', b.id, 'slug', b.slug, 'displayName', b.display_name, 'bio', b.bio, 'avatarMediaId', b.avatar_media_id, 'avatarStorageKey', m.storage_key, 'avatarAlt', m.alt, 'avatarBlurhash', m.blurhash, 'avatarDominantColor', m.dominant_color, 'websiteUrl', b.website_url, 'userId', b.user_id, 'isGuest', b.is_guest, 'createdAt', b.created_at, 'updatedAt', b.updated_at, 'locale', b.locale, 'translationGroup', b.translation_group))) FROM \"_emdash_content_bylines\" AS cb CROSS JOIN \"_emdash_bylines\" AS b ON b.translation_group = cb.byline_id LEFT JOIN \"media\" AS m ON m.id = b.avatar_media_id WHERE cb.collection_slug = ? AND cb.content_id = \"ec_pages\".id AND b.locale = \"ec_pages\".locale) AS \"_emdash_bylines\", (SELECT 1 FROM \"_emdash_bylines\" LIMIT 1) AS \"_emdash_bylines_exist\" FROM \"ec_pages\" WHERE deleted_at IS NULL AND \"status\" = ? ORDER BY \"created_at\" DESC, \"id\" DESC": 1, - "SELECT c.id, c.slug, c.locale, c.title as title, snippet(\"_emdash_fts_posts\", 2, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, + "SELECT c.id, c.slug, c.locale, c.title as title, -- Column -1 lets FTS5 pick the column the query actually matched. -- Hard-coding 2 (the first searchable field) meant a match in any -- other field returned that first field's text, unhighlighted. snippet(\"_emdash_fts_posts\", -1, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?": 1 }, "GET /search (warm)": { @@ -171,7 +171,7 @@ "select * from \"_emdash_menu_items\" where \"menu_id\" = ? order by \"sort_order\" asc": 1, "select * from \"_emdash_menus\" where \"name\" = ? order by \"locale\" asc": 1, "SELECT *, (SELECT json_group_array(json_object('id', t.id, 'name', t.name, 'slug', t.slug, 'label', t.label, 'parent_id', t.parent_id, 'locale', t.locale, 'translation_group', t.translation_group)) FROM \"content_taxonomies\" AS ct CROSS JOIN \"taxonomies\" AS t ON t.translation_group = ct.taxonomy_id WHERE ct.collection = ? AND ct.entry_id = \"ec_pages\".translation_group AND t.locale = \"ec_pages\".locale) AS \"_emdash_terms\", (SELECT json_group_array(json_object('roleLabel', cb.role_label, 'sortOrder', cb.sort_order, 'byline', json_object('id', b.id, 'slug', b.slug, 'displayName', b.display_name, 'bio', b.bio, 'avatarMediaId', b.avatar_media_id, 'avatarStorageKey', m.storage_key, 'avatarAlt', m.alt, 'avatarBlurhash', m.blurhash, 'avatarDominantColor', m.dominant_color, 'websiteUrl', b.website_url, 'userId', b.user_id, 'isGuest', b.is_guest, 'createdAt', b.created_at, 'updatedAt', b.updated_at, 'locale', b.locale, 'translationGroup', b.translation_group))) FROM \"_emdash_content_bylines\" AS cb CROSS JOIN \"_emdash_bylines\" AS b ON b.translation_group = cb.byline_id LEFT JOIN \"media\" AS m ON m.id = b.avatar_media_id WHERE cb.collection_slug = ? AND cb.content_id = \"ec_pages\".id AND b.locale = \"ec_pages\".locale) AS \"_emdash_bylines\", (SELECT 1 FROM \"_emdash_bylines\" LIMIT 1) AS \"_emdash_bylines_exist\" FROM \"ec_pages\" WHERE deleted_at IS NULL AND \"status\" = ? ORDER BY \"created_at\" DESC, \"id\" DESC": 1, - "SELECT c.id, c.slug, c.locale, c.title as title, snippet(\"_emdash_fts_posts\", 2, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, + "SELECT c.id, c.slug, c.locale, c.title as title, -- Column -1 lets FTS5 pick the column the query actually matched. -- Hard-coding 2 (the first searchable field) meant a match in any -- other field returned that first field's text, unhighlighted. snippet(\"_emdash_fts_posts\", -1, '', '', '...', 32) as snippet, bm25(\"_emdash_fts_posts\") as score FROM \"_emdash_fts_posts\" f JOIN \"ec_posts\" c ON f.id = c.id WHERE \"_emdash_fts_posts\" MATCH ? AND c.status = ? AND c.deleted_at IS NULL ORDER BY score LIMIT ?": 1, "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?": 1 }, "GET /tag/webdev (cold)": {