refactor: create fileExplorerStore, consolidate scattered file explor…

[reachraza]

Summary

• Create fileExplorerStore — new Zustand store (159 lines, 8 state fields, 9 actions) that consolidates file explorer UI state previously scattered across uiStore, App.tsx useState, and hook dependency props
• Migrate from uiStore: selectedFileIndex, fileTreeFilter, fileTreeFilterOpen (3 fields + 3 actions removed from uiStore)
• Migrate from App.tsx: filePreviewLoading, flatFileList, isGraphViewOpen, graphFocusFilePath, lastGraphFocusFilePath (5 useState + 1 useRef + 2 useCallbacks + 1 sync useEffect removed)
• Decouple hooks from prop threading: useAppHandlers reads setFilePreviewLoading from store directly; useFileTreeManagement reads fileTreeFilter from store directly
• Atomic document graph actions: focusFileInGraph(), openLastDocumentGraph(), closeGraphView() replace manual multi-setState coordination + ref sync pattern
• Fix file tree filter focus bug: RightPanel container onClick was stealing focus from the filter input via event bubbling — added guard to skip fileTreeContainerRef.focus() when the click target is the filter input

Test plan

• 25 new tests for fileExplorerStore (initial state, functional updaters, file preview loading, flat file list, document graph atomicity/lifecycle, non-React access, store reset)
• Updated uiStore tests (removed migrated file explorer tests, updated action stability tests)
• Updated useFileTreeManagement test (filter state now set via store instead of deps)
• TypeScript check passes (npx tsc --noEmit) — zero errors
• Full test suite passes (18,737 tests)
• Manual: File tree filter/search — open, type, clear, close/reopen
• Manual: File selection & keyboard navigation in file tree
• Manual: File preview loading indicator (especially on SSH remote sessions)
• Manual: Document Graph open → close → reopen-last lifecycle
• Manual: Click away from filter input, click back — should retain focus without holding mouse

[claude]

PR Review: fileExplorerStore Migration

Summary

This is an excellent refactoring that consolidates scattered file explorer state into a dedicated Zustand store. The migration is well-executed with comprehensive test coverage and follows established patterns in the codebase.

✅ Strengths

1. Architectural Improvement

• Clear separation of concerns: File explorer UI state now has a dedicated home instead of being split across uiStore, App.tsx useState, and hook dependencies
• Reduced prop threading: Hooks like useFileTreeManagement and useAppHandlers can now access state directly via store selectors
• Atomic operations: Document graph actions (focusFileInGraph, closeGraphView) replace manual multi-setState coordination, eliminating race conditions

2. Code Quality

• Excellent test coverage: 305 lines of tests covering all state transitions, functional updaters, atomic actions, lifecycle scenarios, and non-React access patterns
• Consistent patterns: Follows existing store patterns from uiStore and tabStore (functional updaters, non-React access helpers)
• Clean migration: Removed 5 useState, 1 useRef, 2 useCallbacks, and 1 sync useEffect from App.tsx (lines 513-526, 560-566)
• Proper documentation: Clear JSDoc comments explaining purpose and migration rationale

3. Bug Fix Included

• File tree filter focus issue (src/renderer/components/RightPanel.tsx:637-642): Added event target check to prevent container from stealing focus when clicking the filter input

  // Before: Container always stole focus
  onClick={() => fileTreeContainerRef.current?.focus()}
  
  // After: Skip focus when clicking filter input
  onClick={(e) => {
    if (e.target !== fileTreeFilterInputRef.current) {
      fileTreeContainerRef.current?.focus();
    }
  }}

4. Performance Benefits

• Eliminated ref synchronization: Removed lastGraphFocusFilePathRef and the useEffect that synced it (App.tsx:526, 563-565)
• Stable action references: Actions extracted via getFileExplorerActions() remain stable across renders
• Reduced re-renders: Components can subscribe to specific slices via Zustand selectors

🔍 Code Review Findings

Critical Issues

None found ✅

Minor Observations

1. No-op Comment in App.tsx:11590

