From e9c65ef2dbd66379e51c5cc84fb6fa042ae5ebbd Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 25 Aug 2026 03:03:11 +0200 Subject: [PATCH] fix(evidence): resolve final review findings --- .../final-fix-report.md | 175 +++++++++++ .../task-12-report.md | 276 ++++++++++++++++++ PROJECT_STATE.md | 9 +- docs/testing/evidence-restructuring-manual.md | 8 + .../tests/test_evidence_formula_migration.py | 121 +++++++- .../test_evidence_restructuring_fixture.py | 19 ++ harness/tests/test_formula.py | 18 +- harness/tests/test_preprocess_cli.py | 26 +- harness/tht/cli/preprocess_cmd.py | 15 +- harness/tht/evidence/formula_store.py | 39 ++- 10 files changed, 686 insertions(+), 20 deletions(-) create mode 100644 .superpowers/sdd/2026-08-24-evidence-restructuring/final-fix-report.md create mode 100644 .superpowers/sdd/2026-08-24-evidence-restructuring/task-12-report.md diff --git a/.superpowers/sdd/2026-08-24-evidence-restructuring/final-fix-report.md b/.superpowers/sdd/2026-08-24-evidence-restructuring/final-fix-report.md new file mode 100644 index 00000000..49f19fb4 --- /dev/null +++ b/.superpowers/sdd/2026-08-24-evidence-restructuring/final-fix-report.md @@ -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. diff --git a/.superpowers/sdd/2026-08-24-evidence-restructuring/task-12-report.md b/.superpowers/sdd/2026-08-24-evidence-restructuring/task-12-report.md new file mode 100644 index 00000000..d488609e --- /dev/null +++ b/.superpowers/sdd/2026-08-24-evidence-restructuring/task-12-report.md @@ -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":""}}]},"exact":true} + +POST /collections/psd-clinical/points/scroll +{"filter":{"must":[{"key":"record_kind","match":{"value":""}}]},"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`). diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 83a592d9..6d281f35 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -3,9 +3,12 @@ ## Evidence restructuring owner gate (#46) — automated acceptance observed; PSD migration pending (#47) (2026-08-25) - **Observed local automation:** `bash scripts/evidence-restructuring-acceptance.sh` completed its - isolated fake-restructurer probe and the selected Evidence/L0 contract suite: **222 passed, 1 - known pytest deprecation warning**. The probe used only a `mktemp` workspace, confirmed the - ThothII worktree status was unchanged, and did not read or write an external PSD authoring path. + isolated fake-restructurer probe and the selected Evidence/L0 contract suite. The owner-gate + run at `d4818c8` observed **225 passed, 1 known pytest deprecation warning**. The final-review + fix added five regressions to the selected files and its fresh run observed + **230 passed, 1 known pytest deprecation warning**. Both probes used only a `mktemp` workspace, + confirmed the ThothII worktree status was unchanged, and did not receive or mutate an external + PSD authoring path. The separately authorized PSD preparation was read-only as recorded below. - **Read-only owner-gate package:** authorized inspection of the clean PSD repository at `47516f85b4db4a67cfa8a86cea4cb2e7b98c5813` recorded 35 moveable source documents plus the retained `evidence/README.md` (36 current evidence files total), and the legacy `psd-clinical` diff --git a/docs/testing/evidence-restructuring-manual.md b/docs/testing/evidence-restructuring-manual.md index 54ec3368..632909df 100644 --- a/docs/testing/evidence-restructuring-manual.md +++ b/docs/testing/evidence-restructuring-manual.md @@ -5,6 +5,11 @@ occur only after the owner authorizes a migration window and exact PSD target br The automated runner is hermetic: it uses a fake restructurer and temporary inputs. The owner-gate inventory below is a separately authorized, read-only PSD snapshot; it did not write, stage, branch, commit, migrate, activate, push, or read secrets. +The owner-gate run at `d4818c8` observed **225 passed, 1 known pytest deprecation warning**. +The final-review fix added five regressions to the selected files and its fresh hermetic run +observed **230 passed, 1 known pytest deprecation warning**. This read-only package and those +immutable results are the current issue #46 record; the authorized migration and manual +acceptance remain pending in issue #47. ## Recorded automated boundary @@ -22,6 +27,9 @@ formula, chunking, candidate-evaluation, hybrid-query/fail-closed, and pinned-Qd L0 suites. The runner supplies only a `mktemp` workspace and asserts the ThothII worktree is unchanged; consequently it performs no external PSD write. +Recorded owner-gate output: `225 passed, 1 warning`. Fresh final-review fix output: +`230 passed, 1 warning`. Both were followed by `PASS evidence restructuring automated acceptance`. + The real Pi invocation is deliberately not automated here. A reviewer must run it once per changed source after the authorization gate and examine every proposed curated file. diff --git a/harness/tests/test_evidence_formula_migration.py b/harness/tests/test_evidence_formula_migration.py index 743498df..a018c663 100644 --- a/harness/tests/test_evidence_formula_migration.py +++ b/harness/tests/test_evidence_formula_migration.py @@ -1,6 +1,10 @@ """Migration boundary: legacy formulas become curated evidence or session proposals.""" +import hashlib + from tht.evidence import formula_store +from tht.evidence.authoring import normalize_source_text +from tht.evidence.canonical import CuratedEvidence from tht.evidence.formula_store import ConceptFormula @@ -13,17 +17,124 @@ def test_reviewed_formula_migration_has_deterministic_provenance_hash(): sources=["Regola clinica approvata dal gruppo pediatrico."], ) + original_source = """--- +concept: fascia pediatrica +columns: [clinical.patient.birth_date] +status: reviewed +sources: + - Regola clinica approvata dal gruppo pediatrico. +--- +CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END +""" + first = formula_store.legacy_formula_to_curated( - formula, legacy_path="formulas/fascia-pediatrica-1.sql.md", + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=original_source, ) second = formula_store.legacy_formula_to_curated( + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=original_source, + ) + + assert isinstance(first, CuratedEvidence) + assert isinstance(second, CuratedEvidence) + assert first.provenance.source_sha256 == second.provenance.source_sha256 + expected = hashlib.sha256(normalize_source_text(original_source).encode("utf-8")).hexdigest() + assert first.provenance.source_sha256 == f"sha256:{expected}" + + +def test_reviewed_formula_without_original_source_fails_closed(): + formula = ConceptFormula( + concept="fascia pediatrica", + columns=["clinical.patient.birth_date"], + sql="CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END", + status="reviewed", + sources=["Regola clinica approvata dal gruppo pediatrico."], + ) + + outcome = formula_store.legacy_formula_to_curated( formula, legacy_path="formulas/fascia-pediatrica-1.sql.md", ) - assert first is not None - assert second is not None - assert first.provenance.source_sha256 == second.provenance.source_sha256 - assert first.provenance.source_sha256.startswith("sha256:") + assert outcome.code == "legacy_formula_requires_manual_review" + assert outcome.problems == ("original_source_required",) + + +def test_reviewed_formula_without_verified_supporting_excerpts_fails_closed(): + formula = ConceptFormula( + concept="fascia pediatrica", + columns=["clinical.patient.birth_date"], + sql="CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END", + status="reviewed", + sources=["Nota non presente nel sorgente originale."], + ) + original_source = """--- +concept: fascia pediatrica +columns: [clinical.patient.birth_date] +status: reviewed +sources: + - >- + Nota non presente nel + sorgente originale. +--- +CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END +""" + + outcome = formula_store.legacy_formula_to_curated( + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=original_source, + ) + + assert outcome.code == "legacy_formula_requires_manual_review" + assert outcome.problems == ("supporting_excerpt_unverified",) + + +def test_reviewed_formula_without_provenance_notes_fails_closed(): + formula = ConceptFormula( + concept="fascia pediatrica", + columns=["clinical.patient.birth_date"], + sql="CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END", + status="reviewed", + sources=[], + ) + + outcome = formula_store.legacy_formula_to_curated( + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=formula.dump(), + ) + + assert outcome.code == "legacy_formula_requires_manual_review" + assert outcome.problems == ("supporting_excerpts_required",) + + +def test_reviewed_formula_must_match_the_original_source_record(): + formula = ConceptFormula( + concept="fascia pediatrica", + columns=["clinical.patient.birth_date"], + sql="CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END", + status="reviewed", + sources=["Regola clinica approvata dal gruppo pediatrico."], + ) + different_source = ConceptFormula( + concept=formula.concept, + columns=formula.columns, + sql="CASE WHEN age < 16 THEN 'pediatrica' ELSE 'adulta' END", + status=formula.status, + sources=formula.sources, + ).dump() + + outcome = formula_store.legacy_formula_to_curated( + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=different_source, + ) + + assert outcome.code == "legacy_formula_requires_manual_review" + assert outcome.problems == ("original_source_mismatch",) def test_incompatible_reviewed_legacy_formula_fails_closed_with_the_original_record(): diff --git a/harness/tests/test_evidence_restructuring_fixture.py b/harness/tests/test_evidence_restructuring_fixture.py index 052f7f4d..a49058f5 100644 --- a/harness/tests/test_evidence_restructuring_fixture.py +++ b/harness/tests/test_evidence_restructuring_fixture.py @@ -38,3 +38,22 @@ def test_owner_gate_manual_records_the_narrow_inventory_and_qdrant_baseline_prot assert "with_payload:false" in text assert "with_payload:true" not in text assert "must be repeated through the successful `vector inspect` command" in normalized + + +def test_owner_gate_current_summaries_supersede_stale_pre_inspection_history(): + root = Path(__file__).parents[2] + report = ( + root / ".superpowers" / "sdd" / "2026-08-24-evidence-restructuring" + / "task-12-report.md" + ).read_text(encoding="utf-8") + state = (root / "PROJECT_STATE.md").read_text(encoding="utf-8") + + current_report = report.split("## Historical record", 1)[0] + current_state = state.split("## Modular workflow refactor candidate", 1)[0] + + assert "225 passed, 1 warning" in current_report + assert "230 passed, 1 warning" in current_report + assert "authorized read-only PSD" in current_report + assert "35 moveable source documents" in current_report + assert "**225 passed, 1 known pytest deprecation warning**" in current_state + assert "**230 passed, 1 known pytest deprecation warning**" in current_state diff --git a/harness/tests/test_formula.py b/harness/tests/test_formula.py index 3db2f518..de40d8f8 100644 --- a/harness/tests/test_formula.py +++ b/harness/tests/test_formula.py @@ -100,7 +100,9 @@ def test_reviewed_legacy_formula_becomes_curated_formula_with_stable_provenance( ) migrated = formula_store.legacy_formula_to_curated( - formula, legacy_path="formulas/fascia-pediatrica-1.sql.md", + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=formula.dump(), ) assert migrated is not None @@ -114,7 +116,9 @@ def test_reviewed_legacy_formula_becomes_curated_formula_with_stable_provenance( assert migrated.provenance.supporting_excerpts == tuple(formula.sources) assert migrated.review_items == () assert formula_store.legacy_formula_to_curated( - formula, legacy_path="formulas/fascia-pediatrica-1.sql.md", + formula, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=formula.dump(), ).id == migrated.id @@ -124,19 +128,25 @@ def test_reviewed_legacy_formulas_with_the_same_concept_keep_distinct_path_ident columns=["clinical.patient.birth_date"], sql="CASE WHEN age < 18 THEN 'pediatrica' ELSE 'adulta' END", status="reviewed", + sources=["Regola legacy revisionata."], ) second = ConceptFormula( concept="fascia pediatrica", columns=["clinical.patient.birth_date"], sql="CASE WHEN age < 16 THEN 'pediatrica' ELSE 'adulta' END", status="reviewed", + sources=["Regola legacy revisionata."], ) first_migration = formula_store.legacy_formula_to_curated( - first, legacy_path="formulas/fascia-pediatrica-1.sql.md", + first, + legacy_path="formulas/fascia-pediatrica-1.sql.md", + source_content=first.dump(), ) second_migration = formula_store.legacy_formula_to_curated( - second, legacy_path="formulas/fascia-pediatrica-2.sql.md", + second, + legacy_path="formulas/fascia-pediatrica-2.sql.md", + source_content=second.dump(), ) assert first_migration is not None diff --git a/harness/tests/test_preprocess_cli.py b/harness/tests/test_preprocess_cli.py index 9ac21bea..e0ec24ba 100644 --- a/harness/tests/test_preprocess_cli.py +++ b/harness/tests/test_preprocess_cli.py @@ -315,11 +315,35 @@ def test_run_from_config_uses_runtime_identity_workspace_id(monkeypatch, tmp_pat command.run_from_config(config) assert calls["init"]["sparse_language"] == "english" - assert callable(calls["init"]["candidate_evaluator"]) + assert calls["init"]["candidate_evaluator"] is None assert calls["run_as_job"]["workspace_id"] == "psd-clinical" assert calls["run_as_job"]["input_fingerprint"] != calls["run_as_job"]["config_fingerprint"] +@pytest.mark.parametrize("schema_version, source_type", [ + (1, "filesystem"), + (1, "http"), + (1, "s3"), + (2, "http"), + (2, "s3"), +]) +def test_candidate_evaluation_is_not_required_outside_v2_filesystem_corpora( + schema_version, source_type, +): + import tht.cli.preprocess_cmd as command + + cfg = SimpleNamespace( + evidence=SimpleNamespace( + schema_version=schema_version, + source_root=None, + sources=[SimpleNamespace(type=source_type)], + ), + language="it", + ) + + assert command._candidate_evaluator(cfg, vector_store=object(), embedder=object()) is None + + def test_preprocess_evidence_gc_json_is_pristine(monkeypatch, tmp_path): import tht.cli.preprocess_cmd as command diff --git a/harness/tht/cli/preprocess_cmd.py b/harness/tht/cli/preprocess_cmd.py index b6907648..ad3cd262 100644 --- a/harness/tht/cli/preprocess_cmd.py +++ b/harness/tht/cli/preprocess_cmd.py @@ -45,8 +45,19 @@ def _evaluation_workspace_root(cfg) -> Path: return root +def _requires_candidate_evaluation(cfg) -> bool: + evidence = cfg.evidence + if evidence is None or evidence.schema_version != 2: + return False + return evidence.source_root is not None or any( + source.type == "filesystem" for source in evidence.sources + ) + + def _candidate_evaluator(cfg, *, vector_store, embedder): """Bind candidate publication to the same read-only retrieval evaluator as the CLI.""" + if not _requires_candidate_evaluation(cfg): + return None from tht.evidence.canonical import load_curated_tree from tht.evidence.evaluation import evaluate_retrieval, load_evaluation_fixture @@ -77,9 +88,7 @@ def _candidate_evaluator(cfg, *, vector_store, embedder): def _validate_materialized_curated_corpus(cfg) -> None: """Fail closed on a v2 pinned filesystem corpus before any vector write is possible.""" - if cfg.evidence is None or cfg.evidence.schema_version != 2: - return - if not any(source.type == "filesystem" for source in cfg.evidence.sources): + if not _requires_candidate_evaluation(cfg): return from tht.evidence import validate_workspace_evidence diff --git a/harness/tht/evidence/formula_store.py b/harness/tht/evidence/formula_store.py index 4e787279..b88a2e22 100644 --- a/harness/tht/evidence/formula_store.py +++ b/harness/tht/evidence/formula_store.py @@ -8,6 +8,7 @@ from __future__ import annotations import hashlib import re +import unicodedata from dataclasses import dataclass from pathlib import Path from typing import Literal @@ -121,6 +122,7 @@ def legacy_formula_to_curated( formula: ConceptFormula, *, legacy_path: str, + source_content: str | None = None, ) -> CuratedEvidence | LegacyFormulaMigrationFailure | None: """Convert one reviewed legacy formula into its deterministic curated counterpart. @@ -129,10 +131,39 @@ def legacy_formula_to_curated( """ if formula.status != "reviewed": return None - source_notes = tuple(formula.sources) or ( - "Legacy formula migrated without a recorded provenance note.", - ) - source_sha256 = hashlib.sha256(formula.dump().encode("utf-8")).hexdigest() + problems: list[str] = [] + normalized_source: str | None = None + if source_content is None: + problems.append("original_source_required") + else: + from tht.evidence.authoring import normalize_source_text + + normalized_source = normalize_source_text(source_content) + try: + original_formula = ConceptFormula.parse(source_content) + except (TypeError, ValidationError, ValueError, yaml.YAMLError): + problems.append("original_source_invalid") + else: + if original_formula != formula: + problems.append("original_source_mismatch") + source_notes = tuple(formula.sources) + if not source_notes: + problems.append("supporting_excerpts_required") + elif normalized_source is not None and any( + unicodedata.normalize("NFC", note.replace("\r\n", "\n").replace("\r", "\n")) + not in normalized_source + for note in source_notes + ): + problems.append("supporting_excerpt_unverified") + if problems: + return LegacyFormulaMigrationFailure( + code="legacy_formula_requires_manual_review", + legacy_path=legacy_path, + formula=formula, + problems=tuple(sorted(problems)), + ) + assert normalized_source is not None + source_sha256 = hashlib.sha256(normalized_source.encode("utf-8")).hexdigest() source_file = legacy_path if legacy_path.startswith("source/") else f"source/{legacy_path}" # The legacy path is the immutable identity of this unit during migration. Keeping # its full digest avoids a duplicate public ID when the same concept has reviewed