fix: index extracted Portable Text prose in FTS, not raw JSON - #2313
Draft
edrpls wants to merge 2 commits into
Draft
fix: index extracted Portable Text prose in FTS, not raw JSON#2313edrpls wants to merge 2 commits into
edrpls wants to merge 2 commits into
Conversation
FTS5 tables were external-content (content='ec_<slug>'), which forces the index to mirror raw column values — and Portable Text fields store JSON, so structural tokens polluted the index (27-29% of it on an audited production database). Searching "normal" (a PT style value) matched 870/906 posts, "_type" matched every document, and snippets showed JSON fragments. Rebuild the FTS tables as self-contained FTS5 whose Portable Text columns hold extracted prose: every JSON string under a text, alt, caption, or code key (span text, image alt/caption, code blocks — the same semantics as extractPlainText). Extraction lives in SQL (json_tree) because the sync triggers cannot call into JS, with json_valid guarding legacy bare-string rows. Self-contained tables also retire the external-content 'delete' choreography and its corruption modes (migration 039's subject): removal is a plain DELETE, a harmless no-op for never-indexed rows, and INSERT OR REPLACE makes concurrent D1 populates converge. Migration 055 rebuilds every search-enabled collection's index and triggers on upgrade; the trigger SQL is lock-step with FTSManager per 039's precedent. The search query layer is unchanged — it joins ec_* by id for metadata, and snippet() now reads the stored prose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
json_tree exposes output columns named key/value/type/path and friends; a bare column reference inside the extraction subquery binds to those instead of the outer ec_* column, so populating a Portable Text field slugged with one of these names silently indexed NULL. Triggers were unaffected (NEW.-qualified). Qualify the populate and migration references with the content table name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 954cc3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
18 tasks
Contributor
Scope checkThis PR changes 772 lines across 8 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Contributor
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes full-text search indexing raw Portable Text JSON instead of prose. On the audited production deployment (Macabro festival site, emdash 0.31.1), structural tokens were 27–29% of the FTS index: searching
normal(a PT style value) matched 870/906 posts,_typematched 2,096 documents, and snippets showed JSON fragments.Why the fix isn't "just call
extractPlainText": the FTS sync is done by SQL triggers, which can't call into JS — and the tables were external-content FTS5 (content='ec_<slug>'), which requires the index to exactly mirror the raw column values (snippet() reads them, and the'delete'command must be fed the inserted values or the index corrupts — the exact corruption class migration 039 exists to fix). So indexing extracted text under external content is structurally impossible without materializing it into real columns on everyec_*table, which would touch every content write path in the codebase.Instead, this PR rebuilds the FTS tables as self-contained FTS5 whose Portable Text columns hold extracted prose, with the extraction done in SQL so triggers and population share one source of truth:
json_tree()collects every JSON string under atext,alt,caption, orcodekey — span text, image alt/caption, and code blocks, the same semantics asextractPlainTextintext-extraction.ts.json_valid()guards legacy rows holding a bare string (indexed as-is).'delete'choreography entirely: removal is a plainDELETE, a harmless no-op for never-indexed (soft-deleted) rows, so theSQLITE_CORRUPT_VTABcorruption class from SQLite Database Corrupt Error on new site #649/migration 039 can no longer occur.INSERT OR REPLACEmakes concurrent D1 populates converge (verified: a plain duplicate-rowid insert throws on self-contained FTS5).{"_type":"block"....Existing deployments rebuild automatically: migration
055_fts_plain_textdrops and recreates every search-enabled collection's FTS table and triggers and repopulates from content, following 039's structure (self-contained lock-step SQL copy,IF NOT EXISTSforms for D1's lockless concurrent migrators, defensive identifier validation). No manual reindex step.The search query layer is unchanged: it joins
ec_*byidfor metadata, bm25 weight positions and the snippet column index are preserved (same column order), and Postgres is unaffected (FTS5 is SQLite-gated throughout). Trade-off worth noting for review: a self-contained FTS table stores its own copy of the indexed text (external content stored none), while the inverted index itself shrinks by the removed structural tokens; net size impact on the audited data set is roughly neutral, and the correctness/corruption-immunity gains are structural.The 039 migration test's fixture was updated to explicitly construct the historical external-content table shape it needs (the current
FTSManagernow builds self-contained tables, on which the historical "broken" triggers are actually correct) — 039's own frozen migration SQL is untouched.Coordinates with the upcoming write-amplification fix (WHEN guards on these same triggers): whichever lands second rebases and ships its own lock-step migration, per 039's rule.
Found during a measured database audit of a production deployment.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. — n/a: no admin UI strings changedAI-generated code disclosure
Screenshots / test output
Failing first (on
main, before the fix):After the fix — new search-quality suite, migration 055 upgrade-path suite (pre-migration assertion proves the polluted baseline, post-migration assertions prove the cleanup and working triggers), plus all existing search/FTS/registry/migration suites:
🤖 Generated with Claude Code