From ec271ce225136d3d20aeb804ee0044f76edbe659 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 18 Oct 2025 20:06:25 -0500 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20=F0=9F=A4=96=20Trigger=20refresh=20a?= =?UTF-8?q?fter=20Track=20All=20to=20update=20UI=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After staging untracked files via Track All button, the UI should automatically refresh to reflect the changes (updated hunks, tree, and untracked count). Solution: Pass onRefresh callback from ReviewPanel through ReviewControls to UntrackedStatus. Call it after successful git add operation. This refresh only happens from user action (clicking Track All), preventing infinite loops. --- src/components/RightSidebar/CodeReview/ReviewControls.tsx | 1 + src/components/RightSidebar/CodeReview/UntrackedStatus.tsx | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/components/RightSidebar/CodeReview/ReviewControls.tsx b/src/components/RightSidebar/CodeReview/ReviewControls.tsx index d7f6c2587..0be80ce91 100644 --- a/src/components/RightSidebar/CodeReview/ReviewControls.tsx +++ b/src/components/RightSidebar/CodeReview/ReviewControls.tsx @@ -210,6 +210,7 @@ export const ReviewControls: React.FC = ({ workspaceId={workspaceId} workspacePath={workspacePath} refreshTrigger={refreshTrigger} + onRefresh={onRefresh} /> diff --git a/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx b/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx index 0d9c88e7e..adfa6e867 100644 --- a/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx +++ b/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx @@ -10,6 +10,7 @@ interface UntrackedStatusProps { workspaceId: string; workspacePath: string; refreshTrigger?: number; + onRefresh?: () => void; } const Container = styled.div` @@ -128,6 +129,7 @@ export const UntrackedStatus: React.FC = ({ workspaceId, workspacePath, refreshTrigger, + onRefresh, }) => { const [untrackedFiles, setUntrackedFiles] = useState([]); const [isLoading, setIsLoading] = useState(false); @@ -210,6 +212,8 @@ export const UntrackedStatus: React.FC = ({ if (result.success) { setUntrackedFiles([]); setShowTooltip(false); + // Trigger refresh to update hunks, tree, and untracked count + onRefresh?.(); } else { console.error("Failed to track files:", result.error); } From 18807152855f7d8bda72a44b3a5a5cc98ae0dce5 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 18 Oct 2025 20:12:07 -0500 Subject: [PATCH 2/3] =?UTF-8?q?refactor:=20=F0=9F=A4=96=20Rename=20'Dirty'?= =?UTF-8?q?=20to=20'Uncommitted'=20for=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Code Review panel checkbox was labeled 'Dirty' but actually includes ALL uncommitted changes (staged + unstaged) via git diff HEAD. 'Uncommitted' is more accurate and less ambiguous than 'Dirty'. Breaking change: localStorage keys changed from review-include-dirty to review-include-uncommitted. Users will lose this preference per workspace (acceptable for early development phase). Changes: - Renamed includeDirty → includeUncommitted in types and all components - Updated localStorage key: review-include-dirty → review-include-uncommitted - Updated UI label: 'Dirty' → 'Uncommitted' - Updated comments to reference 'uncommitted changes' --- .../CodeReview/ReviewControls.tsx | 12 +++++--- .../RightSidebar/CodeReview/ReviewPanel.tsx | 30 +++++++++---------- src/types/review.ts | 4 +-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/components/RightSidebar/CodeReview/ReviewControls.tsx b/src/components/RightSidebar/CodeReview/ReviewControls.tsx index 0be80ce91..74aceb059 100644 --- a/src/components/RightSidebar/CodeReview/ReviewControls.tsx +++ b/src/components/RightSidebar/CodeReview/ReviewControls.tsx @@ -162,8 +162,8 @@ export const ReviewControls: React.FC = ({ } }; - const handleDirtyToggle = (e: React.ChangeEvent) => { - onFiltersChange({ ...filters, includeDirty: e.target.checked }); + const handleUncommittedToggle = (e: React.ChangeEvent) => { + onFiltersChange({ ...filters, includeUncommitted: e.target.checked }); }; const handleSetDefault = () => { @@ -202,8 +202,12 @@ export const ReviewControls: React.FC = ({ )} - - Dirty + + Uncommitted = ({ // Persist diff base per workspace (falls back to global default) const [diffBase, setDiffBase] = usePersistedState(`review-diff-base:${workspaceId}`, defaultBase); - // Persist includeDirty flag per workspace - const [includeDirty, setIncludeDirty] = usePersistedState( - `review-include-dirty:${workspaceId}`, + // Persist includeUncommitted flag per workspace + const [includeUncommitted, setIncludeUncommitted] = usePersistedState( + `review-include-uncommitted:${workspaceId}`, false ); @@ -305,7 +305,7 @@ export const ReviewPanel: React.FC = ({ showReviewed: true, statusFilter: "all", diffBase: diffBase, - includeDirty: includeDirty, + includeUncommitted: includeUncommitted, }); // Load file tree - when workspace, diffBase, or refreshTrigger changes @@ -317,7 +317,7 @@ export const ReviewPanel: React.FC = ({ try { const numstatCommand = buildGitDiffCommand( filters.diffBase, - filters.includeDirty, + filters.includeUncommitted, "", // No path filter for file tree "numstat" ); @@ -352,7 +352,7 @@ export const ReviewPanel: React.FC = ({ return () => { cancelled = true; }; - }, [workspaceId, workspacePath, filters.diffBase, filters.includeDirty, refreshTrigger]); + }, [workspaceId, workspacePath, filters.diffBase, filters.includeUncommitted, refreshTrigger]); // Load diff hunks - when workspace, diffBase, selected path, or refreshTrigger changes useEffect(() => { @@ -369,7 +369,7 @@ export const ReviewPanel: React.FC = ({ const diffCommand = buildGitDiffCommand( filters.diffBase, - filters.includeDirty, + filters.includeUncommitted, pathFilter, "diff" ); @@ -435,7 +435,7 @@ export const ReviewPanel: React.FC = ({ workspaceId, workspacePath, filters.diffBase, - filters.includeDirty, + filters.includeUncommitted, selectedFilePath, refreshTrigger, ]); @@ -445,10 +445,10 @@ export const ReviewPanel: React.FC = ({ setDiffBase(filters.diffBase); }, [filters.diffBase, setDiffBase]); - // Persist includeDirty when it changes + // Persist includeUncommitted when it changes useEffect(() => { - setIncludeDirty(filters.includeDirty); - }, [filters.includeDirty, setIncludeDirty]); + setIncludeUncommitted(filters.includeUncommitted); + }, [filters.includeUncommitted, setIncludeUncommitted]); // For MVP: No review state tracking, just show all hunks const filteredHunks = hunks; diff --git a/src/types/review.ts b/src/types/review.ts index ebfe642d3..c6797885d 100644 --- a/src/types/review.ts +++ b/src/types/review.ts @@ -82,8 +82,8 @@ export interface ReviewFilters { filePathFilter?: string; /** Base reference to diff against (e.g., "HEAD", "main", "origin/main") */ diffBase: string; - /** Whether to include uncommitted dirty changes in the diff */ - includeDirty: boolean; + /** Whether to include uncommitted changes (staged + unstaged) in the diff */ + includeUncommitted: boolean; } /** From c0bb91c677300e07a5db9d602d11b92fc68f8792 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sat, 18 Oct 2025 20:14:10 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20=F0=9F=A4=96=20Eliminate=20race=20co?= =?UTF-8?q?nditions=20in=20UntrackedStatus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed three potential race conditions: 1. **Optimistic update race**: Removed setUntrackedFiles([]) call before onRefresh(). Now refresh reloads from git, avoiding flicker if git index hasn't flushed yet. 2. **Concurrent load race**: Added loadingRef to prevent multiple simultaneous git ls-files commands when refreshTrigger changes rapidly. 3. **hasLoadedOnce persistence**: This ref already resets per-workspace via workspaceId dependency, so no fix needed here. The refresh now correctly happens AFTER git add completes successfully, and the UI updates are sourced from the subsequent git query rather than optimistic local state. --- .../RightSidebar/CodeReview/UntrackedStatus.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx b/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx index adfa6e867..4c8867fb1 100644 --- a/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx +++ b/src/components/RightSidebar/CodeReview/UntrackedStatus.tsx @@ -137,12 +137,17 @@ export const UntrackedStatus: React.FC = ({ const [isTracking, setIsTracking] = useState(false); const containerRef = useRef(null); const hasLoadedOnce = useRef(false); + const loadingRef = useRef(false); // Prevent concurrent loads // Load untracked files useEffect(() => { let cancelled = false; const loadUntracked = async () => { + // Prevent concurrent loads + if (loadingRef.current) return; + loadingRef.current = true; + // Only show loading on first load ever, not on subsequent refreshes if (!hasLoadedOnce.current) { setIsLoading(true); @@ -169,6 +174,7 @@ export const UntrackedStatus: React.FC = ({ } catch (err) { console.error("Failed to load untracked files:", err); } finally { + loadingRef.current = false; setIsLoading(false); } }; @@ -210,9 +216,10 @@ export const UntrackedStatus: React.FC = ({ ); if (result.success) { - setUntrackedFiles([]); + // Close tooltip first setShowTooltip(false); - // Trigger refresh to update hunks, tree, and untracked count + // Trigger refresh - this will reload untracked files from git + // Don't clear untrackedFiles optimistically to avoid flicker onRefresh?.(); } else { console.error("Failed to track files:", result.error);