diff --git a/src/commands/reindex-search-vector.ts b/src/commands/reindex-search-vector.ts index 59524511e..bd9e33c80 100644 --- a/src/commands/reindex-search-vector.ts +++ b/src/commands/reindex-search-vector.ts @@ -179,10 +179,16 @@ export async function runReindexSearchVector( } // Recreate trigger functions. The strings are intentionally identical to - // the v123 migration body — keeping them in lockstep is the contract. + // the v124 migration body — keeping them in lockstep is the contract. // `SET search_path = pg_catalog, public` mirrors the v120/#1647 hardening: // CREATE OR REPLACE resets proconfig, so omitting it here would strip the // hardening from every brain that runs this command. + // + // #2704: compiled_truth (the unbounded whole-page body) is deliberately + // NOT indexed here — it overflows Postgres's 1MB tsvector cap on large + // pages, and content_chunks.search_vector (populated separately, chunk- + // grain, well under the cap) is what searchKeyword() actually queries. + // See migrate.ts's v124 for the full rationale; keep this copy in sync. const recreatePagesFn = ` CREATE OR REPLACE FUNCTION update_page_search_vector() RETURNS trigger SET search_path = pg_catalog, public AS $fn$ DECLARE @@ -195,7 +201,6 @@ export async function runReindexSearchVector( NEW.search_vector := setweight(to_tsvector('${lang}', coalesce(NEW.title, '')), 'A') || - setweight(to_tsvector('${lang}', coalesce(NEW.compiled_truth, '')), 'B') || setweight(to_tsvector('${lang}', coalesce(NEW.timeline, '')), 'C') || setweight(to_tsvector('${lang}', coalesce(timeline_text, '')), 'C'); diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 1e42a958f..c19a07574 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -5602,6 +5602,74 @@ export const MIGRATIONS: Migration[] = [ process.stderr.write(` v123: trigger functions recreated with language='${lang}' + backfilled existing rows\n`); }, }, + { + version: 124, + name: 'page_search_vector_drop_compiled_truth', + // #2704: a single markdown page whose compiled_truth exceeds Postgres's + // hard 1,048,575-byte tsvector cap made update_page_search_vector() + // throw "string is too long for tsvector" INSIDE the pages UPSERT + // transaction — not a per-file ledger entry, a transaction abort. The + // whole source's sync checkpoint stayed pinned (Sync BLOCKED) until the + // oversized file was fixed or manually skipped, even though every + // OTHER file in the run imported fine. + // + // Fix: drop compiled_truth (the unbounded whole-page body) from this + // trigger. It was already redundant — content_chunks.search_vector + // (Cathedral II Layer 3, v0.20.0) is the ACTUAL keyword-search source: + // searchKeyword() in postgres-engine.ts/pglite-engine.ts ranks and + // queries `cc.search_vector` exclusively; `pages.search_vector` is + // written by this trigger but never read by any query in this + // codebase (verified: no `pages.search_vector`/bare `search_vector` + // appears on either side of a WHERE/ts_rank anywhere outside this + // trigger's own definition and the reindex/backfill machinery that + // maintains it). And chunking already bounds each chunk_text well + // under the tsvector limit (chunkText() targets embedding-sized + // pieces, several orders of magnitude smaller than 1MB) — the overflow + // was specific to the whole-page grain this trigger no longer builds. + // + // title + timeline (both naturally small — a compiled_truth-sized + // title or timeline field would be its own bug) stay, so + // pages.search_vector keeps carrying SOME signal rather than going + // fully inert; a future PR can drop the column outright once its + // last non-search consumer (if any turns up) is confirmed gone. + // + // No backfill: existing rows keep whatever search_vector they already + // computed until their next UPDATE (harmless — nothing reads this + // column, so staleness has zero behavioral effect). The brains that + // actually hit this bug never successfully wrote a value for the + // oversized page in the first place, so there's nothing stale to fix + // for them specifically — the NEXT sync of that exact file is what + // proves the fix, not a backfill of already-working rows. + // + // Function body mirrors reindex-search-vector.ts's recreatePagesFn + // (documented contract there: keep both in lockstep) and the fresh- + // install baselines in pglite-schema.ts / schema-embedded.ts — all + // four updated in the same commit as this migration. + sql: '', + handler: async (engine) => { + const lang = getFtsLanguage(); + await engine.executeRaw(` + CREATE OR REPLACE FUNCTION update_page_search_vector() RETURNS trigger SET search_path = pg_catalog, public AS $fn$ + DECLARE + timeline_text TEXT; + BEGIN + SELECT coalesce(string_agg(summary || ' ' || detail, ' '), '') + INTO timeline_text + FROM timeline_entries + WHERE page_id = NEW.id; + + NEW.search_vector := + setweight(to_tsvector('${lang}', coalesce(NEW.title, '')), 'A') || + setweight(to_tsvector('${lang}', coalesce(NEW.timeline, '')), 'C') || + setweight(to_tsvector('${lang}', coalesce(timeline_text, '')), 'C'); + + RETURN NEW; + END; + $fn$ LANGUAGE plpgsql; + `); + console.log(` v124: update_page_search_vector() no longer indexes compiled_truth (was overflowing tsvector on large pages, #2704)`); + }, + }, ]; export const LATEST_VERSION = MIGRATIONS.length > 0 diff --git a/src/core/pglite-schema.ts b/src/core/pglite-schema.ts index a632fcc68..a42837a7c 100644 --- a/src/core/pglite-schema.ts +++ b/src/core/pglite-schema.ts @@ -1022,6 +1022,12 @@ ALTER TABLE pages ADD COLUMN IF NOT EXISTS search_vector tsvector; CREATE INDEX IF NOT EXISTS idx_pages_search ON pages USING GIN(search_vector); +-- #2704: compiled_truth (unbounded whole-page body) deliberately NOT +-- indexed — overflows Postgres's 1MB tsvector cap on large pages. +-- content_chunks.search_vector (chunk-grain, populated separately) is +-- what searchKeyword() actually queries. See migrate.ts's v124 migration +-- for the full rationale; keep in sync with that + reindex-search-vector.ts +-- + schema-embedded.ts. CREATE OR REPLACE FUNCTION update_page_search_vector() RETURNS trigger SET search_path = pg_catalog, public AS $$ DECLARE timeline_text TEXT; @@ -1033,7 +1039,6 @@ BEGIN NEW.search_vector := setweight(to_tsvector('english', coalesce(NEW.title, '')), 'A') || - setweight(to_tsvector('english', coalesce(NEW.compiled_truth, '')), 'B') || setweight(to_tsvector('english', coalesce(NEW.timeline, '')), 'C') || setweight(to_tsvector('english', coalesce(timeline_text, '')), 'C'); diff --git a/src/core/schema-embedded.ts b/src/core/schema-embedded.ts index d576fd1b0..dd0362bd5 100644 --- a/src/core/schema-embedded.ts +++ b/src/core/schema-embedded.ts @@ -830,6 +830,12 @@ ALTER TABLE pages ADD COLUMN IF NOT EXISTS search_vector tsvector; CREATE INDEX IF NOT EXISTS idx_pages_search ON pages USING GIN(search_vector); -- Function to rebuild search_vector for a page +-- #2704: compiled_truth (unbounded whole-page body) deliberately NOT +-- indexed — overflows Postgres's 1MB tsvector cap on large pages. +-- content_chunks.search_vector (chunk-grain, populated separately) is +-- what searchKeyword() actually queries. See migrate.ts's v124 migration +-- for the full rationale; keep in sync with that + reindex-search-vector.ts +-- + pglite-schema.ts. CREATE OR REPLACE FUNCTION update_page_search_vector() RETURNS trigger SET search_path = pg_catalog, public AS \$\$ DECLARE timeline_text TEXT; @@ -843,7 +849,6 @@ BEGIN -- Build weighted tsvector NEW.search_vector := setweight(to_tsvector('english', coalesce(NEW.title, '')), 'A') || - setweight(to_tsvector('english', coalesce(NEW.compiled_truth, '')), 'B') || setweight(to_tsvector('english', coalesce(NEW.timeline, '')), 'C') || setweight(to_tsvector('english', coalesce(timeline_text, '')), 'C'); diff --git a/src/schema.sql b/src/schema.sql index 69222e9a3..8c8faa224 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -826,6 +826,12 @@ ALTER TABLE pages ADD COLUMN IF NOT EXISTS search_vector tsvector; CREATE INDEX IF NOT EXISTS idx_pages_search ON pages USING GIN(search_vector); -- Function to rebuild search_vector for a page +-- #2704: compiled_truth (unbounded whole-page body) deliberately NOT +-- indexed — overflows Postgres's 1MB tsvector cap on large pages. +-- content_chunks.search_vector (chunk-grain, populated separately) is +-- what searchKeyword() actually queries. See migrate.ts's v124 migration +-- for the full rationale; keep in sync with that + reindex-search-vector.ts +-- + pglite-schema.ts. CREATE OR REPLACE FUNCTION update_page_search_vector() RETURNS trigger SET search_path = pg_catalog, public AS $$ DECLARE timeline_text TEXT; @@ -839,7 +845,6 @@ BEGIN -- Build weighted tsvector NEW.search_vector := setweight(to_tsvector('english', coalesce(NEW.title, '')), 'A') || - setweight(to_tsvector('english', coalesce(NEW.compiled_truth, '')), 'B') || setweight(to_tsvector('english', coalesce(NEW.timeline, '')), 'C') || setweight(to_tsvector('english', coalesce(timeline_text, '')), 'C'); diff --git a/test/fts-language-migration.serial.test.ts b/test/fts-language-migration.serial.test.ts index da7841da0..f341e17a4 100644 --- a/test/fts-language-migration.serial.test.ts +++ b/test/fts-language-migration.serial.test.ts @@ -1,6 +1,6 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; import type { BrainEngine } from '../src/core/engine.ts'; -import { MIGRATIONS, LATEST_VERSION } from '../src/core/migrate.ts'; +import { MIGRATIONS } from '../src/core/migrate.ts'; import { resetFtsLanguageCache } from '../src/core/fts-language.ts'; const ENV_KEY = 'GBRAIN_FTS_LANGUAGE'; @@ -24,9 +24,11 @@ describe('configurable_fts_language migration', () => { expect(ftsMig?.version).toBeGreaterThan(115); }); - test('fts migration is the latest migration', () => { - expect(MIGRATIONS.find(m => m.name === 'configurable_fts_language')?.version).toBe(LATEST_VERSION); - }); + // #2704 (v124, page_search_vector_drop_compiled_truth) landed after this + // migration — "is the latest migration" was only ever true at the + // moment v123 was added and would break on every subsequent migration, + // so it's removed rather than bumped to a hardcoded v124. The + // registration + shape assertions below don't depend on migration order. test('ftsMig uses handler (not static SQL) because language interpolation is dynamic', () => { const ftsMig = MIGRATIONS.find(m => m.name === 'configurable_fts_language'); diff --git a/test/page-search-vector-overflow.test.ts b/test/page-search-vector-overflow.test.ts new file mode 100644 index 000000000..8e2a121b6 --- /dev/null +++ b/test/page-search-vector-overflow.test.ts @@ -0,0 +1,93 @@ +/** + * #2704 — a single markdown page whose compiled_truth exceeds Postgres's + * hard 1,048,575-byte tsvector cap made update_page_search_vector() throw + * "string is too long for tsvector" INSIDE the pages UPSERT transaction, + * blocking the whole source's sync checkpoint (Sync BLOCKED) even though + * every other file in the run imported fine. + * + * v124 (migrate.ts) drops compiled_truth from the trigger — it was + * already redundant with content_chunks.search_vector (chunk-grain, + * populated separately and well under the tsvector cap), which is what + * searchKeyword() actually queries. + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; + +// #2704: the 1,048,575-byte tsvector cap is on to_tsvector's SERIALIZED +// OUTPUT (lexemes + position lists), not the raw input byte length — +// repeating the same few words produces a tiny deduplicated vector +// regardless of input size (verified: a 2.2MB string of 5 repeated words +// does NOT overflow). Genuinely diverse, mostly-unique tokens are what +// blows the output past the cap, matching a real large export (a Google +// Docs dump, a long mailing-list thread) where the words don't repeat +// like lorem-ipsum filler does. +const OVERSIZED_BODY = Array.from({ length: 200_000 }, (_, i) => `token${i.toString(36)}`).join(' '); // ~2MB, ~2.7MB serialized tsvector + +describe('#2704: oversized page body no longer overflows pages.search_vector', () => { + let engine: PGLiteEngine; + + beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + }, 60_000); + + afterAll(async () => { + await engine.disconnect(); + }, 60_000); + + beforeEach(async () => { + await resetPgliteState(engine); + }); + + test('putPage with a >1MB compiled_truth succeeds (previously threw "string is too long for tsvector")', async () => { + expect(OVERSIZED_BODY.length).toBeGreaterThan(1_048_575); + + const page = await engine.putPage('oversized-page', { + type: 'note', + title: 'Oversized Page', + compiled_truth: OVERSIZED_BODY, + }); + + expect(page).not.toBeNull(); + expect(page.slug).toBe('oversized-page'); + }, 30_000); + + test('an oversized page is still keyword-searchable via chunk-grain search after import', async () => { + // Mirrors import-file.ts: chunking is what actually feeds + // content_chunks.search_vector, independent of the pages-level + // trigger this fix touches. A distinctive token near the start proves + // the chunk (not just the page row) is queryable. + const distinctiveBody = `zzdistinctivetoken2704 ${OVERSIZED_BODY}`; + await engine.putPage('oversized-searchable', { + type: 'note', + title: 'Oversized Searchable', + compiled_truth: distinctiveBody, + }); + const { chunkText } = await import('../src/core/chunkers/recursive.ts'); + let chunkIndex = 0; + const chunks = chunkText(distinctiveBody).map((c) => ({ + chunk_index: chunkIndex++, + chunk_text: c.text, + chunk_source: 'compiled_truth' as const, + })); + await engine.upsertChunks('oversized-searchable', chunks); + + const results = await engine.searchKeyword('zzdistinctivetoken2704'); + expect(results.some((r) => r.slug === 'oversized-searchable')).toBe(true); + }, 30_000); + + test('normal-sized page search_vector still carries title/timeline signal (not fully inert)', async () => { + await engine.putPage('small-page', { + type: 'note', + title: 'zzTitleToken2704', + compiled_truth: 'short body', + }); + const rows = await engine.executeRaw<{ has_vector: boolean }>( + `SELECT search_vector IS NOT NULL AS has_vector FROM pages WHERE slug = 'small-page'`, + ); + expect(rows[0]?.has_vector).toBe(true); + }, 30_000); +});