From 820b23723926daaa8ab47c8c3fb8d7460a10ad2f Mon Sep 17 00:00:00 2001 From: mptyl Date: Sat, 8 Aug 2026 18:21:20 +0200 Subject: [PATCH] fix: remove http delete-kinds regression --- .../task-6-report.md | 114 ++++++++++++++++++ .../tests/l0/test_vector_adapter_parity.py | 6 + harness/tests/test_qdrant_cli_commands.py | 1 + harness/tests/test_vector_port_contract.py | 1 + harness/tht/adapters/vector/thoth_http.py | 8 -- harness/tht/cli/memory_cmd.py | 9 +- harness/tht/ports/vector.py | 2 - harness/tht/vectorstore/rest_client.py | 16 --- 8 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 .superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-6-report.md diff --git a/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-6-report.md b/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-6-report.md new file mode 100644 index 00000000..a70b3c9b --- /dev/null +++ b/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-6-report.md @@ -0,0 +1,114 @@ +# Task 6 Report + +Date: 2026-08-08 + +Status: implemented and verified + +Summary: + +- Added schema-v3 Qdrant runtime support to the harness config/resource layer and vector factory. +- Made Qdrant payloads carry `workspace_id` and `workspace_revision` on every point. +- Routed schema and memory bulk indexing through the transport-neutral vector port with canonical hash-based dedup. +- Kept Evidence canonical on filesystem and Memory canonical in JSONL; Qdrant remains derived/rebuildable. +- Added focused tests for semantic-kind isolation, shared identity fields, search-pack kind boundaries, and the schema-v3 factory/config path. + +Files changed: + +- `harness/tht/config.py` +- `harness/tht/config_compat.py` +- `harness/tht/adapters/factory.py` +- `harness/tht/adapters/vector/qdrant.py` +- `harness/tht/vectorstore/records.py` +- `harness/tht/cli/vector_cmd.py` +- `harness/tht/cli/memory_cmd.py` +- `harness/tests/test_semantic_kind_isolation.py` +- `harness/tests/test_memory_save_one.py` +- `harness/tests/test_search_pack.py` +- `harness/tests/test_qdrant_vector_store.py` +- `harness/tests/test_adapter_factory.py` +- `harness/tests/test_config_resources.py` + +Verification: + +- Focused RED/GREEN task suite: + - `cd harness && .venv/bin/pytest tests/test_semantic_kind_isolation.py tests/test_memory_save_one.py tests/test_search_pack.py -q` +- Relevant harness suite: + - `cd harness && .venv/bin/pytest tests/test_semantic_kind_isolation.py tests/test_memory_save_one.py tests/test_search_pack.py tests/test_qdrant_vector_store.py tests/test_adapter_factory.py tests/test_config_resources.py tests/test_vector_port_contract.py tests/test_corpus_pipeline.py -q` + - Result: `131 passed` +- Changed-file Ruff: + - `cd harness && .venv/bin/ruff check tht/vectorstore/records.py tht/adapters/vector/qdrant.py tht/config_compat.py tht/config.py tht/adapters/factory.py tht/cli/vector_cmd.py tht/cli/memory_cmd.py tests/test_memory_save_one.py tests/test_search_pack.py tests/test_semantic_kind_isolation.py tests/test_qdrant_vector_store.py tests/test_adapter_factory.py tests/test_config_resources.py` + - Result: clean + +Concerns / follow-up: + +- `memory clear` still retains its older direct-vector assumptions and was not expanded in this task because the brief focused on canonical builders and schema/evidence/memory routing through the active Qdrant path. +- The relevant suite still emits pre-existing warnings (legacy config deprecation in older fixtures, plus existing Pydantic serializer warnings in corpus tests), but they are not introduced by this task. + +## Fix round 1 (2026-08-08) + +Scope: + +- Fixed qdrant-only schema-v3 command gating for `vector index-schema`, `memory promote`, and `memory index`. +- Replaced `memory clear`'s direct-pgvector-only path with vector-port deletion by kind. +- Added focused qdrant-only CLI regression tests and refreshed older CLI fixtures to the enforced internal embedding contract. + +RED evidence: + +- `cd harness && .venv/bin/pytest tests/test_qdrant_cli_commands.py -q` +- Initial result against commit `5e39cfa`: `4 failed` +- Failure signatures: + - `ERRORE: sezioni mancanti nel workspace yaml: vector_db o vector_write_rest.` + - `ERRORE: sezioni mancanti nel workspace yaml: vector_db.` + +GREEN evidence: + +- Focused fix suite: + - `cd harness && .venv/bin/pytest tests/test_qdrant_cli_commands.py tests/test_qdrant_vector_store.py tests/test_adapter_factory.py tests/test_config_resources.py tests/test_memory_save_one.py tests/test_search_pack.py -q` + - Result: `51 passed` +- Relevant broader vector/memory/schema/search suite: + - `cd harness && .venv/bin/pytest tests/test_qdrant_cli_commands.py tests/test_qdrant_vector_store.py tests/test_adapter_factory.py tests/test_config_resources.py tests/test_memory_save_one.py tests/test_search_pack.py tests/test_vector_port_contract.py tests/test_adapter_command_regressions.py tests/test_solved_search_cli.py tests/test_schema_introspect_guard.py tests/test_semantic_kind_isolation.py tests/test_corpus_pipeline.py -q` + - Result: `154 passed` +- Ruff on the fix surface: + - `cd harness && .venv/bin/ruff check tht/ports/vector.py tht/adapters/vector/qdrant.py tht/adapters/vector/pgvector.py tht/adapters/vector/thoth_http.py tht/vectorstore/rest_client.py tht/cli/vector_cmd.py tht/cli/memory_cmd.py tests/test_qdrant_cli_commands.py tests/test_solved_search_cli.py` + - Result: clean + +Notes: + +- `memory clear` now deletes derived `kind=memory` points through the configured writable vector store, while leaving the JSONL registry as the source of truth until the registry file is removed by the command. +- The broader suite still carries the same pre-existing warnings noted above; this fix round did not add new warnings or failures. + +## Fix round 2 (2026-08-08) + +Scope: + +- Removed the accidental HTTP writer `delete_kinds` capability expansion from `ThothHttpVectorStore` and `VectorRestClient`. +- Reworked `memory clear` so schema-v3 Qdrant uses scoped `kind=memory` deletion, while legacy transports keep the pre-task direct-sync path instead of advertising a nonexistent RPC. +- Tightened the qdrant-only memory-clear regression to assert the exact `("memory", ["memory"])` delete scope. + +RED evidence: + +- Re-review found a transport contract mismatch in fix round 1: + - `ThothHttpVectorStore` exposed `delete_kinds(...)` + - `VectorRestClient` exposed `delete_kinds(...)` + - but the legacy HTTP writer migration only allowlists `delete_vector_generation`, not `delete_vector_kinds` +- The new regressions added in this round capture that mismatch and the missing qdrant delete-scope assertion: + - `tests/test_vector_port_contract.py::test_http_store_supports_writer_without_reader` + - `tests/l0/test_vector_adapter_parity.py::test_http_rest_client_does_not_advertise_nonexistent_delete_kinds_rpc` + - `tests/test_qdrant_cli_commands.py::test_memory_clear_accepts_qdrant_only_runtime_config` + +GREEN evidence: + +- Focused regression suite: + - `cd harness && .venv/bin/pytest tests/test_qdrant_cli_commands.py tests/test_vector_port_contract.py tests/l0/test_vector_adapter_parity.py tests/test_adapter_command_regressions.py -q` + - Result: `53 passed` +- Broader relevant vector/memory/search suite: + - `cd harness && .venv/bin/pytest tests/test_qdrant_cli_commands.py tests/test_adapter_command_regressions.py tests/test_vector_port_contract.py tests/l0/test_vector_adapter_parity.py tests/test_solved_search_cli.py tests/test_qdrant_vector_store.py tests/test_search_similar_kinds.py tests/test_corpus_pipeline.py -q` + - Result: `135 passed` +- Ruff on the changed fix surface: + - `cd harness && .venv/bin/ruff check tht/cli/memory_cmd.py tht/ports/vector.py tht/adapters/vector/thoth_http.py tht/vectorstore/rest_client.py tests/test_qdrant_cli_commands.py tests/test_vector_port_contract.py tests/l0/test_vector_adapter_parity.py` + - Result: clean + +Notes: + +- Legacy HTTP/vector-rest deployments do not gain a new destructive RPC surface from this fix; they keep their previous behavior and continue to fail closed for unsupported cleanup. +- The broader suite still emits the same pre-existing deprecation and serializer warnings already noted above; this round did not introduce new warnings. diff --git a/harness/tests/l0/test_vector_adapter_parity.py b/harness/tests/l0/test_vector_adapter_parity.py index ec5899ad..c77f21b1 100644 --- a/harness/tests/l0/test_vector_adapter_parity.py +++ b/harness/tests/l0/test_vector_adapter_parity.py @@ -253,6 +253,12 @@ def test_http_delete_generation_legacy_404_fails_closed_without_body_leak(monkey assert "secret" not in str(error.value) +def test_http_rest_client_does_not_advertise_nonexistent_delete_kinds_rpc(): + client = VectorRestClient(RestConfig(base_url="https://vectors.test", api_key="writer")) + + assert hasattr(client, "delete_kinds") is False + + def test_http_list_evidence_generations_exact_rpc_and_legacy_fail_closed(monkeypatch): calls = [] monkeypatch.setattr( diff --git a/harness/tests/test_qdrant_cli_commands.py b/harness/tests/test_qdrant_cli_commands.py index b7dd0ca1..0a8c49b1 100644 --- a/harness/tests/test_qdrant_cli_commands.py +++ b/harness/tests/test_qdrant_cli_commands.py @@ -161,4 +161,5 @@ def test_memory_clear_accepts_qdrant_only_runtime_config(tmp_path, monkeypatch): res = CliRunner().invoke(app, ["memory", "clear", "--yes", "-c", str(cfg)]) assert res.exit_code == 0, res.output + assert store.deleted == [("memory", ["memory"])] assert not registry.exists() diff --git a/harness/tests/test_vector_port_contract.py b/harness/tests/test_vector_port_contract.py index 42852a52..5fae5236 100644 --- a/harness/tests/test_vector_port_contract.py +++ b/harness/tests/test_vector_port_contract.py @@ -35,6 +35,7 @@ def test_http_store_supports_writer_without_reader(): assert store.capabilities.search is False assert store.capabilities.existing_hashes is True assert store.capabilities.upsert is True + assert hasattr(store, "delete_kinds") is False with pytest.raises(VectorReadUnavailable): store.search(["memory"], [0.1], limit=1) diff --git a/harness/tht/adapters/vector/thoth_http.py b/harness/tht/adapters/vector/thoth_http.py index 7f768e60..a202af14 100644 --- a/harness/tht/adapters/vector/thoth_http.py +++ b/harness/tht/adapters/vector/thoth_http.py @@ -155,14 +155,6 @@ class ThothHttpVectorStore: except VectorRestError as exc: raise VectorStoreError(str(exc)) from exc - def delete_kinds(self, collection: str, kinds: list[str]) -> int: - _collection("vectors", collection) - _validate_collection_kinds(collection, kinds) - try: - return self._require_writer().delete_kinds(collection, kinds) - except VectorRestError as exc: - raise VectorStoreError(str(exc)) from exc - def delete_generation(self, collection: str, generation: str, workspace_id: str) -> int: if collection != "evidence" or re.fullmatch(r"gen:[0-9a-f]{32}", generation) is None: raise VectorStoreError("Only exact Evidence generations may be deleted") diff --git a/harness/tht/cli/memory_cmd.py b/harness/tht/cli/memory_cmd.py index be818e25..fc4d74dc 100644 --- a/harness/tht/cli/memory_cmd.py +++ b/harness/tht/cli/memory_cmd.py @@ -45,8 +45,15 @@ def _resync_memory(cfg): def clear_memory_index(cfg): from tht.adapters.factory import build_vector_store + from tht.cli.vector_cmd import make_embedder, open_store, require_direct_vector_cfg - return build_vector_store(cfg, require_write=True).delete_kinds("memory", ["memory"]) + if cfg.vectors is not None and cfg.vectors.type == "qdrant": + return build_vector_store(cfg, require_write=True).delete_kinds("memory", ["memory"]) + + require_direct_vector_cfg(cfg) + legacy_store = open_store(cfg, "memory") + legacy_store.sync([], make_embedder(cfg.embeddings), kinds={"memory"}) + return 0 @memory_app.command("promote") diff --git a/harness/tht/ports/vector.py b/harness/tht/ports/vector.py index 7aaf5687..e2bdef21 100644 --- a/harness/tht/ports/vector.py +++ b/harness/tht/ports/vector.py @@ -80,8 +80,6 @@ class VectorStore(Protocol): def upsert(self, collection: str, records: list[VectorWriteRecord]) -> int: ... - def delete_kinds(self, collection: str, kinds: list[str]) -> int: ... - def delete_generation(self, collection: str, generation: str, workspace_id: str) -> int: ... def list_evidence_generations(self, collection: str, workspace_id: str) -> list[str]: ... diff --git a/harness/tht/vectorstore/rest_client.py b/harness/tht/vectorstore/rest_client.py index 19edce5f..b94dab79 100644 --- a/harness/tht/vectorstore/rest_client.py +++ b/harness/tht/vectorstore/rest_client.py @@ -150,22 +150,6 @@ class VectorRestClient: return int(payload.get("deleted", 0)) return 0 - def delete_kinds(self, table_name: str, kinds: list[str]) -> int: - try: - payload = self._call( - "delete_vector_kinds", - {"table_name": table_name, "kinds": kinds}, - ) - except VectorRestError as error: - if "HTTP 404" in str(error): - raise VectorRestError( - "delete_vector_kinds RPC is unavailable; deploy the cleanup migration" - ) from None - raise - if isinstance(payload, dict): - return int(payload.get("deleted", 0)) - return 0 - def list_evidence_generations(self, table_name: str, workspace_id: str) -> list[str]: if re.fullmatch(r"[a-z][a-z0-9_-]{0,63}", workspace_id) is None: raise ValueError("workspace namespace must be canonical")