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 1daedd91..cff1be54 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -1092,7 +1092,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). **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 813b8984..d1035bcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,14 @@ Convention (Keep a Changelog 1.1.0): - **ci**: publish versioned container images to GHCR on tag push (INFRA-007). `v*` tags trigger `.github/workflows/publish.yml`, which builds and pushes `ghcr.io/vektralabs/vektra:{version}` and `:{version}-ocr` (GHA build cache, OCI version/revision labels, built-in `GITHUB_TOKEN`, no new secrets). Deployment docs and a new `deploy/docker-compose.image.yml.example` overlay cover the resulting `docker compose pull && docker compose up -d` flow as an alternative to building from source. No `latest` tag. The manual tagging flow is unchanged. +### 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. +- **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. + ## [0.6.0] - 2026-07-13 RAG quality release: parent chunk expansion, retrieval-filter rescue, per-namespace citations, remote TEI providers. 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-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 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 diff --git a/docs/getting-started/index.md b/docs/getting-started/index.md index 3a660c56..e03485bf 100644 --- a/docs/getting-started/index.md +++ b/docs/getting-started/index.md @@ -126,7 +126,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 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_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): 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 ---