interface-review

Interface review

This skill reviews a change rather than a screen. It resolves the scope, expands the changed files to the surfaces they affect, reads both sides of the diff and classifies every finding.

It owns the scope, the classification and the change-scoped report. Domain rules belong to the better-* skills. Severity, consolidation, coverage, the cap and the verdict belong to better-interface, which this skill hands the review to.

Correctness, tests, security and performance belong to the project's general code review. Name such a concern once, point at that review and drop it.

The change, not the codebase

The author is asking "did I make this worse?". Report what the change caused and stay mostly quiet about what it merely touched. Three pre-existing findings is a courtesy; thirty is a different review and one nobody asked for.

Read the whole diff before forming an opinion of it. A skimmed diff produces findings about code the next hunk already fixed.

1. Resolve the change scope first

The whole invocation is the target, so /interface-review pr 482 reviews pull request 482. Scope resolution ↗ holds the accepted targets and how each resolves.

With no target supplied, resolve in this order and stop at the first match:

1. HEAD is ahead of git merge-base origin/<default-branch> HEAD. Review that range plus any uncommitted changes, stating the commit count and the uncommitted file count separately.

2. The working tree is dirty. Review the uncommitted changes.

3. Neither. There is no change to review, so follow With no change, ask rather than invent one.

Record the base commit of the resolved range as $BASE and name it in the scope block. Every command in this skill and its references uses it.

Exclude lockfiles, snapshots, generated output, vendored code and binaries per Excluded paths ↗, and name what you excluded. If nothing survives the exclusions, treat it as no change.

2. With no change, ask rather than invent one

A clean tree with nothing ahead of the merge base means there is no change to review. Never fall back to HEAD~1..HEAD on your own. The last commit is whatever happened to land, often a merge or someone else's work.

Gather the facts in Nothing to review ↗, state them, then offer these routes and wait:

• The open pull request on the current branch, first when there is one. A branch whose commits already landed resolves to no change, while its pull request is still exactly what the user meant.
• The last commit, HEAD~1..HEAD, named by short SHA and subject so the user sees what they would get.
• A target they name, such as pr <n>, a branch, a ref or a range.
• A whole-repository interface audit, which is not a change review. Hand it to better-interface as a repository-scope review, without this skill's scope block, statuses or pre-existing section.

Where exclusions emptied the scope, name the excluded files in the offer. Never report a review of nothing as Approve.

3. A diff is not a surface

A changed file is evidence, not the review subject. Its blast radius is the set of surfaces it renders in; review those.

Expand one hop by default, to the direct importers and callers. Expand a second hop only for design tokens, theme values and shared primitives, where one line reaches the whole product.

Review at most five consumers in total across both hops, ordered by the rule in Scope resolution ↗. State how many you did not expand, since an unstated cutoff reads as completeness.

4. Read the removed lines

Regressions are invisible in the post-change state. Read the - side of every hunk against Removed signals ↗.

A signal is a lead, not a finding. A removal is a regression only when nothing in the change replaces it, and the domain skill owns that judgement. Route each unmatched removal to its owner, report only what that skill confirms and status it Regression.

5. Classify every finding

Give every finding one status:

• Introduced: the change created it.
• Regression: the change weakened something previously correct.
• Pre-existing: present before the change and not made worse by it.

Status by cause, not by location. A finding a changed line causes is Introduced or Regression wherever it surfaces, including an untouched consumer of a changed token. A finding the change neither created nor worsened is Pre-existing, even three lines from a hunk.

To check whether a line predates the change, blame the reviewed range. A ^ prefix marks a line unchanged since the base:

git blame -L <line>,<line> "$BASE"..<head-ref> -- path/to/file

For uncommitted work, drop the range. A line marked Not Committed Yet or blamed on a commit inside $BASE..HEAD belongs to the change.

6. Hold the change to its stated intent

Read the pull request title and body, the linked issue and the commit messages, then review whether the interface delivers what they claim.

