fix(evidence): resolve final review findings

This commit is contained in:
2026-08-25 03:03:11 +02:00
parent d4818c8cc3
commit e9c65ef2db
10 changed files with 686 additions and 20 deletions
@@ -0,0 +1,175 @@
# Final-review fix report — Evidence #43–#46
Date: 2026-08-25
Base ThothII revision: `d4818c8cc33b8b11204377ff3cc65c6c9ee1425e`
Binding inputs:
- requirements: `docs/plans/2026-08-24-evidence-restructuring.md`;
- approved design: `docs/plans/2026-08-24-evidence-restructuring-design.md`;
- final review: `.superpowers/sdd/2026-08-24-evidence-restructuring/final-review.md`.
No PSD path was read or mutated during this fix wave. No issue was closed.
## Verdict
All three Important findings are fixed. Candidate evaluation remains mandatory for schema-v2
filesystem curated corpora and is absent for legacy v1, HTTP-only, and S3-only acquisition.
Reviewed formula migration now fails closed unless the caller supplies the exact original source,
the source parses back to the same formula, and every supporting excerpt is present after the
canonical mechanical normalization. Current owner-gate summaries explicitly distinguish the
immutable 225-test owner-gate observation from the expanded 230-test final-review verification and
place stale 222/224-era statements under an explicitly superseded historical section.
## RED/GREEN record
### Finding 1 — evaluator compatibility boundary
RED:
```text
cd harness
.venv/bin/pytest -q tests/test_preprocess_cli.py tests/test_evidence_formula_migration.py \
tests/test_evidence_restructuring_fixture.py \
-k 'candidate_evaluation_is_not_required or runtime_identity'
6 failed, 18 deselected, 1 warning
```
The v1 filesystem run still received a callable evaluator; v1 HTTP/S3 and v2 HTTP/S3 attempted to
resolve a filesystem evaluation root before the pipeline could run.
GREEN:
```text
6 passed, 9 deselected, 1 warning
```
`_requires_candidate_evaluation()` is now the single boundary used by both curated-corpus
validation and evaluator construction. It returns true only for `schema_version: 2` with a
filesystem source (including an explicit filesystem `source_root`). The existing integrated v2
candidate test remains the positive proof: it validates the corpus, performs all nine
candidate-generation searches while inactive, publishes only after PASS, and compensates a failed
candidate.
### Finding 2 — formula provenance
RED:
```text
cd harness
.venv/bin/pytest -q tests/test_evidence_formula_migration.py \
tests/test_evidence_restructuring_fixture.py
5 failed, 4 passed, 1 warning
```
The converter rejected the new `source_content` argument, returned clean Curated Evidence when no
original source was supplied, and the documentation consistency assertion still failed.
GREEN:
```text
cd harness
.venv/bin/pytest -q tests/test_evidence_formula_migration.py tests/test_formula.py \
tests/test_formula_wiring.py tests/test_preprocess_cli.py \
tests/test_evidence_candidate_publication.py
39 passed, 1 warning
```
The migration now:
1. returns `None` unchanged for `auto` and `draft` formulas;
2. requires original source content for a reviewed formula;
3. normalizes it with the authoring validator's `normalize_source_text()`;
4. parses it and requires equality with the supplied `ConceptFormula`;
5. requires one or more legacy provenance notes and verifies each note in normalized source text;
6. hashes the verified normalized original source, never `formula.dump()`;
7. returns `LegacyFormulaMigrationFailure(code="legacy_formula_requires_manual_review")` with
bounded problem codes for missing, invalid, mismatched, or excerpt-incomplete provenance.
The regression matrix covers exact original formatting, deterministic digest, absent original
source, absent excerpts, a YAML-folded excerpt that cannot validate literally, mismatched original
formula content, invalid full-query Formula Evidence, stable path-derived IDs, and unreviewed
session-only behavior.
### Finding 3 — current owner-gate record
RED:
```text
cd harness
.venv/bin/pytest -q tests/test_evidence_restructuring_fixture.py \
-k current_summaries
1 failed, 3 deselected, 1 warning
```
The current report did not contain the fresh expanded-suite result. Earlier RED also showed that
the leading report still lacked an authoritative read-only PSD summary and `PROJECT_STATE.md`
still said 222.
GREEN:
```text
4 passed, 1 warning
```
The task report now begins with one authoritative current record and moves all earlier scope/count
statements below `Historical record — explicitly superseded`. The manual and `PROJECT_STATE.md`
record both immutable observations without conflating them: 225 passed at the reviewed `d4818c8`
owner gate; 230 passed after five final-review regressions entered the selected acceptance files.
Both summaries describe the authorized PSD package as read-only, with 35 moveable sources plus the
retained README, narrow no-payload/no-vector observations, no secrets, and no mutation. Issue #47
still owns migration and manual acceptance.
## Requirement mapping
| Requirement | Implementation evidence | Verification |
| --- | --- | --- |
| v1 filesystem remains viable without `evaluation.yaml` or a new curated tree | `_requires_candidate_evaluation()` returns false for schema v1; `run_from_config()` passes `None` | v1 runtime identity test and parameterized filesystem case |
| HTTP/S3 preserve existing behavior | evaluator construction returns `None` for HTTP-only/S3-only configs in schema v1 and v2 | four parameterized HTTP/S3 cases |
| v2 filesystem evaluation remains mandatory | shared predicate drives validation and evaluator; no permissive missing-fixture path was added | integrated inactive-candidate publication/failure-compensation test |
| formula digest represents verified normalized original | caller supplies `source_content`; authoring normalization computes digest | literal SHA-256 assertion from independently normalized original fixture |
| formula and source cannot silently mismatch | original source is parsed and compared with the supplied model | `original_source_mismatch` regression |
| supporting excerpts are real and nonempty | notes are required and each normalized note must occur in normalized source | missing and folded/unverifiable excerpt regressions |
| unverifiable migration requires human resolution | every provenance failure returns the existing bounded manual-review result | exact code/problem assertions preserve original path and formula |
| current acceptance record is unambiguous | authoritative current sections plus explicit historical supersession | owner-gate document consistency test |
| PSD scope remains read-only | no PSD tool/path used in fix; current docs retain issue #47 authorization gate | diff review and acceptance isolation probe |
## Verification evidence
```text
focused Evidence compatibility/formula/candidate: 39 passed, 1 warning
owner-gate document contract: 4 passed, 1 warning
harness full: 1093 passed, 4 deselected, 53 warnings
harness Ruff: All checks passed!
acceptance runner: 230 passed, 1 warning; PASS
backend TypeScript: PASS
native tht build: PASS
git diff --check: PASS
```
The full backend Vitest gate remains outside this patch's modified files and reported 13 failures:
the ten previously recorded auth-runtime projection failures, the recorded Node 25 versus Node 24
contract failure, the recorded Argon2 401/429 timing assertion, plus one Windows timeout-helper
marker failure that is timing-sensitive and was not part of the prior stable 12-failure baseline.
The native Go suite with `-timeout 20s` reproduced the recorded authconfig timeout and backup/setup
failures. These unrelated failures were not repaired or hidden; backend TypeScript and native build
both pass.
## SHA-256 artifact hashes
```text
193f9e83634c17160dc5f589ee392f6cefbca2e24efa0770181500e7dce008a7 PROJECT_STATE.md
4f9cfb213fdfbab481fcad9eeb0002ab0bd69d83d7ef0c459c5fccd5f5b44048 docs/testing/evidence-restructuring-manual.md
e47862d4bf9b846f059ef133b77a6311fbeb297671843bfeda0d0445eb2e4933 harness/tht/cli/preprocess_cmd.py
4017f66dc6d3aeaa4f0294bbca581b8d447e3b60bd8daff4efff08048dc43d4b harness/tht/evidence/formula_store.py
35bd73f6d0759cf0c8cb15a9125cc97b5dda64eb02417eda9dd1a9c82ed8a558 harness/tests/test_preprocess_cli.py
a0affbf9213c41c95d5fea8e596ef37ae21836e0df9bdfa8f6adda95d9238fb6 harness/tests/test_evidence_formula_migration.py
6ed7d7679c860c6edc54cbcfc2a04db89c7d32ecab80c7daa72be96cdc14f164 harness/tests/test_formula.py
26d886074244be7cf6ca084f859dcbc0c0ade74c302077a506198a2e96bfc07e harness/tests/test_evidence_restructuring_fixture.py
62743e885230823f8942d14feadae511b6d09b5d391807a04d10a164f71dd919 .superpowers/sdd/2026-08-24-evidence-restructuring/task-12-report.md
e753674bb55d4880448a51e7fbdb3ccf506800ecc5fd3b73f59ed655cf9b63c7 tracked working-tree diff before this report
```
The final commit hash is reported with the completed handoff because a commit cannot include its
own hash without changing itself.
@@ -0,0 +1,276 @@
# Task 12 report — owner migration gate (#46)
## Current owner-gate record
This section supersedes every historical section below. The owner-gate run at `d4818c8` observed
**225 passed, 1 warning**. The final-review fix added five regressions to the selected files; its
fresh run of `bash scripts/evidence-restructuring-acceptance.sh` observed **230 passed, 1 warning**.
Both runs remained hermetic: they used only an isolated temporary authoring workspace and did not
receive or mutate a PSD path.
The authorized read-only PSD owner-gate package inspected the clean repository at immutable
commit `47516f85b4db4a67cfa8a86cea4cb2e7b98c5813`. It recorded 35 moveable source documents plus
the retained `psd-clinical/evidence/README.md` (36 current Evidence files total), and the narrow
legacy Qdrant baseline: 163 `schema_table`, 2,275 `schema_column`, 2 `memory`, and 1
`solved_question`. The replacement observations used exact filtered counts and at most three IDs
per kind with `with_payload:false` and `with_vector:false`. PSD Git status remained clean; no
secret was read and no external mutation, branch, migration, activation, commit, or push occurred.
The real Pi call, PSD migration, human Git review, authoritative pre/post vector inspection,
activation, and complete manual walkthrough remain pending in issue #47 until the owner
authorizes the migration window and exact target branch. The current package is read-only
preparation, not migration or manual acceptance.
## Historical record — explicitly superseded
Everything below preserves the sequence of earlier Task 12 runs for audit history only. Counts,
scope statements, and pending-inventory wording below are not current; the current owner-gate
record above and `docs/testing/evidence-restructuring-manual.md` are authoritative.
### Initial scope and boundary
Implemented only ThothII-local artifacts:
- `harness/tests/fixtures/evidence_authoring/poorly_structured.md` supplies prose, a
rough list, enum values, URL, ambiguity, and a PostgreSQL-expression candidate.
- `scripts/evidence-restructuring-acceptance.sh` uses a fake restructurer and an
isolated `mktemp` authoring workspace. It never receives, discovers, or accesses a
PSD path; it also asserts that the ThothII worktree status is unchanged.
- `docs/testing/evidence-restructuring-manual.md` is the owner-gate package and manual
procedure. It intentionally leaves the PSD 36-path inventory, PSD commit, and
before-counts pending authorization.
- `PROJECT_STATE.md` records only the observed hermetic result and says that issue #47
is pending.
No external PSD authoring repository was accessed, modified, staged, migrated, or
inspected. No GitHub issue was closed.
### Initial TDD record
RED:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_restructuring_fixture.py
FAILED: FileNotFoundError for fixtures/evidence_authoring/poorly_structured.md
```
GREEN:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_restructuring_fixture.py
1 passed
cd harness && .venv/bin/ruff check .
All checks passed!
```
### Initial hermetic acceptance evidence
```text
bash scripts/evidence-restructuring-acceptance.sh
PASS hermetic fake-restructurer: split, review, Git recovery/diff, no-op, dirty, upgrade, orphan
PASS isolation: only a temporary workspace was supplied; no external PSD path was read or written
222 passed, 1 warning
PASS evidence restructuring automated acceptance
```
The runner directly proves typed splitting; visible unresolved-review and orphan
validation failures; one-source manifest membership; recovery of the committed curated
baseline and a visible Git diff; unchanged no-op; dirty-state refusal; no-write
pipeline mismatch refusal and explicit all-source upgrade. Its selected suites cover
all eight typed kinds, no-tool/no-session Pi invocation, formula acceptance/rejection,
atomic semantic chunking and the 4,000-character policy, candidate evaluation and
inactive-generation failure behavior, deterministic query rendering, hybrid branch and
fused ranks, fail-closed search, and the pinned Qdrant Italian-BM25 L0 contract.
### Initial required local gates
| Gate | Result |
| --- | --- |
| `harness/.venv/bin/pytest -q` | PASS — 1080 passed, 4 deselected, 53 warnings |
| `harness/.venv/bin/ruff check .` | PASS |
| `backend/npx tsc --noEmit -p .` | PASS |
| `backend/npx vitest run` | FAIL — pre-existing failures listed below |
| `tools/tht/go build ./cmd/tht` | PASS |
| `tools/tht/go test ./...` | FAIL / timeout — pre-existing failures listed below |
| `bash scripts/evidence-restructuring-acceptance.sh` | PASS — 222 passed, 1 warning |
#### Baseline proof for unrelated failures
The immutable pre-task source `0caa747` was checked out to a temporary detached
worktree. Its targeted backend run reproduced the same 12 stable failures:
- all ten failures in `test/auth-runtime-projection.test.ts` (`authentication runtime
projection is invalid`);
- `test/health.test.ts` expects Node 24 but the host runs Node 25;
- `test/auth-routes-local.test.ts` expects the third Argon2 request to receive 429 but
receives 401.
The native host suite was run first unbounded and remained silent for more than seven
minutes, then was rerun with `go test ./... -timeout 20s`. Both the task worktree and
the detached `0caa747` baseline reproduce the same failures:
- `internal/authconfig: TestRunProjectedMutationHoldsOuterLockAcrossCanonicalAndProjection`
times out;
- `internal/backup` restore/auth publication assertions fail;
- `internal/setup` projected-server/root assertions fail.
These packages and backend files were not changed by Task 12. The suite failures are
therefore recorded as pre-existing concerns and were not repaired or hidden.
### Initial pending owner decision
Focused ThothII commit: `fc83d29b5836b4db173e0874689426ecf2da526f`
(`test(evidence): record restructuring acceptance`). Issue #46 is ready for the owner
migration authorization gate. Issue #47 remains pending: the owner must authorize the
migration window and exact PSD branch before any external repository inspection,
36-file inventory, migration, validation, activation, or manual PSD acceptance work
begins.
### Fix round 1 evidence (Task 12 review)
#### TDD
RED:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_candidate_publication.py tests/test_evidence_restructuring_fixture.py
FAILED: acceptance runner did not include test_evidence_candidate_publication.py
```
The first integrated-test draft also exposed that the real configuration refuses a
non-1024 embedding dimension; the fake was corrected to model the production contract
instead of bypassing configuration loading.
GREEN:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_candidate_publication.py tests/test_evidence_restructuring_fixture.py
3 passed, 1 warning
cd harness && .venv/bin/ruff check tests/test_evidence_candidate_publication.py tests/test_evidence_restructuring_fixture.py
All checks passed!
bash scripts/evidence-restructuring-acceptance.sh
224 passed, 1 warning
```
The new integrated test invokes the real v2 canonical-corpus validator, the real
`preprocess_cmd._candidate_evaluator`, and `CorpusPipeline`, with an observable store,
vector writer, and searcher. It records nine exact-generation branch searches (dense,
BM25, fused for lexical, semantic, and mixed queries), asserts that the candidate is
not ACTIVE during any search, publishes only after the PASS report, then proves a
failed candidate remains inactive and is compensated. It also asserts the 4,000 default,
rendered fragment bound, and rejection/absence of parallel vector-size aliases.
The acceptance probe now creates a real temporary Git repository, commits its initial
and proposed corpus, makes `evidence/curated` genuinely dirty, and calls
`prepare_workspace_evidence` with the default Git-status detection path. It restores
the temporary file before continuing; no injected `git_status` is used by the dirty
case, no-op, pipeline mismatch, or explicit upgrade checks.
#### Authorized read-only PSD preparation
The PSD repository was inspected read-only at immutable commit
`47516f85b4db4a67cfa8a86cea4cb2e7b98c5813`. `git status --porcelain` was empty before
and after, `git diff --quiet` succeeded after, and no PSD secret, worktree content
beyond the listed source names, or mutation command was used. The exact 36 paths,
concrete rollback command, and Qdrant baseline are now in
`docs/testing/evidence-restructuring-manual.md`.
Read-only Qdrant observation for `127.0.0.1:6333`, collection `psd-clinical`:
`schema_table=163`, `schema_column=2275`, `memory=2`, `solved_question=1`; the manual
records representative IDs. The standard `tht workspace vector inspect --json` command
was attempted, but its temporary maintenance container stopped before Qdrant access
because production auth configuration was unavailable. No secret was read to bypass
that guard; the owner-gate package explicitly requires repeating the contract command
immediately before authorized preprocessing.
Fresh no-write verification after the inspection:
```text
status_lines=0 diff_quiet_exit=0
head=47516f85b4db4a67cfa8a86cea4cb2e7b98c5813
```
Fresh required-gate evidence for this fix round:
```text
harness pytest: 1082 passed, 4 deselected, 53 warnings
harness ruff: PASS
acceptance runner: 224 passed, 1 warning
backend tsc: PASS
backend vitest: 12 failures, exactly reproduced at baseline 0caa747
Go build: PASS
Go test -timeout 20s: authconfig timeout plus backup/setup failures, exactly reproduced at baseline 0caa747
```
Fix-round commit: `8542124f2737354b7ae35973933fc38ca85a31c1`
(`test(evidence): harden owner gate acceptance`).
### Fix round 2 evidence (Task 12 re-review)
#### TDD
RED:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_restructuring_fixture.py
1 failed, 2 passed, 1 warning
```
The new manual-contract test failed because the package still described 36 source
documents and retained the old broad Qdrant request.
GREEN: the package now distinguishes exactly 35 moveable source documents from the
retained `psd-clinical/evidence/README.md` (36 current evidence files total). The
authorized migration instruction moves exactly the 35 listed documents to
`evidence/source/` and retains the README at its current path; the owner-only rollback
restores the immutable pre-migration SHA.
#### Narrow replacement Qdrant baseline
The prior `with_payload:true` full-collection scroll was out-of-scope and is withdrawn
as evidence. It is not a permitted fallback, and this report makes no claim that it
did not materialize payloads.
On 2026-08-25, the replacement baseline sent, for each of `schema_table`,
`schema_column`, `memory`, and `solved_question`:
```text
POST /collections/psd-clinical/points/count
{"filter":{"must":[{"key":"record_kind","match":{"value":"<kind>"}}]},"exact":true}
POST /collections/psd-clinical/points/scroll
{"filter":{"must":[{"key":"record_kind","match":{"value":"<kind>"}}]},"limit":3,"with_payload":false,"with_vector":false}
```
The resulting no-payload/no-vector observations were:
```text
schema_table=163: 01bc2535-24d6-5722-a58b-64a122b90b36,
024ddf60-c80d-5223-ac06-9247e7de7027, 086e00c8-b4a9-5063-b82d-e5a67add29cd
schema_column=2275: 00126cc1-7564-521a-a084-c2d670263258,
00365200-2c55-5bb4-86bb-87dd2d1bb529, 004304bc-b543-5b2d-8b40-f18da8e82af7
memory=2: 8d5cd772-563a-5e22-b764-2ca76cf6efca,
db74457a-3de8-5b95-9a31-d28a1ecf8141
solved_question=1: 2b6bb7d2-1a35-5f49-bd8d-b0cdcb98459a
```
No secret was read, no PSD mutation/stage/branch/commit/migration/activation/push command
was invoked, and the PSD repository remained at
`47516f85b4db4a67cfa8a86cea4cb2e7b98c5813` with empty porcelain status and a successful
`git diff --quiet` after the replacement queries. The standard `tht workspace vector
inspect --json` attempt remains blocked by missing production auth and must be repeated
successfully immediately before any owner-authorized preprocessing.
Focused correction gates:
```text
cd harness && .venv/bin/pytest -q tests/test_evidence_restructuring_fixture.py
3 passed, 1 warning
cd harness && .venv/bin/ruff check tests/test_evidence_restructuring_fixture.py
All checks passed!
bash scripts/evidence-restructuring-acceptance.sh
225 passed, 1 warning
```
Fix-round commit: `d4818c8cc33b8b11204377ff3cc65c6c9ee1425e`
(`docs(evidence): narrow owner gate baseline`).