fix(desktop): fetch latest file content when clicking already-open file#660
fix(desktop): fetch latest file content when clicking already-open file#660
Conversation
When clicking on a file in the changes sidebar that's already open in a pane, the content wasn't refreshing because tRPC queries used cached data. Added a contentVersion counter to FileViewerState that increments when clicking on an already-open file, triggering a refetch of the file content. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded@Kitenite has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 32 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 (4)
📝 WalkthroughWalkthroughThe changes enhance file-viewer behavior by automatically pinning reused preview panes when the same file is reopened, and improve sidebar file-open handlers to extract worktreePath from the active workspace and invalidate file content queries upon file access. Changes
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant TRPC as TRPC Utils
participant TabsStore as Tabs Store
User->>Sidebar: Click file to open
Sidebar->>Sidebar: Extract worktreePath
Sidebar->>TRPC: Invalidate file content cache
TRPC-->>Sidebar: Cache invalidated
Sidebar->>TabsStore: Add/reuse file-viewer pane
alt Pane is reused & unpinned
TabsStore->>TabsStore: Pin pane & focus
end
TabsStore-->>Sidebar: Pane updated
Sidebar-->>User: File opened & displayed
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts:
- Around line 72-85: The effect in useFileContent checks both contentVersion and
prevContentVersionRef.current, so when a new pane starts with contentVersion
undefined the first re-click increments to 1 but prev stays undefined and no
refetch occurs; fix this by initializing the pane's contentVersion to 0 when
creating file viewer panes (update the createFileViewerPane / pane creation
logic in store.ts where contentVersion is set) so subsequent increments produce
defined previous and current values and the existing effect in useFileContent
will trigger refetchRaw/refetchDiff as intended.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/shared/tabs-types.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/shared/tabs-types.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/shared/tabs-types.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/shared/tabs-types.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/shared/tabs-types.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
apps/cli/src/lib/storage/lowdb-adapter.ts (1)
set(162-172)
⏰ 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 (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsx (1)
79-79: LGTM!The
contentVersionis correctly extracted from the file viewer state and passed through to theuseFileContenthook. This plumbing enables the content refetch mechanism.Also applies to: 106-106
apps/desktop/src/shared/tabs-types.ts (1)
103-104: LGTM!The
contentVersionproperty is correctly defined as optional with clear documentation. The optional type aligns with the implementation where it startsundefinedand increments to positive integers when the file is re-opened.apps/desktop/src/renderer/stores/tabs/store.ts (2)
400-421: LGTM! Content version incremented correctly for pinned panes.When focusing an already-open pinned file, the
contentVersionis correctly incremented to trigger a content refetch. The increment logic(existingFileViewer.contentVersion ?? 0) + 1properly handles both the initial case (undefined → 1) and subsequent increments.
449-469: LGTM! Content version incremented correctly for preview panes.When clicking the same file that's already in a preview pane, the pane is correctly pinned and
contentVersionis incremented to trigger a content refetch. The increment logic properly handles the undefined → 1 transition and subsequent increments.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts (2)
37-46: LGTM! Raw file query correctly separated with refetch capability.The raw file query is properly separated and exposes the
refetchRawfunction, which is used by the content version effect.
48-66: LGTM! Diff query correctly separated with refetch capability.The diff query is properly separated and exposes the
refetchDifffunction, which is used by the content version effect.
Replace the contentVersion counter approach with React Query's built-in cache invalidation mechanism. This is a cleaner pattern that: - Doesn't pollute the data model with implementation details - Uses React Query's native cache management - Provides better separation of concerns Changes: - Export queryClient as singleton from TRPCProvider for store access - Add worktreePath to AddFileViewerPaneOptions for query invalidation - Remove contentVersion field from FileViewerState - Remove contentVersion-related logic from useFileContent hook 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use trpc.useUtils() in the Sidebar component instead of exporting queryClient as a singleton. This is the idiomatic React Query pattern: - Store handles only state management - Component handles side effects like query invalidation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx:
- Around line 14-25: The invalidateFileContent helper currently discards the
promises from trpcUtils.changes.readWorkingFile.invalidate and
trpcUtils.changes.getFileContents.invalidate, which can hide failures; update
invalidateFileContent to await or handle those promises and catch errors,
logging contextual details (including worktreePath and filePath and which
invalidate call failed) instead of using bare void; reference the
invalidateFileContent function and the two invalidate calls so you can wrap each
invalidate in a try/catch or use Promise.all(...).catch(...) and call the
appropriate logger (e.g., console.error or the app logger) with a clear message.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsxapps/desktop/src/renderer/stores/tabs/store.ts
⏰ 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 (4)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
435-454: LGTM! Pinning behavior improves UX.The logic correctly pins a preview pane when the same file is clicked again. This provides better UX by signaling that repeated clicks indicate intent to keep the file open permanently. The implementation properly updates both the
isPinnedstate and the focused pane.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (3)
9-9: LGTM! Proper extraction of worktreePath.The worktreePath is correctly extracted and used to conditionally enable file operations and cache invalidation.
28-40: LGTM! Proper conditional handler with cache invalidation.The handler correctly guards against missing
workspaceIdorworktreePath, opens the file in preview mode, and invalidates the cache to ensure fresh content is fetched. The sequence is correct: state update followed by cache invalidation.
43-55: LGTM! Consistent implementation for pinned file handling.The handler mirrors the preview mode handler but correctly opens files as pinned (
isPinned: true). Cache invalidation ensures fresh content is fetched for both pinned and preview modes.
Replace bare void with Promise.all().catch() to properly handle potential failures from invalidate calls. Logs worktreePath, filePath, and error details for debugging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…le (#660) * fix(desktop): fetch latest file content when clicking already-open file When clicking on a file in the changes sidebar that's already open in a pane, the content wasn't refreshing because tRPC queries used cached data. Now will invalidate when clicking once again.
Summary
contentVersioncounter toFileViewerStatethat increments when clicking on an already-open fileuseFileContenthook to refetch content whencontentVersionchangesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.