feat(desktop): add Kaleidoscope-style git diff viewer in Changes tab#310
feat(desktop): add Kaleidoscope-style git diff viewer in Changes tab#310
Conversation
Implement a comprehensive git diff viewer similar to Kaleidoscope: - Add tRPC router for git operations (getStatus, getFileContents, stage/unstage) - Create Zustand store for changes state management - Build sidebar components (ChangesView, CategorySection, FileItem, CommitItem) - Build content components (DiffViewer with Monaco, DiffToolbar, FileHeader) - Configure Monaco editor for Electron environment with web workers - Support four change categories: Against Main, Commits, Staged, Unstaged - Add side-by-side and inline diff view modes - Implement auto-refresh via 2.5s polling - Add stage/unstage/discard file actions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 4 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (7)
WalkthroughIntroduces comprehensive git changes/diff viewing for the desktop application. Adds a new tRPC router with 10 procedures for branch enumeration, status polling, file content diffing, and staging/unstaging operations. Implements a multi-component ChangesView sidebar with file listings, categorized sections, and a Monaco-based diff editor in the main content area. Integrates Monaco editor with custom theme support and adds color conversion utilities. Changes
Sequence DiagramsequenceDiagram
actor User
participant ChangesView as ChangesView<br/>(Sidebar)
participant Store as Zustand Store
participant tRPC as tRPC Client
participant Router as tRPC Router<br/>(Backend)
participant Git as Git Operations
participant Editor as Monaco Diff<br/>Editor
User->>ChangesView: Mount/Focus window
activate ChangesView
ChangesView->>tRPC: getStatus(worktreePath)
activate tRPC
tRPC->>Router: getStatus procedure
activate Router
Router->>Git: git status, log, diff-numstat
Git-->>Router: status/log/diff data
Router->>Router: parseGitStatus,<br/>parseDiffNumstat
Router-->>tRPC: GitChangesStatus
deactivate Router
tRPC-->>ChangesView: status + commits
deactivate tRPC
ChangesView->>Store: selectFile(file)
deactivate ChangesView
User->>ChangesView: Click file item
activate ChangesView
ChangesView->>Store: selectFile(file)
Store->>ChangesView: emit state update
ChangesView->>tRPC: getFileContents(filePath, category)
activate tRPC
tRPC->>Router: getFileContents procedure
Router->>Git: git show/read<br/>file contents
Git-->>Router: original & modified
Router->>Router: detectLanguage
Router-->>tRPC: FileContents
deactivate tRPC
tRPC-->>Editor: contents + language
activate Editor
Editor->>Editor: render Monaco<br/>DiffEditor
deactivate Editor
deactivate ChangesView
User->>Editor: Click "Stage" button
activate Editor
Editor->>tRPC: stageFile mutation
activate tRPC
tRPC->>Router: stageFile procedure
Router->>Git: git add
Git-->>Router: ✓ success
Router-->>tRPC: { success: true }
deactivate tRPC
tRPC->>Store: invalidate caches
Store->>ChangesView: refetch status
changesView->>tRPC: getStatus
tRPC->>Router: getStatus
Router->>Git: git status
Git-->>Router: updated status
Router-->>tRPC: GitChangesStatus
tRPC-->>ChangesView: updated state
deactivate Editor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/package.json (1)
36-69: Update@monaco-editor/reactto supportmonaco-editor@0.55.1
@monaco-editor/react@4.7.0bundles@monaco-editor/loader@1.5.0, which targetsmonaco-editor@0.52.2. To usemonaco-editor@0.55.1, update to@monaco-editor/react@4.8.0-rc.3(which includes@monaco-editor/loader@1.7.0with official 0.55.x support), or downgrademonaco-editorto^0.52.2. The current pairing will cause loader initialization failures when the diff viewer integrates Monaco.
🧹 Nitpick comments (15)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx (1)
1-57: Solid Monaco diff integration; add a small guard aroundself.MonacoEnvironmentThis component is well-structured: type-safe props, good use of
DiffEditorwithrenderSideBySidetied toDiffViewMode, sensible read-only/options, and correctloader.config({ monaco })usage for the local Monaco instance.One robustness tweak: the direct assignment
self.MonacoEnvironment = { getWorker(_: unknown, _label: string) { return new editorWorker(); }, };(Line 8) will throw if this module is ever imported in a non-browser/globalThis environment (e.g., Node-based tests or any SSR usage). You can keep the same runtime behavior while making it safer by guarding on
typeof self:- self.MonacoEnvironment = { - getWorker(_: unknown, _label: string) { - return new editorWorker(); - }, - }; + if (typeof self !== "undefined") { + (self as typeof globalThis & { MonacoEnvironment?: unknown }).MonacoEnvironment = { + getWorker(_: unknown, _label: string) { + return new editorWorker(); + }, + }; + }This avoids reference errors while preserving the existing behavior in the Electron renderer.
If you have any Jest/Bun tests or tooling that import
DiffVieweroutside the Electron renderer, please run them after this change to confirm there are no regressions and that Monaco workers still initialize correctly.apps/desktop/src/renderer/stores/changes/store.ts (1)
35-46: Consider typinginitialStateexplicitly to avoid type assertions.The type assertions (
as ChangeCategory,as DiffViewMode) work but are less type-safe. A typed constant would catch typos at compile time.+const initialState: Pick< + ChangesState, + | "selectedCategory" + | "selectedFile" + | "selectedCommitHash" + | "viewMode" + | "expandedSections" +> = { -const initialState = { - selectedCategory: "against-main" as ChangeCategory, + selectedCategory: "against-main", selectedFile: null, selectedCommitHash: null, - viewMode: "side-by-side" as DiffViewMode, + viewMode: "side-by-side", expandedSections: { "against-main": true, committed: true, staged: true, unstaged: true, }, };apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (2)
26-37: Track the hardcodeddefaultBranchvalue.The
defaultBranch: "main"is hardcoded as noted in the TODO. This should be retrieved from project/repository settings to support repos usingmasteror other default branches.Would you like me to open an issue to track retrieving the default branch from git configuration or project settings?
39-52: Consider clearingselectedFileafter successful discard.After discarding changes, the file may no longer have modifications. Consider clearing the selection or re-validating the file state to avoid showing stale UI.
const discardChanges = trpc.changes.discardChanges.useMutation({ onSuccess: () => { utils.changes.getStatus.invalidate(); utils.changes.getFileContents.invalidate(); + // Clear selection since file may no longer have changes + useChangesStore.getState().selectFile(null); }, });apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (2)
11-26: UseFileStatustype instead ofstringfor better type safety.The
getStatusColorfunction acceptsstringbut should use theFileStatustype for consistency and compile-time checks. Also, the "copied" status is handled ingetStatusIndicatorbut missing here.-function getStatusColor(status: string): string { +function getStatusColor(status: FileStatus): string { switch (status) { case "added": return "text-green-500"; case "modified": return "text-yellow-500"; case "deleted": return "text-red-500"; case "renamed": + case "copied": return "text-blue-500"; case "untracked": return "text-muted-foreground"; default: return "text-muted-foreground"; } }Add the import:
-import type { ChangedFile } from "shared/changes-types"; +import type { ChangedFile, FileStatus } from "shared/changes-types";
28-45: Same type safety improvement applies here.Use
FileStatusinstead ofstringfor thestatusparameter.-function getStatusIndicator(status: string): string { +function getStatusIndicator(status: FileStatus): string {apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (2)
49-66: Potential stale data issue withexpandedCommitsiteration order.The
commitFilesMapconstruction relies on the iteration order ofexpandedCommitsmatching the query array order. UsingArray.from(expandedCommits)twice (lines 51 and 61) should be consistent, but if the Set changes between renders, the indices could mismatch.Consider storing the commit hashes array in a ref or useMemo to ensure consistency:
+import { useMemo } from "react"; - // Fetch files for expanded commits - const commitFilesQueries = trpc.useQueries((t) => - Array.from(expandedCommits).map((hash) => + // Memoize expanded commits array for consistent ordering + const expandedCommitsArray = useMemo( + () => Array.from(expandedCommits), + [expandedCommits], + ); + + // Fetch files for expanded commits + const commitFilesQueries = trpc.useQueries((t) => + expandedCommitsArray.map((hash) => t.changes.getCommitFiles({ worktreePath: worktreePath || "", commitHash: hash, }), ), ); // Build a map of commit hash -> files const commitFilesMap = new Map<string, ChangedFile[]>(); - Array.from(expandedCommits).forEach((hash, index) => { + expandedCommitsArray.forEach((hash, index) => { const query = commitFilesQueries[index]; if (query?.data) { commitFilesMap.set(hash, query.data); } });
15-16: Hardcoded default branch needs API exposure from backend.The
"main"fallback is appropriate for now, but the real issue is thatdefaultBranchshould be exposed via the tRPC API. The backend already storesdefaultBranchin the Project schema (apps/desktop/src/lib/trpc/routers/projects/projects.ts), but thegetActivequery returns only the Workspace object without the associated Project'sdefaultBranch. Extend thegetActivereturn type to includedefaultBranchfrom the project, or add it to the Workspace interface.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
36-46: Consider adding an aria-label for accessibility.The refresh button only contains an icon without text. Screen readers won't have context for what the button does.
<Button variant="ghost" size="sm" onClick={onRefresh} disabled={isRefreshing} className="h-7 w-7 p-0" + aria-label={isRefreshing ? "Refreshing..." : "Refresh changes"} > <HiArrowPath className={`w-4 h-4 ${isRefreshing ? "animate-spin" : ""}`} /> </Button>apps/desktop/src/lib/trpc/routers/changes/changes.ts (4)
74-96:againstMaindiff is gated onahead > 0, which can hide relevant changesCurrently the
againstMainfile list is only populated whenahead > 0, so scenarios where the branch is only behind or otherwise diverged fromorigin/${defaultBranch}will show no “Against Main” files even though a diff exists. Consider computingagainstMainwhenever the branch differs from the default (e.g.,ahead > 0 || behind > 0) or unconditionally, and letting the UI decide how to present an empty list.
235-258: Rename diffs in thecommittedcategory will render as pure add/deleteFor
category: "committed",originalis loaded from${commitHash}^:${input.filePath}andmodifiedfrom${commitHash}:${input.filePath}. If the file was renamed in this commit,input.filePathwill correspond to the new path, but the parent commit only knows the old path, so theoriginallookup will fail and fall back to"". That turns a rename into “new file” in the diff viewer. Consider extending the input to includeoldPathfor committed files and using that for the parent lookup when present.
279-303: File-system operations trustfilePathwithout extra normalization or scoping checksFor the
unstagedcase you read fromjoin(input.worktreePath, input.filePath), and fordeleteUntrackedyouunlinkthe same join. This is fine as long asfilePathalways comes from git status and is repo-relative, but it’s worth hardening against accidental directory traversal or misuse (e.g., a future caller passing something like../foo). A simple guard like resolving the path and verifying it remains underworktreePathbefore reading/deleting would make these mutations safer.Also applies to: 387-397
317-328: Destructive git operations lack error handling/feedback beyond{ success: true }The mutation handlers for stage/unstage/discard/stageAll/unstageAll/deleteUntracked all
awaitthe git/fs operations and then unconditionally return{ success: true }. If git fails (e.g., invalid path, permissions, non‑git directory), the error will throw to the tRPC layer but the shape doesn’t convey failure explicitly. Consider wrapping these in small try/catch blocks that either:
- map known git errors to structured tRPC errors, or
- return
{ success: false, error?: string }so the UI can surface failures cleanly.Also applies to: 333-344, 349-360, 365-382, 387-397
apps/desktop/docs/CHANGES_TAB_IMPLEMENTATION.md (2)
66-75: Add fenced code block languages to satisfy markdownlint and improve renderingSeveral fenced blocks that show directory trees and ASCII layouts use plain ``` without a language (e.g., under “Create: .../changes”, “Create: .../stores/changes”, “Sidebar Component (ChangesView)”, “UI Structure”, “Content Component (ChangesContent)”, “UI Layout”). To address MD040 and help renderers, consider tagging them as
text:-``` +```textApply this to each non-code fence that currently omits a language.
Also applies to: 105-111, 145-167, 169-191, 197-217, 219-232
156-164: Component names in the plan diverge from the implemented componentsThe sidebar plan refers to
FileTreeItemandFileStatusBadge, while the rest of the PR (per the summary) usesFileItemand slightly different component organization. Aligning the documented component names and folder structure with the actual code (e.g.,FileItem/FileItem.tsx) will make this document a reliable map for new contributors.Also applies to: 171-188
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
apps/desktop/docs/CHANGES_TAB_IMPLEMENTATION.md(1 hunks)apps/desktop/electron.vite.config.ts(1 hunks)apps/desktop/package.json(2 hunks)apps/desktop/src/lib/trpc/routers/changes/changes.ts(1 hunks)apps/desktop/src/lib/trpc/routers/changes/index.ts(1 hunks)apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts(1 hunks)apps/desktop/src/lib/trpc/routers/index.ts(2 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent.tsx(0 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView.tsx(0 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts(1 hunks)apps/desktop/src/renderer/stores/changes/index.ts(1 hunks)apps/desktop/src/renderer/stores/changes/store.ts(1 hunks)apps/desktop/src/shared/changes-types.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/lib/trpc/routers/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/stores/changes/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/shared/changes-types.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/lib/trpc/routers/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/stores/changes/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/shared/changes-types.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/lib/trpc/routers/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/stores/changes/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsxapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/shared/changes-types.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/stores/changes/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
apps/desktop/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Files:
apps/desktop/src/lib/trpc/routers/changes/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.tsapps/desktop/src/lib/trpc/routers/changes/changes.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/package.jsonapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/stores/changes/store.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/shared/changes-types.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/docs/CHANGES_TAB_IMPLEMENTATION.md
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - only in src/main/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/electron.vite.config.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/stores/changes/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/shared/changes-types.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/terminal-*.ts : Use node-pty for terminal session management in the desktop app
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Use React + TailwindCSS v4 + shadcn/ui for UI components
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/[A-Z]*.tsx : Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
apps/desktop/src/renderer/stores/changes/store.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/lib/**/*.{ts,tsx} : Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Applied to files:
apps/desktop/src/lib/trpc/routers/index.ts
🧬 Code graph analysis (12)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (3)
apps/desktop/src/shared/changes-types.ts (2)
CommitInfo(36-49)ChangedFile(22-33)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (1)
FileItem(57-107)
apps/desktop/src/renderer/stores/changes/store.ts (1)
apps/desktop/src/shared/changes-types.ts (3)
ChangeCategory(15-19)ChangedFile(22-33)DiffViewMode(74-74)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.ts (1)
EmptyState(1-1)
apps/desktop/src/lib/trpc/routers/index.ts (2)
apps/desktop/src/lib/trpc/routers/changes/changes.ts (1)
createChangesRouter(22-400)apps/desktop/src/lib/trpc/routers/changes/index.ts (1)
createChangesRouter(1-1)
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (1)
apps/desktop/src/shared/changes-types.ts (4)
FileStatus(6-12)ChangedFile(22-33)GitChangesStatus(52-71)CommitInfo(36-49)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (5)
apps/desktop/src/renderer/stores/changes/store.ts (1)
useChangesStore(48-108)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsx (1)
EmptyState(8-18)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (1)
FileHeader(44-87)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsx (1)
DiffToolbar(22-96)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx (1)
DiffViewer(22-57)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (3)
apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-33)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.ts (1)
FileItem(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsx (4)
apps/desktop/src/shared/changes-types.ts (2)
DiffViewMode(74-74)ChangeCategory(15-19)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.ts (1)
DiffToolbar(1-1)packages/ui/src/components/toggle-group.tsx (2)
ToggleGroup(82-82)ToggleGroupItem(82-82)packages/ui/src/components/button.tsx (1)
Button(61-61)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.ts (1)
CategorySection(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (3)
apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-33)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.ts (1)
FileHeader(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/lib/trpc/routers/changes/changes.ts (4)
apps/desktop/src/lib/trpc/routers/changes/index.ts (1)
createChangesRouter(1-1)apps/desktop/src/lib/trpc/index.ts (1)
router(15-15)apps/desktop/src/shared/changes-types.ts (3)
GitChangesStatus(52-71)ChangedFile(22-33)FileContents(86-93)apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (5)
parseGitStatus(42-89)parseGitLog(95-118)parseNameStatus(150-196)parseDiffNumstat(124-144)detectLanguage(201-261)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.ts (1)
ChangesHeader(1-1)packages/ui/src/components/button.tsx (1)
Button(61-61)
🪛 markdownlint-cli2 (0.18.1)
apps/desktop/docs/CHANGES_TAB_IMPLEMENTATION.md
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
106-106: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
220-220: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (36)
apps/desktop/src/lib/trpc/routers/changes/index.ts (1)
1-1: Clean barrel export for changes routerThe re-export of
createChangesRouteris straightforward and matches the expected router barrel pattern, keeping the TRPC router surface tidy.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (1)
1-1: Barrel export matches component folder conventionThis index re-export of
ChangesViewfollows the one-folder-per-component + barrel pattern and keeps consumer imports clean.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/index.ts (1)
1-1: Consistent CommitItem barrel export
CommitItemis correctly exposed via an index barrel, matching the established component structure and simplifying imports from the Changes view.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.ts (1)
1-1: FileItem barrel export looks goodThe
FileItemre-export via index.ts follows the component folder convention and makes the Changes sidebar composition cleaner.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.ts (1)
1-1: FileHeader barrel export is consistent and tidyRe-exporting
FileHeaderfrom the index keeps the ChangesContent imports concise and matches the project’s component export pattern.apps/desktop/electron.vite.config.ts (1)
91-98: Monaco worker and optimizeDeps configuration is correct and complete.The
worker: { format: "es" }andoptimizeDeps.include: ["monaco-editor"]configuration properly supports the ESM worker import pattern already used in DiffViewer.tsx (monaco-editor/esm/vs/editor/editor.worker?worker) with the MonacoEnvironment callback correctly set up. No changes needed.apps/desktop/src/renderer/stores/changes/index.ts (1)
1-1: Barrel export foruseChangesStorelooks correctRe-export matches the underlying module name and follows the existing store barrel pattern.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/index.ts (1)
1-1: CategorySection barrel matches component-folder patternExport is named and pathed correctly and follows the component folder + index.ts convention.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/index.ts (1)
1-1: EmptyState barrel export is consistentNaming and local path are correct and align with other ChangesContent component barrels.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/index.ts (1)
1-1: ChangesHeader barrel export follows the component conventionRe-export is correct and keeps the ChangesView component API clean and consistent.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/index.ts (1)
1-1: ChangesContent barrel export is wired correctlyThe export cleanly exposes
ChangesContentand matches the surrounding WorkspaceView barrel patterns.apps/desktop/src/lib/trpc/routers/index.ts (1)
3-3: Changes router is correctly mounted on the main tRPC app routerImporting
createChangesRouterand adding thechangesfield tocreateAppRouteris consistent with the existing domain router pattern and ensures the new changes procedures are exposed throughAppRouterwithout affecting window-dependent routers.Also applies to: 28-35
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/index.ts (1)
1-1: DiffToolbar barrel export is correctThe barrel cleanly re-exports
DiffToolbar, matching the component filename and simplifying imports for ChangesContent.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/index.ts (1)
1-1: DiffViewer barrel export is aligned with renderer patternsThe export correctly surfaces
DiffViewerfrom its component file and fits the renderer’s barrel-export convention.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsx (1)
1-18: LGTM!Clean, reusable empty state component with proper prop typing and conditional rendering. The folder structure with barrel export follows the project conventions.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx (1)
1-53: LGTM!Well-structured collapsible section component. The early return for
count === 0is a sensible UX choice, and the accessibility attributes (type="button") are properly set.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (2)
71-82: Consider using a composite key forFileItemwhen files can have the same path across commits.Using
file.pathas the key works within a single commit, but if commits are reused in lists or the same path appears in multiple contexts, ensure React's reconciliation remains correct. This is fine as-is since eachCommitItemrenders its own isolated list.
15-27: No action needed. The tRPC client inapps/desktop/src/renderer/lib/trpc-client.tsis properly configured with the superjson transformer, which automatically serializes and deserializes Date objects. Thecommit.dateproperty will be a Date instance at runtime, soformatRelativeDatereceives the correct type without any parsing required.Likely an incorrect or invalid review comment.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (1)
44-87: LGTM!The component structure is clean with proper conditional rendering for optional elements (oldPath, stats). The status badge styling with
border-current/30 bg-current/10is a nice approach for dynamic theming.apps/desktop/src/renderer/stores/changes/store.ts (1)
90-95: Confirm thatreset()intentionally preservesselectedCategory.The
resetaction clearsselectedFileandselectedCommitHashbut keepsselectedCategory. If this is intentional (e.g., to keep the user's category context), this is fine. Otherwise, consider resetting it to the default.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsx (1)
1-96: LGTM!Well-implemented toolbar with proper conditional action buttons based on category. The
isActioningflag correctly prevents concurrent mutations, and the destructive styling on the Discard button provides clear visual feedback for the dangerous action.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (1)
1-19: LGTM on overall structure.Good use of tRPC for data fetching and mutations per coding guidelines. The store integration is clean, and the query enabling logic correctly prevents unnecessary fetches.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (2)
47-55: LGTM!Path manipulation helpers are correctly implemented for the renderer process without Node.js dependencies.
57-106: LGTM!Clean component implementation with proper button semantics, accessible click handling, and conditional rendering.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (3)
68-90: LGTM!Event handlers are well-implemented with proper state updates and functional Set manipulation.
92-122: LGTM!Loading and error states are properly handled with appropriate messaging.
130-236: LGTM!Well-structured render with clear category sections and proper selection state handling.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
21-35: LGTM!Clean conditional rendering for the ahead/behind status with proper visual hierarchy.
apps/desktop/src/shared/changes-types.ts (2)
1-33: LGTM!Well-documented type definitions with clear JSDoc comments. The types provide a solid foundation for the git changes feature.
Also applies to: 51-93
35-49: No changes required. Thedatefield inCommitInfois properly handled by tRPC's superjson transformer, which automatically serializes and deserializes Date objects. The renderer'sformatRelativeDatefunction correctly receives a Date object.apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (6)
1-7: LGTM!Clean imports with type-only import for
StatusResultfrom simple-git, which is appropriate for this router utility file.
9-21: LGTM!Git status mapping correctly handles the various status codes from both index and working tree.
42-89: LGTM!Correct categorization of git status results into staged, unstaged, and untracked, with proper handling of files that may appear in multiple categories.
120-144: LGTM!Good handling of binary files and paths containing tabs.
146-196: LGTM!Correct parsing of git name-status output with proper handling of rename/copy status codes that include similarity percentages.
198-261: LGTM!Comprehensive language detection with good coverage of common file types and appropriate plaintext fallback.
| function getStatusColor(status: string): string { | ||
| switch (status) { | ||
| case "added": | ||
| return "text-green-500"; | ||
| case "modified": | ||
| return "text-yellow-500"; | ||
| case "deleted": | ||
| return "text-red-500"; | ||
| case "renamed": | ||
| return "text-blue-500"; | ||
| case "untracked": | ||
| return "text-muted-foreground"; | ||
| default: | ||
| return "text-muted-foreground"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Add handling for "copied" status and use FileStatus type.
The getStatusLabel function handles "copied" status, but getStatusColor does not, causing inconsistent styling for copied files. Also, using the FileStatus type instead of string would improve type safety.
+import type { ChangedFile, FileStatus } from "shared/changes-types";
-function getStatusColor(status: string): string {
+function getStatusColor(status: FileStatus): string {
switch (status) {
case "added":
return "text-green-500";
case "modified":
return "text-yellow-500";
case "deleted":
return "text-red-500";
case "renamed":
+ case "copied":
return "text-blue-500";
case "untracked":
return "text-muted-foreground";
default:
return "text-muted-foreground";
}
}
-function getStatusLabel(status: string): string {
+function getStatusLabel(status: FileStatus): string {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx
around lines 8 to 23, the getStatusColor function uses a plain string parameter
and is missing a case for the "copied" status; change the parameter type from
string to FileStatus (import the FileStatus type from the module where file
statuses are defined) and add a case for "copied" returning the same CSS class
used for copied/renamed in getStatusLabel (e.g., "text-blue-500"); keep the
default fallback as "text-muted-foreground".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (1)
11-18: TightenonToggletyping to reflectCollapsible’sonOpenChangesemanticsCurrent props use
onToggle: () => voidbut it’s wired toCollapsible’sonOpenChange, which usually passes the nextopenstate. Exposing that boolean in the prop keeps types aligned and makes it easier for callers to driveisExpandedfrom the new value instead of manually toggling.Consider:
interface CommitItemProps { // ... - onToggle: () => void; + onToggle: (isOpen: boolean) => void; } <Collapsible open={isExpanded} - onOpenChange={onToggle} + onOpenChange={onToggle} >Callers that don’t care about
isOpencan still pass() => { /* ignore */ }, but those that do will have the information available.Also applies to: 34-41
packages/ui/src/components/accordion.tsx (1)
48-60: Consider applyingclassNameto the outerAccordionContentelementRight now
classNameis applied to the inner<div>, so consumers can’t easily style the RadixContentcontainer itself (animations/overflow/etc. stay fixed). For consistency with other primitives and more flexible theming, consider:- <AccordionPrimitive.Content - data-slot="accordion-content" - className="data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down overflow-hidden text-sm" - {...props} - > - <div className={cn("pt-0 pb-4", className)}>{children}</div> - </AccordionPrimitive.Content> + <AccordionPrimitive.Content + data-slot="accordion-content" + className={cn( + "data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down overflow-hidden text-sm", + className, + )} + {...props} + > + <div className="pt-0 pb-4">{children}</div> + </AccordionPrimitive.Content>This keeps the padding wrapper while letting callers style the outer content element.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsx (1)
1-103: GroupedView logic is solid; consider tightening status helper typingsThe grouping/expand behavior and per‑file row rendering (name, status badge, staged dot, rename hint) look good and match the
FolderGroup/GitFileshapes.Given
file.statusis aGitFileStatusunion, you can improve type safety by narrowing the helper signatures inutils.tsfromstringtoGitFileStatus, so any new status values must be handled explicitly:- export function getStatusColor(status: string): string { + import type { GitFileStatus } from "./types"; + + export function getStatusColor(status: GitFileStatus): string { // ... - export function getStatusIndicator(status: string): string { + export function getStatusIndicator(status: GitFileStatus): string { // ...This will benefit both
GroupedViewandFileTreeViewcall sites.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx (1)
1-78: SimplifyhandleFileSelectto accept onlyGitFileAll current call sites (
FileTreeViewmappingfiles.find(...)andGroupedView) pass aGitFileintohandleFileSelect. The broader union type:(file: GitFile | { path: string; status?: string })and the
"id" in fileruntime guard add complexity without benefit and weaken type safety.You can simplify this to:
- const handleFileSelect = ( - file: GitFile | { path: string; status?: string }, - ) => { - if (onFileSelect && "id" in file) { - onFileSelect(file as GitFile); - } - }; + const handleFileSelect = (file: GitFile) => { + onFileSelect?.(file); + };This keeps the API clear and lets TypeScript catch any future non‑
GitFileusage.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsx (1)
1-104: Tree rendering looks good; align status helper types withGitFileStatusThe recursive
TreeNodeimplementation (folder vs file handling, indentation bylevel, and staged/status indicators) is clean and matches theFileTreeNodemodel.As with
GroupedView, you’re passing a strongly-typednode.status?: GitFileStatusinto helpers that currently acceptstring. After updatinggetStatusColor/getStatusIndicatorto takeGitFileStatus(see suggested diff inGroupedView.tsx), these lines:const statusColor = node.status ? getStatusColor(node.status) : ""; const statusIndicator = node.status ? getStatusIndicator(node.status) : "";will benefit from stricter exhaustiveness checking when new statuses are added.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (2)
47-62: Consider extractingconvertToArrayfor testability.The nested function is well-implemented with proper sorting (folders-first, locale-aware), but extracting it to module scope would make it easier to unit test the conversion and sorting logic independently.
Example:
+function convertToArray( + nodes: Record<string, TreeNodeInternal>, +): FileTreeNode[] { + return Object.values(nodes) + .map((node) => ({ + ...node, + children: node.children ? convertToArray(node.children) : undefined, + })) + .sort((a, b) => { + // Folders first, then files + if (a.type !== b.type) { + return a.type === "folder" ? -1 : 1; + } + return a.name.localeCompare(b.name); + }); +} + export function buildFileTree(files: GitFile[]): FileTreeNode[] { const root: Record<string, TreeNodeInternal> = {}; // ... tree building logic ... - - // Convert nested objects to arrays and sort - function convertToArray( - nodes: Record<string, TreeNodeInternal>, - ): FileTreeNode[] { - return Object.values(nodes) - .map((node) => ({ - ...node, - children: node.children ? convertToArray(node.children) : undefined, - })) - .sort((a, b) => { - // Folders first, then files - if (a.type !== b.type) { - return a.type === "folder" ? -1 : 1; - } - return a.name.localeCompare(b.name); - }); - } return convertToArray(root); }
78-81: Optional: Remove unnecessary optional chaining.The optional chaining at line 81 is redundant since the array is guaranteed to exist after the check and creation at lines 78-80.
if (!folderMap.has(folderPath)) { folderMap.set(folderPath, []); } - folderMap.get(folderPath)?.push(file); + folderMap.get(folderPath)!.push(file);Or more idiomatically:
- if (!folderMap.has(folderPath)) { - folderMap.set(folderPath, []); - } - folderMap.get(folderPath)?.push(file); + const existingFiles = folderMap.get(folderPath); + if (existingFiles) { + existingFiles.push(file); + } else { + folderMap.set(folderPath, [file]); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ModeCarousel/ModeContent.tsx(0 hunks)packages/ui/package.json(2 hunks)packages/ui/src/components/accordion.tsx(1 hunks)packages/ui/src/components/collapsible.tsx(1 hunks)packages/ui/src/components/toggle-group.tsx(1 hunks)packages/ui/src/components/toggle.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ModeCarousel/ModeContent.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
packages/ui/src/components/toggle.tsxpackages/ui/src/components/collapsible.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxpackages/ui/src/components/accordion.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.tspackages/ui/src/components/toggle-group.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
packages/ui/src/components/toggle.tsxpackages/ui/src/components/collapsible.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxpackages/ui/src/components/accordion.tsxpackages/ui/src/components/toggle-group.tsxpackages/ui/package.json
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
packages/ui/src/components/toggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxpackages/ui/src/components/toggle-group.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tspackages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Use React + TailwindCSS v4 + shadcn/ui for UI components
Applied to files:
packages/ui/src/components/collapsible.tsxpackages/ui/src/components/accordion.tsxpackages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.tspackages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/[A-Z]*.tsx : Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsxpackages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/lib/**/*.{ts,tsx} : Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tspackages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/*.test.{ts,tsx} : Co-locate tests with components using the naming pattern ComponentName.test.tsx
Applied to files:
packages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - only in src/main/
Applied to files:
packages/ui/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/*.{ts,tsx} : Maintain type safety and avoid using `any` unless absolutely necessary in TypeScript code
Applied to files:
packages/ui/package.json
🧬 Code graph analysis (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsx (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts (1)
FileTreeNode(4-4)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts (1)
FileTreeNode(19-28)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (2)
getStatusColor(108-123)getStatusIndicator(128-143)packages/ui/src/components/collapsible.tsx (3)
Collapsible(32-32)CollapsibleTrigger(32-32)CollapsibleContent(32-32)packages/ui/src/lib/utils.ts (1)
cn(4-6)
packages/ui/src/components/accordion.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts (3)
FileTreeNode(19-28)GitFile(8-15)FolderGroup(30-34)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (2)
apps/desktop/src/shared/changes-types.ts (2)
CommitInfo(36-49)ChangedFile(22-33)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (1)
FileItem(57-107)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts (2)
GitFile(8-15)ViewMode(17-17)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts (1)
mockGitFiles(3-131)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (2)
buildFileTree(13-65)groupFilesByFolder(70-103)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/FileTreeView.tsx (1)
FileTreeView(90-104)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GroupedView.tsx (1)
GroupedView(85-103)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts (2)
mockGitFiles(2-2)GitFile(6-6)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts (1)
GitFile(8-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (15)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (2)
20-32: Ensurecommit.dateis a realDateinstance in the renderer
formatRelativeDateassumescommit.dateis aDate(usesgetTime()andtoLocaleDateString()), which matches the sharedCommitInfotype. If commit data crosses IPC/tRPC boundaries as JSON without a transformer, the field may be a string at runtime and this will break.Please verify that your tRPC transformer/serialization preserves
Dateobjects forCommitInfo.date; if not, convert at the edge (e.g.,new Date(commit.date)in the store or API adapter) before it reaches this component.Also applies to: 69-70
42-90: Clean, well-structured commit row & file list; aligns with renderer guidelinesThe controlled
Collapsibleusage, selection logic (selectedCommitHash+selectedFile?.path), and reuse ofFileItemare all clear and consistent. Imports use theshared/changes-typesalias and avoid Node.js modules in the renderer, matching the coding guidelines. No issues from a correctness or UX standpoint here.packages/ui/package.json (1)
29-32: Exports and Radix deps look consistent with new UI primitivesThe new export entries and added Radix dependencies line up with the new
accordion,collapsible, and toggle components insrc/components. No issues from a packaging standpoint.Also applies to: 41-43
packages/ui/src/components/accordion.tsx (1)
1-62: Solid Radix accordion wrapper with sensible defaultsThe component set (Accordion, AccordionItem, AccordionTrigger, AccordionContent) cleanly forwards props to Radix, preserves typing via
React.ComponentProps, and applies consistent styling viacn. No correctness or type-safety issues stand out.packages/ui/src/components/toggle.tsx (1)
5-5: Relativecnimport is preferable inside the UI packageSwitching
cnto../lib/utilsavoids relying on app-level path aliases from within the published UI package, which should make builds and external consumption more robust.packages/ui/src/components/toggle-group.tsx (1)
6-7: Localizing imports keeps the UI package self-containedUsing relative imports for
cnandtoggleVariantswithinpackages/uiavoids coupling this package to app-level TS path aliases and matches the neighboringtogglecomponent pattern.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/mockData.ts (1)
1-131: Mock data shape and coverage look solidThe mock entries line up with
GitFile(including a renamed case witholdPath) and cover all status variants, which is useful for exercising the UI states.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/types.ts (1)
1-34: Type definitions are clear and type‑safeThe
GitFileStatusunion and theGitFile/FileTreeNode/FolderGroupmodels are well-structured and avoidany, giving good compile-time guarantees for the rest of the feature.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx (1)
1-60: CategorySection controlled collapsible behavior looks goodThe controlled
Collapsibleusage, header structure (chevron, title, count, optional actions), and the early return whencount === 0are all consistent and straightforward; no issues from a logic or type-safety perspective.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts (1)
1-10: Barrel exports are well-organizedThis index cleanly exposes the main component, mock data, types, and utilities for the Git file tree feature, which should make imports elsewhere in the app straightforward.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (5)
3-8: LGTM! Well-designed internal type.Using a Record for children during tree construction is an efficient approach that simplifies node lookup and insertion. The conversion to arrays happens later in convertToArray.
16-44: LGTM! Tree building logic is sound.The algorithm correctly builds a hierarchical tree structure by:
- Splitting paths and navigating/creating nodes
- Distinguishing files from folders based on path position
- Setting file-specific properties only on leaf nodes
The code implicitly relies on Git's guarantee that a path component cannot be both a file and a directory, which is a valid assumption.
84-102: LGTM! Grouping and sorting logic is well-implemented.The function correctly:
- Extracts folder names from paths with proper handling of root-level files
- Sorts files within each group alphabetically
- Prioritizes the "root" folder group, then sorts remaining groups by path
105-123: LGTM! Status color mapping is comprehensive and conventional.The color choices follow standard Git UI conventions (green for additions, red for deletions, etc.) and include a sensible default fallback.
125-143: LGTM! Status indicator mapping is consistent and conventional.The single-character indicators match standard Git conventions and align well with the
getStatusColorfunction. The empty string default is appropriate for unknown statuses.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (1)
86-94: Add a confirmation step before destructive discard/delete
handleDiscardcurrently callsdiscardChanges/deleteUntrackeddirectly on click, which can irreversibly drop work (tracked changes or untracked files) with no user confirmation. This should be gated behind an explicit confirm dialog (e.g.,AlertDialog) so accidental clicks don’t lose data.You could introduce local state to show a confirm modal and only invoke the appropriate mutation inside a confirmed handler, e.g.:
+ const [confirmingDiscard, setConfirmingDiscard] = useState(false); const handleDiscard = () => { - if (!worktreePath || !selectedFile) return; - // TODO: Add confirmation dialog - if (selectedFile.status === "untracked") { - deleteUntracked.mutate({ worktreePath, filePath: selectedFile.path }); - } else { - discardChanges.mutate({ worktreePath, filePath: selectedFile.path }); - } + if (!worktreePath || !selectedFile) return; + setConfirmingDiscard(true); };and then wire
confirmDiscard/cancelDiscardinto anAlertDialogthat performs the guarded mutation and resetsconfirmingDiscard.apps/desktop/src/lib/trpc/routers/changes/changes.ts (1)
148-155: Renamed/copied files still won’t get correct additions/deletions from numstatIn all four places where you enrich files with numstat:
againstMain(Lines 148–155)- staged (Lines 169–176)
- unstaged (Lines 187–193)
- commit files (Lines 245–249)
you do
stats.get(file.path). For renames/copies,parseNameStatussetsfile.pathto the new path whileparseDiffNumstatkeys stats using the rawnumstatpath (e.g."old.txt => new.txt"), so lookups for renames/copies still fail and their additions/deletions remain zero.A minimal fix at the call sites is to fall back to an
oldPath-based key when the simple lookup misses, e.g.:- const fileStat = stats.get(file.path); + const fileStat = + stats.get(file.path) ?? + (file.oldPath + ? stats.get(`${file.oldPath} => ${file.path}`) + : undefined);and the analogous change for staged/unstaged/commit sections.
Alternatively, you could normalize rename paths inside
parseDiffNumstat, but either way the current lookups won’t populate stats for renames/copies.Also applies to: 169-176, 187-193, 245-249
🧹 Nitpick comments (13)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts (1)
3-14: Consider a discriminated union for stronger type safety.The
FileTreeNodeinterface includes atypediscriminator but doesn't leverage it. Files and folders have different properties (files have status/additions/deletions, folders have children), yet all fields are optional. A discriminated union would ensure type correctness:type FileNode = { id: string; name: string; type: "file"; path: string; status?: string; additions?: number; deletions?: number; oldPath?: string; category?: string; }; type FolderNode = { id: string; name: string; type: "folder"; path: string; children?: FileTreeNode[]; }; export type FileTreeNode = FileNode | FolderNode;Additionally, the
categoryfield could be more strictly typed if it has a known set of values (e.g., "Against Main" | "Commits" | "Staged" | "Unstaged" based on the PR description).apps/desktop/src/main/lib/app-state/index.ts (1)
44-49: Binding methods to_appStateis correct; consider clarifying write semantics on the proxyThe new binding logic is a good fix: it preserves the expected
thisfor lowdb-style methods (e.g.read/write) even when they’re accessed throughappStateor destructured.One small ergonomics note: since the Proxy only traps
get, any direct assignments likeappState.data = …will still write to the dummy target object, not_appState. If you intendappStateto be read-only at the top level, you might want to explicitly disallowset; if you ever want assignments to work, asettrap that forwards to_appStatewould avoid surprising behavior. This is pre-existing, not introduced by this change, but worth being explicit about.apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (2)
18-112: Reduce duplication in token rules and centralize hex stripping
keyword.operatoris defined twice (Lines 33 and 78) and.replace("#", "")is repeated many times. You could DRY this up with a small helper likehex(color)and remove the duplicatekeyword.operatorrule to keep the theme easier to maintain.
118-185: Opacity suffixes assume hex colors; verify TerminalColors can’t be rgba()/namedAll the
${terminal.*}22/15/33strings assumeterminal.*is a#RRGGBBhex value. If any theme ever usesrgba(...)or a named color, these concatenations will produce invalid CSS (e.g.rgba(... )33). Consider either normalizingTerminalColorsto hex-only, or adding a helper that only appends alpha whencolor.startsWith("#")and otherwise just returns the original color.apps/desktop/src/renderer/contexts/MonacoProvider.tsx (2)
8-16: Consider label-based workers and ensureself.MonacoEnvironmentis typed in TSRight now every worker request returns
editorWorker, which is fine for basic editing but may limit richer language services that expect label-specific workers (e.g.json,typescript). Also, in TypeScript,self.MonacoEnvironmentusually needs a cast or global declaration; otherwise you can get a type error onMonacoEnvironmentnot existing onself. If you don’t already have a global type for this, consider something like(self as any).MonacoEnvironment = { ... }and, if needed, expandinggetWorkerto switch onlabel.
52-83: Align theme registration state withgetMonacoThemeNameand drop unused ref
themeRegisteredRefis written (Line 63) but never read, whilegetMonacoThemeNameonly checksmonacoInitialized. That means callers can get"superset-theme"even if the theme hasn’t actually been defined yet, which can be fragile depending on how@monaco-editor/reactapplies the theme.You can track a module-level “theme registered” flag and base both
getMonacoThemeNameandisMonacoReadyon it, removing the unused ref:-// Track if Monaco has been initialized -let monacoInitialized = false; +// Track if Monaco has been initialized +let monacoInitialized = false; +// Track if our custom theme has been registered +let monacoThemeRegistered = false; @@ export function MonacoProvider({ children }: MonacoProviderProps) { const monacoTheme = useMonacoTheme(); - const themeRegisteredRef = useRef(false); @@ - monacoPromise.then(() => { - monaco.editor.defineTheme(SUPERSET_THEME, monacoTheme); - themeRegisteredRef.current = true; - }); + monacoPromise.then(() => { + monaco.editor.defineTheme(SUPERSET_THEME, monacoTheme); + monacoThemeRegistered = true; + }); }, [monacoTheme]); @@ export function getMonacoThemeName(): string { - return monacoInitialized ? SUPERSET_THEME : "vs-dark"; + return monacoThemeRegistered ? SUPERSET_THEME : "vs-dark"; } @@ export function isMonacoReady(): boolean { - return monacoInitialized; + return monacoInitialized && monacoThemeRegistered; }This keeps the public helpers in sync with the actual registration state and removes dead state.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsx (1)
31-39:handleChangedoesn’t match the “use default” commentThe comment says selecting the auto-detected default should “store null to indicate 'use default'”, but the current logic:
if (value === branchData.defaultBranch && baseBranch === null) { return; } setBaseBranch(value);never actually writes
null. Once a custom base branch is chosen, selecting the default just stores that branch name, and there’s no way back to “auto” semantics.If you want “default” to mean “follow whatever the backend reports”, consider:
const handleChange = (value: string) => { if (value === branchData.defaultBranch) { setBaseBranch(null); // revert to auto-default mode } else { setBaseBranch(value); } };This keeps the UI behavior the same but makes the stored state align with the intent in the comment.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsx (1)
64-114: Tree-building edge case and helper duplication (optional)Two minor observations:
buildFileTreewill silently drop entries if a repo contains both a filefooand a folderfoo/barbecause the first path creates afilenode with nochildren, and the second path can’t descend into it. If you care about that rare case, you’d need to allow promoting afilenode into afolder+filecomposite or otherwise handle the collision.getStatusColor/getStatusIndicatorare essentially duplicates of the helpers inSidebar/GitFileTree/utils.ts; centralizing them would keep status styling consistent.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (2)
41-53: Avoid duplicatingviewModein local state; use the persisted changes store
viewModeis already modeled and persisted inuseChangesStore, but this component introduces a separate localuseState, so the store’sviewModeis never updated and other consumers can’t rely on it. It also defeats the persisted preference behavior.You can simplify and align with the “prefer zustand for state” guideline by driving
viewModefrom the store:- const { - selectedFile, - selectedCommitHash, - expandedSections, - selectFile, - selectCommit, - toggleSection, - selectCategory, - } = useChangesStore(); - - // View mode state - const [viewMode, setViewMode] = useState<ChangesViewMode>("grouped"); + const { + selectedFile, + selectedCommitHash, + expandedSections, + viewMode, + setViewMode, + selectFile, + selectCommit, + toggleSection, + selectCategory, + } = useChangesStore();This keeps a single source of truth and preserves cross-session behavior.
Also applies to: 146-155
128-139: Section title “Against Main” may be misleading when base branch differsThe “Against Main” section is always labeled “Against Main” even though the effective base branch is derived dynamically (
status.defaultBranch/effectiveBaseBranch) and can be changed viaBaseBranchSelector. When the base branch ismasteror a feature branch, the label becomes inaccurate.Consider deriving the title from the actual base branch, e.g.:
- <CategorySection - title="Against Main" + <CategorySection + title={`Against ${status.defaultBranch ?? "main"}`}or a generic label like “Against base branch” to avoid confusion.
Also applies to: 165-167
apps/desktop/src/lib/trpc/routers/changes/changes.ts (3)
295-318: Rename commits will render as pure add/delete becauseoldPathisn’t used incommitteddiffsFor the
"committed"category,getFileContentsalways calls:original = await git.show([`${input.commitHash}^:${input.filePath}`]); modified = await git.show([`${input.commitHash}:${input.filePath}`]);If a commit renames a file, the parent commit typically only has
oldPathwhile the current commit only has the newfilePath. One of thesegit showcalls will fail and fall back to"", so the diff appears as a full add or delete instead of a proper rename+edit view.Consider threading
oldPaththrough the API and using it for the parent side:- category: z.enum(["against-main", "committed", "staged", "unstaged"]), + category: z.enum(["against-main", "committed", "staged", "unstaged"]), commitHash: z.string().optional(), defaultBranch: z.string().optional(), + oldPath: z.string().optional(), ... case "committed": { // Original: file at parent commit // Modified: file at specified commit if (!input.commitHash) { throw new Error("commitHash required for committed category"); } + const parentPath = input.oldPath ?? input.filePath; try { - original = await git.show([ - `${input.commitHash}^:${input.filePath}`, - ]); + original = await git.show([ + `${input.commitHash}^:${parentPath}`, + ]);and have the caller pass
oldPathfrom theChangedFilewhere available.
453-472: HardendeleteUntrackedagainst paths escaping the worktree
deleteUntrackedbuildsfullPath = join(input.worktreePath, input.filePath)and then callsrm(fullPath, { recursive: true }). While git generally doesn’t expose..-style paths, a defensive check thatfullPathstays within the resolvedworktreePathwould guard against any future parsing bugs or crafted input.Example hardening:
const root = resolve(input.worktreePath); const fullPath = resolve(root, input.filePath); if (!fullPath.startsWith(root + path.sep)) { throw new Error("Refusing to delete path outside worktree"); } await rm(fullPath, { recursive: true, force: false });This keeps deletion strictly scoped to the current worktree.
1-2: Prefer a single static import fromnode:fs/promisesover mixing static and dynamicYou already import
readFilefromnode:fs/promisesat the top, but later dynamically importrmfrom the same module indeleteUntracked. This adds unnecessary dynamic import overhead without bringing real benefits here.You can simplify to a single static import:
-import { readFile } from "node:fs/promises"; +import { readFile, rm } from "node:fs/promises"; ... - const { rm } = await import("node:fs/promises"); - const fullPath = join(input.worktreePath, input.filePath); + const fullPath = join(input.worktreePath, input.filePath);This keeps the module usage consistent and marginally reduces runtime complexity.
Also applies to: 461-465
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/desktop/src/lib/trpc/routers/changes/changes.ts(1 hunks)apps/desktop/src/main/lib/app-state/index.ts(1 hunks)apps/desktop/src/renderer/contexts/AppProviders.tsx(1 hunks)apps/desktop/src/renderer/contexts/MonacoProvider.tsx(1 hunks)apps/desktop/src/renderer/contexts/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ModeCarousel/ModeContent.tsx(1 hunks)apps/desktop/src/renderer/stores/changes/store.ts(1 hunks)apps/desktop/src/renderer/stores/theme/index.ts(1 hunks)apps/desktop/src/renderer/stores/theme/store.ts(8 hunks)apps/desktop/src/renderer/stores/theme/utils/index.ts(1 hunks)apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx
- apps/desktop/src/renderer/stores/changes/store.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ModeCarousel/ModeContent.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
apps/desktop/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Files:
apps/desktop/src/lib/trpc/routers/changes/changes.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - only in src/main/
Applied to files:
apps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/contexts/AppProviders.tsx
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/[A-Z]*.tsx : Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
apps/desktop/src/renderer/stores/theme/store.ts
🧬 Code graph analysis (8)
apps/desktop/src/renderer/contexts/AppProviders.tsx (2)
apps/desktop/src/renderer/contexts/TRPCProvider.tsx (1)
TRPCProvider(7-19)apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
MonacoProvider(52-68)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/BaseBranchSelector.tsx (2)
apps/desktop/src/renderer/stores/changes/store.ts (1)
useChangesStore(53-118)packages/ui/src/components/select.tsx (5)
Select(181-181)SelectTrigger(189-189)SelectValue(190-190)SelectContent(182-182)SelectItem(184-184)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsx (4)
apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-33)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts (1)
FileTreeNode(3-14)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (2)
getStatusColor(108-123)getStatusIndicator(128-143)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx (5)
apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-33)packages/ui/src/components/collapsible.tsx (3)
Collapsible(32-32)CollapsibleTrigger(32-32)CollapsibleContent(32-32)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (1)
FileItem(51-93)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.ts (1)
FileListGrouped(2-2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (6)
apps/desktop/src/renderer/stores/changes/store.ts (1)
useChangesStore(53-118)apps/desktop/src/shared/changes-types.ts (2)
ChangedFile(22-33)ChangeCategory(15-19)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
ChangesHeader(18-63)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx (1)
CategorySection(19-60)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
FileList(15-49)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (1)
CommitItem(36-99)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (4)
apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-33)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts (1)
ChangesViewMode(1-1)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsx (1)
FileListTree(210-233)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx (1)
FileListGrouped(109-132)
apps/desktop/src/renderer/stores/theme/store.ts (3)
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (2)
MonacoTheme(7-12)toMonacoTheme(191-198)apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts (1)
toXtermTheme(7-36)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (5)
apps/desktop/src/renderer/stores/changes/store.ts (1)
useChangesStore(53-118)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/EmptyState/EmptyState.tsx (1)
EmptyState(8-18)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (1)
FileHeader(44-87)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffToolbar/DiffToolbar.tsx (1)
DiffToolbar(22-96)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiffViewer/DiffViewer.tsx (1)
DiffViewer(10-46)
🔇 Additional comments (15)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts (1)
1-1: LGTM!The view mode union type is clean and appropriate for toggling between grouped and tree views.
apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
9-15: LGTM!The provider nesting is correctly structured with
TRPCProviderwrappingMonacoProvider. This ensures that any tRPC-dependent hooks used withinMonacoProvider(like theme hooks) have access to the query client context.apps/desktop/src/renderer/stores/theme/index.ts (1)
1-8: LGTM!The new
useMonacoThemehook is properly re-exported alongside other theme hooks, maintaining the existing barrel file pattern.apps/desktop/src/renderer/contexts/index.ts (1)
1-3: LGTM!The new exports cleanly extend the public API surface for Monaco integration.
apps/desktop/src/renderer/stores/theme/utils/index.ts (1)
6-8: LGTM!The Monaco theme exports follow the same pattern as the existing terminal theme exports, maintaining consistency in the utils module.
apps/desktop/src/renderer/stores/theme/store.ts (3)
32-34: LGTM!The
monacoThemestate follows the same pattern asterminalTheme- both are derived values that aren't persisted (correctly excluded frompartialize). This is consistent with the store's design.
79-97: Clean refactor ofapplyThemereturn type.Returning an object with named properties (
{ terminalTheme, monacoTheme }) instead of a single value is a good pattern - it makes the API more explicit and easier to extend in the future.
204-204: LGTM!The
useMonacoThemeconvenience hook follows the established pattern of other selector hooks in this store. As per coding guidelines, Zustand is appropriately used for state management here.apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
28-41: Nice use of eager Monaco initialization to hide load latencyPre-starting
initializeMonaco()at module load and centralizing theloader.init()call throughmonacoPromiseis a clean way to avoid editor mount jank and duplicate inits.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ViewModeToggle/ViewModeToggle.tsx (1)
1-30: View mode toggle wiring looks solidProps, ToggleGroup configuration, and value/change handling are consistent with the
ChangesViewModeunion; no issues spotted.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (1)
8-75: Diff data & mutations are wired coherentlyGood use of tRPC + zustand: selection state, effective base-branch derivation, and query/mutation invalidations (
getStatus/getFileContents) are consistent and should keep the UI in sync without extra effects.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/BaseBranchSelector/index.ts (1)
1-1: Barrel export matches component-folder conventionRe-exporting
BaseBranchSelectorfromindex.tskeeps imports clean and aligns with the component structure guideline.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
1-48: FileList delegation by viewMode is cleanThe wrapper neatly routes to
FileListTreevsFileListGrouped, keeps props consistent, and avoids rendering when there are no files; nothing concerning here.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (1)
22-34: Verify thatCommitInfo.dateis correctly typed asDate
formatRelativeDateexpects aDateparameter, but this depends on howCommitInfodefines itsdatefield. IfCommitInfouses SuperJSON serialization (standard for tRPC), the date should be typed asDateand will be properly restored on deserialization. IfCommitInfo.dateis currently a string, update the type toDateand ensure SuperJSON is configured as a tRPC transformer.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
7-61: Header composition and props wiring look goodBranch/ahead/behind display, base-branch selector, refresh button, and view-mode toggle are cleanly wired and typed; nothing blocking here.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (1)
26-43: AligngetStatusLabelparameter type withFileStatus
getStatusLabelstill takes astringeven though all call sites pass aFileStatus, and you already importFileStatus. Narrowing the parameter toFileStatuswill keep this helper in sync with the rest of the changes-types contract:-import type { ChangedFile, FileStatus } from "shared/changes-types"; +import type { ChangedFile, FileStatus } from "shared/changes-types"; -function getStatusLabel(status: string): string { +function getStatusLabel(status: FileStatus): string {The body can remain as is so you still have a sane fallback for any future statuses.
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (1)
96-138: parseGitLog now robustly handles pipe characters and malformed linesThe updated
parseGitLogimplementation:
- Processes empty/whitespace input safely.
- Splits on
"|"but reconstructs the message fromparts.slice(2, -2)so|in commit messages is preserved.- Trims all fields, skips malformed lines with fewer than 5 parts, and safely parses dates with a fallback to
new Date().This resolves the earlier concern about
|in commit messages breaking parsing while keeping behavior predictable for bad input.
🧹 Nitpick comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (1)
8-24: Good use ofFileStatusand complete status color mappingUsing the
FileStatustype here and handlingcopiedalongsiderenamedtightens type safety and keeps the visual mapping consistent with the domain model. Consider later centralizing this mapping with the existinggetStatusColorinSidebar/GitFileTree/utils.tsto avoid divergence, but it’s not a blocker.apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts (1)
72-83: Reduce timing sensitivity in the “invalid date fallback” testThe
"handles invalid date with fallback to current date"test (Lines 72–83) asserts that the parsed date is within 1 second ofnow. On slow or contended CI this threshold can occasionally be exceeded, making the test flaky.Consider either:
- Relaxing the threshold (e.g., 3–5 seconds), or
- Asserting properties that are independent of wall‑clock timing (e.g., that
dateis a validDateand not equal toInvalid Date), possibly by injecting a clock or abstracting date creation if you later refactorparseGitLog.apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (1)
145-176: Dual-keying numstat entries fixes rename/copy stats and keeps behavior sane for odd inputs
parseDiffNumstatnow:
- Handles empty/whitespace input gracefully.
- Correctly treats
"-"as zero for binary files.- For lines like
"old/path.ts => new/path.ts", adds entries for botholdPathandnewPath, which aligns with howparseNameStatussetspath/oldPathand fixes previous lookup issues in the router.You might optionally
trim()oldPath/newPathbefore using them as keys to guard against any leading/trailing whitespace changes in future git output formats, but the current behavior is already solid for typical git output.apps/desktop/src/lib/trpc/routers/changes/changes.ts (4)
92-210: Status aggregation and numstat enrichment look correct; consider broadening “against-main” conditionsThe
getStatusprocedure:
- Uses
parseGitStatusfor staged/unstaged/untracked, then enriches counts viaparseDiffNumstatfor:
againstMain(diff vsorigin/${defaultBranch}...HEAD),- staged (
diff --cached --numstat),- unstaged (
diff --numstat).- With the updated
parseDiffNumstat(dual‑keyingoldPath/newPath), additions/deletions will now populate correctly for renames and copies when you look up byfile.path, which resolves the earlier rename numstat issue.One small behavioral point:
againstMainis only populated whenahead > 0. If a branch is only behind the default branch (or otherwise diverged but not ahead), the"Against Main"section will remain empty even though there is a diff vsorigin/${defaultBranch}. If you want that view to always represent “diff vs default branch”, you may want to computeagainstMainwhenahead > 0 || behind > 0(or unconditionally inside thetry), and let the UI decide how to present it.
258-372: Rename diffs in getFileContents are treated as add/delete rather than true renames
getFileContentsworks well for regular add/modify/delete cases in all categories, but for renames it will generally show “added vs empty” (or “deleted vs empty”) because it only receivesfilePathand never the originaloldPath:
"against-main":originalisorigin/${defaultBranch}:${filePath}andmodifiedisHEAD:${filePath}. For a rename, the old tree path is different, sooriginalfalls back to"", effectively treating it as a new file."committed":originalis${commitHash}^:${filePath}andmodifiedis${commitHash}:${filePath}; again, the parent commit uses the old path."staged":originalisHEAD:${filePath},modifiedis:0:${filePath}, so staged renames are also displayed as add/delete.If you want rename diffs to show true before/after content, consider extending the API to pass
oldPathwhere available (e.g., fromChangedFile.oldPath) and using it as the source for the “original” side:
- Against main:
origin/${defaultBranch}:${oldPath ?? filePath}vsHEAD:${filePath}- Committed:
${commitHash}^:${oldPath ?? filePath}vs${commitHash}:${filePath}- Staged:
HEAD:${oldPath ?? filePath}vs:0:${filePath}This would improve UX for renames without changing semantics for non-rename cases.
377-448: Stage/unstage mutations are straightforward; consider optional error hardeningThe
stageFile,unstageFile,stageAll, andunstageAllmutations are thin wrappers oversimple-gitand return a simple{ success: true }on completion. That’s appropriate for the desktop router.If you start seeing user-facing issues around git failures (e.g., permissions, lock files), you might want to mirror the
discardChanges/deleteUntrackedpattern and catch/normalize errors into a friendly message, but it’s not strictly necessary right now.
453-472: Guard against path traversal in deleteUntracked for defense in depth
deleteUntrackedcomputesfullPath = join(input.worktreePath, input.filePath)and then callsrm(fullPath, { recursive: true, force: true }). In practice,filePathis expected to come from git status (so it should be repo‑relative and not contain..), but this mutation is still externally callable over tRPC.For defense in depth, consider:
- Normalizing and resolving the path (e.g.,
realpath), and- Verifying that
fullPathstays withinworktreePath(prefix check) before deleting.That would prevent a malformed or malicious
filePathlike"../../somewhere"from escaping the repo root and deleting arbitrary filesystem locations if this endpoint were ever called with untrusted input.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/desktop/package.json(2 hunks)apps/desktop/src/lib/trpc/routers/changes/changes.ts(1 hunks)apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts(1 hunks)apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx(1 hunks)packages/ui/src/components/collapsible.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/collapsible.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx
🧰 Additional context used
📓 Path-based instructions (7)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsxapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx
apps/desktop/src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Files:
apps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable
Files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Use React + TailwindCSS v4 + shadcn/ui for UI components
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should have one assert per test
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/*.test.{ts,tsx} : Co-locate tests with components using the naming pattern ComponentName.test.tsx
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be fast
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx (4)
apps/desktop/src/shared/changes-types.ts (2)
ChangedFile(22-33)FileStatus(6-12)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts (1)
getStatusColor(108-123)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/index.ts (1)
FileHeader(1-1)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.test.ts (1)
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (4)
parseGitLog(96-138)parseDiffNumstat(145-176)parseNameStatus(182-228)detectLanguage(233-293)
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (1)
apps/desktop/src/shared/changes-types.ts (4)
FileStatus(6-12)ChangedFile(22-33)GitChangesStatus(52-71)CommitInfo(36-49)
🔇 Additional comments (5)
apps/desktop/src/lib/trpc/routers/changes/utils/parse-status.ts (3)
12-89: parseGitStatus mapping and staging/unstaged classification look correctThe
mapGitStatusandparseGitStatuslogic correctly distinguish untracked (??), staged (non‑space/non‑?index), and unstaged (non‑space/non‑?working tree) changes, and preserveoldPathfor renames viafile.from. The returned shape matchesPick<GitChangesStatus, "branch" | "staged" | "unstaged" | "untracked">, and thestatus.current || "HEAD"fallback is reasonable for detached HEADs.
182-228: parseNameStatus correctly interprets standard and rename/copy statusesThe
parseNameStatusfunction correctly:
- Handles
A,D,MplusR###andC###with theoldpath/newpathsplit.- Defaults unknown status codes (e.g.,
U) to"modified", which is a safe and understandable fallback.- Populates
oldPathonly when applicable and initializes additions/deletions to zero for later enrichment.No issues spotted here.
233-293: Language detection mapping is pragmatic and suitable for MonacoThe
detectLanguagehelper provides a clear extension→language map with sensible defaults (including aplaintextfallback and case‑insensitive handling). This should work well for Monaco’s language selection in the diff viewer.apps/desktop/src/lib/trpc/routers/changes/changes.ts (2)
27-81: Branch enumeration and default-branch inference are reasonable
getBranchescorrectly:
- Separates local and
remotes/origin/*branches (excludingorigin/HEAD).- Derives
defaultBranchfromrefs/remotes/origin/HEADwhen available, with a sensible"main"/"master"fallback based on remote presence.- Returns sorted arrays, which is convenient for UI rendering.
This looks good for the desktop use case.
215-253: getCommitFiles correctly combines name-status and numstat for per-commit file stats
getCommitFilesusesgit diff-tree --name-status/-rplus--numstatand reconciles them viaparseNameStatusandparseDiffNumstat. With numstat now keying both old and new paths for renames/copies,stats.get(file.path)will provide accurate additions/deletions for those cases as well.This should give the UI accurate per-file metrics for commit details.
| describe("parseGitLog", () => { | ||
| test("parses basic log output", () => { | ||
| const logOutput = `abc123|abc|Initial commit|John Doe|2024-01-15T10:30:00Z | ||
| def456|def|Add feature|Jane Smith|2024-01-16T14:20:00Z`; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
|
|
||
| expect(commits).toHaveLength(2); | ||
| expect(commits[0]).toEqual({ | ||
| hash: "abc123", | ||
| shortHash: "abc", | ||
| message: "Initial commit", | ||
| author: "John Doe", | ||
| date: new Date("2024-01-15T10:30:00Z"), | ||
| files: [], | ||
| }); | ||
| expect(commits[1]).toEqual({ | ||
| hash: "def456", | ||
| shortHash: "def", | ||
| message: "Add feature", | ||
| author: "Jane Smith", | ||
| date: new Date("2024-01-16T14:20:00Z"), | ||
| files: [], | ||
| }); | ||
| }); | ||
|
|
||
| test("handles commit messages containing pipe characters", () => { | ||
| const logOutput = `abc123|abc|fix: handle edge case | add fallback|John Doe|2024-01-15T10:30:00Z`; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
|
|
||
| expect(commits).toHaveLength(1); | ||
| expect(commits[0].message).toBe("fix: handle edge case | add fallback"); | ||
| expect(commits[0].author).toBe("John Doe"); | ||
| }); | ||
|
|
||
| test("handles commit messages with multiple pipe characters", () => { | ||
| const logOutput = `abc123|abc|a | b | c | d|Author|2024-01-15T10:30:00Z`; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
|
|
||
| expect(commits).toHaveLength(1); | ||
| expect(commits[0].message).toBe("a | b | c | d"); | ||
| }); | ||
|
|
||
| test("returns empty array for empty input", () => { | ||
| expect(parseGitLog("")).toEqual([]); | ||
| expect(parseGitLog(" ")).toEqual([]); | ||
| expect(parseGitLog("\n\n")).toEqual([]); | ||
| }); | ||
|
|
||
| test("skips malformed lines with fewer than 5 parts", () => { | ||
| const logOutput = `abc123|abc|message|author|2024-01-15T10:30:00Z | ||
| invalid|line | ||
| def456|def|another|person|2024-01-16T14:20:00Z`; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
|
|
||
| expect(commits).toHaveLength(2); | ||
| expect(commits[0].hash).toBe("abc123"); | ||
| expect(commits[1].hash).toBe("def456"); | ||
| }); | ||
|
|
||
| test("handles invalid date with fallback to current date", () => { | ||
| const logOutput = `abc123|abc|message|author|not-a-date`; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
| const now = new Date(); | ||
|
|
||
| expect(commits).toHaveLength(1); | ||
| // Date should be close to now (within 1 second) | ||
| expect(Math.abs(commits[0].date.getTime() - now.getTime())).toBeLessThan( | ||
| 1000, | ||
| ); | ||
| }); | ||
|
|
||
| test("trims whitespace from all fields", () => { | ||
| const logOutput = ` abc123 | abc | message | author | 2024-01-15T10:30:00Z `; | ||
|
|
||
| const commits = parseGitLog(logOutput); | ||
|
|
||
| expect(commits).toHaveLength(1); | ||
| expect(commits[0].hash).toBe("abc123"); | ||
| expect(commits[0].shortHash).toBe("abc"); | ||
| expect(commits[0].message).toBe("message"); | ||
| expect(commits[0].author).toBe("author"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("parseDiffNumstat", () => { | ||
| test("parses basic numstat output", () => { | ||
| const numstatOutput = `10 5 src/file1.ts | ||
| 20 3 src/file2.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.get("src/file1.ts")).toEqual({ additions: 10, deletions: 5 }); | ||
| expect(stats.get("src/file2.ts")).toEqual({ additions: 20, deletions: 3 }); | ||
| }); | ||
|
|
||
| test("handles binary files with dash markers", () => { | ||
| const numstatOutput = `- - image.png | ||
| 10 5 src/code.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.get("image.png")).toEqual({ additions: 0, deletions: 0 }); | ||
| expect(stats.get("src/code.ts")).toEqual({ additions: 10, deletions: 5 }); | ||
| }); | ||
|
|
||
| test("handles renamed files with arrow format", () => { | ||
| const numstatOutput = `5 2 old/path.ts => new/path.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| // Should be accessible by both old and new paths | ||
| expect(stats.get("new/path.ts")).toEqual({ additions: 5, deletions: 2 }); | ||
| expect(stats.get("old/path.ts")).toEqual({ additions: 5, deletions: 2 }); | ||
| }); | ||
|
|
||
| test("handles copied files with arrow format", () => { | ||
| const numstatOutput = `0 0 source.ts => copy.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.get("copy.ts")).toEqual({ additions: 0, deletions: 0 }); | ||
| expect(stats.get("source.ts")).toEqual({ additions: 0, deletions: 0 }); | ||
| }); | ||
|
|
||
| test("handles paths with spaces in rename format", () => { | ||
| const numstatOutput = `3 1 old file.ts => new file.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.get("new file.ts")).toEqual({ additions: 3, deletions: 1 }); | ||
| expect(stats.get("old file.ts")).toEqual({ additions: 3, deletions: 1 }); | ||
| }); | ||
|
|
||
| test("returns empty map for empty input", () => { | ||
| expect(parseDiffNumstat("").size).toBe(0); | ||
| expect(parseDiffNumstat(" ").size).toBe(0); | ||
| expect(parseDiffNumstat("\n\n").size).toBe(0); | ||
| }); | ||
|
|
||
| test("skips lines without path", () => { | ||
| const numstatOutput = `10 5 | ||
| 20 3 valid/path.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.size).toBe(1); | ||
| expect(stats.get("valid/path.ts")).toEqual({ additions: 20, deletions: 3 }); | ||
| }); | ||
|
|
||
| test("handles non-numeric additions/deletions gracefully", () => { | ||
| const numstatOutput = `abc xyz file.ts`; | ||
|
|
||
| const stats = parseDiffNumstat(numstatOutput); | ||
|
|
||
| expect(stats.get("file.ts")).toEqual({ additions: 0, deletions: 0 }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("parseNameStatus", () => { | ||
| test("parses added files", () => { | ||
| const nameStatus = `A src/new-file.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0]).toEqual({ | ||
| path: "src/new-file.ts", | ||
| oldPath: undefined, | ||
| status: "added", | ||
| additions: 0, | ||
| deletions: 0, | ||
| }); | ||
| }); | ||
|
|
||
| test("parses deleted files", () => { | ||
| const nameStatus = `D src/removed.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0].status).toBe("deleted"); | ||
| }); | ||
|
|
||
| test("parses modified files", () => { | ||
| const nameStatus = `M src/changed.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0].status).toBe("modified"); | ||
| }); | ||
|
|
||
| test("parses renamed files with percentage", () => { | ||
| const nameStatus = `R100 old/name.ts new/name.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0]).toEqual({ | ||
| path: "new/name.ts", | ||
| oldPath: "old/name.ts", | ||
| status: "renamed", | ||
| additions: 0, | ||
| deletions: 0, | ||
| }); | ||
| }); | ||
|
|
||
| test("parses copied files with percentage", () => { | ||
| const nameStatus = `C095 source.ts destination.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0]).toEqual({ | ||
| path: "destination.ts", | ||
| oldPath: "source.ts", | ||
| status: "copied", | ||
| additions: 0, | ||
| deletions: 0, | ||
| }); | ||
| }); | ||
|
|
||
| test("parses multiple files", () => { | ||
| const nameStatus = `A added.ts | ||
| M modified.ts | ||
| D deleted.ts | ||
| R100 old.ts new.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(4); | ||
| expect(files[0].status).toBe("added"); | ||
| expect(files[1].status).toBe("modified"); | ||
| expect(files[2].status).toBe("deleted"); | ||
| expect(files[3].status).toBe("renamed"); | ||
| }); | ||
|
|
||
| test("returns empty array for empty input", () => { | ||
| expect(parseNameStatus("")).toEqual([]); | ||
| expect(parseNameStatus(" ")).toEqual([]); | ||
| }); | ||
|
|
||
| test("handles unknown status codes as modified", () => { | ||
| const nameStatus = `U unmerged.ts`; | ||
|
|
||
| const files = parseNameStatus(nameStatus); | ||
|
|
||
| expect(files).toHaveLength(1); | ||
| expect(files[0].status).toBe("modified"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Tests use multiple assertions per test, conflicting with desktop test guidelines
The tests are thorough and nicely cover edge cases for parseGitLog, parseDiffNumstat, and parseNameStatus, but many of them contain multiple expect(...) calls in a single test block (for example, Lines 16–32, 40–43, 67–70, 90–95, 105–107, 115–117, 124–127, 138–145, 178–185, 244–248, etc.).
Per the desktop AGENTS guidelines for apps/desktop/**/*.test.{ts,tsx,js,jsx}, tests should have one assert per test. Consider splitting these into smaller, focused tests (or parameterized/table-driven tests) so each test contains a single assertion while preserving coverage.
Based on learnings, tests should be readable, fast, independent, repeatable, and have one assert per test.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (1)
19-29: Consider consolidating color conversion utilities.The toMonacoHex function duplicates parsing logic from shared/themes/utils.ts toHex. While the alpha handling differs (formatHex8 vs formatHex), extracting common parsing into a shared utility would reduce duplication.
Example approach:
// In shared/themes/utils.ts export function parseColor(color: string) { if (color.startsWith("#")) { return { parsed: parse(color), wasHex: true }; } return { parsed: parse(color), wasHex: false }; } // Then in monaco-theme.ts function toMonacoHex(color: string): string { const { parsed, wasHex } = parseColor(color); if (!parsed) return color; if (parsed.alpha !== undefined && parsed.alpha < 1) { return formatHex8(parsed); } return formatHex(parsed); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/desktop/package.json(4 hunks)apps/desktop/src/renderer/contexts/MonacoProvider.tsx(1 hunks)apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts(1 hunks)apps/desktop/src/shared/themes/built-in/dark.ts(2 hunks)apps/desktop/src/shared/themes/built-in/light.ts(2 hunks)apps/desktop/src/shared/themes/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/shared/themes/built-in/light.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/shared/themes/built-in/light.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/shared/themes/built-in/light.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/renderer/contexts/MonacoProvider.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Use React + TailwindCSS v4 + shadcn/ui for UI components
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/contexts/MonacoProvider.tsx
🧬 Code graph analysis (3)
apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
apps/desktop/src/renderer/contexts/index.ts (2)
SUPERSET_THEME(2-2)MonacoProvider(2-2)
apps/desktop/src/shared/themes/built-in/dark.ts (1)
apps/desktop/src/shared/themes/utils.ts (1)
toHex(6-19)
apps/desktop/src/shared/themes/built-in/light.ts (1)
apps/desktop/src/shared/themes/utils.ts (1)
toHex(6-19)
🔇 Additional comments (6)
apps/desktop/src/shared/themes/built-in/light.ts (1)
2-48: LGTM! Consistent color transformation applied.The toHex wrapper is correctly and consistently applied to all oklch color values. The pattern matches the parallel changes in dark.ts, maintaining theme parity.
apps/desktop/src/shared/themes/built-in/dark.ts (1)
2-48: LGTM! Color transformation consistent with light theme.The toHex transformation is correctly applied to all UI colors while appropriately leaving terminal color hex values unchanged.
apps/desktop/src/shared/themes/utils.ts (1)
6-19: Well-implemented color conversion utility.The toHex function correctly handles edge cases with early returns for hex colors and graceful fallback on parse failures. The console warning aids debugging.
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (1)
47-228: Comprehensive Monaco theme mapping implemented correctly.The createTokenRules and createEditorColors functions provide thorough coverage of Monaco's theming surface, properly mapping terminal colors to syntax tokens and UI elements. The alpha channel handling for selections and highlights is well-structured.
apps/desktop/src/renderer/contexts/MonacoProvider.tsx (2)
3-29: Monaco environment configuration looks correct.The worker imports using the ?worker suffix are appropriate for Vite/Electron, and the MonacoEnvironment.getWorker mapping comprehensively covers all Monaco language workers.
38-38: Verify module-level state pattern for Monaco initialization tracking.The monacoInitialized flag is module-level state (line 38) rather than React state, used by getMonacoThemeName and isMonacoReady. This pattern works but means these functions return stale values until the module-level promise resolves, independent of React's lifecycle.
Consider whether this module-level state could cause issues if:
- Multiple instances of components call these functions before initialization completes
- The app needs to react to initialization status changes (currently no way to subscribe)
If reactive updates are needed, consider using a Zustand store or React state to track initialization status.
Also applies to: 90-99
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/desktop/src/shared/themes/utils.test.ts (3)
4-26: Good coverage fortoHex, but multiple expectations per test conflict with test style guidelineThese tests nicely exercise hex/rgb/oklch/hsl paths and invalid-color passthrough, and they’re pure/fast/independent. However, several
itblocks (e.g., Line 5 and Line 10) contain multipleexpectcalls, which goes against the repo guideline of one assert per test.Consider splitting these into smaller tests (e.g.,
it("converts lower-case hex"),it("converts upper-case hex"), etc.) so eachithas a single expectation while preserving coverage. Based on learnings, tests should stay small and focused.
28-52:toHex8/toHexAutotests are clear; consider splitting multi-expect cases and adding an invalid‑input checkThe scenarios for alpha handling (hex → hex8, rgba preserving alpha, oklch with alpha, and auto 6‑ vs 8‑digit selection) are well covered and easy to read, and everything is deterministic and fast.
To align with the “one assert per test” guideline, you could split Line 29 and Line 44 into separate
itblocks per expectation. Optionally, you might also add a single test each for invalid input passthrough fortoHex8andtoHexAuto, mirroring what you did fortoHex, to lock in that behavior.
54-79:withAlpha/stripHashtests look good; only minor style nit around multiple expectsThese tests cover the important paths: different alpha values on hex and rgb, invalid‐color passthrough for
withAlpha, and stripHash behavior for 6‑ and 8‑digit hex with and without#. They’re pure and repeatable, which matches the testing guidelines from AGENTS. Based on learnings, this is in a good place for speed and independence.The only minor nit is again test style: Lines 55 and 71 have multiple
expectcalls perit. If you want to strictly follow “one assert per test”, you could split those into separateits while keeping the same assertions.apps/desktop/src/shared/themes/utils.ts (1)
44-58: Minor nits: alpha range handling andstripHashsemanticsTwo small, non‑blocking observations:
withAlpha(Lines 44–51) doesn’t clamp or validate thealphaparameter. If callers might accidentally pass values outside[0, 1], you could defensively clamp to that range before assigning, or at least document the expected range in the JSDoc.stripHash(Lines 56–57) useshex.replace("#", ""), which removes the first#anywhere in the string. Since the comment says “Strip # prefix”, usinghex.replace(/^#/, "")or checkingstartsWith("#")would more clearly match the stated intent, even though for normal hex inputs this is effectively equivalent.These are polish-level tweaks; the current behavior is acceptable for typical usage.
If you rely heavily on alpha semantics or nonstandard hex strings, consider verifying expected behavior against design docs or UX requirements before changing this.
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (2)
16-80: Token rule mapping looks good; consider removing duplicatekeyword.operatorentryThe token → color mapping is coherent and lines up well with the terminal palette. The only nit I see is that
keyword.operatoris defined twice (Line 29 and Line 59) with the samecolors.redforeground, which is behaviorally harmless but slightly noisy for future readers. You could safely drop one of them to avoid confusion.
135-141: Base mapping covers light/dark; high‑contrast could later map tohc-black
toMonacoThemecorrectly switches between"vs"and"vs-dark"based ontheme.type. SinceMonacoTheme.basealready allows"hc-black", if yourThemetype ever grows a high‑contrast variant it should be straightforward to extend this function to map that through. Not urgent now, but worth keeping in mind so base stays in sync withTheme’s modes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts(1 hunks)apps/desktop/src/shared/themes/utils.test.ts(1 hunks)apps/desktop/src/shared/themes/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/shared/themes/utils.test.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/shared/themes/utils.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/shared/themes/utils.test.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable
Files:
apps/desktop/src/shared/themes/utils.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/shared/themes/utils.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent
Applied to files:
apps/desktop/src/shared/themes/utils.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable
Applied to files:
apps/desktop/src/shared/themes/utils.test.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be fast
Applied to files:
apps/desktop/src/shared/themes/utils.test.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/*.test.{ts,tsx} : Co-locate tests with components using the naming pattern ComponentName.test.tsx
Applied to files:
apps/desktop/src/shared/themes/utils.test.ts
🧬 Code graph analysis (1)
apps/desktop/src/shared/themes/utils.test.ts (1)
apps/desktop/src/shared/themes/utils.ts (5)
toHex(7-13)toHex8(19-25)toHexAuto(30-39)withAlpha(44-51)stripHash(56-58)
🔇 Additional comments (2)
apps/desktop/src/shared/themes/utils.ts (1)
1-39: Culori-based conversion helpers are small, consistent, and defensively implemented
toHex,toHex8, andtoHexAutoall follow the same pattern: parse once, bail out to the original string on failure, and use culori’s formatters for success. ThetoHexAutoalpha check is straightforward and matches the documented behavior (only emitting hex8 when alpha < 1). Type signatures are precise and avoidany.No changes needed here from a correctness or type-safety perspective.
If you want to double‑check culori API assumptions across versions, you can consult its docs for
parse,formatHex, andformatHex8behavior.apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (1)
83-133: Color format guarantees are solid; code is correctThe
toHexAutoandwithAlphautilities fromshared/themes/utilsconsistently emit the correct formats:toHexAutoreturns#RRGGBB(opaque) or#RRGGBBAA(transparent), andwithAlphaalways returns#RRGGBBAA. TheIColorsobject correctly uses these functions with the#prefix intact, which is exactly what Monaco expects for theme color entries. Token rules properly strip the hash viastripHash(toHexAuto()), matching Monaco's token rule format requirements. The mapping is well-designed and color derivation is sound.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/desktop/src/renderer/contexts/MonacoProvider.tsx (2)
36-46: Add error handling for Monaco initialization.The
initializeMonacofunction andmonacoPromiselack error handling. Ifloader.init()fails, the promise rejection will be unhandled, causing silent failures downstream.-const monacoPromise = initializeMonaco(); +const monacoPromise = initializeMonaco().catch((error) => { + console.error("Failed to initialize Monaco:", error); + throw error; +});
55-61: Add error handling in theme registration effect.The
.then()chain lacks a.catch()handler. If Monaco initialization failed, theme registration will silently fail.monacoPromise.then((monacoInstance) => { monacoInstance.editor.defineTheme(SUPERSET_THEME, monacoTheme); - }); + }).catch((error) => { + console.error("Failed to register Monaco theme:", error); + });
🧹 Nitpick comments (6)
apps/desktop/src/shared/themes/utils.ts (2)
42-49: Clamp or validatealphainwithAlphato avoid subtle bugs.Right now
withAlphaaccepts anynumberforalpha, including values outside the [0, 1] range, which can lead to surprising results at call sites (e.g., passing 50 or 255 out of habit). Consider clamping or validating the value:export function withAlpha(color: string, alpha: number): string { const parsed = parse(color); if (!parsed) { return color; } - parsed.alpha = alpha; + const clampedAlpha = Math.max(0, Math.min(1, alpha)); + parsed.alpha = clampedAlpha; return formatHex8(parsed); }
54-55: TightenstripHashto only remove a leading#.
hex.replace("#", "")removes the first#anywhere in the string. For a utility documented as stripping a prefix, it’s safer and clearer to only remove a leading hash:-export function stripHash(hex: string): string { - return hex.replace("#", ""); -} +export function stripHash(hex: string): string { + return hex.startsWith("#") ? hex.slice(1) : hex; +}apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
34-44: Potential race condition with concurrent calls.The
monacoInitializedflag isn't protected against concurrent calls. IfinitializeMonacois called multiple times before the firstloader.init()completes, it may initialize Monaco multiple times.Consider guarding with the promise itself:
let monacoInitialized = false; +let initPromise: Promise<typeof monaco> | null = null; async function initializeMonaco(): Promise<typeof monaco> { if (monacoInitialized) { return monaco; } + if (initPromise) { + return initPromise; + } - await loader.init(); - monacoInitialized = true; - return monaco; + initPromise = loader.init().then(() => { + monacoInitialized = true; + return monaco; + }); + return initPromise; }apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (1)
29-36: Consider addingstaleTimeto reduce unnecessary refetches.The query refetches every 2.5s and on window focus, which is good for real-time updates. However, without
staleTime, focus events may trigger redundant fetches immediately after a poll.{ enabled: !!worktreePath, refetchInterval: 2500, refetchOnWindowFocus: true, + staleTime: 2000, },apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.ts (1)
14-40: Consider also invalidatinggetFileContentson stage/unstage to keep the open diff freshRight now only
discardChangesanddeleteUntrackedinvalidate both status and file contents, whilestageFile/unstageFileonly invalidategetStatus. If a file is currently open in the diff viewer, staging or unstaging it can change what that diff should show (or which category it belongs to), but thegetFileContentscache may remain stale until some other event forces a refetch.You may want to mirror the discard/delete behavior here:
const stageFile = trpc.changes.stageFile.useMutation({ - onSuccess: () => utils.changes.getStatus.invalidate(), + onSuccess: () => { + utils.changes.getStatus.invalidate(); + utils.changes.getFileContents.invalidate(); + }, }); const unstageFile = trpc.changes.unstageFile.useMutation({ - onSuccess: () => utils.changes.getStatus.invalidate(), + onSuccess: () => { + utils.changes.getStatus.invalidate(); + utils.changes.getFileContents.invalidate(); + }, });apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (1)
26-48: Optional: avoid fetching file contents before default branch is knownBecause
effectiveBaseBranchfalls back to"main"and is used as part of thegetFileContentsquery key, you’ll issue one request against"main"and then another whenbranchData?.defaultBrancharrives (for repos whose default isn’t"main"). That’s harmless but slightly wasteful and can momentarily show a diff against the wrong base.If you care about that extra roundtrip, you could gate
getFileContentson the base branch being resolved whenbaseBranchisn’t already user-chosen, e.g.:const hasBranch = !!baseBranch || !!branchData?.defaultBranch; const { data: contents, // ... } = trpc.changes.getFileContents.useQuery( { /* ... */ defaultBranch: effectiveBaseBranch }, { enabled: !!worktreePath && !!selectedFile && hasBranch, }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.cursor/commands/deslop.md(1 hunks)apps/desktop/package.json(4 hunks)apps/desktop/src/renderer/contexts/MonacoProvider.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts(1 hunks)apps/desktop/src/renderer/stores/changes/store.ts(1 hunks)apps/desktop/src/renderer/stores/theme/store.ts(8 hunks)apps/desktop/src/shared/changes-types.ts(1 hunks)apps/desktop/src/shared/themes/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop/package.json
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/FileHeader/FileHeader.tsx
- apps/desktop/src/renderer/stores/changes/store.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/utils.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/GitFileTree.tsx
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/shared/changes-types.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/shared/changes-types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain type safety and avoid using
anyunless absolutely necessary in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/shared/themes/utils.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/shared/changes-types.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - only in src/main/
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/contexts/MonacoProvider.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/stores/theme/store.ts
**/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/main/lib/*{workspace,worktree}-*.ts : Use git worktree-based workspace management in the desktop app through workspace-manager.ts and worktree-manager.ts
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsxapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/shared/changes-types.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.tsapps/desktop/src/shared/themes/utils.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : Use kebab-case single files for shadcn/ui components in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Keep diffs minimal and use targeted edits only
Applied to files:
.cursor/commands/deslop.md
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to **/components/**/[A-Z]*.tsx : Use one folder per component with structure ComponentName/ComponentName.tsx + index.ts barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/lib/**/*.{ts,tsx} : Never import Node.js modules like node:fs, node:path, node:os in src/lib/electron-router-dom.ts or similar shared code
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts
📚 Learning: 2025-12-08T23:32:32.232Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:32:32.232Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - only in src/main/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts
📚 Learning: 2025-11-24T21:33:13.267Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.267Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
apps/desktop/src/renderer/stores/theme/store.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (8)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (1)
ChangesView(1-1)apps/desktop/src/renderer/stores/changes/store.ts (1)
useChangesStore(42-106)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/types.ts (1)
ChangesViewMode(1-1)apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-28)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
ChangesHeader(18-63)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CategorySection/CategorySection.tsx (1)
CategorySection(19-60)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
FileList(15-49)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (1)
CommitItem(36-99)
apps/desktop/src/renderer/contexts/MonacoProvider.tsx (2)
apps/desktop/src/renderer/contexts/index.ts (2)
SUPERSET_THEME(2-2)MonacoProvider(2-2)apps/desktop/src/renderer/stores/theme/store.ts (1)
useMonacoTheme(198-198)
apps/desktop/src/renderer/stores/theme/store.ts (4)
apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts (2)
MonacoTheme(5-10)toMonacoTheme(135-142)apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts (1)
toXtermTheme(7-36)apps/desktop/src/renderer/stores/theme/index.ts (1)
useMonacoTheme(2-2)
🪛 LanguageTool
.cursor/commands/deslop.md
[grammar] ~3-~3: Use a hyphen to join words.
Context: ...the diff against main, and remove all AI generated slop introduced in this branch...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (21)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/GitFileTree/index.ts (1)
1-9: Barrel exports are clean and follow component-folder conventionsRe-exporting the component, types, and utils from a local
index.tskeeps imports tidy and matches the one-folder-per-component pattern; no renderer/main or Node-module issues here.
Based on learnings, this aligns with the prescribed component structure.apps/desktop/src/shared/themes/utils.ts (1)
1-37: Color conversion helpers are clear, type‑safe, and robust.The culori‑based wrappers are straightforward, avoid
any, and handle parse failures by returning the original string, which keeps callers simple and matches the doc comments. As per coding guidelines, type safety is maintained without resorting toany.apps/desktop/src/renderer/stores/theme/store.ts (5)
11-17: LGTM!Clean import organization bringing in the new Monaco theme utilities alongside existing helpers. The type import for
MonacoThemekeeps the bundle efficient.
32-34: LGTM!The
monacoThemestate field follows the established pattern forterminalTheme, maintaining consistency in how derived theme formats are stored.Also applies to: 106-106
79-96: LGTM!The
applyThemefunction cleanly returns both theme formats in a typed object. This maintains a single source of truth for theme application and keeps the conversion logic centralized.
183-186: Verify synchronous initialization is safe.The
setTimeoutwrapper was removed fromonRehydrateStorage. While this simplifies the flow, ensure thatinitializeThemedoesn't depend on any state that requires a tick to settle (e.g., DOM or async storage operations).
198-198: LGTM!The
useMonacoThemeconvenience hook follows the established pattern foruseTerminalTheme, providing a clean selector-based API for consumers.apps/desktop/src/renderer/contexts/MonacoProvider.tsx (1)
12-28: LGTM!Worker configuration correctly maps language labels to their respective worker implementations. The fallback to
editorWorkerfor unrecognized labels is appropriate.apps/desktop/src/shared/changes-types.ts (4)
1-28: LGTM!Well-structured type definitions with clear JSDoc comments. The
FileStatustype covers the essential git status codes, andChangedFileincludes all necessary metadata for diff viewing.
40-51: LGTM!The
GitChangesStatusinterface provides a comprehensive view of repository state with clear separation between against-main diffs, commits, staged, and unstaged changes.
53-69: LGTM!The
DiffViewMode,FileDiffInput, andFileContentstypes are well-designed for the Monaco diff editor integration. The optionalcommitHashinFileDiffInputcleanly handles both committed and uncommitted diff scenarios.
30-38: VerifyDateserialization over tRPC.The
datefield is typed asDate, but JSON serialization (used by tRPC) convertsDateobjects to ISO strings. Ensure the tRPC router either:
- Uses superjson transformer (configured on both server and client via
initTRPC.create({ transformer: superjson })and client instantiation), or- Returns the date as a string (ISO 8601 format recommended) and parses it client-side.
This requires verification that either approach is properly implemented in the codebase.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (5)
1-10: LGTM!Clean imports using path aliases as per coding guidelines. The separation between UI components, store hooks, and shared types is well-organized.
54-69: LGTM!Good use of
trpc.useQueriesfor dynamically fetching commit files based on expanded state. ThecommitFilesMapconstruction efficiently maps query results to commit hashes.
95-117: LGTM!Clear handling of loading, error, and empty workspace states with user-friendly messages.
133-223: LGTM!The overall component structure is clean with proper separation of header, categories, and file lists. The
ScrollAreawrapper ensures the content is scrollable when exceeding viewport.
152-166: Consider extracting section keys to constants.The string
"against-main"is used directly but the store'sexpandedSectionsmay expect different keys. Ensure consistency with theChangeCategorytype.#!/bin/bash # Check expandedSections type definition and usage ast-grep --pattern 'expandedSections: { $$$ }'apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/index.ts (1)
1-1: LGTM!Clean barrel export following the established pattern for component/hook folder organization.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/hooks/useFileActions/useFileActions.ts (1)
36-68: Hook shape and safety checks look solidThe combined
isPendingflag and the early returns whenworktreePathorfilePathare missing make the actions safe to call from the UI without extra guards. The trpc usage and parameter typing are also in line with the desktop app’s patterns.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/ChangesContent.tsx (1)
50-133: Discard confirmation flow is correctly wired and fixes the destructive-action riskThe new
showDiscardConfirmstate combined withDiscardConfirmDialogcleanly ensures that both tracked discards and untracked deletes go through an explicit confirmation step. The guards onworktreePath/selectedFileand theisUntrackedsplit betweendeleteFilevsdiscardlook correct, and the toolbar only exposesonDiscardfor the unstaged category, which is appropriate.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ChangesContent/components/DiscardConfirmDialog/DiscardConfirmDialog.tsx (1)
20-31: ImportMouseEventtype or ensure React is available forReact.MouseEventtypingThe
handleConfirmhandler is typed as(e: React.MouseEvent), but this requires either a React import or theMouseEventtype to be available. If React is not imported, this will cause a TypeScript compilation error. Ensure either:
Reactis imported (e.g.,import React from "react"if needed)- Or directly import
MouseEventfrom react:import type { MouseEvent } from "react"Then update the handler to use the appropriate type:
-const handleConfirm = (e: React.MouseEvent) => { +const handleConfirm = (e: MouseEvent) => {This maintains type safety while avoiding unnecessary runtime imports under the modern JSX transform.
1 participant
Summary
Changes
Backend (tRPC Router)
changesrouter with procedures for git operationsgetStatus- Full git status with staging info, commits, and against-main diffgetFileContents- File contents for Monaco diff editorstageFile,unstageFile,discardChanges- Git staging actionsFrontend Components
ChangesViewwith collapsible sections (CategorySection), file items (FileItem), and commit items (CommitItem)ChangesContentwith MonacoDiffViewer,DiffToolbar(view mode toggle + actions),FileHeader(file info + stats)Configuration
monaco-editorand@monaco-editor/reactdependenciesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.