Three gaps surfaced by implementing the 2026-08-23 review's recommendations.
1. Recommendations were never verified the way claims were. The report's fix for
the error path leak would have stripped root-cause data from evidence records,
because the review traced one consumer of AppError.message and missed that
format_error_detail writes it to ExecutionAttempt.error_detail. Adds workflow
step 9 (validate recommendations against consumers), a Blast Radius field on
findings, and the worked example so the failure mode is concrete.
2. Fixes that sit between competing invariants were not flagged. The atomicity
recommendation did not note that per-page durability and terminal-status
atomicity pull in opposite directions, so the obvious simplification silently
breaks multi-page durability. Recommendations must now name both invariants,
the test guarding each, and the over-correction to avoid.
3. Severity could not express reachability. Two findings were latent behind a
default setting and a single-instance deployment, which is a sequencing
constraint: they must be fixed before the change that makes them live. Adds an
explicit Reachability field with Live / Latent / Theoretical.
Verified: meta contract guards and traceability tests pass.
Co-authored-by: Copilot App <[email protected]>
The reviewer skill recorded four deterministic checks as unenforced or partial. Add tests so they fail the build instead of relying on a reviewer noticing.
tests/test_model_contract_guards.py:
- Status vocabulary: flags string literals compared against or assigned to status/purpose attributes, plus a narrower sweep that requires every status-valued literal in the package to be a known non-status use.
- Relationship loading: every Relationship must declare lazy='raise' except documented exceptions, and the exception set must match the Relationship Loading Contract in docs/schema.md.
- Schema fidelity: the Field-Accurate Table Contracts tables must match db/models.py on table coverage, field names, and declaration order, and the Authoritative Enumerations section must match the enum members.
tests/test_orphan_sweep.py:
- Locks the set of unreferenced public definitions. Route handlers registered by decorator are exempt, string entrypoint references count, and tests/ and tools/ count as consumers. KNOWN_ORPHANS records the four current orphans with rationale; a new one fails the build.
Each guard was mutation-tested: reverting the fix below, dropping a documented field, widening a lazy strategy, and adding a stranded function each fail their respective test.
Also fix the one violation the status guard found: sources_page.py compared attempt.status.value to the literal 'transcribed' instead of JobSourceStatus.TRANSCRIBED, which would survive an enum rename.
Co-authored-by: Copilot App <[email protected]>
Update the reviewer skill so its procedure matches how this repo actually works:
- Route review reports to docs/reviews/ and mark them non-canonical, resolving the conflict where reports landed in the same docs/ tree they resolve findings against.
- Pin verification commands to uv (uv run ruff check / ty check / pytest -m 'not external').
- Record the pytest contract: strict markers, strict asyncio mode, and the never-awaited-coroutine warning promoted to an error.
- Convert the deterministic checks to a table with an Enforced by column; three checks are unenforced and one only partial, which are now findings by construction.
- Add a consequence-based severity rubric and a Direction column for bidirectional drift.
- Escalate test-suite concerns to test-effectiveness-auditor.
Also fix tests/test_db.py, which was missing 'from sqlalchemy import text' while using it in 14 places. Three tests were failing with NameError. Wrapped the pre-existing long lines in the same file so it lints clean.
Document the deliberate nicegui==3.13.0 pin in pyproject.toml, a new runbook dependency upgrade policy, and the reviewer skill, so the pin is not flagged as a defect or widened as incidental cleanup.
Co-authored-by: Copilot App <[email protected]>
Jim Lancaster
