Skip to content

ISS-13944, ISS-13949: Review unit criteria in code and visual QA - #421

Merged
shafty023 merged 9 commits into
mainfrom
feat/iss-13944-unit-ac-review
Nov 10, 2026
Merged

shafty023 merged 9 commits into
mainfrom
feat/iss-13944-unit-ac-review

Conversation

@shafty023

@shafty023 shafty023 commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • ISS-13944: a grouped unit is not ready, its PR does not open, and its tickets do not move to In Review or Done until every acceptance criterion of every member ticket, plus its PRD's criteria for the pages it touches, passes a review of the completed work in two parts: a code review tied to a file and line, and visual QA showing it in the running product. Daniel, 2026-11-10: "ensuring all acceptance criteria is met ... includes both code and visual-qa".
  • Prevent-recurrence research: 11 of PRD-827's misses traced to the PRD-706 tickets and PRs that shipped them (ISS-12462 / #8885, ISS-12305 / #8886, ISS-13431 / #9442 and others). vibe 1.1.70 would have caught none of them. Classes: PRD criteria never read; one page or surface standing in for the rest; code-only or capture-only evidence; status moves with no gate; no gap list; grouping performance never written as a criterion. A prevention for each class is below.
  • New handoff/scripts/unit-review.mjs:
    • Criteria come from the ticket, --member, --prd and --plan bodies. PRD criteria are read from its acceptance checklist and from lines that lead with their own id (**BR-8-01.**).
    • review.json holds code rows (file:line, checked at HEAD) and visual rows (one image per page and surface, magic-byte checked; one image may not cover two cells).
    • A criterion needs every page and surface it names. "Every list page" and "where available" need every page, "both surfaces" needs every surface, and a criterion naming no page needs every touched page. A PRD criterion naming only untouched pages prints OUT-OF-SCOPE.
    • Output is GAP <ticket> <criterion>: <missing> lines for the operator, and --record saves the result for the fleet board.
  • Exceptions to the two-part review:
    • A unit with no user interface leaves pages and surfaces empty and passes on code rows alone. A criterion there that still names a page or surface is a gap.
    • Inside a UI unit, a criterion with nothing to see may take one NOT-VISIBLE visual row with a reason, only when it names no page or surface and has no cross-page or cross-surface wording. Each one prints NOT-VISIBLE <ticket> <criterion>: <reason>.
  • Performance (list-query-performance.mjs): every grouped, sorted or paginated list query the unit touches is listed in listQueries. Each one needs a criterion naming its page, its kind and a load time, met by a measured row (command, dataset rows, timing within the budget). Otherwise the review prints GAP <ticket> performance <page> <kind>.
  • criteria-audit.mjs:
    • A session with members or a PRD fails until --review passes.
    • For every session, a criterion naming Web, Desktop, the browser or the running app is never MET by typecheck, unit-test or source evidence alone. This also covers ISS-13947's class 5 runtime item.
    • Parsing moved to criteria-list.mjs (re-exported) to break an import cycle.
  • ISS-13949 (own commit): with --passing-on, a PARTIAL not met yet: waits on <ticket> row is accepted while --ticket-status shows that ticket open, and is listed under waiting. It is refused for a PR, with no ticket, with no status, or when the ticket is DONE or CANCELED.
  • Gates and statuses:
    • handoff-gate.mjs --review/--prd/--plan binds those files into the witness (vibe-core's handoff-gate-check.mjs checks them) and records unitReviewPassed, so the PR-create hook refuses a unit's PR until the review passes.
    • board.mjs unit --closed refuses a unit without a passing review, and board.mjs status prints UNIT-UNREVIEWED.
    • status-audit.mjs --board keeps members IN_PROGRESS and flags one moved early.
    • New vibe-core PreToolUse guard hooks/unit-status-guard.mjs (matcher mcp__closedloop__update-document) denies IN_REVIEW or DONE for an unreviewed unit's ticket.
  • Versions: vibe 1.1.77, vibe-core 1.0.2, VIBE_CORE_MIN 1.0.2, with CHANGELOG entries. Merged with origin/main after ISS-13869: Move vibe hooks into the vibe-core plugin #418 and ISS-13846: Accept real runner evidence in mutation-proof gate #422 (plain merges, no rebase).

Breaking changes

  • A vibe session with member tickets or a PRD now needs --review (and its PRD) at the criteria audit and the handoff gate, and a criterion naming Web or Desktop needs runtime evidence. This is the ticket's intent. Single-ticket sessions without such criteria are checked as before.
  • vibe 1.1.77 needs vibe-core 1.0.2 or newer.

UI Feature Flag

  • None. Plugin scripts, hooks and instructions only.

Test plan

  • Second laptop, node CI set at 340855b: 189 files, 1905 tests, 1902 passed. The two failures were plugin-version-bootstrap.test.mjs (fixed in 7dfb31c, 4/4 since) and e2e-lock "a second signal escalates" (passes alone).
  • Second laptop at 31412be: uv run pytest plugins/ 2768 passed, 3 skipped; ruff and pyright clean; vibe-bench typecheck, validate:plugins and npm test (199) passed. The test machine then went down before vibe-workers ran, so vibe-workers is validated by this PR's CI only. Targeted reruns on 31412be: unit-review 18, criteria-audit 9, unit-status-guard 3, handoff-gate-members 3, plugin-version-bootstrap 4, all passing. The vibe-workers dist bundles were rebuilt (only VIBE_CORE_MIN changes in them) after CI flagged them stale.
  • New tests: unit-review.test.mjs (18), criteria-audit.test.mjs (9), unit-status-guard.test.mjs (3), plus additions to handoff-gate-members, board and status-audit. Each core check was turned off once to confirm a test goes red: the code-row requirement, the per-page loop, PRD scoping, list-query gaps, the measured row, the NOT-VISIBLE naming check and the runtime rule.
  • Failures on the primary laptop that also fail on an untouched origin/main: two e2e-lock Compose tests, one docker-disk-admission test, and vibe-workers main-sync.test.ts "runs only the commit-hook checks" (timeout).

A grouped unit is not ready, its PR does not open and its tickets do not
move to In Review or Done until every member and PRD criterion passes a
code review row and a visual QA capture on each page and surface it names.
Adds unit-review.mjs (with list-query performance criteria and NOT-VISIBLE
rows), the criteria-audit runtime-evidence rule, handoff-gate --review,
board and status-audit checks, and the update-document status guard.
criteria-audit.mjs now honours the passing-it-on exception: with
--passing-on, a PARTIAL 'not met yet: waits on <ticket>' row is accepted
while --ticket-status shows that ticket open and is listed under waiting.
It is refused for a pull request, without a ticket, without a status, or
when the ticket is DONE or CANCELED.
Moves the ticket status guard and the unit review record reader into
vibe-core (1.0.1), binds the review, PRD and plan evidence in
vibe-core's handoff-gate-check, and raises VIBE_CORE_MIN to 1.0.1.
Takes vibe 1.1.77 after ISS-13846's 1.1.75; vibe-core stays 1.0.2.
Keeps vibe 1.1.77, vibe-core 1.0.2 and VIBE_CORE_MIN 1.0.2 after ISS-13899's
1.1.76 and 1.0.1; rebuilds the vibe-workers bundles from the merged sources.
@shafty023
shafty023 merged commit 00e62e6 into main Nov 10, 2026
12 checks passed
@shafty023
shafty023 deleted the feat/iss-13944-unit-ac-review branch November 10, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant