Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion .github/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ The `pr-review` action additionally accepts:
reviews, but skips it for ordinary incremental updates.
`on` always enables it and `off` disables it.

The semantic-analysis stage runs under a turn budget: 12 for a low-risk incremental review,
36 for a strong-tier one, and 56 for `deep`.
Roughly ten turns go on mandated context — six pipeline files plus repo guidance — before the
diff is read, and a small diff inside a large file spends many more paging through it, so the
budget tracks files to understand rather than lines changed.
The retry gets half again as many turns as the first attempt, because exhausting the budget is
deterministic and replaying it with the same budget cannot succeed.

Consumers that already create a GitHub App token can opt into the unified identity with:

```yaml
Expand All @@ -60,7 +68,9 @@ The PR reviewer is an explicit staged pipeline:

1. A deterministic preparation step freezes the base and head SHAs, loads the durable review
manifest, fetches prior automated threads, computes the full or incremental diff, and
selects the model tier.
selects the model tier. It then posts or updates the sticky status comment to
`🔄 Review in progress`, so the PR shows the round has started instead of staying silent
until the review lands minutes later.
2. Claude performs semantic analysis and verification with read-only tools.
It returns schema-constrained data and cannot publish comments or resolve threads.
3. A deterministic compiler validates findings, enforces severity budgets, checks RIGHT-side
Expand Down Expand Up @@ -124,6 +134,11 @@ every run, so a reader who meets it mid-thread can tell it describes the current
than the moment it first appeared.
It carries the reviewed range, an update timestamp, this round's counts, and a roll-up of the
questions still awaiting an answer with a link to the review that asked each one.
It moves through three phases: `🔄 Review in progress` from preparation, then either the
finished verdict or `🛠️ Review did not finish` for a round that ends without publishing.
Every non-publishing path — a failure, a discarded stale head — retires the in-progress phase
itself, so the comment never sits at "in progress" after the job ends. A skip-mode round
publishes nothing and is never announced.
When old automated review threads are addressed, the deterministic publisher resolves them
without adding confirmation replies.
A consumer repo's `REVIEW.md` may override or extend the semantic severity guidance.
Expand Down
28 changes: 24 additions & 4 deletions .github/actions/claude-pr-review/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,30 @@ runs:
allowed_tools="${allowed_tools},Task"
fi

# Turn budgets. The prompt mandates reading six pipeline files plus
# repo guidance before analysis begins, so roughly ten turns are spent
# on context before the diff is even read; a review whose diff sits in
# a large file spends many more paging through it. 24 proved too tight
# for exactly that shape (a small diff inside a big file) even though
# it is ample for a typical PR.
selected_model="$STRONG_MODEL"
max_turns=24
max_turns=36
if [[ "$MODEL_TIER" == "fast" ]]; then
selected_model="$INCREMENTAL_MODEL"
max_turns=12
fi
if [[ "$REVIEW_DEPTH" == "deep" ]]; then
max_turns=40
max_turns=56
fi
# Exhausting the budget is deterministic: retrying with the same one
# burns a second full model run that cannot succeed. Give the retry
# half again as many turns so it can actually finish.
retry_turns=$(( (max_turns * 3 + 1) / 2 ))
runtime_flags="--max-turns $max_turns"
retry_runtime_flags="--max-turns $retry_turns"
if [[ -n "$selected_model" ]]; then
runtime_flags="--model $selected_model $runtime_flags"
retry_runtime_flags="--model $selected_model $retry_runtime_flags"
fi

cp "$GITHUB_ACTION_PATH/rubric.md" "$GITHUB_WORKSPACE/.pr-review/rubric.md"
Expand All @@ -126,6 +138,7 @@ runs:

echo "allowed_tools=${allowed_tools}" >> "$GITHUB_OUTPUT"
echo "runtime_flags=${runtime_flags}" >> "$GITHUB_OUTPUT"
echo "retry_runtime_flags=${retry_runtime_flags}" >> "$GITHUB_OUTPUT"

- name: Analyze and verify findings
id: review
Expand Down Expand Up @@ -249,10 +262,14 @@ runs:
--allowedTools "${{ steps.compose.outputs.allowed_tools }}"
--disallowedTools "mcp__github_inline_comment__create_inline_comment"
--json-schema '${{ steps.prepare.outputs.output_schema }}'
${{ steps.compose.outputs.runtime_flags }}
${{ steps.compose.outputs.retry_runtime_flags }}
prompt: |
Produce the schema-bound pull request review result now. The prior attempt
did not return structured output.
did not return structured output. One cause is running out of turns before
answering, so favour returning a well-supported result over exhaustive
exploration: read what you need, stop widening the search once the changed
lines are understood, and keep turns in reserve for the final response. A
smaller set of high-confidence findings beats no answer at all.

Read `.pr-review/review-input.json`, `.pr-review/review.diff`,
`.pr-review/full.diff`, `.pr-review/rubric.md`, and
Expand Down Expand Up @@ -299,6 +316,9 @@ runs:
continue-on-error: true
shell: bash
env:
# Needed because this step retires the sticky status comment when a
# round ends without publishing.
GH_TOKEN: ${{ inputs.github_identity_token || github.token }}
STATE_DIR: ${{ github.workspace }}/.pr-review
PREPARE_OUTCOME: ${{ steps.prepare.outcome }}
COMPOSE_OUTCOME: ${{ steps.compose.outcome }}
Expand Down
133 changes: 120 additions & 13 deletions .github/actions/claude-pr-review/review_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,19 @@ def prepare(args: argparse.Namespace) -> None:
"pipeline_version": pipeline_version,
"rubric_version": rubric_version,
}
if mode != "skip":
# Announce the round before analysis so the PR carries a status comment
# from the start rather than staying silent until the review lands.
# Skip mode publishes nothing, so announcing it would strand the
# comment at "in progress".
input_value["sticky_comment_id"] = set_sticky_phase(
repository=args.repository,
pull_request=args.pull_request,
manifest=manifest,
sticky_comment_id=sticky_comment_id,
scope_text=scope_label(input_value),
phase="in_progress",
)
(state_dir / "review-input.json").write_text(
json.dumps(input_value, indent=2, sort_keys=True) + "\n",
encoding="utf-8",
Expand Down Expand Up @@ -2317,21 +2330,48 @@ def render_status_body(
current head or the moment it first appeared.
"""
summary = payload.get("status_summary") or {}
phase = str(summary.get("phase") or "complete")
scope_text = summary.get("scope_text", "the current head")
findings = (manifest.get("findings") or {}).values()
open_findings = [
item
for item in findings
if isinstance(item, dict) and item.get("status") == "open"
]
pending_questions = open_questions(manifest)
if open_findings:
status_title = f"⚠️ {len(open_findings)} open finding(s)"
if pending_questions:
status_title += f" · {len(pending_questions)} open question(s)"
elif pending_questions:
status_title = f"❓ {len(pending_questions)} open question(s)"
if phase == "in_progress":
status_title = "🔄 Review in progress"
progress_line = f"Reviewing {scope_text} · started {utc_now()}"
detail_line = (
"Findings and questions from this round appear here, and in a"
" review below, once the round finishes. Anything listed below is"
" carried over from earlier rounds."
)
elif phase == "failed":
status_title = "🛠️ Review did not finish"
reason = public_text(summary.get("reason"), maximum=200)
progress_line = f"Attempted {scope_text} · updated {utc_now()}"
detail_line = (
f"This round did not publish{': ' + reason if reason else ''}."
" Anything listed below is from the last round that did. Re-run"
" the workflow or push a new commit to try again."
)
else:
status_title = "✅ Review clean"
if open_findings:
status_title = f"⚠️ {len(open_findings)} open finding(s)"
if pending_questions:
status_title += f" · {len(pending_questions)} open question(s)"
elif pending_questions:
status_title = f"❓ {len(pending_questions)} open question(s)"
else:
status_title = "✅ Review clean"
progress_line = f"Last reviewed: {scope_text} · updated {utc_now()}"
detail_line = (
f"New this round: {summary.get('new_findings', 0)} finding(s),"
f" {summary.get('new_questions', 0)} question(s)"
f" · Resolved this round: {summary.get('resolved_findings', 0)}"
f" · Open questions: {len(pending_questions)}"
)
lines = [
"## Claude review status",
"",
Expand All @@ -2343,13 +2383,9 @@ def render_status_body(
"",
status_title,
"",
f"Last reviewed: {summary.get('scope_text', 'the current head')}"
f" · updated {utc_now()}",
progress_line,
"",
f"New this round: {summary.get('new_findings', 0)} finding(s),"
f" {summary.get('new_questions', 0)} question(s)"
f" · Resolved this round: {summary.get('resolved_findings', 0)}"
f" · Open questions: {len(pending_questions)}",
detail_line,
]
if pending_questions:
lines.extend(["", "Open questions awaiting an answer:"])
Expand All @@ -2361,6 +2397,38 @@ def render_status_body(
return "\n".join(lines)


def set_sticky_phase(
*,
repository: str,
pull_request: int,
manifest: dict[str, Any],
sticky_comment_id: int | None,
scope_text: str,
phase: str,
reason: str | None = None,
) -> int | None:
"""Move the sticky status comment to a non-publishing phase.

Used to announce a round before analysis starts and to close it out when a
round ends without publishing. Best effort: the status comment is a
courtesy, so a GitHub failure here must never fail the review.
"""
payload = {
"repository": repository,
"pull_request": pull_request,
"sticky_comment_id": sticky_comment_id,
"status_summary": {
"scope_text": scope_text,
"phase": phase,
"reason": reason,
},
}
try:
return upsert_sticky(payload, manifest)
except PipelineError:
return sticky_comment_id


def annotate_questions(
payload: dict[str, Any],
manifest: dict[str, Any],
Expand Down Expand Up @@ -2609,6 +2677,30 @@ def render_outcomes(outcomes: dict[str, str]) -> str:
return "\n".join(rows)


def close_out_sticky(state_dir: Path, *, reason: str | None = None) -> None:
"""Move an announced-but-unpublished round out of the in-progress phase.

`prepare` announces every round, so any path that ends without publishing
has to retire the comment itself; otherwise it reads as a review that is
still running hours later.
"""
review_input = read_json_file(state_dir / "review-input.json")
if not review_input:
return
manifest = review_input.get("manifest")
if not isinstance(manifest, dict):
return
set_sticky_phase(
repository=str(review_input["repository"]),
pull_request=int(review_input["pull_request"]),
manifest=manifest,
sticky_comment_id=review_input.get("sticky_comment_id"),
scope_text=scope_label(review_input),
phase="failed",
reason=reason,
)


def report(args: argparse.Namespace) -> None:
state_dir = Path(args.state_dir)
outcomes = step_outcomes()
Expand Down Expand Up @@ -2647,6 +2739,10 @@ def report(args: argparse.Namespace) -> None:
+ details
)
write_job_summary(body)
close_out_sticky(
state_dir,
reason=f"{diagnostic['code']} in phase {diagnostic['phase']}",
)
print(
"Claude PR review: FAILED "
f"[{diagnostic['code']}] phase={diagnostic['phase']}: "
Expand All @@ -2668,6 +2764,17 @@ def report(args: argparse.Namespace) -> None:
stale = os.environ.get("PUBLISH_STALE", "") == "true"
published = os.environ.get("PUBLISH_PUBLISHED", "") == "true"
retry_used = outcomes.get("review_retry") not in {"", "skipped"}
if not published and mode != "skip":
# prepare left the comment at "in progress"; nothing published, so no
# later step will move it. Close it out here or it stays there.
close_out_sticky(
state_dir,
reason=(
"the PR head changed while the review was running"
if stale
else None
),
)
if stale:
status = "⚠️ Review output was discarded because the PR head changed."
elif mode == "skip":
Expand Down
Loading
Loading