Skip to content

Fixes for compare/pr page. - #166

Merged
stylessh merged 1 commit into
mainfrom
fix/compare-pr-review
Apr 21, 2026
Merged

Fixes for compare/pr page.#166
stylessh merged 1 commit into
mainfrom
fix/compare-pr-review

Conversation

@stylessh

@stylesshstylessh commented Apr 21, 2026

Copy link
Copy Markdown
Owner

Summary

address issues raised by coderabbit

Summary by CodeRabbit

  • New Features

    • Added warning notifications when pull request creation includes partial or failed operations.
    • Added warning when comparison includes more files than displayed to the user.
  • Bug Fixes

    • Improved skeleton loading states during branch comparisons.
    • Fixed "ahead/behind" summary formatting in comparison view.
    • Optimized comparison queries to skip unnecessary operations.
  • Improvements

    • Added "Nothing to compare" UI state for clarity when no changes exist.
    • Enhanced error handling for pull request follow-up operations.

- compare-diff-view: reset visibleCount to INITIAL on new comparison (files identity, not length)
- compare-header: "·" separator only when both ahead and behind segments render
- compare-page: distinguish pending state from null comparison; render "Nothing to compare" empty state; broaden invalidate predicate to concrete segments (pulls, overview, recentPushableBranch, branchComparison, compareDetail); surface follow-up warnings via toast
- compare-sidebar: PickerTrigger is now forwardRef with required label prop; removed <span> wrappers around PopoverTrigger asChild so radix attaches directly to the button
- branch-comparison-banner: same-branch early return before pending skeleton
- github.functions (createPullRequest): preserve installUrl in catch; collect Promise.allSettled rejections and return them as optional `warnings` on success instead of silently succeeding
- github.functions (getRecentPushableBranch): use user-scoped context for getViewer() and listActivities to avoid installation-token failures
- github.functions (getCompareDetail): paginate commits up to 3×100; surface files truncation (300-cap) via filesTruncated; switched to getOrRevalidateGitHubResource and bumped resource id to v2; CompareDiffView renders a yellow notice on truncation
@coderabbitai

coderabbitaiBot commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Changes introduce file truncation tracking in PR comparisons, enhanced error handling for PR creation with warning notifications, improved branch comparison logic, and refactored sidebar picker components. A new cached compare resource fetches commits across multiple pages while tracking file count against a cap. The diff view, header, and page components updated to display truncation indicators and handle warnings from PR creation.

Changes

Cohort / File(s)Summary
Compare UI Components
apps/dashboard/src/components/compare/compare-diff-view.tsx, compare-header.tsx, compare-sidebar.tsx
Added filesTruncated prop to diff view with warning notice. Adjusted ahead/behind separator rendering in header. Refactored picker trigger to forwardRef button component with label prop and dual prefetch event handling on onMouseEnter and onFocus.
Compare Page Logic
apps/dashboard/src/components/compare/compare-page.tsx
Updated PR creation cache invalidation to use explicit query-key segments. Added warning toast notifications from PR results. Improved pending/skeleton state handling for missing comparison data. Passes filesTruncated prop to CompareDiffView.
Branch Comparison
apps/dashboard/src/components/repo/branch-comparison-banner.tsx
Refactored branch matching logic to derive isSameBranch boolean, disabling query execution and rendering nothing when branches match.
GitHub API Layer
apps/dashboard/src/lib/github.functions.ts, github.types.ts
Extended CreatePullRequestResult to include optional warnings array. Replaced single-call compare with cached resource fetching commits across multiple pages while capping files and computing filesTruncated. Follow-up mutations (labels/assignees/reviewers) failures accumulated as warnings instead of failing request. Added separate user context for viewer/activity queries.

Sequence Diagram(s)

sequenceDiagram
actor User
participant CompareUI as Compare Page UI
participant APILayer as GitHub API Layer
participant GitHub as GitHub GraphQL
User->>CompareUI: Trigger comparison fetch
CompareUI->>APILayer: Call getCompareDetail()
activate APILayer
APILayer->>GitHub: Query commits (page 1)
GitHub-->>APILayer: Return commits + files (max 300)
APILayer->>APILayer: Collect files from page 1
loop Fetch remaining pages (up to max)
APILayer->>GitHub: Query commits (page N)
GitHub-->>APILayer: Return commits
APILayer->>APILayer: Skip files, only collect commits
end
APILayer->>APILayer: Compute filesTruncated (files > cap)
APILayer-->>CompareUI: Return { commits, files, filesTruncated }
deactivate APILayer
CompareUI->>CompareUI: Render diff view with filesTruncated
alt filesTruncated is true
CompareUI->>User: Display warning: "More files than shown"
end
Loading
sequenceDiagram
actor User
participant CompareUI as Compare Page UI
participant APILayer as GitHub API Layer
participant GitHub as GitHub GraphQL
User->>CompareUI: Submit create PR form
CompareUI->>APILayer: Call createPullRequest()
activate APILayer
APILayer->>GitHub: Create pull request
GitHub-->>APILayer: Return { pullNumber }
par Parallel follow-up mutations
APILayer->>GitHub: Add labels
APILayer->>GitHub: Add assignees
APILayer->>GitHub: Add reviewers
end
GitHub-->>APILayer: Results via Promise.allSettled
APILayer->>APILayer: Inspect results, collect failures as warnings
alt Any mutation failed
APILayer-->>CompareUI: Return { ok: true, pullNumber, warnings }
else All succeeded
APILayer-->>CompareUI: Return { ok: true, pullNumber }
end
deactivate APILayer
CompareUI->>CompareUI: Invalidate query caches
alt warnings exist
CompareUI->>User: Display toast with warnings
end
CompareUI->>User: Navigate to PR
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Compare / Pull Request page. #165: Modifies CompareDiffView, compare-page, and github.functions to introduce/adjust filesTruncated behavior and compare detail caching logic.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check nameStatusExplanationResolution
Description check⚠️ WarningThe description is incomplete, missing required sections. Only 'Summary' is partially filled ('address issues raised by coderabbit'); 'Changes', 'Test Plan', and 'Screenshots' sections are entirely absent.Complete the PR description by adding detailed 'Changes', 'Test Plan', and 'Screenshots' (if UI changes apply) sections following the repository template.
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check❓ InconclusiveThe title 'Fixes for compare/pr page' is vague and generic, lacking specificity about which issues were fixed or what the main change is.Provide a more specific title that describes the primary fix or feature, e.g., 'Add file truncation notice and improve compare page UI' or similar.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/compare-pr-review

Comment @coderabbitai help to get the list of available commands and usage tips.

@stylessh
stylessh merged commit f951281 into mainApr 21, 2026
4 of 5 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@stylessh