This is how you find the incomplete change, which a surface review misses because it inspects only the states that exist. Look for the absent ones:

• A new variant, size or theme applied to some states but not all of hover, focus, active, disabled, loading and selected.
• A new user-facing string with no entry in the translation catalogue the project maintains.
• A new component with no empty, loading, error, disabled or narrow-width state.
• A control added to one surface but not to the siblings that already carry its peers.

Do not report scope creep. Whether a change does too much is a process question, not an interface one.

7. Hand the review to better-interface

Hand better-interface the scope block, the affected surfaces and a status on every finding, and report in the format below.

If better-interface is unavailable, report the resolved scope and the file inventory, name it as the missing skill and stop. Do not invent a severity scale, a cap or a verdict.

Never mutate the working tree

A change review is read-only, including the checkout. git fetch writes only to .git and is permitted. gh pr checkout, git checkout, git switch and git stash rewrite the files the author has open, so they are never permitted. Fetch pull request refs and read them in place.

Rendered verification is opt-in here, in place of better-interface's Verify what can be verified. Mark visual and runtime claims Not verified unless the project exposes a cheap preview or the user asks for a rendered review. For working and branch, the checkout already is the head, so render it in place. For any other target, render an isolated worktree such as git worktree add /tmp/review-<n> refs/remotes/pr/<n> and remove it when done.

Before you finish

| Mistake | Fix |

| --- | --- |

| The scope block counts only uncommitted files while HEAD is ahead of the merge base | Review the branch range plus the uncommitted changes, with both counts |

| A report on HEAD~1..HEAD the user never chose | State the facts and offer the routes in With no change, ask rather than invent one |

| A changed shared component with no consumer under Surfaces expanded | Expand to its consumers and name the ones you skipped |

| A deleted aria-label, outline or prefers-reduced-motion with no finding and no note | Route it to its owner through Removed signals ↗ |

| A Regression whose removed attribute reappears elsewhere in the diff | Check the equivalent replacements ↗ first |

| A finding on an untouched consumer of a changed token statused Pre-existing | The change caused it, so it is Introduced or Regression |

| A line outside every hunk statused Introduced with no causing change named | Blame it against the reviewed range |

| gh pr checkout or git switch in the command log | Fetch the ref and read files with git show |

| A cited line that does not match git show <head-ref>:path | Cite against the head ref named in the scope block |

Review output format

Open with the scope block:

| Field | Value |

| --- | --- |

| Target | branch, working, staged, pr 482 or the range as entered |

| Base ref | origin/main at a1b2c3d |

| Head ref | refs/remotes/pr/482 at e4f5g6h |

| Commits | 7 committed, 2 files uncommitted |

| Files in scope | 12 after exclusions |

| Excluded | pnpm-lock.yaml, src/__snapshots__/, lockfile and snapshots |

| Surfaces expanded | CheckoutPage, SettingsPanel; 3 further Button consumers not expanded |

The coverage table from better-interface follows it. A domain with no evidence in the change scope is Not reviewed: no evidence in the change scope, which is a coverage statement rather than a gap.

Then the findings, with a Status column per Classify every finding:

| Severity | Domain | Status | Location | Before | After | Why |

| --- | --- | --- | --- | --- | --- | --- |

| HIGH | Accessibility | Regression | src/Dialog.tsx:42 | aria-label="Close" removed in this change | Restore aria-label="Close" on the icon-only control | The close control had an accessible name before this change and no longer does |

With no Introduced or Regression findings, omit the table and state "No actionable interface findings in this change."

Then Pre-existing findings, at most three, highest severity first, stated plainly as not this change's responsibility. Omit the section when there are none.

| Severity | Domain | Location | Issue |

| --- | --- | --- | --- |

| MEDIUM | Typography | src/Toolbar.tsx:7 | Numeric badges use proportional figures; predates this change |

Verification and the verdict follow in better-interface's format.