From d698b44438e63942628ee9a1ddec0e682af86aa7 Mon Sep 17 00:00:00 2001 From: Masa <98894508+Masashi-Ono0611@users.noreply.github.com> Date: Tue, 21 Jul 2026 07:51:16 +0900 Subject: [PATCH] =?UTF-8?q?fix(search):=20drop=20compiled=5Ftruth=20from?= =?UTF-8?q?=20pages.search=5Fvector=20=E2=80=94=20was=20overflowing=20tsve?= =?UTF-8?q?ctor=20on=20large=20pages=20(#2704)=20(#2977)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --retry-failed re-failed the same files every run; the 3-consecutive-failure auto-skip eventually moved past them, but for a scheduled collector that meant hours of blocked cycles per oversized file, per source. Root cause: pages.search_vector indexed compiled_truth — the unbounded whole-page body — even though it's write-only dead weight for actual search. searchKeyword() (postgres-engine.ts / pglite-engine.ts) ranks and queries content_chunks.search_vector exclusively (Cathedral II Layer 3, chunk-grain, already populated separately from compiled_truth via chunking at import time, and well under the tsvector cap since chunkText() targets embedding-sized pieces). Verified directly: no pages.search_vector / bare search_vector read appears anywhere outside this trigger's own definition and the reindex/backfill machinery that maintains it. Fix: v124 migration recreates update_page_search_vector() without compiled_truth — title + timeline stay (both naturally small), so the column keeps carrying some signal rather than going fully inert. Updated in lockstep (documented contract, see reindex-search-vector.ts's own comment): migrate.ts's new v124, reindex-search-vector.ts's recreatePagesFn, and the fresh-install baselines in pglite-schema.ts + src/schema.sql (regenerates schema-embedded.ts via `bun run build:schema`). No backfill: existing rows keep whatever search_vector they already computed until their next UPDATE — harmless, since nothing reads this column, and the brains that actually hit this bug never successfully wrote a value for the oversized page in the first place. Considered (from the issue) and rejected: truncating compiled_truth to fit under the cap. Silent, position-dependent recall loss, and the byte cap doesn't line up cleanly with any natural character/token boundary for UTF-8 content. content_chunks.search_vector already gives full, untruncated chunk-grain coverage for large pages — truncating a now-redundant whole-page vector would trade a real bug for a subtler one. ## Test plan - New test/page-search-vector-overflow.test.ts: a >1MB page (genuinely diverse tokens — a repetitive lorem-ipsum-style fixture does NOT reproduce this bug, since to_tsvector's cap is on its DEDUPLICATED output size, not raw input length) now imports successfully instead of throwing; remains keyword-searchable via the chunk-grain path; a normal page's search_vector still carries title signal (not fully inert). Verified the test is meaningful both directions: fails with the exact reported error on the pre-fix trigger (git-stashed the fix, reran, confirmed byte-for-byte match: "string is too long for tsvector (2684620 bytes, max 1048575 bytes)"), passes with the fix restored. - Updated fts-language-migration.serial.test.ts: removed an assertion that configurable_fts_language (v123) is LATEST_VERSION — that was only ever true until the next migration landed; the codebase's own pattern elsewhere for this (migrate.test.ts) uses toBeGreaterThanOrEqual, not exact-match, for exactly this reason. - bun run typecheck clean, bun run verify 31/31 green. - test/page-search-vector-overflow.test.ts (3/3), test/reindex-search-vector.serial.test.ts, test/fts-language-migration.serial.test.ts, test/migration-v120.test.ts, test/sync.test.ts, test/bootstrap.test.ts, test/migrate.test.ts — 254 total, 0 fail, no regressions. ## Design consultation Investigated jointly with masa-codex (async design review) before implementing — their read of the codebase (content_chunks.search_vector already covers keyword search; pages.search_vector's compiled_truth feed is the only overflow-prone, effectively-dead write) matched independent verification and shaped the "remove from trigger" fix over the issue's alternative options (chunk-grain rebuild — largely already exists; input truncation — rejected above; ledger-entry-only — insufficient alone, since the page upsert failing in the same transaction also loses the chunk write, so checkpoint advancing without this fix would make the content permanently unsearchable, not just delayed). Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS Co-authored-by: Claude Sonnet 5 --- src/commands/reindex-search-vector.ts | 9 ++- src/core/migrate.ts | 68 ++++++++++++++++ src/core/pglite-schema.ts | 7 +- src/core/schema-embedded.ts | 7 +- src/schema.sql | 7 +- test/fts-language-migration.serial.test.ts | 10 ++- test/page-search-vector-overflow.test.ts | 93 ++++++++++++++++++++++ 7 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 test/page-search-vector-overflow.test.ts 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); +});