138 lines
6.0 KiB
Markdown
138 lines
6.0 KiB
Markdown
# 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`.
|