fix(github-app): reduce PR review noise#503
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds early action filtering to the GitHub webhook handler, constrains PR review comments to added diff lines only, switches from bundled review requests to individual per-comment POST submissions, and introduces summary-only handling for dependency findings in check-run artifacts. ChangesGitHub Webhook and PR Review Pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/github_app/review.rs (2)
259-288: 🩺 Stability & Availability | 💤 Low valuePartial comment posting on mid-loop HTTP failure.
If the loop fails partway through (e.g., comment 3 of 5 returns a 4xx/5xx), the function returns an error immediately, but the earlier comments remain posted. The caller then proceeds to delete the old foxguard comments, leaving the PR with a partial set of new comments and no easy way to recover.
Consider either:
- Collecting errors and continuing to post remaining comments, returning a partial-success outcome.
- Tracking successfully posted comment IDs and deleting them on failure before returning.
Given the existing best-effort semantics and that stale comments get cleaned up on the next successful run, this may be acceptable for now—but worth documenting or revisiting if partial states cause user confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/github_app/review.rs` around lines 259 - 288, The loop in post_review_comments posts comments one-by-one and aborts on the first HTTP error, leaving earlier comments posted; update post_review_comments to track successfully posted comment IDs (parse the POST response JSON for the "id" after each .send().await?.error_for_status() call) and, if any subsequent POST fails, iterate those IDs and DELETE each comment via the same GitHub API client (use the same bearer_auth and headers against "repos/{owner}/{name}/pulls/comments/{id}" or the appropriate comment delete endpoint), swallowing/logging deletion errors but attempting best-effort rollback before returning the original error; ensure commit_id injection and URL construction logic (RepositoryPath parsing, self.endpoint, and foxguard ignore) remain unchanged.
451-472: 📐 Maintainability & Code Quality | 💤 Low valueDuplicate patch-parsing logic with
src/report/github_pr.rs.
added_lines_from_patchhere is nearly identical tocommentable_lines_from_patchingithub_pr.rs. Consider extracting a shared helper (e.g., in a common module) to avoid divergence over time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/github_app/review.rs` around lines 451 - 472, Extract the duplicate patch-parsing logic into a shared helper (e.g., parse_added_lines_from_patch) and replace both added_lines_from_patch and commentable_lines_from_patch with calls to that helper; specifically, move the loop that parses hunks, tracks new_line via hunk_new_start, and collects added line numbers into a common utility function, keep the same signature semantics (Option<&str> -> Option<HashSet<usize>>), and update the callers in review.rs (added_lines_from_patch) and github_pr.rs (commentable_lines_from_patch) to call the new helper to avoid duplicated code and future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/github_app/review.rs`:
- Around line 259-288: The loop in post_review_comments posts comments
one-by-one and aborts on the first HTTP error, leaving earlier comments posted;
update post_review_comments to track successfully posted comment IDs (parse the
POST response JSON for the "id" after each .send().await?.error_for_status()
call) and, if any subsequent POST fails, iterate those IDs and DELETE each
comment via the same GitHub API client (use the same bearer_auth and headers
against "repos/{owner}/{name}/pulls/comments/{id}" or the appropriate comment
delete endpoint), swallowing/logging deletion errors but attempting best-effort
rollback before returning the original error; ensure commit_id injection and URL
construction logic (RepositoryPath parsing, self.endpoint, and foxguard ignore)
remain unchanged.
- Around line 451-472: Extract the duplicate patch-parsing logic into a shared
helper (e.g., parse_added_lines_from_patch) and replace both
added_lines_from_patch and commentable_lines_from_patch with calls to that
helper; specifically, move the loop that parses hunks, tracks new_line via
hunk_new_start, and collects added line numbers into a common utility function,
keep the same signature semantics (Option<&str> -> Option<HashSet<usize>>), and
update the callers in review.rs (added_lines_from_patch) and github_pr.rs
(commentable_lines_from_patch) to call the new helper to avoid duplicated code
and future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0047492-3c12-4ea2-8ae4-947477b0a83c
📒 Files selected for processing (3)
src/bin/foxguard_github_app.rssrc/github_app/review.rssrc/report/github_pr.rs
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests