From 094c0f4b71c623be998472b3a48ef0da0c8b09d6 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 09:42:36 +0000 Subject: [PATCH 1/5] test(eval): add full-corpus dataset variant and harness README dataset-full.jsonl retargets the 55 questions to the eval-full namespace (clean full Italian Constitution from Wikisource, 72 chunks + official OHCHR UDHR PDF, 6 chunks). The excerpt corpus saturates hit rate at 100%; the full corpus is discriminative (hit 89.1%, MRR 0.806 at baseline). README documents corpus provenance, the senato.it PDF extraction trap, and how to run/extend the harness. Co-Authored-By: Claude Fable 5 --- tests/eval/README.md | 52 +++++++++++++++++++++++++++++++++ tests/eval/dataset-full.jsonl | 55 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tests/eval/README.md create mode 100644 tests/eval/dataset-full.jsonl diff --git a/tests/eval/README.md b/tests/eval/README.md new file mode 100644 index 00000000..e09afe41 --- /dev/null +++ b/tests/eval/README.md @@ -0,0 +1,52 @@ +# RAG evaluation harness (TECH-002) + +Two-stage evaluation against a running Vektra stack. Both scripts are pure HTTP +clients: they require `VEKTRA_API_URL` and `VEKTRA_API_KEY` in the environment. + +```bash +make eval-retrieval # /api/v1/search (no LLM) - hit rate, MRR, precision@k +make eval-e2e # /api/v1/query (full pipeline + LLM) - grounded rate, latency +# extra args: +make eval-retrieval EVAL_ARGS="--dataset tests/eval/dataset-full.jsonl --output tests/eval/results_retrieval_full.jsonl" +``` + +Results are written as JSONL next to the datasets (`results_*.jsonl`, gitignored: +record aggregates in the active `.s2s/plans/` file and in vektra-internal). + +## Datasets + +| File | Namespace | Corpus | +|------|-----------|--------| +| `dataset.jsonl` | `default` | Excerpt corpus, 12 chunks: `costituzione_italiana.md` v2 (6), `udhr_excerpts.md` v2 (4), `sample.pdf` (2). Hit rate saturates here; useful for smoke/regression, not for tuning. | +| `dataset-full.jsonl` | `eval-full` | Full-document corpus, 78 chunks: clean full Italian Constitution (72) + official UDHR English PDF (6). Same 55 questions, discriminative metrics. | + +Both files share the same 55 questions (21 factual, 15 reasoning, 10 multi-chunk, +9 adversarial without ground truth; 37 IT / 18 EN). Entry shape: +`{id, question, expected_keywords, namespace, category, language}`. Relevance is +keyword-based (`expected_keywords`, diacritic-insensitive substring match), so it +is chunking-independent and survives reingestion. + +## Corpus provenance (eval-full) + +- `costituzione-full-clean.md`: full text of the Italian Constitution converted + from Wikisource (`https://it.wikisource.org/api/rest_v1/page/html/Costituzione_della_Repubblica_italiana`, + CC BY-SA), metadata header stripped. Kept outside the repo at + `/mnt/ai/datasets/vektra-eval/` on the dev machine. +- `udhr-en-ohchr.pdf`: official OHCHR English UDHR + (`https://www.ohchr.org/sites/default/files/UDHR/Documents/UDHR_Translations/eng.pdf`). +- Do NOT use the senato.it combined PDF (`costituzione.pdf`, 506 pages): its print + layout breaks pdfplumber extraction (fused words like + `COSTITUZIONEDELLAREPUBBLICAITALIANA`, preserved hyphenation) and invalidates + keyword ground truth. Measured impact: IT hit rate 72% garbled vs 88% clean on + the same questions. Worth keeping in mind as a future "dirty extraction" test + case, but never as a retrieval-quality corpus. + +To rebuild the corpus: download the two sources, then +`scripts/ingest.sh eval-full` for each. + +## Adding questions + +Append JSONL entries with a unique `id`, the target `namespace`, and 1-3 +`expected_keywords` that only appear in the passages that truly answer the +question. Adversarial entries (expected refusal) omit `expected_keywords` and are +reported separately (`has_ground_truth: false`). diff --git a/tests/eval/dataset-full.jsonl b/tests/eval/dataset-full.jsonl new file mode 100644 index 00000000..c486de7f --- /dev/null +++ b/tests/eval/dataset-full.jsonl @@ -0,0 +1,55 @@ +{"id": "IT-F-01", "question": "Quali sono i diritti inviolabili dell'uomo riconosciuti dalla Costituzione italiana?", "expected_keywords": ["diritti inviolabili", "formazioni sociali"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-02", "question": "Su cosa si fonda la Repubblica italiana?", "expected_keywords": ["fondata sul lavoro"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-03", "question": "Cosa stabilisce l'articolo 3 della Costituzione sull'uguaglianza?", "expected_keywords": ["pari dignita' sociale", "senza distinzione"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-04", "question": "A chi appartiene la sovranita' nella Repubblica italiana?", "expected_keywords": ["sovranita' appartiene al popolo"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-05", "question": "Cosa prevede la Costituzione italiana riguardo al diritto al lavoro?", "expected_keywords": ["diritto al lavoro", "condizioni che rendano effettivo"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-06", "question": "Come e' composta la bandiera della Repubblica italiana?", "expected_keywords": ["tricolore", "verde, bianco e rosso"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-07", "question": "Cosa tutela l'articolo 9 della Costituzione?", "expected_keywords": ["cultura", "ricerca scientifica", "paesaggio"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-08", "question": "Cosa stabilisce la Costituzione sulla liberta' personale?", "expected_keywords": ["liberta' personale", "inviolabile", "autorita' giudiziaria"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-09", "question": "Cosa prevede la Costituzione sulle minoranze linguistiche?", "expected_keywords": ["minoranze linguistiche", "tutela"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-10", "question": "Quali sono le garanzie per la liberta' di corrispondenza?", "expected_keywords": ["corrispondenza", "inviolabili", "autorita' giudiziaria"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-11", "question": "Cosa dice la Costituzione sul diritto di riunione?", "expected_keywords": ["riunirsi pacificamente", "senz'armi"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-12", "question": "Cosa prevede l'articolo 32 sulla salute?", "expected_keywords": ["salute", "fondamentale diritto", "cure gratuite"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-13", "question": "Cosa dice la Costituzione sull'obbligo scolastico?", "expected_keywords": ["istruzione inferiore", "obbligatoria e gratuita", "otto anni"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-14", "question": "Qual e' il rapporto tra Stato e Chiesa nella Costituzione?", "expected_keywords": ["indipendenti e sovrani", "Patti Lateranensi"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-F-15", "question": "Cosa prevede l'articolo 21 sulla liberta' di pensiero?", "expected_keywords": ["manifestare liberamente", "pensiero", "stampa"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "IT-R-01", "question": "Qual e' il legame tra il principio di uguaglianza e il diritto al lavoro nella Costituzione?", "expected_keywords": ["uguaglianza", "lavoro"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-02", "question": "Come si conciliano autonomia locale e unita' nazionale nella Costituzione italiana?", "expected_keywords": ["autonomie locali", "una e indivisibile", "decentramento"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-03", "question": "In che modo la Costituzione bilancia liberta' di stampa e intervento giudiziario?", "expected_keywords": ["stampa", "sequestro", "autorita' giudiziaria"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-04", "question": "Quali limiti pone la Costituzione alla liberta' di circolazione?", "expected_keywords": ["circolare", "soggiornare", "sanita'", "sicurezza"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-05", "question": "Come protegge la Costituzione i cittadini capaci ma privi di mezzi economici nel percorso scolastico?", "expected_keywords": ["capaci e meritevoli", "borse di studio"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-06", "question": "In che modo la Costituzione italiana affronta il rapporto tra diritti individuali e solidarieta' sociale?", "expected_keywords": ["diritti inviolabili", "doveri inderogabili", "solidarieta'"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-07", "question": "Quali sono le condizioni per limitare la liberta' personale secondo la Costituzione?", "expected_keywords": ["atto motivato", "autorita' giudiziaria", "casi e modi previsti dalla legge"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-08", "question": "Come affronta la Costituzione la liberta' religiosa per le confessioni diverse dalla cattolica?", "expected_keywords": ["confessioni religiose", "egualmente libere", "intese"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-09", "question": "Qual e' il rapporto tra trattamento sanitario obbligatorio e rispetto della persona nella Costituzione?", "expected_keywords": ["trattamento sanitario", "disposizione di legge", "rispetto della persona umana"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "IT-R-10", "question": "Come la Costituzione tutela l'ambiente rispetto all'attivita' economica?", "expected_keywords": ["ambiente", "biodiversita'", "attivita' economica", "fini sociali e ambientali"], "namespace": "eval-full", "category": "reasoning", "language": "it"} +{"id": "EN-F-01", "question": "What does Article 1 of the UDHR say about human beings?", "expected_keywords": ["born free and equal", "dignity and rights"], "namespace": "eval-full", "category": "factual", "language": "en"} +{"id": "EN-F-02", "question": "What does the UDHR say about slavery?", "expected_keywords": ["slavery", "servitude", "prohibited"], "namespace": "eval-full", "category": "factual", "language": "en"} +{"id": "EN-F-03", "question": "What rights does Article 19 of the UDHR protect?", "expected_keywords": ["freedom of opinion", "expression", "information"], "namespace": "eval-full", "category": "factual", "language": "en"} +{"id": "EN-F-04", "question": "What does the UDHR say about education?", "expected_keywords": ["right to education", "free", "compulsory"], "namespace": "eval-full", "category": "factual", "language": "en"} +{"id": "EN-F-05", "question": "What does Article 9 of the UDHR state about arrest?", "expected_keywords": ["arbitrary arrest", "detention", "exile"], "namespace": "eval-full", "category": "factual", "language": "en"} +{"id": "EN-R-01", "question": "How does the UDHR connect the right to work with human dignity?", "expected_keywords": ["right to work", "remuneration", "dignity"], "namespace": "eval-full", "category": "reasoning", "language": "en"} +{"id": "EN-R-02", "question": "What is the relationship between marriage and equality in the UDHR?", "expected_keywords": ["marry", "equal rights", "free and full consent"], "namespace": "eval-full", "category": "reasoning", "language": "en"} +{"id": "EN-R-03", "question": "How does the UDHR protect against discrimination?", "expected_keywords": ["without distinction", "equal protection", "discrimination"], "namespace": "eval-full", "category": "reasoning", "language": "en"} +{"id": "EN-R-04", "question": "What protections does the UDHR provide for motherhood and childhood?", "expected_keywords": ["motherhood", "childhood", "special care"], "namespace": "eval-full", "category": "reasoning", "language": "en"} +{"id": "EN-R-05", "question": "How does the UDHR balance individual rights with government authority?", "expected_keywords": ["will of the people", "elections", "universal", "suffrage"], "namespace": "eval-full", "category": "reasoning", "language": "en"} +{"id": "MC-01", "question": "Come si confrontano le garanzie sulla liberta' personale nella Costituzione italiana e nella Dichiarazione Universale?", "expected_keywords": ["liberta' personale", "arbitrary arrest"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-02", "question": "Quali analogie esistono tra l'articolo 3 della Costituzione italiana e l'articolo 2 della UDHR sulla non discriminazione?", "expected_keywords": ["senza distinzione", "without distinction"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-03", "question": "How do both the Italian Constitution and the UDHR address freedom of religion?", "expected_keywords": ["fede religiosa", "freedom of thought"], "namespace": "eval-full", "category": "multi-chunk", "language": "en"} +{"id": "MC-04", "question": "Quali somiglianze ci sono nel trattamento del diritto all'istruzione tra Costituzione italiana e UDHR?", "expected_keywords": ["istruzione", "education", "obbligatoria"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-05", "question": "How do both documents address the right to work and fair employment?", "expected_keywords": ["diritto al lavoro", "right to work"], "namespace": "eval-full", "category": "multi-chunk", "language": "en"} +{"id": "MC-06", "question": "Quali differenze ci sono nella tutela della famiglia tra la Costituzione italiana e la UDHR?", "expected_keywords": ["famiglia", "matrimonio", "family"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-07", "question": "Come affrontano entrambi i documenti il diritto alla privacy e all'inviolabilita' del domicilio?", "expected_keywords": ["domicilio", "privacy", "correspondence"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-08", "question": "What similarities exist in how both documents treat freedom of expression and press?", "expected_keywords": ["pensiero", "expression", "stampa"], "namespace": "eval-full", "category": "multi-chunk", "language": "en"} +{"id": "MC-09", "question": "Come entrambi i documenti proteggono il diritto alla salute e a un tenore di vita adeguato?", "expected_keywords": ["salute", "health", "well-being"], "namespace": "eval-full", "category": "multi-chunk", "language": "it"} +{"id": "MC-10", "question": "How do the Italian Constitution and UDHR compare on the presumption of innocence and fair trial?", "expected_keywords": ["presumed innocent", "autorita' giudiziaria"], "namespace": "eval-full", "category": "multi-chunk", "language": "en"} +{"id": "ADV-01", "question": "Cosa dice la Costituzione italiana sul diritto di voto degli stranieri?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "it"} +{"id": "ADV-02", "question": "Qual e' la pena prevista dalla Costituzione per chi viola i diritti fondamentali?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "it"} +{"id": "ADV-03", "question": "What does the UDHR say about the right to bear arms?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "en"} +{"id": "ADV-04", "question": "Cosa stabilisce la Costituzione italiana sulla pena di morte?", "expected_keywords": ["pena di morte", "non e' ammessa"], "namespace": "eval-full", "category": "factual", "language": "it"} +{"id": "ADV-05", "question": "What is the UDHR's position on artificial intelligence and data privacy?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "en"} +{"id": "ADV-06", "question": "Quanti articoli ha la Costituzione italiana?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "it"} +{"id": "ADV-07", "question": "Who wrote the Universal Declaration of Human Rights?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "en"} +{"id": "ADV-08", "question": "Cosa dice la Costituzione sul numero massimo di mandati presidenziali?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "it"} +{"id": "ADV-09", "question": "What penalties does the UDHR prescribe for nations that violate human rights?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "en"} +{"id": "ADV-10", "question": "Come regolamenta la Costituzione italiana l'uso dei social media?", "expected_keywords": [], "namespace": "eval-full", "category": "adversarial", "language": "it"} From 746ddc5a5229617f376abcbfc0e98f5e4471a8bf Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 21:14:44 +0000 Subject: [PATCH 2/5] feat(rag): plumb parent chunk linkage and expansion step (FEAT-017) Store time: DualStrategyChunking parents now carry their own hierarchy id; run_ingest remaps chunker-local ids to deterministic stored ids (uuid5(doc_id, position)) and sets ChunkEmbedding.parent_id on children. Qdrant persists parent_id as a top-level payload key; pgvector fills the existing (always-NULL until now) parent_id column and honors caller ids. Search time: both providers exclude chunk_level=parent from search results (parents are context material, not retrieval targets). New VectorStoreProvider.retrieve(namespace, chunk_ids) fetches chunks by id. Query time: AdvancedQueryPipeline step 6.5 (VEKTRA_PARENT_EXPANSION_ENABLED, default false) replaces child text with the parent chunk text after the retrieval filter and before token budgeting (ARCH-055); children of the same parent collapse into the highest-scored one. Trace records children_expanded, siblings_merged, parents_fetched. Co-Authored-By: Claude Fable 5 --- .env.example | 1 + docs/reference/configuration.md | 1 + .../src/vektra_core/advanced_pipeline.py | 71 ++++++++++++++++++ vektra-index/src/vektra_index/adapters.py | 11 +++ .../src/vektra_index/providers/pgvector.py | 73 ++++++++++++++++++- .../src/vektra_index/providers/qdrant.py | 46 +++++++++++- vektra-ingest/src/vektra_ingest/chunking.py | 5 +- vektra-ingest/src/vektra_ingest/pipeline.py | 14 ++++ vektra-ingest/tests/test_dual_chunking.py | 15 ++-- vektra-shared/src/vektra_shared/config.py | 8 ++ vektra-shared/src/vektra_shared/protocols.py | 6 ++ vektra-shared/src/vektra_shared/types.py | 2 + 12 files changed, 239 insertions(+), 14 deletions(-) diff --git a/.env.example b/.env.example index 5cdbec2f..90a215fa 100644 --- a/.env.example +++ b/.env.example @@ -84,6 +84,7 @@ # VEKTRA_QUERY_PIPELINE=advanced # VEKTRA_MIN_RELEVANCE_SCORE=0.15 # VEKTRA_CHUNK_DEDUP_ENABLED=true +# VEKTRA_PARENT_EXPANSION_ENABLED=false # VEKTRA_RESPONSE_TOKEN_RESERVE=2048 # VEKTRA_CONTEXT_CHUNK_RATIO=0.6 # VEKTRA_PROMPT_TEMPLATES_DIR= diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index c0753eed..48313c8f 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -81,6 +81,7 @@ The model name must match the vLLM `--model` path exactly (e.g., `/models/qwen35 | `VEKTRA_QUERY_PIPELINE` | str | `advanced` | Pipeline implementation: `simple`, `advanced` | | `VEKTRA_MIN_RELEVANCE_SCORE` | float | `0.15` | Minimum relevance score for chunk inclusion (0.0-1.0). Safety net filter; top-k is the primary control. | | `VEKTRA_CHUNK_DEDUP_ENABLED` | bool | `true` | Deduplicate overlapping adjacent chunks from the same document | +| `VEKTRA_PARENT_EXPANSION_ENABLED` | bool | `false` | Replace retrieved child chunks with their parent chunk text before prompt construction (advanced pipeline only). Requires documents ingested with `VEKTRA_CHUNKING_STRATEGY=dual`. | | `VEKTRA_RESPONSE_TOKEN_RESERVE` | int | `2048` | Tokens reserved for LLM response generation | | `VEKTRA_CONTEXT_CHUNK_RATIO` | float | `0.6` | Fraction of context window allocated to retrieved chunks (0.0-1.0) | | `VEKTRA_PROMPT_TEMPLATES_DIR` | str | - | Directory for custom Jinja2 prompt templates (`system.j2`, `context.j2`, `conversation.j2`). Uses built-in defaults if unset. | diff --git a/vektra-core/src/vektra_core/advanced_pipeline.py b/vektra-core/src/vektra_core/advanced_pipeline.py index 68cee4aa..1a8eea16 100644 --- a/vektra-core/src/vektra_core/advanced_pipeline.py +++ b/vektra-core/src/vektra_core/advanced_pipeline.py @@ -8,6 +8,8 @@ 4. rerank - cross-encoder reranking (skip if not configured) 5. retrieval_filter - score threshold + overlap dedup (ARCH-056) 6. post_retrieval - SafeguardHook.post_retrieval() (DEBT-003) + 6.5 parent_expansion - replace child text with parent chunk text (FEAT-017, + skip unless VEKTRA_PARENT_EXPANSION_ENABLED) 7. build_prompt - token budget + Jinja2 rendering (ARCH-055) 8. llm_call - LLM with graceful degradation (ARCH-043) 9. pre_response - SafeguardHook.pre_response() (ARCH-049) @@ -16,6 +18,7 @@ from __future__ import annotations import asyncio +import dataclasses import hashlib import time from collections.abc import AsyncGenerator, AsyncIterator @@ -99,6 +102,7 @@ def __init__( self._rewrite_enabled = pipeline_config.rewrite.enabled self._eval_mode = pipeline_config.eval_mode self._debug_log_queries = pipeline_config.debug_log_queries + self._parent_expansion = pipeline_config.parent_expansion_enabled # -- Helpers (delegating to shared module-level functions) -- @@ -407,6 +411,20 @@ async def _run_pre_llm_steps( if not no_relevant_context and not filtered: no_relevant_context = True + # Step 6.5: Parent chunk expansion (FEAT-017) + if self._parent_expansion and filtered: + t0 = time.monotonic() + filtered, expansion_meta = await self._expand_parents( + query.namespace, filtered + ) + steps.append( + StepTrace( + name="parent_expansion", + duration_ms=_elapsed_ms(t0), + metadata=expansion_meta, + ) + ) + return ( steps, filtered, @@ -416,6 +434,59 @@ async def _run_pre_llm_steps( history, ) + async def _expand_parents( + self, + namespace: str, + results: list[SearchResult], + ) -> tuple[list[SearchResult], dict[str, object]]: + """Replace child chunk text with the parent chunk text (FEAT-017). + + Children of the same parent collapse into a single result (the + highest-scored one, since results arrive score-ordered): the parent + text already contains all its children. Runs before token budgeting + (ARCH-055) so the budget sees the expanded text. Results keep the + child's chunk_id and score for trace comparability. + """ + parent_ids: list[str] = [] + for r in results: + if r.parent_id and r.parent_id not in parent_ids: + parent_ids.append(r.parent_id) + if not parent_ids: + return results, {"children_expanded": 0, "parents_fetched": 0} + + try: + parents = await self._vector_store.retrieve( + namespace=namespace, chunk_ids=parent_ids + ) + except Exception as exc: + log.warning("parent_expansion_failed", error=str(exc)) + return results, {"skipped": True, "error": str(exc)} + + parent_text = {p.chunk_id: p.text_snippet for p in parents} + expanded: list[SearchResult] = [] + seen_parents: set[str] = set() + children_expanded = 0 + siblings_merged = 0 + for r in results: + if r.parent_id and r.parent_id in parent_text: + if r.parent_id in seen_parents: + siblings_merged += 1 + continue + seen_parents.add(r.parent_id) + children_expanded += 1 + expanded.append( + dataclasses.replace(r, text_snippet=parent_text[r.parent_id]) + ) + else: + expanded.append(r) + + return expanded, { + "children_expanded": children_expanded, + "siblings_merged": siblings_merged, + "parents_fetched": len(parents), + "after_expansion": len(expanded), + } + def _build_prompt( self, query: QueryRequest, diff --git a/vektra-index/src/vektra_index/adapters.py b/vektra-index/src/vektra_index/adapters.py index 4b4d184d..efb67280 100644 --- a/vektra-index/src/vektra_index/adapters.py +++ b/vektra-index/src/vektra_index/adapters.py @@ -120,6 +120,17 @@ async def search( raw_filters=raw_filters, ) + async def retrieve( + self, + namespace: str, + chunk_ids: list[str], + ) -> list[SearchResult]: + factory = self._get_session_factory() + pgvector = self._get_pgvector() + + async with factory() as session: + return await pgvector.retrieve(session, namespace, chunk_ids) + async def delete(self, namespace: str, ids: list[str]) -> int: """Delete all chunks for each document_id in ids. diff --git a/vektra-index/src/vektra_index/providers/pgvector.py b/vektra-index/src/vektra_index/providers/pgvector.py index 62f4cb03..7834fa83 100644 --- a/vektra-index/src/vektra_index/providers/pgvector.py +++ b/vektra-index/src/vektra_index/providers/pgvector.py @@ -73,7 +73,18 @@ async def store( inserted_ids: list[str] = [] for position, chunk in enumerate(chunks): - chunk_id = uuid4() + # Honor caller-provided deterministic ids (uuid5 from ingest); + # parent linkage relies on them (FEAT-017). + try: + chunk_id = UUID(chunk.chunk_id) if chunk.chunk_id else uuid4() + except ValueError: + chunk_id = uuid4() + parent_uuid: UUID | None = None + if chunk.parent_id: + try: + parent_uuid = UUID(chunk.parent_id) + except ValueError: + parent_uuid = None sparse_data = None if chunk.sparse is not None: sparse_data = { @@ -90,6 +101,7 @@ async def store( chunk_metadata=chunk.metadata, position=chunk.metadata.get("position", position), index_version=self._active_index_version, + parent_id=parent_uuid, ) session.add(orm_obj) inserted_ids.append(str(chunk_id)) @@ -151,6 +163,7 @@ async def _search_dense( DocumentChunkOrm.document_id, DocumentChunkOrm.content, DocumentChunkOrm.chunk_metadata, + DocumentChunkOrm.parent_id, score_expr, ) .where( @@ -161,6 +174,7 @@ async def _search_dense( .limit(top_k) ) + stmt = self._exclude_parent_chunks(stmt) stmt = self._apply_filters(stmt, filters) stmt = self._join_source_documents(stmt) @@ -228,6 +242,7 @@ async def _search_sparse( DocumentChunkOrm.document_id, DocumentChunkOrm.content, DocumentChunkOrm.chunk_metadata, + DocumentChunkOrm.parent_id, score_col, ) .where( @@ -239,6 +254,7 @@ async def _search_sparse( .limit(top_k) ) + stmt = self._exclude_parent_chunks(stmt) stmt = self._apply_filters(stmt, filters) stmt = self._join_source_documents(stmt) @@ -314,10 +330,27 @@ async def _search_hybrid( document_id=r.document_id, document_version=r.document_version, metadata=r.metadata, + parent_id=r.parent_id, ) for rrf_score, r in rrf_scored[:top_k] ] + @staticmethod + def _exclude_parent_chunks(stmt: Any) -> Any: + """Exclude parent-level chunks from search results (FEAT-017). + + Parent chunks are context material fetched by id during expansion, + not retrieval targets. Chunks without a chunk_level key (fixed + chunking) are unaffected: NULL IS DISTINCT FROM 'parent'. + """ + from vektra_index.models import DocumentChunkOrm + + return stmt.where( + DocumentChunkOrm.chunk_metadata["chunk_level"] + .as_string() + .is_distinct_from("parent") + ) + @staticmethod def _apply_filters(stmt: Any, filters: SearchFilters | None) -> Any: """Apply JSONB metadata filters in the same SQL query (REQ-063).""" @@ -361,10 +394,48 @@ def _rows_to_results(rows: Sequence[Any]) -> list[SearchResult]: document_id=row.document_id, document_version=row.document_version, metadata=row.chunk_metadata or {}, + parent_id=str(row.parent_id) if row.parent_id else None, ) for row in rows ] + async def retrieve( + self, + session: AsyncSession, + namespace: str, + chunk_ids: list[str], + ) -> list[SearchResult]: + """Fetch chunks by id (no vector search). Used for parent chunk + expansion (FEAT-017); score is 0.0 by convention. + """ + from vektra_index.models import DocumentChunkOrm + + valid_ids: list[UUID] = [] + for cid in chunk_ids: + try: + valid_ids.append(UUID(cid)) + except ValueError: + logger.warning("retrieve_invalid_chunk_id: %s", cid) + if not valid_ids: + return [] + + stmt = select( + DocumentChunkOrm.id, + DocumentChunkOrm.document_id, + DocumentChunkOrm.content, + DocumentChunkOrm.chunk_metadata, + DocumentChunkOrm.parent_id, + literal_column("0.0").label("score"), + ).where( + DocumentChunkOrm.id.in_(valid_ids), + DocumentChunkOrm.namespace_id == namespace, + DocumentChunkOrm.index_version == self._active_index_version, + ) + stmt = self._join_source_documents(stmt) + + result = await session.execute(stmt) + return self._rows_to_results(result.all()) + async def delete( self, session: AsyncSession, diff --git a/vektra-index/src/vektra_index/providers/qdrant.py b/vektra-index/src/vektra_index/providers/qdrant.py index bb27d8ae..9904dbef 100644 --- a/vektra-index/src/vektra_index/providers/qdrant.py +++ b/vektra-index/src/vektra_index/providers/qdrant.py @@ -184,6 +184,7 @@ async def store( "text": chunk.text, "metadata": chunk.metadata, "document_id": chunk.metadata.get("document_id", ""), + "parent_id": chunk.parent_id, }, ) ) @@ -333,6 +334,30 @@ async def _search_hybrid( ) return self._points_to_results(results.points) + async def retrieve( + self, + namespace: str, + chunk_ids: list[str], + ) -> list[SearchResult]: + """Fetch points by id (no vector search). Used for parent chunk + expansion (FEAT-017); score is 0.0 by convention. + + Points whose payload namespace does not match are dropped (namespace + isolation): Qdrant retrieve() takes no filter. + """ + if not chunk_ids: + return [] + + records = await self._client.retrieve( + collection_name=self._collection_name, + ids=chunk_ids, + with_payload=True, + ) + matching = [ + r for r in records if (r.payload or {}).get("namespace_id") == namespace + ] + return self._points_to_results(matching) + async def delete(self, namespace: str, ids: list[str]) -> int: """Delete points by document_id payload filter. @@ -428,11 +453,24 @@ def _build_filter( ) ) - return models.Filter(must=must_conditions) + # Parent chunks are context material, not retrieval targets (FEAT-017) + return models.Filter( + must=must_conditions, + must_not=[ + models.FieldCondition( + key="metadata.chunk_level", + match=models.MatchValue(value="parent"), + ) + ], + ) @staticmethod def _points_to_results(points: list[Any]) -> list[SearchResult]: - """Convert Qdrant ScoredPoint list to SearchResult list.""" + """Convert Qdrant ScoredPoint/Record list to SearchResult list. + + Record objects (from retrieve()) have no score attribute: score + defaults to 0.0. + """ results: list[SearchResult] = [] for point in points: payload = point.payload or {} @@ -442,16 +480,18 @@ def _points_to_results(points: list[Any]) -> list[SearchResult]: except (ValueError, AttributeError): doc_id = UUID(int=0) + score = getattr(point, "score", None) results.append( SearchResult( chunk_id=str(point.id), - score=float(point.score) if point.score is not None else 0.0, + score=float(score) if score is not None else 0.0, text_snippet=payload.get("text", ""), document_id=doc_id, document_version=payload.get("metadata", {}).get( "document_version", 1 ), metadata=payload.get("metadata", {}), + parent_id=payload.get("parent_id"), ) ) return results diff --git a/vektra-ingest/src/vektra_ingest/chunking.py b/vektra-ingest/src/vektra_ingest/chunking.py index 194c67b2..2d45cd0c 100644 --- a/vektra-ingest/src/vektra_ingest/chunking.py +++ b/vektra-ingest/src/vektra_ingest/chunking.py @@ -265,6 +265,9 @@ async def _chunk_impl( parent_start = parent_end continue + # The parent carries its own hierarchy id so the ingest + # pipeline can remap these chunker-local ids to stored + # chunk ids (FEAT-017). yield DocumentChunk( text=parent_text, element_type=ElementType.TEXT, @@ -274,7 +277,7 @@ async def _chunk_impl( "token_count": len(parent_tokens), "chunk_level": "parent", }, - parent_id=None, + parent_id=parent_id, ) chunk_index += 1 diff --git a/vektra-ingest/src/vektra_ingest/pipeline.py b/vektra-ingest/src/vektra_ingest/pipeline.py index 4a3663ee..4c8be799 100644 --- a/vektra-ingest/src/vektra_ingest/pipeline.py +++ b/vektra-ingest/src/vektra_ingest/pipeline.py @@ -395,6 +395,15 @@ async def run_ingest( ), ) + # Map chunker-local hierarchy ids to stored chunk ids (FEAT-017). + # Parent chunks carry their own transient id in chunk.parent_id; + # children reference it. Stored ids are deterministic: uuid5(doc_id, position). + parent_stored_ids = { + chunk.parent_id: str(uuid5(doc_id, str(i))) + for i, chunk in enumerate(all_chunks) + if chunk.parent_id and chunk.metadata.get("chunk_level") == "parent" + } + # Build ChunkEmbedding objects with document_id in metadata _extra = extra_metadata or {} chunk_embeddings = [ @@ -411,6 +420,11 @@ async def run_ingest( "element_type": chunk.element_type.value, "position": i, }, + parent_id=( + parent_stored_ids.get(chunk.parent_id) + if chunk.parent_id and chunk.metadata.get("chunk_level") == "child" + else None + ), ) for i, (chunk, embedding, sparse) in enumerate( zip(all_chunks, embeddings, sparse_vectors) diff --git a/vektra-ingest/tests/test_dual_chunking.py b/vektra-ingest/tests/test_dual_chunking.py index 7222dc92..d92d343a 100644 --- a/vektra-ingest/tests/test_dual_chunking.py +++ b/vektra-ingest/tests/test_dual_chunking.py @@ -74,20 +74,17 @@ async def test_text_elements_split_with_overlap(): chunks = await _collect(chunker, elements) # Should have at least one parent and at least one child - parents = [ - c for c in chunks if c.parent_id is None and c.element_type == ElementType.TEXT - ] - children = [c for c in chunks if c.parent_id is not None] + parents = [c for c in chunks if c.metadata.get("chunk_level") == "parent"] + children = [c for c in chunks if c.metadata.get("chunk_level") == "child"] assert len(parents) >= 1 assert len(children) >= 1 + # Parents carry their own hierarchy id; children reference it (FEAT-017) + parent_ids = {p.parent_id for p in parents} + assert None not in parent_ids for child in children: - assert child.parent_id is not None - assert child.metadata.get("chunk_level") == "child" - - for parent in parents: - assert parent.metadata.get("chunk_level") == "parent" + assert child.parent_id in parent_ids @pytest.mark.asyncio diff --git a/vektra-shared/src/vektra_shared/config.py b/vektra-shared/src/vektra_shared/config.py index 5b602fae..987b5903 100644 --- a/vektra-shared/src/vektra_shared/config.py +++ b/vektra-shared/src/vektra_shared/config.py @@ -197,6 +197,11 @@ class QueryPipelineConfig(BaseSettings): alias="VEKTRA_CHUNK_DEDUP_ENABLED", description="Enable overlap deduplication for adjacent chunks from the same document.", ) + parent_expansion_enabled: bool = Field( + False, + alias="VEKTRA_PARENT_EXPANSION_ENABLED", + description="Replace retrieved child chunks with their parent chunk text before prompt construction (FEAT-017). Requires dual chunking at ingest time.", + ) response_token_reserve: int = Field( 2048, ge=1, @@ -493,6 +498,9 @@ class VektraSettings(BaseSettings): query_pipeline: str = Field("advanced", alias="VEKTRA_QUERY_PIPELINE") min_relevance_score: float = Field(0.15, alias="VEKTRA_MIN_RELEVANCE_SCORE") chunk_dedup_enabled: bool = Field(True, alias="VEKTRA_CHUNK_DEDUP_ENABLED") + parent_expansion_enabled: bool = Field( + False, alias="VEKTRA_PARENT_EXPANSION_ENABLED" + ) response_token_reserve: int = Field(2048, alias="VEKTRA_RESPONSE_TOKEN_RESERVE") context_chunk_ratio: float = Field(0.6, alias="VEKTRA_CONTEXT_CHUNK_RATIO") prompt_templates_dir: str | None = Field(None, alias="VEKTRA_PROMPT_TEMPLATES_DIR") diff --git a/vektra-shared/src/vektra_shared/protocols.py b/vektra-shared/src/vektra_shared/protocols.py index 7c633f11..1415d9bc 100644 --- a/vektra-shared/src/vektra_shared/protocols.py +++ b/vektra-shared/src/vektra_shared/protocols.py @@ -117,6 +117,12 @@ async def search( raw_filters: dict[str, Any] | None = None, ) -> list[SearchResult]: ... + async def retrieve( + self, + namespace: str, + chunk_ids: list[str], + ) -> list[SearchResult]: ... + async def delete(self, namespace: str, ids: list[str]) -> int: ... async def health_check(self) -> HealthStatus: ... diff --git a/vektra-shared/src/vektra_shared/types.py b/vektra-shared/src/vektra_shared/types.py index d1d90342..38729007 100644 --- a/vektra-shared/src/vektra_shared/types.py +++ b/vektra-shared/src/vektra_shared/types.py @@ -121,6 +121,7 @@ class ChunkEmbedding: dense: list[float] sparse: SparseVector | None = None # Phase 2: hybrid search metadata: dict[str, Any] = field(default_factory=dict) + parent_id: str | None = None # stored chunk_id of the parent chunk (FEAT-017) # --------------------------------------------------------------------------- @@ -157,6 +158,7 @@ class SearchResult: document_version: int = 1 # from SourceDocument.version (REQ-056) metadata: dict[str, Any] = field(default_factory=dict) original_score: float | None = None # pre-reranker score (BUG-015) + parent_id: str | None = None # stored chunk_id of the parent chunk (FEAT-017) @dataclass From 48c0594663980f82f7c495f39a2891d340c24c99 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 21:21:23 +0000 Subject: [PATCH 3/5] test(rag): cover parent linkage, search exclusion, and expansion (FEAT-017) - ingest: dual ingest maps children to the parent's deterministic stored id (uuid5(doc_id, position)); parents and tables carry no parent_id - qdrant: parent_id in payload, must_not filter on chunk_level=parent, retrieve() namespace isolation and Record (no score) handling - pgvector: caller-provided uuid ids honored, parent_id column filled, dense search excludes parents, retrieve() maps rows and skips bad ids - advanced pipeline: expansion replaces child text with parent text, merges siblings, degrades gracefully on retrieve failure, keeps children when the parent is missing, and feeds the token budget with the expanded text; default-off leaves the pipeline untouched Co-Authored-By: Claude Fable 5 --- vektra-core/tests/test_advanced_pipeline.py | 189 ++++++++++++++++++++ vektra-index/tests/test_pgvector_unit.py | 141 +++++++++++++++ vektra-index/tests/test_qdrant_provider.py | 126 +++++++++++++ vektra-ingest/tests/test_pipeline.py | 121 +++++++++++++ 4 files changed, 577 insertions(+) diff --git a/vektra-core/tests/test_advanced_pipeline.py b/vektra-core/tests/test_advanced_pipeline.py index bdd1c488..2956340c 100644 --- a/vektra-core/tests/test_advanced_pipeline.py +++ b/vektra-core/tests/test_advanced_pipeline.py @@ -770,3 +770,192 @@ async def test_eval_mode_off_excludes_prompt_messages(): build_step = next(s for s in trace.steps if s.name == "build_prompt") assert "messages" not in build_step.metadata + + +# --------------------------------------------------------------------------- +# Parent chunk expansion (FEAT-017) +# --------------------------------------------------------------------------- + + +def _make_child_result( + score: float, text: str, parent_id: str, doc_id: UUID | None = None +) -> SearchResult: + return SearchResult( + chunk_id=str(uuid4()), + score=score, + text_snippet=text, + document_id=doc_id or uuid4(), + document_version=1, + parent_id=parent_id, + ) + + +def _make_parent_chunk(chunk_id: str, text: str, doc_id: UUID) -> SearchResult: + return SearchResult( + chunk_id=chunk_id, + score=0.0, + text_snippet=text, + document_id=doc_id, + document_version=1, + ) + + +async def test_parent_expansion_replaces_child_text(): + """Expanded children carry the parent text into prompt and sources.""" + doc_id = uuid4() + parent_text = "PARENT SECTION: full surrounding context for the child." + child = _make_child_result(0.9, "tiny child snippet", "parent-1", doc_id) + orphan = _make_search_result(0.8, "standalone chunk without parent") + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[child, orphan]) + vector_store.retrieve = AsyncMock( + return_value=[_make_parent_chunk("parent-1", parent_text, doc_id)] + ) + + pipeline = _make_pipeline( + vector_store=vector_store, + pipeline_config=_make_pipeline_config(VEKTRA_PARENT_EXPANSION_ENABLED=True), + ) + response, trace = await pipeline.execute( + QueryRequest(question="test", namespace="default") + ) + + vector_store.retrieve.assert_awaited_once_with( + namespace="default", chunk_ids=["parent-1"] + ) + # Expanded result keeps the child's chunk_id and score + by_id = {s.chunk_id: s for s in response.sources} + assert by_id[child.chunk_id].snippet == parent_text + assert by_id[child.chunk_id].score == child.score + assert by_id[orphan.chunk_id].snippet == orphan.text_snippet + + step = next(s for s in trace.steps if s.name == "parent_expansion") + assert step.metadata["children_expanded"] == 1 + assert step.metadata["siblings_merged"] == 0 + assert step.metadata["parents_fetched"] == 1 + + +async def test_parent_expansion_merges_siblings_of_same_parent(): + """Children of the same parent collapse into the highest-scored one.""" + doc_id = uuid4() + parent_text = "PARENT: contains both sibling fragments." + sibling_hi = _make_child_result(0.9, "fragment one", "parent-1", doc_id) + sibling_lo = _make_child_result(0.7, "fragment two", "parent-1", doc_id) + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[sibling_hi, sibling_lo]) + vector_store.retrieve = AsyncMock( + return_value=[_make_parent_chunk("parent-1", parent_text, doc_id)] + ) + + pipeline = _make_pipeline( + vector_store=vector_store, + pipeline_config=_make_pipeline_config(VEKTRA_PARENT_EXPANSION_ENABLED=True), + ) + response, trace = await pipeline.execute(QueryRequest(question="test")) + + assert len(response.sources) == 1 + assert response.sources[0].chunk_id == sibling_hi.chunk_id + assert response.sources[0].snippet == parent_text + + step = next(s for s in trace.steps if s.name == "parent_expansion") + assert step.metadata["children_expanded"] == 1 + assert step.metadata["siblings_merged"] == 1 + + +async def test_parent_expansion_disabled_by_default(): + """Without the flag no retrieve call is made and no step is traced.""" + child = _make_child_result(0.9, "child snippet", "parent-1") + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[child]) + + pipeline = _make_pipeline(vector_store=vector_store) + response, trace = await pipeline.execute(QueryRequest(question="test")) + + vector_store.retrieve.assert_not_awaited() + assert all(s.name != "parent_expansion" for s in trace.steps) + assert response.sources[0].snippet == "child snippet" + + +async def test_parent_expansion_retrieve_failure_keeps_children(): + """A vector store failure during expansion degrades gracefully.""" + child = _make_child_result(0.9, "child snippet", "parent-1") + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[child]) + vector_store.retrieve = AsyncMock(side_effect=RuntimeError("qdrant down")) + + pipeline = _make_pipeline( + vector_store=vector_store, + pipeline_config=_make_pipeline_config(VEKTRA_PARENT_EXPANSION_ENABLED=True), + ) + response, trace = await pipeline.execute(QueryRequest(question="test")) + + assert response.answer is not None + assert response.sources[0].snippet == "child snippet" + step = next(s for s in trace.steps if s.name == "parent_expansion") + assert step.metadata.get("skipped") is True + + +async def test_parent_expansion_missing_parent_keeps_child(): + """Children whose parent is not returned by retrieve() stay unexpanded.""" + child = _make_child_result(0.9, "child snippet", "parent-gone") + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[child]) + vector_store.retrieve = AsyncMock(return_value=[]) + + pipeline = _make_pipeline( + vector_store=vector_store, + pipeline_config=_make_pipeline_config(VEKTRA_PARENT_EXPANSION_ENABLED=True), + ) + response, trace = await pipeline.execute(QueryRequest(question="test")) + + assert response.sources[0].snippet == "child snippet" + step = next(s for s in trace.steps if s.name == "parent_expansion") + assert step.metadata["children_expanded"] == 0 + assert step.metadata["parents_fetched"] == 0 + + +async def test_parent_expansion_feeds_token_budget_with_parent_text(): + """Token budgeting (ARCH-055) operates on the expanded parent text.""" + doc_id = uuid4() + parent_text = "PARENT TEXT " * 50 + child = _make_child_result(0.9, "tiny", "parent-1", doc_id) + + vector_store = AsyncMock() + vector_store.search = AsyncMock(return_value=[child]) + vector_store.retrieve = AsyncMock( + return_value=[_make_parent_chunk("parent-1", parent_text, doc_id)] + ) + + llm = MagicMock() + llm.complete = AsyncMock( + return_value=CompletionResponse( + content="The answer.", + model="ollama/llama3", + prompt_tokens=10, + completion_tokens=20, + total_tokens=30, + ) + ) + token_counts: list[str] = [] + + def _count(text: str, model: str | None = None) -> int: + token_counts.append(text) + return len(text.split()) + + llm.count_tokens = MagicMock(side_effect=_count) + + pipeline = _make_pipeline( + vector_store=vector_store, + llm=llm, + pipeline_config=_make_pipeline_config(VEKTRA_PARENT_EXPANSION_ENABLED=True), + ) + await pipeline.execute(QueryRequest(question="test")) + + # The budget counted the parent text, not the child snippet + assert any(parent_text == t for t in token_counts) + assert all(t != "tiny" for t in token_counts) diff --git a/vektra-index/tests/test_pgvector_unit.py b/vektra-index/tests/test_pgvector_unit.py index 19233c1d..e8e8e06f 100644 --- a/vektra-index/tests/test_pgvector_unit.py +++ b/vektra-index/tests/test_pgvector_unit.py @@ -163,3 +163,144 @@ async def test_custom_index_version_used(self): provider = PgvectorProvider(active_index_version=2) assert provider._active_index_version == 2 + + +class TestPgvectorParentChunks: + """Deterministic ids, parent linkage, search exclusion, retrieve (FEAT-017).""" + + def _make_session(self): + session = AsyncMock() + session.flush = AsyncMock() + session.execute = AsyncMock() + session.add = MagicMock() + return session + + @pytest.mark.asyncio + async def test_store_honors_deterministic_ids_and_parent_linkage(self): + """store() keeps caller-provided uuid ids and fills the parent_id column.""" + from vektra_index.providers.pgvector import PgvectorProvider + + session = self._make_session() + provider = PgvectorProvider() + + parent_uuid = uuid4() + child_uuid = uuid4() + chunks = [ + ChunkEmbedding( + chunk_id=str(parent_uuid), + text="parent", + dense=[0.1] * 384, + metadata={"chunk_level": "parent"}, + ), + ChunkEmbedding( + chunk_id=str(child_uuid), + text="child", + dense=[0.1] * 384, + metadata={"chunk_level": "child"}, + parent_id=str(parent_uuid), + ), + ] + + with patch("vektra_index.models.DocumentChunkOrm") as MockOrm: + MockOrm.return_value = MagicMock() + result = await provider.store(session, "default", uuid4(), chunks) + + assert result == [str(parent_uuid), str(child_uuid)] + orm_kwargs = [c.kwargs for c in MockOrm.call_args_list] + assert orm_kwargs[0]["id"] == parent_uuid + assert orm_kwargs[0]["parent_id"] is None + assert orm_kwargs[1]["id"] == child_uuid + assert orm_kwargs[1]["parent_id"] == parent_uuid + + @pytest.mark.asyncio + async def test_store_falls_back_to_random_id_on_non_uuid(self): + """Non-UUID caller ids (external store-chunks callers) get random uuids.""" + from vektra_index.providers.pgvector import PgvectorProvider + + session = self._make_session() + provider = PgvectorProvider() + + chunks = [ + ChunkEmbedding( + chunk_id="not-a-uuid", + text="x", + dense=[0.1] * 384, + metadata={}, + parent_id="also-not-a-uuid", + ), + ] + + with patch("vektra_index.models.DocumentChunkOrm") as MockOrm: + MockOrm.return_value = MagicMock() + result = await provider.store(session, "default", uuid4(), chunks) + + UUID(result[0]) # random but valid + assert MockOrm.call_args.kwargs["parent_id"] is None + + @pytest.mark.asyncio + async def test_dense_search_excludes_parent_chunks(self): + """The dense search SQL filters out chunk_level=parent rows.""" + from sqlalchemy.dialects import postgresql + + from vektra_index.providers.pgvector import PgvectorProvider + from vektra_shared.types import QueryEmbedding + + session = self._make_session() + empty_result = MagicMock() + empty_result.all.return_value = [] + session.execute = AsyncMock(return_value=empty_result) + + provider = PgvectorProvider() + await provider.search( + session, "default", QueryEmbedding(dense=[0.1] * 384), top_k=5 + ) + + stmt = session.execute.call_args.args[0] + compiled = stmt.compile(dialect=postgresql.dialect()) + assert "IS DISTINCT FROM" in str(compiled) + # The JSONB accessor key and the excluded value are bind parameters + assert "chunk_level" in compiled.params.values() + assert "parent" in compiled.params.values() + + @pytest.mark.asyncio + async def test_retrieve_maps_rows_and_skips_invalid_ids(self): + from vektra_index.providers.pgvector import PgvectorProvider + + session = self._make_session() + parent_uuid = uuid4() + doc_id = uuid4() + row = FakeRow( + id=parent_uuid, + document_id=doc_id, + content="parent text", + chunk_metadata={"chunk_level": "parent"}, + parent_id=None, + score=0.0, + document_version=1, + ) + rows_result = MagicMock() + rows_result.all.return_value = [row] + session.execute = AsyncMock(return_value=rows_result) + + provider = PgvectorProvider() + results = await provider.retrieve( + session, "default", [str(parent_uuid), "not-a-uuid"] + ) + + assert len(results) == 1 + assert results[0].chunk_id == str(parent_uuid) + assert results[0].score == 0.0 + assert results[0].text_snippet == "parent text" + assert results[0].parent_id is None + + @pytest.mark.asyncio + async def test_retrieve_all_invalid_ids_returns_empty(self): + from vektra_index.providers.pgvector import PgvectorProvider + + session = self._make_session() + provider = PgvectorProvider() + + results = await provider.retrieve(session, "default", ["nope", "still-nope"]) + + assert results == [] + session.execute.assert_not_called() diff --git a/vektra-index/tests/test_qdrant_provider.py b/vektra-index/tests/test_qdrant_provider.py index 686f6ffd..0b5ffe16 100644 --- a/vektra-index/tests/test_qdrant_provider.py +++ b/vektra-index/tests/test_qdrant_provider.py @@ -244,3 +244,129 @@ async def test_unhealthy(self): status = await provider.health_check() assert status.status == "unhealthy" assert "connection refused" in status.message + + +class TestQdrantParentChunks: + """Parent chunk linkage, search exclusion, and retrieve-by-id (FEAT-017).""" + + @pytest.mark.asyncio + async def test_store_includes_parent_id_in_payload(self): + provider, _client = _make_provider() + doc_id = str(uuid4()) + + _mock_qdrant_models.PointStruct.reset_mock() + chunks = [ + ChunkEmbedding( + chunk_id="child-1", + text="child text", + dense=[0.1] * 384, + metadata={"document_id": doc_id, "chunk_level": "child"}, + parent_id="parent-1", + ), + ChunkEmbedding( + chunk_id="parent-1", + text="parent text", + dense=[0.1] * 384, + metadata={"document_id": doc_id, "chunk_level": "parent"}, + ), + ] + + await provider.store("default", chunks) + + payloads = [ + c.kwargs["payload"] for c in _mock_qdrant_models.PointStruct.call_args_list + ] + assert payloads[0]["parent_id"] == "parent-1" + assert payloads[1]["parent_id"] is None + + @pytest.mark.asyncio + async def test_build_filter_excludes_parent_chunks(self): + provider, _client = _make_provider() + + _mock_qdrant_models.Filter.reset_mock() + _mock_qdrant_models.FieldCondition.reset_mock() + provider._build_filter("default") + + filter_kwargs = _mock_qdrant_models.Filter.call_args.kwargs + assert "must_not" in filter_kwargs + assert len(filter_kwargs["must_not"]) == 1 + condition_keys = [ + c.kwargs.get("key") + for c in _mock_qdrant_models.FieldCondition.call_args_list + ] + assert "metadata.chunk_level" in condition_keys + + @pytest.mark.asyncio + async def test_search_results_carry_parent_id(self): + provider, client = _make_provider() + doc_id = str(uuid4()) + + query_result = MagicMock() + query_result.points = [ + _make_scored_point( + "child-1", + 0.9, + { + "text": "child text", + "document_id": doc_id, + "metadata": {"chunk_level": "child"}, + "namespace_id": "default", + "parent_id": "parent-1", + }, + ), + ] + client.query_points = AsyncMock(return_value=query_result) + + results = await provider.search( + "default", QueryEmbedding(dense=[0.1] * 384), 5, SearchMode.DENSE + ) + + assert results[0].parent_id == "parent-1" + + @pytest.mark.asyncio + async def test_retrieve_filters_by_namespace(self): + from types import SimpleNamespace + + provider, client = _make_provider() + doc_id = str(uuid4()) + + # Qdrant retrieve() returns Record objects with no score attribute + records = [ + SimpleNamespace( + id="parent-1", + payload={ + "text": "parent text", + "document_id": doc_id, + "metadata": {"chunk_level": "parent"}, + "namespace_id": "default", + "parent_id": None, + }, + ), + SimpleNamespace( + id="alien-1", + payload={ + "text": "other tenant", + "document_id": doc_id, + "metadata": {}, + "namespace_id": "other", + "parent_id": None, + }, + ), + ] + client.retrieve = AsyncMock(return_value=records) + + results = await provider.retrieve("default", ["parent-1", "alien-1"]) + + client.retrieve.assert_awaited_once() + assert [r.chunk_id for r in results] == ["parent-1"] + assert results[0].score == 0.0 + assert results[0].text_snippet == "parent text" + + @pytest.mark.asyncio + async def test_retrieve_empty_ids_short_circuits(self): + provider, client = _make_provider() + + results = await provider.retrieve("default", []) + + assert results == [] + client.retrieve.assert_not_called() diff --git a/vektra-ingest/tests/test_pipeline.py b/vektra-ingest/tests/test_pipeline.py index 76236ac7..2968be15 100644 --- a/vektra-ingest/tests/test_pipeline.py +++ b/vektra-ingest/tests/test_pipeline.py @@ -711,3 +711,124 @@ async def _gen(): session=session, registry=registry, ) + + +# --------------------------------------------------------------------------- +# Parent chunk linkage at store time (FEAT-017) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_dual_ingest_links_children_to_stored_parent_ids(monkeypatch): + """Children reference the parent's deterministic stored id, not the + chunker-local transient id; parents and tables carry no parent_id.""" + from uuid import uuid5 + + from vektra_ingest.pipeline import run_ingest + + monkeypatch.setenv("VEKTRA_CHUNKING_STRATEGY", "dual") + monkeypatch.setenv("VEKTRA_PARENT_CHILD_LEVELS", "1") + + doc_id = uuid4() + + session = AsyncMock() + + async def _execute(stmt, *args, **kwargs): + mock_result = MagicMock() + mock_result.scalar_one_or_none.return_value = None + return mock_result + + added_docs: list = [] + + def _session_add(obj): + added_docs.append(obj) + + async def _flush_side_effect(): + for obj in added_docs: + obj.id = doc_id + + session.execute = _execute + session.add = _session_add + session.flush = AsyncMock(side_effect=_flush_side_effect) + session.commit = AsyncMock() + + registry, mock_embedding, mock_vs = _make_registry() + mock_embedding.embed_documents = AsyncMock(return_value=[[0.1] * 384] * 4) + mock_vs.store = AsyncMock(return_value=["i0", "i1", "i2", "i3"]) + + async def _fake_extract(req): + async def _gen(): + yield DocumentChunk(text="whatever", element_type=ElementType.TEXT) + + return _gen() + + # Controlled dual output: parent + two children + standalone table + async def _fake_chunk(elements): + async def _gen(): + yield DocumentChunk( + text="parent text", + element_type=ElementType.TEXT, + metadata={"chunk_level": "parent"}, + parent_id="transient-1", + ) + yield DocumentChunk( + text="child a", + element_type=ElementType.TEXT, + metadata={"chunk_level": "child"}, + parent_id="transient-1", + ) + yield DocumentChunk( + text="child b", + element_type=ElementType.TEXT, + metadata={"chunk_level": "child"}, + parent_id="transient-1", + ) + yield DocumentChunk( + text="standalone
", + element_type=ElementType.TABLE, + metadata={}, + parent_id=None, + ) + + return _gen() + + mock_extractor = MagicMock() + mock_extractor.extract = _fake_extract + mock_extractor.supported_types.return_value = {"application/pdf"} + + with patch( + "vektra_ingest.pipeline.detect_content_type", return_value="application/pdf" + ): + with patch( + "vektra_ingest.pipeline._get_extractor", return_value=mock_extractor + ): + with patch( + "vektra_ingest.pipeline.DualStrategyChunking" + ) as mock_chunker_class: + mock_chunker = MagicMock() + mock_chunker.chunk = _fake_chunk + mock_chunker_class.return_value = mock_chunker + + result = await run_ingest( + file_content=b"%PDF-1.4 fake pdf", + filename="dual.pdf", + namespace="default", + session=session, + registry=registry, + ) + + assert result.status == "new" + stored_chunks = mock_vs.store.call_args.args[1] + assert len(stored_chunks) == 4 + + parent, child_a, child_b, table = stored_chunks + expected_parent_stored_id = str(uuid5(doc_id, "0")) + + assert parent.chunk_id == expected_parent_stored_id + assert parent.parent_id is None + assert child_a.parent_id == expected_parent_stored_id + assert child_b.parent_id == expected_parent_stored_id + assert table.parent_id is None + # Deterministic ids for every chunk: uuid5(doc_id, position) + for i, chunk in enumerate(stored_chunks): + assert chunk.chunk_id == str(uuid5(doc_id, str(i))) From ddbddcb5ca6ca069674014e0e237e6bd4163176c Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 21:48:09 +0000 Subject: [PATCH 4/5] docs(s2s): record FEAT-017 A/B measurement; file TECH-007; extend DEBT-025 fixture - plan: section 3 complete; Notes with measure A (dual, no expansion: retrieval -6.5pp hit from parent-boundary chunking, e2e unchanged) and measure B (expansion on: grounding-neutral, zero flips, sources 1.4->1.0 via sibling merge, expansion verified in traces) - backlog: FEAT-017 completed with premise corrections and honest numbers; TECH-007 filed with the multi-chunk collapse root cause (rerank scores for multi-part questions all below min_relevance_score 0.15; MC-01 max rerank 0.088 vs raw RRF 0.61 - the funnel wipes candidates before expansion can run) - changelog: FEAT-017 Added entry + Changed entries (parent exclusion in search, pgvector deterministic ids) - tests: DEBT-025 hermetic fixture replicated in vektra-core and vektra-ingest (ambient VEKTRA_CHUNKING_STRATEGY=dual from the dev .env broke 12 tests there; vektra-shared alone was covered) Co-Authored-By: Claude Fable 5 --- .s2s/BACKLOG.md | 35 ++++++++++--- .s2s/plans/20260712-sprint3-rag-quality.md | 57 +++++++++++++++++----- CHANGELOG.md | 11 ++++- vektra-core/tests/conftest.py | 26 ++++++++++ vektra-ingest/tests/conftest.py | 26 ++++++++++ 5 files changed, 135 insertions(+), 20 deletions(-) create mode 100644 vektra-core/tests/conftest.py create mode 100644 vektra-ingest/tests/conftest.py diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 1aab8686..d048fd96 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -283,19 +283,23 @@ The `title` field would contain `filename + page` (e.g., "Costituzione italiana. ### FEAT-017: Parent chunk expansion in query pipeline -**Status**: planned | **Priority**: medium | **Created**: 2026-03-23 +**Status**: completed (2026-07-12) | **Priority**: medium | **Created**: 2026-03-23 **Analysis**: `vektra-internal/stack/20260323-rag-prompt-chunk-confusion-analysis.md` **Context**: when a child chunk is retrieved via search, the pipeline should optionally expand it to the parent chunk for broader context. The infrastructure is already in place: `DualStrategyChunking` creates parent-child hierarchy (parent every 3000 tokens, children at 500 tokens with overlap), `DocumentChunkOrm` has `parent_id` column, and both are stored in the database. Missing: (1) filter parent chunks from default search results (search currently returns both), (2) parent expansion logic in AdvancedQueryPipeline when a child matches. +**Premise corrections found during implementation**: the hierarchy was NOT actually persisted anywhere (`run_ingest` dropped `chunk.parent_id`, `ChunkEmbedding` had no field, Qdrant payload had none, `DocumentChunkOrm.parent_id` was always NULL); parent size is `chunk_size*3` (1500 tokens with Combo D 500, not 3000); pgvector `store()` ignored caller chunk ids (generated uuid4), so deterministic ids had to be plumbed there too. + **Traceability**: ARCH-037 (ChunkingStrategy), ARCH-055 (token budget), core-pipeline-v2 **Acceptance criteria**: -- [ ] Search excludes parent chunks by default (WHERE parent_id IS NOT NULL for children only) -- [ ] AdvancedQueryPipeline fetches parent chunk when child matches and includes it in context -- [ ] Parent expansion is configurable (on/off, via env var) -- [ ] Token budget accounts for expanded parent chunk size -- [ ] Tested: truncated-context answers improve with parent expansion enabled +- [x] Search excludes parent chunks by default (`chunk_level=parent` filtered: Qdrant `must_not`, pgvector `IS DISTINCT FROM`) +- [x] AdvancedQueryPipeline fetches parent chunk when child matches and includes it in context (step 6.5, new `VectorStoreProvider.retrieve()`) +- [x] Parent expansion is configurable (`VEKTRA_PARENT_EXPANSION_ENABLED`, default off) +- [x] Token budget accounts for expanded parent chunk size (expansion runs before ARCH-055 allocation) +- [x] Tested: 17 unit tests (linkage, exclusion, expansion, budget); measured on `eval-full` — expansion is grounding-neutral there (35/55 with and without, zero per-question flips, avg sources 1.4 → 1.0 via sibling merge). The multi-chunk collapse it targeted turned out to be upstream: the rerank+threshold funnel wipes all candidates before expansion (TECH-007). The corpus also understates expansion benefit (short self-contained articles — TECH-005 collection 1 is the real test bench). + +**Resolution (2026-07-12, Sprint 3)**: shipped default-off on `feat/feat-017-parent-chunk-expansion`. Measured A/B on `eval-full` reingested dual (105 points = 22 parents + 83 children): dual chunking alone costs ~6.5pp retrieval hit vs fixed (children stop rolling overlap across 1500-token parent boundaries; IT-F-13, IT-R-02, EN-F-03 flip to miss). Full numbers in plan `20260712-sprint3-rag-quality` Notes. --- @@ -357,6 +361,25 @@ The `title` field would contain `filename + page` (e.g., "Costituzione italiana. --- +### TECH-007: Multi-part questions wiped by rerank+threshold funnel (multi-chunk collapse root cause) + +**Status**: planned | **Priority**: high | **Created**: 2026-07-12 +**Origin**: FEAT-017 measurement (plan `20260712-sprint3-rag-quality`) - expansion turned out to be downstream of the real failure. + +**Context**: on `eval-full`, 9/10 multi-chunk questions end with `retrieval_filter before=5 after=0` → `no_relevant_context` → refusal, despite 90% raw retrieval hit for the category. Cause: bge-reranker-v2-m3 scores each partial-answer chunk of a comparative/multi-part question low (each chunk answers only one part), and `VEKTRA_MIN_RELEVANCE_SCORE=0.15` — calibrated in the tuning sprint on single-fact questions (DEBT-010) — wipes the entire candidate set. Evidence (MC-01, eval mode traces): max reranker score 0.088 on a candidate whose raw RRF score was 0.61. Parent expansion (FEAT-017) never runs because zero results survive the filter. + +**Candidate directions** (evaluate, do not assume): (a) floor semantics - keep top-N post-rerank chunks regardless of threshold when the raw retrieval score was strong (e.g. min(top_k, after_rerank) >= 2); (b) per-category or per-score-source thresholds (reranker scores are not calibrated on the same scale as RRF); (c) query decomposition for multi-part questions (rewrite step already exists, ARCH-061); (d) rescore against the parent text instead of the child (combines with FEAT-017). + +**Traceability**: ARCH-056 (retrieval quality controls), ADR-0021, DEBT-010, FEAT-017, TECH-005 + +**Acceptance criteria**: +- [ ] Reproduce with the eval harness and document the score distributions per category +- [ ] Chosen mitigation implemented behind config, default preserving current single-fact behavior +- [ ] `eval-full` multi-chunk grounded moves from 0-1/10 without regressing factual (19/21) or adversarial refusals (no answered-without-context) +- [ ] Decision and numbers recorded in the sprint plan and vektra-internal + +--- + ### FEAT-024: Remote embedding and reranker providers (TEI) **Status**: planned | **Priority**: medium | **Created**: 2026-07-12 diff --git a/.s2s/plans/20260712-sprint3-rag-quality.md b/.s2s/plans/20260712-sprint3-rag-quality.md index d0ad9238..aeb0274f 100644 --- a/.s2s/plans/20260712-sprint3-rag-quality.md +++ b/.s2s/plans/20260712-sprint3-rag-quality.md @@ -116,7 +116,7 @@ the backlog assumptions: - [x] Autouse fixture in `vektra-shared/tests/conftest.py` scrubs `VEKTRA_*` + external keys - [x] `make test` green with populated `.env` (638 passed) + `make lint` green - [x] Backlog entry updated (completed + resolution), changelog entry -- [ ] PR created and merged +- [x] PR created and merged (#84; #85 BUG-021, #86 backlog, #87 BUG-022 merged the same day) ### 2. Baseline eval (no branch; results recorded, not committed as code) - [x] Eval corpus verified intact in namespace `default` (excerpt corpus, 12 chunks — no reingest needed) @@ -126,19 +126,23 @@ the backlog assumptions: - [x] Numbers logged in this plan (Notes) and in vektra-internal (`stack/20260712-sprint3-baseline-eval.md`) ### 3. FEAT-017 — parent chunk expansion (branch `feat/feat-017-parent-chunk-expansion`) -- [ ] Propagate `parent_id` into `ChunkEmbedding` → Qdrant payload + pgvector column; - keep deterministic child/parent ids at store time -- [ ] Search excludes `chunk_level=parent` by default (both providers) -- [ ] `VEKTRA_PARENT_EXPANSION_ENABLED` (default false) in `QueryPipelineConfig` +- [x] Propagate `parent_id` into `ChunkEmbedding` → Qdrant payload + pgvector column; + keep deterministic child/parent ids at store time (pgvector now honors + caller-provided uuid5 ids instead of generating uuid4) +- [x] Search excludes `chunk_level=parent` by default (both providers) +- [x] `VEKTRA_PARENT_EXPANSION_ENABLED` (default false) in `QueryPipelineConfig` + mirrored in `VektraSettings` -- [ ] Expansion step in AdvancedQueryPipeline: fetch parent text by id via vector - store, replace child text **before** token budgeting (ARCH-055); dedup children - of the same parent (child text is a substring of parent → existing 80% overlap - dedup interacts) -- [ ] Trace metadata records expansion (children expanded, parents fetched) -- [ ] Reingest eval corpus with `dual` strategy; measure: dual+exclusion without - expansion (≈ fixed baseline expected), then with expansion; record both -- [ ] Unit tests: store-time linkage, search filter, expansion logic, budget accounting +- [x] Expansion step in AdvancedQueryPipeline: fetch parent text by id via new + `VectorStoreProvider.retrieve()`, replace child text **before** token + budgeting (ARCH-055); children of the same parent collapse into the + highest-scored one (runs after the 80% overlap dedup, so no interaction) +- [x] Trace metadata records expansion (children_expanded, siblings_merged, + parents_fetched) +- [x] Reingest eval corpus with `dual` strategy; measured both (see Notes): + A dual-no-expansion ≈ baseline on e2e but -6.5pp retrieval hit (boundary + effect); B expansion-on: zero e2e flips, sources 1.4 → 1.0 +- [x] Unit tests: store-time linkage, search filter, expansion logic, budget + accounting (17 new tests; suite 658 passed) ### 4. FEAT-018 — verification first (no branch unless justified) - [ ] Multi-turn scenario runner against `/api/v1/query` with `conversation_id` @@ -247,3 +251,30 @@ the bundle and a manual smoke in the Moodle dev stack. - March numbers (factual 90 / reasoning 80 / multi-chunk 10) are not comparable: different pipeline (pre BUG-015/016/017, pre FEAT-020) and per-question results were never versioned (gitignored file, since overwritten). +- 2026-07-12: **FEAT-017 measured** (full corpus `eval-full`, Combo D, same + dataset/config as baseline; corpus reingested with `dual` 500/100, parent 1500: + 105 points = 22 parents + 83 children, parents excluded from search). + Baseline to compare (fixed 500/100, 78 chunks): retrieval hit 89.1% / + MRR 0.8062 / P@5 0.3174; e2e grounded 35/55, multi-chunk 0/10, avg sources 1.4. + - **Measure A** (dual + parent exclusion, expansion off): retrieval hit 82.6% / + MRR 0.7029 / P@5 0.3130; e2e grounded 35/55 (64%), multi-chunk 1/10, avg + sources 1.4, p50 3935ms. Dual chunking itself costs ~6.5pp hit rate on this + corpus: children no longer roll overlap across parent boundaries, and 3 + questions whose keywords straddle a 1500-token section edge flip to miss + (IT-F-13, IT-R-02, EN-F-03). + - **Measure B** (expansion on): e2e grounded 35/55, multi-chunk 1/10, avg + sources 1.0 (sibling merge working: same grounding with a more compact + prompt), p50 3813ms. Zero per-question flips vs A. Expansion verified live + in traces (`parent_expansion` step, children_expanded/parents_fetched). + - **Root cause of the multi-chunk collapse found (and it is upstream of + FEAT-017)**: 9/10 multi-chunk questions end with `retrieval_filter + before=5 after=0` — bge-reranker-v2-m3 scores each partial-answer chunk + of a comparative/multi-part question below `min_relevance_score` 0.15 + (MC-01 with eval mode: max rerank score 0.088 while the raw RRF candidate + was 0.61). The whole candidate set is wiped before expansion can run. + Filed as TECH-007. Parent expansion works as designed but cannot touch + this failure mode; the Costituzione corpus also understates its benefit + (short self-contained articles — see TECH-005 collection 1). + - Local dev `.env` now: `VEKTRA_CHUNKING_STRATEGY=dual`, + `VEKTRA_PARENT_CHILD_LEVELS=1`, `VEKTRA_PARENT_EXPANSION_ENABLED=true`, + `VEKTRA_EVAL_MODE=true` (left on for FEAT-018 trace inspection). diff --git a/CHANGELOG.md b/CHANGELOG.md index f752c79d..06fe6a24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,11 +19,20 @@ Convention (Keep a Changelog 1.1.0): +### Added + +- **rag**: optional parent chunk expansion in the advanced query pipeline (FEAT-017, `VEKTRA_PARENT_EXPANSION_ENABLED`, default off). With `VEKTRA_CHUNKING_STRATEGY=dual`, retrieved child chunks are replaced with their parent chunk's text after the retrieval filter and before token budgeting; children of the same parent collapse into the highest-scored one. Parent-child linkage is now actually persisted (deterministic `uuid5(doc_id, position)` ids, `parent_id` in the Qdrant payload and in the pgvector column), a new `VectorStoreProvider.retrieve()` fetches chunks by id, and the trace records `children_expanded`/`siblings_merged`/`parents_fetched` in a `parent_expansion` step. + +### Changed + +- **index**: vector search now excludes parent-level chunks (`chunk_level=parent`) in both providers; parents are context material fetched by id during expansion, not retrieval targets. Only affects documents ingested with `dual` chunking, whose parents previously polluted search results. +- **index**: pgvector `store()` honors caller-provided UUID chunk ids (deterministic ids from ingest) instead of always generating random ones; non-UUID ids still fall back to random. + ### Fixed - **docker**: the `INSTALL_UNSTRUCTURED=true` image variant builds again (BUG-022). torchvision (transitive via unstructured-inference) resolved from PyPI with CUDA-built wheels while torch is pinned to the CPU index, crashing the build with `operator torchvision::nms does not exist`. It is now declared in the `ocr` extra and pinned to the pytorch-cpu index; a new path-filtered CI workflow builds the OCR variant so it cannot silently regress. - **index**: `/api/v1/search` now resolves the embedding, sparse-embedding, and vector-store providers from the ProviderRegistry instead of hardcoding pgvector and reading a never-populated `app.state` attribute (BUG-021). In Qdrant deployments the endpoint returned zero results (it searched the empty `document_chunks` table) and hybrid mode always fell back to dense; the RAG pipeline (`/api/v1/query`) was unaffected. Found by the Sprint 3 baseline `make eval-retrieval` run. -- **tests**: unit tests are now hermetic against the developer's local `.env` (DEBT-025). Importing litellm during pytest collection loads `.env` into the process environment, which made 4 default-assertion tests in `vektra-shared` fail on dev machines while CI stayed green. An autouse fixture in `vektra-shared/tests/conftest.py` scrubs ambient `VEKTRA_*` variables; production settings loading is unchanged. +- **tests**: unit tests are now hermetic against the developer's local `.env` (DEBT-025). Importing litellm during pytest collection loads `.env` into the process environment, which made 4 default-assertion tests in `vektra-shared` fail on dev machines while CI stayed green. An autouse fixture in `vektra-shared/tests/conftest.py` scrubs ambient `VEKTRA_*` variables; production settings loading is unchanged. The fixture is replicated in `vektra-core` and `vektra-ingest` (FEAT-017 surfaced the same leak there: ambient `VEKTRA_CHUNKING_STRATEGY=dual` broke 12 chunker and pipeline-default tests). ## [0.5.1] - 2026-07-12 diff --git a/vektra-core/tests/conftest.py b/vektra-core/tests/conftest.py new file mode 100644 index 00000000..577e6fbb --- /dev/null +++ b/vektra-core/tests/conftest.py @@ -0,0 +1,26 @@ +"""Shared fixtures for vektra-core unit tests.""" + +from __future__ import annotations + +import os + +import pytest + +# External provider keys read by ExternalApiKeys (no VEKTRA_ prefix). +_EXTERNAL_API_KEYS = ("OPENAI_API_KEY", "ANTHROPIC_API_KEY") + + +@pytest.fixture(autouse=True) +def _hermetic_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Scrub ambient config vars so default assertions stay hermetic (DEBT-025). + + When the full suite runs on a dev machine, imports during collection + (litellm calls dotenv.load_dotenv) leak the local .env into os.environ, + overriding config defaults (e.g. VEKTRA_PARENT_EXPANSION_ENABLED, + VEKTRA_EVAL_MODE). CI has no .env and never sees the difference. Tests + that need a specific value still set it explicitly via constructor + kwargs or monkeypatch.setenv. + """ + for name in list(os.environ): + if name.startswith("VEKTRA_") or name in _EXTERNAL_API_KEYS: + monkeypatch.delenv(name) diff --git a/vektra-ingest/tests/conftest.py b/vektra-ingest/tests/conftest.py new file mode 100644 index 00000000..7f4339f7 --- /dev/null +++ b/vektra-ingest/tests/conftest.py @@ -0,0 +1,26 @@ +"""Shared fixtures for vektra-ingest unit tests.""" + +from __future__ import annotations + +import os + +import pytest + +# External provider keys read by ExternalApiKeys (no VEKTRA_ prefix). +_EXTERNAL_API_KEYS = ("OPENAI_API_KEY", "ANTHROPIC_API_KEY") + + +@pytest.fixture(autouse=True) +def _hermetic_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Scrub ambient config vars so default assertions stay hermetic (DEBT-025). + + When the full suite runs on a dev machine, imports during collection + (litellm calls dotenv.load_dotenv) leak the local .env into os.environ, + overriding config defaults (e.g. VEKTRA_CHUNKING_STRATEGY=dual makes + run_ingest pick the wrong chunker under test). CI has no .env and never + sees the difference. Tests that need a specific value still set it + explicitly via constructor kwargs or monkeypatch.setenv. + """ + for name in list(os.environ): + if name.startswith("VEKTRA_") or name in _EXTERNAL_API_KEYS: + monkeypatch.delenv(name) From 3eb25235708e74e223fad014b440c42dd0f7feae Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 21:54:08 +0000 Subject: [PATCH 5/5] fix(index): validate chunk ids before Qdrant retrieve; dedup via dict.fromkeys Addresses review comments 3567194789 (invalid point ids would fail the whole retrieve batch server-side; now filtered and logged like pgvector) and 3567194791 (O(N^2) unique-preserving scan). +2 tests. Co-Authored-By: Claude Fable 5 --- .../src/vektra_core/advanced_pipeline.py | 5 +--- .../src/vektra_index/providers/qdrant.py | 12 ++++++-- vektra-index/tests/test_qdrant_provider.py | 30 ++++++++++++++++--- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/vektra-core/src/vektra_core/advanced_pipeline.py b/vektra-core/src/vektra_core/advanced_pipeline.py index 1a8eea16..b328f34f 100644 --- a/vektra-core/src/vektra_core/advanced_pipeline.py +++ b/vektra-core/src/vektra_core/advanced_pipeline.py @@ -447,10 +447,7 @@ async def _expand_parents( (ARCH-055) so the budget sees the expanded text. Results keep the child's chunk_id and score for trace comparability. """ - parent_ids: list[str] = [] - for r in results: - if r.parent_id and r.parent_id not in parent_ids: - parent_ids.append(r.parent_id) + parent_ids = list(dict.fromkeys(r.parent_id for r in results if r.parent_id)) if not parent_ids: return results, {"children_expanded": 0, "parents_fetched": 0} diff --git a/vektra-index/src/vektra_index/providers/qdrant.py b/vektra-index/src/vektra_index/providers/qdrant.py index 9904dbef..a5028de9 100644 --- a/vektra-index/src/vektra_index/providers/qdrant.py +++ b/vektra-index/src/vektra_index/providers/qdrant.py @@ -345,12 +345,20 @@ async def retrieve( Points whose payload namespace does not match are dropped (namespace isolation): Qdrant retrieve() takes no filter. """ - if not chunk_ids: + valid_ids: list[str] = [] + for cid in chunk_ids: + try: + UUID(cid) + except ValueError: + logger.warning("qdrant_retrieve_invalid_chunk_id: %s", cid) + continue + valid_ids.append(cid) + if not valid_ids: return [] records = await self._client.retrieve( collection_name=self._collection_name, - ids=chunk_ids, + ids=valid_ids, with_payload=True, ) matching = [ diff --git a/vektra-index/tests/test_qdrant_provider.py b/vektra-index/tests/test_qdrant_provider.py index 0b5ffe16..16a60d76 100644 --- a/vektra-index/tests/test_qdrant_provider.py +++ b/vektra-index/tests/test_qdrant_provider.py @@ -329,11 +329,13 @@ async def test_retrieve_filters_by_namespace(self): provider, client = _make_provider() doc_id = str(uuid4()) + parent_id = str(uuid4()) + alien_id = str(uuid4()) # Qdrant retrieve() returns Record objects with no score attribute records = [ SimpleNamespace( - id="parent-1", + id=parent_id, payload={ "text": "parent text", "document_id": doc_id, @@ -343,7 +345,7 @@ async def test_retrieve_filters_by_namespace(self): }, ), SimpleNamespace( - id="alien-1", + id=alien_id, payload={ "text": "other tenant", "document_id": doc_id, @@ -355,13 +357,24 @@ async def test_retrieve_filters_by_namespace(self): ] client.retrieve = AsyncMock(return_value=records) - results = await provider.retrieve("default", ["parent-1", "alien-1"]) + results = await provider.retrieve("default", [parent_id, alien_id]) client.retrieve.assert_awaited_once() - assert [r.chunk_id for r in results] == ["parent-1"] + assert [r.chunk_id for r in results] == [parent_id] assert results[0].score == 0.0 assert results[0].text_snippet == "parent text" + @pytest.mark.asyncio + async def test_retrieve_skips_invalid_ids(self): + """Non-UUID ids never reach the Qdrant client (it would reject the batch).""" + provider, client = _make_provider() + client.retrieve = AsyncMock(return_value=[]) + valid = str(uuid4()) + + await provider.retrieve("default", ["not-a-uuid", valid]) + + assert client.retrieve.await_args.kwargs["ids"] == [valid] + @pytest.mark.asyncio async def test_retrieve_empty_ids_short_circuits(self): provider, client = _make_provider() @@ -370,3 +383,12 @@ async def test_retrieve_empty_ids_short_circuits(self): assert results == [] client.retrieve.assert_not_called() + + @pytest.mark.asyncio + async def test_retrieve_all_invalid_ids_short_circuits(self): + provider, client = _make_provider() + + results = await provider.retrieve("default", ["parent-1", "child-2"]) + + assert results == [] + client.retrieve.assert_not_called()