Skip to content

TDD 0075: reviewer contracts — design rigor, fixed build reviewer, non-findings - #202

Merged
cahenesy merged 7 commits into
masterfrom
build/20261012-165626/0075-reviewer-contracts
Oct 15, 2026
Merged

cahenesy merged 7 commits into
masterfrom
build/20261012-165626/0075-reviewer-contracts

Conversation

@cahenesy

Copy link
Copy Markdown
Owner

Implements TDD 0075: reviewer contracts. It makes design and build review rigorous, adds a fixed build reviewer, and makes reviewers list what they considered and dismissed (non-findings). PRD FR-108, FR-109 and FR-110 (refs #195). Plugin 3.56.0 → 3.57.0.

Stacked on #201 (TDD 0074), which is stacked on #200 (TDD 0073). Merge in order: #200, then #201, then this PR. Auto-delete retargets each PR to master.

What it changes

  • agents/design-reviewer.md gains five report rules:

    • a cited ### Simulation trace (uncitable steps marked [unverified]);
    • the shared R-n <severity> — <loc> — <finding> line;
    • a 4-column alternatives table for every blocker or major finding;
    • terminology-drift and duplication passes;
    • ### Non-findings, plus an exact METRICS: line.

    Existing passages and the DESIGN_REVIEW: verdict lines are unchanged.

  • agents/build-reviewer.md (new) has ten rules:

    • it diffs <base-sha>..HEAD, so a stacked build is reviewed only on its own changes;
    • it always applies the security checklist;
    • findings outside the diff are labelled pre-existing and don't block;
    • FAIL if and only if a blocker or major finding is not pre-existing;
    • the implementer and verify reports are inputs.
  • /build-tdds:

    • step 6 records base-sha= in the per-TDD log;
    • step 9 documents the parent-dispatched fixture protocol (PENDING-PARENT);
    • step 10 dispatches the build reviewer by its prompt file, with a flagged merge-base fallback for older logs.
  • /tdd-author keeps the design review in the session and carries the trace, the tables, METRICS: and non-findings into the design-PR body.

  • ux-reviewer gets the shared finding line and the non-findings rule; both it and security-reviewer get the rule against dismissing a whole category of finding.

Gates (run 20261012-165626)

Gate Result
test-first PASS
ci-checks PASS. reviewer-contracts 141/0. The evals that read these files are all green: verdicts, build-tdds-skill, bounded-tdd-scope, evaluation-rubric, token-spend-reduction, model-roles, ux-pipeline, ux-author-skill and others
runtime-verify PASS. Obs 1–5 are mechanical. Obs 6–8 were parent-dispatched against fixtures, running the new prompts, results below
review PASS. Run with the new agents/build-reviewer.md itself

Behavioural fixtures (obs 6–8)

Fixture Prompt Expected Observed
fx6: "task" vs "job" naming drift, and an rc 2 missing from the interface design-reviewer BLOCK, cited trace, table, METRICS DESIGN_REVIEW: BLOCK. Trace cites its sources, with [unverified] marks. Three major findings, each with the table. METRICS: traced 2/2 requirements; findings blocker=0 major=3 minor=2 nit=2
fx7: build changes the bad-input rc from 2 to 1 build-reviewer FAIL citing the change REVIEW_RESULT: FAIL, R-1 and R-2 blockers at tool.sh:5 and tool.sh:6, plus an undeclared-deviation major finding
fx8: correct build, plus a planted unquoted rm -rf $dir/* on an untouched line build-reviewer PASS, defect labelled pre-existing REVIEW_RESULT: PASS, with R-1 minor pre-existing — tool.sh:6, which the Security section also flagged

Fixed after review

  • R-1 (test(failing) then fix). Rule 7 banned PASS on a trace made only of [unverified] steps, but rule 9 (FAIL if and only if a non-pre-existing blocker or major exists) never produced FAIL in that case. Rule 7 now records that case as a major finding. Gates were re-recorded on the new HEAD.

Follow-ups (non-blocking)

  • The build-reviewer prompt cites "(FR-55, ADR 0005)". Those are throughline's own requirement and ADR numbers, which mean nothing in a consumer repo, and design-reviewer already cites FR-55 the same way. The TDD pins this text.
  • tdd-author step 7b still says "Spawn the design-reviewer subagent", which names an agent type. The model-roles eval pins that wording, and it predates this build.
  • In --combined mode, a later TDD's review diff can include earlier TDDs' changes. This is disclosed through the merge-base fallback.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NpGG21Y2VGGhTQaFr4oshB

cahenesy and others added 7 commits October 12, 2026 13:57
Text checks (by necessity) for the design-reviewer and build-reviewer
contracts, FR-110 non-findings across reviewers with a readable-first
negated category-dismissal grep, /build-tdds steps 6/9/10, and the
/tdd-author design-PR body. Red against current code.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NpGG21Y2VGGhTQaFr4oshB
…ndings (TDD 0075)

- design-reviewer: cited simulation trace with the [unverified] rule,
  R-n findings line with the 4-column alternatives table for blocker/major,
  terminology and duplication passes, ### Non-findings, exact METRICS line;
  existing passages and the DESIGN_REVIEW verdict lines unchanged.
- agents/build-reviewer.md (new): 7 inputs incl. base-sha, the implementer
  and verify reports and the absolute security-checklist path; ten rules;
  pre-existing debt by diff hunks with the caller/callee clause; FAIL iff a
  blocker/major is not pre-existing; unchanged REVIEW_RESULT line.
- /build-tdds: step 6 records base-sha=<sha>; step 9 writes down the
  parent-dispatched fixture protocol (PENDING-PARENT); step 10 dispatches
  one worker by $(tl_plugin_root)/agents/build-reviewer.md with no
  checklist of its own (merge-base fallback for older logs).
- /tdd-author 7b/9: the trace, blocker/major tables, METRICS and
  Non-findings go into the design-PR body, held in-session, never drafted.
- ux-reviewer: shared finding line + Non-findings rule; security-reviewer:
  no-category-dismissal sentence.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NpGG21Y2VGGhTQaFr4oshB
…ing (review R-1)

Rule 7 forbade PASS on an all-[unverified] trace while rule 9 says FAIL iff
a non-pre-existing blocker/major exists, leaving that case with no
consistent verdict. Route it through rule 9 as a major finding.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NpGG21Y2VGGhTQaFr4oshB
Base automatically changed from build/20261012-165626/0074-tdd-lint-grounding to master October 15, 2026 13:41
@cahenesy
cahenesy merged commit 715b6dd into master Oct 15, 2026
@cahenesy
cahenesy deleted the build/20261012-165626/0075-reviewer-contracts branch October 15, 2026 13:41
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