From b49ce23e5cf7808af366a7c1e168895119d79a25 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Mon, 13 Jul 2026 13:28:21 +0000 Subject: [PATCH 1/5] fix(index): wire up check_provider_registration at startup (ARCH-057) 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) --- CHANGELOG.md | 4 ++++ vektra-app/src/vektra_app/main.py | 9 ++++++++ vektra-index/src/vektra_index/startup.py | 2 +- vektra-index/tests/test_startup.py | 29 ++++++++++++++++++++++-- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1122283a..93d131e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ Convention (Keep a Changelog 1.1.0): +### Fixed + +- **index**: `check_provider_registration` (ARCH-057 step 5) is now actually called at startup, and checks the `embedding`/`default` alias instead of a hardcoded `sentence-transformers` name. The function was dead code — nothing called it outside its own unit test — which is why TEI mode (`VEKTRA_EMBEDDING_PROVIDER=tei`) started fine even though the check could never see its provider. Found in review. + ## [0.6.0] - 2026-07-13 RAG quality release: parent chunk expansion, retrieval-filter rescue, per-namespace citations, remote TEI providers. diff --git a/vektra-app/src/vektra_app/main.py b/vektra-app/src/vektra_app/main.py index 30e8f1bb..739d0dbb 100644 --- a/vektra-app/src/vektra_app/main.py +++ b/vektra-app/src/vektra_app/main.py @@ -339,6 +339,15 @@ async def _step_5_register_providers( vektra_shared.audit.set_log_fn(_audit_impl) + # --- ARCH-057 step 5 (cont'd): verify provider registration --- + from vektra_index.startup import check_provider_registration + + await check_provider_registration( + registry, + vector_store_provider=settings.vector_store_provider, + sparse_embedding_provider=settings.sparse_embedding_provider, + ) + async def _step_6_embedding_warmup(registry: ProviderRegistry) -> None: """ARCH-057 step 6: warm up the embedding model.""" diff --git a/vektra-index/src/vektra_index/startup.py b/vektra-index/src/vektra_index/startup.py index ab4a6d2c..46c54e0e 100644 --- a/vektra-index/src/vektra_index/startup.py +++ b/vektra-index/src/vektra_index/startup.py @@ -30,7 +30,7 @@ async def check_provider_registration( and optionally the sparse embedding provider. """ required = [ - ("embedding", "sentence-transformers"), + ("embedding", "default"), ("vector_store", vector_store_provider), ] if sparse_embedding_provider: diff --git a/vektra-index/tests/test_startup.py b/vektra-index/tests/test_startup.py index e6a47dcc..4da142e4 100644 --- a/vektra-index/tests/test_startup.py +++ b/vektra-index/tests/test_startup.py @@ -17,13 +17,25 @@ def _make_registry( embed_dims: int = 384, reported_dims: int = 384, vector_store_name: str = "pgvector", + embedding_names: set[str] | None = None, ) -> MagicMock: - """Build a mock ProviderRegistry for startup checks.""" + """Build a mock ProviderRegistry for startup checks. + + ``embedding_names`` mirrors the names actually registered under the + "embedding" category (see ``_step_5_register_providers``): "default" + plus either "sentence-transformers" or "tei". Defaults to the + sentence-transformers registration when unset. + """ registry = MagicMock() + if embedding_names is None: + embedding_names = ( + {"default", "sentence-transformers"} if has_embedding else set() + ) + def _has(category: str, name: str) -> bool: if category == "embedding": - return has_embedding + return has_embedding and name in embedding_names if category == "vector_store": return has_vector_store and name == vector_store_name if category == "sparse_embedding": @@ -114,6 +126,19 @@ async def test_check_provider_registration_missing_sparse() -> None: ) +async def test_check_provider_registration_tei_only() -> None: + """Regression guard: a TEI-only registry (embedding registered as + "default" + "tei", no "sentence-transformers") must pass. The check + looks up the "default" alias, which resolves to whichever embedding + provider is active, instead of hardcoding "sentence-transformers" + (see vektra_app._step_5_register_providers). + """ + from vektra_index.startup import check_provider_registration + + registry = _make_registry(embedding_names={"default", "tei"}) + await check_provider_registration(registry) # should not raise + + # --- check_embedding_model --- From 58591b8be34c4dc2b0db751a3c86bd41769c40ca Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Mon, 13 Jul 2026 13:28:37 +0000 Subject: [PATCH 2/5] fix(docker): replace broken curl-based Qdrant healthcheck 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) --- CHANGELOG.md | 1 + docker-compose.yml | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93d131e6..c627a1c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Convention (Keep a Changelog 1.1.0): ### Fixed - **index**: `check_provider_registration` (ARCH-057 step 5) is now actually called at startup, and checks the `embedding`/`default` alias instead of a hardcoded `sentence-transformers` name. The function was dead code — nothing called it outside its own unit test — which is why TEI mode (`VEKTRA_EMBEDDING_PROVIDER=tei`) started fine even though the check could never see its provider. Found in review. +- **docker**: the Qdrant healthcheck (`docker-compose.yml`, `qdrant` service) shelled out to `curl`, which the `qdrant/qdrant` image does not ship, so the container was permanently reported `unhealthy` in every deployment using the `qdrant` profile. Replaced with a `bash`-only check against `/healthz` over `/dev/tcp`, verified against the pinned `v1.17.0` image (empirically confirmed to flip the live container from `unhealthy` to `healthy`). The TEI healthcheck was also audited and does ship `curl`, so it is unchanged. ## [0.6.0] - 2026-07-13 diff --git a/docker-compose.yml b/docker-compose.yml index 39c56f4a..a29e7923 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -94,7 +94,16 @@ services: ports: - "127.0.0.1:${QDRANT_PORT:-6333}:6333" healthcheck: - test: ["CMD", "curl", "--fail", "http://localhost:6333/healthz"] + # qdrant/qdrant has no curl/wget (Debian-slim base, verified empirically + # against v1.17.0). bash is present, so use its /dev/tcp pseudo-device + # to speak raw HTTP and grep the status line. + test: + [ + "CMD", + "bash", + "-c", + "exec 3<>/dev/tcp/localhost/6333 && printf 'GET /healthz HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n' >&3 && head -1 <&3 | grep -q '200 OK'", + ] interval: 10s timeout: 5s retries: 3 From 54b74749dbcd3d72daf5e2bdce2730150480f3c7 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Mon, 13 Jul 2026 13:28:54 +0000 Subject: [PATCH 3/5] fix(docker): make CMD_TARGET env var actually take precedence-eligible 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) --- CHANGELOG.md | 1 + Dockerfile | 6 +++++- docker/entrypoint.sh | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c627a1c6..36672c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Convention (Keep a Changelog 1.1.0): - **index**: `check_provider_registration` (ARCH-057 step 5) is now actually called at startup, and checks the `embedding`/`default` alias instead of a hardcoded `sentence-transformers` name. The function was dead code — nothing called it outside its own unit test — which is why TEI mode (`VEKTRA_EMBEDDING_PROVIDER=tei`) started fine even though the check could never see its provider. Found in review. - **docker**: the Qdrant healthcheck (`docker-compose.yml`, `qdrant` service) shelled out to `curl`, which the `qdrant/qdrant` image does not ship, so the container was permanently reported `unhealthy` in every deployment using the `qdrant` profile. Replaced with a `bash`-only check against `/healthz` over `/dev/tcp`, verified against the pinned `v1.17.0` image (empirically confirmed to flip the live container from `unhealthy` to `healthy`). The TEI healthcheck was also audited and does ship `curl`, so it is unchanged. +- **docker**: `CMD_TARGET=migrate docker run ` (the env-var invocation form) now actually runs the one-shot migration instead of silently booting a full server. The Dockerfile's `CMD ["server"]` was always passed as `$1` to the entrypoint, which the precedence chain favors over the `CMD_TARGET` env var, so the documented env-var form never worked — this hung a production deployment script waiting on a server that was never going to migrate. Removed the Dockerfile `CMD` (the entrypoint already defaults to `server` with no argument); `docker run migrate` continues to work unchanged. ## [0.6.0] - 2026-07-13 diff --git a/Dockerfile b/Dockerfile index a0eb597c..6f4666d6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -137,4 +137,8 @@ HEALTHCHECK --interval=10s --timeout=5s --start-period=30s --retries=3 \ CMD curl -so /dev/null http://localhost:8000/health || exit 1 ENTRYPOINT ["/app/entrypoint.sh"] -CMD ["server"] +# No CMD: entrypoint.sh already defaults CMD_TARGET to "server" when no +# positional argument is given. A CMD here would always win over the +# CMD_TARGET env var (Docker always passes CMD as $1), making +# `CMD_TARGET=migrate docker run image` silently start a full server +# instead of running the one-shot migration. diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 0451bb0e..6d3a3c5d 100644 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -11,6 +11,8 @@ set -e # Prefer positional arg ($1 from CMD), then env var, then default. # This allows both: docker run image migrate # and: CMD_TARGET=migrate docker run image +# The second form only works because the Dockerfile has no CMD: Docker +# always passes CMD as $1, so a CMD would permanently win over the env var. CMD_TARGET="${1:-${CMD_TARGET:-server}}" case "$CMD_TARGET" in From a0256ac39d521ff162bcc427d88016faf31382c0 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Mon, 13 Jul 2026 13:29:25 +0000 Subject: [PATCH 4/5] test(index): stop mocking DocumentChunkOrm in pgvector unit tests 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) --- .s2s/BACKLOG.md | 2 +- CHANGELOG.md | 1 + vektra-index/tests/test_pgvector_unit.py | 23 +++++++++++------------ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index c8f9feca..d2f975d8 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -1090,7 +1090,7 @@ Three near-duplicates is the threshold where extraction starts to pay off (a fou **Resolution**: the endpoint now resolves embedding, sparse embedding, and vector store from `request.app.state.registry` (same contract as the pipeline). The per-request `SentenceTransformersProvider` instantiation and the now-unused `session` dependency were removed. Unit tests added (`vektra-index/tests/test_api_search.py`): registry resolution, hybrid→dense fallback without sparse, hybrid with sparse. -**Same family, not fixed here**: `POST /documents/{id}/chunks`, `DELETE /documents/{id}` and `GET /stats` still hardcode pgvector (the stats-vs-Qdrant mismatch was already a known issue). Also `run_reindex` (`vektra-index/reindex.py`): it re-embeds with the registry's active embedding provider but stores through a hardcoded `PgvectorProvider`, so in Qdrant mode a reindex reports "completed" while the Qdrant collection receives nothing (found during the FEAT-024 live smoke, 2026-07-13: reindexing eval-full to v2 wrote to Postgres only). Track separately if needed. +**Same family, not fixed here**: `POST /documents/{id}/chunks`, `DELETE /documents/{id}` and `GET /stats` still hardcode pgvector (the stats-vs-Qdrant mismatch was already a known issue). Also `run_reindex` (`vektra-index/reindex.py`): it re-embeds with the registry's active embedding provider but stores through a hardcoded `PgvectorProvider`, so in Qdrant mode a reindex reports "completed" while the Qdrant collection receives nothing (found during the FEAT-024 live smoke, 2026-07-13: reindexing eval-full to v2 wrote to Postgres only). Track separately if needed. **Also found in review (2026-07-13)**: `QdrantVectorStoreProvider.retrieve()` (`vektra-index/src/vektra_index/providers/qdrant.py:357-388`, used for FEAT-017 parent chunk expansion) filters returned points only by `namespace_id`, with no `index_version` check — unlike `PgvectorProvider.retrieve()`, which filters on both `namespace_id` and `index_version == self._active_index_version`. In Qdrant mode, retrieving a chunk_id that belongs to a stale index version (e.g. pre-reindex) would still succeed instead of being excluded. Not fixed here. **Traceability**: ARCH-039 (ProviderRegistry), ARCH-051 (full-store contract), TECH-002 (eval harness) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36672c8b..9d36d57c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Convention (Keep a Changelog 1.1.0): - **index**: `check_provider_registration` (ARCH-057 step 5) is now actually called at startup, and checks the `embedding`/`default` alias instead of a hardcoded `sentence-transformers` name. The function was dead code — nothing called it outside its own unit test — which is why TEI mode (`VEKTRA_EMBEDDING_PROVIDER=tei`) started fine even though the check could never see its provider. Found in review. - **docker**: the Qdrant healthcheck (`docker-compose.yml`, `qdrant` service) shelled out to `curl`, which the `qdrant/qdrant` image does not ship, so the container was permanently reported `unhealthy` in every deployment using the `qdrant` profile. Replaced with a `bash`-only check against `/healthz` over `/dev/tcp`, verified against the pinned `v1.17.0` image (empirically confirmed to flip the live container from `unhealthy` to `healthy`). The TEI healthcheck was also audited and does ship `curl`, so it is unchanged. - **docker**: `CMD_TARGET=migrate docker run ` (the env-var invocation form) now actually runs the one-shot migration instead of silently booting a full server. The Dockerfile's `CMD ["server"]` was always passed as `$1` to the entrypoint, which the precedence chain favors over the `CMD_TARGET` env var, so the documented env-var form never worked — this hung a production deployment script waiting on a server that was never going to migrate. Removed the Dockerfile `CMD` (the entrypoint already defaults to `server` with no argument); `docker run migrate` continues to work unchanged. +- **tests** (index): `test_pgvector_unit.py`'s parent-chunk tests now exercise the real `DocumentChunkOrm` class via a `session.add` intercept instead of mocking the ORM class and asserting on its constructor call args, per `.coderabbit.yaml`'s instruction to never mock SQLAlchemy ORM classes. ## [0.6.0] - 2026-07-13 diff --git a/vektra-index/tests/test_pgvector_unit.py b/vektra-index/tests/test_pgvector_unit.py index e8e8e06f..71360faa 100644 --- a/vektra-index/tests/test_pgvector_unit.py +++ b/vektra-index/tests/test_pgvector_unit.py @@ -181,6 +181,8 @@ async def test_store_honors_deterministic_ids_and_parent_linkage(self): from vektra_index.providers.pgvector import PgvectorProvider session = self._make_session() + added_orms = [] + session.add.side_effect = added_orms.append provider = PgvectorProvider() parent_uuid = uuid4() @@ -201,16 +203,13 @@ async def test_store_honors_deterministic_ids_and_parent_linkage(self): ), ] - with patch("vektra_index.models.DocumentChunkOrm") as MockOrm: - MockOrm.return_value = MagicMock() - result = await provider.store(session, "default", uuid4(), chunks) + 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 + assert added_orms[0].id == parent_uuid + assert added_orms[0].parent_id is None + assert added_orms[1].id == child_uuid + assert added_orms[1].parent_id == parent_uuid @pytest.mark.asyncio async def test_store_falls_back_to_random_id_on_non_uuid(self): @@ -218,6 +217,8 @@ async def test_store_falls_back_to_random_id_on_non_uuid(self): from vektra_index.providers.pgvector import PgvectorProvider session = self._make_session() + added_orms = [] + session.add.side_effect = added_orms.append provider = PgvectorProvider() chunks = [ @@ -230,12 +231,10 @@ async def test_store_falls_back_to_random_id_on_non_uuid(self): ), ] - with patch("vektra_index.models.DocumentChunkOrm") as MockOrm: - MockOrm.return_value = MagicMock() - result = await provider.store(session, "default", uuid4(), chunks) + result = await provider.store(session, "default", uuid4(), chunks) UUID(result[0]) # random but valid - assert MockOrm.call_args.kwargs["parent_id"] is None + assert added_orms[0].parent_id is None @pytest.mark.asyncio async def test_dense_search_excludes_parent_chunks(self): From 68852d415356f897533e06698c3659e7e392dfd4 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Mon, 13 Jul 2026 18:06:55 +0000 Subject: [PATCH 5/5] docs: correct ingest status, markdown support, and the Postgres/Qdrant 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) --- .claude/CLAUDE.md | 2 +- .s2s/BACKLOG.md | 2 +- CHANGELOG.md | 1 + docs/getting-started/index.md | 2 +- docs/reference/api.md | 6 ++++-- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 984ba8ac..1efeb0a2 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -17,7 +17,7 @@ Do not construct API calls from memory or guesswork. When querying Postgres directly (psql, DB inspection): - **Always run `\d table_name` first** to check column names and types. Common pitfalls: `namespace_id` (not `namespace`), `bytea` columns that need decryption, columns that don't exist. - **Conversation turns are encrypted**: `question` and `answer` are `pgp_sym_encrypt()`'d. To read them: `SELECT pgp_sym_decrypt(question, '') FROM conversation_turns WHERE ...` using `VEKTRA_CONVERSATION_KEY` from `.env`. -- **Postgres holds metadata/text, Qdrant holds vectors**: chunk text is in `document_chunks` (Postgres), but vector search runs against Qdrant (check `VEKTRA_QDRANT_URL` and `VEKTRA_QDRANT_COLLECTION` in config for host/port/collection). To inspect vectors or search results, query Qdrant REST API directly. The Qdrant payload uses `namespace_id` as the namespace filter field. +- **Where chunk text lives depends on the vector store provider**: `document_chunks` (Postgres) is written *only* by the pgvector provider (`providers/pgvector.py`). With `VEKTRA_VECTOR_STORE_PROVIDER=qdrant` that table stays **empty**: the Qdrant provider stores chunk text and metadata in the Qdrant payload (`text`, `namespace_id`, `document_id`, `parent_id`, `metadata.chunk_level`), and Qdrant is the only source of truth for per-chunk content. Postgres still holds `source_documents` (one row per document, with `chunk_count`). So in Qdrant mode, inspect chunks via the Qdrant REST API, not SQL. The Qdrant payload uses `namespace_id` as the namespace filter field. This is also why `reindex` is a no-op in Qdrant mode (BUG-021): it reads the empty `document_chunks` table. ## Query pipeline vs search endpoint diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index d2f975d8..37e80399 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -1090,7 +1090,7 @@ Three near-duplicates is the threshold where extraction starts to pay off (a fou **Resolution**: the endpoint now resolves embedding, sparse embedding, and vector store from `request.app.state.registry` (same contract as the pipeline). The per-request `SentenceTransformersProvider` instantiation and the now-unused `session` dependency were removed. Unit tests added (`vektra-index/tests/test_api_search.py`): registry resolution, hybrid→dense fallback without sparse, hybrid with sparse. -**Same family, not fixed here**: `POST /documents/{id}/chunks`, `DELETE /documents/{id}` and `GET /stats` still hardcode pgvector (the stats-vs-Qdrant mismatch was already a known issue). Also `run_reindex` (`vektra-index/reindex.py`): it re-embeds with the registry's active embedding provider but stores through a hardcoded `PgvectorProvider`, so in Qdrant mode a reindex reports "completed" while the Qdrant collection receives nothing (found during the FEAT-024 live smoke, 2026-07-13: reindexing eval-full to v2 wrote to Postgres only). Track separately if needed. **Also found in review (2026-07-13)**: `QdrantVectorStoreProvider.retrieve()` (`vektra-index/src/vektra_index/providers/qdrant.py:357-388`, used for FEAT-017 parent chunk expansion) filters returned points only by `namespace_id`, with no `index_version` check — unlike `PgvectorProvider.retrieve()`, which filters on both `namespace_id` and `index_version == self._active_index_version`. In Qdrant mode, retrieving a chunk_id that belongs to a stale index version (e.g. pre-reindex) would still succeed instead of being excluded. Not fixed here. +**Same family, not fixed here**: `POST /documents/{id}/chunks`, `DELETE /documents/{id}` and `GET /stats` still hardcode pgvector (the stats-vs-Qdrant mismatch was already a known issue). Also `run_reindex` (`vektra-index/reindex.py`): it re-embeds with the registry's active embedding provider but stores through a hardcoded `PgvectorProvider`, so in Qdrant mode a reindex reports "completed" while the Qdrant collection receives nothing (found during the FEAT-024 live smoke, 2026-07-13: reindexing eval-full to v2 wrote to Postgres only). **Root cause established 2026-07-13** (TECH-005 ingest): `document_chunks` is written *only* by `PgvectorProvider` (`providers/pgvector.py:94`); in Qdrant mode the table is empty for every namespace, because the Qdrant provider keeps chunk text in the Qdrant payload. So `run_reindex` reads zero chunks from an empty table, re-embeds nothing, and writes nothing — while still reporting success. Any code path that reads chunk text from Postgres (reindex, stats, `GET /documents/{id}/chunks`) is broken the same way in Qdrant mode. Track separately if needed. **Also found in review (2026-07-13)**: `QdrantVectorStoreProvider.retrieve()` (`vektra-index/src/vektra_index/providers/qdrant.py:357-388`, used for FEAT-017 parent chunk expansion) filters returned points only by `namespace_id`, with no `index_version` check — unlike `PgvectorProvider.retrieve()`, which filters on both `namespace_id` and `index_version == self._active_index_version`. In Qdrant mode, retrieving a chunk_id that belongs to a stale index version (e.g. pre-reindex) would still succeed instead of being excluded. Not fixed here. **Traceability**: ARCH-039 (ProviderRegistry), ARCH-051 (full-store contract), TECH-002 (eval harness) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d36d57c..d3ddc121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Convention (Keep a Changelog 1.1.0): - **index**: `check_provider_registration` (ARCH-057 step 5) is now actually called at startup, and checks the `embedding`/`default` alias instead of a hardcoded `sentence-transformers` name. The function was dead code — nothing called it outside its own unit test — which is why TEI mode (`VEKTRA_EMBEDDING_PROVIDER=tei`) started fine even though the check could never see its provider. Found in review. - **docker**: the Qdrant healthcheck (`docker-compose.yml`, `qdrant` service) shelled out to `curl`, which the `qdrant/qdrant` image does not ship, so the container was permanently reported `unhealthy` in every deployment using the `qdrant` profile. Replaced with a `bash`-only check against `/healthz` over `/dev/tcp`, verified against the pinned `v1.17.0` image (empirically confirmed to flip the live container from `unhealthy` to `healthy`). The TEI healthcheck was also audited and does ship `curl`, so it is unchanged. +- **docs**: corrected two claims that were false for the default Qdrant deployment. The ingest API reference documented the sync response `status` as `"indexed"` (the code returns `new`, `exists` or `alias`; `indexed` is an *async job* status) and omitted Markdown from the supported formats, although `MarkdownExtractor` has been registered for `text/markdown` all along. The agent instructions (`.claude/CLAUDE.md`) claimed chunk text lives in the Postgres `document_chunks` table: that table is written only by the pgvector provider, so in Qdrant mode it is empty and Qdrant's payload is the sole source of chunk text — which is also the root cause of the `reindex` no-op tracked under BUG-021. - **docker**: `CMD_TARGET=migrate docker run ` (the env-var invocation form) now actually runs the one-shot migration instead of silently booting a full server. The Dockerfile's `CMD ["server"]` was always passed as `$1` to the entrypoint, which the precedence chain favors over the `CMD_TARGET` env var, so the documented env-var form never worked — this hung a production deployment script waiting on a server that was never going to migrate. Removed the Dockerfile `CMD` (the entrypoint already defaults to `server` with no argument); `docker run migrate` continues to work unchanged. - **tests** (index): `test_pgvector_unit.py`'s parent-chunk tests now exercise the real `DocumentChunkOrm` class via a `session.add` intercept instead of mocking the ORM class and asserting on its constructor call args, per `.coderabbit.yaml`'s instruction to never mock SQLAlchemy ORM classes. diff --git a/docs/getting-started/index.md b/docs/getting-started/index.md index 08e3d337..bc6f456e 100644 --- a/docs/getting-started/index.md +++ b/docs/getting-started/index.md @@ -107,7 +107,7 @@ The response includes a `document_id` and `chunk_count`: { "document_id": "550e8400-e29b-41d4-a716-446655440000", "chunk_count": 12, - "status": "indexed" + "status": "new" } ``` diff --git a/docs/reference/api.md b/docs/reference/api.md index d1d75611..6add49fb 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -232,7 +232,7 @@ Query parameters: |-------|------|---------|-------------| | `namespace` | string | `default` | Target namespace for the document | -Supported formats: PDF, DOCX, PPTX. Maximum file size: 50 MB (configurable via `VEKTRA_MAX_FILE_SIZE_MB`). +Supported formats: PDF, DOCX, PPTX, Markdown (`text/markdown`). Maximum file size: 50 MB (configurable via `VEKTRA_MAX_FILE_SIZE_MB`). Sync response (HTTP 200, files <= 10 MB): @@ -240,10 +240,12 @@ Sync response (HTTP 200, files <= 10 MB): { "document_id": "550e8400-...", "chunk_count": 24, - "status": "indexed" + "status": "new" } ``` +Sync status values: `new` (extracted, embedded and stored), `exists` (exact duplicate: same content hash and same filename, no work done), `alias` (same content under a different filename: an alias is added to the existing document). + Async response (HTTP 202, files > 10 MB): ```json