Conversation
Add .github/workflows/publish.yml: on v* tag push, build and push
ghcr.io/vektralabs/vektra:{version} and :{version}-ocr with GHA build
cache and OCI version/revision labels, using the built-in GITHUB_TOKEN
(no new secrets). No latest tag. The manual tagging flow is unchanged.
Add deploy/docker-compose.image.yml.example so hosts can point
docker-compose.yml at the published image (docker compose pull && up -d)
instead of building from source, and document the flow in
docs/getting-started/index.md and README.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
check_provider_registration hardcoded the embedding provider name to "sentence-transformers" and was never called outside its own unit test, so it silently stopped protecting startup once TEI mode (VEKTRA_EMBEDDING_PROVIDER=tei) was added. Call it at the end of _step_5_register_providers with the configured vector_store_provider and sparse_embedding_provider, and check the "default" alias instead of the hardcoded name so it validates whichever embedding provider is active. Adds a regression test for a TEI-only registry (default + tei, no sentence-transformers). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The qdrant/qdrant:v1.17.0 image has no curl, so ["CMD", "curl", "--fail", "http://localhost:6333/healthz"] always failed with "exec: curl: executable file not found in $PATH", permanently reporting the container unhealthy in every deployment using the qdrant profile (verified on a live stack). bash is present in the image, so switch to a bash-only check that speaks raw HTTP over /dev/tcp and greps the status line. Verified empirically: recreated the live container with the new healthcheck and it flipped from unhealthy (FailingStreak 10235) to healthy (ExitCode 0) on the first probe. Also audited the TEI healthcheck (same curl pattern) against the pinned cpu-1.6 image: curl is present there, so it is left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The entrypoint's CMD_TARGET="${1:-${CMD_TARGET:-server}}" prefers the
positional arg over the env var, but the Dockerfile's CMD ["server"]
meant Docker always passed "server" as $1, so CMD_TARGET could never
win: `CMD_TARGET=migrate docker run image` silently booted a full server
instead of running the one-shot migration (this hung a production
deployment script). Remove the Dockerfile CMD - the entrypoint already
defaults CMD_TARGET to "server" when no argument is given, so
`docker run image` and `docker run image migrate` are unaffected. Fix
the comment that claimed both forms already worked.
Verified: grepped the repo for anything relying on the image's default
CMD (compose files, CI workflows, scripts, docs) - nothing does;
docker-compose.yml already sets CMD_TARGET=server as an env var for the
vektra service, so its runtime behavior is unaffected. Confirmed the
dispatch logic itself in isolation (sh, no Docker) for all four
invocation combinations, including reproducing the prior bug for
contrast. Did not perform a full image build (expensive, out of scope
per instructions).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_store_honors_deterministic_ids_and_parent_linkage and test_store_falls_back_to_random_id_on_non_uuid patched DocumentChunkOrm and asserted on the mock's call_args_list, which tests the mock's bookkeeping rather than the real ORM mapping and hides mapping errors. .coderabbit.yaml's path instructions explicitly forbid mocking SQLAlchemy ORM classes. Both tests now let store() construct real DocumentChunkOrm instances and capture them via a session.add side effect (_make_session() already stubs session.add as a MagicMock), asserting on the captured instances' .id/.parent_id attributes instead. Also annotates BUG-021 in .s2s/BACKLOG.md with a related parity gap found in the same review: QdrantVectorStoreProvider.retrieve() filters by namespace_id only, unlike PgvectorProvider.retrieve() which also filters by index_version. Not fixed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- disable credential persistence on checkout (matches the OCR build workflow) - document the pull-based deployment via COMPOSE_FILE instead of repeating -f Addresses review comments on PR #96. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t chunk-storage claim The sync ingest response returns new/exists/alias, never "indexed" (that is an async job status), and MarkdownExtractor has always handled text/markdown. The agent instructions claimed chunk text lives in document_chunks; that table is written only by PgvectorProvider, so in Qdrant mode it is empty for every namespace and the Qdrant payload is the only source of chunk text. Recorded in BUG-021 as the root cause of the reindex no-op: run_reindex reads that empty table and reports success after writing nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- INFRA-007: publish ghcr.io/vektralabs/vektra:{version} and :{version}-ocr on v* tag push
- GHA layer cache per variant, OCI version/revision labels, GITHUB_TOKEN auth, no new secrets
- deploy/docker-compose.image.yml.example: overlay that makes the documented pull flow actually work
- docs: pull-based deployment via COMPOSE_FILE
Reviews: 2/2 addressed (persist-credentials, COMPOSE_FILE)
Tests: 699 passed; CI green
Refs: INFRA-007. Publishing starts from the next tag: v0.6.0 predates this workflow.
- 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
…ress BUG-023: in Qdrant mode document_chunks is empty (only PgvectorProvider writes it), so reindex, stats and the chunk-listing endpoints operate on an empty table and report success. Root cause of the reindex no-op previously noted under BUG-021. DEBT-027: VEKTRA_PARENT_CHILD_LEVELS is validated but never consumed. TECH-008: compare chunk-RAG against link-graph navigation on a structured markdown wiki, with a mandatory closed-book control and link-aware expansion as the possible practical outcome. TECH-005: collection 1 corpus built, ingested and machine-validated; the human spot-check remains open and is now an explicit acceptance criterion, alongside the closed-book control as a standing requirement for every eval dataset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- BUG-023 (high): in Qdrant mode document_chunks is empty, so reindex/stats/chunk-listing operate on an empty table and report success - DEBT-027: VEKTRA_PARENT_CHILD_LEVELS is validated but never consumed - TECH-008: chunk-RAG vs link-graph navigation on a structured wiki, with a mandatory closed-book control - TECH-005: collection 1 progress; human spot-check made an explicit acceptance criterion; closed-book control now required for every eval dataset Reviews: 2/2 addressed (declined: referencing vektra-internal paths is the established convention in this file) Docs only, no code touched
…(BUG-023)
In Qdrant mode, which is what every real deployment runs, `document_chunks` is
empty: only PgvectorProvider writes it. Every path that read chunks from
Postgres therefore worked on an empty table and reported success. They did not
fail, they lied:
- `run_reindex` re-embedded nothing and marked the job `completed` (the root
cause of the no-op previously attributed to BUG-021)
- `GET /stats` reported `chunk_count: 0` for namespaces holding 12/105/562 chunks
- `POST /documents/{id}/chunks` wrote to the inactive store
- `GET /health` reported the index healthy without looking at the store behind it
- `DELETE /documents/{id}` returned 200 with `chunks_removed: 0`, left the points
in Qdrant, and the deleted document went on answering queries
- retention (REQ-057) hard-deleted the document row and relied on the
document_chunks CASCADE, leaving the content searchable and untraceable
ADR-0026 records the decision: `document_chunks` is private to the pgvector
provider, and chunk text, counts and deletion all go through the
VectorStoreProvider Protocol. The rejected alternative (a neutral table both
providers write) would have made Postgres a mandatory co-store under every
provider, which is the redundancy the pluggable-store design exists to avoid.
The Protocol grows three things, each with a real caller: `list_chunks()`,
`count_chunks()`, and an optional `index_version` on `store()` for reindex.
Reindex derives target-version chunk ids so it writes alongside the live version
instead of overwriting it in Qdrant, and remaps parent links (FEAT-017)
accordingly. A reindex over a non-empty namespace that stores nothing now fails
loudly, and `reindex_jobs.chunks_reindexed` (migration 0007) makes the work it
did observable.
The gap that let this survive was the test layer: the only Qdrant test mocked
the qdrant_client module wholesale, and the integration suite ran against
pgvector only. It now runs as a CI matrix over both providers, and
test_chunk_lifecycle.py asserts the lifecycle through the public API, including
that a deleted document is not retrievable.
Verified against the live Qdrant stack: reindex took the target version from
0 to 12 points with the source version intact, stats match the real point
counts, and deleting a document removes it from search.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-024) Startup validation looks the sparse provider up by the name in VEKTRA_SPARSE_EMBEDDING_PROVIDER (`fastembed-bm25`), but the app registered it only under "default": Provider 'fastembed-bm25' not registered in category 'sparse_embedding'. Available: ['default'] The vector store registers both aliases; the sparse provider registered one. So every stack with sparse embeddings enabled failed to start, which means hybrid search could not be turned on at all. Introduced by b49ce23, which wired up the check that had until then been dead code. Found while rebuilding the development stack to verify BUG-023: the container that had been up for days survived only because its image predated the defect. Registering the alias is one line. Why nothing caught it is the larger half: - `vektra-app/tests/` was executed by nothing, neither `make test` nor CI, even though it holds the tests for the module that wires every provider together. It now runs in both, via a new `test-app` job gated in the CI aggregator. - The existing check tests could not have caught it: they hand check_provider_registration a mock registry that already contains the name, so they assert the check against a fiction. test_provider_registration.py instead wires the real registration step to the real check, and fails with the exact production error when this fix is reverted. - Two app test files need Docker and said so in their docstrings, but carried no `integration` marker, so they could not be excluded from a unit run. They do now. Verified on the live stack: with VEKTRA_SPARSE_EMBEDDING_PROVIDER=fastembed-bm25 the container starts, logs sparse_embedding_registered, and reports healthy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ment Review follow-up on #101 (gemini, coderabbit). The tests pinned only the three settings they assert on; everything else fell back to defaults, which are not hermetic. Two leaks, both real: - os.environ: importing litellm (registration does) runs load_dotenv(), pulling the repo .env into the process environment for the rest of the session, so _env_file=None was not enough. - the .env file itself: registration builds its own sub-configs (QueryPipelineConfig), which resolve env_file=".env" against the working directory and bypass the settings passed in. An autouse fixture now scrubs VEKTRA_* and runs from an empty cwd, so the tests exercise the same code path on a developer machine as in CI. Reranking is on by default and is read from that internal config, so registration was loading a cross-encoder on every test: pinned off, since nothing here asserts on the reranker. Test time drops from 10.8s to 1.5s and stops depending on a model being downloadable. Also set persist-credentials: false on the test-app checkout: the job runs PR-authored test code and has no reason to keep the token in .git/config. Verified: with the alias registration removed from main.py the suite still fails (2 of 3), so the guard is intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on #102 (gemini). DELETE /documents/{id} never enforced namespace binding (H5): it took the namespace straight from the query string, so an admin key bound to one namespace could name another and have it honoured. The gap predates this PR, but it was survivable only while the delete was a no-op against the active store. Now that the delete actually removes the chunks, the same request is a real cross-namespace deletion, so it is fixed here rather than filed. The new GET /documents/{id}/chunks had the mirror-image bug: `namespace` defaulted to the literal "default", so a namespace-bound key that omitted the parameter collided with it and got a 403 on its own documents. Both endpoints now default the parameter to None and resolve it from the key binding, like /stats and /search do. The 403 now returns the ERR-AUTH-003 envelope (insufficient scope) instead of a bare string, per the repository style guide. Note the reviewer proposed ERR-AUTH-001, which is the missing/invalid-token code; the scope-violation code is ERR-AUTH-003. test_api_namespace_binding.py covers both endpoints, and fails on the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r matrix Branch protection requires a status check named "Integration tests + NFR gates". Turning the integration job into a matrix renamed it to "Integration tests (pgvector)" and "(qdrant)", so the required check would never report and every PR would sit BLOCKED forever. Add an aggregator job that carries the required name and fails if any provider in the matrix failed, mirroring the ci-gate pattern already used in ci-unit.yml. Adding a provider to the matrix now needs no change to branch protection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lias - Register the sparse embedding provider under its configured name, not only "default": startup validation looked it up by name, so any stack with VEKTRA_SPARSE_EMBEDDING_PROVIDER set failed to boot and hybrid search could not be enabled at all - Wire vektra-app/tests into make test and a new test-app CI job, gated in the aggregator: the suite was executed by nothing, which is why a provider-wiring defect shipped unnoticed - Add test_provider_registration.py, which connects the real registration step to the real check (the existing tests asserted the check against a mock registry that already had the name) - Mark the two Docker-dependent app test files as integration, as their own docstrings always claimed - Isolate the tests from the local .env (litellm's load_dotenv leaks it into os.environ; sub-configs read the file directly) Reviews: 3/3 addressed (Gemini 1, CodeRabbit 2) Tests: 718 passed, lint clean, 8 import contracts kept Verified: live stack boots with sparse enabled; guard fails on the pre-fix code Refs: BUG-024
The review follow-up commit changes shipped behaviour (a cross-namespace deletion becomes impossible) and had no changelog entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Route every chunk path (reindex, stats, delete, store, health, retention purge) through the VectorStoreProvider Protocol: in Qdrant mode they all read an empty Postgres table and reported success, so reindex re-embedded nothing while reporting "completed", stats reported 0 chunks, and a deleted document went on answering queries
- Record ADR-0026: document_chunks is private to the pgvector provider; the two providers stay interchangeable peers instead of Postgres becoming a mandatory co-store
- Protocol grows list_chunks(), count_chunks() and an optional index_version on store(); reindex fails loudly when it stores nothing, and reindex_jobs.chunks_reindexed (migration 0007) makes its work observable
- Run the integration suite as a CI matrix over both providers: it ran against pgvector only, which is why this class of bug survived for months
- Enforce namespace binding on DELETE and the new GET /documents/{id}/chunks (found in review): a no-op delete becoming a real one turned a dormant gap into a cross-namespace deletion
Reviews: 3/3 addressed (Gemini)
Tests: 733 passed, lint clean, integration green on pgvector and qdrant
Verified live on Qdrant: reindex 0 -> 12 points on the target version, stats match the real counts, deleted document drops out of search
Refs: BUG-023, ADR-0026, migration 0007
…ord the plan Backlog: - BUG-023 and BUG-024 marked completed with their PR numbers, the measured before/after evidence, and a pointer to the full record in vektra-internal - DEBT-029: DEBT-025's test isolation reaches three test packages out of eight, and a scrub of os.environ is not sufficient anyway, because sub-configs read the .env file relative to the working directory. A test that passes locally and in CI for different reasons is worse than a missing one. - DEBT-030: two vektra-app test files (the ARCH-057 startup sequence and the REQ-011 error contract) carry the integration marker but no workflow runs them, so they still test nothing - DOCS-009: the reindex endpoints are undocumented, including chunks_reindexed, which is the field an operator needs to tell a real reindex from one that did nothing Plans: - Sprint 3 closed (all five tasks done, FEAT-018 as a recorded deferral) - New plan 20260714-qdrant-parity: what shipped, why the ADR went the way it did, and the two traps found on the way (branch protection matches check names; litellm's load_dotenv defeats test hermeticity) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…riterion) Review follow-up on #103 (gemini): reranking is on by default and read from an internal config, so provider registration loads a cross-encoder on any test that touches it. Pinning it off took the new test file from 10.8s to 1.5s and removed its dependency on a model being downloadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Close BUG-023 and BUG-024 with PR numbers and the measured before/after evidence; full record in vektra-internal - File DEBT-029: DEBT-025's test isolation reaches three test packages out of eight, and scrubbing os.environ is not enough anyway (sub-configs read the .env file relative to the cwd) - File DEBT-030: the ARCH-057 startup test and the REQ-011 error-contract test carry the integration marker but no workflow runs them - DOCS-009: the reindex endpoints are undocumented, including chunks_reindexed - Close the Sprint 3 plan; add 20260714-qdrant-parity recording what shipped and the two traps found on the way Reviews: 1/1 addressed (Gemini; the model-download criterion taken, the .env rewording declined with evidence) Docs only, no code changes Refs: BUG-023, BUG-024, ADR-0026
…, DEBT-030) Two structural holes in the test layer, both of which had already let a bug through (BUG-024). DEBT-029. The local .env reached every test package, including the three that carried DEBT-025's scrub fixture. litellm calls dotenv.load_dotenv() at import and finds the repo .env by walking up from its own module inside .venv/, which lives inside the repo. The scrub cannot stop that: it runs before the test body, while the import that re-injects the file happens inside it, because the product imports litellm lazily. Measured: after `import litellm` in a test, a config left at its default resolved to the developer's .env value (qdrant) instead of the code's default (pgvector) in all four packages checked, vektra-core and vektra-shared included. Tests were running a different code path locally than in CI, which is the one thing a test must not do. Two claims in the DEBT-029 entry were wrong and measuring corrected them: the settings classes never read the .env file (no env_file in their SettingsConfigDict; they only read os.environ), and monkeypatch.chdir does nothing, because load_dotenv resolves the file relative to the calling module, not the cwd. The backlog entry now records what was actually true. The fix disarms the leak instead of undoing it: vektra_shared.testing sets LITELLM_MODE before litellm can be imported, so litellm skips its dotenv load outright. All eight test packages import that single fixture, and a structural test fails if a package is added without it. Two empty tests/__init__.py files (analytics, learn) made pytest derive the same module name for two conftests and refused to run the suite at all; removed. DEBT-030. vektra-app's two Docker-backed test files (the ARCH-057 startup sequence, the REQ-011/NFR-009 error contract) were run by no workflow. Unrun, they had rotted: both built the test container's URL with str(make_url(...)), which masks the password as "***", so Alembic authenticated with a literal "***" and every test errored at setup. They had never worked. Fixed, and they now run in an app-integration CI job gated by the integration aggregator, which keeps the name branch protection requires. 8 tests pass. Suite: 755 passed (from 733), lint clean, 8 import contracts kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on #104 (gemini): read_text() falls back to the system encoding, which is not UTF-8 everywhere. Matches templates.py, which already pins it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion - Close the local-env leak at its source: litellm's import-time load_dotenv() re-injected the repo .env into os.environ after any scrub could run, so tests ran a different code path locally than in CI. vektra_shared.testing now sets LITELLM_MODE before litellm can be imported; all eight test packages import the one shared fixture, and a structural test fails if a package is added without it - Correct two wrong claims in the DEBT-029 entry: the settings classes never read the .env file, and chdir does nothing, because load_dotenv resolves relative to the calling module - Remove two empty tests/__init__.py files that made pytest derive the same module name for two conftests and refuse to run the suite - Run vektra-app's two Docker-backed test files (ARCH-057 startup, REQ-011 error contract) in a new app-integration CI job, gated by the integration aggregator - Fix those two files, which had never worked: str(make_url(...)) masks the container password as "***", so Alembic authenticated with a literal "***" and every test errored at setup. 8 tests now pass Reviews: 1/1 addressed (Gemini) Tests: 755 passed (from 733), lint clean, app integration 8 passed, both providers green Verified: every new test fails on the pre-fix code; the canary reads pgvector in all eight packages, where it used to read qdrant Refs: DEBT-029, DEBT-030
The Qdrant-parity plan carried three explanations for why test isolation was broken. DEBT-029 (#104) measured them and all three were wrong: the scrub fixtures protected zero packages rather than three of eight (they run before the lazy litellm import that re-injects the .env), sub-configs do not read the .env file at all (no env_file in their SettingsConfigDict) and monkeypatch.chdir cannot help since load_dotenv resolves relative to the calling module, and a root conftest.py would not even be loaded in CI because each package declares its own pytest rootdir. Corrected in place rather than quietly rewritten: plausible-and-wrong is the failure mode that plan is about, and the note was not immune to it. DEBT-031: the guard from DEBT-029 checks that every test package imports the isolation fixture, but nothing checks that a test package is *executed*. Both `make test` and ci-unit.yml enumerate the eight packages by hand, so a package added tomorrow is silently unrun and no test fails. That is the exact hole BUG-024 fell through, still open one level up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…criteria Review follow-up on #105 (gemini), and the first one is instructive: the correction itself carried a wrong number. Three packages had the scrub fixture (vektra-shared, vektra-core, vektra-ingest), not two. Verified against 0a812d4. DEBT-031's proposed approach asked for both the CI job and the make test target to be checked, but the acceptance criteria only named CI. Both lists are hand-maintained and can drift apart independently, so both are now criteria. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Correct the DEBT-029 diagnosis in the Qdrant-parity plan: all three explanations it gave for the broken test isolation were wrong (the scrub protected zero packages, not three of eight; sub-configs never read the .env file; a root conftest would not even load in CI). Corrected visibly, not quietly rewritten - File DEBT-031: the DEBT-029 guard checks that every test package imports the isolation fixture, but nothing checks that a package is executed at all. make test and ci-unit.yml enumerate the eight packages by hand, so a package added tomorrow is silently unrun Reviews: 2/2 addressed (Gemini; one of them caught a wrong number inside the correction itself) Docs only, no code changes Refs: DEBT-029, DEBT-030, DEBT-031, BUG-024
- Bump version 0.6.0 -> 0.7.0 across 8 components
- Promote CHANGELOG entries from [Unreleased] to [0.7.0] - 2026-07-18
(new endpoints: GET /documents/{id}/chunks, DELETE /index-versions/{version};
removed config: VEKTRA_PARENT_CHILD_LEVELS, VEKTRA_RERANK_TOP_K;
fixes: DEBT-033/034 error envelope, BUG-023/024/025/026,
DEBT-028/029/030/031/032, INFRA-007 GHCR publish)
- Refresh uv.lock for the version change
Prepares the develop -> main release PR for v0.7.0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
CodeRabbit (PR #120, comment 3607913835) flagged three package __version__ constants left behind by the pyproject-only bump: - vektra-app: 0.6.0 -> 0.7.0 — feeds app.state.version and the OpenAPI `version` field (main.py:558, 690), so /health and /openapi.json would otherwise report 0.6.0 for a 0.7.0 build - vektra-analytics: 0.4.0-dev -> 0.7.0 - vektra-shared: 0.4.0-dev -> 0.7.0 Kept as manual constants (matching the v0.6.0 release); did not adopt the importlib.metadata derivation suggestion, to keep the change minimal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
- Cut v0.7.0 from [Unreleased]: new endpoints (GET documents/{id}/chunks, DELETE index-versions/{version}), removed config (PARENT_CHILD_LEVELS, RERANK_TOP_K), REQ-010 root envelope, BUG-023/024/025/026, DEBT-028/029/030/031/032, INFRA-007 GHCR publish
- Bump 8 components + uv.lock to 0.7.0
- Align runtime __version__ constants (app/analytics/shared) to 0.7.0 (fixes /health + OpenAPI version reporting)
Reviews: 1/1 addressed (CodeRabbit Major; Gemini no feedback)
Tests: 800 passed; make lint + make test green; CI green
Refs: v0.7.0 release prep
fvadicamo
added
documentation
github-actions
Bot
added
component:ingest
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughVersion 0.7.0 adds provider-neutral chunk lifecycle APIs, versioned reindexing and cleanup, structured error envelopes, startup validation, hermetic test coverage, provider-matrix CI, published container workflows, and expanded API/operator documentation. ChangesPlatform and indexing
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-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 a major refactoring pass to achieve Qdrant vector store parity, unifies the REQ-010 error envelope to the JSON document root, and introduces a structural test execution guard to prevent unrun test suites. Key changes include routing all chunk-level operations through the VectorStoreProvider protocol (making document_chunks private to pgvector), implementing index version cleanup, bounding query top_k to prevent CPU-bound abuse, and fixing startup validation tracebacks. Since no review comments were provided, there is no feedback to evaluate on the review itself.
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
|
Resolving the 7
None are introduced by v0.7.0 (which changes CHANGELOG, versions, and uv.lock). Out of scope for a version-bump promotion; not tracked as debt. |
The "Variable count summary" claimed 59 Pydantic-validated VEKTRA_* variables; config.py actually declares 66 distinct VEKTRA_-aliased fields (each with a single alias, no AliasChoices). The number was already stale before v0.7.0 — v0.6.0 shipped the same "59" against a different underlying count — so this is chronic drift, not a v0.7.0 regression, surfaced while auditing docs for the release. - table row: 59 -> 66 - Total documented cascades 64 -> 71 (66 + 1 CORS_ORIGINS + 2 external API keys + 2 infrastructure) - prose sentence: 59 -> 66 No config surface changed; only the descriptive count is corrected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
…review) Gemini review on #122 correctly noted that the corrected count of 66 Pydantic-validated VEKTRA_* variables was not fully backed by table rows: only 64 had entries. Add the two missing variables so the count matches the documented set: - VEKTRA_DEBUG_LOG_QUERIES -> Query pipeline table (bool, default false) - VEKTRA_ANALYTICS_STORE_TRACES -> Observability table (bool, default auto: on in dev, off in prod) Verified: all 66 code-defined VEKTRA_ aliases now have a documentation row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
- Correct the config variable count in docs/reference/configuration.md: 59 -> 66 Pydantic-validated VEKTRA_* variables; Total documented 64 -> 71 - Document the 2 previously-missing variables: VEKTRA_DEBUG_LOG_QUERIES (Query pipeline), VEKTRA_ANALYTICS_STORE_TRACES (Observability) Reviews: 1/1 addressed (Gemini medium; incomplete tables fixed) Docs only; make lint green; all 66 code-defined VEKTRA_ aliases now documented Refs: v0.7.0 docs audit
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
vektra-index/src/vektra_index/api.py (1)
154-191: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore document existence validation before chunk writes The adapter only checks that all chunks share the same
document_id; it does not prove a liveSourceDocumentOrmexists for(document_id, effective_ns)before insert. Add a namespace-scoped existence check here, or this endpoint can create searchable orphan chunks that never appear in document bookkeeping.🤖 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 154 - 191, The chunk-writing flow in the endpoint around vector_store.store must validate that a live SourceDocumentOrm exists for (document_id, effective_ns) before constructing or inserting chunks. Add the namespace-scoped document existence check before the store call, and reject the request using the endpoint’s established missing-document error behavior when no matching document exists.Source: Path instructions
vektra-index/src/vektra_index/providers/pgvector.py (1)
80-117: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake
store()idempotent for deterministic chunk IDs.run_reindexalready reuses stable target-version chunk IDs, sosession.add()here will fail on a partial retry as soon as any rows from the same document/version are already present. Use an upsert or replace the target-version rows before inserting.🤖 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/pgvector.py` around lines 80 - 117, The chunk insertion logic in store() must be idempotent when deterministic IDs already exist for the same document and target version. Update the session.add() flow to upsert or replace conflicting target-version rows before insertion, while preserving deterministic chunk IDs, parent linkage, and the existing inserted_ids 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 `@docs/getting-started/index.md`:
- Line 61: Update the published-image version references to v0.7.0: change the
walkthrough value in docs/getting-started/index.md lines 61-61, the standard and
OCR examples in deploy/docker-compose.image.yml.example lines 6-7 to 0.7.0 and
0.7.0-ocr, and the validation message in deploy/docker-compose.image.yml.example
line 24 to match those examples.
In `@scripts/reindex.sh`:
- Line 125: Update the success-response handling in scripts/reindex.sh,
including the paths around CLEANUP_BODY and the reindex response, to validate
that chunks_removed, chunks_reindexed, and source_index_version exist and have
their expected JSON types before indexing them. On missing or incorrectly typed
fields, fail with a clear API-contract error rather than exposing a Python
traceback or opaque shell failure; preserve normal processing for valid
responses.
- Around line 64-70: Update the positional-argument handling in the script’s
option-parsing case so only the first two positional arguments populate VERSION
and NAMESPACE. Reject any argument when POSITIONAL is already 2 by emitting an
error and exiting nonzero, rather than overwriting NAMESPACE.
In `@tests/integration/test_chunk_lifecycle.py`:
- Around line 155-164: Update the search precondition in the chunk lifecycle
test to verify that the returned search results contain the specific doomed_id
document, not merely that found["total"] is positive. Preserve the existing
query and search request, and assert against the result identifier field used by
the API before deleting the document.
In `@vektra-app/src/vektra_app/main.py`:
- Around line 546-552: Update the startup-step loop around step_fn in main.py to
catch unexpected exceptions and convert them into StartupValidationError,
including the failing step name and preserving the original exception as the
cause. Keep existing StartupValidationError propagation unchanged and ensure
failed steps do not emit startup_step_complete. This allows _serve() to handle
provider, import, and initialization failures through the structured
startup-failure path.
In `@vektra-index/src/vektra_index/reindex.py`:
- Around line 320-325: Update the reindexing flow around the vector_store.store
call to verify that len(stored) exactly matches the number of requested
chunk_embeddings. If the counts differ, fail the job instead of incrementing
chunks_reindexed or allowing status="completed"; preserve the existing
successful path for complete batches.
In `@vektra-index/tests/test_index_version_cleanup.py`:
- Around line 171-182: Update
test_deletes_a_superseded_version_and_reports_the_count to inspect the second
session.execute call and assert the Pgvector DELETE statement includes both the
"default" namespace predicate and index version 1 predicate. Keep the existing
removal-count and execute-call-count assertions.
In `@vektra-index/tests/test_qdrant_provider.py`:
- Around line 515-523: Update test_count_chunks_scopes_to_active_index_version
to inspect the count_filter passed through client.count and assert it includes
namespace_id="default" and index_version=2, while preserving the existing
exact=True assertion.
In `@vektra-ingest/src/vektra_ingest/jobs.py`:
- Around line 229-265: Refactor the cleanup flow around the session factory so
the initial expired-document query uses a short-lived session that closes before
the vector_store.delete calls begin. Perform all external deletions outside any
active database session, then open a second short-lived session to delete purged
IDs and commit; preserve the existing logging, skip behavior, and empty-result
handling.
In `@vektra-ingest/tests/test_cleanup.py`:
- Around line 59-94: Update test_cleanup_keeps_document_when_chunk_removal_fails
to retain the generated document ID from mock_result.all and assert that
vector_store.delete was awaited with that ID before verifying
session.execute.call_count and session.commit. Preserve the existing
database-row preservation assertions.
In `@vektra-shared/src/vektra_shared/config.py`:
- Around line 488-493: Update the llm_provider Field validation to strip
surrounding whitespace before applying the minimum-length check, ensuring
whitespace-only values are rejected while preserving the existing
VEKTRA_LLM_PROVIDER alias and description.
In `@vektra-shared/tests/test_config.py`:
- Around line 141-144: Restore the deprecated top_k property on RerankConfig,
returning the existing fetch_k value for backward-compatible reads. Update
test_stale_top_k_ignored to assert top_k remains accessible and maps safely to
fetch_k, while preserving support for the legacy VEKTRA_RERANK_TOP_K input.
In `@vektra-shared/tests/test_env_isolation_coverage.py`:
- Line 12: Strengthen the import validation in the test’s hermetic_env coverage
check to parse the source and require an actual import of the hermetic_env
symbol from vektra_shared.testing, rather than matching substrings that may
occur in comments or unrelated imports. Update the checks around the existing
import-validation logic while preserving the test’s intended failure behavior
when hermetic_env is absent.
In `@vektra-shared/tests/test_errors.py`:
- Around line 219-233: Update
TestDebt033Codes.test_new_codes_have_expected_prefixes to assert the exact
expected values for ERR_CONV_001, ERR_CONV_002, and ERR_CONV_003, while
retaining the existing exact assertions for the other error constants.
In `@vektra-shared/tests/test_suite_execution_coverage.py`:
- Around line 135-144: Update _suites() to collect both pytest-discoverable
filename patterns, test_*.py and *_test.py, while retaining the existing
directory and __pycache__ filtering and classification behavior.
---
Outside diff comments:
In `@vektra-index/src/vektra_index/api.py`:
- Around line 154-191: The chunk-writing flow in the endpoint around
vector_store.store must validate that a live SourceDocumentOrm exists for
(document_id, effective_ns) before constructing or inserting chunks. Add the
namespace-scoped document existence check before the store call, and reject the
request using the endpoint’s established missing-document error behavior when no
matching document exists.
In `@vektra-index/src/vektra_index/providers/pgvector.py`:
- Around line 80-117: The chunk insertion logic in store() must be idempotent
when deterministic IDs already exist for the same document and target version.
Update the session.add() flow to upsert or replace conflicting target-version
rows before insertion, while preserving deterministic chunk IDs, parent linkage,
and the existing inserted_ids behavior.
🪄 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: 5a112664-8930-4871-8a04-99ecc73b0182
⛔ Files ignored due to path filters (6)
.s2s/BACKLOG.mdis excluded by!.s2s/**.s2s/decisions/ADR-0026-document-chunks-pgvector-internal.mdis excluded by!.s2s/**.s2s/plans/20260712-sprint3-rag-quality.mdis excluded by!.s2s/**.s2s/plans/20260714-qdrant-parity.mdis excluded by!.s2s/**migrations/versions/0007_reindex_chunks_counter.pyis excluded by!**/migrations/**uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (93)
.claude/CLAUDE.md.env.example.gemini/styleguide.md.github/workflows/ci-unit.yml.github/workflows/integration.yml.github/workflows/publish.ymlCHANGELOG.mdDockerfileMakefileREADME.mddeploy/docker-compose.image.yml.exampledocker-compose.ymldocker/entrypoint.shdocs/getting-started/index.mddocs/reference/api.mddocs/reference/configuration.mddocs/reference/error-codes.mdscripts/reindex.shtests/integration/test_chunk_lifecycle.pytests/integration/test_e2e_flow.pytests/nfr/test_nfr_hard.pytests/nfr/test_performance.pytests/test_startup.pyvektra-admin/pyproject.tomlvektra-admin/src/vektra_admin/api.pyvektra-admin/src/vektra_admin/quotas.pyvektra-admin/tests/conftest.pyvektra-admin/tests/test_admin_turns.pyvektra-admin/tests/test_integration.pyvektra-admin/tests/test_quotas.pyvektra-analytics/pyproject.tomlvektra-analytics/src/vektra_analytics/__init__.pyvektra-analytics/tests/__init__.pyvektra-analytics/tests/conftest.pyvektra-analytics/tests/test_api.pyvektra-app/README.mdvektra-app/pyproject.tomlvektra-app/src/vektra_app/__init__.pyvektra-app/src/vektra_app/main.pyvektra-app/tests/conftest.pyvektra-app/tests/test_app_integration.pyvektra-app/tests/test_error_codes.pyvektra-app/tests/test_provider_registration.pyvektra-core/pyproject.tomlvektra-core/src/vektra_core/advanced_pipeline.pyvektra-core/src/vektra_core/api.pyvektra-core/tests/conftest.pyvektra-core/tests/test_advanced_pipeline.pyvektra-core/tests/test_api.pyvektra-core/tests/test_env_isolation.pyvektra-core/tests/test_feedback_api.pyvektra-index/README.mdvektra-index/pyproject.tomlvektra-index/src/vektra_index/adapters.pyvektra-index/src/vektra_index/api.pyvektra-index/src/vektra_index/models.pyvektra-index/src/vektra_index/providers/pgvector.pyvektra-index/src/vektra_index/providers/qdrant.pyvektra-index/src/vektra_index/reindex.pyvektra-index/src/vektra_index/startup.pyvektra-index/tests/conftest.pyvektra-index/tests/test_api_namespace_binding.pyvektra-index/tests/test_index_version_cleanup.pyvektra-index/tests/test_pgvector_unit.pyvektra-index/tests/test_qdrant_provider.pyvektra-index/tests/test_reindex.pyvektra-index/tests/test_startup.pyvektra-ingest/pyproject.tomlvektra-ingest/src/vektra_ingest/jobs.pyvektra-ingest/tests/conftest.pyvektra-ingest/tests/test_api.pyvektra-ingest/tests/test_cleanup.pyvektra-ingest/tests/test_pipeline.pyvektra-learn/pyproject.tomlvektra-learn/src/vektra_learn/query.pyvektra-learn/tests/__init__.pyvektra-learn/tests/conftest.pyvektra-learn/tests/test_api.pyvektra-learn/widget/src/api-client.jsvektra-shared/pyproject.tomlvektra-shared/src/vektra_shared/__init__.pyvektra-shared/src/vektra_shared/config.pyvektra-shared/src/vektra_shared/errors.pyvektra-shared/src/vektra_shared/http_errors.pyvektra-shared/src/vektra_shared/protocols.pyvektra-shared/src/vektra_shared/testing.pyvektra-shared/src/vektra_shared/types.pyvektra-shared/tests/conftest.pyvektra-shared/tests/test_auth.pyvektra-shared/tests/test_config.pyvektra-shared/tests/test_env_isolation_coverage.pyvektra-shared/tests/test_errors.pyvektra-shared/tests/test_suite_execution_coverage.py
💤 Files with no reviewable changes (1)
- vektra-ingest/tests/test_pipeline.py
The getting-started walkthrough and deploy/docker-compose.image.yml.example referenced VEKTRA_VERSION=0.6.1, a tag that was never published (INFRA-007 ships its first image at 0.7.0). Point operators at the tag this release actually publishes. Addresses CodeRabbit #121 (comment 3608554399). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
Every positional argument after the second silently replaced NAMESPACE, so 'reindex.sh --cleanup 1 tenant-a tenant-b' would delete from tenant-b. Reject a third positional argument with a clear error instead. Addresses CodeRabbit #121 (comment 3608554401). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
A provider returning fewer ids than requested still let the job reach status=completed, so an operator could switch the active version to an incomplete index. Raise when len(stored) != len(chunk_embeddings); run_reindex's handler marks the job failed with the message. Addresses CodeRabbit #121 (comment 3608554413). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
cleanup_soft_deleted_task held one Postgres connection open across per-document vector-store delete calls (external network I/O), risking pool exhaustion on a large backlog. Split into two short-lived sessions with the external deletions in between; behaviour (skip-empty, per-doc error skip, final log) is unchanged. Addresses CodeRabbit #121 (comment 3608554417). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
The precondition asserted found[total] > 0, which the shared fixture text could satisfy without the target document being indexed. Assert doomed_id is in the returned results, mirroring the post-delete check. Addresses CodeRabbit #121 (comment 3608554407). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
Records the nine minor CodeRabbit findings from the #121 release review that were deferred (test-strengthening plus two minor edges), and the one rejected on policy grounds (restoring removed VEKTRA_RERANK_TOP_K). The four Major items were fixed in this same PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Francesco Vadicamo <f.vadicamo@gmail.com>
- docs(deploy): published-image examples 0.6.1 -> 0.7.0 (never-published tag) - fix(scripts): reindex.sh rejects extra positional args instead of overwriting namespace - fix(index): reindex fails when the store writes a partial batch (no false completed) - fix(ingest): cleanup splits into two short-lived sessions, no DB connection held across vector-store network deletes - test(index): chunk-lifecycle asserts the doomed doc is searchable before delete - docs(backlog): DEBT-036 tracks the 9 deferred minor findings; the RERANK_TOP_K backward-compat request is rejected on policy Reviews: 2/2 addressed (Gemini; private _session_factory is a pre-existing module-wide pattern) Tests: make lint + make test green (800 passed); integration change runs in CI Refs: v0.7.0 review (#121), DEBT-036
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
Promotes the accumulated
developwork tomainas v0.7.0 (81 commits since v0.6.0). Highlights:New endpoints
GET /api/v1/documents/{id}/chunks— inspect a document's chunksDELETE /api/v1/index-versions/{version}(admin) — reclaim superseded index versions; refuses to delete the live one (DEBT-032,ERR-INDEX-001/ 409)Error contract
Correctness & hardening
top_kbounded 1..100 on/query+/learn/query(BUG-026)DELETE /documents/{id}and the chunk endpointsVectorStoreProviderProtocol (BUG-023, ADR-0026)VEKTRA_LLM_PROVIDERrejected; sparse provider registered under its configured name (BUG-024);check_provider_registrationwired at startup (ARCH-057)__version__aligned to 0.7.0 (fixes/health+ OpenAPI version reporting)Config
VEKTRA_PARENT_CHILD_LEVELS(DEBT-027) andVEKTRA_RERANK_TOP_K(DEBT-013) — both were dead/unwired, no runtime behavior change; addedVEKTRA_RERANK_FETCH_KCI / infra
ghcr.io/vektralabs/vektra:{version}+:{version}-ocron tag push (INFRA-007)CMD_TARGET=migratehonoredWhy
Post-Phase-2 correctness and operability release: closes the storage-correctness family (chunks belong to the vector store, ADR-0026), unifies the REQ-010 error envelope, and lands the GHCR publish pipeline so tagged builds ship images automatically. Ships two new admin/inspection endpoints alongside.
Testing
make lint(ruff + ruff format + mypy 77 files + import-linter 8/8 contracts) andmake test(800 passed, 3 skipped, 52 integration deselected) green on developChecklist
[0.7.0] - 2026-07-18complete; versions bumped across the 8 components + runtime__version__constantsv0.7.0follows after merge and triggerspublish.yml(GHCR images)Excluded from this release: PR #99 (TECH-005 textbook eval, ground truth pending review) — orthogonal eval material.
🤖 Generated with Claude Code
Summary by CodeRabbit
errorobject.