refactor: extract actions and hooks from large files #690
Conversation
Refactor two high-priority large files to improve maintainability: **store.ts (924 → 675 lines, -27%)** - Extract `addFileViewerPane` logic to `actions/file-viewer-actions.ts` - Extract split operations to `actions/split-actions.ts` - Move `findNextTab` helper to `utils.ts` **prompt-input.tsx (1413 → 1272 lines, -10%)** - Extract speech recognition to `hooks/use-speech-recognition.ts` - Create `hooks/use-attachments.ts` for reusable attachment management - Extract blob URL conversion to `utils/blob-to-data-url.ts` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughThis PR adds new file viewer pane management and pane splitting actions while refactoring the tabs store to delegate logic to dedicated action modules. It also introduces attachment and speech recognition hooks with supporting utilities for the AI elements UI components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/ai-elements/prompt-input.tsx (1)
713-755: Silent error handling in Promise chain.The
.catchblocks at lines 739-741 and 753-755 silently swallow errors without any logging. Per coding guidelines, errors should at minimum be logged with context.Suggested fix
if (result instanceof Promise) { result .then(() => { clear(); if (usingProvider) { controller.textInput.clear(); } }) - .catch(() => { + .catch((error) => { + console.error("[prompt-input] Submit handler error", error); // Don't clear on error - user may want to retry }); } else { // ... } - } catch { + } catch (error) { + console.error("[prompt-input] Submit handler error", error); // Don't clear on error - user may want to retry } }) - .catch(() => { + .catch((error) => { + console.error("[prompt-input] Blob URL conversion error", error); // Don't clear on error - user may want to retry });
🧹 Nitpick comments (6)
apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.ts (1)
100-109: Duplicated markdown extension check.This logic duplicates the
MARKDOWN_EXTENSIONSconstant and check fromutils.ts(lines 10, 130-134 increateFileViewerPane). Consider extracting a shared helper likeisMarkdownFile(filePath)to keep the logic DRY.♻️ Suggested refactor
In
utils.ts, add:export const isMarkdownFile = (filePath: string): boolean => MARKDOWN_EXTENSIONS.some((ext) => filePath.endsWith(ext));Then in
file-viewer-actions.ts:+import { isMarkdownFile } from "../utils"; - } else if ( - options.filePath.endsWith(".md") || - options.filePath.endsWith(".markdown") || - options.filePath.endsWith(".mdx") - ) { + } else if (isMarkdownFile(options.filePath)) {apps/desktop/src/renderer/stores/tabs/actions/split-actions.ts (1)
16-23: Consider using object parameters per coding guidelines.The function has 6 parameters. Per the coding guidelines for
**/*.{ts,tsx}: "Use object parameters for functions with 2+ parameters instead of positional arguments."♻️ Suggested refactor
+interface SplitPaneParams { + state: TabsState; + tabId: string; + sourcePaneId: string; + direction: "row" | "column"; + path?: MosaicBranch[]; + options?: CreatePaneOptions; +} -export function splitPane( - state: TabsState, - tabId: string, - sourcePaneId: string, - direction: "row" | "column", - path?: MosaicBranch[], - options?: CreatePaneOptions, -): SplitResult | null { +export function splitPane({ + state, + tabId, + sourcePaneId, + direction, + path, + options, +}: SplitPaneParams): SplitResult | null {Apply similar changes to
splitPaneVerticalandsplitPaneHorizontal.packages/ui/src/components/ai-elements/hooks/use-speech-recognition.ts (2)
107-109: Extract hardcoded language to a named constant or make it configurable.The language
"en-US"is hardcoded. Consider extracting it as a module-level constant or adding it as an option toUseSpeechRecognitionOptionsfor flexibility.Suggested improvement
+const DEFAULT_LANGUAGE = "en-US"; + export interface UseSpeechRecognitionOptions { textareaRef?: RefObject<HTMLTextAreaElement | null>; onTranscriptionChange?: (text: string) => void; + language?: string; }Then use it:
- speechRecognition.lang = "en-US"; + speechRecognition.lang = language ?? DEFAULT_LANGUAGE;
98-155: Potential recognition restart ifonTranscriptionChangeis not memoized.The
useEffectrecreates theSpeechRecognitioninstance whenevertextareaReforonTranscriptionChangechanges. If the consumer passes an inline callback withoutuseCallback, this will create a new instance on every render, potentially interrupting active speech recognition.Consider using a ref for the callback to avoid this issue, or document that
onTranscriptionChangeshould be memoized.Suggested fix using a ref for the callback
export function useSpeechRecognition({ textareaRef, onTranscriptionChange, }: UseSpeechRecognitionOptions): UseSpeechRecognitionResult { const [isListening, setIsListening] = useState(false); const [recognition, setRecognition] = useState<SpeechRecognition | null>( null, ); const recognitionRef = useRef<SpeechRecognition | null>(null); + const onTranscriptionChangeRef = useRef(onTranscriptionChange); + onTranscriptionChangeRef.current = onTranscriptionChange; useEffect(() => { // ... setup code ... if (finalTranscript && textareaRef?.current) { // ... - onTranscriptionChange?.(newValue); + onTranscriptionChangeRef.current?.(newValue); } }; // ... - }, [textareaRef, onTranscriptionChange]); + }, [textareaRef]);packages/ui/src/components/ai-elements/hooks/use-attachments.ts (1)
83-132: Consider logging rejected files for debugging purposes.The
addfunction correctly reports errors viaonError, but when some files are rejected (e.g., wrong type or size) while others are accepted, there's no visibility into which specific files failed. Adding debug logging would help troubleshoot issues.Optional: Add debug logging
const add = useCallback( (fileList: File[] | FileList) => { const incoming = Array.from(fileList); const accepted = incoming.filter((f) => matchesAccept(f)); if (incoming.length && accepted.length === 0) { + console.debug("[attachments] No files matched accept pattern", { accept, files: incoming.map(f => f.name) }); onError?.({ code: "accept", message: "No files match the accepted types.", }); return; }packages/ui/src/components/ai-elements/prompt-input.tsx (1)
493-589: Consider usinguseAttachmentshook to reduce code duplication.The
matchesAccept,addLocal,removeLocal, andclearLocalfunctions duplicate the logic in the newly createduseAttachmentshook. While the dual-mode architecture (provider vs local) may warrant keeping local state management in the component, the validation and blob URL lifecycle logic could be unified.If integration is not pursued, consider documenting why the duplication is intentional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.tsapps/desktop/src/renderer/stores/tabs/actions/split-actions.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/utils.tspackages/ui/src/components/ai-elements/hooks/index.tspackages/ui/src/components/ai-elements/hooks/use-attachments.tspackages/ui/src/components/ai-elements/hooks/use-speech-recognition.tspackages/ui/src/components/ai-elements/prompt-input.tsxpackages/ui/src/components/ai-elements/utils/blob-to-data-url.tspackages/ui/src/components/ai-elements/utils/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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:
packages/ui/src/components/ai-elements/hooks/index.tspackages/ui/src/components/ai-elements/utils/index.tsapps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.tspackages/ui/src/components/ai-elements/prompt-input.tsxapps/desktop/src/renderer/stores/tabs/actions/split-actions.tspackages/ui/src/components/ai-elements/hooks/use-speech-recognition.tspackages/ui/src/components/ai-elements/hooks/use-attachments.tspackages/ui/src/components/ai-elements/utils/blob-to-data-url.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/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:
packages/ui/src/components/ai-elements/hooks/index.tspackages/ui/src/components/ai-elements/utils/index.tsapps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.tspackages/ui/src/components/ai-elements/prompt-input.tsxapps/desktop/src/renderer/stores/tabs/actions/split-actions.tspackages/ui/src/components/ai-elements/hooks/use-speech-recognition.tspackages/ui/src/components/ai-elements/hooks/use-attachments.tspackages/ui/src/components/ai-elements/utils/blob-to-data-url.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/renderer/stores/tabs/store.ts
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/stores/tabs/actions/file-viewer-actions.tsapps/desktop/src/renderer/stores/tabs/actions/split-actions.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/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/stores/tabs/actions/file-viewer-actions.tsapps/desktop/src/renderer/stores/tabs/actions/split-actions.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/renderer/stores/tabs/store.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.tsapps/desktop/src/renderer/stores/tabs/actions/split-actions.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/renderer/stores/tabs/store.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
packages/ui/src/components/ai-elements/prompt-input.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
packages/ui/src/components/ai-elements/hooks/index.tspackages/ui/src/components/ai-elements/prompt-input.tsx
🧬 Code graph analysis (7)
apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.ts (3)
apps/desktop/src/renderer/stores/tabs/types.ts (2)
TabsState(26-28)AddFileViewerPaneOptions(41-52)apps/desktop/src/renderer/stores/tabs/utils.ts (2)
extractPaneIdsFromLayout(54-65)createFileViewerPane(120-158)apps/desktop/src/shared/types/mosaic.ts (1)
MosaicNode(1-8)
packages/ui/src/components/ai-elements/prompt-input.tsx (1)
packages/ui/src/components/ai-elements/hooks/use-speech-recognition.ts (1)
useSpeechRecognition(88-174)
apps/desktop/src/renderer/stores/tabs/actions/split-actions.ts (3)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
TabsState(26-28)apps/desktop/src/renderer/stores/tabs/utils.ts (2)
CreatePaneOptions(73-76)createPane(81-97)apps/desktop/src/shared/types/mosaic.ts (1)
MosaicNode(1-8)
packages/ui/src/components/ai-elements/hooks/use-speech-recognition.ts (1)
packages/ui/src/components/ai-elements/hooks/index.ts (3)
UseSpeechRecognitionOptions(9-9)UseSpeechRecognitionResult(10-10)useSpeechRecognition(11-11)
packages/ui/src/components/ai-elements/utils/blob-to-data-url.ts (2)
packages/ui/src/components/ai-elements/utils/index.ts (1)
convertBlobUrlToDataUrl(1-1)apps/desktop/src/main/lib/api-client.ts (1)
fetch(25-39)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
Tab(18-20)
apps/desktop/src/renderer/stores/tabs/store.ts (2)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
findNextTab(432-477)apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.ts (1)
addFileViewerPaneAction(17-142)
⏰ 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 API
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (15)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
424-477: Well-structured tab selection logic.The three-tier priority (history → position → fallback) is a good UX pattern. The implementation correctly filters by workspace and handles edge cases.
Minor observation: The checks
allWorkspaceTabs[nextIndex].id !== tabIdToClose(line 466) andallWorkspaceTabs[prevIndex].id !== tabIdToClose(line 470) are redundant sincenextIndexandprevIndexare computed fromcurrentIndex, and the tab atcurrentIndexis the one being closed—not the adjacent ones. This is harmless defensive code but could be simplified.apps/desktop/src/renderer/stores/tabs/actions/file-viewer-actions.ts (2)
17-48: Clean action pattern with proper state derivation.The pinned pane lookup correctly matches on
filePath,diffCategory, andcommitHashto identify exact duplicates. Returning bothresultandpaneIdis a good pattern that separates state changes from the returned identifier.
144-174: Well-encapsulated helper function.
createNewFileViewerPaneproperly delegates tocreateFileViewerPanefrom utils and constructs the mosaic layout. The immutable state update pattern is correctly applied.apps/desktop/src/renderer/stores/tabs/store.ts (4)
5-10: Clean import organization for action modules.The imports are well-organized, separating action modules from utilities. The aliasing of
splitPaneVertical as splitPaneVerticalActionavoids naming conflicts with the store methods.Also applies to: 18-18
347-353: Clean delegation to addFileViewerPaneAction.The store now correctly delegates to the extracted action and applies the result via
set(result). This maintains the same public API while improving maintainability.
548-568: Consistent pattern for split actions.Both
splitPaneVerticalandsplitPaneHorizontalfollow the same delegation pattern, correctly checking fornullbefore applying state updates. This aligns with the action module's contract of returningnullwhen validation fails.
104-108: Delegation to findNextTab simplifies removeTab.The refactored code correctly delegates tab selection to
findNextTab. The type system properly supports this pattern:findNextTabreturnsstring | null(which can benullwhen no tabs remain in the workspace), andactiveTabIdsis typed asRecord<string, string | null>, making the assignment type-safe.apps/desktop/src/renderer/stores/tabs/actions/split-actions.ts (2)
24-69: Solid split implementation with proper validation.The function correctly:
- Validates both tab existence and pane ownership (line 28)
- Uses
updateTreefor path-based splits, preserving existing layout structure- Falls back to wrapping the entire layout when no path is provided
- Returns immutable state updates
71-95: Clean convenience wrappers.
splitPaneVerticalandsplitPaneHorizontalare thin wrappers that map to the appropriate direction. The JSDoc comments correctly describe the visual result ("side by side" vs "stacked").packages/ui/src/components/ai-elements/hooks/use-speech-recognition.ts (1)
88-173: LGTM - Well-structured hook with proper cleanup.The hook correctly:
- Uses object parameters per coding guidelines
- Uses prefixed console logging
[speech-recognition]- Handles browser API availability gracefully
- Cleans up the recognition instance on unmount
packages/ui/src/components/ai-elements/hooks/use-attachments.ts (1)
44-183: LGTM - Solid attachment management hook.The hook properly:
- Uses object parameters with sensible defaults
- Validates file types (including wildcard patterns), sizes, and counts
- Manages blob URL lifecycle with proper cleanup on removal and unmount
- Uses the ref pattern to avoid stale closures in cleanup
packages/ui/src/components/ai-elements/utils/index.ts (1)
1-1: LGTM - Clean re-export.Simple barrel file with a single re-export. No issues.
packages/ui/src/components/ai-elements/hooks/index.ts (1)
1-12: LGTM - Clean public API surface.The barrel file properly consolidates types and functions from both hooks, providing a clean public API for consumers.
packages/ui/src/components/ai-elements/prompt-input.tsx (2)
72-73: LGTM - Clean integration of extracted hook and utility.The imports correctly reference the new extracted modules, enabling the speech recognition hook and blob URL utility to be used in the component.
1055-1068: LGTM - Speech recognition hook integration.The
PromptInputSpeechButtonnow cleanly uses the extracteduseSpeechRecognitionhook, replacing previous inline logic. TheisSupportedcheck properly disables the button when the browser doesn't support speech recognition.
| reader.onloadend = () => resolve(reader.result as string); | ||
| reader.onerror = () => resolve(null); | ||
| reader.readAsDataURL(blob); | ||
| }); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Errors are swallowed silently - add logging with context.
Per coding guidelines, errors should not be swallowed silently. The catch block and onerror handler both return null without any logging, making it difficult to debug conversion failures.
Suggested fix
export async function convertBlobUrlToDataUrl(
url: string,
): Promise<string | null> {
try {
const response = await fetch(url);
const blob = await response.blob();
return new Promise((resolve) => {
const reader = new FileReader();
reader.onloadend = () => resolve(reader.result as string);
- reader.onerror = () => resolve(null);
+ reader.onerror = () => {
+ console.error("[blob-to-data-url] FileReader error", { url });
+ resolve(null);
+ };
reader.readAsDataURL(blob);
});
- } catch {
+ } catch (error) {
+ console.error("[blob-to-data-url] Fetch error", { url, error });
return null;
}
}
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
store.ts(924 → 675 lines, -27%) by extracting file viewer and split actionsprompt-input.tsx(1413 → 1272 lines, -10%) by extracting speech recognition hook and utilitiesuseAttachmentshook for attachment managementChanges
Desktop - Tabs Store
store.tsutils.tsfindNextTab)actions/file-viewer-actions.tsactions/split-actions.tsUI - Prompt Input
prompt-input.tsxhooks/use-speech-recognition.tshooks/use-attachments.tsutils/blob-to-data-url.tsTest plan
bun run typecheckpassesbun run lint:fixpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.