From b7ea5443b3afa233a9ffe7e31b4a4cf883934a3f Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 11 Aug 2026 08:06:18 +0200 Subject: [PATCH] fix: close task3 qdrant and config quality gaps --- harness/README.md | 1 + harness/pyproject.toml | 2 +- harness/tests/qdrant_test_helpers.py | 8 +- harness/tests/test_config_resources.py | 113 ++++++++++++++++++++ harness/tests/test_qdrant_cli_commands.py | 29 ++++++ harness/tests/test_qdrant_vector_store.py | 121 ++++++++++++++++------ harness/tht/adapters/vector/qdrant.py | 27 +++-- harness/tht/config.py | 36 +++++++ 8 files changed, 290 insertions(+), 47 deletions(-) diff --git a/harness/README.md b/harness/README.md index 040a2f5e..788bcc17 100644 --- a/harness/README.md +++ b/harness/README.md @@ -118,6 +118,7 @@ See `docs/testing.md` for what each interaction level validates. ```bash pytest # L0 (testcontainers, real Postgres) + L1 (pure logic + gate builders) npm test # gate widget-builder golden + fuzzy tests (JS) +pytest -m integration # marked cross-runtime checks (requires repository-local toolchains) pytest -m l2 # L2: real GLM 5.2 + remote DWH (pre-release; needs .env + VPN + CA bundle) ``` diff --git a/harness/pyproject.toml b/harness/pyproject.toml index b0853e8b..860d906a 100644 --- a/harness/pyproject.toml +++ b/harness/pyproject.toml @@ -46,4 +46,4 @@ markers = [ "l2: end-to-end tests requiring real GLM 5.2 + remote DB (skipped when .env incomplete)", "integration: cross-runtime integration tests requiring repository-local toolchains", ] -addopts = "-m 'not l2'" # L0 runs by default (Docker present); L2 opt-in +addopts = "-m 'not l2 and not integration'" # default harness gate excludes L2 and cross-runtime integration diff --git a/harness/tests/qdrant_test_helpers.py b/harness/tests/qdrant_test_helpers.py index 69cdd672..6080a26f 100644 --- a/harness/tests/qdrant_test_helpers.py +++ b/harness/tests/qdrant_test_helpers.py @@ -80,6 +80,8 @@ class FakeQdrantHttp: return FakeResponse(200, {"result": {"status": "acknowledged"}}) if method == "POST" and path == "/collections/workspace-semantic/points/query": + if self.collection is None: + return FakeResponse(404, {"status": "error"}) if self.malformed_query: return FakeResponse(200, {"result": {"points": "nope"}}) wanted = _match_points(self.points.values(), json["filter"]) @@ -97,6 +99,8 @@ class FakeQdrantHttp: return FakeResponse(200, {"result": {"points": scored[: json["limit"]]}}) if method == "POST" and path == "/collections/workspace-semantic/points/scroll": + if self.collection is None: + return FakeResponse(404, {"status": "error"}) if self.malformed_scroll: return FakeResponse(200, {"result": {"points": "bad"}}) if self.scroll_pages is not None: @@ -118,6 +122,8 @@ class FakeQdrantHttp: return FakeResponse(200, {"result": {"points": wanted, "next_page_offset": None}}) if method == "POST" and path == "/collections/workspace-semantic/points/delete": + if self.collection is None: + return FakeResponse(404, {"status": "error"}) doomed = [point["id"] for point in _match_points(self.points.values(), json["filter"])] for point_id_value in doomed: self.points.pop(point_id_value, None) @@ -159,5 +165,3 @@ def _write_record(record_id: str, kind: str, *, metadata=None): embedding=[0.1] * 1024, content_hash="sha256:" + "a" * 64, ) - - diff --git a/harness/tests/test_config_resources.py b/harness/tests/test_config_resources.py index d20853cc..58699a78 100644 --- a/harness/tests/test_config_resources.py +++ b/harness/tests/test_config_resources.py @@ -1,7 +1,12 @@ +import json + import pytest +import yaml +from typer.testing import CliRunner from tht.adapters.evidence import FilesystemEvidenceSource, HttpManifestEvidenceSource from tht.adapters.factory import build_evidence_sources +from tht.cli import app from tht.config import ( ConfigError, PgvectorDirectConfig, @@ -401,3 +406,111 @@ dwh: message = str(caught.value) assert "dwh.postgres_direct.connection.password" in message assert "missing" in message + + +@pytest.mark.parametrize("resources", [None, [], "malformed", 7]) +def test_raw_resources_non_mapping_is_a_safe_config_error(tmp_path, resources): + values = { + "dwh": { + "type": "postgres_direct", + "connection": {"database": "d", "schema": "public", "user": "u", "password": "p"}, + }, + "resources": resources, + } + path = tmp_path / "invalid-resources.yaml" + path.write_text(yaml.safe_dump(values)) + + with pytest.raises(ConfigError, match="resources"): + load_config(path) + config_result = CliRunner().invoke(app, ["config", "check", "--config", str(path)]) + assert config_result.exit_code == 1 + assert "Traceback" not in config_result.stderr + vector_result = CliRunner().invoke( + app, ["vector", "index-schema", "--json", "-c", str(path)] + ) + assert vector_result.exit_code == 1 + assert vector_result.stderr == "" + assert json.loads(vector_result.stdout) == {"status": "failed", "code": "invalid_configuration"} + + +@pytest.mark.parametrize("vector", [None, [], "malformed", 7]) +def test_raw_resources_vector_non_mapping_is_a_safe_config_error(tmp_path, vector): + values = { + "dwh": { + "type": "postgres_direct", + "connection": {"database": "d", "schema": "public", "user": "u", "password": "p"}, + }, + "resources": {"vector": vector}, + } + path = tmp_path / "invalid-resources-vector.yaml" + path.write_text(yaml.safe_dump(values)) + + with pytest.raises(ConfigError, match="resources.vector"): + load_config(path) + config_result = CliRunner().invoke(app, ["config", "check", "--config", str(path)]) + assert config_result.exit_code == 1 + assert "Traceback" not in config_result.stderr + vector_result = CliRunner().invoke( + app, ["vector", "index-schema", "--json", "-c", str(path)] + ) + assert vector_result.exit_code == 1 + assert vector_result.stderr == "" + assert json.loads(vector_result.stdout) == {"status": "failed", "code": "invalid_configuration"} + + +@pytest.mark.parametrize("embeddings", [None, [], "malformed", 7]) +def test_raw_resources_embeddings_non_mapping_is_not_silently_accepted(tmp_path, embeddings): + values = { + "dwh": { + "type": "postgres_direct", + "connection": {"database": "d", "schema": "public", "user": "u", "password": "p"}, + }, + "resources": {"embeddings": embeddings}, + } + path = tmp_path / "invalid-resources-embeddings.yaml" + path.write_text(yaml.safe_dump(values)) + + with pytest.raises(ConfigError, match="resources.embeddings"): + load_config(path) + config_result = CliRunner().invoke(app, ["config", "check", "--config", str(path)]) + assert config_result.exit_code == 1 + assert "Traceback" not in config_result.stderr + vector_result = CliRunner().invoke( + app, ["vector", "index-schema", "--json", "-c", str(path)] + ) + assert vector_result.exit_code == 1 + assert vector_result.stderr == "" + assert json.loads(vector_result.stdout) == {"status": "failed", "code": "invalid_configuration"} + + +@pytest.mark.parametrize("mutator", [ + lambda values: values.update({1: "not-a-string-key"}), + lambda values: values["resources"].update({1: {"provider": "bad"}}), + lambda values: values["resources"].update({"embeddings": {1: "bad"}}), +]) +def test_raw_non_string_mapping_keys_are_safe_config_errors(tmp_path, mutator): + values = { + "dwh": { + "type": "postgres_direct", + "connection": {"database": "d", "schema": "public", "user": "u", "password": "p"}, + }, + "resources": {"embeddings": { + "provider": "ollama_internal", "base_url": "http://embedding:11434", + "model": "qwen3-embedding:0.6b", "dimensions": 1024, + }}, + } + mutator(values) + path = tmp_path / "invalid-mapping-key.yaml" + path.write_text(yaml.safe_dump(values)) + + with pytest.raises(ConfigError, match="mapping key"): + load_config(path) + config_result = CliRunner().invoke(app, ["config", "check", "--config", str(path)]) + assert config_result.exit_code == 1 + assert "Traceback" not in config_result.stderr + vector_result = CliRunner().invoke( + app, ["vector", "index-schema", "--json", "-c", str(path)] + ) + assert vector_result.exit_code == 1 + assert vector_result.stderr == "" + assert json.loads(vector_result.stdout) == {"status": "failed", "code": "invalid_configuration"} diff --git a/harness/tests/test_qdrant_cli_commands.py b/harness/tests/test_qdrant_cli_commands.py index 9d1e9f83..34c62534 100644 --- a/harness/tests/test_qdrant_cli_commands.py +++ b/harness/tests/test_qdrant_cli_commands.py @@ -278,6 +278,35 @@ def test_vector_index_schema_json_rejects_incompatible_empty_collection(monkeypa assert not [call for call in calls if call[1].endswith("/points/scroll")] +def test_vector_index_schema_json_maps_compatible_scroll_404(monkeypatch, tmp_path): + cfg = _qdrant_runtime_config(tmp_path) + _write_schema_artifacts(tmp_path) + keyword_indexes = { + "content_hash", "document_id", "kind", "record_key", "record_kind", + "vector_generation", "workspace_id", "workspace_revision", + } + calls = [] + + def request(method, url, **kwargs): + calls.append((method, url)) + if method == "GET" and url.endswith("/collections/psd-clinical"): + return _Response(200, {"result": { + "config": {"params": {"vectors": {"size": 1024, "distance": "Cosine"}}}, + "payload_schema": {key: {"data_type": "keyword"} for key in keyword_indexes}, + }}) + if method == "POST" and url.endswith("/points/scroll"): + return _Response(404, {"status": "error"}) + raise AssertionError((method, url)) + + monkeypatch.setattr("requests.request", request) + response = CliRunner().invoke(app, ["vector", "index-schema", "--json", "-c", str(cfg)]) + + assert response.exit_code == 1 + assert response.stdout == '{"code":"semantic_index_incompatible","status":"failed"}\n' + assert response.stderr == "" + assert not [call for call in calls if call[0] in {"PUT", "DELETE"}] + + def test_vector_index_schema_json_maps_require_existing_delete_race(monkeypatch, tmp_path): cfg = _qdrant_runtime_config(tmp_path) _write_schema_artifacts(tmp_path) diff --git a/harness/tests/test_qdrant_vector_store.py b/harness/tests/test_qdrant_vector_store.py index ee82dafa..38d48d0c 100644 --- a/harness/tests/test_qdrant_vector_store.py +++ b/harness/tests/test_qdrant_vector_store.py @@ -5,7 +5,17 @@ import requests from qdrant_test_helpers import FakeQdrantHttp, FakeResponse, _write_record from tht.adapters.vector.qdrant import QdrantVectorStore, point_id -from tht.ports.vector import VectorStoreError +from tht.ports.vector import ( + SemanticIndexIncompatibleError, + VectorResponseError, + VectorStoreError, + VectorTransportError, +) + +_REQUIRED_INDEXES = { + "content_hash", "document_id", "kind", "record_key", "record_kind", + "vector_generation", "workspace_id", "workspace_revision", +} def _store(fake: FakeQdrantHttp, *, collection_lifecycle="create_if_missing") -> QdrantVectorStore: @@ -51,29 +61,7 @@ def test_require_existing_refuses_missing_collection_without_mutations(): assert [call for call in fake.calls if call[0] == "PUT"] == [] -def test_require_existing_preflights_before_existing_hash_scroll(): - fake = FakeQdrantHttp() - original_request = fake.request - - def request(method, url, **kwargs): - if method == "POST" and url.endswith("/points/scroll"): - return FakeResponse(404, {"status": "error"}) - return original_request(method, url, **kwargs) - - store = QdrantVectorStore( - base_url="http://qdrant:6333", collection="workspace-semantic", workspace_id="demo", - workspace_revision="a" * 40, expected_dimension=1024, - collection_lifecycle="require_existing", request=request, - ) - - with pytest.raises(VectorStoreError, match="semantic_index_incompatible"): - store.existing_hashes("memory", ["memory"]) - - assert [call for call in fake.calls if call[1].endswith("/points/scroll")] == [] - assert [call for call in fake.calls if call[0] == "PUT"] == [] - - -def test_require_existing_delete_maps_collection_404_after_preflight(): +def test_require_existing_maps_scroll_404_after_compatible_preflight(): fake = FakeQdrantHttp() fake.collection = {"vectors": {"size": 1024, "distance": "Cosine"}} fake.payload_indexes = { @@ -81,16 +69,9 @@ def test_require_existing_delete_maps_collection_404_after_preflight(): "vector_generation", "workspace_id", "workspace_revision", } original_request = fake.request - deleted = False def request(method, url, **kwargs): - nonlocal deleted - if method == "GET" and url.endswith("/collections/workspace-semantic") and not deleted: - response = original_request(method, url, **kwargs) - deleted = True - fake.collection = None - return response - if method == "POST" and url.endswith("/points/delete?wait=true"): + if method == "POST" and url.endswith("/points/scroll"): original_request(method, url, **kwargs) return FakeResponse(404, {"status": "error"}) return original_request(method, url, **kwargs) @@ -101,8 +82,79 @@ def test_require_existing_delete_maps_collection_404_after_preflight(): collection_lifecycle="require_existing", request=request, ) - with pytest.raises(VectorStoreError, match="semantic_index_incompatible"): - store.delete_kinds("memory", ["memory"]) + with pytest.raises(SemanticIndexIncompatibleError): + store.existing_hashes("memory", ["memory"]) + + assert [call for call in fake.calls if call[1].endswith("/points/scroll")] + assert [call for call in fake.calls if call[0] == "PUT"] == [] + + +@pytest.mark.parametrize( + ("operation", "generation"), + [("delete_kinds", None), ("delete_generation", "gen:" + "a" * 32)], +) +def test_require_existing_maps_delete_scroll_404_after_compatible_preflight(operation, generation): + fake = FakeQdrantHttp() + fake.collection = {"vectors": {"size": 1024, "distance": "Cosine"}} + fake.payload_indexes = { + "content_hash", "document_id", "kind", "record_key", "record_kind", + "vector_generation", "workspace_id", "workspace_revision", + } + original_request = fake.request + + def request(method, url, **kwargs): + if method == "POST" and url.endswith("/points/scroll"): + original_request(method, url, **kwargs) + return FakeResponse(404, {"status": "error"}) + return original_request(method, url, **kwargs) + + store = QdrantVectorStore( + base_url="http://qdrant:6333", collection="workspace-semantic", workspace_id="demo", + workspace_revision="a" * 40, expected_dimension=1024, + collection_lifecycle="require_existing", request=request, + ) + + with pytest.raises(SemanticIndexIncompatibleError): + if operation == "delete_kinds": + store.delete_kinds("memory", ["memory"]) + else: + store.delete_generation("evidence", generation, "demo") + + assert [call for call in fake.calls if call[1].endswith("/points/scroll")] + assert [call for call in fake.calls if call[0] == "POST" and "delete" in call[1]] == [] + + +def test_require_existing_scroll_non_404_remains_transport_error(): + fake = FakeQdrantHttp() + fake.collection = {"vectors": {"size": 1024, "distance": "Cosine"}} + fake.payload_indexes = set(_REQUIRED_INDEXES) + original_request = fake.request + + def request(method, url, **kwargs): + if method == "POST" and url.endswith("/points/scroll"): + original_request(method, url, **kwargs) + return FakeResponse(503, {"status": "error"}) + return original_request(method, url, **kwargs) + + store = QdrantVectorStore( + base_url="http://qdrant:6333", collection="workspace-semantic", workspace_id="demo", + workspace_revision="a" * 40, expected_dimension=1024, + collection_lifecycle="require_existing", request=request, + ) + with pytest.raises(VectorTransportError) as caught: + store.existing_hashes("memory", ["memory"]) + assert caught.value.status_code == 503 + + +def test_require_existing_scroll_malformed_remains_response_error(): + fake = FakeQdrantHttp() + fake.collection = {"vectors": {"size": 1024, "distance": "Cosine"}} + fake.payload_indexes = set(_REQUIRED_INDEXES) + fake.malformed_scroll = True + store = _store(fake, collection_lifecycle="require_existing") + with pytest.raises(VectorResponseError): + store.existing_hashes("memory", ["memory"]) + @pytest.mark.parametrize( ("dimension", "distance", "indexes", "index_types"), @@ -581,6 +633,7 @@ def test_upsert_payload_keeps_canonical_identity_when_metadata_collides(): def test_scroll_based_operations_paginate_until_next_page_offset_is_absent(): fake = FakeQdrantHttp() + fake.collection = {"vectors": {"size": 1024, "distance": "Cosine"}} generation_a = "gen:" + "1" * 32 generation_b = "gen:" + "2" * 32 fake.scroll_pages = [ diff --git a/harness/tht/adapters/vector/qdrant.py b/harness/tht/adapters/vector/qdrant.py index 6b73ec18..4ecb18da 100644 --- a/harness/tht/adapters/vector/qdrant.py +++ b/harness/tht/adapters/vector/qdrant.py @@ -420,16 +420,23 @@ class QdrantVectorStore: offset = None seen_offsets = set() while True: - response = self._call( - "POST", - f"/collections/{self._collection}/points/scroll", - { - "with_payload": True, - "limit": 10000, - "filter": {"must": must}, - "offset": offset, - }, - ) + try: + response = self._call( + "POST", + f"/collections/{self._collection}/points/scroll", + { + "with_payload": True, + "limit": 10000, + "filter": {"must": must}, + "offset": offset, + }, + ) + except VectorTransportError as exc: + if self._collection_lifecycle == "require_existing" and exc.status_code == 404: + raise SemanticIndexIncompatibleError( + "Qdrant collection disappeared during semantic index reconciliation" + ) from exc + raise result = response.get("result", {}) page = result.get("points") if not isinstance(page, list): diff --git a/harness/tht/config.py b/harness/tht/config.py index f879d7d6..9a26b5d9 100644 --- a/harness/tht/config.py +++ b/harness/tht/config.py @@ -537,6 +537,41 @@ def _format_validation_error(error: ValidationError) -> str: return "\n".join(lines) +def _validate_raw_config_shape(raw: dict[str, Any], path: Path) -> None: + """Reject unsafe YAML shapes before compatibility translation or sorting keys.""" + seen: set[int] = set() + + def walk(value: Any, location: str) -> None: + if isinstance(value, dict): + marker = id(value) + if marker in seen: + return + seen.add(marker) + for key, item in value.items(): + if not isinstance(key, str): + raise ConfigError( + f"Configurazione non valida in {path}: mapping key at {location} " + "must be a string" + ) + walk(item, f"{location}.{key}") + elif isinstance(value, list): + for index, item in enumerate(value): + walk(item, f"{location}[{index}]") + + walk(raw, "configuration") + resources = raw.get("resources") + if "resources" in raw and not isinstance(resources, dict): + raise ConfigError( + f"Configurazione non valida in {path}: resources must be a mapping" + ) + if isinstance(resources, dict): + for name in ("vector", "embeddings"): + if name in resources and not isinstance(resources[name], dict): + raise ConfigError( + f"Configurazione non valida in {path}: resources.{name} must be a mapping" + ) + + def load_config(path: Path) -> Config: if not path.exists(): raise ConfigError(f"File di configurazione non trovato: {path}") @@ -546,6 +581,7 @@ def load_config(path: Path) -> Config: raise ConfigError(f"Configurazione YAML non valida: {path}") from exc if not isinstance(raw, dict): raise ConfigError(f"Configurazione non valida (atteso un mapping YAML): {path}") + _validate_raw_config_shape(raw, path) expanded = _resolve_secret_files(_resolve_evidence_secret_files(_expand_env(raw))) _validate_internal_embedding_contract(expanded, path) _validate_internal_vector_contract(expanded, path)