Conversation
- New Callout UI primitive with filled variants (default/info/warning/success/destructive) - Refactor Review-Requested banner onto Callout - BranchComparisonBanner on repo overview / tree routes showing ahead/behind vs default branch - New `/compare/$base...$head` route: commits list, file diffs (Unified/Split toggle), progressive file rendering (30 initial, 12-chunk load-more) - `?expand=1` renders the PR creation form + sidebar (title, markdown body, labels, assignees, reviewers + teams, split-button Create / Create draft) - New server fns: createPullRequest, getCompareDetail; query options + keys - Repo overview branch switch navigates to /tree/<branch>; tree route renders overview at root path - Markdown editor: focus ring persists during toolbar clicks, autoFocus textarea when clicking Write tab - Input transitions include border-color so focus ring animates smoothly
Shows a "Compare & pull request" callout on the repo overview when the viewing user has a recent push to a non-default branch that has no open PR. - New `getRecentPushableBranch` server fn, wired through `getOrRevalidateGitHubResource` with `repoCode`/`repoMeta` signal keys so the cache is invalidated by push webhooks - Client subscribes via `useGitHubSignalStream`, so the banner appears live when the viewer pushes
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a full compare-and-create-pull-request feature: UI components (compare view, form, header, sidebar), routing for compare splat routes, new GitHub server/query functions (compare/detail/recent/PR creation), UI primitives (Callout), and small editor/input refinements. Changes
Sequence DiagramsequenceDiagram
participant User as rgba(40,40,255,0.5)
participant Browser as rgba(0,128,0,0.5)
participant ComparePage as rgba(0,183,255,0.5)
participant CompareForm as rgba(255,153,51,0.5)
participant QueryClient as rgba(128,0,128,0.5)
participant GitHubAPI as rgba(220,20,60,0.5)
participant Router as rgba(34,139,34,0.5)
User->>Browser: Navigate to /:owner/:repo/compare/base...head?expand=1
Browser->>ComparePage: render with loader data (overview, viewer, compare)
ComparePage->>QueryClient: fetch commits/files, collaborators
ComparePage->>CompareForm: render with mention candidates & template
User->>CompareForm: fill title/body, pick reviewers/assignees/labels
User->>CompareForm: submit create PR (draft or final)
CompareForm->>GitHubAPI: createPullRequest(input)
GitHubAPI->>GitHubAPI: create PR, add labels/assignees, request reviewers
GitHubAPI-->>CompareForm: return PR result (number/url)
CompareForm->>QueryClient: invalidate pulls & repo meta keys
QueryClient->>ComparePage: refetch updated data
CompareForm->>Router: navigate to new PR detail route
Router-->>User: show PR page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ow CTAs - `getRepoTemplate` server fn detects single-file templates via one GraphQL call across all legacy paths (.github/, docs/, root, both casings). Cached with repoCode signal. - Compare form auto-applies PR template to body once (only when body is empty); new-issue form same for issue template. - Yellow CTAs (Compare & pull request, Review changes) now match the Starred button's light/dark palette. - Create PR CTA disables while submitting with same style as the chevron toggle.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/compare/compare-diff-view.tsx`:
- Around line 54-61: The effect currently depends only on files.length and keeps
the previous visibleCount (via setVisibleCount(prev => ...)), so when the
comparison swaps to a different results array with the same length it preserves
the old visibleCount; update the useEffect (the useEffect + setVisibleCount
call) to depend on the files array identity (use files instead of files.length)
and reset visibleCount to Math.min(files.length, files.length === 0 ? 0 :
INITIAL_VISIBLE_COUNT) (i.e. drop the prev-based logic) so a new comparison
always starts at the INITIAL_VISIBLE_COUNT cap; reference: useEffect,
setVisibleCount, files, INITIAL_VISIBLE_COUNT.
In `@apps/dashboard/src/components/compare/compare-form.tsx`:
- Around line 100-108: The primary create button currently remains interactive
when submitting or when !canSubmit; update the button that uses handleExecute
and draftMode to reflect disabled semantics by adding a disabled={submitting ||
!canSubmit} prop and aria-disabled={submitting || !canSubmit}, remove or guard
the onClick (call handleExecute only when not disabled) and adjust the className
logic to include a disabled style branch (e.g., "opacity-50 pointer-events-none"
or your design token) so appearance and accessibility match state; apply the
same changes to the second button block referenced (lines ~110-118) that uses
similar variables.
In `@apps/dashboard/src/components/compare/compare-header.tsx`:
- Around line 68-71: The dot separator is always rendered when behindBy > 0
causing a leading separator in the "behind-only" state; update the JSX in the
CompareHeader component so the separator (the span containing "·") is only
rendered when there is an ahead segment as well (e.g., check aheadBy > 0) or
render the separator between segments conditionally (only when both aheadBy and
behindBy are truthy) so the dot never appears at the start when only behindBy is
present.
In `@apps/dashboard/src/components/compare/compare-page.tsx`:
- Around line 210-212: The current render gates the skeleton solely on absence
of repoData or compare, but getCompareDetail can resolve to null causing a
permanent ComparePageSkeleton; change the logic to render ComparePageSkeleton
only while overviewQuery.isPending or compareQuery.isPending (or similar
pending/loading flags), and when both queries are settled but compare is null
render an explicit empty/error state (e.g. a new CompareNotFound or ErrorEmpty
component or a message) instead of ComparePageSkeleton; update the branch that
references overviewQuery, compareQuery, repoData, compare and ensure errors
thrown for overviewQuery.error/compareQuery.error remain unchanged.
- Around line 168-180: The invalidateQueries predicate currently checks for
"repoMeta" which is a cache-policy label and doesn't match concrete query keys
like githubQueryKeys.repo.overview(...) or
githubQueryKeys.repo.recentPushableBranch(...); update the predicate in the
queryClient.invalidateQueries call to also match those concrete keys — e.g.,
when iterating query.queryKey, treat array segments that may contain nested
arrays/objects by stringifying or checking for string segments that include
"repo.overview" or "repo.recentPushableBranch" (or directly compare against the
serialized forms of githubQueryKeys.repo.overview and
githubQueryKeys.repo.recentPushableBranch) so the default-branch banner is
invalidated after PR creation.
In `@apps/dashboard/src/components/compare/compare-sidebar.tsx`:
- Around line 97-107: The PickerTrigger component currently doesn't forward ref
or popover trigger props and lacks an accessible name; update PickerTrigger to
be a React.forwardRef component that accepts and spreads incoming props (e.g.,
...props) onto the actual button, add a required label prop and set it as
aria-label on the button, remove any outer <span> wrappers so the popover's
asChild attaches directly to the button, and preserve the onPrefetch behavior by
calling it on onMouseEnter/onFocus; ensure the forwarded ref is attached to the
button (referencing PickerTrigger and its onPrefetch/label props).
In `@apps/dashboard/src/components/repo/branch-comparison-banner.tsx`:
- Around line 34-38: Guard the same-branch case before rendering the pending
skeleton: inside the BranchComparisonBanner component (where currentBranch,
defaultBranch and comparisonQuery.isPending are used), add an explicit early
return (return null) when currentBranch === defaultBranch before checking
comparisonQuery.isPending so the pending skeleton is not shown for disabled
comparisons; update both locations referenced (the pending branch around
comparisonQuery.isPending and the similar block at lines ~47-49) to perform this
same-branch check first.
In `@apps/dashboard/src/lib/github.functions.ts`:
- Around line 7270-7272: The catch block in the create-pull-request path
currently discards the installUrl from toMutationError(), so a 403 from GitHub
App auth loses its remediation link; update the catch return in
github.functions.ts (the block that calls toMutationError("create pull request",
error)) to return the full error payload expected by CreatePullRequestResult —
include the result.error and preserve result.installUrl (and any other fields
from result) instead of forcing an empty string for error, so callers can
surface the install URL for remediation.
- Around line 7222-7259: The current code pushes label/assignee/reviewer
requests into followUps and awaits Promise.allSettled(followUps) but then always
returns success; change this to inspect the Promise.allSettled results (the
followUps array and the calls to context.octokit.rest.issues.addLabels,
addAssignees, and context.octokit.rest.pulls.requestReviewers), collect any
rejected results (and their error messages), and surface them instead of
silently succeeding—either by throwing an aggregated error when any follow-up
failed or by returning a failure payload that includes the individual error
details so the caller can see which follow-ups failed. Ensure you preserve
successful results while including clear identification of which operation
(addLabels/addAssignees/requestReviewers) failed in the aggregated errors.
- Around line 8511-8556: The code calls getViewer() using a potentially
installation-scoped context from getGitHubContextForRepository(), which can
return an installation token (causing GET /user to fail); change to obtain and
use a user-scoped GitHub context before calling getViewer() and performing
user-scoped requests (like listActivities). Specifically: replace the usage of
the original context for the viewer and listActivities with a user-scoped
context returned by the appropriate helper (e.g., a
getGitHubContextForRepository call variant that returns a user token or a
getGitHubUserContext/getUserScopedContext helper), verify it is non-null, then
call getViewer(contextUser) and contextUser.octokit.rest.repos.listActivities;
if a user-scoped context cannot be obtained, return null as before.
- Around line 8641-8670: The current request/mapData pair in
apps/dashboard/src/lib/github.functions.ts uses a single
compareCommitsWithBasehead call (in the request lambda) with per_page: 100,
which only returns page 1 commits and up to 300 files; change the request to
paginate: repeatedly call context.octokit.rest.repos.compareCommitsWithBasehead
with page=1..N (keeping per_page=100) and accumulate comparison.commits into a
single commits array until you have collected comparison.total_commits or a page
returns no commits, then pass the combined commits into mapData (keep
totalCommits = comparison.total_commits). For files, preserve files from the
first page but explicitly surface the 300-file limitation (e.g., set
filesTruncated: true or log when (comparison.files ?? []).length === 300) so
callers know file results may be incomplete; keep existing mapping logic
(mapActor, CompareDetail["status"], PullFile["status"]) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fc7b14d7-2957-4f56-9b82-27c7c2802976
📒 Files selected for processing (23)
apps/dashboard/src/components/compare/compare-diff-view.tsxapps/dashboard/src/components/compare/compare-form.tsxapps/dashboard/src/components/compare/compare-header.tsxapps/dashboard/src/components/compare/compare-page.tsxapps/dashboard/src/components/compare/compare-sidebar.tsxapps/dashboard/src/components/issues/new/new-issue-page.tsxapps/dashboard/src/components/pulls/detail/pull-detail-header.tsxapps/dashboard/src/components/repo/branch-comparison-banner.tsxapps/dashboard/src/components/repo/recent-push-banner.tsxapps/dashboard/src/components/repo/repo-explorer-layout.tsxapps/dashboard/src/components/repo/repo-overview-page.tsxapps/dashboard/src/lib/github.functions.tsapps/dashboard/src/lib/github.query.tsapps/dashboard/src/lib/github.types.tsapps/dashboard/src/lib/parse-repo-ref.tsapps/dashboard/src/routeTree.gen.tsapps/dashboard/src/routes/_protected/$owner/$repo/compare.$.tsxapps/dashboard/src/routes/_protected/$owner/$repo/index.tsxapps/dashboard/src/routes/_protected/$owner/$repo/tree.$.tsxpackages/icons/src/index.tspackages/ui/src/components/callout.tsxpackages/ui/src/components/input.tsxpackages/ui/src/components/markdown-editor.tsx
Comment on lines
+54
to
+61
| useEffect(() => { | ||
| setVisibleCount((prev) => | ||
| Math.min( | ||
| files.length, | ||
| Math.max(files.length === 0 ? 0 : INITIAL_VISIBLE_COUNT, prev), | ||
| ), | ||
| ); | ||
| }, [files.length]); |
Contributor
There was a problem hiding this comment.
Reset visibleCount when the comparison changes, not just when the file count changes.
This effect only watches files.length, so moving to another compare result with the same count keeps the previous visibleCount. That skips the intended 30-file initial cap and can dump the full diff immediately on the next compare view.
♻️ Suggested change
- useEffect(() => {
- setVisibleCount((prev) =>
- Math.min(
- files.length,
- Math.max(files.length === 0 ? 0 : INITIAL_VISIBLE_COUNT, prev),
- ),
- );
- }, [files.length]);
+ useEffect(() => {
+ setVisibleCount(Math.min(files.length, INITIAL_VISIBLE_COUNT));
+ }, [files]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| setVisibleCount((prev) => | |
| Math.min( | |
| files.length, | |
| Math.max(files.length === 0 ? 0 : INITIAL_VISIBLE_COUNT, prev), | |
| ), | |
| ); | |
| }, [files.length]); | |
| useEffect(() => { | |
| setVisibleCount(Math.min(files.length, INITIAL_VISIBLE_COUNT)); | |
| }, [files]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/compare/compare-diff-view.tsx` around lines 54
- 61, The effect currently depends only on files.length and keeps the previous
visibleCount (via setVisibleCount(prev => ...)), so when the comparison swaps to
a different results array with the same length it preserves the old
visibleCount; update the useEffect (the useEffect + setVisibleCount call) to
depend on the files array identity (use files instead of files.length) and reset
visibleCount to Math.min(files.length, files.length === 0 ? 0 :
INITIAL_VISIBLE_COUNT) (i.e. drop the prev-based logic) so a new comparison
always starts at the INITIAL_VISIBLE_COUNT cap; reference: useEffect,
setVisibleCount, files, INITIAL_VISIBLE_COUNT.
Comment on lines
+100
to
+108
| <button | ||
| type="button" | ||
| onClick={handleExecute} | ||
| className={cn( | ||
| "flex h-8 items-center gap-1.5 px-3 text-xs font-medium transition-[background-color,opacity]", | ||
| draftMode | ||
| ? "bg-surface-2 text-foreground hover:bg-surface-2/80" | ||
| : "bg-primary text-primary-foreground hover:bg-primary/90", | ||
| )} |
Contributor
There was a problem hiding this comment.
Expose disabled state on the primary create action.
At Line 100, the button stays interactive while submitting or !canSubmit; handleExecute blocks, but the control should reflect disabled semantics in the UI/accessibility tree.
🔧 Suggested fix
<button
type="button"
onClick={handleExecute}
+ disabled={submitting || !canSubmit}
className={cn(
- "flex h-8 items-center gap-1.5 px-3 text-xs font-medium transition-[background-color,opacity]",
+ "flex h-8 items-center gap-1.5 px-3 text-xs font-medium transition-[background-color,opacity] disabled:pointer-events-none disabled:opacity-50",
draftMode
? "bg-surface-2 text-foreground hover:bg-surface-2/80"
: "bg-primary text-primary-foreground hover:bg-primary/90",
)}
>Also applies to: 110-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/compare/compare-form.tsx` around lines 100 -
108, The primary create button currently remains interactive when submitting or
when !canSubmit; update the button that uses handleExecute and draftMode to
reflect disabled semantics by adding a disabled={submitting || !canSubmit} prop
and aria-disabled={submitting || !canSubmit}, remove or guard the onClick (call
handleExecute only when not disabled) and adjust the className logic to include
a disabled style branch (e.g., "opacity-50 pointer-events-none" or your design
token) so appearance and accessibility match state; apply the same changes to
the second button block referenced (lines ~110-118) that uses similar variables.
Comment on lines
+68
to
+71
| {behindBy > 0 ? ( | ||
| <> | ||
| <span className="text-muted-foreground/50">·</span> | ||
| <span className="flex items-center gap-1"> |
Contributor
There was a problem hiding this comment.
Avoid leading separator in the “behind-only” state.
At Line 70, the dot separator is always shown for behindBy > 0, so you can render · first when there is no ahead segment.
🔧 Suggested fix
- {behindBy > 0 ? (
+ {behindBy > 0 ? (
<>
- <span className="text-muted-foreground/50">·</span>
+ {canPr ? <span className="text-muted-foreground/50">·</span> : null}
<span className="flex items-center gap-1">
<span className="tabular-nums font-medium text-foreground">
{behindBy}
</span>
{behindBy === 1 ? "commit" : "commits"} behind
</span>
</>
) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/compare/compare-header.tsx` around lines 68 -
71, The dot separator is always rendered when behindBy > 0 causing a leading
separator in the "behind-only" state; update the JSX in the CompareHeader
component so the separator (the span containing "·") is only rendered when there
is an ahead segment as well (e.g., check aheadBy > 0) or render the separator
between segments conditionally (only when both aheadBy and behindBy are truthy)
so the dot never appears at the start when only behindBy is present.
Comment on lines
+168
to
+180
| await queryClient.invalidateQueries({ | ||
| predicate: (query) => { | ||
| const key = query.queryKey; | ||
| return ( | ||
| Array.isArray(key) && | ||
| key.some( | ||
| (k) => | ||
| typeof k === "string" && | ||
| (k.includes("pulls") || k.includes("repoMeta")), | ||
| ) | ||
| ); | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Invalidate the concrete repo banner queries after PR creation.
This predicate never matches githubQueryKeys.repo.overview(...) or githubQueryKeys.repo.recentPushableBranch(...) because "repoMeta" is a cache-policy label, not a query-key segment. After a successful create, the default-branch “Compare & pull request” banner can stay stale until a later refresh.
♻️ Suggested change
import {
type GitHubQueryScope,
+ githubQueryKeys,
githubCompareDetailQueryOptions,
githubRepoCollaboratorsQueryOptions,
githubRepoOverviewQueryOptions,
githubViewerQueryOptions,
} from "#/lib/github.query";
@@
- await queryClient.invalidateQueries({
- predicate: (query) => {
- const key = query.queryKey;
- return (
- Array.isArray(key) &&
- key.some(
- (k) =>
- typeof k === "string" &&
- (k.includes("pulls") || k.includes("repoMeta")),
- )
- );
- },
- });
+ await Promise.all([
+ queryClient.invalidateQueries({
+ predicate: (query) =>
+ Array.isArray(query.queryKey) &&
+ query.queryKey[0] === "github" &&
+ query.queryKey[1] === scope.userId &&
+ query.queryKey.includes("pulls"),
+ }),
+ queryClient.invalidateQueries({
+ queryKey: githubQueryKeys.repo.overview(scope, { owner, repo }),
+ }),
+ queryClient.invalidateQueries({
+ queryKey: githubQueryKeys.repo.recentPushableBranch(scope, {
+ owner,
+ repo,
+ }),
+ }),
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await queryClient.invalidateQueries({ | |
| predicate: (query) => { | |
| const key = query.queryKey; | |
| return ( | |
| Array.isArray(key) && | |
| key.some( | |
| (k) => | |
| typeof k === "string" && | |
| (k.includes("pulls") || k.includes("repoMeta")), | |
| ) | |
| ); | |
| }, | |
| }); | |
| await Promise.all([ | |
| queryClient.invalidateQueries({ | |
| predicate: (query) => | |
| Array.isArray(query.queryKey) && | |
| query.queryKey[0] === "github" && | |
| query.queryKey[1] === scope.userId && | |
| query.queryKey.includes("pulls"), | |
| }), | |
| queryClient.invalidateQueries({ | |
| queryKey: githubQueryKeys.repo.overview(scope, { owner, repo }), | |
| }), | |
| queryClient.invalidateQueries({ | |
| queryKey: githubQueryKeys.repo.recentPushableBranch(scope, { | |
| owner, | |
| repo, | |
| }), | |
| }), | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/compare/compare-page.tsx` around lines 168 -
180, The invalidateQueries predicate currently checks for "repoMeta" which is a
cache-policy label and doesn't match concrete query keys like
githubQueryKeys.repo.overview(...) or
githubQueryKeys.repo.recentPushableBranch(...); update the predicate in the
queryClient.invalidateQueries call to also match those concrete keys — e.g.,
when iterating query.queryKey, treat array segments that may contain nested
arrays/objects by stringifying or checking for string segments that include
"repo.overview" or "repo.recentPushableBranch" (or directly compare against the
serialized forms of githubQueryKeys.repo.overview and
githubQueryKeys.repo.recentPushableBranch) so the default-branch banner is
invalidated after PR creation.
Comment on lines
+210
to
+212
| if (overviewQuery.error) throw overviewQuery.error; | ||
| if (compareQuery.error) throw compareQuery.error; | ||
| if (!repoData || !compare) return <ComparePageSkeleton />; |
Contributor
There was a problem hiding this comment.
Handle resolved null compare results explicitly.
getCompareDetail can return null, so this branch can leave the page in a permanent skeleton state even after loading has finished. Gate the skeleton on the pending state, then render an empty/error state when the compare payload is absent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/compare/compare-page.tsx` around lines 210 -
212, The current render gates the skeleton solely on absence of repoData or
compare, but getCompareDetail can resolve to null causing a permanent
ComparePageSkeleton; change the logic to render ComparePageSkeleton only while
overviewQuery.isPending or compareQuery.isPending (or similar pending/loading
flags), and when both queries are settled but compare is null render an explicit
empty/error state (e.g. a new CompareNotFound or ErrorEmpty component or a
message) instead of ComparePageSkeleton; update the branch that references
overviewQuery, compareQuery, repoData, compare and ensure errors thrown for
overviewQuery.error/compareQuery.error remain unchanged.
Comment on lines
+34
to
+38
| enabled: currentBranch !== defaultBranch, | ||
| }); | ||
|
|
||
| if (comparisonQuery.isPending) { | ||
| return ( |
Contributor
There was a problem hiding this comment.
Guard same-branch case before pending rendering.
If currentBranch === defaultBranch, enabled is false but isPending may still be true with no data, causing a persistent skeleton. Add an explicit same-branch null return before the pending branch.
🔧 Suggested fix
const comparisonQuery = useQuery({
...githubBranchComparisonQueryOptions(scope, {
owner,
repo,
base: defaultBranch,
head: currentBranch,
}),
enabled: currentBranch !== defaultBranch,
});
+
+ if (currentBranch === defaultBranch) return null;
if (comparisonQuery.isPending) {
return (Also applies to: 47-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/repo/branch-comparison-banner.tsx` around lines
34 - 38, Guard the same-branch case before rendering the pending skeleton:
inside the BranchComparisonBanner component (where currentBranch, defaultBranch
and comparisonQuery.isPending are used), add an explicit early return (return
null) when currentBranch === defaultBranch before checking
comparisonQuery.isPending so the pending skeleton is not shown for disabled
comparisons; update both locations referenced (the pending branch around
comparisonQuery.isPending and the similar block at lines ~47-49) to perform this
same-branch check first.
Comment on lines
+7222
to
+7259
| const followUps: Array<Promise<unknown>> = []; | ||
| if (data.labels && data.labels.length > 0) { | ||
| followUps.push( | ||
| context.octokit.rest.issues.addLabels({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| issue_number: pullNumber, | ||
| labels: data.labels, | ||
| }), | ||
| ); | ||
| } | ||
| if (data.assignees && data.assignees.length > 0) { | ||
| followUps.push( | ||
| context.octokit.rest.issues.addAssignees({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| issue_number: pullNumber, | ||
| assignees: data.assignees, | ||
| }), | ||
| ); | ||
| } | ||
| if ( | ||
| (data.reviewers && data.reviewers.length > 0) || | ||
| (data.teamReviewers && data.teamReviewers.length > 0) | ||
| ) { | ||
| followUps.push( | ||
| context.octokit.rest.pulls.requestReviewers({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| pull_number: pullNumber, | ||
| reviewers: data.reviewers ?? [], | ||
| team_reviewers: data.teamReviewers ?? [], | ||
| }), | ||
| ); | ||
| } | ||
| if (followUps.length > 0) { | ||
| await Promise.allSettled(followUps); | ||
| } |
Contributor
There was a problem hiding this comment.
Surface follow-up failures instead of silently succeeding.
Line 7258 makes the mutation return { ok: true } even when labels, assignees, or reviewer requests fail. That leaves the new PR in a state that does not match the form the user just submitted, with no feedback about what was skipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/github.functions.ts` around lines 7222 - 7259, The
current code pushes label/assignee/reviewer requests into followUps and awaits
Promise.allSettled(followUps) but then always returns success; change this to
inspect the Promise.allSettled results (the followUps array and the calls to
context.octokit.rest.issues.addLabels, addAssignees, and
context.octokit.rest.pulls.requestReviewers), collect any rejected results (and
their error messages), and surface them instead of silently succeeding—either by
throwing an aggregated error when any follow-up failed or by returning a failure
payload that includes the individual error details so the caller can see which
follow-ups failed. Ensure you preserve successful results while including clear
identification of which operation (addLabels/addAssignees/requestReviewers)
failed in the aggregated errors.
Comment on lines
+7270
to
+7272
| } catch (error) { | ||
| const result = toMutationError("create pull request", error); | ||
| return { ok: false, error: result.ok ? "" : result.error }; |
Contributor
There was a problem hiding this comment.
Preserve installUrl from toMutationError().
This catch path drops installUrl, so a 403 from GitHub App auth becomes a dead-end error even though CreatePullRequestResult supports a remediation link. Returning the full error payload here keeps the new PR flow recoverable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/github.functions.ts` around lines 7270 - 7272, The
catch block in the create-pull-request path currently discards the installUrl
from toMutationError(), so a 403 from GitHub App auth loses its remediation
link; update the catch return in github.functions.ts (the block that calls
toMutationError("create pull request", error)) to return the full error payload
expected by CreatePullRequestResult — include the result.error and preserve
result.installUrl (and any other fields from result) instead of forcing an empty
string for error, so callers can surface the install URL for remediation.
Comment on lines
+8511
to
+8556
| const context = await getGitHubContextForRepository(data); | ||
| if (!context) return null; | ||
|
|
||
| const repoCodeKey = githubRevalidationSignalKeys.repoCode(data); | ||
| const repoMetaKey = githubRevalidationSignalKeys.repoMeta(data); | ||
|
|
||
| try { | ||
| return await getOrRevalidateGitHubResource<RecentPushableBranch | null>({ | ||
| userId: context.session.user.id, | ||
| resource: "repo.recentPushableBranch.v1", | ||
| params: data, | ||
| freshForMs: githubCachePolicy.detail.staleTimeMs, | ||
| signalKeys: [repoCodeKey, repoMetaKey], | ||
| namespaceKeys: [repoCodeKey, repoMetaKey], | ||
| cacheMode: "split", | ||
| fetcher: async () => { | ||
| const viewer = await getViewer(context); | ||
| if (!viewer.login) { | ||
| return { | ||
| kind: "success", | ||
| data: null, | ||
| metadata: createGitHubResponseMetadata(200, {}), | ||
| }; | ||
| } | ||
|
|
||
| const overview = await context.octokit.rest.repos.get({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| }); | ||
| const defaultBranch = overview.data.default_branch; | ||
| if (!defaultBranch) { | ||
| return { | ||
| kind: "success", | ||
| data: null, | ||
| metadata: createGitHubResponseMetadata(200, {}), | ||
| }; | ||
| } | ||
|
|
||
| const activities = await context.octokit.rest.repos.listActivities({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| actor: viewer.login, | ||
| activity_type: "push", | ||
| time_period: "day", | ||
| per_page: 10, | ||
| }); |
Contributor
There was a problem hiding this comment.
Use a user-scoped context before calling getViewer() here.
getGitHubContextForRepository() can swap in an installation token, but GitHub documents GET /user for GitHub App user access tokens and fine-grained PATs, not installation access tokens. That means this banner can fall back to null on installed repos before listActivities() even runs. GitHub’s guidance also says user-driven actions should use a user access token rather than an installation token. (docs.github.com)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/github.functions.ts` around lines 8511 - 8556, The
code calls getViewer() using a potentially installation-scoped context from
getGitHubContextForRepository(), which can return an installation token (causing
GET /user to fail); change to obtain and use a user-scoped GitHub context before
calling getViewer() and performing user-scoped requests (like listActivities).
Specifically: replace the usage of the original context for the viewer and
listActivities with a user-scoped context returned by the appropriate helper
(e.g., a getGitHubContextForRepository call variant that returns a user token or
a getGitHubUserContext/getUserScopedContext helper), verify it is non-null, then
call getViewer(contextUser) and contextUser.octokit.rest.repos.listActivities;
if a user-scoped context cannot be obtained, return null as before.
Comment on lines
+8641
to
+8670
| request: (headers) => | ||
| context.octokit.rest.repos.compareCommitsWithBasehead({ | ||
| owner: data.owner, | ||
| repo: data.repo, | ||
| basehead: `${data.base}...${data.head}`, | ||
| per_page: 100, | ||
| headers, | ||
| }), | ||
| mapData: (comparison) => ({ | ||
| aheadBy: comparison.ahead_by, | ||
| behindBy: comparison.behind_by, | ||
| status: comparison.status as CompareDetail["status"], | ||
| totalCommits: comparison.total_commits, | ||
| commits: comparison.commits.map((c) => ({ | ||
| sha: c.sha, | ||
| message: c.commit.message, | ||
| createdAt: c.commit.committer?.date ?? c.commit.author?.date ?? "", | ||
| author: mapActor(c.author), | ||
| })), | ||
| files: (comparison.files ?? []).map((f) => ({ | ||
| sha: f.sha ?? null, | ||
| filename: f.filename, | ||
| status: f.status as PullFile["status"], | ||
| additions: f.additions, | ||
| deletions: f.deletions, | ||
| changes: f.changes, | ||
| patch: f.patch ?? null, | ||
| previousFilename: f.previous_filename ?? null, | ||
| })), | ||
| }), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the complete context of this function
fd github.functions.tsRepository: stylessh/diffkit
Length of output: 102
🏁 Script executed:
# Search for pagination handling in GitHub API calls within the file
rg -n "compareCommitsWithBasehead" apps/dashboard/src/lib/github.functions.ts -A 30 -B 5Repository: stylessh/diffkit
Length of output: 4457
🏁 Script executed:
# Check if there are other pagination patterns in the codebase
rg -n "paginate" apps/dashboard/src/lib/github.functions.tsRepository: stylessh/diffkit
Length of output: 99
🌐 Web query:
GitHub API compareCommitsWithBasehead endpoint pagination behavior files per_page
💡 Result:
The GitHub REST API endpoint POST /repos/{owner}/{repo}/compare/{basehead} (also referred to as compareCommitsWithBasehead in some client libraries like Octokit) supports standard pagination parameters per_page (max 100, default 30) and page for paginating the list of commits in the comparison. However, the files array in the response, which lists all changed files across the entire comparison: - Appears only on the first page of results. - Is limited to a maximum of 300 files, even for comparisons with more changes. - Does not support direct pagination of the files list itself; pagination applies only to commits. For comparisons exceeding 300 files, there is no API mechanism to retrieve the complete list via this endpoint. Workarounds include: - Cloning the repository and using git diff --name-only {base}...{head}. - Paginates through commits and aggregates files from each individual commit's API endpoint (GET /repos/{owner}/{repo}/commits/{ref}), noting this may include duplicates or misses for renames across the range. This behavior is consistent across official documentation snippets, Stack Overflow discussions, GitHub issues, and client library notes (e.g., PyGithub explicitly states "Only the first 300 changed files"). No changes noted as of 2026.
Citations:
- 1: https://stackoverflow.com/questions/72539598/how-do-you-paginate-the-results-of-githubs-compare-commit-api
- 2: https://docs.github.com/v3/repos/commits
- 3: https://docs.github.com/en/rest/commits/commits?apiVersion=2026-03-10
- 4: https://docs.github.com/en/rest/using-the-rest-api/using-pagination-in-the-rest-api
- 5: https://github.apidog.io/api-4078874
- 6: https://stackoverflow.com/questions/26925312/github-api-how-to-compare-2-commits
- 7: https://docs.github.com/rest/commits/commits
- 8: https://docs.github.com/en/rest/commits/commits
- 9: https://pygithub.readthedocs.io/en/stable/github%5Fobjects/Comparison.html
- 10: https://stackoverflow.com/questions/72405908/how-can-i-get-a-complete-list-of-changed-files-between-two-commits-in-github
- 11: [Schema Inaccuracy] Commit compare endpoint limits the number of files returned github/rest-api-description#33
Implement pagination to retrieve all commits and acknowledge the 300-file limitation.
The code makes a single request with per_page: 100 and returns incomplete data without indication:
- Commits: Only page 1 (≤100 commits) is fetched, but
totalCommitsin the response reflects the actual comparison size, creating a silent mismatch - Files: GitHub caps the files array at 300 total across the entire comparison and only returns them on page 1; larger comparisons will silently lose file data
To fix: Loop through pages using the page parameter to collect all commits, and document or handle the 300-file limitation explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/github.functions.ts` around lines 8641 - 8670, The
current request/mapData pair in apps/dashboard/src/lib/github.functions.ts uses
a single compareCommitsWithBasehead call (in the request lambda) with per_page:
100, which only returns page 1 commits and up to 300 files; change the request
to paginate: repeatedly call
context.octokit.rest.repos.compareCommitsWithBasehead with page=1..N (keeping
per_page=100) and accumulate comparison.commits into a single commits array
until you have collected comparison.total_commits or a page returns no commits,
then pass the combined commits into mapData (keep totalCommits =
comparison.total_commits). For files, preserve files from the first page but
explicitly surface the 300-file limitation (e.g., set filesTruncated: true or
log when (comparison.files ?? []).length === 300) so callers know file results
may be incomplete; keep existing mapping logic (mapActor,
CompareDetail["status"], PullFile["status"]) intact.
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds functionality and user flow to create Pull Requests from DiffKit.
Summary by CodeRabbit
New Features
Improvements