setLastGraphFocusFilePath: () => {}, // no-op: focusFileInGraph sets both atomically

Observation: This no-op is intentional since focusFileInGraph now handles both graphFocusFilePath and lastGraphFocusFilePath atomically. However, consider whether this prop can be removed entirely from the interface if it's always a no-op.

Recommendation: Document in a follow-up whether the setLastGraphFocusFilePath prop is still needed or can be removed from the interface definition.

2. Test Store Reset Pattern

The test helper resetStore() (fileExplorerStore.test.ts:31-42) manually lists all fields. This is brittle if new state is added.

Current:

function resetStore() {
  useFileExplorerStore.setState({
    selectedFileIndex: 0,
    fileTreeFilter: '',
    // ... all 8 fields manually listed
  });
}

Suggestion: Consider extracting initial state as a constant:

const INITIAL_STATE: FileExplorerStoreState = {
  selectedFileIndex: 0,
  fileTreeFilter: '',
  // ...
};

export const useFileExplorerStore = create<FileExplorerStore>()((set, get) => ({
  ...INITIAL_STATE,
  // actions...
}));

// In tests:
function resetStore() {
  useFileExplorerStore.setState(INITIAL_STATE);
}

This ensures tests and store initialization stay in sync. Not blocking for this PR.

3. TypeScript any in Tests

src/tests/renderer/hooks/useFileTreeManagement.test.ts uses any[] for flatFileList mock (removed in this PR). Good cleanup.

🔒 Security

No concerns ✅

• No user input validation issues
• No external API calls
• No authentication/authorization changes

⚡ Performance

Positive impact ✅

• Removes unnecessary useState updates in App.tsx
• Eliminates ref synchronization overhead
• Atomic state updates reduce intermediate render cycles

🧪 Test Coverage

Excellent ✅

• 25 new tests for fileExplorerStore
• Updated tests for affected components (useFileTreeManagement, uiStore)
• All scenarios covered: initial state, functional updaters, atomic actions, full lifecycle, non-React access, store reset
• TypeScript passes: npx tsc --noEmit with zero errors
• 18,737 tests pass in full suite

📋 Adherence to CLAUDE.md

✅ Follows Guidelines

1. Tabs for indentation: Consistent throughout
2. Error handling: No changes to error handling (migration only)
3. Scope discipline: Surgical changes, only touched what's needed
4. Dead code hygiene: Properly removed migrated state from uiStore
5. Standardized vernacular: Uses established terms (Right Bar, File Explorer)
6. Test coverage: Comprehensive tests added
7. Documentation: Clear comments explaining rationale

✅ Matches Existing Patterns

• Store structure matches uiStore.ts and tabStore.ts
• Functional updater pattern (resolves value-or-function)
• Non-React access helpers (getFileExplorerState, getFileExplorerActions)
• Test structure matches uiStore.test.ts

🎯 Recommendations

Must Address Before Merge

None - PR is ready to merge as-is ✅

Nice to Have (Future Work)

1. Extract initial state constant (see observation Add Claude Code GitHub Workflow #2) for easier maintenance
2. Consider removing setLastGraphFocusFilePath prop if it's always a no-op now
3. Add JSDoc examples to atomic actions showing typical usage patterns:

   /**
    * Open graph focused on a file. Atomically sets focus path, last path, and opens view.
    * @example
    * // Open document graph for a specific file
    * useFileExplorerStore.getState().focusFileInGraph('src/components/App.tsx');
    */

✨ Final Verdict

Approve with enthusiasm ✅

This is a textbook example of how to refactor scattered state into a cohesive store:

• Clear motivation (consolidate 3 scattered sources)
• Comprehensive test coverage (305 lines of tests)
• Includes bug fix (filter focus issue)
• Performance improvements (eliminates ref sync, reduces re-renders)
• Zero breaking changes (all consumers updated)
• Follows established patterns

The code is production-ready. Great work! 🎉

---

Confidence: High - Reviewed 515 additions, 133 deletions across 9 files with comprehensive test coverage.