Skip to content
Open
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
24 changes: 20 additions & 4 deletions review-pr/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ runs:
echo "chunk_manifest=$(jq -nc --argjson f "$FILES" '{"1": $f}')" >> $GITHUB_OUTPUT
echo "✅ Single chunk ($TOTAL_LINES lines)"
else
# Large diff — split at file boundaries, targeting ~1000 lines per chunk
# Large diff — split at file boundaries, targeting ~600 lines per chunk
CHUNK=1
CHUNK_LINES=0
CURRENT_FILE=""
Expand All @@ -301,9 +301,9 @@ runs:
DIR=$(dirname "$FILE")

# Start new chunk if:
# - over soft limit (1000 lines) and directory changed, OR
# - over soft limit (600 lines) and directory changed, OR
# - over hard limit (2000 lines) regardless of directory
if ([ "$CHUNK_LINES" -gt 1000 ] && [ "$DIR" != "$CURRENT_DIR" ]) || [ "$CHUNK_LINES" -gt 2000 ]; then
if ([ "$CHUNK_LINES" -gt 600 ] && [ "$DIR" != "$CURRENT_DIR" ]) || [ "$CHUNK_LINES" -gt 2000 ]; then
# Save current chunk's file list to manifest
MANIFEST=$(echo "$MANIFEST" | jq --argjson files "$CHUNK_FILES" --arg k "$CHUNK" '.[$k] = $files')
CHUNK=$((CHUNK + 1))
Expand All @@ -326,7 +326,7 @@ runs:

echo "chunk_count=$CHUNK" >> $GITHUB_OUTPUT
echo "chunk_manifest=$(echo "$MANIFEST" | jq -c .)" >> $GITHUB_OUTPUT
echo "✅ Split into $CHUNK chunks (target ~1000 lines each)"
echo "✅ Split into $CHUNK chunks (target ~600 lines each)"

for i in $(seq 1 $CHUNK); do
LINES=$(wc -l < /tmp/drafter_chunk_${i}.diff | tr -d ' ')
Expand Down Expand Up @@ -806,6 +806,7 @@ runs:
run: |
REVIEW_URL="https://github.com/$REPOSITORY/pull/$PR_NUMBER"
echo "review-url=$REVIEW_URL" >> $GITHUB_OUTPUT
TIMEOUT_NOTE=""

if [ "$SKIP_REASON" = "concurrent" ]; then
# Stay silent — the 👀 reaction on the triggering comment (added by the
Expand All @@ -814,6 +815,17 @@ runs:
STATUS="⏭️ **Review skipped** — another review is already in progress"
elif [ -z "$EXIT_CODE" ]; then
STATUS="⏭️ **Review skipped** — agent did not run"
elif [ "$EXIT_CODE" = "124" ]; then
# Timeout (SIGKILL after 1800 s) — provide actionable guidance
STATUS="⏱️ **Review timed out** (exit code: 124, limit: 1800 s)"
TIMEOUT_NOTE="- **Exit code:** 124 (SIGKILL — 1800 s timeout)
- **Timeout limit:** 1800 s
- Verbose log artifact (${VERBOSE_LOG_FILE}) uploaded for debugging."
if ! gh api "repos/$REPOSITORY/issues/$PR_NUMBER/comments" \
-f body="⏱️ **PR Review Timed Out** — The review agent hit the 1800 s time limit before completing. This usually happens on large or complex diffs. You can re-trigger with \`/review\` — if it times out again, consider splitting the PR into smaller pieces." \
2>&1; then
echo "::warning::Failed to post timeout comment to PR"
fi
elif [ "$EXIT_CODE" != "0" ]; then
# Check if agent actually posted a review despite the error exit code.
# This happens when a sub-agent fails (e.g., API overload) but the root
Expand Down Expand Up @@ -841,6 +853,10 @@ runs:
echo "## PR Review Summary"
echo ""
echo "$STATUS"
if [ -n "$TIMEOUT_NOTE" ]; then
echo ""
printf '%s\n' "$TIMEOUT_NOTE"
fi
echo ""
echo "📝 [View Pull Request #$PR_NUMBER]($REVIEW_URL)"
} >> $GITHUB_STEP_SUMMARY
Expand Down
25 changes: 21 additions & 4 deletions review-pr/agents/pr-review.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ agents:
c. Delegate to the drafter once with the path `./pr-review.diff`.

**Both modes**:
d. Merge all findings arrays into a single list before proceeding.
d. Merge all findings arrays into a single list before proceeding. Each finding
object uses fields `category`, `issue`, `details`, `in_diff` — not `title`/`body`.
e. Include any relevant learned patterns from memory in each delegation.

## CRITICAL: How to delegate to the drafter
Expand All @@ -170,12 +171,26 @@ agents:
- File history: relevant entries from `/tmp/file_history.txt` (CI mode only — skip in console mode)
- Available files: run `ls` on directories of changed files so drafter knows real paths
- Learned patterns: any relevant memories

**CRITICAL — drafter response schema (schema-validated, strict):**
The drafter returns a JSON object with these exact top-level fields:
- `findings`: array of objects with fields `file`, `line`, `severity`, `category`,
`issue` (one-line summary), `details` (trigger + impact), `in_diff` (boolean)
- `summary`: string — overall assessment
- `review_complete`: boolean — true if the full chunk was reviewed

**NEVER mention `title` or `body` as finding field names in the delegation
message.** Those fields do not exist in the drafter schema. If the task string
asks for `title`/`body`, the schema validator rejects every emit attempt and the
drafter enters an infinite file-read retry loop. Always use `issue` and `details`.
5. Parse the drafter's JSON response. Check `review_complete`:
- If `review_complete` is `true` AND zero findings → skip directly to step 8 (Decision Rules).
- If `review_complete` is `false` AND zero findings → the drafter likely exhausted its context.
Post a COMMENT review noting the review was incomplete and do NOT approve.
- Otherwise, collect all findings with severity "high" or "medium"
and delegate them to the `verifier` in a single batch. Skip verification for "low" findings.
- Otherwise, collect all findings with severity "high" or "medium" — reference
each finding's one-line summary via the `issue` field and its explanation via
the `details` field (not `title`/`body`) — and delegate them to the `verifier`
in a single batch. Skip verification for "low" findings.
Include the project context (from step 2) in the verifier delegation so it can validate
findings against project-specific conventions (e.g., language version, available APIs).
**The verifier has no file access — you must provide all code context inline.**
Expand All @@ -194,7 +209,9 @@ agents:
`in_changed_code == false` or `in_diff == false`.
7. **Verify line numbers** before posting (see below)
8. Apply the Decision Rules (see below) to determine the review verdict
9. Build inline comments from CONFIRMED/LIKELY issues and post the review
9. Build inline comments from CONFIRMED/LIKELY issues using each finding's `issue`
(one-line summary), `details` (full explanation), `severity`, `category`, `file`,
and `line` fields. Post the review.
10. Always report ALL HIGH severity bugs. Limit MEDIUM/LOW to 5 comments max.

Find **real bugs in the changed code**, not style issues. If the changed code works correctly, approve it.
Expand Down
Loading