xiduzo/wtf/wtf-pr-review

PR Review

Review a pull request as a tech lead.

This skill reads the diff against the linked Trace spec (story, Scenario Claim, Spine Position, Contracts). The claimed scenario text is canonical in the parent Feature body — the review fetches it from there. It catches spec drift, missing test coverage, contract violations, and work outside the claim before merge. It does this by reading the code, not by running the software. Review a PR for a legacy Task against the Gherkin and Impacted Areas in the Task body.

Distinct from `wtf.verify-trace`: wtf.verify-trace is a QA engineer running the implemented behavior against the claimed Gherkin scenarios (does the software do what it says?). This skill is a developer reviewing the code itself (is the code written correctly against the spec?).

Process

0. GitHub CLI setup

Run steps 1–2 of ../references/gh-setup.md. Stop if gh is not installed or not authenticated.

1. Identify the PR

If a PR number was passed in, use it directly. Otherwise:

gh pr list --state open --json number,title,headRefName --limit 20

Call AskUserQuestion (per ../references/questioning-style.md):

• question: "Which PR are you reviewing?"
• header: "PR"
• options: from open PRs listed above

Fetch the PR:

gh pr view <pr_number> --json number,title,body,headRefName,baseRefName,additions,deletions,changedFiles

2. Fetch the spec hierarchy

Extract a Trace number from the PR body (Closes #<n> or Fixes #<n>) per the PR-extraction recipe in ../references/spec-hierarchy.md. If found, walk Trace → Feature → Epic per the same reference. From the Trace, extract the story, the Scenario Claim, the Spine Position, Contracts, and DoD. From the Feature, extract the canonical text of each claimed scenario, plus ACs and Goal. Extract constraints from the Epic. The Feature body is canonical for scenario text — review against it, not against the Trace's synced copy.

If the linked issue is a legacy Task (see ../references/issue-classification.md), review against the Task body instead. Extract its in-body Gherkin, Contracts, Impacted Areas, and DoD per the legacy sections of ../references/spec-hierarchy.md.

If no Trace number is found, call AskUserQuestion (per ../references/questioning-style.md):

• question: "Is there a Trace issue linked to this PR?"
• header: "Linked trace"
• options:
• No linked trace → review from diff only
• Yes — I'll provide the number → enter the trace issue number

If there is no linked Trace, the review proceeds from diff context alone (step 4 will note the absence of a spec as a finding).

3. Load the technical steering document

Load docs/steering/TECH.md per the best-effort consumer-side load in ../references/steering-doc-process.md. Apply its patterns, constraints, and conventions as the baseline for code quality judgements throughout this review.

4. Inspect the diff

git diff <base_branch>...<pr_branch> --stat
git diff <base_branch>...<pr_branch>

Read the full diff. Note:

• Which files changed and which layers they touch
• What was added vs. removed vs. moved
• Whether every change serves a claimed scenario or a contract (for a legacy Task: whether the changes stay within its Impacted Areas)

5. Run the review checklist

Evaluate each dimension. Record findings as PASS / FAIL / WARN per item.

a. Spec adherence

For each scenario in the Scenario Claim, use the canonical text from the Feature body (for a legacy Task: each Gherkin scenario in the Task body):

• Is there a test that covers this scenario? (search test files for scenario name or equivalent describe/it block)
• Does the implementation match the Given/When/Then behavior — not just passing the test, but structurally doing what the scenario describes?

Flag: missing test for a claimed scenario, test present but testing the wrong behavior, implementation that bypasses the scenario entirely. Also flag implementation or tests for scenarios outside the claim — those belong to other Traces in the Trace Plan (scope creep).

b. Contract compliance

For each entry in the Trace's Contracts & Interfaces section:

• Does the code implement exactly the specified shape? (field names, types, optionality)
• Are there any added fields not in the spec (scope creep)?
• Are any specified fields missing from the implementation?

Flag: schema drift, extra fields, missing fields, wrong types.

c. Spine Position

Read the Trace's Spine Position and review the diff against it:

• Skeleton — review for lean-but-complete: production quality with error handling and tests, minimal surface, no gold-plating. Flag speculative abstraction, handling for unclaimed edge cases, or configuration nothing in the claim needs.
• Extension / Deepening — confirm the diff builds on the named prior Traces and does not rework the Spine without a stated reason.
• In both cases, flag changed files that no claimed scenario and no contract explains (scope creep signal).

For a legacy Task, replace this dimension with an Impacted Areas check. Cross-reference the Task's Impacted Areas list against the actual changed files:

• Unexpected files changed outside the listed areas (scope creep signal)
• Listed areas with no changes (may indicate incomplete implementation)

d. Code quality against TECH.md

Review changed code against the patterns and constraints in docs/steering/TECH.md:

• Naming conventions (domain language vs. generic names)
• Layer separation (e.g. business logic in wrong layer)
• Error handling patterns
• Function length and nesting (flag >40 lines or deep nesting as a warning, not a blocker)

e. Test coverage

• Are there tests for the happy path AND at least one error/edge path per public function changed?
• Does the test file pattern match the project convention (from TECH.md)?
• Are there any skip, todo, or commented-out assertions in the new tests?

f. Definition of Done

Check the Trace's DoD checklist against the diff:

• Are all DoD items evidenced in the diff or existing code?
• The releasability item ("System releasable after merge") matters most — a Trace never merges as a prototype.

6. Summarise findings

Produce a structured review summary:

PR Review — #<pr_number>: <title>
───────────────────────────────────────────────
Linked trace: #<n> | Claim coverage: [n/n claimed scenarios covered]
 
PASS  Spec adherence     — all [n] claimed scenarios have matching tests, nothing outside the claim
WARN  Contract compliance — `paymentId` field missing from response shape (Trace spec: required)
PASS  Spine position     — Skeleton stays lean: minimal surface, no gold-plating
WARN  Code quality       — `processSettlement()` is 58 lines (threshold: 40)
PASS  Test coverage      — happy path + 2 error paths
FAIL  DoD                — "Observability: metrics emitted" not evidenced in diff
 
Verdict: REQUEST CHANGES
───────────────────────────────────────────────
Required (blocking merge):
  1. [Contract] Add `paymentId` to settlement response — Trace spec requires it (Contracts §2)
  2. [DoD] Emit the `settlement_processed` metric — listed as required in Trace DoD
 
Suggested (non-blocking):
  3. [Quality] Extract `processSettlement()` into smaller functions — currently 58 lines

Verdicts:

• APPROVE — no blocking findings
• APPROVE WITH COMMENTS — suggestions only, nothing blocking
• REQUEST CHANGES — one or more FAIL items that must be resolved before merge

7. Review with user

Show the summary. Then call AskUserQuestion (per ../references/questioning-style.md):

• question: "Does this look right? Should I post this as a GitHub PR review?"
• header: "Post review"
• options:
• Post it → submit as a GitHub PR review comment
• I have edits → adjust before posting
• Don't post — just the summary → keep it in the conversation

Apply any edits, then proceed.

8. Post the review

Apply strict STE per ../references/ste-writing.md before writing any durable body.

Write the review body to a temp file ($BODY) with the Write tool. Then post it through the gh body helper (../references/gh-body-helper.md) so it survives UTF-8 on Windows:

# $BODY is the temp file you wrote the review body to with the Write tool.
python3 .wtf/gh-body.py review <pr_number> \
  --<approve|request-changes|comment> \
  --body-file "$BODY"

Use:

• --approve for APPROVE or APPROVE WITH COMMENTS
• --request-changes for REQUEST CHANGES
• --comment if the user chose "Don't post — just the summary" but then changed their mind

Print the PR URL.