mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(adapter): close final foundation review

Browse Source
This commit is contained in:
marcopan
2026-07-11 21:14:32 +02:00
parent 45d57756aa
commit 3e2ad7955c
15 changed files with 460 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/adapter-final-fix-report.md
+137
View File
@@ -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`.
.superpowers/sdd/progress.md
+14
View File
@@ -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.