Skip to content

fix(store): bound FTS search result hydration for oversized rows - #963

Open
Tomodad wants to merge 2 commits into
mksglu:nextfrom
Tomodad:codex/fix-fts-highlight-bounded-search
Open

fix(store): bound FTS search result hydration for oversized rows#963
Tomodad wants to merge 2 commits into
mksglu:nextfrom
Tomodad:codex/fix-fts-highlight-bounded-search

Conversation

@Tomodad

@Tomodad Tomodad commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs #947.

FTS ranking currently selects full chunks.content and materializes highlight() before the server clips output. A legacy 53 MiB row can therefore turn a millisecond MATCH into a multi-minute synchronous highlight operation.

This PR keeps the scope to the search path:

  • rank porter/trigram candidates using bounded metadata only (rowid, title, type, timestamp, source, rank, session id);
  • preserve the historical full content + FTS5 highlight result for normal rows at or below MAX_CHUNK_BYTES;
  • preserve ranked-row identity on Node 22/better-sqlite3 using an FTS5-compatible rowid constraint;
  • hydrate legacy oversized rows with a match-position lookup and a bounded substr() window;
  • preserve source/content-type filters, session attribution, BM25, RRF, and result markers;
  • rebuild the tracked server/CLI bundles.

It deliberately does not implement Markdown chunking (#878/#898), ingestion budgets (#961), trigram/vocabulary policy (#962), or a whole-handler deadline/worker model.

TDD

RED on next after adding the regression tests:

PASS (0) FAIL (3) skipped (153)
porter content:   1,200,022 chars
trigram content:  1,200,031 chars
filtered content: 1,200,025 chars

Node 22.5 CI exposed a second RED after the first push:

existing intent-search scenarios: 3 failed
ranked metadata rowids: distinct
hydrated content: first MATCH row repeated for porter and trigram

A focused multi-row regression now asserts that each ranked porter/trigram candidate hydrates its own row.

GREEN:

tests/core/search.test.ts: 157 passed
tests/core/search.test.ts + tests/store.test.ts: 279 passed, 1 skipped
search/highlight/snippet server subset: 26 passed
npm run typecheck: passed
npm run build: passed (bundle and asymmetric-drift assertions included)
git diff --check: passed

A complete tests/core/server.test.ts run in the managed Windows sandbox also exposed three pre-existing process/path tests that fail with STACK_TRACE_ERROR/EPERM; the directly affected search/formatting subset is green.

Before/after: isolated 1.2 MiB legacy row

path before after returned content/highlight
porter 20.445 ms 11.078 ms 1,200,022/1,200,024 -> 315/317 chars
trigram 30.406 ms 9.248 ms 1,200,022/1,200,024 -> 315/317 chars
fallback 49.629 ms 17.163 ms 1,200,022/1,200,024 -> 315/317 chars

Node 22.5 after the rowid-compatibility follow-up: porter 4.977 ms, trigram 4.337 ms, fallback 8.046 ms; all still returned 315/317 chars with the match preserved.

Compatibility

Normal capped chunks retain full-content/highlight behavior. Only legacy rows that already violate MAX_CHUNK_BYTES return a bounded window. Proximity reranking for those legacy rows therefore uses the window rather than the full document, which can change ordering among multiple giant rows. The metadata-first + hydration path adds a small DB round-trip cost for normal rows (~0.045 ms/search in a local 500-iteration microbenchmark).

@Tomodad
Tomodad marked this pull request as ready for review July 16, 2026 09:06
@Tomodad

Tomodad commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Fresh Windows/Codex recurrence on the official marketplace build v1.0.169 (2026-07-26):

  • ctx_batch_execute hit the host error timed out awaiting tools/call after 300s during bounded local inspection.
  • A subsequent ctx_execute, limited to walking the installed plugin's src tree, also hit the same 300s host timeout.
  • After switching away from CTX, bounded native RTK/PowerShell probes completed in seconds.
  • The active content store was large (a20afb028fd78de8.db: 712,667,136 bytes; WAL: 700,503,032 bytes), but as noted in [Bug]: ctx_batch_execute timeout does not bound indexing/search, so Claude Code agents can hang for hours #947, database size alone is not causal. Without phase watchdogs, I cannot attribute this fresh incident specifically to the full-row highlight() path.

This is fresh evidence that v1.0.169 can still expose users to a 300-second blocking failure. PR #963 remains the focused fix for the confirmed oversized-row hydration path; broader whole-handler deadline/worker/backpressure work still belongs in #947.

All five checks on this PR are green and GitHub currently reports it clean/mergeable. @mksglu, would you prefer any further split/rebase or an additional regression fixture before review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant