Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/browser/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { buildSortedWorkspacesByProject } from "./utils/ui/workspaceFiltering";
import { useResumeManager } from "./hooks/useResumeManager";
import { useUnreadTracking } from "./hooks/useUnreadTracking";
import { useWorkspaceStoreRaw, useWorkspaceRecency } from "./stores/WorkspaceStore";
import { setActiveWorkspace } from "./stores/GitStatusStore";

import { useStableReference, compareMaps } from "./hooks/useStableReference";
import { CommandRegistryProvider, useCommandRegistry } from "./contexts/CommandRegistryContext";
Expand Down Expand Up @@ -133,6 +134,11 @@ function AppInner() {
prevWorkspaceRef.current = selectedWorkspace;
}, [selectedWorkspace, telemetry]);

// Tell GitStatusStore which workspace is active (for prioritized 1s refresh)
useEffect(() => {
setActiveWorkspace(selectedWorkspace?.workspaceId ?? null);
}, [selectedWorkspace?.workspaceId]);

// Track last-read timestamps for unread indicators
const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace);

Expand Down
34 changes: 30 additions & 4 deletions src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,30 @@
import React, { useState, useRef, useEffect } from "react";
import { Tooltip, TooltipTrigger, TooltipContent } from "@/browser/components/ui/tooltip";
import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds";
import { formatRelativeTimeCompact } from "@/browser/utils/ui/dateTime";
import { cn } from "@/common/lib/utils";
import type { LastRefreshInfo, RefreshTrigger } from "@/browser/utils/RefreshController";

interface RefreshButtonProps {
onClick: () => void;
isLoading?: boolean;
/** Debug info about last refresh (timestamp and trigger) */
lastRefreshInfo?: LastRefreshInfo | null;
}

