mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 19:49:14 +00:00
fix(search): drop compiled_truth from pages.search_vector — was overflowing tsvector on large pages (#2704) (#2977)
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 <noreply@anthropic.com>
This commit is contained in:
+6
-1
@@ -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');
|
||||
|
||||
|
||||
Reference in New Issue
Block a user