From 501282289ac9150252ef2c32ac4f584d508d56af Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 29 Jul 2026 20:06:30 +0200 Subject: [PATCH] fix(database): verify the title index folds, rather than trust the marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createTables` declares content_title_fts with the plain trigram tokenizer, and the diacritics migration declares it again with folding. Two sources of truth for one tokenizer: if the table ever went missing after the marker was written, `CREATE TABLE IF NOT EXISTS` would restore the unfolded form and the migration would skip it on the marker alone. The upgrade now reads the live table's own DDL from sqlite_master and rebuilds unless it really folds. A degraded index is invisible from the outside — discovery just stops finding "Pokémon" for "pokemon" — so the record has to be checked against the thing it describes. Co-Authored-By: Claude Opus 5 --- docs/architecture/vod-multi-source.md | 11 ++++- .../src/lib/connection-migrations.spec.ts | 43 ++++++++++++++++++- libs/shared/database/src/lib/connection.ts | 30 ++++++++++++- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index 9f14d12f1..517b92411 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -531,7 +531,16 @@ CREATE time, so existing databases are recreated and rebuilt once behind `remove_diacritics` needs SQLite 3.45+. The migration probes support on a temp table first and does nothing when the runtime rejects it, leaving the working index in place — and does NOT record itself as done, so a later app version -shipping a newer SQLite upgrades then. +shipping a newer SQLite upgrades then. (better-sqlite3 currently bundles 3.53, +so the fallback is defensive rather than a path anyone is on.) + +The completed marker is not taken as proof. `createTables` declares this table +too, with the plain tokenizer, and `CREATE TABLE IF NOT EXISTS` would recreate +it unfolded if it ever went missing after the marker was written — leaving two +sources of truth for one tokenizer. The migration therefore reads the live +table's own DDL out of `sqlite_master` and rebuilds unless it really folds. A +degraded index is otherwise invisible: discovery simply stops finding "Pokémon" +for "pokemon", with nothing to indicate it should have. **Case folding for non-ASCII, on the FTS tier, is still not possible.** `LOWER()` and the trigram tokenizer both fold ASCII only, so a Cyrillic title diff --git a/libs/shared/database/src/lib/connection-migrations.spec.ts b/libs/shared/database/src/lib/connection-migrations.spec.ts index d28810379..89d160a13 100644 --- a/libs/shared/database/src/lib/connection-migrations.spec.ts +++ b/libs/shared/database/src/lib/connection-migrations.spec.ts @@ -54,6 +54,16 @@ const completedMigrationStateRule: HandlerRule = [ { get: () => ({ value: 'done' }) }, ]; +/** The live title index, already carrying the folding tokenizer. */ +const foldedIndexRule: HandlerRule = [ + 'SELECT sql FROM sqlite_master', + { + get: () => ({ + sql: "CREATE VIRTUAL TABLE content_title_fts USING fts5(title, tokenize='trigram remove_diacritics 1')", + }), + }, +]; + describe('createTables', () => { it('executes every fresh-install statement against the connection in order', () => { const { exec, sqlite } = createSqliteMock([]); @@ -86,7 +96,7 @@ describe('runMigrations error tolerance', () => { } }); const { sqlite } = createSqliteMock( - [completedMigrationStateRule], + [completedMigrationStateRule, foldedIndexRule], exec ); @@ -111,7 +121,7 @@ describe('runMigrations error tolerance', () => { } }); const { sqlite } = createSqliteMock( - [completedMigrationStateRule], + [completedMigrationStateRule, foldedIndexRule], exec ); @@ -299,6 +309,7 @@ describe('content title FTS tokenizer upgrade', () => { it('does nothing once the migration has completed', () => { const { sqlite, exec } = createSqliteMock([ completedMigrationStateRule, + foldedIndexRule, ]); upgradeContentTitleFtsTokenizer(sqlite); @@ -306,6 +317,34 @@ describe('content title FTS tokenizer upgrade', () => { expect(exec).not.toHaveBeenCalled(); }); + it('rebuilds when the record says done but the index is not folded', () => { + const rebuild = { run: jest.fn() }; + const { sqlite, exec } = createSqliteMock([ + completedMigrationStateRule, + [ + 'SELECT sql FROM sqlite_master', + { + get: () => ({ + sql: "CREATE VIRTUAL TABLE content_title_fts USING fts5(title, tokenize='trigram')", + }), + }, + ], + ['INSERT INTO content_title_fts(content_title_fts)', rebuild], + ]); + + upgradeContentTitleFtsTokenizer(sqlite); + + // `createTables` declares this table too, with the plain tokenizer, so + // the marker and the live table can disagree. Trusting the marker + // leaves a silently degraded index: discovery simply stops finding + // "Pokémon" for "pokemon", with nothing to show that it should have. + const statements = exec.mock.calls.map(([sql]) => compactSql(sql)); + expect(statements).toContainEqual( + expect.stringContaining("tokenize='trigram remove_diacritics 1'") + ); + expect(rebuild.run).toHaveBeenCalled(); + }); + it('leaves the index alone when the runtime rejects the tokenizer', () => { const marker = { run: jest.fn() }; const exec = jest.fn((statement: string) => { diff --git a/libs/shared/database/src/lib/connection.ts b/libs/shared/database/src/lib/connection.ts index c139ef488..c4c664b0e 100644 --- a/libs/shared/database/src/lib/connection.ts +++ b/libs/shared/database/src/lib/connection.ts @@ -647,6 +647,26 @@ function contentTitleFtsStatement(removeDiacritics: boolean): string { } /** Whether this SQLite accepts the folding tokenizer, asked without risk. */ +/** + * Whether the title index that actually exists folds diacritics, read from its + * own stored DDL rather than from the migration record. + * + * A missing table answers `false`, which is the useful answer: there is nothing + * folded to keep, so the caller rebuilds. + */ +function contentTitleFtsFoldsDiacritics(sqliteDb: Database.Database): boolean { + const row = sqliteDb + .prepare( + `SELECT sql FROM sqlite_master + WHERE type = 'table' AND name = 'content_title_fts'` + ) + .get() as { sql?: unknown } | undefined; + + return ( + typeof row?.sql === 'string' && row.sql.includes('remove_diacritics') + ); +} + function supportsTrigramDiacriticFolding(sqliteDb: Database.Database): boolean { try { sqliteDb.exec( @@ -676,7 +696,15 @@ function upgradeContentTitleFtsTokenizer(sqliteDb: Database.Database): boolean { | { value?: unknown } | undefined; - if (migrationState?.value === 'done') { + // The marker alone is not evidence. `createTables` declares this table + // too, with the plain tokenizer, so a table recreated by that path + // after the marker was written would be silently unfolded — and a + // degraded index is invisible: discovery just stops finding "Pokémon" + // for "pokemon". Ask the live table instead of trusting the record. + if ( + migrationState?.value === 'done' && + contentTitleFtsFoldsDiacritics(sqliteDb) + ) { return false; }