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):
| Priority | Meaning |
|---|---|
| P0 — critical | must fix; broken, unsafe, data-losing, or otherwise unshippable |
| P1 — high | should fix before merge; a real bug or security/correctness issue |
| P2 — medium | worth fixing; maintainability, performance, or edge-case issue |
| P3 — low | minor 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, andxiv stack build: a review agent reads the working-tree diff against the issue's acceptance criteria and returnsapprove/comment/request_changesplus 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.