feat(desktop): add path and branch info to workspace header#400
feat(desktop): add path and branch info to workspace header#400
Conversation
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 11 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 (15)
WalkthroughAdds branch-management backend changes (last-commit dates, checked-out worktrees, new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as WorkspaceHeader (UI)
participant TRPC as TRPC Client
participant Backend as changes.ts (Server)
participant Git as Git
participant DB as Database
User->>UI: open BranchSelector / choose branch
UI->>TRPC: switchBranch({ worktreePath, branch })
TRPC->>Backend: invoke switchBranch mutation
Backend->>Git: git.checkout(branch) on worktreePath
alt checkout succeeds
Git-->>Backend: success
Backend->>DB: persist worktree.branch and worktree.gitStatus.branch (if present)
DB-->>Backend: persisted
Backend-->>TRPC: { success: true }
TRPC-->>UI: success response
UI->>UI: invalidate caches / refresh branch list
else checkout fails
Git-->>Backend: error
Backend-->>TRPC: throw descriptive error
TRPC-->>UI: error response
UI->>UI: show error toast
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes
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 Deployment🔗 Preview Links
Preview updates automatically with new commits |
Kitenite
force-pushed
the
more-top-bar-space
branch
from
e725351 to
ca85ce9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
1-14: Consider handling future timestamps gracefully.If
timestampis in the future (greater thanDate.now()),diffMsbecomes negative, resulting in outputs like"-5m". Depending on usage, you may want to guard against this:export function formatRelativeTime(timestamp: number): string { const now = Date.now(); const diffMs = now - timestamp; + if (diffMs < 0) return "now"; // Future timestamp fallback const diffMinutes = Math.floor(diffMs / 60000);This is a minor edge case since
lastCommitDateshould always be in the past, but it adds defensive handling.apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts (2)
19-25: Consider splitting to one assertion per test.Per coding guidelines, tests should have one assert per test. Each test case currently contains multiple assertions, which can make it harder to identify which specific case failed.
Consider restructuring using
it.eachor separate test cases:describe("returns 'now' for timestamps less than 1 minute ago", () => { it("returns 'now' for current time", () => { mockNow(); expect(formatRelativeTime(NOW)).toBe("now"); restoreNow(); }); it("returns 'now' for 30 seconds ago", () => { mockNow(); expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now"); restoreNow(); }); // ... });Or use
it.eachfor parameterized tests.Based on coding guidelines: "Tests should have one assert per test".
10-17: Consider usingbeforeEach/afterEachfor mock lifecycle.The manual
mockNow()/restoreNow()calls in each test are repetitive and could miss restoration if a test throws. Using test hooks improves reliability:+ beforeEach(() => { + Date.now = () => NOW; + }); + + afterEach(() => { + Date.now = originalDateNow; + }); - const mockNow = () => { - Date.now = () => NOW; - }; - const restoreNow = () => { - Date.now = originalDateNow; - };Then remove
mockNow()andrestoreNow()calls from individual tests.apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (2)
48-63:checkedOutBranchesSet causes unnecessary recalculations.
checkedOutBranchesis a newSetinstance on every render, making theuseMemodependency referentially unstable. The memoization won't prevent recalculations.Move the Set creation inside the
useMemoor memoize it separately:- const localBranches = branchData?.local ?? []; - const checkedOutBranches = new Set(branchData?.checkedOutBranches ?? []); - - // Sort: current branch first, then other checked out branches, then the rest (already sorted by commit date) - const sortedBranches = useMemo( - () => [ + const localBranches = branchData?.local ?? []; + const checkedOutBranchesArray = branchData?.checkedOutBranches ?? []; + + const { sortedBranches, checkedOutBranches } = useMemo(() => { + const checkedOutSet = new Set(checkedOutBranchesArray); + return { + checkedOutBranches: checkedOutSet, + sortedBranches: [ - ...localBranches.filter((b) => b.branch === currentBranch), - ...localBranches.filter( - (b) => checkedOutBranches.has(b.branch) && b.branch !== currentBranch, - ), - ...localBranches.filter( - (b) => !checkedOutBranches.has(b.branch) && b.branch !== currentBranch, - ), - ], - [localBranches, checkedOutBranches, currentBranch], - ); + ...localBranches.filter((b) => b.branch === currentBranch), + ...localBranches.filter( + (b) => checkedOutSet.has(b.branch) && b.branch !== currentBranch, + ), + ...localBranches.filter( + (b) => !checkedOutSet.has(b.branch) && b.branch !== currentBranch, + ), + ], + }; + }, [localBranches, checkedOutBranchesArray, currentBranch]);
79-91: Consider adding loading feedback during branch switch.The
handleBranchChangefunction closes the popover immediately but provides no visual feedback while the mutation is in flight. Users may be confused if the switch takes time or if there are uncommitted changes.Consider showing a loading state or keeping the popover open until completion:
const handleBranchChange = async (branch: string) => { if (!worktreePath || branch === currentBranch) return; - setOpen(false); try { await switchBranch.mutateAsync({ worktreePath, branch }); + setOpen(false); utils.workspaces.getActive.invalidate(); utils.changes.getBranches.invalidate(); } catch (error) { + setOpen(false); toast.error( error instanceof Error ? error.message : "Failed to switch branch", ); } };Alternatively, use
switchBranch.isPendingto show a loading indicator on the button.apps/desktop/src/lib/trpc/routers/changes/changes.ts (1)
50-74: Consider explicit validation for the parsing logic.The git for-each-ref approach is efficient and well-structured. The try-catch with fallback to zero dates provides good error handling.
For added robustness, consider explicitly checking if
lastSpaceIdxis -1 before parsing, though the current try-catch and includes-check already provide safety:const lastSpaceIdx = line.lastIndexOf(" "); +if (lastSpaceIdx === -1) continue; const branch = line.substring(0, lastSpaceIdx);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/changes/changes.ts(5 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts(1 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts(1 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx(0 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/TopBar/index.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/lib/formatRelativeTime/index.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.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/lib/formatRelativeTime/index.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/renderer/lib/formatRelativeTime/index.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/renderer/lib/formatRelativeTime/index.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/lib/trpc/routers/changes/changes.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/lib/formatRelativeTime/index.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
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/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
🧠 Learnings (8)
📚 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/lib/formatRelativeTime/index.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx,js,jsx} apps/desktop/src/lib/**/*.{ts,tsx,js,jsx} apps/desktop/src/shared/**/*.{ts,tsx,js,tsx} : Never import Node.js modules in renderer process or shared code, only in main process
Applied to files:
apps/desktop/src/renderer/lib/formatRelativeTime/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/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.test.{ts,tsx} : Place tests co-located with component files using .test.tsx suffix
Applied to files:
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations in the `packages/db` package
Applied to files:
apps/desktop/src/lib/trpc/routers/changes/changes.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
apps/desktop/src/renderer/lib/formatRelativeTime/index.ts (1)
formatRelativeTime(1-1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (1)
WorkspaceHeader(25-211)packages/ui/src/components/ui/resizable.tsx (3)
ResizablePanelGroup(56-56)ResizablePanel(56-56)ResizableHandle(56-56)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (1)
Sidebar(7-29)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsx (1)
ContentView(5-13)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts (1)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
formatRelativeTime(1-14)
⏰ 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). (6)
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Docs
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/src/renderer/lib/formatRelativeTime/index.ts (1)
1-1: LGTM!Clean barrel export following project conventions.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (2)
15-15: LGTM!Clean import following project alias conventions.
109-139: LGTM!The layout restructure cleanly integrates
WorkspaceHeaderabove the resizable panel group. The existing resize state management is properly preserved, and theworktreePathprop is correctly passed from the active workspace query.apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (1)
159-183: LGTM!The branch list rendering logic is well-structured with clear conditional formatting for current, checked-out, and available branches. The relative time display using
formatRelativeTimeintegrates cleanly.apps/desktop/src/lib/trpc/routers/changes/changes.ts (1)
22-109: LGTM: getBranches enhancement aligns well with PR objectives.The refactored
getBranchesprocedure effectively:
- Retrieves last commit dates for local branches via a single git command
- Computes checked-out branches from the database to mark them as "(in use)" in the UI
- Provides appropriate fallback handling
The implementation supports the PR's goal of displaying relative time since last commit and marking checked-out branches.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (1)
34-35: Home directory regex is macOS-specific.This issue was already flagged in a previous review. The regex
/^\/Users\/[^/]+/only matches macOS home directories and won't work for Linux or Windows paths.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (1)
79-91: Consider adding loading state during branch switch.The branch switching logic is well-implemented with proper guards, error handling, and query invalidation. However, there's no visual feedback while the mutation is in progress.
Consider adding a loading state:
export function WorkspaceHeader({ worktreePath }: WorkspaceHeaderProps) { const [open, setOpen] = useState(false); const [search, setSearch] = useState(""); const [visibleCount, setVisibleCount] = useState(PAGE_SIZE); + const [isSwitching, setIsSwitching] = useState(false); // ... rest of code ... const handleBranchChange = async (branch: string) => { if (!worktreePath || branch === currentBranch) return; setOpen(false); + setIsSwitching(true); try { await switchBranch.mutateAsync({ worktreePath, branch }); utils.workspaces.getActive.invalidate(); utils.changes.getBranches.invalidate(); } catch (error) { toast.error( error instanceof Error ? error.message : "Failed to switch branch", ); + } finally { + setIsSwitching(false); } };Then use
isSwitchingto disable the button or show a spinner.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/changes/changes.ts(5 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts(1 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts(1 hunks)apps/desktop/src/renderer/lib/formatRelativeTime/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx(0 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/lib/formatRelativeTime/index.ts
- apps/desktop/src/lib/trpc/routers/changes/changes.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.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/WorkspaceHeader/WorkspaceHeader.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.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/WorkspaceHeader/WorkspaceHeader.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsxapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.tsapps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
🧠 Learnings (6)
📚 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/WorkspaceHeader/WorkspaceHeader.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/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.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/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.test.{ts,tsx} : Place tests co-located with component files using .test.tsx suffix
Applied to files:
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
🧬 Code graph analysis (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (4)
packages/ui/src/components/ui/tooltip.tsx (3)
Tooltip(76-76)TooltipTrigger(76-76)TooltipContent(76-76)packages/ui/src/components/ui/command.tsx (5)
Command(175-175)CommandInput(177-177)CommandList(178-178)CommandEmpty(179-179)CommandItem(181-181)apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
formatRelativeTime(1-14)apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx (1)
OpenInButton(86-226)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts (1)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
formatRelativeTime(1-14)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
apps/desktop/src/renderer/lib/formatRelativeTime/index.ts (1)
formatRelativeTime(1-1)
⏰ 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). (4)
- GitHub Check: Deploy Web
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Build
🔇 Additional comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceHeader/WorkspaceHeader.tsx (7)
1-19: LGTM!Imports are well-organized and use appropriate aliases. The PAGE_SIZE constant for pagination is clearly defined.
37-46: LGTM!TRPC queries and mutations are properly set up with type safety. The conditional query enablement based on
worktreePathprevents unnecessary requests.
48-64: LGTM!The branch sorting logic correctly prioritizes the current branch, then checked-out branches, then the rest. The
useMemooptimization with proper dependencies prevents unnecessary re-sorting.
67-73: LGTM!The search filtering is correctly implemented with case-insensitive matching and proper memoization dependencies.
75-77: LGTM!Pagination logic is clean and straightforward, correctly calculating visible branches and remaining items.
93-99: LGTM!The popover state management correctly resets search and pagination when closing, ensuring a clean state on reopen.
102-209: LGTM!The rendering logic is well-structured with:
- Proper conditional rendering based on data availability
- Accessible tooltip for path display
- Clean CSS-based text truncation with fade effect
- Proper integration of Command component for searchable branch selection
- Correct handling of branch states (current, checked-out, disabled)
- Pagination controls for large branch lists
The
lastCommitDate > 0check on line 164 correctly prevents displaying stale time formatting for invalid timestamps.
| it('returns "now" for timestamps less than 1 minute ago', () => { | ||
| mockNow(); | ||
| expect(formatRelativeTime(NOW)).toBe("now"); | ||
| expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now"); // 30 seconds ago | ||
| expect(formatRelativeTime(NOW - 59 * 1000)).toBe("now"); // 59 seconds ago | ||
| restoreNow(); | ||
| }); | ||
|
|
||
| it("returns minutes for timestamps between 1-59 minutes ago", () => { | ||
| mockNow(); | ||
| expect(formatRelativeTime(NOW - 1 * MINUTE)).toBe("1m"); | ||
| expect(formatRelativeTime(NOW - 5 * MINUTE)).toBe("5m"); | ||
| expect(formatRelativeTime(NOW - 30 * MINUTE)).toBe("30m"); | ||
| expect(formatRelativeTime(NOW - 59 * MINUTE)).toBe("59m"); | ||
| restoreNow(); | ||
| }); | ||
|
|
||
| it("returns hours for timestamps between 1-23 hours ago", () => { | ||
| mockNow(); | ||
| expect(formatRelativeTime(NOW - 1 * HOUR)).toBe("1h"); | ||
| expect(formatRelativeTime(NOW - 5 * HOUR)).toBe("5h"); | ||
| expect(formatRelativeTime(NOW - 12 * HOUR)).toBe("12h"); | ||
| expect(formatRelativeTime(NOW - 23 * HOUR)).toBe("23h"); | ||
| restoreNow(); | ||
| }); | ||
|
|
||
| it("returns days for timestamps between 1-29 days ago", () => { | ||
| mockNow(); | ||
| expect(formatRelativeTime(NOW - 1 * DAY)).toBe("1d"); | ||
| expect(formatRelativeTime(NOW - 7 * DAY)).toBe("7d"); | ||
| expect(formatRelativeTime(NOW - 14 * DAY)).toBe("14d"); | ||
| expect(formatRelativeTime(NOW - 29 * DAY)).toBe("29d"); | ||
| restoreNow(); | ||
| }); | ||
|
|
||
| it("returns months for timestamps 30+ days ago", () => { | ||
| mockNow(); | ||
| expect(formatRelativeTime(NOW - 30 * DAY)).toBe("1mo"); | ||
| expect(formatRelativeTime(NOW - 60 * DAY)).toBe("2mo"); | ||
| expect(formatRelativeTime(NOW - 90 * DAY)).toBe("3mo"); | ||
| expect(formatRelativeTime(NOW - 365 * DAY)).toBe("12mo"); | ||
| restoreNow(); | ||
| }); | ||
|
|
||
| it("handles edge cases at boundaries", () => { | ||
| mockNow(); | ||
| // Just under 1 hour should show 59m | ||
| expect(formatRelativeTime(NOW - 59 * MINUTE - 59 * 1000)).toBe("59m"); | ||
| // Exactly 1 hour should show 1h | ||
| expect(formatRelativeTime(NOW - 60 * MINUTE)).toBe("1h"); | ||
| // Just under 1 day should show 23h | ||
| expect(formatRelativeTime(NOW - 23 * HOUR - 59 * MINUTE)).toBe("23h"); | ||
| // Exactly 1 day should show 1d | ||
| expect(formatRelativeTime(NOW - 24 * HOUR)).toBe("1d"); | ||
| restoreNow(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split tests to have one assertion per test case.
Each test currently contains multiple assertions (3-4 per test), which violates the coding guideline: "Tests should have one assert per test".
As per coding guidelines, split each test into separate test cases. For example:
- it('returns "now" for timestamps less than 1 minute ago', () => {
+ it('returns "now" for current timestamp', () => {
mockNow();
expect(formatRelativeTime(NOW)).toBe("now");
- expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now"); // 30 seconds ago
- expect(formatRelativeTime(NOW - 59 * 1000)).toBe("now"); // 59 seconds ago
restoreNow();
});
+
+ it('returns "now" for timestamp 30 seconds ago', () => {
+ mockNow();
+ expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now");
+ restoreNow();
+ });
+
+ it('returns "now" for timestamp 59 seconds ago', () => {
+ mockNow();
+ expect(formatRelativeTime(NOW - 59 * 1000)).toBe("now");
+ restoreNow();
+ });Apply similar splitting for all remaining test cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('returns "now" for timestamps less than 1 minute ago', () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW)).toBe("now"); | |
| expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now"); // 30 seconds ago | |
| expect(formatRelativeTime(NOW - 59 * 1000)).toBe("now"); // 59 seconds ago | |
| restoreNow(); | |
| }); | |
| it("returns minutes for timestamps between 1-59 minutes ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * MINUTE)).toBe("1m"); | |
| expect(formatRelativeTime(NOW - 5 * MINUTE)).toBe("5m"); | |
| expect(formatRelativeTime(NOW - 30 * MINUTE)).toBe("30m"); | |
| expect(formatRelativeTime(NOW - 59 * MINUTE)).toBe("59m"); | |
| restoreNow(); | |
| }); | |
| it("returns hours for timestamps between 1-23 hours ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * HOUR)).toBe("1h"); | |
| expect(formatRelativeTime(NOW - 5 * HOUR)).toBe("5h"); | |
| expect(formatRelativeTime(NOW - 12 * HOUR)).toBe("12h"); | |
| expect(formatRelativeTime(NOW - 23 * HOUR)).toBe("23h"); | |
| restoreNow(); | |
| }); | |
| it("returns days for timestamps between 1-29 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * DAY)).toBe("1d"); | |
| expect(formatRelativeTime(NOW - 7 * DAY)).toBe("7d"); | |
| expect(formatRelativeTime(NOW - 14 * DAY)).toBe("14d"); | |
| expect(formatRelativeTime(NOW - 29 * DAY)).toBe("29d"); | |
| restoreNow(); | |
| }); | |
| it("returns months for timestamps 30+ days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 30 * DAY)).toBe("1mo"); | |
| expect(formatRelativeTime(NOW - 60 * DAY)).toBe("2mo"); | |
| expect(formatRelativeTime(NOW - 90 * DAY)).toBe("3mo"); | |
| expect(formatRelativeTime(NOW - 365 * DAY)).toBe("12mo"); | |
| restoreNow(); | |
| }); | |
| it("handles edge cases at boundaries", () => { | |
| mockNow(); | |
| // Just under 1 hour should show 59m | |
| expect(formatRelativeTime(NOW - 59 * MINUTE - 59 * 1000)).toBe("59m"); | |
| // Exactly 1 hour should show 1h | |
| expect(formatRelativeTime(NOW - 60 * MINUTE)).toBe("1h"); | |
| // Just under 1 day should show 23h | |
| expect(formatRelativeTime(NOW - 23 * HOUR - 59 * MINUTE)).toBe("23h"); | |
| // Exactly 1 day should show 1d | |
| expect(formatRelativeTime(NOW - 24 * HOUR)).toBe("1d"); | |
| restoreNow(); | |
| }); | |
| it('returns "now" for current timestamp', () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW)).toBe("now"); | |
| restoreNow(); | |
| }); | |
| it('returns "now" for timestamp 30 seconds ago', () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 30 * 1000)).toBe("now"); | |
| restoreNow(); | |
| }); | |
| it('returns "now" for timestamp 59 seconds ago', () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 59 * 1000)).toBe("now"); | |
| restoreNow(); | |
| }); | |
| it("returns minutes for 1 minute ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * MINUTE)).toBe("1m"); | |
| restoreNow(); | |
| }); | |
| it("returns minutes for 5 minutes ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 5 * MINUTE)).toBe("5m"); | |
| restoreNow(); | |
| }); | |
| it("returns minutes for 30 minutes ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 30 * MINUTE)).toBe("30m"); | |
| restoreNow(); | |
| }); | |
| it("returns minutes for 59 minutes ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 59 * MINUTE)).toBe("59m"); | |
| restoreNow(); | |
| }); | |
| it("returns hours for 1 hour ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * HOUR)).toBe("1h"); | |
| restoreNow(); | |
| }); | |
| it("returns hours for 5 hours ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 5 * HOUR)).toBe("5h"); | |
| restoreNow(); | |
| }); | |
| it("returns hours for 12 hours ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 12 * HOUR)).toBe("12h"); | |
| restoreNow(); | |
| }); | |
| it("returns hours for 23 hours ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 23 * HOUR)).toBe("23h"); | |
| restoreNow(); | |
| }); | |
| it("returns days for 1 day ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 1 * DAY)).toBe("1d"); | |
| restoreNow(); | |
| }); | |
| it("returns days for 7 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 7 * DAY)).toBe("7d"); | |
| restoreNow(); | |
| }); | |
| it("returns days for 14 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 14 * DAY)).toBe("14d"); | |
| restoreNow(); | |
| }); | |
| it("returns days for 29 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 29 * DAY)).toBe("29d"); | |
| restoreNow(); | |
| }); | |
| it("returns months for 30 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 30 * DAY)).toBe("1mo"); | |
| restoreNow(); | |
| }); | |
| it("returns months for 60 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 60 * DAY)).toBe("2mo"); | |
| restoreNow(); | |
| }); | |
| it("returns months for 90 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 90 * DAY)).toBe("3mo"); | |
| restoreNow(); | |
| }); | |
| it("returns months for 365 days ago", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 365 * DAY)).toBe("12mo"); | |
| restoreNow(); | |
| }); | |
| it("shows 59m just under 1 hour", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 59 * MINUTE - 59 * 1000)).toBe("59m"); | |
| restoreNow(); | |
| }); | |
| it("shows 1h exactly 1 hour", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 60 * MINUTE)).toBe("1h"); | |
| restoreNow(); | |
| }); | |
| it("shows 23h just under 1 day", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 23 * HOUR - 59 * MINUTE)).toBe("23h"); | |
| restoreNow(); | |
| }); | |
| it("shows 1d exactly 1 day", () => { | |
| mockNow(); | |
| expect(formatRelativeTime(NOW - 24 * HOUR)).toBe("1d"); | |
| restoreNow(); | |
| }); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.test.ts
around lines 19-74, the tests contain multiple assertions per it() block; split
each assertion into its own test so there is exactly one expect() per it(). For
each original group, create separate it() cases for each expectation (e.g.,
"returns now for 30s ago", "returns now for 59s ago"; "returns 1m", "returns
5m", etc.), call mockNow() at the start and restoreNow() at the end of each new
test (or use beforeEach/afterEach to set/restore), keep descriptive test names
matching the single assertion, and preserve existing constants (NOW, MINUTE,
HOUR, DAY) and edge-case checks as individual tests.
ca85ce9 to
4c7e677
Compare
- Display worktree path with folder icon in workspace header - Add searchable branch dropdown with pagination (10 branches per page) - Show relative time since last commit for each branch (e.g., 2h, 3d) - Mark checked-out branches as disabled with "(in use)" label - Sort branches: current first, then checked-out, then by recent commit - Add switchBranch tRPC mutation to change branches within worktree - Extract formatRelativeTime utility with tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Kitenite
force-pushed
the
more-top-bar-space
branch
from
4c7e677 to
03d67db
Compare
Extract WorkspaceHeader into smaller, focused components following project structure guidelines: - PathDisplay: Shows truncated path with tooltip - BranchSelector: Branch dropdown with search, pagination, and checked-out branch handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…play Replace macOS-specific regex with shared shortenHomePath utility that uses trpc to get the home directory and handles Windows backslashes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
switchBranchtRPC mutation to change branches within worktreeformatRelativeTimeutility with testsTest plan
bun test formatRelativeTimeto verify utility tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.