export const RefreshButton: React.FC<RefreshButtonProps> = ({ onClick, isLoading = false }) => {
/** Human-readable trigger labels */
const TRIGGER_LABELS: Record<RefreshTrigger, string> = {
manual: "manual click",
scheduled: "tool completion",
priority: "tool completion (priority)",
focus: "window focus",
visibility: "tab visible",
unpaused: "interaction ended",
"in-flight-followup": "queued followup",
};

export const RefreshButton: React.FC<RefreshButtonProps> = (props) => {
const { onClick, isLoading = false, lastRefreshInfo } = props;
// Track animation state for graceful stopping
const [animationState, setAnimationState] = useState<"idle" | "spinning" | "stopping">("idle");
const spinOnceTimeoutRef = useRef<number | null>(null);
Expand Down Expand Up @@ -76,9 +92,19 @@ export const RefreshButton: React.FC<RefreshButtonProps> = ({ onClick, isLoading
</button>
</TooltipTrigger>
<TooltipContent side="bottom" align="start">
{animationState !== "idle"
? "Refreshing..."
: `Refresh diff (${formatKeybind(KEYBINDS.REFRESH_REVIEW)})`}
{animationState !== "idle" ? (
"Refreshing..."
) : (
<span>
Refresh diff ({formatKeybind(KEYBINDS.REFRESH_REVIEW)})
{lastRefreshInfo && (
<span className="text-muted block text-[10px]">
Last: {formatRelativeTimeCompact(lastRefreshInfo.timestamp)} via{" "}
{TRIGGER_LABELS[lastRefreshInfo.trigger] ?? lastRefreshInfo.trigger}
</span>
)}
</span>
)}
</TooltipContent>
</Tooltip>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import React, { useState } from "react";
import { usePersistedState } from "@/browser/hooks/usePersistedState";
import type { ReviewFilters, ReviewStats } from "@/common/types/review";
import type { LastRefreshInfo } from "@/browser/utils/RefreshController";
import { RefreshButton } from "./RefreshButton";
import { UntrackedStatus } from "./UntrackedStatus";

Expand All @@ -17,6 +18,8 @@ interface ReviewControlsProps {
workspaceId: string;
workspacePath: string;
refreshTrigger?: number;
/** Debug info about last refresh */
lastRefreshInfo?: LastRefreshInfo | null;
}

export const ReviewControls: React.FC<ReviewControlsProps> = ({
Expand All @@ -28,6 +31,7 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
workspaceId,
workspacePath,
refreshTrigger,
lastRefreshInfo,
}) => {
// Local state for input value - only commit on blur/Enter
const [inputValue, setInputValue] = useState(filters.diffBase);
Expand Down Expand Up @@ -83,7 +87,13 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({

return (
<div className="bg-separator border-border-light flex flex-wrap items-center gap-3 border-b px-3 py-2 text-[11px]">
{onRefresh && <RefreshButton onClick={onRefresh} isLoading={isLoading} />}
{onRefresh && (
<RefreshButton
onClick={onRefresh}
isLoading={isLoading}
lastRefreshInfo={lastRefreshInfo}
/>
)}
<label className="text-muted font-medium whitespace-nowrap">Base:</label>
<input
type="text"
Expand Down
109 changes: 105 additions & 4 deletions src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,31 @@ type DiffState =
const REVIEW_PANEL_CACHE_MAX_ENTRIES = 20;
const REVIEW_PANEL_CACHE_MAX_SIZE_BYTES = 20 * 1024 * 1024; // 20MB

/**
* Preserve object references for unchanged hunks to prevent re-renders.
* Compares by ID and content - if a hunk exists in prev with same content, reuse it.
*/
function preserveHunkReferences(prev: DiffHunk[], next: DiffHunk[]): DiffHunk[] {
if (prev.length === 0) return next;

const prevById = new Map(prev.map((h) => [h.id, h]));
let allSame = prev.length === next.length;

const result = next.map((hunk, i) => {
const prevHunk = prevById.get(hunk.id);
// Fast path: same ID and content means unchanged (content hash is part of ID)
if (prevHunk && prevHunk.content === hunk.content) {
if (allSame && prev[i]?.id !== hunk.id) allSame = false;
return prevHunk;
}
allSame = false;
return hunk;
});

// If all hunks are reused in same order, return prev array to preserve top-level reference
return allSame ? prev : result;
}

interface ReviewPanelDiffCacheValue {
hunks: DiffHunk[];
truncationWarning: string | null;
Expand All @@ -118,6 +143,53 @@ const reviewPanelCache = new LRUCache<string, ReviewPanelCacheValue>({
sizeCalculation: (value) => estimateJsonSizeBytes(value),
});

function getOriginBranchForFetch(diffBase: string): string | null {
const trimmed = diffBase.trim();
if (!trimmed.startsWith("origin/")) return null;

const branch = trimmed.slice("origin/".length);

// Avoid shell injection; diffBase is user-controlled.
if (!/^[0-9A-Za-z._/-]+$/.test(branch)) return null;

return branch;
}

interface OriginFetchState {
key: string;
promise: Promise<void>;
}

async function ensureOriginFetched(params: {
api: APIClient;
workspaceId: string;
diffBase: string;
refreshToken: number;
originFetchRef: React.MutableRefObject<OriginFetchState | null>;
}): Promise<void> {
const originBranch = getOriginBranchForFetch(params.diffBase);
if (!originBranch) return;

const key = [params.workspaceId, params.diffBase, String(params.refreshToken)].join("\u0000");
const existing = params.originFetchRef.current;
if (existing?.key === key) {
await existing.promise;
return;
}

// Ensure manual refresh doesn't hang on credential prompts.
const promise = params.api.workspace
.executeBash({
workspaceId: params.workspaceId,
script: `GIT_TERMINAL_PROMPT=0 git fetch origin ${originBranch} --quiet || true`,
options: { timeout_secs: 30 },
})
.then(() => undefined)
.catch(() => undefined);

params.originFetchRef.current = { key, promise };
await promise;
}
function makeReviewPanelCacheKey(params: {
workspaceId: string;
workspacePath: string;
Expand Down Expand Up @@ -161,6 +233,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
isCreating = false,
onStatsChange,
}) => {
const originFetchRef = useRef<OriginFetchState | null>(null);
const { api } = useAPI();
const panelRef = useRef<HTMLDivElement>(null);
const scrollContainerRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -309,6 +382,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
const loadFileTree = async () => {
setIsLoadingTree(true);
try {
await ensureOriginFetched({
api,
workspaceId,
diffBase: filters.diffBase,
refreshToken: refreshTrigger,
originFetchRef,
});
if (cancelled) return;

const tree = await executeWorkspaceBashAndCache({
api,
workspaceId,
Expand Down Expand Up @@ -420,6 +502,15 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({

const loadDiff = async () => {
try {
await ensureOriginFetched({
api,
workspaceId,
diffBase: filters.diffBase,
refreshToken: refreshTrigger,
originFetchRef,
});
if (cancelled) return;

// Git-level filters (affect what data is fetched):
// - diffBase: what to diff against
// - includeUncommitted: include working directory changes
Expand Down Expand Up @@ -458,10 +549,19 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
if (cancelled) return;

setDiagnosticInfo(data.diagnosticInfo);
setDiffState({
status: "loaded",
hunks: data.hunks,
truncationWarning: data.truncationWarning,

// Preserve object references for unchanged hunks to prevent unnecessary re-renders.
// HunkViewer is memoized on hunk object identity, so reusing references avoids
// re-rendering (and re-highlighting) hunks that haven't actually changed.
setDiffState((prev) => {
const prevHunks =
prev.status === "loaded" || prev.status === "refreshing" ? prev.hunks : [];
const hunks = preserveHunkReferences(prevHunks, data.hunks);
return {
status: "loaded",
hunks,
truncationWarning: data.truncationWarning,
};
});

if (data.hunks.length > 0) {
Expand Down Expand Up @@ -811,6 +911,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
workspaceId={workspaceId}
workspacePath={workspacePath}
refreshTrigger={refreshTrigger}
lastRefreshInfo={refreshController.lastRefreshInfo}
/>

{diffState.status === "error" ? (
Expand Down
Loading