27% of the knowledge base was invisible to semantic search - #141
Conversation
The embedding client truncates its input at 8,000 characters. On a live 1,206-page store 217 pages (18%) exceed that, discarding roughly 1.54M characters — 27% of all body text. A scan for globally-unique tokens found 111 of 114 large pages carry content that exists ONLY past the cutoff, so that text could not be reached semantically at any ranking quality. Long multi-topic pages also average into one vector that is weakly similar to everything on them. Index oversized pages a second time at chunk granularity: split on markdown h1-h3 boundaries, pack sections toward ~3,000 chars, prefix each chunk with its page title and heading path, and store the vectors in a new chunk_embeddings table. vector_search now scans both tiers and collapses to the best score per page, so a chunk hit returns the excerpt that actually matched instead of the page's opening 240 characters. Pages under the threshold are deliberately untouched and keep exactly one vector. Vector count is per-turn latency, so this grows the index ~2x rather than the ~6x that chunking everything would cost. That latency headroom comes from top_matches(), which replaces the interpreted cosine loop with a numpy matmul: 105ms -> 0.7ms for 1,203 vectors at dim 3072. numpy stays OPTIONAL because the suite must pass from a bare clone, so an exact pure-Python fallback remains and an equivalence test asserts both paths rank identically — a fast path that silently reorders results would be worse than no fast path. Measured on the real corpus with 40 tail-only queries (each an author-written section heading, globally unique, absent from the page's first 8,000 chars) through the full FTS+vector+rerank pipeline: recall@1 45% -> 60%, recall@5 52% -> 60%, MRR 0.467 -> 0.600, median latency 1424ms -> 1621ms. Six queries improved, none regressed. The lexical tier was already rescuing some tail content, so this is smaller than a vector-only comparison would imply. Chunk rows are invalidated with their page on edit and delete, chunking is verified lossless against all 218 oversized pages in the live corpus, and the nightly doctor and backfill script both maintain the new tier.
The scan selected p.body for every embedded page and c.text for every chunk, then used those columns only to cut 240-char snippets for the handful of winners. Measured on a 1,200-page store: ~29MB peak allocation per query, on the gateway thread, for text discarded immediately. The scan now carries rel_path plus the embedding blob. Candidates are collapsed to best-score-per-page first, then page and chunk text are hydrated for the survivors only. Peak allocation for the scan drops to 14.3MB and median query latency improved; recall on the 40-query tail set is unchanged within reranker run-to-run variance. Also handles a page deleted between scan and hydration instead of raising a KeyError, and adds a test asserting the scan SQL never selects a body column.
- chunk_embedding_text prepends title, heading path and tags. Those are all page-derived, so a pathological one could consume the embedder's entire input budget and push out the chunk body. Each field is now capped. - A failure partway through the chunk write loop (bad vector, page deleted concurrently) left an open write transaction on a cached per-thread connection after the process lock was released, blocking the next writer. The batch now rolls back and reports zero, and an unpackable vector is skipped rather than aborting the batch.
A page rescued by the chunk tier could still be judged on its opening lines. Fusion prefers the highlighted lexical snippet for display, which is right for display, but it overwrote the chunk excerpt entirely -- so when a generic lexical match landed near the top of a long page, the reranker never saw the passage that actually matched the query. The chunk excerpt and its heading path are now carried separately as chunk_evidence, set on first sight of the chunk row regardless of tier order, and given a guaranteed labelled slot in the reranker input. Display behavior is unchanged. (cherry picked from commit 6e04687c60ec07f2607d61839b89cdec65087480)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3782eec37d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # force=True is only needed when the model name itself changed and | ||
| # we want the whole corpus rewritten rather than row-by-row. | ||
| changed = store.backfill_embeddings(force=bool(coverage["foreign_model_rows"])) | ||
| chunked = store.backfill_chunk_embeddings(force=bool(coverage["foreign_model_rows"])) |
There was a problem hiding this comment.
On an existing semantic store where every page already has a valid page-level embedding, coverage["ok"] is true, so this call is skipped even under --repair. Such stores are the primary upgrade case for this change: their new chunk_embeddings table starts empty and remains empty indefinitely, leaving the oversized-page tails inaccessible unless an operator separately reruns the one-time backfill script. Chunk coverage needs to be checked and repaired independently of page-embedding coverage.
Useful? React with 👍 / 👎.
|
|
||
| t0 = time.time() | ||
| n = st.backfill_embeddings(force=args.force) | ||
| c = st.backfill_chunk_embeddings(force=args.force) |
There was a problem hiding this comment.
When page embedding succeeds but the chunk request fails, backfill_chunk_embeddings() catches the error and returns 0, which is indistinguishable here from having no chunk work. The script then validates only page coverage and exits 0 with OK: full coverage, even if oversized pages have no usable chunks and their tails remain semantically invisible. Verify expected chunk coverage or propagate a distinct failure result before reporting success.
Useful? React with 👍 / 👎.
Corrected measurementThe earlier note flagged that the original 40-query eval set was biased. Here is the corrected result. Setup. Full-corpus replica (1,207 pages, 1,207 page vectors + 1,404 chunk vectors), scored through the complete production pipeline: FTS5 + vector + live Cohere-class reranker. Four arms, isolating each change. Held-out paraphrase set — queries rewritten as lowercase natural questions that emit zero phrase clauses, so they cannot favour the sibling ranking PR:
Original heading-derived set (biased toward the ranking PR, kept for continuity):
Two independent repetitions of the baseline/chunk pair are reported as ranges; the reranker has run-to-run variance and the gap holds across both. Conclusion. The chunk tier is where the retrieval gain actually comes from, and it holds on unbiased queries: +12 points recall@1, +5 pages recovered at recall@5. This is consistent with the mechanism — 27% of body text was previously unreachable. Two earlier numbers in this PR's description were measured on a store whose |
Two findings from the Codex reviewer, both about the chunk tier never being built on the stores that need it most. The nightly doctor's repair path was gated on `not coverage["ok"]`, which counts PAGES only. On an existing store every page is already embedded, so that is True and the chunk backfill never ran -- exactly the upgrade case this feature exists for. Chunk backfill is now independent of page coverage. The backfill script printed "OK: full coverage" whenever page coverage was complete, even if every oversized page was missing its chunks. It now checks chunk coverage separately against oversized_page_count() and exits non-zero with a specific message.
|
Both findings were addressed in cdc11ca (the commit they are flagged against); the bot appears to have re-posted them rather than re-evaluated.
|
1 participant
The problem
The embedding client truncates its input at 8,000 characters (
embeddings.py), and pages were embedded whole. Measured on a live 1,207-page store:A page's later sections were unreachable by semantic search, with no error and no log line. Retrieval looked healthy.
The change
Oversized pages are split on h1-h3 headings into sub-page chunks, each embedded separately into a new
chunk_embeddingstable.vector_search()scans both tiers and collapses to best-score-per-page, so a page can win on either. A chunk hit carries its own excerpt, so the snippet shows the passage that actually matched.Only pages over the cap are chunked. Vector count is per-turn scan cost, so small pages stay single-vector by design: 1,207 pages produce ~2,374 vectors, not 10,000.
top_matches()adds a numpy-accelerated scan with a pure-Python fallback (numpy stays optional; the test suite passes with the import blocked, and both paths are asserted to agree).Review fixes included
Testing
277 tests pass, with and without numpy. Chunking verified lossless across all 218 oversized pages on a full-corpus replica: zero text dropped, no chunk over the cap.
Note on measurement
An earlier A/B on a 40-query set showed recall@1 45% → 60%. That eval set was later found to be biased (queries were verbatim section headings). A corrected held-out evaluation is in progress and will be posted here before merge. The lossless-chunking and memory results above are independent of that eval.