feat(desktop): auto-commit messages, built-in PR creation & inline diff comments#1380
feat(desktop): auto-commit messages, built-in PR creation & inline diff comments#1380
Conversation
…line diff commenting - Add generateCommitMessage tRPC procedure that analyzes staged diffs and produces conventional commit messages with sparkle button in UI - Replace browser-redirect PR creation with built-in `gh pr create` flow via CreatePRDialog with auto-generated title/body - Add inline commenting system for diffs with persistent Zustand store, comment thread UI per file, and comment count badges in headers
📝 WalkthroughWalkthroughThe PR introduces AI-powered commit message generation, PR body composition via git diff analysis, inline diff commenting with a persistent Zustand store, and refactors PR creation to use GitHub CLI. It adds three new TRPC endpoints, a comment management store, multiple UI components for comments and PR dialogs, and integrates these features into the existing diff and commit workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CommitInput as CommitInput UI
participant Store as generateCommitMessage<br/>(tRPC)
participant GitOps as Git Operations
participant Git as Git CLI
User->>CommitInput: Click sparkles button
CommitInput->>Store: generateCommitMessage(worktreePath)
Store->>Git: Get staged diff & file list
Git-->>Store: Diff output, file statuses
Store->>GitOps: Analyze diff for conventional commit
GitOps->>GitOps: Build prompt + extract scope
GitOps-->>Store: Suggested message
Store-->>CommitInput: { message: string }
CommitInput->>CommitInput: Auto-fill message field
CommitInput-->>User: Display generated message
sequenceDiagram
participant User as User
participant CommitInput as CommitInput UI
participant Dialog as CreatePRDialog
participant PRBody as generatePRBody<br/>(tRPC)
participant GitOps as Git Operations
participant GH as gh CLI
participant Store as useDiffCommentsStore
User->>CommitInput: Click create PR / push
CommitInput->>CommitInput: Show CreatePRDialog
Dialog->>Dialog: onOpenChange(true)
Dialog->>PRBody: generatePRBody(worktreePath)
PRBody->>GitOps: Analyze branch & commits
GitOps-->>PRBody: { title, body }
PRBody-->>Dialog: Auto-fill title & body
User->>Dialog: Review & click Create
Dialog->>Dialog: createPR mutation
Dialog->>GH: Execute gh pr create<br/>(title, body, draft, baseBranch)
GH-->>Dialog: PR URL output
Dialog->>Dialog: Parse PR number from URL
Dialog-->>CommitInput: onSuccess()
CommitInput-->>User: Toast with PR link & number
CommitInput->>CommitInput: Close dialog
sequenceDiagram
participant User as User
participant DiffView as FileDiffSection
participant CommentUI as DiffCommentThread
participant Store as useDiffCommentsStore
User->>DiffView: View file diff
DiffView->>Store: getFileCommentCount(worktreePath, filePath)
Store-->>DiffView: commentCount
DiffView->>DiffView: Render FileDiffHeader + badge
DiffView->>CommentUI: Render DiffCommentThread
CommentUI->>Store: getFileComments(worktreePath, filePath)
Store-->>CommentUI: DiffComment[]
CommentUI->>CommentUI: Render comment list
User->>CommentUI: Click add comment
CommentUI->>CommentUI: Show form (line, text)
User->>CommentUI: Enter text & submit
CommentUI->>Store: addComment({ worktreePath, filePath, lineNumber, side, text })
Store->>Store: Generate id & createdAt
Store-->>CommentUI: Comment added
CommentUI->>CommentUI: Re-render with new comment
CommentUI-->>User: Display comment inline
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
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. ✨ Finishing touches
🧪 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.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx (1)
85-96: 🛠️ Refactor suggestion | 🟠 Major
createPRMutationis dead code — never invoked.PR creation now goes through
CreatePRDialog's own internal mutation. This mutation is declared butcreatePRMutation.mutate()is never called anywhere in this component. ItsisPendingstate (line 120) will never betrue, adding noise to the pending check.Remove this mutation and its reference in
isPending.Proposed fix
- const createPRMutation = electronTrpc.changes.createPR.useMutation({ - onSuccess: (data) => { - toast.success(`PR #${data.number} created`, { - action: { - label: "Open", - onClick: () => window.open(data.url, "_blank"), - }, - }); - onRefresh(); - }, - onError: (error) => toast.error(`Failed: ${error.message}`), - }); - ... const isPending = commitMutation.isPending || pushMutation.isPending || pullMutation.isPending || syncMutation.isPending || - createPRMutation.isPending || fetchMutation.isPending || generateMessageMutation.isPending;
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts`:
- Line 504: The fallback to a hardcoded "main" in the assignment const base =
input.baseBranch || "main" is brittle for repos using "master" or other
defaults; change logic in the git-operations handler to resolve the remote
default branch when input.baseBranch is absent (e.g., run a git query such as
`git remote show origin` or `git rev-parse --abbrev-ref origin/HEAD` or use the
GitHub API/gh CLI to fetch defaultBranchRef) and assign that result to base,
falling back to a sensible last-resort value only if detection fails; update any
callers that expect base to be present to handle async detection and surface
detection errors via the existing error path in the surrounding function.
- Around line 170-178: The isFix heuristic (diffLower and isFix) is too broad
and causes false positives; change isFix to only inspect added lines from the
diff (lines starting with '+') and match narrower patterns such as /\bfixes?\b/,
/\bbugfix\b/, /^fix:/, /fixes\s+#\d+/ or similar commit-like tokens rather than
any occurrence of "fix", "bug", "error", or "issue"; then use that tightened
isFix when computing type (alongside existing isTest and isDocs) so type
assignment remains correct (the symbols to update are isFix, diffLower usage,
and the type = isTest ? ... line).
- Around line 513-516: The empty catch after the git log call swallows all
errors; replace it with handling that logs the failure (including error.message
and error.stack) with context (e.g., "failed to run git log to generate PR body
from base branch <baseBranch>") and only fall back to the simple branch name for
the known "missing base branch" case — for other errors rethrow (or propagate)
so callers can surface the failure; implement this change in the git log
handling block in git-operations.ts (the catch directly after the git log
invocation that builds the PR body).
- Around line 77-102: _buildCommitMessagePrompt is unused dead code; either
remove it or wire it into the commit-message flow. If you intend to keep an
LLM-based generator, update generateCommitMessage (or the function that
currently calls generateMessageFromDiff) to call _buildCommitMessagePrompt with
{stagedFiles, diffStat, detailedDiff} and pass its result to the LLM invocation;
otherwise delete the _buildCommitMessagePrompt function and any imports/comments
referencing it to avoid unused code warnings.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/DiffCommentThread.tsx`:
- Around line 26-38: handleAdd currently sets lineNumber to 0 when newLine is
empty; change it so an empty newLine does not produce 0 by either making
submission invalid or by omitting/defaulting the value — e.g., in handleAdd
replace the ternary that produces 0 with a value of undefined or 1 (or disable
the add action when newLine.trim() === ""), and update the call to addComment
(and any consumers expecting lineNumber) to accept undefined/omit and avoid
showing "line 0" in InlineComment; reference: handleAdd, addComment, newLine,
lineNumber, InlineComment.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx`:
- Around line 412-417: CommitInput currently renders CreatePRDialog without
passing the optional baseBranch prop, causing generatePRBody to default to
"main"; update the CreatePRDialog JSX in CommitInput to pass the repository's
default branch via the baseBranch prop (e.g., baseBranch={defaultBranch} or
derived value) — ensure you obtain the default branch from the surrounding
CommitInput props/state/context where it's available (or compute it from the
repo metadata used in CommitInput) and pass it into CreatePRDialog so
generatePRBody receives the correct base branch.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CreatePRDialog.tsx`:
- Around line 42-47: The effect in CreatePRDialog currently re-runs when
title/body change which causes cleared fields to be immediately re-filled;
update the useEffect that reads prSuggestion/open/title/body so it only
auto-populates once when suggestion data first arrives (or when dialog opens) by
removing title and body from the dependency array and adding a ref flag (e.g.,
hasAutoFilledRef) to track whether auto-fill has already occurred; inside
useEffect (watching prSuggestion and open) check hasAutoFilledRef.current before
calling setTitle/setBody and set the flag true after populating, or
alternatively move the population into the query's success handler that calls
setTitle/setBody once.
In `@apps/desktop/src/renderer/stores/diff-comments/store.ts`:
- Around line 122-130: getFileComments currently returns a fresh [] when no
comments exist which breaks reference stability; change the selector to return a
stable shared value instead of a new array (e.g. introduce a single frozen
EMPTY_COMMENTS constant and return workspaceComments[filePath] ??
EMPTY_COMMENTS) or return undefined and update consumers to handle that; modify
the getFileComments function (leave getFileCommentCount unchanged) so consumers
like DiffCommentThread (which spreads the result) receive a stable reference.
🧹 Nitpick comments (7)
apps/desktop/src/renderer/stores/diff-comments/store.ts (1)
40-42: Positional arguments forgetFileCommentsandgetFileCommentCountviolate the coding guideline.These functions take 2 positional parameters. Per coding guidelines, use object parameters for functions with 2 or more parameters.
Proposed fix
In the interface:
- getFileComments: (worktreePath: string, filePath: string) => DiffComment[]; - getFileCommentCount: (worktreePath: string, filePath: string) => number; + getFileComments: (params: { worktreePath: string; filePath: string }) => DiffComment[]; + getFileCommentCount: (params: { worktreePath: string; filePath: string }) => number;And in the implementation:
- getFileComments: (worktreePath, filePath) => { - const workspaceComments = get().comments[worktreePath] ?? {}; - return workspaceComments[filePath] ?? []; + getFileComments: ({ worktreePath, filePath }) => { + const workspaceComments = get().comments[worktreePath] ?? {}; + return workspaceComments[filePath] ?? []; }, - getFileCommentCount: (worktreePath, filePath) => { - const workspaceComments = get().comments[worktreePath] ?? {}; - return (workspaceComments[filePath] ?? []).length; + getFileCommentCount: ({ worktreePath, filePath }) => { + const workspaceComments = get().comments[worktreePath] ?? {}; + return (workspaceComments[filePath] ?? []).length; },As per coding guidelines: "Use object parameters for functions with 2 or more parameters instead of positional arguments"
apps/desktop/src/lib/trpc/routers/changes/git-operations.ts (2)
264-269: Extract the truncation limit to a named constant.
8000is a magic number. As per coding guidelines, extract hardcoded magic numbers to named constants at module top.Proposed fix
At the top of the file (after imports):
const MAX_DIFF_LENGTH = 8000;Then:
- const truncatedDiff = - detailedDiff.length > 8000 - ? `${detailedDiff.slice(0, 8000)}\n... (truncated)` - : detailedDiff; + const truncatedDiff = + detailedDiff.length > MAX_DIFF_LENGTH + ? `${detailedDiff.slice(0, MAX_DIFF_LENGTH)}\n... (truncated)` + : detailedDiff;As per coding guidelines: "Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic"
244-281:generateCommitMessageis defined as a mutation, but it performs no writes.This is a read-only operation that analyzes staged diffs and returns a message. A
querywould be more semantically correct and would allow tRPC to deduplicate and cache the result. However, since it's triggered by a user action (button click) and you likely want fresh results each time, a mutation is pragmatically fine — just noting the semantic mismatch.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/InlineComment.tsx (2)
18-19:editTextcan go stale ifcomment.textchanges while not editing.If the parent re-renders with an updated
comment.text(e.g., after a store sync), the localeditTextstate retains the old value. Consider resetting it when the comment prop changes.Suggested fix
const [isEditing, setIsEditing] = useState(false); const [editText, setEditText] = useState(comment.text); + + useEffect(() => { + if (!isEditing) { + setEditText(comment.text); + } + }, [comment.text, isEditing]);
108-117:getTimeAgois fine as a co-located helper, but consider using a well-known library.If the app already depends on a date utility (e.g.,
date-fns),formatDistanceToNowwould handle edge cases (e.g., months, years) and i18n. If not, this is acceptable for now.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/DiffCommentThread.tsx (2)
17-20: Unselective store subscription will cause unnecessary re-renders.Line 20 destructures actions from
useDiffCommentsStore()without a selector, subscribing to all store state changes. Since the comment list selector on line 17 already handles data reads, extract the actions separately to avoid coupling:Suggested fix
const comments = useDiffCommentsStore((s) => s.getFileComments(worktreePath, filePath), ); -const { addComment, deleteComment, editComment } = useDiffCommentsStore(); +const addComment = useDiffCommentsStore((s) => s.addComment); +const deleteComment = useDiffCommentsStore((s) => s.deleteComment); +const editComment = useDiffCommentsStore((s) => s.editComment);Also note: the
getFileCommentsselector returnsworkspaceComments[filePath] ?? [], which creates a new array reference when there are no comments, triggering re-renders on every store update. Consider returning a stable empty array constant from the store.
48-50: Sorting on every render — consideruseMemo.
sortedCommentscreates a new sorted copy each render. WithuseMemokeyed oncomments, the sort only re-runs when comments actually change.Suggested fix
+import { useMemo, useState } from "react"; -import { useState } from "react"; ... -const sortedComments = [...comments].sort( - (a, b) => a.lineNumber - b.lineNumber || a.createdAt - b.createdAt, -); +const sortedComments = useMemo( + () => [...comments].sort( + (a, b) => a.lineNumber - b.lineNumber || a.createdAt - b.createdAt, + ), + [comments], +);
| function _buildCommitMessagePrompt({ | ||
| stagedFiles, | ||
| diffStat, | ||
| detailedDiff, | ||
| }: { | ||
| stagedFiles: string; | ||
| diffStat: string; | ||
| detailedDiff: string; | ||
| }): string { | ||
| return [ | ||
| "Generate a concise conventional commit message for these changes.", | ||
| "Use format: <type>(<scope>): <description>", | ||
| "Types: feat, fix, refactor, docs, style, test, chore, perf", | ||
| "Keep the description under 72 characters.", | ||
| "Only output the commit message, nothing else.", | ||
| "", | ||
| "Files changed:", | ||
| stagedFiles, | ||
| "", | ||
| "Diff stat:", | ||
| diffStat, | ||
| "", | ||
| "Diff:", | ||
| detailedDiff, | ||
| ].join("\n"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n '_buildCommitMessagePrompt' --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg -n '_buildCommitMessagePrompt'Repository: superset-sh/superset
Length of output: 163
🏁 Script executed:
# Check if _buildCommitMessagePrompt is exported or used as a callback/reference anywhere
rg -n 'buildCommitMessagePrompt' apps/desktop/src/lib/trpc/routers/changes/git-operations.tsRepository: superset-sh/superset
Length of output: 103
🏁 Script executed:
# Check entire codebase for any usage outside the definition
rg '_buildCommitMessagePrompt' --count-matchesRepository: superset-sh/superset
Length of output: 125
🏁 Script executed:
# Examine the generateCommitMessage and generateMessageFromDiff functions to confirm they don't call _buildCommitMessagePrompt
rg -A 20 'function generateMessageFromDiff' apps/desktop/src/lib/trpc/routers/changes/git-operations.tsRepository: superset-sh/superset
Length of output: 895
🏁 Script executed:
# Check for any generateCommitMessage procedure that might call _buildCommitMessagePrompt
rg -A 30 'generateCommitMessage.*:.*procedure' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for generateCommitMessage procedure definition
rg -B 5 -A 30 'generateCommitMessage' apps/desktop/src/lib/trpc/routers/changes/git-operations.ts | head -50Repository: superset-sh/superset
Length of output: 1221
🏁 Script executed:
# Get the complete generateCommitMessage procedure to see what it returns
rg -A 40 'generateCommitMessage: publicProcedure' apps/desktop/src/lib/trpc/routers/changes/git-operations.tsRepository: superset-sh/superset
Length of output: 1154
_buildCommitMessagePrompt is dead code — never called anywhere.
This function builds a prompt string but is never invoked. The generateCommitMessage procedure delegates to the heuristic-based generateMessageFromDiff instead. Remove it if it's a leftover from an earlier LLM-based approach, or wire it up if intentional.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 77
- 102, _buildCommitMessagePrompt is unused dead code; either remove it or wire
it into the commit-message flow. If you intend to keep an LLM-based generator,
update generateCommitMessage (or the function that currently calls
generateMessageFromDiff) to call _buildCommitMessagePrompt with {stagedFiles,
diffStat, detailedDiff} and pass its result to the LLM invocation; otherwise
delete the _buildCommitMessagePrompt function and any imports/comments
referencing it to avoid unused code warnings.
|
|
||
| // Analyze diff for fix indicators | ||
| const isFix = | ||
| diffLower.includes("fix") || | ||
| diffLower.includes("bug") || | ||
| diffLower.includes("error") || | ||
| diffLower.includes("issue"); | ||
|
|
||
| const type = isTest ? "test" : isDocs ? "docs" : isFix ? "fix" : "feat"; |
There was a problem hiding this comment.
isFix heuristic has a high false-positive rate.
Matching "fix", "bug", "error", or "issue" against the lowercased diff content will trigger on extremely common patterns — error handling code, CSS prefixes (-webkit-→ no, but fix appears in "prefix"), import of ErrorBoundary, issue tracker URLs in comments, etc. This could mislabel many commits as fix when they're actually feat or refactor.
Consider narrowing the heuristic to only match added lines (+ prefixed lines in the diff) or to match more specific patterns like fixes #, bugfix, or commit-message-like patterns rather than raw diff content.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 170
- 178, The isFix heuristic (diffLower and isFix) is too broad and causes false
positives; change isFix to only inspect added lines from the diff (lines
starting with '+') and match narrower patterns such as /\bfixes?\b/,
/\bbugfix\b/, /^fix:/, /fixes\s+#\d+/ or similar commit-like tokens rather than
any occurrence of "fix", "bug", "error", or "issue"; then use that tightened
isFix when computing type (alongside existing isTest and isDocs) so type
assignment remains correct (the symbols to update are isFix, diffLower usage,
and the type = isTest ? ... line).
|
|
||
| const git = simpleGit(input.worktreePath); | ||
| const branch = (await git.revparse(["--abbrev-ref", "HEAD"])).trim(); | ||
| const base = input.baseBranch || "main"; |
There was a problem hiding this comment.
Hardcoded "main" fallback won't work for repos using "master" or other default branches.
When baseBranch is not provided, this falls back to "main". Repositories using master, develop, or other conventions will get an empty or incorrect commit log since origin/main..HEAD won't resolve. Consider detecting the default branch dynamically, e.g., via git remote show origin or gh repo view --json defaultBranchRef.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` at line 504, The
fallback to a hardcoded "main" in the assignment const base = input.baseBranch
|| "main" is brittle for repos using "master" or other defaults; change logic in
the git-operations handler to resolve the remote default branch when
input.baseBranch is absent (e.g., run a git query such as `git remote show
origin` or `git rev-parse --abbrev-ref origin/HEAD` or use the GitHub API/gh CLI
to fetch defaultBranchRef) and assign that result to base, falling back to a
sensible last-resort value only if detection fails; update any callers that
expect base to be present to handle async detection and surface detection errors
via the existing error path in the surrounding function.
| ]); | ||
| } catch { | ||
| // Fall back to just the branch name | ||
| } |
There was a problem hiding this comment.
Silent catch block swallows errors without logging.
This violates the guideline to never swallow errors silently. If git log fails for a reason other than a missing base branch (e.g., corrupt repo, permission error), the failure is silently ignored and the user gets an empty PR body with no indication of the problem.
Proposed fix
} catch {
- // Fall back to just the branch name
+ catch (error) {
+ console.warn("[git/generatePRBody] Failed to get commit log, falling back to branch name:", error);
+ }As per coding guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly"
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 513
- 516, The empty catch after the git log call swallows all errors; replace it
with handling that logs the failure (including error.message and error.stack)
with context (e.g., "failed to run git log to generate PR body from base branch
<baseBranch>") and only fall back to the simple branch name for the known
"missing base branch" case — for other errors rethrow (or propagate) so callers
can surface the failure; implement this change in the git log handling block in
git-operations.ts (the catch directly after the git log invocation that builds
the PR body).
| const handleAdd = () => { | ||
| if (!newText.trim()) return; | ||
| addComment({ | ||
| worktreePath, | ||
| filePath, | ||
| lineNumber: newLine ? Number.parseInt(newLine, 10) : 0, | ||
| side: "modified", | ||
| text: newText.trim(), | ||
| }); | ||
| setNewText(""); | ||
| setNewLine(""); | ||
| setIsAdding(false); | ||
| }; |
There was a problem hiding this comment.
Line number defaults to 0 when omitted, which is not a valid line number.
If the user leaves the line field blank, the comment is stored with lineNumber: 0. This could confuse users seeing "on line 0" in the InlineComment display. Consider either requiring a line number (disable submit when empty) or using 1 as a default, or omitting the line display when it's unset.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/DiffCommentThread.tsx`
around lines 26 - 38, handleAdd currently sets lineNumber to 0 when newLine is
empty; change it so an empty newLine does not produce 0 by either making
submission invalid or by omitting/defaulting the value — e.g., in handleAdd
replace the ternary that produces 0 with a value of undefined or 1 (or disable
the add action when newLine.trim() === ""), and update the call to addComment
(and any consumers expecting lineNumber) to accept undefined/omit and avoid
showing "line 0" in InlineComment; reference: handleAdd, addComment, newLine,
lineNumber, InlineComment.
| <CreatePRDialog | ||
| open={showCreatePRDialog} | ||
| onOpenChange={setShowCreatePRDialog} | ||
| worktreePath={worktreePath} | ||
| onSuccess={onRefresh} | ||
| /> |
There was a problem hiding this comment.
baseBranch prop is not passed to CreatePRDialog.
The dialog accepts an optional baseBranch prop which feeds into generatePRBody. Without it, the backend falls back to hardcoded "main". If the CommitInput component has access to the current repo's default branch (or if it can be derived), passing it here would make the PR body generation more accurate for repos not using main.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CommitInput.tsx`
around lines 412 - 417, CommitInput currently renders CreatePRDialog without
passing the optional baseBranch prop, causing generatePRBody to default to
"main"; update the CreatePRDialog JSX in CommitInput to pass the repository's
default branch via the baseBranch prop (e.g., baseBranch={defaultBranch} or
derived value) — ensure you obtain the default branch from the surrounding
CommitInput props/state/context where it's available (or compute it from the
repo metadata used in CommitInput) and pass it into CreatePRDialog so
generatePRBody receives the correct base branch.
| useEffect(() => { | ||
| if (prSuggestion && open) { | ||
| if (!title) setTitle(prSuggestion.title); | ||
| if (!body) setBody(prSuggestion.body); | ||
| } | ||
| }, [prSuggestion, open, body, title]); |
There was a problem hiding this comment.
Bug: Users cannot clear the auto-filled title or body.
Including title and body in the dependency array means that when a user clears either field (setting it to ""), the effect re-fires, !title / !body evaluates to true, and the cached prSuggestion immediately re-fills the field.
A simpler approach is to populate the fields only once when the suggestion data first arrives, using a ref to track whether auto-fill has already occurred, or by leveraging the query's onSuccess-like pattern.
Proposed fix using a ref flag
+import { useEffect, useRef, useState } from "react";
...
+ const hasAutoFilled = useRef(false);
+
+ // Reset form when dialog closes
+ useEffect(() => {
+ if (!open) {
+ setTitle("");
+ setBody("");
+ setIsDraft(false);
+ hasAutoFilled.current = false;
+ }
+ }, [open]);
useEffect(() => {
- if (prSuggestion && open) {
- if (!title) setTitle(prSuggestion.title);
- if (!body) setBody(prSuggestion.body);
+ if (prSuggestion && open && !hasAutoFilled.current) {
+ setTitle(prSuggestion.title);
+ setBody(prSuggestion.body);
+ hasAutoFilled.current = true;
}
- }, [prSuggestion, open, body, title]);
+ }, [prSuggestion, open]);🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitInput/CreatePRDialog.tsx`
around lines 42 - 47, The effect in CreatePRDialog currently re-runs when
title/body change which causes cleared fields to be immediately re-filled;
update the useEffect that reads prSuggestion/open/title/body so it only
auto-populates once when suggestion data first arrives (or when dialog opens) by
removing title and body from the dependency array and adding a ref flag (e.g.,
hasAutoFilledRef) to track whether auto-fill has already occurred; inside
useEffect (watching prSuggestion and open) check hasAutoFilledRef.current before
calling setTitle/setBody and set the flag true after populating, or
alternatively move the population into the query's success handler that calls
setTitle/setBody once.
| getFileComments: (worktreePath, filePath) => { | ||
| const workspaceComments = get().comments[worktreePath] ?? {}; | ||
| return workspaceComments[filePath] ?? []; | ||
| }, | ||
|
|
||
| getFileCommentCount: (worktreePath, filePath) => { | ||
| const workspaceComments = get().comments[worktreePath] ?? {}; | ||
| return (workspaceComments[filePath] ?? []).length; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=tsx 'getFileComments|getFileCommentCount' -g '!**/store.ts' -B2 -A2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg -n 'getFileComments|getFileCommentCount' -g '!**/store.ts' --type-list | head -20Repository: superset-sh/superset
Length of output: 453
🏁 Script executed:
rg -n 'getFileComments|getFileCommentCount' -g '!**/store.ts' -B2 -A2 2>/dev/null | head -100Repository: superset-sh/superset
Length of output: 1721
🏁 Script executed:
cat -n apps/desktop/src/renderer/stores/diff-comments/store.ts | head -150Repository: superset-sh/superset
Length of output: 4762
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/DiffCommentThread.tsx | sed -n '10,30p'Repository: superset-sh/superset
Length of output: 739
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx | sed -n '210,230p'Repository: superset-sh/superset
Length of output: 773
🏁 Script executed:
# Check if there are any infinite loop or performance issues reported related to these components
git log --oneline --all --grep="DiffCommentThread\|FileDiffSection\|re-render" 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 792
🏁 Script executed:
# Check if the components use these selectors in ways that would expose the reference problem
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/InlineComment/DiffCommentThread.tsx | head -80Repository: superset-sh/superset
Length of output: 2786
Separate concerns: getFileComments has a reference stability issue, but getFileCommentCount does not.
getFileComments returns a new empty array ([]) when a file has no comments, causing the selector to return a different reference on each store change. DiffCommentThread then spreads this array at line 48, compounding the issue. However, getFileCommentCount returns a number (compared by value), so it won't trigger re-renders due to reference changes.
For getFileComments, consider either:
- Having consumers select the raw
commentsstate and derive values inline, or - Wrapping the selector with a shallow equality check:
const comments = useDiffCommentsStore(
(s) => s.comments[worktreePath]?.[filePath] ?? [],
shallow,
);getFileCommentCount can remain as-is.
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/stores/diff-comments/store.ts` around lines 122 -
130, getFileComments currently returns a fresh [] when no comments exist which
breaks reference stability; change the selector to return a stable shared value
instead of a new array (e.g. introduce a single frozen EMPTY_COMMENTS constant
and return workspaceComments[filePath] ?? EMPTY_COMMENTS) or return undefined
and update consumers to handle that; modify the getFileComments function (leave
getFileCommentCount unchanged) so consumers like DiffCommentThread (which
spreads the result) receive a stable reference.
1 participant
Summary
ghCLI, and inline commenting on diffsChanges
Auto-generate commit messages
generateCommitMessagetRPC procedure ingit-operations.tsthat analyzes staged diffs (--name-status,--stat,--cached)Built-in PR creation using
ghCLIcreatePRprocedure (which opened a GitHub compare URL in the browser) withgh pr createexecutiongeneratePRBodyquery that auto-generates PR title from branch name and body from commit logCreatePRDialogcomponent with title input, markdown description textarea, draft checkbox, and base branch supportInline commenting of diffs
diff-commentsZustand store (persisted to localStorage) with add/edit/delete/clear operations, keyed by workspace and file pathInlineCommentcomponent: displays individual comments with edit/delete actions and relative time formattingDiffCommentThreadcomponent: renders sorted comment thread per file with "Add comment" form (line number + text input)FileDiffSectionFileDiffHeader(chat bubble icon + count)Test Plan
Summary by CodeRabbit