Skip to content

Commit c875c5c

Browse files
authored
Refresh instead of hard reload (#107)
1 parent abc68ee commit c875c5c

12 files changed

Lines changed: 516 additions & 70 deletions

File tree

core/App.tsx

Lines changed: 137 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ import { isNativeInputTarget } from './lib/keyboard.ts';
6969
import { buildCommitModel, buildGenericCommitModel } from './lib/narrative-walkthrough.ts';
7070
import {
7171
consumeReloadSelection,
72+
getChangedPaths,
7273
getReloadDeltaPaths,
7374
getReloadHistorySource,
75+
getReloadMainMode,
7476
getReloadSelectionPath,
7577
writeReloadSelection,
7678
} from './lib/reload-selection.ts';
@@ -650,6 +652,17 @@ export default function App() {
650652
: {};
651653
const reloadSelectedPath = getReloadSelectionPath(reloadSelection, orderedState);
652654
const nextReloadDeltaPaths = getReloadDeltaPaths(reloadSelection, orderedState);
655+
// Reopen the commit view after a reload, but only while it would still be
656+
// openable (same conditions as openCommitView); e.g. once the commit
657+
// lands the working tree may be empty and we fall back to the review.
658+
const restoreCommitView =
659+
getReloadMainMode(reloadSelection, orderedState) === 'commit' &&
660+
orderedState.source.type === 'working-tree' &&
661+
orderedState.files.length > 0;
662+
if (restoreCommitView) {
663+
setSidebarMode('tree');
664+
setMainMode('commit');
665+
}
653666

654667
setHistoryEntries(history.entries);
655668
setHistoryHasMore(history.entries.length >= HISTORY_PAGE_SIZE);
@@ -1352,9 +1365,63 @@ export default function App() {
13521365
[scrollPathIntoReview],
13531366
);
13541367

1355-
const reloadWindow = useCallback(() => {
1356-
window.location.reload();
1357-
}, []);
1368+
// Refresh the repository state in place after the working tree changed.
1369+
// Unlike a window reload, this keeps all review UI state (selection, scroll,
1370+
// search, walkthrough navigation, commit drafts, pending comments) and only
1371+
// re-renders the files whose fingerprints actually moved.
1372+
const refreshRepository = useCallback(() => {
1373+
const previousState = stateRef.current;
1374+
if (!previousState || pendingSource) {
1375+
return;
1376+
}
1377+
1378+
const request = sourceRequestRef.current + 1;
1379+
sourceRequestRef.current = request;
1380+
1381+
Promise.all([
1382+
window.codiff.getRepositoryState(previousState.source),
1383+
window.codiff.getRepositoryHistory(historyLimit, historySourceRef.current ?? undefined),
1384+
])
1385+
.then(([nextState, history]) => {
1386+
if (sourceRequestRef.current !== request) {
1387+
return;
1388+
}
1389+
1390+
const orderedState = {
1391+
...nextState,
1392+
files: sortFiles(nextState.files),
1393+
};
1394+
const changedPaths = getChangedPaths(previousState.files, orderedState.files);
1395+
1396+
stateGenerationRef.current += 1;
1397+
setState(orderedState);
1398+
setReloadDeltaPaths(changedPaths);
1399+
for (const path of changedPaths) {
1400+
bumpItemVersion(path);
1401+
}
1402+
setHistoryEntries(history.entries);
1403+
setHistoryHasMore(history.entries.length >= historyLimit);
1404+
setSelectedPath((current) =>
1405+
current != null && orderedState.files.some((file) => file.path === current)
1406+
? current
1407+
: (orderedState.files[0]?.path ?? null),
1408+
);
1409+
if (
1410+
mainModeRef.current === 'commit' &&
1411+
(orderedState.source.type !== 'working-tree' || orderedState.files.length === 0)
1412+
) {
1413+
setMainMode('review');
1414+
}
1415+
setLocalChangesDetected(false);
1416+
})
1417+
.catch(() => {
1418+
// Keep the current state; the banner stays up as a retry affordance.
1419+
});
1420+
}, [bumpItemVersion, historyLimit, pendingSource]);
1421+
1422+
// ⌘R / the View menu's "Refresh Changes" item route here from the main
1423+
// process instead of reloading the window.
1424+
useEffect(() => window.codiff.onRefreshRequest(refreshRepository), [refreshRepository]);
13581425

13591426
// Commit the files a reviewer chose from the walkthrough's staging set. The
13601427
// working-tree watcher surfaces a "reload to see changes" banner afterwards.
@@ -1380,7 +1447,12 @@ export default function App() {
13801447

13811448
useEffect(() => {
13821449
const writeCurrentReloadSelection = () => {
1383-
writeReloadSelection(stateRef.current, selectedPathRef.current, historySourceRef.current);
1450+
writeReloadSelection(
1451+
stateRef.current,
1452+
selectedPathRef.current,
1453+
historySourceRef.current,
1454+
mainModeRef.current,
1455+
);
13841456
};
13851457

13861458
window.addEventListener('beforeunload', writeCurrentReloadSelection);
@@ -1514,6 +1586,58 @@ export default function App() {
15141586
handle.addEventListener('pointercancel', handleEnd);
15151587
}, []);
15161588

1589+
// Ask the connected agent for a narrative walkthrough of the given source.
1590+
// Results are dropped if the reviewer switched sources while it was running.
1591+
const loadNarrativeWalkthrough = useCallback((source: ReviewSource) => {
1592+
const sourceKey = getSourceKey(source);
1593+
setWalkthroughLoading(true);
1594+
setWalkthroughError(null);
1595+
window.codiff
1596+
.getNarrativeWalkthrough(source)
1597+
.then((result) => {
1598+
if (getSourceKey(stateRef.current?.source ?? source) !== sourceKey) {
1599+
return;
1600+
}
1601+
1602+
if (result.status === 'ready') {
1603+
setNarrativeWalkthrough(result.walkthrough);
1604+
if (sidebarModeRef.current === 'walkthrough') {
1605+
setSidebarMode('walkthrough');
1606+
} else {
1607+
setWalkthroughUnread(true);
1608+
}
1609+
} else {
1610+
setWalkthroughError(result);
1611+
}
1612+
})
1613+
.catch((error: unknown) => {
1614+
if (getSourceKey(stateRef.current?.source ?? source) !== sourceKey) {
1615+
return;
1616+
}
1617+
1618+
setWalkthroughError({
1619+
reason: error instanceof Error ? error.message : String(error),
1620+
status: 'unavailable',
1621+
});
1622+
})
1623+
.finally(() => {
1624+
if (getSourceKey(stateRef.current?.source ?? source) === sourceKey) {
1625+
setWalkthroughLoading(false);
1626+
}
1627+
});
1628+
}, []);
1629+
1630+
// Regenerate the walkthrough on demand, e.g. after an in-place refresh
1631+
// surfaced changes the current walkthrough doesn't narrate. The existing
1632+
// walkthrough stays visible until the new one arrives.
1633+
const regenerateWalkthrough = useCallback(() => {
1634+
const currentState = stateRef.current;
1635+
if (!currentState || currentState.files.length === 0 || walkthroughLoading) {
1636+
return;
1637+
}
1638+
loadNarrativeWalkthrough(currentState.source);
1639+
}, [loadNarrativeWalkthrough, walkthroughLoading]);
1640+
15171641
const changeSidebarMode = useCallback(
15181642
(mode: SidebarMode) => {
15191643
setMainMode('review');
@@ -1539,44 +1663,9 @@ export default function App() {
15391663
return;
15401664
}
15411665

1542-
const sourceKey = getSourceKey(state.source);
1543-
setWalkthroughLoading(true);
1544-
setWalkthroughError(null);
1545-
window.codiff
1546-
.getNarrativeWalkthrough(state.source)
1547-
.then((result) => {
1548-
if (getSourceKey(stateRef.current?.source ?? state.source) !== sourceKey) {
1549-
return;
1550-
}
1551-
1552-
if (result.status === 'ready') {
1553-
setNarrativeWalkthrough(result.walkthrough);
1554-
if (sidebarModeRef.current === 'walkthrough') {
1555-
setSidebarMode('walkthrough');
1556-
} else {
1557-
setWalkthroughUnread(true);
1558-
}
1559-
} else {
1560-
setWalkthroughError(result);
1561-
}
1562-
})
1563-
.catch((error: unknown) => {
1564-
if (getSourceKey(stateRef.current?.source ?? state.source) !== sourceKey) {
1565-
return;
1566-
}
1567-
1568-
setWalkthroughError({
1569-
reason: error instanceof Error ? error.message : String(error),
1570-
status: 'unavailable',
1571-
});
1572-
})
1573-
.finally(() => {
1574-
if (getSourceKey(stateRef.current?.source ?? state.source) === sourceKey) {
1575-
setWalkthroughLoading(false);
1576-
}
1577-
});
1666+
loadNarrativeWalkthrough(state.source);
15781667
},
1579-
[narrativeWalkthrough, state, walkthroughError, walkthroughLoading],
1668+
[loadNarrativeWalkthrough, narrativeWalkthrough, state, walkthroughError, walkthroughLoading],
15801669
);
15811670

15821671
const openCommitView = useCallback(() => {
@@ -1798,9 +1887,9 @@ export default function App() {
17981887
title: 'Open Config File',
17991888
}),
18001889
registry.register({
1801-
execute: reloadWindow,
1890+
execute: refreshRepository,
18021891
id: 'reload',
1803-
title: 'Reload Window',
1892+
title: 'Refresh Changes',
18041893
}),
18051894
];
18061895
setCommandBarCommands(registry.commands);
@@ -1816,7 +1905,7 @@ export default function App() {
18161905
getReviewCommandTarget,
18171906
openDiffSearch,
18181907
openSelectedFile,
1819-
reloadWindow,
1908+
refreshRepository,
18201909
setFileViewedState,
18211910
toggleSidebar,
18221911
toggleViewed,
@@ -2431,7 +2520,7 @@ export default function App() {
24312520
</div>
24322521
) : null}
24332522
<RepositoryChangeBanner
2434-
onReload={reloadWindow}
2523+
onRefresh={refreshRepository}
24352524
visible={localChangesDetected && (pendingSource ?? state.source).type === 'working-tree'}
24362525
/>
24372526
<WalkthroughOutdatedBanner
@@ -2546,12 +2635,15 @@ export default function App() {
25462635
/>
25472636
) : showNarrativeWalkthrough && narrativeWalkthrough ? (
25482637
<NarrativeWalkthroughView
2638+
changedPaths={reloadDeltaPaths}
25492639
files={state.files}
25502640
navigation={narrativeNavigation}
25512641
onActiveReviewTargetChange={updateActiveWalkthroughReviewTarget}
25522642
onCommit={commitWalkthrough}
2643+
onRegenerateWalkthrough={regenerateWalkthrough}
25532644
onShareWalkthrough={enabledShareWalkthrough}
25542645
onUpdateCommitMessage={updateWalkthroughCommitMessage}
2646+
regenerateDisabled={walkthroughLoading}
25552647
renderDiffBlocks={renderWalkthroughDiffBlocks}
25562648
shareWalkthroughDisabled={walkthroughSharing}
25572649
showWhitespace={showWhitespace}

core/__tests__/App-render.test.tsx

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ const createCodiffMock = (overrides: Partial<Window['codiff']> = {}): Window['co
180180
onFindInDiffs: vi.fn(() => () => {}),
181181
onMarkdownDocumentChanged: vi.fn(() => () => {}),
182182
onPlanCloseRequested: vi.fn(() => () => {}),
183+
onRefreshRequest: vi.fn(() => () => {}),
183184
onRepositoryChanged: vi.fn(() => () => {}),
184185
onWindowFullScreenChanged: vi.fn(() => () => {}),
185186
openConfigFile: vi.fn(async () => {}),
@@ -2479,6 +2480,115 @@ test('repository changes show the update banner without refreshing the working t
24792480
}
24802481
});
24812482

2483+
test('clicking the change banner refreshes the repository in place', async () => {
2484+
const initialFile = {
2485+
fingerprint: 'src/app.ts:1',
2486+
path: 'src/app.ts',
2487+
sections: [
2488+
{
2489+
binary: false,
2490+
id: 'src/app.ts:unstaged',
2491+
kind: 'unstaged',
2492+
patch: 'diff --git a/src/app.ts b/src/app.ts\n@@ -1 +1 @@\n-old\n+new\n',
2493+
},
2494+
],
2495+
status: 'modified',
2496+
} satisfies ChangedFile;
2497+
const addedFile = {
2498+
fingerprint: 'src/added.ts:1',
2499+
path: 'src/added.ts',
2500+
sections: [
2501+
{
2502+
binary: false,
2503+
id: 'src/added.ts:unstaged',
2504+
kind: 'unstaged',
2505+
patch: 'diff --git a/src/added.ts b/src/added.ts\n@@ -0,0 +1 @@\n+created\n',
2506+
},
2507+
],
2508+
status: 'added',
2509+
} satisfies ChangedFile;
2510+
2511+
let onRepositoryChanged: ((change: { root: string }) => void) | null = null;
2512+
let stateRequests = 0;
2513+
const getRepositoryState = vi.fn<Window['codiff']['getRepositoryState']>(async () => {
2514+
stateRequests += 1;
2515+
return stateRequests === 1
2516+
? { ...repositoryState, files: [initialFile] }
2517+
: {
2518+
...repositoryState,
2519+
files: [
2520+
addedFile,
2521+
{
2522+
...initialFile,
2523+
fingerprint: 'src/app.ts:2',
2524+
sections: [
2525+
{
2526+
binary: false,
2527+
id: 'src/app.ts:unstaged',
2528+
kind: 'unstaged',
2529+
patch: 'diff --git a/src/app.ts b/src/app.ts\n@@ -1 +1 @@\n-old\n+newer\n',
2530+
},
2531+
],
2532+
},
2533+
],
2534+
};
2535+
});
2536+
2537+
window.codiff = createCodiffMock({
2538+
getRepositoryState,
2539+
onRepositoryChanged: vi.fn((callback) => {
2540+
onRepositoryChanged = callback;
2541+
return () => {
2542+
onRepositoryChanged = null;
2543+
};
2544+
}),
2545+
});
2546+
2547+
const container = document.createElement('div');
2548+
document.body.append(container);
2549+
let root: Root | null = null;
2550+
2551+
try {
2552+
await act(async () => {
2553+
root = createRoot(container);
2554+
root.render(<App />);
2555+
});
2556+
2557+
await waitFor(() => {
2558+
expect(container.querySelector('.loading')).toBeNull();
2559+
expect(onRepositoryChanged).not.toBeNull();
2560+
});
2561+
expect(container.textContent).not.toContain('added.ts');
2562+
2563+
await act(async () => {
2564+
onRepositoryChanged?.({ root: '/repo' });
2565+
await new Promise((resolve) => setTimeout(resolve, 0));
2566+
});
2567+
2568+
const refreshButton = container.querySelector<HTMLButtonElement>('.repository-change-reload');
2569+
expect(refreshButton).not.toBeNull();
2570+
await act(async () => {
2571+
refreshButton?.click();
2572+
await new Promise((resolve) => setTimeout(resolve, 0));
2573+
});
2574+
2575+
// New file appears without a window reload, and the banner clears.
2576+
expect(getRepositoryState).toHaveBeenCalledTimes(2);
2577+
expect(container.textContent).toContain('added.ts');
2578+
expect(container.querySelector('.repository-change-banner.visible')).toBeNull();
2579+
2580+
// The changed file must not appear twice (old + new version).
2581+
const appOccurrences = container.textContent?.split('app.ts').length ?? 1;
2582+
expect(appOccurrences - 1).toBeLessThanOrEqual(2);
2583+
expect(container.textContent).not.toContain('-old\n+new\n');
2584+
} finally {
2585+
if (root) {
2586+
await act(async () => root?.unmount());
2587+
}
2588+
container.remove();
2589+
}
2590+
});
2591+
24822592
test('walkthrough launch errors stay on the walkthrough tab without automatic retries', async () => {
24832593
const changedFile = {
24842594
fingerprint: 'src/app.ts:1',

0 commit comments

Comments
 (0)