auto-claude: 155-fix-pr-list-diff-display-metrics #1458
Conversation
…te listPRs
- Add githubGraphQL helper function for making GraphQL API requests
- Add GraphQLPRListResponse interface for type safety
- Add LIST_PRS_QUERY GraphQL query to fetch PRs with additions/deletions/changedFiles
- Update listPRs handler to use GraphQL API instead of REST
- Import normalizeRepoReference from utils to parse owner/repo
The REST API /repos/{owner}/{repo}/pulls does NOT return diff stats
(additions/deletions/changedFiles). Only individual PR endpoints include
these fields. Switching to GraphQL solves this in a single request.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughPR listing migrates from REST/paginated loads to a single GitHub GraphQL query fetching up to 100 open PRs with extended metadata. The public API returns a Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant IPC as Main IPC Handler
participant GraphQL as GitHub GraphQL API
Component->>IPC: invokeIpc(GITHUB_PR_LIST, projectId)
IPC->>IPC: normalizeRepoReference(projectId) / validate repo
IPC->>GraphQL: LIST_PRS_QUERY (owner, repo, first:100, states: OPEN, orderBy: UPDATED_AT)
GraphQL-->>IPC: nodes[] + pageInfo{hasNextPage}
IPC->>IPC: map nodes -> PRData[] (additions,deletions,changedFiles,assignees,refs,timestamps)
IPC-->>Component: PRListResult { prs: PRData[], hasNextPage: boolean }
Component->>Component: render PR list and status (first 100 / all loaded)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
github-actions
bot
added
area/frontend
Summary of ChangesHello @AndyMik90, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses the problem of inaccurate addition/deletion metrics in the GitHub PR list by migrating from the REST API to the GraphQL API. This change ensures that the UI displays the correct diff statistics for pull requests. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully migrates the GitHub PR listing functionality from the REST API to the GraphQL API to retrieve diff statistics (additions, deletions, changed files). This change introduces new GraphQL types, a dedicated GraphQL client function, and the GraphQL query itself. The listPRs handler has been updated to use this new GraphQL functionality, including repository reference normalization and robust error handling. The change improves the accuracy of PR list metrics.
| // GraphQL pagination uses cursor-based "after", simulate page-based by fetching first N*page | ||
| // For simplicity, we fetch 100 items for page 1, and handle pagination client-side |
There was a problem hiding this comment.
The comment states, "GraphQL pagination uses cursor-based 'after', simulate page-based by fetching first N*page," but the current implementation only fetches the first 100 items (first: 100, after: null). If the intention is to truly simulate page-based pagination beyond the first 100 items, the after cursor should be utilized to fetch subsequent pages. If only the first 100 PRs are intended to be fetched, the comment should be updated to reflect this more accurately.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // GraphQL pagination uses cursor-based "after", simulate page-based by fetching first N*page | ||
| // For simplicity, we fetch 100 items for page 1, and handle pagination client-side | ||
| const response = await githubGraphQL<GraphQLPRListResponse>( | ||
| config.token, | ||
| `/repos/${config.repo}/pulls?state=open&per_page=100&page=${page}` | ||
| )) as Array<{ | ||
| number: number; | ||
| title: string; | ||
| body?: string; | ||
| state: string; | ||
| user: { login: string }; | ||
| head: { ref: string }; | ||
| base: { ref: string }; | ||
| additions: number; | ||
| deletions: number; | ||
| changed_files: number; | ||
| assignees?: Array<{ login: string }>; | ||
| created_at: string; | ||
| updated_at: string; | ||
| html_url: string; | ||
| }>; | ||
| LIST_PRS_QUERY, | ||
| { | ||
| owner, | ||
| repo, | ||
| first: 100, // GitHub GraphQL max is 100 | ||
| after: null, // First page | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| first: 100, // GitHub GraphQL max is 100 | ||
| after: null, // First page | ||
| } | ||
| ); |
There was a problem hiding this comment.
Pagination broken: page parameter always ignored
High Severity
The page parameter accepted by the listPRs handler is completely ignored, causing all pagination requests to return the same first 100 PRs. The GraphQL after cursor is hardcoded to null regardless of the requested page, breaking "Load More" functionality. Repositories with more than 100 open PRs cannot display PRs beyond the first page, making them inaccessible through the UI.
| } | ||
| ); | ||
|
|
||
| const prs = response.data.repository.pullRequests.nodes; |
There was a problem hiding this comment.
Null repository causes crash without error handling
High Severity
Accessing response.data.repository.pullRequests.nodes without checking if repository is null causes a crash. GitHub's GraphQL API returns repository: null (without an error) when the repo doesn't exist, is private, or the token lacks access. This crashes the handler with "Cannot read property 'pullRequests' of null" instead of returning an empty array like the previous REST API implementation.
| additions: pr.additions, | ||
| deletions: pr.deletions, | ||
| changedFiles: pr.changedFiles, | ||
| assignees: pr.assignees.nodes.map((a) => ({ login: a.login })), |
There was a problem hiding this comment.
Crash when assignee user account is deleted
Medium Severity
The assignees mapping crashes with "Cannot read property 'login' of null" when any assignee user account has been deleted or suspended. In GraphQL connections, the nodes array can contain null entries for deleted entities, but the code directly maps without filtering nulls: pr.assignees.nodes.map((a) => ({ login: a.login })). This breaks PR list loading whenever a PR has a deleted user as an assignee.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| headers: { | ||
| "Authorization": `Bearer ${token}`, | ||
| "Content-Type": "application/json", | ||
| "User-Agent": "Auto-Claude-UI", | ||
| }, |
Check warning
Code scanning / CodeQL
File data in outbound network request Medium
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
🟠 Needs revision - 2 issue(s) require attention.
2 issue(s) must be addressed (2 required, 0 recommended)
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 2 issue(s)
Generated by Auto Claude PR Review
Findings (2 selected of 2 total)
🟠 [75c3b5c1d263] [HIGH] [Potential] Page parameter accepted but ignored - pagination broken
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1360
The handler accepts a page parameter (line 1332) but the GraphQL call always uses first: 100, after: null, meaning only the first 100 PRs are ever returned regardless of page value. Comment claims 'simulate page-based by fetching first N*page' but this is not implemented. Clients requesting page=2 get identical results to page=1.
Suggested fix:
Either implement cursor-based pagination by tracking cursors across calls, or remove the `page` parameter from the handler signature. If keeping client-side pagination, document that all PRs are fetched at once and sliced client-side.
🟠 [43fa7203d78d] [HIGH] [Potential] Missing null check for repository response causes crash
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1364
The code accesses response.data.repository.pullRequests.nodes without checking if repository is null. GitHub GraphQL API returns repository: null (not an error) when the repository doesn't exist or user lacks access. This will cause TypeError: Cannot read properties of null (reading 'pullRequests').
Suggested fix:
Add null check: `if (!response.data.repository) { debugLog('Repository not found or access denied', { owner, repo }); return []; }`
This review was generated by Auto Claude.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 2 unresolved finding(s) from previous review.
Resolution Status
- ✅ Resolved: 0 previous findings addressed
- ❌ Unresolved: 2 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 2 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] [Potential] Page parameter accepted but ignored - pagination broken
- quality: [UNRESOLVED] [Potential] Missing null check for repository response causes crash
- quality: Type definition does not match GitHub GraphQL reality
- quality: [FROM COMMENTS] Pagination parameter ignored (Sentry concern)
Verdict
All 20 CI checks passing. However, 2 HIGH severity findings from the previous review remain UNRESOLVED and have been CONFIRMED_VALID by the finding-validator with strong cross-agent agreement: (1) Page parameter is accepted but ignored - pagination is broken for repos with more than 100 open PRs, (2) Missing null check on response.data.repository causes crash when repository does not exist. The merge commit from develop did not address either issue. Both issues are easily fixable: add null check for repository, and either implement cursor pagination or remove the misleading page parameter.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, comment-analyzer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (4 selected of 4 total)
🟠 [75c3b5c1d263] [HIGH] [UNRESOLVED] [Potential] Page parameter accepted but ignored - pagination broken
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1360
The handler accepts a page parameter (line 1332) but the GraphQL call always uses first: 100, after: null, meaning only the first 100 PRs are ever returned regardless of page value. Comment claims 'simulate page-based by fetching first N*page' but this is not implemented. Clients requesting page=2 get identical results to page=1.
Resolution note: const response = await githubGraphQL(
config.token,
LIST_PRS_QUERY,
{
owner,
repo,
first: 100,
after: null, // First page - hardcoded regardless of page parameter
}
);
Suggested fix:
Either implement cursor-based pagination by tracking cursors across calls, or remove the `page` parameter from the handler signature. If keeping client-side pagination, document that all PRs are fetched at once and sliced client-side.
🟠 [43fa7203d78d] [HIGH] [UNRESOLVED] [Potential] Missing null check for repository response causes crash
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1364
The code accesses response.data.repository.pullRequests.nodes without checking if repository is null. GitHub GraphQL API returns repository: null (not an error) when the repository doesn't exist or user lacks access. This will cause TypeError: Cannot read properties of null (reading 'pullRequests').
Resolution note: const prs = response.data.repository.pullRequests.nodes;
Suggested fix:
Add null check: `if (!response.data.repository) { debugLog('Repository not found or access denied', { owner, repo }); return []; }`
🟡 [NEW-003] [MEDIUM] Type definition does not match GitHub GraphQL reality
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:43
The GraphQLPRListResponse interface defines repository as a required object, but GitHub GraphQL API can return repository: null when the repo does not exist or is inaccessible. This type mismatch means TypeScript will not flag the unsafe access at line 1364.
Suggested fix:
Update the type to reflect nullable repository: repository: { pullRequests: { ... } } | null
🟠 [CMT-001] [HIGH] [FROM COMMENTS] Pagination parameter ignored (Sentry concern)
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1360
Sentry correctly identified the same issue as finding 75c3b5c1d263 - the page parameter is accepted but the GraphQL call always uses after: null, making pagination non-functional.
Suggested fix:
Implement cursor-based pagination or remove misleading page parameter
This review was generated by Auto Claude.
- Add null check for response.data.repository before accessing pullRequests to prevent crash when repo doesn't exist or user lacks access - Update GraphQLPRListResponse type to make repository nullable - Remove misleading page parameter from listPRs handler since it was never used (always fetched first 100 PRs regardless of page value) - Update preload API and hook to match new signature Fixes PR review findings: - Missing null check causes crash on non-existent repos - Page parameter accepted but ignored breaks pagination Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 2 blocking issue(s) require fixes.
Resolution Status
- ✅ Resolved: 4 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
🚨 Blocking Issues
- quality: Interface signature mismatch: listPRs still accepts optional page param
- quality: loadMore function is effectively non-functional after API change
Verdict
✅ CI Status: All 20 CI checks passing - no blockers from CI.
✅ Previous Findings: All 4 previous findings (2 HIGH pagination issues, 1 HIGH null check, 1 MEDIUM type mismatch) have been properly resolved. The commit message accurately describes the fixes: null check added and unused page parameter removed.
❌ New Issues Found: 2 MEDIUM severity issues introduced:
- Interface mismatch (NEW-001): The
GitHubAPI.listPRsinterface still declares an optionalpageparameter that the implementation ignores - this is misleading to API consumers. - Dead code (NEW-002): The
loadMorefunction is now non-functional since the API fetches all PRs at once without cursor-based pagination.
Why NEEDS_REVISION: Per quality guidelines, both HIGH and MEDIUM severity findings block merge. These are straightforward fixes: (1) update the interface signature to remove page, and (2) either implement cursor-based pagination or remove the dead loadMore functionality. The core fixes are solid - these are cleanup items from an incomplete refactor."
Review Process
Agents invoked: resolution-verifier, new-code-reviewer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (2 selected of 2 total)
🟡 [NEW-001] [MEDIUM] Interface signature mismatch: listPRs still accepts optional page param
📁 apps/frontend/src/preload/api/modules/github-api.ts:266
The GitHubAPI interface defines listPRs with an optional page parameter (page?: number), but the implementation at line 656 no longer uses this parameter. API consumers might pass a page parameter expecting pagination to work, but it will be silently ignored.
Suggested fix:
Update the interface at line 266 to: `listPRs: (projectId: string) => Promise<PRData[]>;` to match the implementation and clearly communicate that page-based pagination is no longer supported.
🟡 [NEW-002] [MEDIUM] loadMore function is effectively non-functional after API change
📁 apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts:408
The loadMore function calls fetchPRs(currentPage + 1, true) to fetch additional PRs, but the underlying API now fetches all PRs (up to 100) in a single call without cursor-based pagination. Calling loadMore triggers a network request that returns identical data, resulting in a no-op that still shows loading states.
Suggested fix:
Either (1) implement cursor-based pagination using the GraphQL `endCursor` from `pageInfo` to actually fetch more PRs, or (2) remove the loadMore functionality entirely since the API now fetches all open PRs at once. If removing, also clean up: hasMore state, currentPage state, isLoadingMore state, and the append logic in fetchPRs.
This review was generated by Auto Claude.
- Remove unused page parameter from listPRs interface signature (NEW-001) - Remove non-functional loadMore functionality since API fetches all PRs at once (NEW-002) - Remove isLoadingMore, currentPage state and related infinite scroll code - Simplify PRList component by removing unused pagination props - Update UI to show "Showing first 100 PRs" when GitHub GraphQL limit is hit - Clean up unused imports (useRef, useEffect, useCallback, Loader2) The API fetches up to 100 open PRs in a single call (GitHub GraphQL limit). The loadMore function was triggering redundant network requests returning identical data. This cleanup removes the dead code from the incomplete pagination refactor. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 3 new issue(s) found in recent changes.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 3 new findings in recent changes
Verdict
CI Status: 14 checks still pending - cannot approve until CI completes. Code Quality: Both previous findings (NEW-001 interface mismatch, NEW-002 non-functional loadMore) are fully RESOLVED. The fix commit correctly removes dead pagination code throughout the stack. 3 new LOW severity quality issues found (unused state variable, verbose error messages, unused GraphQL pageInfo) - these are optional suggestions that do not block merge. Once CI passes, this PR should be READY_TO_MERGE.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (3 selected of 3 total)
🔵 [QUAL-001] [LOW] Unused state variable after pagination removal
📁 apps/frontend/src/renderer/components/github-prs/components/PRList.tsx:206
The viewportElement state variable is declared and setViewportElement is passed to onViewportRef, but the state value is never read or used in the component. This is dead code left over from the removed infinite scroll functionality.
🔵 [QUAL-002] [LOW] Error response body included in error message
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:91
When a GitHub GraphQL request fails, the entire error response body is included in the thrown error message. While GitHub API errors typically do not contain sensitive data, including raw API response bodies makes logs verbose.
🔵 [QUAL-003] [LOW] GraphQL pageInfo queried but discarded
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:47
The GraphQL query includes pageInfo with hasNextPage and endCursor but this information is never used. The frontend uses a heuristic instead. The actual hasNextPage field from the API would be more accurate.
This review was generated by Auto Claude.
- QUAL-001: Remove unused viewportElement state and onViewportRef prop
from PRList after pagination removal
- QUAL-002: Simplify GraphQL error message by removing verbose response body
- QUAL-003: Use actual hasNextPage from GitHub API instead of length heuristic
- Add PRListResult interface with { prs, hasNextPage }
- Update IPC handler to return pageInfo.hasNextPage from API
- Update hook to use result.hasNextPage instead of result.length === 100
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts (1)
149-200: Clear PR list + hasMore when disconnected or fetch fails.Right now, a connection failure leaves stale PRs/hasMore from the previous successful fetch, which can mislead users after disconnects or auth errors.
💡 Suggested fix
if (connectionResult.success && connectionResult.data) { setIsConnected(connectionResult.data.connected); setRepoFullName(connectionResult.data.repoFullName || null); - if (connectionResult.data.connected) { + if (connectionResult.data.connected) { // Fetch PRs (returns up to 100 open PRs at once - GitHub GraphQL limit) const result = await window.electronAPI.github.listPRs(projectId); if (result) { // Use hasNextPage from API to determine if more PRs exist setHasMore(result.hasNextPage); setPrs(result.prs); @@ } + } else { + setHasMore(false); + setPrs([]); } } else { setIsConnected(false); setRepoFullName(null); + setHasMore(false); + setPrs([]); setError(connectionResult.error || "Failed to check connection"); } } catch (err) { setError(err instanceof Error ? err.message : "Failed to fetch PRs"); setIsConnected(false); + setHasMore(false); + setPrs([]); } finally { setIsLoading(false); }
♻️ Duplicate comments (1)
apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts (1)
39-66: Guard against nullable assignee nodes to avoid crashes.GitHub GraphQL connections can return null entries for deleted/suspended users; mapping
nodesdirectly can throw at runtime. Consider filtering nulls and loosening the response type.🔧 Suggested fix (null-safe assignee mapping)
- assignees: pr.assignees.nodes.map((a) => ({ login: a.login })), + assignees: pr.assignees.nodes + .filter((a): a is { login: string } => Boolean(a?.login)) + .map((a) => ({ login: a.login })),Does GitHub GraphQL return null entries in assignees.nodes when a user is deleted/suspended? (GraphQL connection node nullability)Also applies to: 1377-1393
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 2 new issue(s) found in recent changes.
Resolution Status
- ✅ Resolved: 3 previous findings addressed
- ❌ Unresolved: 0 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Verdict
CI Status: ⏳ 1 CI check still pending - cannot approve until CI completes.
Code Quality: Excellent progress. All 3 previous findings (QUAL-001, QUAL-002, QUAL-003) are fully RESOLVED. The commit successfully:
- Removed dead viewportElement state and IntersectionObserver code
- Fixed error handling to not expose response bodies
- Properly uses API's hasNextPage instead of heuristic
New Findings: 2 LOW severity suggestions identified (NEW-001, NEW-002). One medium-severity finding (NEW-003 about silent error handling) was dismissed as out of scope - the error-returns-empty pattern is pre-existing behavior unchanged by this PR.
Recommendation: Once CI passes, this PR is ready to merge. The remaining 2 low-severity suggestions are optional polish items that can be addressed in a follow-up if desired.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (2 selected of 2 total)
🔵 [NEW-001] [LOW] Consider generic error messages for user-facing errors
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:91
The githubGraphQL function exposes HTTP status codes and GraphQL error messages in exceptions. While this is standard practice and the fix is better than before (no longer includes response body), consider using more generic user-friendly messages like 'Failed to connect to GitHub' for the UI while logging details server-side.
Suggested fix:
Consider wrapping errors with user-friendly messages at the IPC boundary
🔵 [NEW-002] [LOW] Owner/repo split could be stricter for malformed input
📁 apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts:1351
The code splits normalized repo reference by '/' and takes first two parts. If normalizeRepoReference returns a path with extra segments (e.g., 'owner/repo/extra'), only the first two are used silently. While unlikely in practice, stricter validation could help debugging.
Suggested fix:
Validate split produces exactly 2 parts before proceeding
This review was generated by Auto Claude.
- Use generic error messages in exceptions while logging details for debugging - Add stricter validation for owner/repo format (must be exactly 2 parts) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| "Content-Type": "application/json", | ||
| "User-Agent": "Auto-Claude-UI", | ||
| }, | ||
| body: JSON.stringify({ query, variables }), |
Check warning
Code scanning / CodeQL
File data in outbound network request Medium
AndyMik90
deleted the
auto-claude/155-fix-pr-list-diff-display-metrics
branch
Fix GitHub PR list showing
0 / 0for additions/deletions by switching to GraphQL API which includes diff stats. The GitHub REST API/repos/{owner}/{repo}/pullsdoes NOT returnadditions,deletions, orchangedFilesfields - these are only available when fetching individual PRs.Summary by CodeRabbit
New Features
Bug Fixes / UX
✏️ Tip: You can customize this high-level summary in your review settings.