Skip to content
xiv

xiv-review-core

The code-review judgment itself — the contract the automated local review step follows.

This skill defines what a review is in xiv: how to gather context, what clears the bar as a finding, and how findings are reported. It has no CLI of its own and, by design, never posts, submits, or pushes anything. Invoke it whenever someone wants a change reviewed — a branch, an open PR, or work-in-progress diff — and wants findings, not side effects.

It is a single SKILL.md with no reference docs: the contract is meant to be loaded whole.

Gather context before judging the diff

The skill front-loads context so findings are grounded, not diff-local:

  • The linked Linear issue. Found via the issue ID (usually ENG-####) in the branch name, PR title, or description. Its acceptance criteria are read and checked for parity against what was implemented — a gap between criteria and implementation is itself a finding.
  • The open PR, fully. Sync local HEAD to the latest pushed commit first, then read every comment surface — formal reviews, inline diff comments, and conversation comments (where bots and humans most often post, and the surface most easily missed). Prior findings are verified against the current code, not the thread's resolved/unresolved state: still-unaddressed ones are re-flagged, already-covered ones are not re-raised.
  • CI / status checks. A failing build, test, or type check often points straight at a finding.

What counts as a finding

An issue is flagged only when all of these hold: it meaningfully affects accuracy, performance, security, or maintainability; it is discrete and actionable (one specific fix, not a diffuse critique); it was introduced by this change (pre-existing bugs are out of scope); the author would likely fix it once aware and it isn't clearly intentional; it holds without unstated assumptions (cross-file breakage must name code provably affected); and the fix matches the rigor of the surrounding codebase.

Every finding that clears the bar is reported — the review doesn't stop at the first — but if nothing clears it, the correct output is no findings, not padding.

Output

Findings print as numbered items, each with a priority, a short bold title, a one-paragraph explanation, and the file (plus line range when useful):

PriorityMeaning
P0 — criticalmust fix; broken, unsafe, data-losing, or otherwise unshippable
P1 — highshould fix before merge; a real bug or security/correctness issue
P2 — mediumworth fixing; maintainability, performance, or edge-case issue
P3 — lowminor cleanup the author would still want to know about

Comment style is codified too: lead with why it's a bug and the conditions it needs, match severity to reality, matter-of-fact tone with no flattery, no code chunks over three lines, and suggestion blocks only for concrete replacement code with exact leading whitespace preserved.

One contract, two callers

The same judgment runs in two places, so the standard is identical whether a human asked for a review or the implement loop produced one:

  • Interactively, when an agent loads this skill on a "review this" request — or underneath xiv pr review, where the CLI pulls the PR into a worktree and this skill is the authority on what's worth flagging.
  • Automatically, as the local review step inside xiv implement, xiv ship, and xiv stack build: a review agent reads the working-tree diff against the issue's acceptance criteria and returns approve / comment / request_changes plus findings under this same contract, before anything is pushed. See Review model.

Choosing it over the xiv pr commands

When the user wants findings on a real PR but nothing posted, this skill and xiv pr review overlap. The tiebreaker: "just tell me what's wrong" leans xiv-review-core (lighter, no worktree, purpose-built for the judgment); "take a look and maybe leave comments" leans xiv pr review. Anything that should change code (xiv pr fix, xiv pr refine) is a different tool entirely — the xiv skill walks through the disambiguation.