feat(rag): contextual ingest done right — heading paths, context stored beside the body (#216)#243
Merged
Merged
Conversation
…de a verbatim body Completes Variant A of #216 (structural contextual retrieval) per the issue's recommended storage split, extending the first cut from #224: - New `blocks.context` column (sql/0007, `Migrator::ensure_block_context` on every boot, SCHEMA_VERSION 5 → 6): the situating prefix now lives BESIDE the chunk instead of overwriting it. `blocks.body` is verbatim again — snippets, `expand` and byte-span provenance are clean. - Prefix format grows the heading path: `[<title> › <heading path> › p.<page>]`, derived deterministically from the markdown ATX hierarchy open at each chunk's byte_start (`heading_path_at`); a heading equal to the title is deduped. No inference, no new deps. - The contextualised text feeds retrieval only: dense embeddings embed `context\n body` (`IndexChunk::embed_text`), the FTS index covers ('body', 'context') in both the fresh migration and `refresh_fts`, and the rerank passage is `concat_ws(chr(10), context, body)`. - `materialize_document` / `write_document_blocks` take `&[IndexChunk]` ({context, body}); live ingest and rebuild share `contextualized_chunks`, so a from-scratch rebuild reproduces identical rows — rebuild is also the operator's re-embed cutover after flipping `ESCUREL_INGEST_CONTEXTUALIZE` (off | structural, default structural, unchanged from #224). - Variant B (LLM-situated context) stays out of scope behind future config, per the issue. Tests (red first, real DuckDB + FsStore + HashEmbedder, no mocks) in crates/escurel-index/tests/contextual_retrieval.rs: - title_token_retrieves_chunk_but_display_body_stays_clean - stored_matrix_body_verbatim_context_carries_prefix - contextualize_off_stores_no_context - heading_context_disambiguates_similar_chunks - rerank_passage_carries_structural_context plus document.rs unit tests for heading_path_at / the prefix format / the storage split. Full gate green: fmt, clippy -D warnings, workspace tests, release build. Refs #216. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Open
7 tasks
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.
Summary
Completes Variant A of #216 per the issue's own 'Open question' recommendation. #224 had shipped a
[title › p.N]prefix that overwroteblocks.body; this PR moves the context OUT of the display text and adds the heading path:[<title> › <heading path…> › p.<page>]— deterministic markdown-outline derivation, no LLM (Variant B stays behind future config).blocks.bodyverbatim (snippets/expand clean); newblocks.contextcolumn carries the prefix; dense embeddings embedcontext\nbody; FTS indexes both fields; the rerank passage concatenates them — one consistent indexed representation, zero consumer churn on thesearchtool.SCHEMA_VERSION5→6 (manifest guard refuses mismatched artifacts),ensure_block_contextadds the column idempotently on boot, andrebuildre-derives chunks+contexts from retained blobs — flippingESCUREL_INGEST_CONTEXTUALIZE+ rebuild is the operator story. Rows ingested under feat(rag): contextual retrieval Variant A (structural) — prefix chunks with [title › p.N] #224's baked-in prefix normalize on rebuild.Test plan (red-first, no mocks —
crates/escurel-index/tests/contextual_retrieval.rs)title_token_retrieves_chunk_but_display_body_stays_clean,stored_matrix_body_verbatim_context_carries_prefix(exact prefix pinned),heading_context_disambiguates_similar_chunks(the e2e point of the feature),rerank_passage_carries_structural_context,contextualize_off_stores_no_context, plus document.rs unit tests. All ranking-sensitive suites pass unmodified. Four-command gate green.Not in this PR
Variant B (planned later, behind config); the eval on/off delta — the eval harness ingests via its own pre-embedded pass and needs a small follow-up to feed contextualized chunks (command documented in the issue comment).
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.