diff --git a/.superpowers/sdd/adapter-final-fix-report.md b/.superpowers/sdd/adapter-final-fix-report.md new file mode 100644 index 00000000..dfdd4c7e --- /dev/null +++ b/.superpowers/sdd/adapter-final-fix-report.md @@ -0,0 +1,137 @@ +# Adapter Foundations final-review fix report + +Date: 2026-07-11 +Branch: `codex/portable-deployment` +Worktree: `/Users/mp/projects/ThothII/.worktrees/portable-deployment` +Binding findings: `.superpowers/sdd/adapter-final-review-findings.md` + +## Outcome + +All seven final-review findings are addressed as one coherent adapter-foundations change: + +1. HTTP vector reader and writer clients are independently optional. Capabilities reflect the + configured side; writer-only new and legacy configurations build successfully for targeted + writes; search without a reader raises public `VectorReadUnavailable`. +2. `VectorHealth` now reports read/write configured and reachable state independently, preserves + side-specific errors, and reports expected/observed embedding dimensions plus compatibility. + HTTP diagnostics cover read-only, write-only, both-up, and writer-down cases. Direct health + exposes its configured expected dimension without adding schema or migration work. +3. `ThothRestDwhAdapter` accepts `DatabaseIdentityConfig`, matching its resource contract. +4. Both vector adapters reject bools, floats, zero, and negative search limits using one exact + positive-integer guard. +5. Port tests explicitly cover public exports and frozen capability records. +6. A real `tht` subprocess test proves one legacy deprecation warning per config load on stderr + while JSON stdout remains parseable and uncontaminated. +7. The adapter plan and SDD progress explicitly constrain `build_vector_loader` to transitional + bulk sync and schedule its removal/migration in the local pgvector plan. Targeted memory and + solved-question writes remain on `build_vector_store(..., require_write=True)`. + +No pgvector schema or migration changes were made. + +## Files changed + +- `harness/tht/ports/vector.py` +- `harness/tht/ports/__init__.py` +- `harness/tht/adapters/vector/thoth_http.py` +- `harness/tht/adapters/vector/legacy_direct.py` +- `harness/tht/adapters/factory.py` +- `harness/tht/adapters/dwh/thoth_rest.py` +- `harness/tests/test_vector_port_contract.py` +- `harness/tests/test_adapter_factory.py` +- `harness/tests/test_config_resources.py` +- `harness/tests/test_config_legacy_compat.py` +- `harness/tests/test_adapter_command_regressions.py` +- `harness/tests/test_dwh_port_contract.py` +- `docs/superpowers/plans/2026-07-11-adapter-foundations.md` +- `.superpowers/sdd/progress.md` +- `.superpowers/sdd/adapter-final-fix-report.md` + +## TDD and verification evidence + +RED: + +```text +cd harness && .venv/bin/pytest tests/test_vector_port_contract.py \ + tests/test_adapter_factory.py tests/test_config_resources.py \ + tests/test_config_legacy_compat.py -q +``` + +Result: collection failed as expected because `VectorReadUnavailable` did not exist. After the +initial implementation, the same command exposed two expected contract/test-harness corrections: +dimension mismatch makes aggregate health unhealthy, and the installed CLI entry point is `tht` +rather than `python -m tht.cli`. + +GREEN, covering adapter/config/command regressions: + +```text +cd harness && .venv/bin/pytest tests/test_vector_port_contract.py \ + tests/test_adapter_factory.py tests/test_config_resources.py \ + tests/test_config_legacy_compat.py tests/test_adapter_command_regressions.py \ + tests/test_dwh_port_contract.py tests/test_memory_save_one.py \ + tests/test_solved_question.py tests/test_search_similar_kinds.py \ + tests/test_vector_dual_key.py -q +``` + +Result: `66 passed in 0.45s`. + +Docker availability: + +```text +docker info --format '{{.ServerVersion}}' +``` + +Result: `29.4.1` (available; command required Docker socket access). + +Full repository-default non-L2 harness suite, with Docker available for L0 tests: + +```text +cd harness && .venv/bin/pytest -q +``` + +Result: `433 passed, 5 deselected, 17 warnings in 9.14s`. The five deselections are the configured +L2/live-service tests. Warnings are existing legacy-workspace `FutureWarning` emissions. + +Scoped lint and diff hygiene: + +```text +cd harness && .venv/bin/ruff check tht/ports tht/adapters \ + tests/test_vector_port_contract.py tests/test_adapter_factory.py \ + tests/test_config_resources.py tests/test_config_legacy_compat.py \ + tests/test_adapter_command_regressions.py tests/test_dwh_port_contract.py +git diff --check +``` + +Result: `All checks passed!`; `git diff --check` produced no output. + +## Commit + +Commit subject: `fix(adapter): close final foundation review` + +The report is part of that same final commit. A Git object cannot contain its own SHA without +changing that SHA; the exact resulting commit ID is therefore recorded in the task handoff from +`git rev-parse HEAD` after creation. + +## Self-review + +- Reader/writer separation is preserved: search dereferences only `_reader`; hashes/upsert only + `_writer`; health probes each configured client independently and never substitutes one result + for the other. +- Writer failure contributes to aggregate `ok=False`, even when the reader succeeds. +- Dimension compatibility is derived only from configured embedding dimension and existing + `list_tables` metadata. Missing metadata remains `None`, not a guessed success/failure. +- The shared limit guard uses `type(limit) is int`, intentionally rejecting Python booleans and + numeric coercions before either adapter reaches its transport. +- Existing JSON/CLI behavior is preserved; the subprocess regression parses stdout as JSON and + counts exactly one deprecation marker on stderr. +- Scope remains adapter foundations. No vector DDL, schema initialization, or migration work was + introduced. + +## Concerns / follow-up + +- Write reachability uses the existing `list_tables` diagnostic on the separately authenticated + writer client. Deployments must allow that non-mutating diagnostic RPC to the writer credential; + failures are intentionally visible rather than hidden by reader success. +- Existing legacy-workspace tests emit 17 `FutureWarning`s in the full suite. This wave pins the + required production stderr behavior but does not migrate unrelated test fixtures. +- `build_vector_loader` remains transitional technical debt only for bulk sync, explicitly assigned + to `2026-07-11-local-pgvector-profile.md`. diff --git a/.superpowers/sdd/progress.md b/.superpowers/sdd/progress.md new file mode 100644 index 00000000..c6b9d14a --- /dev/null +++ b/.superpowers/sdd/progress.md @@ -0,0 +1,14 @@ +# Portable deployment SDD progress + +Plan: `docs/superpowers/plans/2026-07-11-adapter-foundations.md` +Branch: `codex/portable-deployment` +Worktree: `/Users/mp/projects/ThothII/.worktrees/portable-deployment` + +Task 1: complete (commits e02e61e..a4eb6cc, review clean) +Task 1 final-review follow-up: public exports and frozen capability records now have explicit regressions. +Task 2: complete (commits a4eb6cc..f6302b3, review clean after authorized contract correction) +Task 3: complete (commits f6302b3..fe8d70d, review clean after authorized write-envelope correction) +Task 4: complete (commits fe8d70d..1e0911b, review clean) +Task 4 final-review follow-up: a real `tht` subprocess now proves exactly one legacy warning on stderr and pristine JSON stdout. +Task 5: complete (commits 1e0911b..dbbab6d, review clean after two fix waves) +Final adapter review fix wave: complete (`fix(adapter): close final foundation review`). HTTP vector reader/writer endpoints are independently optional; writer-only targeted memory/solved writes are supported. Vector health reports each side separately plus configured/observed embedding dimensions. `build_vector_loader` remains an explicitly tracked bulk-sync-only exception scheduled for the local pgvector migration plan; it is not used by interactive/targeted writes. diff --git a/docs/superpowers/plans/2026-07-11-adapter-foundations.md b/docs/superpowers/plans/2026-07-11-adapter-foundations.md index b11fc8ea..3de4f6ae 100644 --- a/docs/superpowers/plans/2026-07-11-adapter-foundations.md +++ b/docs/superpowers/plans/2026-07-11-adapter-foundations.md @@ -294,6 +294,13 @@ positive limit, and direct DWH construction injects `cfg.execution.statement_tim Targeted vector writes consume the factory-returned `VectorStore` and pass `VectorWriteRecord` objects to `upsert`. +Transitional exception: `build_vector_loader` remains solely for bulk collection sync +(`vector init`/rebuild/index flows). It may still construct the legacy table-scoped writer +directly until `docs/superpowers/plans/2026-07-11-local-pgvector-profile.md` migrates the +local pgvector/vector schema and bulk-sync path. Interactive and targeted writes +(`memory save-one` and solved-question indexing) are not covered by this exception and must +continue through `build_vector_store(..., require_write=True)` and the public vector port. + - [ ] **Step 1: Write exact factory selection and missing-writer tests** ```python diff --git a/harness/tests/test_adapter_command_regressions.py b/harness/tests/test_adapter_command_regressions.py index ad52c825..21a67802 100644 --- a/harness/tests/test_adapter_command_regressions.py +++ b/harness/tests/test_adapter_command_regressions.py @@ -82,3 +82,35 @@ def test_memory_command_writes_through_factory_vector_store(monkeypatch): memory_cmd.save_one_cmd(session="s1", decision=7, json_out=True) from tht.ports.vector import VectorWriteRecord assert len(captured) == 1 and isinstance(captured[0], VectorWriteRecord) + + +def test_solved_index_writes_through_writer_only_factory_store(monkeypatch): + writer_only_store = SimpleNamespace( + capabilities=SimpleNamespace(search=False, upsert=True), + existing_hashes=lambda *args: {}, + upsert=lambda table, rows: 1, + ) + cfg = SimpleNamespace(embeddings=object(), vector_write_rest=object()) + manifest = SimpleNamespace(id="s1") + solved_record = object() + calls = [] + + monkeypatch.setattr(memory_cmd, "has_vector_write_rest", lambda cfg: True) + monkeypatch.setattr(memory_cmd, "load_session_or_exit", lambda cfg, session: manifest) + monkeypatch.setattr(memory_cmd, "session_dir", lambda *args: None) + monkeypatch.setattr( + "tht.adapters.factory.build_vector_store", + lambda cfg, require_write: calls.append(require_write) or writer_only_store, + ) + monkeypatch.setattr("tht.cli.sql_cmd.promoted_tables_for", lambda *args: []) + monkeypatch.setattr("tht.solved.build_solved_record", lambda *args: solved_record) + monkeypatch.setattr( + "tht.solved.save_solved_question", + lambda record, *, store, embedder: int( + record is solved_record and store is writer_only_store + ), + ) + monkeypatch.setattr("tht.cli.vector_cmd.make_embedder", lambda cfg: object()) + + assert memory_cmd.index_solved_session(cfg, "s1") == 1 + assert calls == [True] diff --git a/harness/tests/test_adapter_factory.py b/harness/tests/test_adapter_factory.py index 52cf75ed..1ef283f2 100644 --- a/harness/tests/test_adapter_factory.py +++ b/harness/tests/test_adapter_factory.py @@ -6,7 +6,7 @@ from tht.adapters.factory import build_dwh, build_vector_store from tht.config import Config, ConfigError -def _config(*, dwh_type="thoth_rest", vector_type="thoth_vector_http", writer=True): +def _config(*, dwh_type="thoth_rest", vector_type="thoth_vector_http", reader=True, writer=True): dwh = ( { "type": "thoth_rest", @@ -28,7 +28,7 @@ def _config(*, dwh_type="thoth_rest", vector_type="thoth_vector_http", writer=Tr vectors = ( { "type": "thoth_vector_http", - "reader": {"base_url": "https://vectors.test/", "api_key": "reader"}, + **({"reader": {"base_url": "https://vectors.test/", "api_key": "reader"}} if reader else {}), **( {"writer": {"base_url": "https://vectors.test/", "api_key": "writer"}} if writer @@ -78,6 +78,15 @@ def test_factory_selects_http_vector_and_requires_writer(): build_vector_store(config, require_write=True) +def test_factory_builds_writer_only_http_vector_when_write_is_required(): + config = _config(reader=False, writer=True) + + store = build_vector_store(config, require_write=True) + assert isinstance(store, ThothHttpVectorStore) + assert store.capabilities.search is False + assert store.capabilities.upsert is True + + def test_factory_selects_direct_vector_reader(): config = _config(vector_type="pgvector_direct") diff --git a/harness/tests/test_config_legacy_compat.py b/harness/tests/test_config_legacy_compat.py index d14da00f..2166ad09 100644 --- a/harness/tests/test_config_legacy_compat.py +++ b/harness/tests/test_config_legacy_compat.py @@ -1,10 +1,14 @@ import json +import os +import subprocess +from pathlib import Path import pytest from typer.testing import CliRunner from tht.cli import app from tht.config import load_config +from tht.adapters.factory import build_vector_store def _write_old_workspace(tmp_path): @@ -103,3 +107,42 @@ def test_legacy_warning_does_not_contaminate_cli_json(tmp_path): assert "DEPRECATION" not in result.stdout assert result.stderr == "" assert len(warnings) == 1 + + +def test_legacy_cli_subprocess_warns_once_on_stderr_and_keeps_json_stdout(tmp_path): + workspace = _write_old_workspace(tmp_path) + result = subprocess.run( + [ + str(Path(__file__).parents[1] / ".venv" / "bin" / "tht"), + "session", + "list", + "--json", + "-c", + str(workspace), + ], + cwd=tmp_path, + env={**os.environ, "PYTHONWARNINGS": "default"}, + text=True, + capture_output=True, + check=False, + ) + + assert result.returncode == 0 + json.loads(result.stdout) + assert "DEPRECATION" not in result.stdout + assert result.stderr.count("DEPRECATION") == 1 + + +def test_legacy_writer_only_vector_config_builds_for_targeted_writes(tmp_path): + workspace = _write_old_workspace(tmp_path) + content = workspace.read_text().replace( + "vector_rest:\n base_url: https://vectors.example.test/\n api_key: vector-reader\n", + "", + ) + workspace.write_text(content) + + with pytest.warns(FutureWarning): + cfg = load_config(workspace) + store = build_vector_store(cfg, require_write=True) + assert store.capabilities.search is False + assert store.capabilities.upsert is True diff --git a/harness/tests/test_config_resources.py b/harness/tests/test_config_resources.py index b606c590..3ee884ef 100644 --- a/harness/tests/test_config_resources.py +++ b/harness/tests/test_config_resources.py @@ -69,3 +69,23 @@ vectors: assert cfg.database.transport == "direct" assert isinstance(cfg.vectors, PgvectorDirectConfig) assert cfg.vector_db.db_schema == "vectors" + + +def test_loads_writer_only_http_vector_resource(tmp_path): + workspace = tmp_path / "workspace.yaml" + workspace.write_text( + """ +dwh: + type: thoth_rest + database: {database: analytics, schema: mart} + endpoint: {base_url: https://dwh.test/, api_key: reader} +vectors: + type: thoth_vector_http + writer: {base_url: https://vectors.test/, api_key: writer} +embeddings: {base_url: http://ollama:11434, dim: 768} +""" + ) + + cfg = load_config(workspace) + assert cfg.vectors.reader is None + assert cfg.vectors.writer.api_key == "writer" diff --git a/harness/tests/test_dwh_port_contract.py b/harness/tests/test_dwh_port_contract.py index 55e7bac9..aad615c7 100644 --- a/harness/tests/test_dwh_port_contract.py +++ b/harness/tests/test_dwh_port_contract.py @@ -55,8 +55,16 @@ def test_contract_types_are_public_and_capabilities_are_immutable(): def test_all_contract_types_are_exported_from_public_package(): + from tht.ports import DwhAdapter as PublicDwhAdapter + from tht.ports import DwhCapabilities as PublicDwhCapabilities + from tht.ports import DwhHealth as PublicDwhHealth from tht.ports import DistinctValues as PublicDistinctValues + from tht.ports import UnsupportedCapability as PublicUnsupportedCapability result = PublicDistinctValues(values=["a"], truncated=True) assert result.values == ["a"] assert result.truncated is True + assert PublicDwhAdapter is DwhAdapter + assert PublicDwhCapabilities is DwhCapabilities + assert PublicDwhHealth is DwhHealth + assert PublicUnsupportedCapability is UnsupportedCapability diff --git a/harness/tests/test_vector_port_contract.py b/harness/tests/test_vector_port_contract.py index 9ab9c519..db97831d 100644 --- a/harness/tests/test_vector_port_contract.py +++ b/harness/tests/test_vector_port_contract.py @@ -1,13 +1,16 @@ +from dataclasses import FrozenInstanceError from unittest.mock import MagicMock import pytest from tht.adapters.vector.thoth_http import ThothHttpVectorStore +from tht.adapters.vector.legacy_direct import LegacyDirectVectorStore from tht.evidence.model import EvidenceDoc from tht.ports.vector import ( VectorHit, VectorRecord, VectorStore, + VectorReadUnavailable, VectorWriteRecord, VectorWriteUnavailable, ) @@ -24,6 +27,25 @@ def test_http_store_reports_reader_without_writer(): store.upsert("memory", []) +def test_http_store_supports_writer_without_reader(): + writer = MagicMock() + store = ThothHttpVectorStore(reader=None, writer=writer, expected_dimension=768) + + assert store.capabilities.search is False + assert store.capabilities.existing_hashes is True + assert store.capabilities.upsert is True + with pytest.raises(VectorReadUnavailable): + store.search(["memory"], [0.1], limit=1) + + +@pytest.mark.parametrize("limit", [True, False, 1.0, 0, -1]) +def test_http_search_requires_a_strict_positive_integer_limit(limit): + store = ThothHttpVectorStore(reader=MagicMock(), writer=None) + + with pytest.raises(ValueError, match="positive integer"): + store.search(["memory"], [0.1], limit=limit) + + def test_http_store_keeps_reader_and_writer_operations_separate(): reader = MagicMock() reader.search_similar.return_value = [ @@ -139,10 +161,19 @@ def test_vector_contract_is_exported_from_public_packages(): from tht.adapters.vector import ThothHttpVectorStore as PublicHttpStore from tht.ports import VectorStore as PublicVectorStore from tht.ports import VectorWriteRecord as PublicVectorWriteRecord + from tht.ports import VectorReadUnavailable as PublicVectorReadUnavailable assert PublicHttpStore is ThothHttpVectorStore assert PublicVectorStore is VectorStore assert PublicVectorWriteRecord is VectorWriteRecord + assert PublicVectorReadUnavailable is VectorReadUnavailable + + capabilities = store_capabilities = ThothHttpVectorStore( + reader=MagicMock(), writer=None + ).capabilities + assert capabilities.search is True + with pytest.raises(FrozenInstanceError): + store_capabilities.search = False def test_http_health_uses_reader_list_tables_and_reports_failure(): @@ -154,3 +185,66 @@ def test_http_health_uses_reader_list_tables_and_reports_failure(): health = store.health() assert health.ok is False assert health.detail == "offline" + + +def test_http_health_reports_read_write_and_dimension_status_independently(): + reader = MagicMock() + reader.list_tables.return_value = [ + {"table_name": "memory", "vector_dimensions": 768} + ] + writer = MagicMock() + writer.list_tables.return_value = [ + {"table_name": "memory", "vector_dimensions": 768} + ] + store = ThothHttpVectorStore(reader, writer, expected_dimension=768) + + health = store.health() + assert health.ok is True + assert health.read_configured is True + assert health.read_reachable is True + assert health.write_configured is True + assert health.write_reachable is True + assert health.expected_dimension == 768 + assert health.observed_dimensions == (768,) + assert health.dimension_compatible is True + + +def test_http_health_does_not_hide_writer_failure_behind_reader_success(): + reader = MagicMock() + reader.list_tables.return_value = [] + writer = MagicMock() + writer.list_tables.side_effect = RuntimeError("writer offline") + store = ThothHttpVectorStore(reader, writer, expected_dimension=768) + + health = store.health() + assert health.ok is False + assert health.read_reachable is True + assert health.write_reachable is False + assert health.write_detail == "writer offline" + assert health.dimension_compatible is None + + +def test_http_health_covers_read_only_and_write_only_configuration(): + reader = MagicMock() + reader.list_tables.return_value = [{"vector_dimensions": 384}] + read_health = ThothHttpVectorStore(reader, None, expected_dimension=768).health() + assert read_health.ok is False + assert read_health.write_configured is False + assert read_health.write_reachable is None + assert read_health.dimension_compatible is False + + writer = MagicMock() + writer.list_tables.return_value = [{"vector_dimensions": 768}] + write_health = ThothHttpVectorStore(None, writer, expected_dimension=768).health() + assert write_health.ok is True + assert write_health.read_configured is False + assert write_health.read_reachable is None + assert write_health.dimension_compatible is True + + +@pytest.mark.parametrize("limit", [True, False, 1.0, 0, -1]) +def test_legacy_direct_search_requires_a_strict_positive_integer_limit(limit): + store = LegacyDirectVectorStore(engine=MagicMock()) + + with pytest.raises(ValueError, match="positive integer"): + store.search(["memory"], [0.1], limit=limit) diff --git a/harness/tht/adapters/dwh/thoth_rest.py b/harness/tht/adapters/dwh/thoth_rest.py index 367ac156..db738dbd 100644 --- a/harness/tht/adapters/dwh/thoth_rest.py +++ b/harness/tht/adapters/dwh/thoth_rest.py @@ -1,6 +1,6 @@ """Thoth/PostgREST implementation of the DWH port.""" -from tht.config import DatabaseConfig, RestConfig +from tht.config import DatabaseIdentityConfig, RestConfig from tht.db.introspect import introspect_rest from tht.db import sampling from tht.execute import ExecResult, ExecutionError, PlanSummary @@ -13,7 +13,7 @@ from tht.rest.execute import explain_rest, run_controlled_rest class ThothRestDwhAdapter: capabilities = DwhCapabilities() - def __init__(self, database: DatabaseConfig, rest: RestConfig): + def __init__(self, database: DatabaseIdentityConfig, rest: RestConfig): self._database = database self._client = RestClient(rest) diff --git a/harness/tht/adapters/factory.py b/harness/tht/adapters/factory.py index a30fdc17..6db43732 100644 --- a/harness/tht/adapters/factory.py +++ b/harness/tht/adapters/factory.py @@ -41,13 +41,12 @@ def build_vector_store(cfg: Config, *, require_write: bool = False) -> VectorSto dim=dim, ) case "thoth_vector_http": - if resource.reader is None: - raise ConfigError("Vector reader non configurato") if require_write and resource.writer is None: raise ConfigError("Vector writer non configurato") return ThothHttpVectorStore( - VectorRestClient(resource.reader), + VectorRestClient(resource.reader) if resource.reader is not None else None, VectorRestClient(resource.writer) if resource.writer is not None else None, + expected_dimension=cfg.embeddings.dim if cfg.embeddings is not None else None, ) case other: # pragma: no cover - Pydantic's discriminator rejects this first. raise ConfigError(f"Adapter vector non supportato: {other}") diff --git a/harness/tht/adapters/vector/legacy_direct.py b/harness/tht/adapters/vector/legacy_direct.py index 8d121ac4..bdec704c 100644 --- a/harness/tht/adapters/vector/legacy_direct.py +++ b/harness/tht/adapters/vector/legacy_direct.py @@ -7,6 +7,7 @@ from tht.ports.vector import ( VectorHealth, VectorWriteRecord, VectorWriteUnavailable, + require_positive_limit, ) from tht.vectorstore.store import VectorHit, VectorStore as TableVectorStore @@ -26,8 +27,20 @@ class LegacyDirectVectorStore: with self._engine.connect() as connection: connection.exec_driver_sql("SELECT 1") except Exception as exc: - return VectorHealth(ok=False, detail=str(exc)) - return VectorHealth(ok=True) + return VectorHealth( + ok=False, + detail=str(exc), + read_configured=True, + read_reachable=False, + read_detail=str(exc), + expected_dimension=self._dim, + ) + return VectorHealth( + ok=True, + read_configured=True, + read_reachable=True, + expected_dimension=self._dim, + ) def search( self, @@ -37,6 +50,7 @@ class LegacyDirectVectorStore: limit: int, kinds: list[str] | None = None, ) -> list[VectorHit]: + require_positive_limit(limit) hits: list[VectorHit] = [] for collection in collections: table = TableVectorStore( diff --git a/harness/tht/adapters/vector/thoth_http.py b/harness/tht/adapters/vector/thoth_http.py index 9ec65c12..5d7d1a02 100644 --- a/harness/tht/adapters/vector/thoth_http.py +++ b/harness/tht/adapters/vector/thoth_http.py @@ -4,8 +4,10 @@ from tht.ports.vector import ( VectorCapabilities, VectorHealth, VectorHit, + VectorReadUnavailable, VectorWriteRecord, VectorWriteUnavailable, + require_positive_limit, ) from tht.vectorstore.rest_client import VectorRestClient from tht.vectorstore.store import hit_from_metadata @@ -18,21 +20,63 @@ def _merge(hits: list[VectorHit], limit: int) -> list[VectorHit]: class ThothHttpVectorStore: """Vector port backed by the existing allowlisted REST RPCs.""" - def __init__(self, reader: VectorRestClient, writer: VectorRestClient | None): + def __init__( + self, + reader: VectorRestClient | None, + writer: VectorRestClient | None, + expected_dimension: int | None = None, + ): self._reader = reader self._writer = writer + self._expected_dimension = expected_dimension @property def capabilities(self) -> VectorCapabilities: writable = self._writer is not None - return VectorCapabilities(search=True, existing_hashes=writable, upsert=writable) + return VectorCapabilities( + search=self._reader is not None, existing_hashes=writable, upsert=writable + ) def health(self) -> VectorHealth: + read_reachable, read_detail, read_tables = self._probe(self._reader) + write_reachable, write_detail, write_tables = self._probe(self._writer) + dimensions = tuple(sorted({ + dimension + for row in [*read_tables, *write_tables] + if type(dimension := row.get("vector_dimensions")) is int + })) + compatible = ( + None + if self._expected_dimension is None or not dimensions + else dimensions == (self._expected_dimension,) + ) + reachable = [ + status for status in (read_reachable, write_reachable) if status is not None + ] + ok = bool(reachable) and all(reachable) and compatible is not False + details = [detail for detail in (read_detail, write_detail) if detail] + return VectorHealth( + ok=ok, + detail="; ".join(details) or None, + read_configured=self._reader is not None, + read_reachable=read_reachable, + read_detail=read_detail, + write_configured=self._writer is not None, + write_reachable=write_reachable, + write_detail=write_detail, + expected_dimension=self._expected_dimension, + observed_dimensions=dimensions, + dimension_compatible=compatible, + ) + + @staticmethod + def _probe(client: VectorRestClient | None) -> tuple[bool | None, str | None, list[dict]]: + if client is None: + return None, None, [] try: - self._reader.list_tables() + return True, None, client.list_tables() except Exception as exc: - return VectorHealth(ok=False, detail=str(exc)) - return VectorHealth(ok=True) + return False, str(exc), [] def search( self, @@ -42,6 +86,9 @@ class ThothHttpVectorStore: limit: int, kinds: list[str] | None = None, ) -> list[VectorHit]: + require_positive_limit(limit) + if self._reader is None: + raise VectorReadUnavailable("Vector reader credential is not configured") hits: list[VectorHit] = [] for collection in collections: rows = self._reader.search_similar(collection, embedding, limit, kinds=kinds) diff --git a/harness/tht/ports/__init__.py b/harness/tht/ports/__init__.py index de7937c7..1bd4e631 100644 --- a/harness/tht/ports/__init__.py +++ b/harness/tht/ports/__init__.py @@ -12,6 +12,7 @@ from tht.ports.vector import ( VectorHealth, VectorHit, VectorRecord, + VectorReadUnavailable, VectorStore, VectorStoreError, VectorWriteRecord, @@ -28,6 +29,7 @@ __all__ = [ "VectorHealth", "VectorHit", "VectorRecord", + "VectorReadUnavailable", "VectorStore", "VectorStoreError", "VectorWriteRecord", diff --git a/harness/tht/ports/vector.py b/harness/tht/ports/vector.py index 06261df9..1b29ea62 100644 --- a/harness/tht/ports/vector.py +++ b/harness/tht/ports/vector.py @@ -18,6 +18,15 @@ class VectorCapabilities: class VectorHealth: ok: bool detail: str | None = None + read_configured: bool = False + read_reachable: bool | None = None + read_detail: str | None = None + write_configured: bool = False + write_reachable: bool | None = None + write_detail: str | None = None + expected_dimension: int | None = None + observed_dimensions: tuple[int, ...] = () + dimension_compatible: bool | None = None @dataclass(frozen=True) @@ -37,6 +46,16 @@ class VectorWriteUnavailable(VectorStoreError): """Raised when a deployment has no vector writer credential.""" +class VectorReadUnavailable(VectorStoreError): + """Raised when a deployment has no vector reader credential.""" + + +def require_positive_limit(limit: int) -> None: + """Reject coercible values: vector limits are exact positive integers.""" + if type(limit) is not int or limit <= 0: + raise ValueError("Vector search limit must be a positive integer") + + @runtime_checkable class VectorStore(Protocol): @property @@ -63,6 +82,7 @@ __all__ = [ "VectorHealth", "VectorHit", "VectorRecord", + "VectorReadUnavailable", "VectorStore", "VectorStoreError", "VectorWriteRecord",