ci: Fix several issues in claude-review workflow

Address feedback from facebook/bpfilter#472:

- Fix setFailed error message counting file-level comments (without
  line numbers) that are intentionally skipped, use inlineComments.length
  instead of comments.length
- Fix double severity prefix in inline comments: the prompt told Claude
  to prefix body with **must-fix**/etc but the post job also prepended
  "Claude: ", producing "Claude: **must-fix**: ...". Now the prompt says
  not to prefix and the post job adds "Claude **severity**: " using the
  structured severity field
- Move error tracking instructions to a top-level section after all phases
  so they apply to all runs, not just the first run
- Clarify that line is optional: use "should be" instead of "must be"
  and document that omitting line still surfaces the comment in the
  tracking comment summary
- Distinguish cancelled vs failed in tracking comment message
- Add side: "RIGHT" and subject_type: "line" to createReviewComment
  per GitHub API recommendations
- Downgrade partial inline comment posting failures to warnings; only
  fail the job when no comments at all could be posted

Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>

diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml
index 160bab85c4965a31602dce8c9029769dd14002b7..293aef3b6a2de6e32af4b0fbb92dc55b1a1927a4 100644 (file)
--- a/.github/workflows/claude-review.yml
+++ b/.github/workflows/claude-review.yml
@@ -209,7 +209,7 @@ jobs:
               HEAD SHA: ${{ needs.setup.outputs.head_sha }}
 
               You are a code reviewer for the ${{ github.repository }} project. Review this pull request and
-              produce a structured JSON result containing your review comments. Do NOT attempt
+              produce a structured JSON result containing your review. Do NOT attempt
               to post comments yourself — just return the JSON. You are in the upstream repo
               without the patch applied. Do not apply it.
 
@@ -251,12 +251,13 @@ jobs:
               Give each subagent the PR title, description, full patch, and the list of changed files.
 
               Each subagent must return a JSON array of issues:
-              `[{"file": "path", "line": <number>, "severity": "must-fix|suggestion|nit", "body": "..."}]`
+              `[{"file": "path", "line": <number> (optional), "severity": "must-fix|suggestion|nit", "body": "..."}]`
 
-              `line` must be a line number from the NEW side of the diff **that appears inside
+              `line` should be a line number from the NEW side of the diff **that appears inside
               a diff hunk** (i.e. a line that is shown in the patch output). GitHub's review
               comment API rejects lines outside the diff context, so never reference lines
-              that are not visible in the patch.
+              that are not visible in the patch. If you cannot determine a valid diff line,
+              omit `line` — the comment will still appear in the tracking comment summary.
 
               Each subagent MUST verify its findings before returning them:
               - For style/convention claims, check at least 3 existing examples in the codebase to confirm
@@ -273,8 +274,9 @@ jobs:
                  comment if one already exists on the same file and line about the same problem.
                  Also check for author replies that dismiss or reject a previous comment — do NOT
                  re-raise an issue the PR author has already responded to disagreeing with.
-              4. Prefix ALL comment bodies with a severity tag: `**must-fix**: `, `**suggestion**: `,
-                 or `**nit**: `.
+              4. Do NOT prefix `body` with a severity tag — the severity is already
+                 captured in the `severity` field and will be added automatically when
+                 posting inline comments.
               5. Write a `summary` field in markdown for a top-level tracking comment.
 
                  **If no existing tracking comment was found (first run):**
@@ -298,13 +300,6 @@ jobs:
                  Omit empty sections. Each checkbox item must correspond to an entry in `comments`.
                  If there are no issues at all, write a short message saying the PR looks good.
 
-                 Throughout all phases, track any errors that prevented you from doing
-                 your job fully: permission denials (403, "Resource not accessible by
-                 integration"), tools that were not available, rate limits, or any other
-                 failures that degraded the review quality. If there were any, append a
-                 `### Errors` section listing each failed tool/action and the error
-                 message, so maintainers can fix the workflow configuration.
-
                  **If an existing tracking comment was found (subsequent run):**
                  Use the existing comment as the starting point. Preserve the order and wording
                  of all existing items. Then apply these updates:
@@ -318,6 +313,15 @@ jobs:
                    in the appropriate severity section, after the existing items.
                  - Do NOT reorder, reword, or remove existing items.
 
+              ## Error tracking
+
+              Throughout all phases, track any errors that prevented you from doing
+              your job fully: permission denials (403, "Resource not accessible by
+              integration"), tools that were not available, rate limits, or any other
+              failures that degraded the review quality. If there were any, append a
+              `### Errors` section to the summary listing each failed tool/action and
+              the error message, so maintainers can fix the workflow configuration.
+
               ## CRITICAL: Return structured JSON output
 
               Your FINAL action must be to return a JSON object matching the following
@@ -361,11 +365,12 @@ jobs:
             /* If the review job failed or was cancelled, update the tracking
              * comment to reflect that and bail out. */
             if (process.env.REVIEW_RESULT !== "success") {
+              const verb = process.env.REVIEW_RESULT === "cancelled" ? "was cancelled" : "failed";
               await github.rest.issues.updateComment({
                 owner,
                 repo,
                 comment_id: commentId,
-                body: `Claude review failed — see [workflow run](${runUrl}) for details.\n\n${MARKER}`,
+                body: `Claude review ${verb} — see [workflow run](${runUrl}) for details.\n\n${MARKER}`,
               });
               core.setFailed("Review job did not succeed.");
               return;
@@ -412,7 +417,9 @@ jobs:
                   commit_id: headSha,
                   path: c.file,
                   line: c.line,
-                  body: `Claude: ${c.body}`,
+                  side: "RIGHT",
+                  subject_type: "line",
+                  body: `Claude: **${c.severity}**: ${c.body}`,
                 });
                 posted++;
               } catch (e) {
@@ -429,8 +436,6 @@ jobs:
             else
               console.log("No inline comments to post.");
 
-            const failed = inlineComments.length > 0 && posted < inlineComments.length;
-
             /* Update the tracking comment with Claude's summary. */
             if (!summary)
               summary = "Claude review: no issues found :tada:\n\n" + MARKER;
@@ -447,5 +452,7 @@ jobs:
 
             console.log("Tracking comment updated successfully.");
 
-            if (failed)
-              core.setFailed(`Failed to post ${comments.length - posted}/${comments.length} inline comment(s).`);
+            if (inlineComments.length > 0 && posted === 0)
+              core.setFailed(`Could not post any of ${inlineComments.length} inline comment(s) — see warnings above.`);
+            else if (posted < inlineComments.length)
+              core.warning(`${inlineComments.length - posted}/${inlineComments.length} inline comment(s) could not be posted.`);