Review Pr Skill for Claude

clickhouse-go PR Review Skill

Reviews an open GitHub pull request and posts the findings. The review criteria — the review

gates, the clickhouse-go supporting checks, and the severity model — are defined in

@.claude/skills/review-pr/review-core.md. When this skill is invoked as /review-pr from the

repo root, that reference injects the file into context automatically; if the core content is

not already in your context (e.g. you were pointed at this file directly), Read that path

before reviewing. Apply it in full — this file adds only the GitHub plumbing: fetching the PR,

the findings JSON schema, and posting.

Reviewing a local diff with no PR (e.g. before opening one)? Use review-core.md directly and

report findings as plain text — none of the JSON/posting machinery below applies.

Arguments

• $0 (required): PR number (e.g. 1869).

1. Obtain the diff and context

gh pr view "$0" --json title,body,headRefName,baseRefName,author,url
gh pr diff "$0"

• Read the title, description, and any linked issues for the intended behavior.
• For each changed file, Read the surrounding code when the diff alone is insufficient to judge

the change — a finding is only valid if you understand the code it touches.

• If the diff is large (>2000 lines), use the Explore agent to analyze parts in parallel.

Existing review threads (re-review)

If existing-threads.json is present (the CI workflow writes it; create it for interactive runs

with python3 .claude/skills/review-pr/post_review.py fetch --repo ClickHouse/clickhouse-go --pr "$0" > existing-threads.json),

Read it. It lists the review threads you already started, each with its prior comment(s), any

author replies, and is_resolved/is_outdated state. For every open (is_resolved: false)

thread, decide an action (section 3, thread_actions):

• The author replied with a question or pushback that still stands → reply addressing it.
• The concern is now addressed (the current code satisfies it, or the author's reply adequately

resolves/declines it) → resolve (optionally with a short closing reply).

• The concern still stands and there is no new author activity → keep (no new comment).
• The thread is is_outdated and the underlying concern no longer applies → resolve.

Do not raise a brand-new findings entry for a line that already has one of your threads — use

a thread_actions reply instead, or the poster will skip it as a duplicate.

2. Review the change

Work through the review gates and the clickhouse-go supporting checks from

review-core.md, and grade each finding with its severity model. Anchor each

finding to a specific changed line where possible.

3. Output: write the findings JSON

Anchor every finding you can to a specific changed line so it becomes an inline comment. Write

the result to claude-review.json (in the repo root) with this exact schema:

{
  "summary": "Structured markdown (see 'Writing the summary' below): a short intro line, then short paragraphs and/or bullet lists. Use `\\n` for line breaks. Cover what the PR does, the high-level verdict, and any blind spots.",
  "verdict": "approve | request_changes | needs_discussion",
  "findings": [
    {
      "path": "lib/column/date.go",
      "line": 142,
      "severity": "must_fix",
      "title": "short imperative title",
      "body": "Structured markdown (see 'Writing inline comments' below): lead with one sentence naming the broken invariant and its impact, then bullets when there is more than one point, then a ```suggestion``` block or diff for the fix. Use `\\n` for line breaks. Keep it short."
    }
  ],
  "general_findings": [
    {
      "severity": "should_fix",
      "title": "missing HTTP-path regression test",
      "body": "Findings that are NOT anchorable to a changed line (cross-cutting, or about code outside the diff). These render in the summary comment."
    }
  ],
  "thread_actions": [
    {
      "thread_id": "PRRT_kwDO...",
      "root_comment_id": 3421542714,
      "action": "reply | resolve | keep",
      "reply_body": "Required for action=reply. Optional closing note for action=resolve. Omit for keep."
    }
  ]
}

thread_actions is only for re-reviews where existing-threads.json listed open threads; copy

thread_id and root_comment_id verbatim from that file. Omit the array (or leave it empty) on a

first review. The poster skips replies it has already posted, so re-running is safe.

Writing the summary

The summary string is rendered verbatim as markdown in the PR comment, so format it for fast

scanning — never a single dense paragraph. Keep it tight; the inline comments carry the detail.

• Lead with one or two sentences stating what the PR does and the overall verdict.
• Break the rest into short paragraphs (2–3 sentences each) separated by a blank line (\n\n),

one idea per paragraph.

• Use a bullet list (\n between - items) whenever you are enumerating things — affected

surfaces, blind spots, follow-ups, or the key issues driving the verdict. Bullets beat prose for

any list of two or more items.

• Bold short inline labels (e.g. Blind spots:) to anchor sections when it aids skimming.
• Do not restate every inline finding here; reference them collectively and highlight only what

shapes the verdict.

Example shape (adapt freely):

Adds `DateTime64` scale handling to the native path. The core change is sound, but the HTTP
path is left uncovered.

**Key concerns:**
- Scale > 9 silently overflows (see inline on `lib/column/datetime64.go`).
- No regression test for the `std` API surface.

**Blind spots:** could not validate the HTTP round-trip without a live server.

Writing inline comments

Each finding's body renders as markdown directly beneath a bold severity + title header that

the poster prepends (❌ Must fix — <title>), so do not repeat the title or severity in the

body. Apply the same scan-first discipline as the summary: a reviewer reading the comment on the line

should grasp the problem and the fix in one pass. Never a single dense block of prose.

• Lead with one sentence naming the broken invariant and its concrete impact — e.g. "Scale > 9

overflows the int32 multiplier, so sub-second values silently truncate." No preamble ("I noticed

that…", "It looks like…"); state the problem directly.

• Keep the whole comment tight (aim for under ~6 lines). The reviewer needs the bug and the fix,

not exhaustive reasoning — push deep rationale to the summary or omit it.

• When the body has more than one distinct point (root cause, an affected sibling path, a test

gap), use a short bullet list (\n between - items) instead of stringing them into one

paragraph. One point → one sentence is fine; don't pad it into bullets.

• Put any concrete fix in a suggestion block (GitHub applies it in one click) or a fenced diff,

separated from the prose by a blank line (\n\n). Suggestion blocks must contain only the

replacement line(s) for the commented range.

Example shape (indented here to show the literal markdown, including a nested suggestion block):

Send() doesn't reset b.sent under the lock, so a concurrent Append races the flag and

can slip a row into an already-sent batch.

• The read in Append (line 88) is unsynchronized against this write.
• The std wrapper hits the same path via ExecContext.

        b.mu.Lock()
        b.sent = true
        b.mu.Unlock()

Rules for the JSON:

• line is the line number in the new version of the file, and must be a line that appears

in the diff (added or context). If a finding concerns code not in the diff, put it in

general_findings instead — the poster will reject off-diff inline lines and demote them anyway,

but placing them correctly keeps the output clean.

• One finding per distinct issue. Group issues that share a single root cause into one finding.
• If there are no findings, emit empty arrays and an approve verdict with a one-line summary.
• Do not invent line numbers. When unsure of the exact line, use general_findings.

4. Post the review

The CI workflow posts the JSON automatically as a separate step. When running interactively, post

it yourself:

python3 .claude/skills/review-pr/post_review.py post --repo ClickHouse/clickhouse-go --pr "$0" --input claude-review.json

post_review.py post is idempotent: it attaches inline comments to the relevant lines, skips

findings it already posted (so re-reviews do not duplicate), demotes any off-diff finding into the

summary, replies into existing threads, resolves addressed/outdated threads, and updates a single

summary comment in place instead of stacking new ones.