Conversation
Importing litellm during pytest collection runs dotenv.load_dotenv(), leaking the developer's .env into os.environ and breaking 4 default-assertion tests in vektra-shared when the full suite runs (CI has no .env and never sees the difference). Add an autouse fixture in vektra-shared/tests/conftest.py that scrubs VEKTRA_* and external provider keys per-test via monkeypatch. Production settings loading is unchanged (no settings class uses env_file). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plan grounded in codebase analysis (2026-07-12): corrects the FEAT-017 backlog premise (parent-child linkage is not persisted today), notes Qdrant must-only filters (FEAT-018 needs must_not), FEAT-020 namespace resolution as the pattern for FEAT-021 citations, and eval harness ground truths (no RAGAS, dataset all in namespace default). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
/api/v1/search instantiated PgvectorProvider directly and read the sparse provider from a never-populated app.state attribute. In Qdrant deployments every search hit the empty document_chunks table (zero results with HTTP 200) and hybrid always fell back to dense. Resolve embedding, sparse embedding, and vector store from app.state.registry (the pipeline's contract); drop the per-request SentenceTransformers instantiation and the now-unused session dependency. Add endpoint tests. Found by the Sprint 3 baseline eval: make eval-retrieval returned zero results for all 55 questions against a healthy Qdrant stack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Baseline on the excerpt corpus (Combo D, qwen36-35b-a3b-fp8, strict): retrieval hit 100% / MRR 0.8957 / P@5 0.3261; e2e grounded 38/55 (69%), multi-chunk 3/10 is the bottleneck (chunks retrieved but cut by rerank+threshold before the prompt) - primary FEAT-017 target. Also logs the two blockers fixed to get here (BUG-021, stale vLLM model id). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… reviews TECH-005 eval suite expansion (collections, multi-turn runner, public regression slice); TECH-006 extractor/OCR bake-off + routing (GLM-OCR, PaddleOCR-VL, MinerU findings; tesseract-ita gap); FEAT-024 TEI remote embedding/reranker providers (the promised 'tei' option does not exist; includes the MiniLM 128-token truncation finding and the qdrant dense_dimensions latent bug); INFRA-007 GHCR image publishing for pull-based deployment updates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ra (BUG-022) uv resolved torchvision (transitive via unstructured-inference -> timm) from PyPI with CUDA-built wheels while torch is pinned to the CPU index, so any INSTALL_UNSTRUCTURED=true build crashed at the model pre-cache step with "operator torchvision::nms does not exist". Declare it in the ocr extra pinned to pytorch-cpu (torchvision 0.25.0+cpu; torch unchanged at 2.10.0, upper bound <0.26 keeps the relock surgical) and add a path-filtered CI workflow that builds the OCR variant so it cannot silently regress: regular CI never builds with the flag, which is why this was invisible since the extra was introduced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Autouse fixture scrubs ambient VEKTRA_* env in vektra-shared unit tests (DEBT-025) - Sprint 3 RAG-quality implementation plan added under .s2s/plans/ - Baseline eval results (excerpts + full corpus) recorded in the plan Reviews: 0 open threads Tests: CI green (all component suites + integration) Refs: DEBT-025
Addresses review comments 3566046191, 3566046197, 3566046204 (gemini-code-assist): style guide rule 6 requires annotations on all public functions, including tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-registry-providers # Conflicts: # CHANGELOG.md
…-037) ARCH-037 is the ChunkingStrategy Protocol; the DocumentExtractor-related decisions are ARCH-030 (pdfplumber extraction) and ARCH-042 (extractor dispatch). Addresses review comment 3566643596. Note: the reviewer's suggested ARCH-009 is the arq job payload design, also unrelated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The workflow only builds an image and never pushes to git, so the token actions/checkout stores in .git/config is unnecessary exposure (zizmor artipacked). Addresses review comment 3566659191. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oviders - /api/v1/search resolves embedding, sparse-embedding, and vector-store providers from the ProviderRegistry (BUG-021) - Qdrant deployments no longer get empty results; hybrid mode no longer silently degrades to dense - 3 endpoint unit tests added (registry resolution, hybrid fallback, sparse propagation) Reviews: 3/3 addressed (Gemini, style annotations) Tests: 641 passed locally; CI green Refs: BUG-021
- Backlog: TECH-005 (eval suite expansion), TECH-006 (extractor/OCR bake-off + routing), FEAT-024 (TEI embedding/reranker providers), INFRA-007 (GHCR image publishing) - TECH-006 traceability corrected to ARCH-030/ARCH-042 after review Reviews: 1/1 addressed (Gemini) Tests: n/a (docs only), CI green Refs: TECH-005, TECH-006, FEAT-024, INFRA-007
…ctured-cpu-torchvision # Conflicts: # CHANGELOG.md
…orchvision - torchvision declared in the ocr extra and pinned to the pytorch-cpu index (BUG-022): INSTALL_UNSTRUCTURED=true image builds again - New path-filtered CI workflow builds the OCR variant so it cannot silently regress - persist-credentials disabled on the workflow checkout step (review) Reviews: 2/2 addressed (CodeRabbit fixed, Gemini declined with rationale) Tests: CI green incl. OCR image build Refs: BUG-022
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…T-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 <noreply@anthropic.com>
…T-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 <noreply@anthropic.com>
….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 <noreply@anthropic.com>
…ansion - Parent-child linkage persisted end to end (deterministic uuid5 ids, Qdrant payload, pgvector column) - Search excludes parent-level chunks in both providers; new VectorStoreProvider.retrieve() - AdvancedQueryPipeline step 6.5 parent_expansion (default off) with sibling merge and trace metadata - Measured on eval-full: expansion grounding-neutral there; multi-chunk collapse root-caused to the rerank+threshold funnel (TECH-007 filed) - DEBT-025 hermetic fixture extended to vektra-core and vektra-ingest Reviews: 3/3 addressed (2 fixed, 1 declined with rationale) Tests: 660 passed locally; required CI green Refs: FEAT-017, TECH-007, ARCH-037, ARCH-055
Four multi-turn scenarios (same-topic follow-up, topic switch, give-me-more, negation) run against /api/v1/query with conversation_id on eval-full. Rewrite + FEAT-020 history already mitigate topic switch and negation; same-topic follow-ups die at the rerank threshold (TECH-007 signature); "give me more" confirms the premise but degrades gracefully. Exclusion with the current funnel would increase refusals. Plan section 4 checked off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New namespaces.config key citations_enabled (admin PATCH whitelist +
GET resolved defaults, strictly bool, no env var, default off). When
enabled, system.j2 Rule 1 instructs inline [n] markers matching the
<source id> elements, context.j2 adds a title attribute ("filename,
p.N" composed at prompt-build time from _fetch_document_names + chunk
metadata page), and SourceRef/source payloads carry a title field
(core sync + streaming, learn course response). The learn widget
renders [n] as a superscript and attaches the source title as a
tooltip in addSources(); bundle is built by the Docker widget-builder
stage. Default-off renders byte-identical prompts; the advanced
pipeline is the only consumer (simple pipeline untouched).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-021) - shared: resolve_citations_enabled (config true, missing key, non-bool value ignored, db error -> default false) - core templates: Rule 1 both branches, no-citations-without-context, default-off byte-identical rendering, title attribute escaped and omitted when absent, legacy chunks without title key unchanged - core pipeline: citations flow end to end (system rule, context title, SourceRef.title) and default-off leaves prompt untouched - admin integration: PATCH sets citations_enabled, rejects non-bool, GET resolved includes the new key with default false Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Backlog resolution records the deltas vs the original design (<source> tag kept, no new SearchResult fields, no env var). api.md documents the citations_enabled config key and the sources[].title response field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- FEAT-018 verified no-go: deferred behind TECH-007 with full multi-turn evidence - Plan section 4 checked off; Notes entry with per-scenario summary Reviews: 1/1 addressed (declined with rationale) Tests: n/a (docs only), required CI green Refs: FEAT-018, TECH-007, FEAT-020, ADR-0023
…EAT-021) The manual SourceRef -> SourceRefBody mapping in the sync /api/v1/query path dropped the new title field (schema had it, mapping did not), so citations-enabled namespaces got [n] markers in the answer but null titles in sources. Found by the live smoke test; regression asserted in test_query_json_returns_response. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- SourceRef.title passed through the sync query response mapping (was dropped, breaking citation tooltips on non-streaming responses) - Regression asserted in test_query_json_returns_response Reviews: 0 comments Tests: 672 passed + regression; CI green Refs: FEAT-021
…s the set (TECH-007) Multi-part and comparative questions get uniformly low cross-encoder scores (each chunk answers only one part of the question), so VEKTRA_MIN_RELEVANCE_SCORE=0.15 wiped the whole candidate set and the pipeline refused despite 90% raw retrieval hit on the category (9/10 multi-chunk eval questions ended with before=5 after=0). Add an opt-in rescue to the retrieval filter (ARCH-056): when the threshold empties the set and VEKTRA_RETRIEVAL_RESCUE_TOP_K > 0, keep the top-N chunks scoring at least VEKTRA_RETRIEVAL_RESCUE_FLOOR and let the LLM arbitrate via strict grounding. Default off: 0-value top_k preserves the current behavior exactly. The retrieval_filter trace step now records the rescued count in all pipelines. Score distributions measured on eval-full show no static threshold separates multi-chunk from adversarial questions (wiped multi-chunk max rerank 0.005-0.088 vs wiped adversarial 0.000-0.142), so the filter cannot discriminate: the LLM already arbitrates for the 4/9 adversarial questions the reranker scores above the threshold today. Refs: TECH-007, ARCH-056, ADR-0021 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Score distributions, mitigation rationale, and A/B numbers recorded in the sprint plan Notes and in vektra-internal (20260713-tech007-retrieval-rescue.md + eval artifacts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…irror Review: gemini #3567544954, coderabbit #3567551399. QueryPipelineConfig already enforces ge=0; the VektraSettings mirror (startup validation, ARCH-057) now rejects it too, symmetric with the rescue floor validator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…filter Review: coderabbit nitpick on the docstring promising score-descending order that the plain-threshold path does not itself enforce. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Retrieval-filter rescue: when the min-score threshold empties the candidate set, keep top-N chunks above an absolute floor (VEKTRA_RETRIEVAL_RESCUE_TOP_K/FLOOR, default off) - retrieval_filter trace step records rescued count (advanced + simple pipelines) - Measured on eval-full (top_k=3, floor=0.005): grounded 35/55 -> 54/55, multi-chunk 1 -> 10/10 harness, 0 adversarial hallucinations, latency unchanged - Config docs, .env.example, CHANGELOG, backlog and plan Notes updated Reviews: 2/2 addressed (Gemini 1, CodeRabbit 1) Tests: 684 passed Refs: TECH-007, ARCH-056, ADR-0021
VEKTRA_EMBEDDING_PROVIDER=tei was documented but no provider existed: main.py unconditionally instantiated in-process SentenceTransformers, so every instance duplicates embedding compute and long-window models served by shared TEI instances (bge-m3, 8192-token window vs MiniLM's 128) were unreachable. - TEIEmbeddingProvider (vektra-index): native /embed API, batch chunking to the TEI client batch limit, dimensions from /info with an /embed probe fallback, Bearer auth, health check. - TEIRerankerService (vektra-core): TEI /rerank with the same RerankResult semantics as the in-process service (BUG-015 score propagation, sigmoid normalization when raw scores are detected); shared _build_rerank_result helper and RerankerProtocol for typing. - Dimension plumbing: the Qdrant collection is sized from the active embedding provider instead of the hardcoded 384 default, and ensure_collection fails fast with a clear message on a dimension mismatch with an existing collection. - The cohere rerank option now passes VEKTRA_RERANK_API_KEY through; it never received a key before (dead as wired). Refs: FEAT-024, ADR-0013, ARCH-035, ARCH-036 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
check_embedding_model resolved the provider by the hardcoded 'sentence-transformers' name, so startup failed with a registration error whenever VEKTRA_EMBEDDING_PROVIDER=tei was active (found in the FEAT-024 live smoke). The 'default' alias always points to the active provider. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bge-m3 via TEI on the eval-full questions (same dual chunks): retrieval hit 82.6% -> 93.5%, MRR 0.7029 -> 0.8478, e2e stable. Configuration reference and .env.example cover the new provider options; the reindex pgvector-hardcoding discovery is recorded in the BUG-021 family note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- create_reranker: the tei branch now degrades to None on init/URL errors like the in-process path, and logs the URL redacted (coderabbit #3567707446, #3567707458) - TEIRerankerService gains aclose(); the app lifespan teardown closes remote provider clients best-effort via a new reranker registry entry (coderabbit #3567707442, #3567707464) - _embed_batch warms the dimensions cache so the startup warmup makes the synchronous dimensions() call free (coderabbit #3567707461) - _fetch_dimensions tolerates non-JSON /info responses and falls back to the /embed probe (gemini #3567695402) - ensure_collection reads the dense vector params via getattr fallback for non-dict vector configs (gemini #3567695404) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- TEIEmbeddingProvider: remote embedding via TEI /embed with dimension discovery, auth, batching - TEIRerankerService: TEI /rerank with in-process RerankResult semantics; graceful init degradation - Qdrant collection sized from the active embedding provider; clear error on dimension mismatch - Startup warmup resolves the default embedding alias; cohere rerank api_key pass-through fixed - Measured: bge-m3 vs MiniLM on eval-full questions, retrieval hit 82.6% -> 93.5%, MRR 0.7029 -> 0.8478 Reviews: 9/9 addressed (Gemini 4, CodeRabbit 5; 2 declined with rationale) Tests: 698 passed Refs: FEAT-024, ADR-0013, ARCH-035, ARCH-036
- Bump version 0.5.1 -> 0.6.0 across 8 components - Promote CHANGELOG entries from [Unreleased] to [0.6.0] - 2026-07-13 (FEAT-017 parent expansion, TECH-007 retrieval rescue, FEAT-021 citations, FEAT-024 TEI providers, BUG-021/BUG-022, DEBT-025) - Align vektra_app.__version__ (was stuck at 0.4.0-dev since v0.5.0; surfaced by /health and the OpenAPI spec) - Refresh uv.lock for the version change Prepares the develop -> main release PR for v0.6.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Version 0.5.1 -> 0.6.0 across 8 components (+ vektra_app.__version__ realignment) - CHANGELOG promoted to [0.6.0] - 2026-07-13 with fresh Unreleased block - uv.lock refreshed Reviews: none posted Tests: 698 passed Refs: release v0.6.0
📝 WalkthroughWalkthroughVektra 0.6.0 adds TEI embedding and reranking, retrieval rescue, parent chunk expansion, namespace-controlled inline citations, parent-child persistence, registry-based search providers, expanded tests, evaluation assets, documentation, and release automation. ChangesRAG platform and release
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Sprint 3 RAG quality improvements, introducing parent chunk expansion (FEAT-017), retrieval-filter rescue for multi-part questions (TECH-007), optional per-namespace inline citations (FEAT-021), and remote TEI embedding and reranking providers (FEAT-024). It also resolves several bugs, including fixing the search endpoint's provider resolution (BUG-021), repairing the OCR image build (BUG-022), and isolating unit tests from local environment variables (DEBT-025). Review feedback points out a potential ValueError crash in the URL redaction helper when parsing non-integer ports, and a repository style guide violation where a synchronous HTTP call is used to fetch embedding dimensions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
vektra-index/src/vektra_index/api.py (1)
208-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a module-level logger over per-request import/lookup.
import loggingandlogging.getLogger(__name__)inside the request handler run on every call. Hoist to module scope for clarity and to avoid the repeated lookup on the hot path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vektra-index/src/vektra_index/api.py` around lines 208 - 214, Move the logging import and logger initialization out of the request handler to module scope, defining the module-level logger once near the other imports. Update the handler to reuse that logger while leaving the registry lookup and search behavior unchanged.vektra-app/src/vektra_app/main.py (1)
188-191: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap TEI dimension lookup in
StartupValidationError
embedding_provider.dimensions()is a synchronous HTTP call during startup; if TEI is unreachable, a rawhttpxexception escapes the startup loop instead of surfacing the existing remediation path. Catch and re-raise it here so the failure is handled cleanly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vektra-app/src/vektra_app/main.py` around lines 188 - 191, Wrap the synchronous dimensions lookup in the startup flow around embedding_provider.dimensions() with StartupValidationError handling. Catch the underlying HTTP/client failure and re-raise StartupValidationError so unreachable TEI uses the existing startup remediation path, while preserving the returned embedding dimensions on success.vektra-ingest/src/vektra_ingest/chunking.py (1)
268-281: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClass docstring is now stale for the parent level.
The change makes Level 0 parents carry their own transient
parent_id, but the class docstring (Parent-child hierarchy section) still statesparent_id = Nonefor parents. Consider updating it so the hierarchy contract matches the FEAT-017 remapping behavior described in the new inline comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vektra-ingest/src/vektra_ingest/chunking.py` around lines 268 - 281, Update the Parent-child hierarchy section of the relevant chunking class docstring to state that Level 0 parent chunks carry their own transient parent_id for ingest-time remapping, replacing the outdated claim that parent_id is None. Keep the documented child relationship unchanged.vektra-index/src/vektra_index/providers/qdrant.py (1)
357-388: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
retrieve()does not scope byindex_version(unlike the pgvector provider).
PgvectorProvider.retrieve()filters on bothnamespace_idandindex_version, but this implementation filters only bynamespace_idin the post-fetch step. Parent IDs currently originate from search results (already index-version scoped), so this is low risk today, but the divergence could surface stale-version parents if IDs are ever reused across index versions. Consider mirroring the index-version scope for parity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vektra-index/src/vektra_index/providers/qdrant.py` around lines 357 - 388, The retrieve method currently filters records only by namespace, so add index_version scoping to match PgvectorProvider.retrieve(). Obtain the active index version through the existing provider configuration or helper, require each payload’s index_version to match alongside namespace_id, and pass only those records to _points_to_results.vektra-core/src/vektra_core/advanced_pipeline.py (1)
644-687: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated name-map resolution logic between
execute()and_stream().The
citations_enabled ? prompt_name_map : fetch_document_names(selected_chunks)block (lines 657-662 and 930-935) is newly introduced and duplicated verbatim across the two execution paths. A small helper would keep them in sync as this feature evolves.♻️ Proposed refactor
+ async def _resolve_source_name_map( + self, + query: QueryRequest, + prompt_name_map: dict[str, str], + selected_chunks: list[SearchResult], + ) -> dict[str, str]: + if query.citations_enabled: + return prompt_name_map + return await _fetch_document_names([r.document_id for r in selected_chunks])Then call
name_map = await self._resolve_source_name_map(query, prompt_name_map, selected_chunks)in bothexecute()and_stream().Also applies to: 824-961
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vektra-core/src/vektra_core/advanced_pipeline.py` around lines 644 - 687, Extract the duplicated source name-map selection logic into a helper method named _resolve_source_name_map, accepting query, prompt_name_map, and selected_chunks. Have it reuse prompt_name_map when citations are enabled and otherwise await _fetch_document_names for selected_chunks, then call this helper from both execute() and _stream() while preserving the existing document_names tracing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@vektra-index/src/vektra_index/startup.py`:
- Around line 55-57: Update check_provider_registration() to validate the active
embedding provider alias rather than always requiring
embedding/sentence-transformers. Align its lookup with the embedding_provider
value retrieved as "default", while preserving validation of the corresponding
TEI or sentence-transformers registration.
In `@vektra-index/tests/test_pgvector_unit.py`:
- Around line 204-213: Update both affected tests around provider.store to stop
patching DocumentChunkOrm and inspecting constructor call arguments. Replace the
ORM mock with a session.add MagicMock side effect that captures the real
objects, then assert each added instance’s id and parent_id attributes while
preserving the existing result assertions.
---
Nitpick comments:
In `@vektra-app/src/vektra_app/main.py`:
- Around line 188-191: Wrap the synchronous dimensions lookup in the startup
flow around embedding_provider.dimensions() with StartupValidationError
handling. Catch the underlying HTTP/client failure and re-raise
StartupValidationError so unreachable TEI uses the existing startup remediation
path, while preserving the returned embedding dimensions on success.
In `@vektra-core/src/vektra_core/advanced_pipeline.py`:
- Around line 644-687: Extract the duplicated source name-map selection logic
into a helper method named _resolve_source_name_map, accepting query,
prompt_name_map, and selected_chunks. Have it reuse prompt_name_map when
citations are enabled and otherwise await _fetch_document_names for
selected_chunks, then call this helper from both execute() and _stream() while
preserving the existing document_names tracing behavior.
In `@vektra-index/src/vektra_index/api.py`:
- Around line 208-214: Move the logging import and logger initialization out of
the request handler to module scope, defining the module-level logger once near
the other imports. Update the handler to reuse that logger while leaving the
registry lookup and search behavior unchanged.
In `@vektra-index/src/vektra_index/providers/qdrant.py`:
- Around line 357-388: The retrieve method currently filters records only by
namespace, so add index_version scoping to match PgvectorProvider.retrieve().
Obtain the active index version through the existing provider configuration or
helper, require each payload’s index_version to match alongside namespace_id,
and pass only those records to _points_to_results.
In `@vektra-ingest/src/vektra_ingest/chunking.py`:
- Around line 268-281: Update the Parent-child hierarchy section of the relevant
chunking class docstring to state that Level 0 parent chunks carry their own
transient parent_id for ingest-time remapping, replacing the outdated claim that
parent_id is None. Keep the documented child relationship unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 293cee21-5fe4-4a05-a600-4363963a2a45
⛔ Files ignored due to path filters (3)
.s2s/BACKLOG.mdis excluded by!.s2s/**.s2s/plans/20260712-sprint3-rag-quality.mdis excluded by!.s2s/**uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (59)
.env.example.github/workflows/docker-ocr-build.ymlCHANGELOG.mddocs/reference/api.mddocs/reference/configuration.mdtests/eval/README.mdtests/eval/dataset-full.jsonlvektra-admin/pyproject.tomlvektra-admin/src/vektra_admin/api.pyvektra-admin/tests/test_integration.pyvektra-analytics/pyproject.tomlvektra-app/pyproject.tomlvektra-app/src/vektra_app/__init__.pyvektra-app/src/vektra_app/main.pyvektra-core/pyproject.tomlvektra-core/src/vektra_core/advanced_pipeline.pyvektra-core/src/vektra_core/api.pyvektra-core/src/vektra_core/pipeline.pyvektra-core/src/vektra_core/reranker.pyvektra-core/src/vektra_core/templates.pyvektra-core/src/vektra_core/templates/context.j2vektra-core/src/vektra_core/templates/system.j2vektra-core/tests/conftest.pyvektra-core/tests/test_advanced_pipeline.pyvektra-core/tests/test_api.pyvektra-core/tests/test_pipeline.pyvektra-core/tests/test_reranker.pyvektra-core/tests/test_templates.pyvektra-index/pyproject.tomlvektra-index/src/vektra_index/adapters.pyvektra-index/src/vektra_index/api.pyvektra-index/src/vektra_index/providers/pgvector.pyvektra-index/src/vektra_index/providers/qdrant.pyvektra-index/src/vektra_index/providers/tei.pyvektra-index/src/vektra_index/startup.pyvektra-index/tests/test_api_search.pyvektra-index/tests/test_pgvector_unit.pyvektra-index/tests/test_qdrant_provider.pyvektra-index/tests/test_tei_provider.pyvektra-ingest/pyproject.tomlvektra-ingest/src/vektra_ingest/chunking.pyvektra-ingest/src/vektra_ingest/pipeline.pyvektra-ingest/tests/conftest.pyvektra-ingest/tests/test_dual_chunking.pyvektra-ingest/tests/test_pipeline.pyvektra-learn/pyproject.tomlvektra-learn/src/vektra_learn/api.pyvektra-learn/src/vektra_learn/query.pyvektra-learn/widget/src/chat-ui.jsvektra-learn/widget/src/markdown.jsvektra-learn/widget/src/styles.jsvektra-shared/pyproject.tomlvektra-shared/src/vektra_shared/config.pyvektra-shared/src/vektra_shared/namespace.pyvektra-shared/src/vektra_shared/protocols.pyvektra-shared/src/vektra_shared/types.pyvektra-shared/tests/conftest.pyvektra-shared/tests/test_config.pyvektra-shared/tests/test_namespace.py
fvadicamo
added a commit
that referenced
this pull request
Jul 13, 2026
- wire check_provider_registration into startup (ARCH-057 step 5 was dead code); checks the embedding/default alias - fix the Qdrant healthcheck: the image ships no curl, so the container was permanently unhealthy; bash /dev/tcp probe instead - remove Dockerfile CMD ["server"] so CMD_TARGET=migrate actually migrates (it hung the v0.6.0 rollout script) - stop mocking DocumentChunkOrm in pgvector tests; use the session.add intercept - docs: sync ingest returns new/exists/alias (not "indexed"), markdown is supported, and document_chunks is EMPTY in Qdrant mode (root cause of the BUG-021 reindex no-op) Reviews: Gemini no findings; CodeRabbit no findings Tests: 699 passed; CI green Refs: follow-ups from #93/#95 review + two deployment bugs found during the v0.6.0 rollout
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This release includes:
VEKTRA_PARENT_EXPANSION_ENABLED, default off)VEKTRA_RETRIEVAL_RESCUE_TOP_K/FLOOR, default off) — eval grounded 35/55 -> 54/55 with the rescue activecitations_enablednamespace config, default off)/api/v1/searchresolves providers from the registry (Qdrant mode returned empty results)INSTALL_UNSTRUCTURED=trueimage builds again (torchvision pinned to the CPU index).envWhy
Second post-Phase-2 release: closes the Sprint 3 "RAG quality" arc (multi-chunk collapse root cause fixed) plus the TEI provider work that unlocks shared host inference and long-window embedding models.
Testing
Checklist
[0.6.0] - 2026-07-13complete; versions bumped across 8 components🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation