The handoff brief was a work order for phases 2-5. That work is done, so the
document now describes a future that already happened and would misdirect
anyone who found it.
The review report itself had the same problem in weaker form: its findings read
as open. Adds a status banner marking it closed and retained for reasoning only.
The banner also records that two of its recommendations were wrong on contact.
The HIGH-03 fix as written would have stripped root-cause data from
ExecutionAttempt provenance, and the HIGH-01 fix had to preserve per-page
durability the report never mentioned. Leaving that unstated invites someone to
'restore' the report's version later.
Co-authored-by: Copilot App <[email protected]>
Records the implementation plan derived from the 2026-08-23 review: per-task
acceptance criteria, the verification baseline, and environment constraints.
Lives in docs/reviews/ so it is discoverable from the repo rather than from
session state, and is indexed from docs/reviews/README.md. Non-canonical, like
everything under docs/reviews/**.
Co-authored-by: Copilot App <[email protected]>
Phase 1 of docs/reviews/2026-08-23-code-review.md.
HIGH-01: process_queued_job committed page evidence and the terminal job
status in separate transactions, so a crash between them left a transcript
persisted against a job stuck in PROCESSING that the worker never reclaims.
The final page's write is now deferred into _finalize_batch_outcome so it
shares the terminal transaction. Intermediate pages remain individually
durable, and the terminal commit is shielded against cancellation the same
way per-page writes already were.
HIGH-04: added tests/integration/test_pipeline_atomicity.py covering both
Transaction B and Transaction C. Confirmed failing against the previous
implementation before the fix.
HIGH-03: classify_unexpected_error interpolated the raw exception into
AppError.message, which the UI renders and the API serializes, leaking the
database path from OperationalError. message is now generic. Because message
also feeds format_error_detail, which writes evidence records, the root cause
is preserved on a new internal-only AppError.detail field rather than
discarded.
HIGH-02: replaced 8 hand-rolled ui.notify error calls in home_page and
people_page with error_presenter.show_error, restoring the correlation
error_id, canonical category, and suggestion. Added an AST guard to
test_ui_boundaries.py so pages cannot hand-roll error notifications again.
Docs updated per documentation-sync: the message/detail split in
docs/error_handling.md and the multi-page atomicity rule in
services.instructions.md.
Verification: ruff clean, 381 tests passing, ty unchanged at 10 known
SQLAlchemy descriptor false positives.
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
