authorgravatar for admin@snappeh.comTristan Knight <admin@snappeh.com> 2026-09-16 22:44:51+01:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-16 18:44:51-03:00
logaae361c4ef6478e4dcf970d574f3fd38d908e6b9
tree13042789147074367f749dadc43eed60f738b482
parent32e8b2584556c0c55ce0d1d8f72b506cb771d60d
signature Signed by PGP key B5690EEEBB952194

fix(review): show complete counts and load large diffs progressively (#10822)

Co-authored-by: maria-rcks <maria@kuuro.net>

22 files changed, 1483 insertions(+), 389 deletions(-)

apps/mobile/src/features/review/ReviewSheet.tsx+39-20
...@@ -382,24 +382,40 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -382,24 +382,40 @@ export function ReviewSheet(props: ReviewSheetProps) {
382 useEffect(() => {382 useEffect(() => {
383 showAuxiliaryPane("inspector");383 showAuxiliaryPane("inspector");
384 }, [environmentId, showAuxiliaryPane, threadId]);384 }, [environmentId, showAuxiliaryPane, threadId]);
385 const { error, reviewSections, selectedSection, refreshSelectedSection, selectSection } =385 const {
386 useReviewSections({386 error,
387 enabled: isEnvironmentReady,387 reviewSections,
388 environmentId,388 selectedSection,
389 threadId,389 refreshSelectedSection,
390 reviewCache,390 selectSection,
391 });391 isSelectedSectionPending,
392 diffPreviewRevision,
393 } = useReviewSections({
394 enabled: isEnvironmentReady,
395 environmentId,
396 threadId,
397 reviewCache,
398 });
392 useReviewDiffPrewarming({399 useReviewDiffPrewarming({
393 threadKey: reviewCache.threadKey,400 threadKey: reviewCache.threadKey,
394 sections: reviewSections,401 sections: reviewSections,
395 selectedSectionId: selectedSection?.id ?? null,402 selectedSectionId: selectedSection?.id ?? null,
396 });403 });
397 const { headerDiffSummary, nativeReviewDiffData, parsedDiff, pendingReviewCommentCount } =404 const {
398 useReviewDiffData({405 headerDiffSummary,
399 threadKey: reviewCache.threadKey,406 nativeReviewDiffData,
400 selectedSection,407 parsedDiff,
401 draftMessage,408 pendingReviewCommentCount,
402 });409 loadVisibleFile,
410 isPending: areFilePatchesPending,
411 } = useReviewDiffData({
412 threadKey: reviewCache.threadKey,
413 environmentId,
414 cwd: selectedThreadCwd,
415 selectedSection,
416 revision: diffPreviewRevision,
417 draftMessage,
418 });
403 // Resolution returns null while Expo registers the native view (or forever419 // Resolution returns null while Expo registers the native view (or forever
404 // when the binary lacks it). Rendering a null component type crashes the420 // when the binary lacks it). Rendering a null component type crashes the
405 // app, so callers must fall back — ThreadFeed's ReviewCommentCard does the421 // app, so callers must fall back — ThreadFeed's ReviewCommentCard does the
...@@ -469,6 +485,7 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -469,6 +485,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
469485
470 const handleSelectFile = useCallback(486 const handleSelectFile = useCallback(
471 (fileId: string | null) => {487 (fileId: string | null) => {
488 loadVisibleFile(fileId, true);
472 commentSelection.clearSelection();489 commentSelection.clearSelection();
473 if (fileId !== null && collapsedFileIds.includes(fileId)) {490 if (fileId !== null && collapsedFileIds.includes(fileId)) {
474 toggleExpandedFile(fileId);491 toggleExpandedFile(fileId);
...@@ -481,13 +498,14 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -481,13 +498,14 @@ export function ReviewSheet(props: ReviewSheetProps) {
481 console.error("[review] Failed to navigate to diff file", error);498 console.error("[review] Failed to navigate to diff file", error);
482 });499 });
483 },500 },
484 [collapsedFileIds, commentSelection, toggleExpandedFile],501 [collapsedFileIds, commentSelection, toggleExpandedFile, loadVisibleFile],
485 );502 );
486 const handleVisibleFileChange = useCallback(503 const handleVisibleFileChange = useCallback(
487 (event: NativeSyntheticEvent<{ readonly fileId?: string | null }>) => {504 (event: NativeSyntheticEvent<{ readonly fileId?: string | null }>) => {
505 loadVisibleFile(event.nativeEvent.fileId ?? null);
488 reviewFileNavigatorRef.current?.setVisibleFile(event.nativeEvent.fileId ?? null);506 reviewFileNavigatorRef.current?.setVisibleFile(event.nativeEvent.fileId ?? null);
489 },507 },
490 [],508 [loadVisibleFile],
491 );509 );
492 const renderInspector = useCallback(510 const renderInspector = useCallback(
493 () => (511 () => (
...@@ -508,10 +526,11 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -508,10 +526,11 @@ export function ReviewSheet(props: ReviewSheetProps) {
508 (event: NativeSyntheticEvent<{ readonly fileId?: string }>) => {526 (event: NativeSyntheticEvent<{ readonly fileId?: string }>) => {
509 const { fileId } = event.nativeEvent;527 const { fileId } = event.nativeEvent;
510 if (fileId) {528 if (fileId) {
529 loadVisibleFile(fileId, true);
511 toggleExpandedFile(fileId);530 toggleExpandedFile(fileId);
512 }531 }
513 },532 },
514 [toggleExpandedFile],533 [toggleExpandedFile, loadVisibleFile],
515 );534 );
516535
517 const handleNativeToggleViewedFile = useCallback(536 const handleNativeToggleViewedFile = useCallback(
...@@ -574,12 +593,12 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -574,12 +593,12 @@ export function ReviewSheet(props: ReviewSheetProps) {
574 (event: { nativeEvent: { event: string } }) => {593 (event: { nativeEvent: { event: string } }) => {
575 const id = event.nativeEvent.event;594 const id = event.nativeEvent.event;
576 if (id === "refresh") {595 if (id === "refresh") {
577 void refreshSelectedSection();596 void handlePullToRefresh();
578 } else if (id.startsWith("section:")) {597 } else if (id.startsWith("section:")) {
579 selectSection(id.slice("section:".length));598 selectSection(id.slice("section:".length));
580 }599 }
581 },600 },
582 [refreshSelectedSection, selectSection],601 [handlePullToRefresh, selectSection],
583 );602 );
584 const handleRetryEnvironment = useCallback(() => {603 const handleRetryEnvironment = useCallback(() => {
585 void retryEnvironment(environmentId);604 void retryEnvironment(environmentId);
...@@ -818,7 +837,7 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -818,7 +837,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
818 <NativeReviewDiffView837 <NativeReviewDiffView
819 collapsable={false}838 collapsable={false}
820 testID="review-native-diff-view"839 testID="review-native-diff-view"
821 refreshing={isPullRefreshing}840 refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
822 onPullToRefresh={() => void handlePullToRefresh()}841 onPullToRefresh={() => void handlePullToRefresh()}
823 style={StyleSheet.absoluteFill}842 style={StyleSheet.absoluteFill}
824 appearanceScheme={selectedTheme}843 appearanceScheme={selectedTheme}
...@@ -862,7 +881,7 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -862,7 +881,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
862 // iOS has no other refresh affordance here (the explicit881 // iOS has no other refresh affordance here (the explicit
863 // "Refresh current diff" menu is Android-only).882 // "Refresh current diff" menu is Android-only).
864 <RefreshControl883 <RefreshControl
865 refreshing={isPullRefreshing}884 refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
866 onRefresh={() => void handlePullToRefresh()}885 onRefresh={() => void handlePullToRefresh()}
867 />886 />
868 }887 }
apps/mobile/src/features/review/nativeReviewDiffAdapter.test.ts+23
...@@ -98,6 +98,29 @@ function appTheme(themeId: MobileThemeId, appearance: MobileThemeAppearance) {...@@ -98,6 +98,29 @@ function appTheme(themeId: MobileThemeId, appearance: MobileThemeAppearance) {
98}98}
9999
100describe("getCachedNativeReviewDiffData", () => {100describe("getCachedNativeReviewDiffData", () => {
101 it.each([true, false])(
102 "preserves available diff rows before a notice (has excerpt: %s)",
103 (hasExcerpt) => {
104 if (parsedDiff.kind !== "files") throw new Error("Expected a parsed file diff");
105 const notice = "This file preview was truncated.";
106 const result = buildNativeReviewDiffData({
107 parsedDiff: {
108 ...parsedDiff,
109 files: parsedDiff.files.map((file) => ({
110 ...file,
111 rows: hasExcerpt ? file.rows : [],
112 notice,
113 })),
114 },
115 });
116 const original = buildNativeReviewDiffData({ parsedDiff });
117 expect(result.rows.slice(0, -1)).toEqual(
118 hasExcerpt ? original.rows : original.rows.filter((row) => row.kind === "file"),
119 );
120 expect(result.rows.at(-1)).toMatchObject({ kind: "notice", text: notice });
121 },
122 );
123
101 it("reuses the row model for equivalent empty comment arrays", () => {124 it("reuses the row model for equivalent empty comment arrays", () => {
102 const first = getCachedNativeReviewDiffData(buildInput([]));125 const first = getCachedNativeReviewDiffData(buildInput([]));
103 const second = getCachedNativeReviewDiffData(buildInput([]));126 const second = getCachedNativeReviewDiffData(buildInput([]));
apps/mobile/src/features/review/nativeReviewDiffAdapter.ts+23-9
...@@ -125,6 +125,8 @@ interface PreparedNativeReviewFileRows {...@@ -125,6 +125,8 @@ interface PreparedNativeReviewFileRows {
125 readonly filePath: string;125 readonly filePath: string;
126 readonly lineCount: number;126 readonly lineCount: number;
127 readonly rows: ReadonlyArray<NativeReviewDiffRow>;127 readonly rows: ReadonlyArray<NativeReviewDiffRow>;
128 readonly commentTargetsByRowId: ReadonlyMap<string, NativeReviewDiffCommentTarget>;
129 readonly rowIdByCommentLineId: ReadonlyMap<string, string>;
128 commentedRows: {130 commentedRows: {
129 readonly commentsKey: string;131 readonly commentsKey: string;
130 readonly rows: ReadonlyArray<NativeReviewDiffRow>;132 readonly rows: ReadonlyArray<NativeReviewDiffRow>;
...@@ -136,6 +138,7 @@ interface PreparedNativeReviewDiffData extends Omit<NativeReviewDiffData, "rows"...@@ -136,6 +138,7 @@ interface PreparedNativeReviewDiffData extends Omit<NativeReviewDiffData, "rows"
136}138}
137139
138const nativeReviewDiffDataCache = new WeakMap<ReviewParsedDiff, CachedNativeReviewDiffData>();140const nativeReviewDiffDataCache = new WeakMap<ReviewParsedDiff, CachedNativeReviewDiffData>();
141const nativeReviewFileRowsCache = new WeakMap<ReviewRenderableFile, PreparedNativeReviewFileRows>();
139142
140function buildReviewCommentsCacheKey(comments: ReadonlyArray<ReviewInlineComment>): string {143function buildReviewCommentsCacheKey(comments: ReadonlyArray<ReviewInlineComment>): string {
141 if (comments.length === 0) {144 if (comments.length === 0) {
...@@ -261,6 +264,7 @@ function createNoticeRow(fileId: string, suffix: string, text: string): NativeRe...@@ -261,6 +264,7 @@ function createNoticeRow(fileId: string, suffix: string, text: string): NativeRe
261}264}
262265
263function noticeRowsForFile(file: ReviewRenderableFile): ReadonlyArray<NativeReviewDiffRow> {266function noticeRowsForFile(file: ReviewRenderableFile): ReadonlyArray<NativeReviewDiffRow> {
267 if (file.notice) return [createNoticeRow(file.id, "loading", file.notice)];
264 if (file.rows.length > 0) {268 if (file.rows.length > 0) {
265 return [];269 return [];
266 }270 }
...@@ -406,11 +410,11 @@ function mapLineRow(...@@ -406,11 +410,11 @@ function mapLineRow(
406 };410 };
407}411}
408412
409function prepareFileRows(413function prepareFileRows(file: ReviewRenderableFile): PreparedNativeReviewFileRows {
410 file: ReviewRenderableFile,414 const cached = nativeReviewFileRowsCache.get(file);
411 commentTargetsByRowId: Map<string, NativeReviewDiffCommentTarget>,415 if (cached) return cached;
412 rowIdByCommentLineId: Map<string, string>,416 const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
413): PreparedNativeReviewFileRows {417 const rowIdByCommentLineId = new Map<string, string>();
414 const rows: NativeReviewDiffRow[] = [418 const rows: NativeReviewDiffRow[] = [
415 {419 {
416 kind: "file",420 kind: "file",
...@@ -449,14 +453,18 @@ function prepareFileRows(...@@ -449,14 +453,18 @@ function prepareFileRows(
449 });453 });
450454
451 rows.push(...noticeRowsForFile(file));455 rows.push(...noticeRowsForFile(file));
452 return {456 const prepared: PreparedNativeReviewFileRows = {
453 fileId: file.id,457 fileId: file.id,
454 filePath: file.path,458 filePath: file.path,
455 lineCount: lineRows.length,459 lineCount: lineRows.length,
456 // Comments must not split the source deletion/addition runs used for word matching.460 // Comments must not split the source deletion/addition runs used for word matching.
457 rows: addNativeWordDiffRanges(rows),461 rows: addNativeWordDiffRanges(rows),
462 commentTargetsByRowId,
463 rowIdByCommentLineId,
458 commentedRows: null,464 commentedRows: null,
459 };465 };
466 nativeReviewFileRowsCache.set(file, prepared);
467 return prepared;
460}468}
461469
462function insertFileComments(470function insertFileComments(
...@@ -526,9 +534,15 @@ function prepareNativeReviewDiffData(parsedDiff: ReviewParsedDiff): PreparedNati...@@ -526,9 +534,15 @@ function prepareNativeReviewDiffData(parsedDiff: ReviewParsedDiff): PreparedNati
526 }));534 }));
527 const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();535 const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
528 const rowIdByCommentLineId = new Map<string, string>();536 const rowIdByCommentLineId = new Map<string, string>();
529 const fileRows = parsedDiff.files.map((file) =>537 const fileRows = parsedDiff.files.map(prepareFileRows);
530 prepareFileRows(file, commentTargetsByRowId, rowIdByCommentLineId),538 for (const file of fileRows) {
531 );539 for (const [rowId, target] of file.commentTargetsByRowId) {
540 commentTargetsByRowId.set(rowId, target);
541 }
542 for (const [lineId, rowId] of file.rowIdByCommentLineId) {
543 rowIdByCommentLineId.set(lineId, rowId);
544 }
545 }
532546
533 return {547 return {
534 fileRows,548 fileRows,
apps/mobile/src/features/review/reviewModel.test.ts+61
...@@ -8,6 +8,7 @@ import {...@@ -8,6 +8,7 @@ import {
8} from "@t3tools/contracts";8} from "@t3tools/contracts";
99
10import {10import {
11 applyReviewDiffMetadata,
11 buildReviewParsedDiff,12 buildReviewParsedDiff,
12 buildReviewSectionItems,13 buildReviewSectionItems,
13 getDefaultReviewSectionId,14 getDefaultReviewSectionId,
...@@ -135,6 +136,35 @@ describe("buildReviewSectionItems", () => {...@@ -135,6 +136,35 @@ describe("buildReviewSectionItems", () => {
135});136});
136137
137describe("buildReviewParsedDiff", () => {138describe("buildReviewParsedDiff", () => {
139 it.each([
140 ["a/example.ts", "a/example.ts"],
141 ["b/example.ts", "b/example.ts"],
142 ["a/before.ts", "b/after.ts"],
143 ])("preserves repository paths from %s to %s", (previousPath, path) => {
144 const parsed = buildReviewParsedDiff(
145 [
146 `diff --git a/${previousPath} b/${path}`,
147 ...(previousPath === path
148 ? []
149 : ["similarity index 50%", `rename from ${previousPath}`, `rename to ${path}`]),
150 `--- a/${previousPath}`,
151 `+++ b/${path}`,
152 "@@ -1 +1 @@",
153 "-before",
154 "+after",
155 ].join("\n"),
156 "repository-paths",
157 );
158 expect(parsed.kind).toBe("files");
159 if (parsed.kind !== "files") return;
160 expect(parsed.files[0]).toMatchObject({
161 path,
162 previousPath: previousPath === path ? null : previousPath,
163 additions: 1,
164 deletions: 1,
165 });
166 });
167
138 it("builds renderable rows from a unified patch", () => {168 it("builds renderable rows from a unified patch", () => {
139 const parsed = buildReviewParsedDiff(169 const parsed = buildReviewParsedDiff(
140 [170 [
...@@ -271,3 +301,34 @@ describe("buildReviewParsedDiff", () => {...@@ -271,3 +301,34 @@ describe("buildReviewParsedDiff", () => {
271 });301 });
272 });302 });
273});303});
304
305describe("applyReviewDiffMetadata", () => {
306 it("uses complete counts even when the preview contains only part of one file", () => {
307 const parsed = buildReviewParsedDiff(
308 [
309 "diff --git a/large.txt b/large.txt",
310 "--- a/large.txt",
311 "+++ b/large.txt",
312 "@@ -1 +1 @@",
313 "-before",
314 "+after",
315 ].join("\n"),
316 "partial",
317 );
318 const result = applyReviewDiffMetadata(parsed, {
319 truncated: true,
320 files: [
321 { path: "large.txt", previousPath: null, additions: 4000, deletions: 3000 },
322 { path: "unseen.txt", previousPath: null, additions: 100, deletions: 20 },
323 ],
324 });
325 expect(result.kind).toBe("files");
326 if (result.kind !== "files") return;
327 expect(result.fileCount).toBe(2);
328 expect(result.additions).toBe(4100);
329 expect(result.deletions).toBe(3020);
330 expect(result.files[0]?.additions).toBe(4000);
331 expect(result.notice).toContain("Counts include all changes");
332 expect(applyReviewDiffMetadata(parsed, null)).toEqual(parsed);
333 });
334});
apps/mobile/src/features/review/reviewModel.ts+33-12
...@@ -18,6 +18,9 @@ export interface ReviewSectionItem {...@@ -18,6 +18,9 @@ export interface ReviewSectionItem {
18 readonly subtitle: string | null;18 readonly subtitle: string | null;
19 readonly diff: string | null;19 readonly diff: string | null;
20 readonly isLoading: boolean;20 readonly isLoading: boolean;
21 readonly files?: ReviewDiffPreviewSource["files"];
22 readonly truncated?: boolean;
23 readonly source?: ReviewDiffPreviewSource;
21}24}
2225
23export interface ReviewRenderableHunkRow {26export interface ReviewRenderableHunkRow {
...@@ -47,6 +50,7 @@ export type ReviewRenderableRow = ReviewRenderableHunkRow | ReviewRenderableLine...@@ -47,6 +50,7 @@ export type ReviewRenderableRow = ReviewRenderableHunkRow | ReviewRenderableLine
47export interface ReviewRenderableFile {50export interface ReviewRenderableFile {
48 readonly id: string;51 readonly id: string;
49 readonly cacheKey: string;52 readonly cacheKey: string;
53 readonly notice?: string;
50 readonly path: string;54 readonly path: string;
51 readonly previousPath: string | null;55 readonly previousPath: string | null;
52 readonly changeType: ChangeTypes;56 readonly changeType: ChangeTypes;
...@@ -126,16 +130,6 @@ function gitSubtitle(section: ReviewDiffPreviewSource): string | null {...@@ -126,16 +130,6 @@ function gitSubtitle(section: ReviewDiffPreviewSource): string | null {
126 return "Base branch unavailable";130 return "Base branch unavailable";
127}131}
128132
129function stripGitPrefix(pathValue: string | undefined): string | null {
130 if (!pathValue) {
131 return null;
132 }
133 if (pathValue.startsWith("a/") || pathValue.startsWith("b/")) {
134 return pathValue.slice(2);
135 }
136 return pathValue;
137}
138
139function stripTrailingNewline(value: string): string {133function stripTrailingNewline(value: string): string {
140 return value.endsWith("\n") ? value.slice(0, -1) : value;134 return value.endsWith("\n") ? value.slice(0, -1) : value;
141}135}
...@@ -378,8 +372,8 @@ function buildRenderableRows(file: FileDiffMetadata): ReadonlyArray<ReviewRender...@@ -378,8 +372,8 @@ function buildRenderableRows(file: FileDiffMetadata): ReadonlyArray<ReviewRender
378}372}
379373
380function mapRenderableFile(file: FileDiffMetadata): ReviewRenderableFile {374function mapRenderableFile(file: FileDiffMetadata): ReviewRenderableFile {
381 const path = stripGitPrefix(file.name) ?? stripGitPrefix(file.prevName) ?? file.name;375 const path = file.name || file.prevName || "";
382 const previousPath = stripGitPrefix(file.prevName);376 const previousPath = file.prevName || null;
383 const additions = file.hunks.reduce((total, hunk) => total + hunk.additionLines, 0);377 const additions = file.hunks.reduce((total, hunk) => total + hunk.additionLines, 0);
384 const deletions = file.hunks.reduce((total, hunk) => total + hunk.deletionLines, 0);378 const deletions = file.hunks.reduce((total, hunk) => total + hunk.deletionLines, 0);
385 const cacheKey = file.cacheKey ?? `${previousPath ?? "none"}:${path}:${file.type}`;379 const cacheKey = file.cacheKey ?? `${previousPath ?? "none"}:${path}:${file.type}`;
...@@ -442,6 +436,9 @@ export function buildReviewSectionItems(input: {...@@ -442,6 +436,9 @@ export function buildReviewSectionItems(input: {
442 title: section.title,436 title: section.title,
443 subtitle: gitSubtitle(section),437 subtitle: gitSubtitle(section),
444 diff: section.diff,438 diff: section.diff,
439 source: section,
440 ...(section.files ? { files: section.files } : {}),
441 truncated: section.truncated,
445 isLoading: false,442 isLoading: false,
446 }));443 }));
447 const hasDirtyWorktreeItem = gitItems.some((item) => item.id === DIRTY_WORKTREE_SECTION_ID);444 const hasDirtyWorktreeItem = gitItems.some((item) => item.id === DIRTY_WORKTREE_SECTION_ID);
...@@ -527,3 +524,27 @@ export function buildReviewParsedDiff(...@@ -527,3 +524,27 @@ export function buildReviewParsedDiff(
527 };524 };
528 }525 }
529}526}
527
528export function applyReviewDiffMetadata(
529 previewDiff: ReviewParsedDiff,
530 selectedSection: Pick<ReviewSectionItem, "files" | "truncated"> | null,
531): ReviewParsedDiff {
532 if (previewDiff.kind === "empty") return previewDiff;
533 const notice = selectedSection?.truncated
534 ? `This preview exceeds the size limit. Changes shown are incomplete.${selectedSection.files ? " Counts include all changes." : ""}`
535 : previewDiff.notice;
536 if (previewDiff.kind !== "files" || !selectedSection?.files) return { ...previewDiff, notice };
537 const totals = selectedSection.files.reduce(
538 (total, file) => ({
539 additions: total.additions + file.additions,
540 deletions: total.deletions + file.deletions,
541 }),
542 { additions: 0, deletions: 0 },
543 );
544 const stats = new Map(selectedSection.files.map((file) => [file.path, file]));
545 const files = previewDiff.files.map((file) => {
546 const stat = stats.get(file.path);
547 return stat ? { ...file, additions: stat.additions, deletions: stat.deletions } : file;
548 });
549 return { ...previewDiff, ...totals, files, fileCount: selectedSection.files.length, notice };
550}
apps/mobile/src/features/review/useReviewDiffData.ts+213-19
...@@ -1,12 +1,87 @@...@@ -1,12 +1,87 @@
1import { useEffect, useMemo } from "react";1import { useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
22
3import { countReviewCommentContexts, parseReviewInlineComments } from "./reviewCommentSelection";3import { countReviewCommentContexts, parseReviewInlineComments } from "./reviewCommentSelection";
4import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";4import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter";
5import { markReviewEvent, measureReviewWork } from "./reviewPerf";5import { markReviewEvent, measureReviewWork } from "./reviewPerf";
6import { getCachedReviewParsedDiff } from "./reviewState";6import { getCachedReviewParsedDiff } from "./reviewState";
7import type { ReviewParsedDiff, ReviewSectionItem } from "./reviewModel";7import {
8 applyReviewDiffMetadata,
9 buildReviewParsedDiff,
10 type ReviewParsedDiff,
11 type ReviewRenderableFile,
12 type ReviewSectionItem,
13} from "./reviewModel";
14
15import type {
16 EnvironmentId,
17 ReviewDiffFileStat,
18 ReviewDiffPreviewSource,
19} from "@t3tools/contracts";
20import { RegistryContext, useAtomValue } from "@effect/atom-react";
21import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
22import * as Atom from "effect/unstable/reactivity/Atom";
23import { reviewEnvironment } from "../../state/review";
824
9const EMPTY_INLINE_REVIEW_COMMENTS = Object.freeze([]);25const EMPTY_INLINE_REVIEW_COMMENTS = Object.freeze([]);
26type ParsedFilePatch = AsyncResult.AsyncResult<
27 { source: ReviewDiffPreviewSource; parsed: ReviewParsedDiff },
28 unknown
29>;
30const normalizedFiles = new WeakMap<
31 ParsedFilePatch,
32 { stat: ReviewDiffFileStat; diffHash: string; file: ReviewRenderableFile }
33>();
34
35function getCachedReviewFile(
36 stat: ReviewDiffFileStat,
37 diffHash: string,
38 patch: ParsedFilePatch | null | undefined,
39): ReviewRenderableFile {
40 const cached = patch && normalizedFiles.get(patch);
41 if (cached && cached.stat === stat && cached.diffHash === diffHash) return cached.file;
42 const parsed = patch?._tag === "Success" ? patch.value.parsed : null;
43 const sourceFiles = patch?._tag === "Success" ? patch.value.source.files : undefined;
44 const loaded =
45 parsed?.kind === "files"
46 ? (parsed.files.find((file) => file.path === stat.path) ??
47 (parsed.files.length === 1 &&
48 sourceFiles?.length === 1 &&
49 sourceFiles[0]?.path === stat.path
50 ? parsed.files[0]
51 : undefined))
52 : undefined;
53 const file: ReviewRenderableFile = {
54 ...(loaded ?? {
55 path: stat.path,
56 previousPath: stat.previousPath,
57 changeType: "change" as const,
58 languageHint: null,
59 additionLines: [],
60 deletionLines: [],
61 rows: [],
62 cacheKey: `${diffHash}:${stat.path}`,
63 }),
64 id: stat.path,
65 path: stat.path,
66 previousPath: stat.previousPath,
67 additions: stat.additions,
68 deletions: stat.deletions,
69 ...(patch?._tag === "Success"
70 ? patch.value.source.truncated
71 ? { notice: "File preview exceeds the size limit. Counts include all changes." }
72 : loaded
73 ? {}
74 : { notice: "Could not display file preview." }
75 : {
76 notice:
77 patch?._tag === "Failure"
78 ? "Could not load diff. Select the file to retry."
79 : "Loading diff…",
80 }),
81 };
82 if (patch) normalizedFiles.set(patch, { stat, diffHash, file });
83 return file;
84}
1085
11function isReviewDiffDebugLoggingEnabled(): boolean {86function isReviewDiffDebugLoggingEnabled(): boolean {
12 return typeof __DEV__ !== "undefined" ? __DEV__ : false;87 return typeof __DEV__ !== "undefined" ? __DEV__ : false;
...@@ -25,39 +100,156 @@ function logReviewDiffDiagnostic(message: string, details?: Record<string, unkno...@@ -25,39 +100,156 @@ function logReviewDiffDiagnostic(message: string, details?: Record<string, unkno
25 console.log(`[review-sheet] ${message}`);100 console.log(`[review-sheet] ${message}`);
26}101}
27102
28export function formatHeaderDiffSummary(parsedDiff: ReviewParsedDiff): {103export function formatHeaderDiffSummary(
104 parsedDiff: ReviewParsedDiff,
105 files?: ReviewSectionItem["files"],
106): {
29 readonly additions: string | null;107 readonly additions: string | null;
30 readonly deletions: string | null;108 readonly deletions: string | null;
31} {109} {
32 if (parsedDiff.kind !== "files") {110 if (files) {
33 return { additions: null, deletions: null };111 return {
112 additions: `+${files.reduce((total, file) => total + file.additions, 0)}`,
113 deletions: `-${files.reduce((total, file) => total + file.deletions, 0)}`,
114 };
34 }115 }
35116 if (parsedDiff.kind !== "files") return { additions: null, deletions: null };
36 return {117 return { additions: `+${parsedDiff.additions}`, deletions: `-${parsedDiff.deletions}` };
37 additions: `+${parsedDiff.additions}`,
38 deletions: `-${parsedDiff.deletions}`,
39 };
40}118}
41119
42export function useReviewDiffData(input: {120export function useReviewDiffData(input: {
43 readonly threadKey: string | null;121 readonly threadKey: string | null;
122 readonly environmentId: EnvironmentId | undefined;
123 readonly cwd: string | null;
44 readonly selectedSection: ReviewSectionItem | null;124 readonly selectedSection: ReviewSectionItem | null;
125 readonly revision: string | undefined;
45 readonly draftMessage: string;126 readonly draftMessage: string;
46}) {127}) {
47 const { draftMessage, selectedSection, threadKey } = input;128 const { draftMessage, selectedSection, threadKey } = input;
48 const selectedSectionId = selectedSection?.id ?? null;129 const selectedSectionId = selectedSection?.id ?? null;
49 const parsedDiff = useMemo(130 const source = selectedSection?.source;
131 const lazySource = source?.truncated && source.files ? source : null;
132 const previewDiff = useMemo<ReviewParsedDiff>(
50 () =>133 () =>
51 measureReviewWork("parse-diff", () =>134 lazySource
52 getCachedReviewParsedDiff({135 ? { kind: "empty" }
53 threadKey,136 : measureReviewWork("parse-diff", () =>
54 sectionId: selectedSection?.id ?? null,137 getCachedReviewParsedDiff({
55 diff: selectedSection?.diff,138 threadKey,
56 }),139 sectionId: selectedSection?.id ?? null,
140 diff: selectedSection?.diff,
141 }),
142 ),
143 [lazySource, selectedSection?.diff, selectedSection?.id, threadKey],
144 );
145 const registry = useContext(RegistryContext);
146 const { environmentId, cwd } = input;
147 const scope = JSON.stringify([
148 environmentId,
149 cwd,
150 source?.kind,
151 source?.baseRef,
152 source?.diffHash,
153 ]);
154 const [requested, setRequested] = useState({ scope, indices: [0, 1, 2] });
155 const indices = useMemo(
156 () => new Set(requested.scope === scope ? requested.indices : [0, 1, 2]),
157 [requested, scope],
158 );
159 const queries = useMemo(
160 () =>
161 !environmentId || !cwd || !lazySource
162 ? []
163 : (lazySource.files ?? []).map((file, index) =>
164 indices.has(index)
165 ? reviewEnvironment.diffFilePatch({
166 environmentId,
167 input: {
168 cacheKey: scope,
169 request: {
170 cwd,
171 ...(lazySource.baseRef ? { baseRef: lazySource.baseRef } : {}),
172 file: {
173 path: file.path,
174 previousPath: file.previousPath,
175 sourceKind: lazySource.kind,
176 },
177 },
178 },
179 })
180 : null,
181 ),
182 [environmentId, cwd, lazySource, indices, scope],
183 );
184 const parsedQuery = useMemo(
185 () =>
186 Atom.family((query: ReturnType<typeof reviewEnvironment.diffFilePatch>) =>
187 Atom.map(query, (result) =>
188 AsyncResult.map(result, (source) => ({
189 source,
190 parsed: buildReviewParsedDiff(source.diff, source.diffHash),
191 })),
192 ),
57 ),193 ),
58 [selectedSection?.diff, selectedSection?.id, threadKey],194 [],
195 );
196 const patches = useAtomValue(
197 useMemo(
198 () => Atom.make((get) => queries.map((query) => (query ? get(parsedQuery(query)) : null))),
199 [queries, parsedQuery],
200 ),
201 );
202 const previousPreview = useRef({
203 scope,
204 revision: input.revision,
205 queries: [] as typeof queries,
206 });
207 useEffect(() => {
208 const previous = previousPreview.current;
209 previousPreview.current = { scope, revision: input.revision, queries };
210 for (const query of queries) {
211 if (!query) continue;
212 const changed = previous.scope === scope && previous.revision !== input.revision;
213 const cached =
214 !previous.queries.includes(query) &&
215 registry.get(query)._tag !== "Initial" &&
216 !registry.get(query).waiting;
217 if (changed || cached) registry.refresh(query);
218 }
219 }, [input.revision, queries, registry, scope]);
220 const loadVisibleFile = useCallback(
221 (fileId: string | null, retry = false) => {
222 const index =
223 fileId === null ? 0 : (lazySource?.files?.findIndex((file) => file.path === fileId) ?? -1);
224 if (index < 0) return;
225 setRequested((current) => {
226 const previous = current.scope === scope ? current.indices : [0, 1, 2];
227 const added = [index, index + 1, index + 2].filter((next) => !previous.includes(next));
228 return added.length === 0 ? current : { scope, indices: [...previous, ...added] };
229 });
230 if (retry && patches[index]?._tag === "Failure" && queries[index])
231 registry.refresh(queries[index]);
232 },
233 [lazySource, scope, patches, queries, registry],
234 );
235 const parsedDiff = useMemo<ReviewParsedDiff>(() => {
236 if (!lazySource?.files) return applyReviewDiffMetadata(previewDiff, selectedSection);
237 const files = lazySource.files.map((stat, index) =>
238 getCachedReviewFile(stat, lazySource.diffHash, patches[index]),
239 );
240 return {
241 kind: "files",
242 files,
243 fileCount: files.length,
244 additions: files.reduce((total, file) => total + file.additions, 0),
245 deletions: files.reduce((total, file) => total + file.deletions, 0),
246 notice: null,
247 };
248 }, [lazySource, previewDiff, selectedSection, patches]);
249 const headerDiffSummary = useMemo(
250 () => formatHeaderDiffSummary(parsedDiff, selectedSection?.files),
251 [parsedDiff, selectedSection?.files],
59 );252 );
60 const headerDiffSummary = useMemo(() => formatHeaderDiffSummary(parsedDiff), [parsedDiff]);
61 const inlineReviewComments = useMemo(253 const inlineReviewComments = useMemo(
62 () => parseReviewInlineComments(draftMessage),254 () => parseReviewInlineComments(draftMessage),
63 [draftMessage],255 [draftMessage],
...@@ -104,6 +296,8 @@ export function useReviewDiffData(input: {...@@ -104,6 +296,8 @@ export function useReviewDiffData(input: {
104296
105 return {297 return {
106 parsedDiff,298 parsedDiff,
299 loadVisibleFile,
300 isPending: patches.some((patch) => patch?._tag === "Initial" || patch?.waiting),
107 headerDiffSummary,301 headerDiffSummary,
108 nativeReviewDiffData,302 nativeReviewDiffData,
109 pendingReviewCommentCount,303 pendingReviewCommentCount,
apps/mobile/src/features/review/useReviewSections.ts+8-1
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import { useCallback, useEffect, useMemo } from "react";1import { useCallback, useEffect, useMemo } from "react";
2import * as DateTime from "effect/DateTime";
23
3import type { EnvironmentId, OrchestrationCheckpointSummary, ThreadId } from "@t3tools/contracts";4import type { EnvironmentId, OrchestrationCheckpointSummary, ThreadId } from "@t3tools/contracts";
45
...@@ -66,13 +67,14 @@ export function useReviewSections(input: {...@@ -66,13 +67,14 @@ export function useReviewSections(input: {
66 () =>67 () =>
67 buildReviewSectionItems({68 buildReviewSectionItems({
68 checkpoints: readyCheckpoints,69 checkpoints: readyCheckpoints,
69 gitSections: reviewCache.gitSections,70 gitSections: diffPreview.data?.sources ?? reviewCache.gitSections,
70 turnDiffById: reviewCache.turnDiffById,71 turnDiffById: reviewCache.turnDiffById,
71 loadingTurnIds,72 loadingTurnIds,
72 loadingGitSections: diffPreview.isPending,73 loadingGitSections: diffPreview.isPending,
73 }),74 }),
74 [75 [
75 diffPreview.isPending,76 diffPreview.isPending,
77 diffPreview.data?.sources,
76 loadingTurnIds,78 loadingTurnIds,
77 readyCheckpoints,79 readyCheckpoints,
78 reviewCache.gitSections,80 reviewCache.gitSections,
...@@ -173,7 +175,12 @@ export function useReviewSections(input: {...@@ -173,7 +175,12 @@ export function useReviewSections(input: {
173175
174 return {176 return {
175 error: diffPreview.error ?? activeTurnDiff.error ?? reviewCache.asyncState.error,177 error: diffPreview.error ?? activeTurnDiff.error ?? reviewCache.asyncState.error,
178 isSelectedSectionPending:
179 selectedSection?.kind === "turn" ? activeTurnDiff.isPending : diffPreview.isPending,
176 loadingGitDiffs: diffPreview.isPending,180 loadingGitDiffs: diffPreview.isPending,
181 diffPreviewRevision: diffPreview.data
182 ? DateTime.formatIso(diffPreview.data.generatedAt)
183 : undefined,
177 loadingTurnIds,184 loadingTurnIds,
178 reviewSections,185 reviewSections,
179 selectedSection,186 selectedSection,
apps/server/src/vcs/GitVcsDriverCore.test.ts+288-1
...@@ -15,12 +15,17 @@ import * as Queue from "effect/Queue";...@@ -15,12 +15,17 @@ import * as Queue from "effect/Queue";
15import * as Ref from "effect/Ref";15import * as Ref from "effect/Ref";
16import * as Result from "effect/Result";16import * as Result from "effect/Result";
17import * as Scope from "effect/Scope";17import * as Scope from "effect/Scope";
18import * as Schema from "effect/Schema";
18import * as Sink from "effect/Sink";19import * as Sink from "effect/Sink";
19import * as Stream from "effect/Stream";20import * as Stream from "effect/Stream";
20import * as TestClock from "effect/testing/TestClock";21import * as TestClock from "effect/testing/TestClock";
21import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";22import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
2223
23import { GitCommandError, type ReviewDiffFileContentsInput } from "@t3tools/contracts";24import {
25 GitCommandError,
26 ReviewDiffPreviewInput,
27 type ReviewDiffFileContentsInput,
28} from "@t3tools/contracts";
24import { ServerConfig } from "../config.ts";29import { ServerConfig } from "../config.ts";
25import { gitCommandDuration } from "../observability/Metrics.ts";30import { gitCommandDuration } from "../observability/Metrics.ts";
26import {31import {
...@@ -958,6 +963,119 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -958,6 +963,119 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
958 });963 });
959964
960 describe("review diff previews", () => {965 describe("review diff previews", () => {
966 it.effect("loads repository-relative files from a nested project directory", () =>
967 Effect.gen(function* () {
968 const cwd = yield* makeTmpDir();
969 const { initialBranch } = yield* initRepoWithCommit(cwd);
970 yield* git(cwd, ["checkout", "-b", "feature/nested"]);
971 yield* writeTextFile(cwd, "nested/tracked.txt", "committed\n");
972 yield* git(cwd, ["add", "."]);
973 yield* git(cwd, ["commit", "-m", "nested file"]);
974 yield* writeTextFile(cwd, "nested/tracked.txt", "changed\n");
975 yield* writeTextFile(cwd, "untracked.txt", "new\n");
976 const path = yield* Path.Path;
977 const driver = yield* GitVcsDriver.GitVcsDriver;
978 const nestedCwd = path.join(cwd, "nested");
979 const preview = yield* driver.getReviewDiffPreview({
980 cwd: nestedCwd,
981 baseRef: initialBranch,
982 });
983 assert.equal(
984 preview.sources.find((source) => source.kind === "working-tree")!.files!.length,
985 2,
986 );
987 for (const source of preview.sources) {
988 for (const file of source.files ?? []) {
989 const scoped = yield* driver.getReviewDiffPreview({
990 cwd: nestedCwd,
991 baseRef: initialBranch,
992 file: { path: file.path, previousPath: file.previousPath, sourceKind: source.kind },
993 });
994 const patch = scoped.sources.find((item) => item.kind === source.kind)!;
995 assert.deepStrictEqual(patch.files, [file]);
996 assert.include(patch.diff, `b/${file.path}`);
997 }
998 }
999 }),
1000 );
1001
1002 it.effect("reads complete tracked and untracked manifests beyond 1 MB", () =>
1003 Effect.gen(function* () {
1004 const cwd = yield* makeTmpDir();
1005 const { initialBranch } = yield* initRepoWithCommit(cwd);
1006 yield* writeTextFile(cwd, "untracked.txt", "untracked content\n");
1007 const paths = Array.from({ length: 5000 }, (_, index) => `${"a".repeat(220)}-${index}.txt`);
1008 const stats = paths.map((path) => `1\t0\t${path}\0`).join("");
1009 const untracked = [...paths, "untracked.txt"].join("\0") + "\0";
1010 assert.isAbove(stats.length, 1024 * 1024);
1011 assert.isAbove(untracked.length, 1024 * 1024);
1012 let readLargeUntracked = false;
1013 const delegate = yield* ChildProcessSpawner.ChildProcessSpawner;
1014 const spawner = ChildProcessSpawner.make((command) => {
1015 if (ChildProcess.isStandardCommand(command)) {
1016 if (
1017 command.args.includes("--numstat") &&
1018 command.args.includes(`${initialBranch}...HEAD`)
1019 ) {
1020 return Effect.succeed(makeSuccessfulHandle(stats));
1021 }
1022 if (command.args.includes("ls-files") && command.args.includes("--others")) {
1023 return Effect.succeed(makeSuccessfulHandle(readLargeUntracked ? untracked : ""));
1024 }
1025 }
1026 return delegate.spawn(command);
1027 });
1028 const driver = yield* makeGitVcsDriverCore().pipe(
1029 Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
1030 Effect.provide(ServerConfigLayer),
1031 );
1032 const branch = yield* driver.getReviewDiffPreview({
1033 cwd,
1034 baseRef: initialBranch,
1035 });
1036 const files = branch.sources.find((source) => source.kind === "branch-range")!.files!;
1037 assert.equal(files.length, paths.length);
1038 assert.equal(files.at(-1)?.path, paths.at(-1));
1039 assert.equal(
1040 files.reduce((total, file) => total + file.additions, 0),
1041 paths.length,
1042 );
1043 readLargeUntracked = true;
1044 const dirty = yield* driver.getReviewDiffPreview({
1045 cwd,
1046 file: { path: "untracked.txt", previousPath: null, sourceKind: "working-tree" },
1047 });
1048 const source = dirty.sources.find((source) => source.kind === "working-tree")!;
1049 assert.deepStrictEqual(source.files, [
1050 { path: "untracked.txt", previousPath: null, additions: 1, deletions: 0 },
1051 ]);
1052 assert.include(source.diff, "+untracked content");
1053 }),
1054 );
1055
1056 it.effect("propagates patch failures instead of reporting an empty complete diff", () =>
1057 Effect.gen(function* () {
1058 const cwd = yield* makeTmpDir();
1059 yield* initRepoWithCommit(cwd);
1060 yield* writeTextFile(cwd, "README.md", "changed\n");
1061 const delegate = yield* ChildProcessSpawner.ChildProcessSpawner;
1062 const spawner = ChildProcessSpawner.make((command) =>
1063 ChildProcess.isStandardCommand(command) && command.args.includes("--patch")
1064 ? Effect.succeed(makeNonRepositoryHandle())
1065 : delegate.spawn(command),
1066 );
1067 const driver = yield* makeGitVcsDriverCore().pipe(
1068 Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner),
1069 Effect.provide(ServerConfigLayer),
1070 );
1071 const result = yield* driver.getReviewDiffPreview({ cwd }).pipe(Effect.result);
1072 assert.isTrue(Result.isFailure(result));
1073 if (Result.isFailure(result)) {
1074 assert.equal(result.failure.operation, "GitVcsDriver.getReviewDiffPreview.patch");
1075 }
1076 }),
1077 );
1078
961 it.effect("drops an unterminated path from truncated NUL-separated git output", () =>1079 it.effect("drops an unterminated path from truncated NUL-separated git output", () =>
962 Effect.sync(() => {1080 Effect.sync(() => {
963 const paths = splitNullSeparatedGitStdoutPaths({1081 const paths = splitNullSeparatedGitStdoutPaths({
...@@ -1008,6 +1126,14 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1008,6 +1126,14 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
1008 ignored.sources.find((source) => source.kind === "working-tree")?.diff,1126 ignored.sources.find((source) => source.kind === "working-tree")?.diff,
1009 "",1127 "",
1010 );1128 );
1129 assert.deepStrictEqual(
1130 ignored.sources.find((source) => source.kind === "working-tree")?.files,
1131 [],
1132 );
1133 assert.deepStrictEqual(
1134 ignored.sources.find((source) => source.kind === "branch-range")?.files,
1135 [],
1136 );
1011 assert.strictEqual(1137 assert.strictEqual(
1012 ignored.sources.find((source) => source.kind === "branch-range")?.diff,1138 ignored.sources.find((source) => source.kind === "branch-range")?.diff,
1013 "",1139 "",
...@@ -1057,6 +1183,20 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1057,6 +1183,20 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
10571183
1058 assert.include(diff, "+literal pathspec contents");1184 assert.include(diff, "+literal pathspec contents");
1059 assert.include(diff, "+ordinary contents");1185 assert.include(diff, "+ordinary contents");
1186 const scoped = yield* driver.getReviewDiffPreview({
1187 cwd,
1188 file: {
1189 path: ":(exclude)after.ts",
1190 previousPath: null,
1191 sourceKind: "working-tree",
1192 },
1193 });
1194 const scopedSource = scoped.sources.find((source) => source.kind === "working-tree")!;
1195 assert.deepStrictEqual(scopedSource.files, [
1196 { path: ":(exclude)after.ts", previousPath: null, additions: 1, deletions: 0 },
1197 ]);
1198 assert.include(scopedSource.diff, "+literal pathspec contents");
1199 assert.notInclude(scopedSource.diff, "ordinary.ts");
1060 assert.strictEqual(yield* git(cwd, ["ls-files", "--stage"]), indexBefore);1200 assert.strictEqual(yield* git(cwd, ["ls-files", "--stage"]), indexBefore);
1061 }),1201 }),
1062 );1202 );
...@@ -1096,6 +1236,17 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1096,6 +1236,17 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
1096 .filter((entry) => entry.startsWith("sharedindex."))1236 .filter((entry) => entry.startsWith("sharedindex."))
1097 .sort();1237 .sort();
10981238
1239 const source = preview.sources.find((candidate) => candidate.kind === "working-tree")!;
1240 assert.deepStrictEqual(source.files, [
1241 { path: "after.ts", previousPath: "before.ts", additions: 1, deletions: 1 },
1242 ]);
1243 const scoped = yield* driver.getReviewDiffPreview({
1244 cwd,
1245 file: { path: "after.ts", previousPath: "before.ts", sourceKind: "working-tree" },
1246 });
1247 const scopedSource = scoped.sources.find((candidate) => candidate.kind === "working-tree")!;
1248 assert.deepStrictEqual(scopedSource.files, source.files);
1249 assert.equal(scopedSource.diff, diff);
1099 assert.include(diff, "rename from before.ts");1250 assert.include(diff, "rename from before.ts");
1100 assert.include(diff, "rename to after.ts");1251 assert.include(diff, "rename to after.ts");
1101 assert.include(diff, "-three");1252 assert.include(diff, "-three");
...@@ -1169,6 +1320,142 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {...@@ -1169,6 +1320,142 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
1169 }),1320 }),
1170 );1321 );
11711322
1323 it.effect("keeps complete stats for files beyond the combined patch limit", () =>
1324 Effect.gen(function* () {
1325 const cwd = yield* makeTmpDir();
1326 const { initialBranch } = yield* initRepoWithCommit(cwd);
1327 const driver = yield* GitVcsDriver.GitVcsDriver;
1328 const largeContents = "a long line of changed content for the diff preview\n".repeat(4000);
1329 yield* git(cwd, ["checkout", "-b", "feature/large"]);
1330 yield* writeTextFile(cwd, "a-large.txt", largeContents);
1331 yield* writeTextFile(cwd, "z-last.txt", "last file\n");
1332 yield* git(cwd, ["add", "."]);
1333 yield* git(cwd, ["commit", "-m", "large change"]);
1334 yield* writeTextFile(cwd, "a-large.txt", largeContents.replaceAll("changed", "updated"));
1335 yield* writeTextFile(cwd, "z-last.txt", "last file updated\n");
1336 yield* writeTextFile(cwd, "untracked.txt", largeContents);
1337
1338 const preview = yield* driver.getReviewDiffPreview({ cwd, baseRef: initialBranch });
1339 const branch = preview.sources.find((source) => source.kind === "branch-range")!;
1340 const dirty = preview.sources.find((source) => source.kind === "working-tree")!;
1341 assert.isTrue(branch.truncated);
1342 assert.isTrue(dirty.truncated);
1343 assert.notInclude(branch.diff, "z-last.txt");
1344 for (const source of [branch, dirty]) {
1345 for (const file of source.files ?? []) {
1346 const individual = yield* driver.getReviewDiffPreview({
1347 cwd,
1348 baseRef: initialBranch,
1349 file: { path: file.path, previousPath: file.previousPath, sourceKind: source.kind },
1350 });
1351 const patch = individual.sources.find((candidate) => candidate.kind === source.kind)!;
1352 assert.isFalse(patch.truncated);
1353 assert.deepStrictEqual(patch.files, [file]);
1354 assert.include(patch.diff, `b/${file.path}`);
1355 assert.isEmpty(
1356 individual.sources.find((candidate) => candidate.kind !== source.kind)!.diff,
1357 );
1358 }
1359 }
1360
1361 assert.deepStrictEqual(branch.files, [
1362 { path: "a-large.txt", previousPath: null, additions: 4000, deletions: 0 },
1363 { path: "z-last.txt", previousPath: null, additions: 1, deletions: 0 },
1364 ]);
1365 assert.deepStrictEqual(dirty.files, [
1366 { path: "a-large.txt", previousPath: null, additions: 4000, deletions: 4000 },
1367 { path: "untracked.txt", previousPath: null, additions: 4000, deletions: 0 },
1368 { path: "z-last.txt", previousPath: null, additions: 1, deletions: 1 },
1369 ]);
1370 }),
1371 );
1372
1373 it.effect("preserves renames, unusual paths, modes, and binary statistics", () =>
1374 Effect.gen(function* () {
1375 const cwd = yield* makeTmpDir();
1376 const { initialBranch } = yield* initRepoWithCommit(cwd);
1377 const driver = yield* GitVcsDriver.GitVcsDriver;
1378 yield* writeTextFile(cwd, "mode-only.sh", "echo unchanged\n");
1379 yield* git(cwd, ["add", "mode-only.sh"]);
1380 yield* git(cwd, ["commit", "-m", "add executable candidate"]);
1381 yield* git(cwd, ["checkout", "-b", "feature/paths"]);
1382 yield* git(cwd, ["mv", "README.md", "renamed.md"]);
1383 yield* writeTextFile(cwd, "[literal].txt", "literal\n");
1384 yield* writeTextFile(cwd, " leading.txt", "whitespace path\n");
1385 yield* writeTextFile(cwd, "l.txt", "other\n");
1386 yield* writeTextFile(cwd, "binary.dat", "binary\0data");
1387 if ((yield* HostProcessPlatform) !== "win32") {
1388 yield* writeTextFile(cwd, "tab\tand\nnewline.txt", "unusual path\n");
1389 }
1390 yield* git(cwd, ["add", "."]);
1391 yield* git(cwd, ["update-index", "--chmod=+x", "mode-only.sh"]);
1392 yield* git(cwd, ["commit", "-m", "rename and add files"]);
1393 const preview = yield* driver.getReviewDiffPreview({
1394 cwd,
1395 baseRef: initialBranch,
1396 });
1397 const branch = preview.sources.find((source) => source.kind === "branch-range")!;
1398 for (const path of ["renamed.md", "[literal].txt", " leading.txt", "mode-only.sh"]) {
1399 const stat = branch.files!.find((file) => file.path === path)!;
1400 const request = yield* Schema.decodeEffect(ReviewDiffPreviewInput)({
1401 cwd,
1402 baseRef: initialBranch,
1403 file: { path, previousPath: stat.previousPath, sourceKind: "branch-range" },
1404 });
1405 const result = yield* driver.getReviewDiffPreview(request);
1406 const scoped = result.sources.find((source) => source.kind === "branch-range")!;
1407 assert.deepStrictEqual(scoped.files, [stat]);
1408 assert.notInclude(scoped.diff, "b/l.txt");
1409 if (path === "renamed.md") assert.include(scoped.diff, "rename from README.md");
1410 if (path === "mode-only.sh") {
1411 assert.include(scoped.diff, "old mode 100644");
1412 assert.include(scoped.diff, "new mode 100755");
1413 }
1414 }
1415 assert.include(branch.diff, "rename from README.md");
1416 assert.include(branch.diff, "rename to renamed.md");
1417 assert.deepInclude(branch.files ?? [], {
1418 path: "renamed.md",
1419 previousPath: "README.md",
1420 additions: 0,
1421 deletions: 0,
1422 });
1423 assert.deepInclude(branch.files ?? [], {
1424 path: "binary.dat",
1425 previousPath: null,
1426 additions: 0,
1427 deletions: 0,
1428 });
1429 if ((yield* HostProcessPlatform) !== "win32") {
1430 assert.deepInclude(branch.files ?? [], {
1431 path: "tab\tand\nnewline.txt",
1432 previousPath: null,
1433 additions: 1,
1434 deletions: 0,
1435 });
1436 }
1437 }),
1438 );
1439
1440 it.effect("reports staged and untracked changes before the first commit", () =>
1441 Effect.gen(function* () {
1442 const cwd = yield* makeTmpDir();
1443 yield* git(cwd, ["init"]);
1444 yield* writeTextFile(cwd, "staged.txt", "staged\n");
1445 yield* git(cwd, ["add", "staged.txt"]);
1446 yield* writeTextFile(cwd, "untracked.txt", "untracked\n");
1447 const driver = yield* GitVcsDriver.GitVcsDriver;
1448 const preview = yield* driver.getReviewDiffPreview({ cwd });
1449 const dirty = preview.sources.find((source) => source.kind === "working-tree")!;
1450 assert.deepStrictEqual(dirty.files, [
1451 { path: "staged.txt", previousPath: null, additions: 1, deletions: 0 },
1452 { path: "untracked.txt", previousPath: null, additions: 1, deletions: 0 },
1453 ]);
1454 assert.include(dirty.diff, "b/staged.txt");
1455 assert.include(dirty.diff, "b/untracked.txt");
1456 }),
1457 );
1458
1172 it.effect("loads full file contents for working-tree diff expansion", () =>1459 it.effect("loads full file contents for working-tree diff expansion", () =>
1173 Effect.gen(function* () {1460 Effect.gen(function* () {
1174 const cwd = yield* makeTmpDir();1461 const cwd = yield* makeTmpDir();
apps/server/src/vcs/GitVcsDriverCore.ts+176-223
...@@ -1,4 +1,3 @@...@@ -1,4 +1,3 @@
1import * as Arr from "effect/Array";
2import * as Cache from "effect/Cache";1import * as Cache from "effect/Cache";
3import * as Data from "effect/Data";2import * as Data from "effect/Data";
4import * as Crypto from "effect/Crypto";3import * as Crypto from "effect/Crypto";
...@@ -23,10 +22,12 @@ import {...@@ -23,10 +22,12 @@ import {
23 GitCommandError,22 GitCommandError,
24 type ReviewDiffFileContentsInput,23 type ReviewDiffFileContentsInput,
25 type ReviewDiffPreviewInput,24 type ReviewDiffPreviewInput,
25 type ReviewDiffFileStat,
26 type ReviewDiffPreviewSource,26 type ReviewDiffPreviewSource,
27 type VcsRef,27 type VcsRef,
28} from "@t3tools/contracts";28} from "@t3tools/contracts";
29import { dedupeRemoteBranchesWithLocalMatches, normalizeGitRemoteUrl } from "@t3tools/shared/git";29import { dedupeRemoteBranchesWithLocalMatches, normalizeGitRemoteUrl } from "@t3tools/shared/git";
30import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
30import { compactTraceAttributes } from "@t3tools/shared/observability";31import { compactTraceAttributes } from "@t3tools/shared/observability";
31import { decodeJsonResult } from "@t3tools/shared/schemaJson";32import { decodeJsonResult } from "@t3tools/shared/schemaJson";
32import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts";33import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts";
...@@ -52,13 +53,12 @@ const RANGE_COMMIT_SUMMARY_MAX_OUTPUT_BYTES = 19_000;...@@ -52,13 +53,12 @@ const RANGE_COMMIT_SUMMARY_MAX_OUTPUT_BYTES = 19_000;
52const RANGE_DIFF_SUMMARY_MAX_OUTPUT_BYTES = 19_000;53const RANGE_DIFF_SUMMARY_MAX_OUTPUT_BYTES = 19_000;
53const RANGE_DIFF_PATCH_MAX_OUTPUT_BYTES = 59_000;54const RANGE_DIFF_PATCH_MAX_OUTPUT_BYTES = 59_000;
54const REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES = 120_000;55const REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES = 120_000;
55const REVIEW_UNTRACKED_DIFF_MAX_OUTPUT_BYTES = 80_000;56const REVIEW_METADATA_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
56const REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024;57const REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024;
57// Patches the clients render are parsed against git's default a/ and b/ path58// Patches the clients render are parsed against git's default a/ and b/ path
58// prefixes. A repository or global diff.noprefix or diff.mnemonicPrefix would59// prefixes. A repository or global diff.noprefix or diff.mnemonicPrefix would
59// otherwise leak into the patch and leave every parsed file unnamed.60// otherwise leak into the patch and leave every parsed file unnamed.
60export const PATCH_RENDER_PREFIX_ARGS = ["--src-prefix=a/", "--dst-prefix=b/"] as const;61export const PATCH_RENDER_PREFIX_ARGS = ["--src-prefix=a/", "--dst-prefix=b/"] as const;
61const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 120_000;
62const STATUS_UPSTREAM_REFRESH_INTERVAL = Duration.seconds(15);62const STATUS_UPSTREAM_REFRESH_INTERVAL = Duration.seconds(15);
63const STATUS_UPSTREAM_REFRESH_TIMEOUT = Duration.seconds(5);63const STATUS_UPSTREAM_REFRESH_TIMEOUT = Duration.seconds(5);
6464
...@@ -192,6 +192,26 @@ function parseNumstatEntries(...@@ -192,6 +192,26 @@ function parseNumstatEntries(
192 return entries;192 return entries;
193}193}
194194
195// -z preserves tabs/newlines in paths and gives renames two separate path fields.
196function parseReviewNumstat(stdout: string): ReviewDiffFileStat[] {
197 const fields = stdout.split("\0");
198 const files: ReviewDiffFileStat[] = [];
199 for (let index = 0; index < fields.length; index++) {
200 const field = fields[index]!;
201 const match = /^(\d+|-)\t(\d+|-)\t([\s\S]*)$/.exec(field);
202 if (!match) continue;
203 const previousPath = match[3] === "" ? fields[++index]! : null;
204 const path = previousPath !== null ? fields[++index]! : match[3]!;
205 files.push({
206 path,
207 previousPath,
208 additions: match[1] === "-" ? 0 : Number(match[1]),
209 deletions: match[2] === "-" ? 0 : Number(match[2]),
210 });
211 }
212 return files;
213}
214
195function parsePorcelainPath(line: string): string | null {215function parsePorcelainPath(line: string): string | null {
196 if (line.startsWith("? ") || line.startsWith("! ")) {216 if (line.startsWith("? ") || line.startsWith("! ")) {
197 const simple = line.slice(2).trim();217 const simple = line.slice(2).trim();
...@@ -2258,102 +2278,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2258,102 +2278,19 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2258 };2278 };
2259 });2279 });
22602280
2261 const readUntrackedReviewDiffs = Effect.fn("readUntrackedReviewDiffs")(function* (cwd: string) {2281 // Use the same temporary index for patch and statistics so unstaged renames agree.
2262 const untrackedResult = yield* executeGit(2282 const prepareReviewIndex = Effect.fn("prepareReviewIndex")(function* (
2263 "GitVcsDriver.readUntrackedReviewDiffs.list",
2264 cwd,
2265 ["ls-files", "--others", "--exclude-standard", "-z"],
2266 {
2267 maxOutputBytes: WORKSPACE_FILES_MAX_OUTPUT_BYTES,
2268 appendTruncationMarker: true,
2269 },
2270 );
2271 const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult);
2272 if (untrackedPaths.length === 0) {
2273 return { diff: "", truncated: untrackedResult.stdoutTruncated };
2274 }
2275
2276 const diffs = yield* Effect.forEach(
2277 untrackedPaths,
2278 (relativePath) =>
2279 executeGit(
2280 "GitVcsDriver.readUntrackedReviewDiffs.diff",
2281 cwd,
2282 [
2283 "diff",
2284 "--no-index",
2285 "--patch",
2286 "--no-color",
2287 "--no-ext-diff",
2288 "--no-textconv",
2289 "--minimal",
2290 ...PATCH_RENDER_PREFIX_ARGS,
2291 "--",
2292 "/dev/null",
2293 relativePath,
2294 ],
2295 {
2296 allowNonZeroExit: true,
2297 maxOutputBytes: REVIEW_UNTRACKED_DIFF_MAX_OUTPUT_BYTES,
2298 appendTruncationMarker: true,
2299 },
2300 ),
2301 { concurrency: 4 },
2302 );
2303
2304 return {
2305 diff: Arr.filterMap(diffs, (result) =>
2306 result.stdout.trim().length > 0 ? Result.succeed(result.stdout) : Result.failVoid,
2307 ).join("\n"),
2308 truncated: untrackedResult.stdoutTruncated || diffs.some((result) => result.stdoutTruncated),
2309 };
2310 });
2311
2312 const readTrackedReviewDiff = Effect.fn("readTrackedReviewDiff")(function* (
2313 cwd: string,
2314 ignoreWhitespace: boolean | undefined,
2315 ) {
2316 const result = yield* executeGit(
2317 "GitVcsDriver.readTrackedReviewDiff",
2318 cwd,
2319 [
2320 "diff",
2321 "--patch",
2322 "--no-color",
2323 "--no-ext-diff",
2324 "--no-textconv",
2325 "--minimal",
2326 ...PATCH_RENDER_PREFIX_ARGS,
2327 "--find-renames",
2328 ...(ignoreWhitespace ? ["--ignore-all-space"] : []),
2329 "HEAD",
2330 "--",
2331 ],
2332 {
2333 maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
2334 appendTruncationMarker: true,
2335 },
2336 );
2337 return { diff: result.stdout, truncated: result.stdoutTruncated };
2338 });
2339
2340 const readUnifiedWorkingTreeReviewDiff = Effect.fn("readUnifiedWorkingTreeReviewDiff")(function* (
2341 cwd: string,2283 cwd: string,
2342 untrackedPaths: ReadonlyArray<string>,2284 untrackedPaths: ReadonlyArray<string>,
2343 pathsTruncated: boolean,
2344 ignoreWhitespace: boolean | undefined,
2345 ) {2285 ) {
2346 const [stagedDeletionsStdout, indexValue] = yield* Effect.all(2286 const [stagedDeletionsStdout, indexValue] = yield* Effect.all(
2347 [2287 [
2348 runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.stagedDeletions", cwd, [2288 runGitStdoutWithOptions(
2349 "diff",2289 "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.stagedDeletions",
2350 "--cached",2290 cwd,
2351 "--name-only",2291 ["diff", "--cached", "--name-only", "--diff-filter=D", "-z", "HEAD", "--"],
2352 "--diff-filter=D",2292 { allowNonZeroExit: true, maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES },
2353 "-z",2293 ),
2354 "HEAD",
2355 "--",
2356 ]),
2357 runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.indexPath", cwd, [2294 runGitStdout("GitVcsDriver.readUnifiedWorkingTreeReviewDiff.indexPath", cwd, [
2358 "rev-parse",2295 "rev-parse",
2359 "--git-path",2296 "--git-path",
...@@ -2364,10 +2301,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2364,10 +2301,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2364 );2301 );
2365 const stagedDeletions = new Set(stagedDeletionsStdout.split("\0").filter(Boolean));2302 const stagedDeletions = new Set(stagedDeletionsStdout.split("\0").filter(Boolean));
2366 const pathsToAdd = untrackedPaths.filter((relativePath) => !stagedDeletions.has(relativePath));2303 const pathsToAdd = untrackedPaths.filter((relativePath) => !stagedDeletions.has(relativePath));
2367 if (pathsToAdd.length === 0) {2304 if (pathsToAdd.length === 0) return undefined;
2368 const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace);
2369 return { ...tracked, truncated: pathsTruncated || tracked.truncated };
2370 }
23712305
2372 const indexPath = path.isAbsolute(indexValue.trim())2306 const indexPath = path.isAbsolute(indexValue.trim())
2373 ? indexValue.trim()2307 ? indexValue.trim()
...@@ -2375,7 +2309,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2375,7 +2309,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2375 const tempIndexPath = yield* fileSystem.makeTempFileScoped({2309 const tempIndexPath = yield* fileSystem.makeTempFileScoped({
2376 prefix: `t3code-review-index-${process.pid}-`,2310 prefix: `t3code-review-index-${process.pid}-`,
2377 });2311 });
2378 yield* fileSystem.copyFile(indexPath, tempIndexPath);2312 const indexExists = yield* fileSystem.exists(indexPath);
2313 if (indexExists) yield* fileSystem.copyFile(indexPath, tempIndexPath);
2379 const env = { GIT_INDEX_FILE: tempIndexPath } satisfies NodeJS.ProcessEnv;2314 const env = { GIT_INDEX_FILE: tempIndexPath } satisfies NodeJS.ProcessEnv;
2380 const tempIndexConfig = [2315 const tempIndexConfig = [
2381 "-c",2316 "-c",
...@@ -2383,6 +2318,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2383,6 +2318,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2383 "-c",2318 "-c",
2384 "splitIndex.sharedIndexExpire=never",2319 "splitIndex.sharedIndexExpire=never",
2385 ];2320 ];
2321 if (!indexExists) {
2322 yield* executeGit("GitVcsDriver.review.emptyIndex", cwd, ["read-tree", "--empty"], { env });
2323 }
2386 yield* executeGit(2324 yield* executeGit(
2387 "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.expandSplitIndex",2325 "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.expandSplitIndex",
2388 cwd,2326 cwd,
...@@ -2402,86 +2340,27 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2402,86 +2340,27 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2402 ],2340 ],
2403 { env, stdin: `${pathsToAdd.join("\0")}\0` },2341 { env, stdin: `${pathsToAdd.join("\0")}\0` },
2404 );2342 );
2405 const result = yield* executeGit(2343 return env;
2406 "GitVcsDriver.readUnifiedWorkingTreeReviewDiff.diff",
2407 cwd,
2408 [
2409 ...tempIndexConfig,
2410 "diff",
2411 "--patch",
2412 "--no-color",
2413 "--no-ext-diff",
2414 "--no-textconv",
2415 "--minimal",
2416 ...PATCH_RENDER_PREFIX_ARGS,
2417 "--find-renames",
2418 ...(ignoreWhitespace ? ["--ignore-all-space"] : []),
2419 "HEAD",
2420 "--",
2421 ],
2422 {
2423 env,
2424 maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
2425 appendTruncationMarker: true,
2426 },
2427 );
2428 return { diff: result.stdout, truncated: pathsTruncated || result.stdoutTruncated };
2429 });
2430
2431 const readWorkingTreeReviewDiff = Effect.fn("readWorkingTreeReviewDiff")(function* (
2432 cwd: string,
2433 ignoreWhitespace: boolean | undefined,
2434 ) {
2435 const untrackedResult = yield* executeGit(
2436 "GitVcsDriver.readWorkingTreeReviewDiff.listUntracked",
2437 cwd,
2438 ["ls-files", "--others", "--exclude-standard", "-z"],
2439 {
2440 maxOutputBytes: WORKSPACE_FILES_MAX_OUTPUT_BYTES,
2441 appendTruncationMarker: true,
2442 },
2443 ).pipe(Effect.option);
2444 if (untrackedResult._tag === "None") {
2445 return yield* readTrackedReviewDiff(cwd, ignoreWhitespace);
2446 }
2447 const untrackedPaths = splitNullSeparatedGitStdoutPaths(untrackedResult.value);
2448 if (untrackedPaths.length === 0) {
2449 const tracked = yield* readTrackedReviewDiff(cwd, ignoreWhitespace);
2450 return { ...tracked, truncated: untrackedResult.value.stdoutTruncated || tracked.truncated };
2451 }
2452
2453 return yield* readUnifiedWorkingTreeReviewDiff(
2454 cwd,
2455 untrackedPaths,
2456 untrackedResult.value.stdoutTruncated,
2457 ignoreWhitespace,
2458 ).pipe(
2459 Effect.scoped,
2460 Effect.catch(() =>
2461 Effect.all([
2462 readTrackedReviewDiff(cwd, ignoreWhitespace).pipe(
2463 Effect.orElseSucceed(() => ({ diff: "", truncated: false })),
2464 ),
2465 readUntrackedReviewDiffs(cwd).pipe(
2466 Effect.orElseSucceed(() => ({ diff: "", truncated: false })),
2467 ),
2468 ]).pipe(
2469 Effect.map(([tracked, untracked]) => ({
2470 diff: [tracked.diff.trimEnd(), untracked.diff.trimEnd()]
2471 .filter((diff) => diff.length > 0)
2472 .join("\n"),
2473 truncated: tracked.truncated || untracked.truncated,
2474 })),
2475 ),
2476 ),
2477 );
2478 });2344 });
24792345
2480 const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* (2346 const getReviewDiffPreview = Effect.fn("getReviewDiffPreview")(function* (
2481 input: ReviewDiffPreviewInput,2347 input: ReviewDiffPreviewInput,
2482 ) {2348 ) {
2483 const details = yield* statusDetailsLocal(input.cwd);2349 const pathArgs = input.file
2484 if (!details.isRepo) {2350 ? [input.file.path, ...(input.file.previousPath ? [input.file.previousPath] : [])].map(
2351 (path) => `:(top,literal)${path}`,
2352 )
2353 : [];
2354 const patchLimit = input.file
2355 ? REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES
2356 : REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES;
2357 const repository = yield* resolveRepositoryPathsUncached(input.cwd).pipe(
2358 Effect.catchTags({
2359 GitCommandError: (error) =>
2360 isMissingGitCwdError(error) ? Effect.succeed(null) : Effect.fail(error),
2361 }),
2362 );
2363 if (!repository?.worktreeRoot) {
2485 return {2364 return {
2486 cwd: input.cwd,2365 cwd: input.cwd,
2487 generatedAt: yield* DateTime.now,2366 generatedAt: yield* DateTime.now,
...@@ -2489,71 +2368,143 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2489,71 +2368,143 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2489 };2368 };
2490 }2369 }
24912370
2492 const branch = details.branch;2371 const cwd = repository.worktreeRoot;
2372 const branch = repository.currentBranch;
2493 const baseRef =2373 const baseRef =
2494 input.baseRef ??2374 input.baseRef ??
2495 (branch2375 (branch
2496 ? yield* resolveBaseBranchForNoUpstream(input.cwd, branch).pipe(2376 ? yield* resolveBaseBranchForNoUpstream(cwd, branch).pipe(Effect.orElseSucceed(() => null))
2497 Effect.orElseSucceed(() => null),
2498 )
2499 : null);2377 : null);
25002378
2501 const dirtyResult = yield* readWorkingTreeReviewDiff(input.cwd, input.ignoreWhitespace).pipe(2379 const diffArgs = [
2502 Effect.orElseSucceed(() => ({2380 "diff",
2503 diff: "",2381 "--find-renames",
2504 truncated: false,2382 "--no-color",
2505 })),2383 "--no-ext-diff",
2384 "--no-textconv",
2385 "--minimal",
2386 ...PATCH_RENDER_PREFIX_ARGS,
2387 ...(input.ignoreWhitespace ? ["--ignore-all-space"] : []),
2388 ];
2389 const readStats = Effect.fn("GitVcsDriver.getReviewDiffPreview.stat")(function* (
2390 ref: string,
2391 env?: NodeJS.ProcessEnv,
2392 ) {
2393 const args = [...diffArgs, "--numstat", "-z"];
2394 const result = yield* executeGit(
2395 "GitVcsDriver.getReviewDiffPreview.stat",
2396 cwd,
2397 [...args, ref, "--", ...pathArgs],
2398 { allowNonZeroExit: true, maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES, env },
2399 );
2400 if (result.exitCode === 0) return { ref, files: parseReviewNumstat(result.stdout) };
2401 if (ref === "HEAD" && isUnbornHeadStderr(result.stderr)) {
2402 const emptyTree = (yield* runGitStdout("GitVcsDriver.getReviewDiffPreview.emptyTree", cwd, [
2403 "hash-object",
2404 "-t",
2405 "tree",
2406 (yield* HostProcessPlatform) === "win32" ? "NUL" : "/dev/null",
2407 ])).trim();
2408 const stdout = yield* runGitStdoutWithOptions(
2409 "GitVcsDriver.getReviewDiffPreview.unbornStat",
2410 cwd,
2411 [...args, emptyTree, "--", ...pathArgs],
2412 { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES, env },
2413 );
2414 return { ref: emptyTree, files: parseReviewNumstat(stdout) };
2415 }
2416 return yield* new GitCommandError({
2417 operation: "GitVcsDriver.getReviewDiffPreview.stat",
2418 cwd,
2419 command: "git diff --numstat",
2420 detail: "Could not read complete diff statistics.",
2421 exitCode: result.exitCode,
2422 });
2423 });
2424 const readTrackedDiff = Effect.fn("GitVcsDriver.getReviewDiffPreview.tracked")(function* (
2425 ref: string | null,
2426 env?: NodeJS.ProcessEnv,
2427 ) {
2428 if (ref === null) return { stdout: "", stdoutTruncated: false, files: [] };
2429 const stat = yield* readStats(ref, env);
2430 if (stat.files.length === 0) return { stdout: "", stdoutTruncated: false, files: [] };
2431 const patch = yield* executeGit(
2432 "GitVcsDriver.getReviewDiffPreview.patch",
2433 cwd,
2434 [...diffArgs, "--patch", stat.ref, "--", ...pathArgs],
2435 { maxOutputBytes: patchLimit, appendTruncationMarker: true, env },
2436 );
2437 return { ...patch, files: stat.files };
2438 });
2439 const readDirty = Effect.gen(function* () {
2440 if (input.file?.sourceKind === "branch-range") return yield* readTrackedDiff(null);
2441 const untracked = yield* executeGit(
2442 "GitVcsDriver.review.listUntracked",
2443 cwd,
2444 ["ls-files", "--others", "--exclude-standard", "-z", "--", ...pathArgs],
2445 { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES },
2446 ).pipe(
2447 Effect.catchIf(
2448 (error) => error.outputLength === undefined,
2449 () => Effect.succeed(null),
2450 ),
2451 );
2452 if (untracked === null) {
2453 const tracked = yield* readTrackedDiff("HEAD");
2454 return { ...tracked, files: undefined, stdoutTruncated: true };
2455 }
2456 const paths = splitNullSeparatedGitStdoutPaths(untracked).filter(
2457 (candidate) => !input.file || candidate === input.file.path,
2458 );
2459 if (paths.length === 0) return yield* readTrackedDiff("HEAD");
2460 const env = yield* prepareReviewIndex(cwd, paths).pipe(
2461 Effect.catchTags({
2462 PlatformError: (cause) =>
2463 Effect.fail(
2464 new GitCommandError({
2465 operation: "GitVcsDriver.prepareReviewIndex",
2466 cwd,
2467 command: "git diff",
2468 detail: "Could not prepare the review index.",
2469 cause,
2470 }),
2471 ),
2472 }),
2473 );
2474 return yield* readTrackedDiff("HEAD", env);
2475 }).pipe(Effect.scoped);
2476 const [dirtyTrackedResult, baseResult] = yield* Effect.all(
2477 [
2478 readDirty,
2479 readTrackedDiff(
2480 baseRef && branch && input.file?.sourceKind !== "working-tree"
2481 ? `${baseRef}...HEAD`
2482 : null,
2483 ),
2484 ],
2485 { concurrency: 2 },
2506 );2486 );
2507 const dirtyDiff = dirtyResult.diff;2487 const dirtyFiles = dirtyTrackedResult.files;
25082488 const baseFiles = baseResult.files;
2509 const baseResult =2489 const dirtyDiff = dirtyTrackedResult.stdout;
2510 baseRef && branch2490 const baseDiff = baseResult.stdout;
2511 ? yield* executeGit(2491 const hashDiff = (diff: string, files: ReadonlyArray<ReviewDiffFileStat>) =>
2512 "GitVcsDriver.getReviewDiffPreview.base",2492 crypto.digest("SHA-256", new TextEncoder().encode(JSON.stringify([diff, files]))).pipe(
2513 input.cwd,
2514 [
2515 "diff",
2516 "--patch",
2517 "--no-color",
2518 "--no-ext-diff",
2519 "--no-textconv",
2520 "--minimal",
2521 ...PATCH_RENDER_PREFIX_ARGS,
2522 ...(input.ignoreWhitespace ? ["--ignore-all-space"] : []),
2523 `${baseRef}...HEAD`,
2524 ],
2525 {
2526 maxOutputBytes: REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES,
2527 appendTruncationMarker: true,
2528 },
2529 ).pipe(
2530 Effect.orElseSucceed(() => ({
2531 exitCode: 0,
2532 stdout: "",
2533 stderr: "",
2534 stdoutTruncated: false,
2535 stderrTruncated: false,
2536 })),
2537 )
2538 : null;
2539 const baseDiff = baseResult?.stdout ?? "";
2540 const hashDiff = (diff: string) =>
2541 crypto.digest("SHA-256", new TextEncoder().encode(diff)).pipe(
2542 Effect.map(Encoding.encodeHex),2493 Effect.map(Encoding.encodeHex),
2543 Effect.mapError(2494 Effect.mapError(
2544 (cause) =>2495 (cause) =>
2545 new GitCommandError({2496 new GitCommandError({
2546 operation: "GitVcsDriver.getReviewDiffPreview.hash",2497 operation: "GitVcsDriver.getReviewDiffPreview.hash",
2547 command: "crypto.digest SHA-256",2498 command: "crypto.digest SHA-256",
2548 cwd: input.cwd,2499 cwd,
2549 detail: "Failed to hash review diff.",2500 detail: "Failed to hash review diff.",
2550 cause,2501 cause,
2551 }),2502 }),
2552 ),2503 ),
2553 );2504 );
2554 const [dirtyDiffHash, baseDiffHash] = yield* Effect.all([2505 const [dirtyDiffHash, baseDiffHash] = yield* Effect.all([
2555 hashDiff(dirtyDiff),2506 hashDiff(dirtyDiff, dirtyFiles ?? []),
2556 hashDiff(baseDiff),2507 hashDiff(baseDiff, baseFiles),
2557 ]);2508 ]);
25582509
2559 const sources: ReviewDiffPreviewSource[] = [2510 const sources: ReviewDiffPreviewSource[] = [
...@@ -2564,8 +2515,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2564,8 +2515,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2564 baseRef: "HEAD",2515 baseRef: "HEAD",
2565 headRef: null,2516 headRef: null,
2566 diff: dirtyDiff,2517 diff: dirtyDiff,
2518 ...(dirtyFiles === undefined ? {} : { files: dirtyFiles }),
2567 diffHash: dirtyDiffHash,2519 diffHash: dirtyDiffHash,
2568 truncated: dirtyResult.truncated,2520 truncated: dirtyTrackedResult.stdoutTruncated,
2569 },2521 },
2570 {2522 {
2571 id: "branch-range",2523 id: "branch-range",
...@@ -2574,8 +2526,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*...@@ -2574,8 +2526,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
2574 baseRef,2526 baseRef,
2575 headRef: branch ?? "HEAD",2527 headRef: branch ?? "HEAD",
2576 diff: baseDiff,2528 diff: baseDiff,
2529 files: baseFiles,
2577 diffHash: baseDiffHash,2530 diffHash: baseDiffHash,
2578 truncated: baseResult?.stdoutTruncated ?? false,2531 truncated: baseResult.stdoutTruncated,
2579 },2532 },
2580 ];2533 ];
25812534
apps/web/src/components/DiffPanel.tsx+212-84
...@@ -1,6 +1,6 @@...@@ -1,6 +1,6 @@
1import { RefreshIcon } from "~/components/ui/refresh-icon";1import { RefreshIcon } from "~/components/ui/refresh-icon";
2import { useAtomValue } from "@effect/atom-react";2import { useAtomValue } from "@effect/atom-react";
3import type { FileDiffContentsLoader } from "@pierre/diffs";3import type { FileDiffContentsLoader, FileDiffMetadata } from "@pierre/diffs";
4import { useParams } from "@tanstack/react-router";4import { useParams } from "@tanstack/react-router";
5import {5import {
6 isAtomCommandInterrupted,6 isAtomCommandInterrupted,
...@@ -23,6 +23,7 @@ import {...@@ -23,6 +23,7 @@ import {
23 TextWrapIcon,23 TextWrapIcon,
24} from "lucide-react";24} from "lucide-react";
25import * as Schema from "effect/Schema";25import * as Schema from "effect/Schema";
26import * as DateTime from "effect/DateTime";
26import { useCallback, useEffect, useMemo, useRef, useState } from "react";27import { useCallback, useEffect, useMemo, useRef, useState } from "react";
27import { useCodeViewFileReveal } from "./diffs/useCodeViewFileReveal";28import { useCodeViewFileReveal } from "./diffs/useCodeViewFileReveal";
28import { useOpenInPreferredEditor } from "../editorPreferences";29import { useOpenInPreferredEditor } from "../editorPreferences";
...@@ -86,9 +87,29 @@ import { vcsEnvironment } from "../state/vcs";...@@ -86,9 +87,29 @@ import { vcsEnvironment } from "../state/vcs";
86import { buildBaseRefChoices, filterBaseRefChoices } from "../lib/baseRefChoices";87import { buildBaseRefChoices, filterBaseRefChoices } from "../lib/baseRefChoices";
87import { createGitDiffFileContentsLoader } from "../lib/diffFileContents";88import { createGitDiffFileContentsLoader } from "../lib/diffFileContents";
8889
90import { useReviewFilePatches } from "./diffs/useReviewFilePatches";
91import { DiffFileLoadingBoundary } from "./diffs/DiffFileLoadingBoundary";
92import { DiffFileStatus } from "./diffs/DiffFileStatus";
93
89type DiffThemeType = "light" | "dark";94type DiffThemeType = "light" | "dark";
90const AUTOMATIC_BASE_REF = "__automatic_base_ref__";95const AUTOMATIC_BASE_REF = "__automatic_base_ref__";
91const DIFF_FILE_TREE_STORAGE_KEY = "t3code.diffFileTreeOpen";96const DIFF_FILE_TREE_STORAGE_KEY = "t3code.diffFileTreeOpen";
97const fileEntryCache = new WeakMap<
98 FileDiffMetadata,
99 { fileDiff: FileDiffMetadata; fileKey: string; fileVersion: number }
100>();
101
102function getCachedFileEntry(fileDiff: FileDiffMetadata) {
103 const cached = fileEntryCache.get(fileDiff);
104 if (cached) return cached;
105 const entry = {
106 fileDiff,
107 fileKey: buildFileDiffIdentityKey(fileDiff),
108 fileVersion: buildFileDiffContentVersion(fileDiff),
109 };
110 fileEntryCache.set(fileDiff, entry);
111 return entry;
112}
92113
93interface CollapsedDiffFilesState {114interface CollapsedDiffFilesState {
94 readonly scopeKey: string | null;115 readonly scopeKey: string | null;
...@@ -284,30 +305,18 @@ export default function DiffPanel({...@@ -284,30 +305,18 @@ export default function DiffPanel({
284 const branchDiffPreview = shouldRetryBranchDiffAtEnvironmentCwd305 const branchDiffPreview = shouldRetryBranchDiffAtEnvironmentCwd
285 ? fallbackBranchDiffPreview306 ? fallbackBranchDiffPreview
286 : primaryBranchDiffPreview;307 : primaryBranchDiffPreview;
287 const refreshBranchDiffPreview = branchDiffPreview.refresh;
288 const canRefreshGitDiff =308 const canRefreshGitDiff =
289 isGitRepo && selectedTurnId === null && activeThread != null && activeCwd != null;309 isGitRepo && selectedTurnId === null && activeThread != null && activeCwd != null;
290 const activeThreadRefreshKey = routeThreadRef310 const activeThreadRefreshKey = routeThreadRef
291 ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}`311 ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}`
292 : null;312 : null;
293313
294 useEffect(() => {
295 if (!canRefreshGitDiff) return;
296 const refreshOnFocus = () => refreshBranchDiffPreview();
297 window.addEventListener("focus", refreshOnFocus);
298 return () => window.removeEventListener("focus", refreshOnFocus);
299 }, [canRefreshGitDiff, refreshBranchDiffPreview]);
300
301 useWorkspaceMutationRefresh({
302 enabled: canRefreshGitDiff,
303 mutationId: workspaceMutationId,
304 refresh: refreshBranchDiffPreview,
305 resourceKey: `diff:${activeThreadRefreshKey ?? ""}`,
306 });
307
308 const selectedGitSource = branchDiffPreview.data?.sources.find(314 const selectedGitSource = branchDiffPreview.data?.sources.find(
309 (source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"),315 (source) => source.kind === (selectedGitScope === "unstaged" ? "working-tree" : "branch-range"),
310 );316 );
317 const refreshPreviewQuery = branchDiffPreview.refresh;
318 const refreshDiffFromUserAction = refreshPreviewQuery;
319
311 const currentLoadDiffFiles = useMemo<FileDiffContentsLoader | undefined>(() => {320 const currentLoadDiffFiles = useMemo<FileDiffContentsLoader | undefined>(() => {
312 const preview = branchDiffPreview.data;321 const preview = branchDiffPreview.data;
313 if (selectedTurnId !== null || !activeThread || !preview || !selectedGitSource) {322 if (selectedTurnId !== null || !activeThread || !preview || !selectedGitSource) {
...@@ -394,31 +403,64 @@ export default function DiffPanel({...@@ -394,31 +403,64 @@ export default function DiffPanel({
394 const selectedPatchError = selectedTurn ? activeCheckpointDiff.error : branchDiffPreview.error;403 const selectedPatchError = selectedTurn ? activeCheckpointDiff.error : branchDiffPreview.error;
395 const hasResolvedPatch = typeof selectedPatch === "string";404 const hasResolvedPatch = typeof selectedPatch === "string";
396 const hasNoNetChanges = hasResolvedPatch && selectedPatch.trim().length === 0;405 const hasNoNetChanges = hasResolvedPatch && selectedPatch.trim().length === 0;
406 const lazySource =
407 !selectedTurn && selectedGitSource?.truncated && selectedGitSource.files
408 ? selectedGitSource
409 : null;
397 const renderablePatch = useMemo(410 const renderablePatch = useMemo(
398 () =>411 () =>
399 getRenderablePatch(selectedPatch, `diff-panel:${resolvedTheme}`, {412 lazySource
400 compactPartialHunkOffsets: selectedTurnId === null,413 ? null
401 }),414 : getRenderablePatch(selectedPatch, `diff-panel:${resolvedTheme}`, {
402 [resolvedTheme, selectedPatch, selectedTurnId],415 compactPartialHunkOffsets: selectedTurnId === null,
416 }),
417 [lazySource, resolvedTheme, selectedPatch, selectedTurnId],
403 );418 );
404 const renderableFiles = useMemo(() => {419 const fileStats = useMemo(
405 if (!renderablePatch || renderablePatch.kind !== "files") {420 () => new Map(lazySource?.files?.map((file) => [file.path, file])),
406 return [];421 [lazySource?.files],
407 }422 );
408 return renderablePatch.files.toSorted((left, right) =>423 const {
409 resolveFileDiffPath(left).localeCompare(resolveFileDiffPath(right), undefined, {424 scope: filePatchScope,
410 numeric: true,425 isPending: areFilePatchesPending,
411 sensitivity: "base",426 fileStates,
412 }),427 retry,
413 );428 requestFile,
414 }, [renderablePatch]);429 readyFilePaths,
430 renderableFiles,
431 settledFileCount,
432 loadNextFiles,
433 } = useReviewFilePatches({
434 environmentId: activeThread?.environmentId,
435 cwd: branchDiffPreview.data?.cwd,
436 source: lazySource,
437 baseRef: lazySource?.baseRef ?? selectedBaseRef,
438 ignoreWhitespace: diffIgnoreWhitespace,
439 theme: resolvedTheme,
440 revision: branchDiffPreview.data
441 ? DateTime.formatIso(branchDiffPreview.data.generatedAt)
442 : undefined,
443 preview: renderablePatch,
444 });
445 const refreshBranchDiffPreview = refreshPreviewQuery;
446
447 useEffect(() => {
448 if (!canRefreshGitDiff) return;
449 const refreshOnFocus = () => refreshBranchDiffPreview();
450 window.addEventListener("focus", refreshOnFocus);
451 return () => window.removeEventListener("focus", refreshOnFocus);
452 }, [canRefreshGitDiff, refreshBranchDiffPreview]);
453
454 useWorkspaceMutationRefresh({
455 enabled: canRefreshGitDiff,
456 mutationId: workspaceMutationId,
457 refresh: refreshBranchDiffPreview,
458 resourceKey: `diff:${activeThreadRefreshKey ?? ""}`,
459 });
460
461 const isRefreshingDiff = branchDiffPreview.isPending || areFilePatchesPending;
415 const renderableFileEntries = useMemo(462 const renderableFileEntries = useMemo(
416 () =>463 () => renderableFiles.map(getCachedFileEntry),
417 renderableFiles.map((fileDiff) => ({
418 fileDiff,
419 fileKey: buildFileDiffIdentityKey(fileDiff),
420 fileVersion: buildFileDiffContentVersion(fileDiff),
421 })),
422 [renderableFiles],464 [renderableFiles],
423 );465 );
424 const defaultCollapsedDiffFileKeys = useMemo(466 const defaultCollapsedDiffFileKeys = useMemo(
...@@ -432,22 +474,51 @@ export default function DiffPanel({...@@ -432,22 +474,51 @@ export default function DiffPanel({
432 collapsedDiffFiles.scopeKey === collapseScopeKey474 collapsedDiffFiles.scopeKey === collapseScopeKey
433 ? collapsedDiffFiles.fileKeys475 ? collapsedDiffFiles.fileKeys
434 : defaultCollapsedDiffFileKeys;476 : defaultCollapsedDiffFileKeys;
477 const renderLoadingBoundary = useCallback(
478 () =>
479 settledFileCount < renderableFiles.length ? (
480 <DiffFileLoadingBoundary
481 load={loadNextFiles}
482 count={renderableFiles.length - settledFileCount}
483 />
484 ) : null,
485 [settledFileCount, renderableFiles.length, loadNextFiles],
486 );
435 const codeViewFiles = useMemo(487 const codeViewFiles = useMemo(
436 () =>488 () =>
437 renderableFileEntries.map(({ fileDiff, fileKey, fileVersion }) => {489 renderableFileEntries
438 return {490 .filter(({ fileDiff }) => !lazySource || readyFilePaths.has(resolveFileDiffPath(fileDiff)))
439 fileDiff,491 .map(({ fileDiff, fileKey, fileVersion }) => {
440 filePath: resolveFileDiffPath(fileDiff),492 return {
441 fileKey,493 fileDiff,
442 fileVersion,494 filePath: resolveFileDiffPath(fileDiff),
443 collapsed: collapsedDiffFileKeys.has(fileKey),495 fileKey,
444 };496 fileVersion,
445 }),497 // Header-only placeholders use the viewer's collapsed geometry until their patch arrives.
446 [collapsedDiffFileKeys, renderableFileEntries],498 collapsed:
499 collapsedDiffFileKeys.has(fileKey) ||
500 fileDiff.cacheKey?.endsWith(":pending") === true,
501 };
502 }),
503 [collapsedDiffFileKeys, renderableFileEntries, lazySource, readyFilePaths],
504 );
505 const diffFileKeys = useMemo(
506 () => renderableFileEntries.map((file) => file.fileKey),
507 [renderableFileEntries],
447 );508 );
448 const diffFileKeys = useMemo(() => codeViewFiles.map((file) => file.fileKey), [codeViewFiles]);
449 const allDiffFilesCollapsed = areAllDiffFilesCollapsed(diffFileKeys, collapsedDiffFileKeys);509 const allDiffFilesCollapsed = areAllDiffFilesCollapsed(diffFileKeys, collapsedDiffFileKeys);
450 const diffLineStat = useMemo(() => getDiffLineStat(renderableFiles), [renderableFiles]);510 const diffLineStat = useMemo(() => {
511 if (!selectedTurn && selectedGitSource?.files) {
512 return selectedGitSource.files.reduce(
513 (total, file) => ({
514 additions: total.additions + file.additions,
515 deletions: total.deletions + file.deletions,
516 }),
517 { additions: 0, deletions: 0 },
518 );
519 }
520 return getDiffLineStat(renderableFiles);
521 }, [renderableFiles, selectedGitSource, selectedTurn]);
451 const fileTreeEntries = useMemo(() => diffFileTreeEntries(renderableFiles), [renderableFiles]);522 const fileTreeEntries = useMemo(() => diffFileTreeEntries(renderableFiles), [renderableFiles]);
452 const selectedDiffFileKey = selectedFilePath523 const selectedDiffFileKey = selectedFilePath
453 ? (codeViewFiles.find((candidate) => candidate.filePath === selectedFilePath)?.fileKey ?? null)524 ? (codeViewFiles.find((candidate) => candidate.filePath === selectedFilePath)?.fileKey ?? null)
...@@ -462,25 +533,54 @@ export default function DiffPanel({...@@ -462,25 +533,54 @@ export default function DiffPanel({
462 () => ({ collapseScopeKey, diffSelection }),533 () => ({ collapseScopeKey, diffSelection }),
463 [collapseScopeKey, diffSelection],534 [collapseScopeKey, diffSelection],
464 );535 );
465 const requestTreeReveal = useCodeViewFileReveal(codeView, treeRevealScope);536 const requestTreeReveal = useCodeViewFileReveal(
537 codeView,
538 treeRevealScope,
539 codeViewFiles.map((file) => file.fileKey),
540 );
466 const revealDiffFile = useCallback(541 const revealDiffFile = useCallback(
467 (filePath: string) => {542 (filePath: string) => {
468 const file = codeViewFiles.find((candidate) => candidate.filePath === filePath);543 const index = renderableFileEntries.findIndex(
544 (candidate) => resolveFileDiffPath(candidate.fileDiff) === filePath,
545 );
546 const file = renderableFileEntries[index];
469 if (!file) return;547 if (!file) return;
470 if (file.collapsed) {548 setCollapsedDiffFiles((current) => {
471 setCollapsedDiffFiles((current) => {549 const next = new Set(
472 const next = new Set(550 current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys,
473 current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys,551 );
474 );552 next.delete(file.fileKey);
475 next.delete(file.fileKey);553 return { scopeKey: collapseScopeKey, fileKeys: next };
476 return { scopeKey: collapseScopeKey, fileKeys: next };554 });
477 });555 if (lazySource && index >= settledFileCount) {
556 requestFile(index);
478 }557 }
479 requestTreeReveal(file.fileKey);558 requestTreeReveal(file.fileKey);
480 },559 },
481 [codeViewFiles, collapseScopeKey, defaultCollapsedDiffFileKeys, requestTreeReveal],560 [
561 renderableFileEntries,
562 collapseScopeKey,
563 defaultCollapsedDiffFileKeys,
564 requestTreeReveal,
565 lazySource,
566 settledFileCount,
567 requestFile,
568 ],
482 );569 );
483570
571 const externalRevealRef = useRef<{ cache: string; key: string } | null>(null);
572 useEffect(() => {
573 if (!lazySource || !selectedFilePath) return;
574 const key = `${selectedFilePath}:${selectedFileRevealRequestId}`;
575 if (
576 externalRevealRef.current?.cache === filePatchScope &&
577 externalRevealRef.current.key === key
578 )
579 return;
580 externalRevealRef.current = { cache: filePatchScope, key };
581 revealDiffFile(selectedFilePath);
582 }, [lazySource, selectedFilePath, selectedFileRevealRequestId, filePatchScope, revealDiffFile]);
583
484 const openDiffFile = useCallback(584 const openDiffFile = useCallback(
485 (filePath: string) => {585 (filePath: string) => {
486 openDiffFilePrimaryAction({586 openDiffFilePrimaryAction({
...@@ -754,14 +854,14 @@ export default function DiffPanel({...@@ -754,14 +854,14 @@ export default function DiffPanel({
754 )}854 )}
755 </div>855 </div>
756 <div className="flex shrink-0 items-center gap-1 [-webkit-app-region:no-drag]">856 <div className="flex shrink-0 items-center gap-1 [-webkit-app-region:no-drag]">
757 {codeViewFiles.length > 0 && (857 {codeViewFiles.length > 0 || (!selectedTurn && selectedGitSource?.files?.length) ? (
758 <DiffStatLabel858 <DiffStatLabel
759 additions={diffLineStat.additions}859 additions={diffLineStat.additions}
760 deletions={diffLineStat.deletions}860 deletions={diffLineStat.deletions}
761 className="mr-1 text-[11px]"861 className="mr-1 text-[11px]"
762 layout="inline"862 layout="inline"
763 />863 />
764 )}864 ) : null}
765 {canRefreshGitDiff && (865 {canRefreshGitDiff && (
766 <Tooltip>866 <Tooltip>
767 <TooltipTrigger867 <TooltipTrigger
...@@ -770,19 +870,19 @@ export default function DiffPanel({...@@ -770,19 +870,19 @@ export default function DiffPanel({
770 type="button"870 type="button"
771 size="icon-sm"871 size="icon-sm"
772 variant="ghost"872 variant="ghost"
773 aria-label={branchDiffPreview.isPending ? "Refreshing diff" : "Refresh diff"}873 aria-label={isRefreshingDiff ? "Refreshing diff" : "Refresh diff"}
774 onClick={refreshBranchDiffPreview}874 onClick={refreshDiffFromUserAction}
775 />875 />
776 }876 }
777 >877 >
778 <RefreshIcon className="size-3.5" refreshing={branchDiffPreview.isPending} />878 <RefreshIcon className="size-3.5" refreshing={isRefreshingDiff} />
779 </TooltipTrigger>879 </TooltipTrigger>
780 <TooltipPopup side="top">880 <TooltipPopup side="top">
781 {branchDiffPreview.isPending ? "Refreshing diff…" : "Refresh diff"}881 {isRefreshingDiff ? "Refreshing diff…" : "Refresh diff"}
782 </TooltipPopup>882 </TooltipPopup>
783 </Tooltip>883 </Tooltip>
784 )}884 )}
785 {codeViewFiles.length > 0 && (885 {diffFileKeys.length > 0 && (
786 <Tooltip>886 <Tooltip>
787 <TooltipTrigger887 <TooltipTrigger
788 render={888 render={
...@@ -867,7 +967,7 @@ export default function DiffPanel({...@@ -867,7 +967,7 @@ export default function DiffPanel({
867 {diffIgnoreWhitespace ? "Show whitespace changes" : "Hide whitespace changes"}967 {diffIgnoreWhitespace ? "Show whitespace changes" : "Hide whitespace changes"}
868 </TooltipPopup>968 </TooltipPopup>
869 </Tooltip>969 </Tooltip>
870 {codeViewFiles.length > 0 && (970 {diffFileKeys.length > 0 && (
871 <Tooltip>971 <Tooltip>
872 <TooltipTrigger972 <TooltipTrigger
873 render={973 render={
...@@ -908,10 +1008,10 @@ export default function DiffPanel({...@@ -908,10 +1008,10 @@ export default function DiffPanel({
908 ) : (1008 ) : (
909 <>1009 <>
910 <div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden bg-background">1010 <div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden bg-background">
911 {isSelectedPatchTruncated && (1011 {isSelectedPatchTruncated && !lazySource && (
912 <p className="shrink-0 border-b border-border/70 bg-muted/40 px-3 py-1.5 text-[11px] text-muted-foreground">1012 <p className="shrink-0 border-b border-border/70 bg-muted/40 px-3 py-1.5 text-[11px] text-muted-foreground">
913 This diff was truncated because it exceeded the preview limit. The changes shown are1013 This preview exceeds the size limit. Changes shown are incomplete.
914 incomplete.1014 {selectedGitSource?.files ? " Totals include all changes." : ""}
915 </p>1015 </p>
916 )}1016 )}
917 {selectedPatchError && !renderablePatch && (1017 {selectedPatchError && !renderablePatch && (
...@@ -919,7 +1019,7 @@ export default function DiffPanel({...@@ -919,7 +1019,7 @@ export default function DiffPanel({
919 <p className="mb-2 text-[11px] text-error/80">{selectedPatchError}</p>1019 <p className="mb-2 text-[11px] text-error/80">{selectedPatchError}</p>
920 </div>1020 </div>
921 )}1021 )}
922 {!renderablePatch ? (1022 {!renderablePatch && !lazySource ? (
923 isLoadingSelectedPatch ? (1023 isLoadingSelectedPatch ? (
924 <DiffPanelLoadingState1024 <DiffPanelLoadingState
925 label={1025 label={
...@@ -939,7 +1039,7 @@ export default function DiffPanel({...@@ -939,7 +1039,7 @@ export default function DiffPanel({
939 </p>1039 </p>
940 </div>1040 </div>
941 )1041 )
942 ) : renderablePatch.kind === "files" ? (1042 ) : lazySource || renderablePatch?.kind === "files" ? (
943 <div className="flex min-h-0 flex-1 overflow-hidden">1043 <div className="flex min-h-0 flex-1 overflow-hidden">
944 <div1044 <div
945 className="min-h-0 min-w-0 flex-1"1045 className="min-h-0 min-w-0 flex-1"
...@@ -957,7 +1057,7 @@ export default function DiffPanel({...@@ -957,7 +1057,7 @@ export default function DiffPanel({
957 (node): node is HTMLElement =>1057 (node): node is HTMLElement =>
958 node instanceof HTMLElement && node.hasAttribute("data-title"),1058 node instanceof HTMLElement && node.hasAttribute("data-title"),
959 );1059 );
960 const filePath = title?.textContent?.trim();1060 const filePath = title?.textContent;
961 // The filename remains the explicit "open in editor" affordance.1061 // The filename remains the explicit "open in editor" affordance.
962 if (filePath) {1062 if (filePath) {
963 openDiffFile(filePath);1063 openDiffFile(filePath);
...@@ -967,9 +1067,7 @@ export default function DiffPanel({...@@ -967,9 +1067,7 @@ export default function DiffPanel({
967 (node): node is HTMLElement =>1067 (node): node is HTMLElement =>
968 node instanceof HTMLElement && node.hasAttribute("data-diffs-header"),1068 node instanceof HTMLElement && node.hasAttribute("data-diffs-header"),
969 );1069 );
970 const headerFilePath = header1070 const headerFilePath = header?.querySelector("[data-title]")?.textContent;
971 ?.querySelector("[data-title]")
972 ?.textContent?.trim();
973 if (!headerFilePath) return;1071 if (!headerFilePath) return;
974 const file = codeViewFiles.find(1072 const file = codeViewFiles.find(
975 (candidate) => candidate.filePath === headerFilePath,1073 (candidate) => candidate.filePath === headerFilePath,
...@@ -980,16 +1078,43 @@ export default function DiffPanel({...@@ -980,16 +1078,43 @@ export default function DiffPanel({
980 <AnnotatableCodeView1078 <AnnotatableCodeView
981 key={collapseScopeKey ?? reviewSectionId}1079 key={collapseScopeKey ?? reviewSectionId}
982 viewerRef={setCodeView}1080 viewerRef={setCodeView}
983 codeViewKey={codeViewMountKey}1081 codeViewKey={`${codeViewMountKey}:${lazySource ? filePatchScope : "preview"}`}
984 className="h-full min-h-0 overflow-auto"1082 className="h-full min-h-0 overflow-auto"
985 files={codeViewFiles}1083 files={codeViewFiles}
1084 renderCodeViewFooter={renderLoadingBoundary}
986 sectionId={reviewSectionId}1085 sectionId={reviewSectionId}
987 sectionTitle={reviewSectionTitle}1086 sectionTitle={reviewSectionTitle}
988 composerDraftTarget={composerDraftTarget}1087 composerDraftTarget={composerDraftTarget}
989 renderHeaderFilenameSuffix={(fileDiff) => (1088 renderHeaderFilenameSuffix={(fileDiff) => {
990 <DiffFilePathCopyButton filePath={resolveFileDiffPath(fileDiff)} />1089 const path = resolveFileDiffPath(fileDiff);
991 )}1090 const stat = fileStats.get(path);
992 renderHeaderPrefix={(fileDiff, fileKey, collapsed) => {1091 return (
1092 <>
1093 <DiffFilePathCopyButton filePath={path} />
1094 {stat ? (
1095 <DiffFileStatus {...fileStates.get(path)} retry={() => retry(path)} />
1096 ) : null}
1097 </>
1098 );
1099 }}
1100 {...(lazySource
1101 ? {
1102 unsafeCSSExtra:
1103 "[data-additions-count], [data-deletions-count] { display: none; }",
1104 renderHeaderMetadata: (fileDiff: FileDiffMetadata) => {
1105 const stat = fileStats.get(resolveFileDiffPath(fileDiff));
1106 return stat ? (
1107 <DiffStatLabel
1108 additions={stat.additions}
1109 deletions={stat.deletions}
1110 />
1111 ) : null;
1112 },
1113 }
1114 : {})}
1115 renderHeaderPrefix={(fileDiff, fileKey) => {
1116 const unavailable = fileDiff.cacheKey?.endsWith(":pending") === true;
1117 const collapsed = unavailable || collapsedDiffFileKeys.has(fileKey);
993 const filePath = resolveFileDiffPath(fileDiff);1118 const filePath = resolveFileDiffPath(fileDiff);
994 return (1119 return (
995 <Tooltip>1120 <Tooltip>
...@@ -1006,6 +1131,7 @@ export default function DiffPanel({...@@ -1006,6 +1131,7 @@ export default function DiffPanel({
1006 collapsed ? `Expand ${filePath}` : `Collapse ${filePath}`1131 collapsed ? `Expand ${filePath}` : `Collapse ${filePath}`
1007 }1132 }
1008 aria-expanded={!collapsed}1133 aria-expanded={!collapsed}
1134 disabled={unavailable}
1009 onClick={(event) => {1135 onClick={(event) => {
1010 event.stopPropagation();1136 event.stopPropagation();
1011 toggleDiffFileCollapsed(fileKey);1137 toggleDiffFileCollapsed(fileKey);
...@@ -1052,7 +1178,9 @@ export default function DiffPanel({...@@ -1052,7 +1178,9 @@ export default function DiffPanel({
1052 ) : (1178 ) : (
1053 <div className="min-h-0 flex-1 overflow-auto p-2">1179 <div className="min-h-0 flex-1 overflow-auto p-2">
1054 <div className="space-y-2">1180 <div className="space-y-2">
1055 <p className="text-[11px] text-muted-foreground/75">{renderablePatch.reason}</p>1181 <p className="text-[11px] text-muted-foreground/75">
1182 {renderablePatch?.kind === "raw" ? renderablePatch.reason : null}
1183 </p>
1056 <pre1184 <pre
1057 className={cn(1185 className={cn(
1058 "max-h-[72vh] rounded-md border border-border/70 bg-background/70 p-3 font-mono text-[11px] leading-relaxed text-muted-foreground/90",1186 "max-h-[72vh] rounded-md border border-border/70 bg-background/70 p-3 font-mono text-[11px] leading-relaxed text-muted-foreground/90",
...@@ -1061,7 +1189,7 @@ export default function DiffPanel({...@@ -1061,7 +1189,7 @@ export default function DiffPanel({
1061 : "overflow-auto",1189 : "overflow-auto",
1062 )}1190 )}
1063 >1191 >
1064 {renderablePatch.text}1192 {renderablePatch?.kind === "raw" ? renderablePatch.text : null}
1065 </pre>1193 </pre>
1066 </div>1194 </div>
1067 </div>1195 </div>
apps/web/src/components/DiffPanelShell.tsx+1-1
...@@ -46,7 +46,7 @@ export function DiffPanelShell(props: {...@@ -46,7 +46,7 @@ export function DiffPanelShell(props: {
46 );46 );
47}47}
4848
49function DiffFileHeaderSkeleton({ titleClassName }: { titleClassName: string }) {49export function DiffFileHeaderSkeleton({ titleClassName }: { titleClassName: string }) {
50 return (50 return (
51 <div className="flex h-8 items-center gap-2 px-2 pr-3">51 <div className="flex h-8 items-center gap-2 px-2 pr-3">
52 <div className="flex size-5 shrink-0 items-center justify-center">52 <div className="flex size-5 shrink-0 items-center justify-center">
apps/web/src/components/diffs/AnnotatableCodeView.tsx+14
...@@ -86,6 +86,9 @@ interface AnnotatableCodeViewProps {...@@ -86,6 +86,9 @@ interface AnnotatableCodeViewProps {
86 options: StyledDiffCodeViewOptions<DiffCommentAnnotationGroup>;86 options: StyledDiffCodeViewOptions<DiffCommentAnnotationGroup>;
87 viewerRef?: Ref<AnnotatableCodeViewHandle>;87 viewerRef?: Ref<AnnotatableCodeViewHandle>;
88 className?: string;88 className?: string;
89 renderCodeViewFooter?: () => ReactNode;
90 unsafeCSSExtra?: string;
91 renderHeaderMetadata?: (fileDiff: FileDiffMetadata) => ReactNode;
89 renderHeaderFilenameSuffix: (fileDiff: FileDiffMetadata) => ReactNode;92 renderHeaderFilenameSuffix: (fileDiff: FileDiffMetadata) => ReactNode;
90 renderHeaderPrefix: (93 renderHeaderPrefix: (
91 fileDiff: FileDiffMetadata,94 fileDiff: FileDiffMetadata,
...@@ -107,6 +110,9 @@ export function AnnotatableCodeView({...@@ -107,6 +110,9 @@ export function AnnotatableCodeView({
107 options,110 options,
108 viewerRef,111 viewerRef,
109 className,112 className,
113 renderCodeViewFooter,
114 unsafeCSSExtra,
115 renderHeaderMetadata,
110 renderHeaderFilenameSuffix,116 renderHeaderFilenameSuffix,
111 renderHeaderPrefix,117 renderHeaderPrefix,
112}: AnnotatableCodeViewProps) {118}: AnnotatableCodeViewProps) {
...@@ -245,6 +251,14 @@ export function AnnotatableCodeView({...@@ -245,6 +251,14 @@ export function AnnotatableCodeView({
245 key={codeViewKey}251 key={codeViewKey}
246 {...(viewerRef ? { viewerRef } : {})}252 {...(viewerRef ? { viewerRef } : {})}
247 {...(className ? { className } : {})}253 {...(className ? { className } : {})}
254 {...(unsafeCSSExtra ? { unsafeCSSExtra } : {})}
255 {...(renderHeaderMetadata
256 ? {
257 renderHeaderMetadata: (item: CodeViewItem<DiffCommentAnnotationGroup>) =>
258 item.type === "diff" ? renderHeaderMetadata(item.fileDiff) : null,
259 }
260 : {})}
261 {...(renderCodeViewFooter ? { renderCodeViewFooter } : {})}
248 items={items}262 items={items}
249 selectedLines={selectedLines}263 selectedLines={selectedLines}
250 onSelectedLinesChange={setSelectedLines}264 onSelectedLinesChange={setSelectedLines}
apps/web/src/components/diffs/DiffFileLoadingBoundary.tsx created+27
...@@ -0,0 +1,27 @@
1import { useEffect, useRef } from "react";
2import { DiffFileHeaderSkeleton } from "../DiffPanelShell";
3
4/** Load the next batch before the reader reaches the end of the current files. */
5export function DiffFileLoadingBoundary({ load, count }: { load: () => void; count: number }) {
6 const ref = useRef<HTMLDivElement>(null);
7 useEffect(() => {
8 if (!ref.current) return;
9 const observer = new IntersectionObserver(
10 (entries) => {
11 if (entries.some((entry) => entry.isIntersecting)) load();
12 },
13 { rootMargin: "600px" },
14 );
15 observer.observe(ref.current);
16 return () => observer.disconnect();
17 }, [load]);
18 return (
19 <div ref={ref} role="status" aria-label="Loading diff…">
20 {Array.from({ length: Math.min(count, 4) }, (_, index) => (
21 <div key={index} aria-hidden className="border-b border-border/40">
22 <DiffFileHeaderSkeleton titleClassName="w-1/2 max-w-64" />
23 </div>
24 ))}
25 </div>
26 );
27}
apps/web/src/components/diffs/DiffFileStatus.tsx created+39
...@@ -0,0 +1,39 @@
1import { InfoIcon, RotateCwIcon } from "lucide-react";
2import { Button } from "../ui/button";
3import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
4
5export function DiffFileStatus({
6 error,
7 truncated,
8 retry,
9}: {
10 error?: boolean | undefined;
11 truncated?: boolean | undefined;
12 retry: () => void;
13}) {
14 if (!error && !truncated) return null;
15 return (
16 <Tooltip>
17 <TooltipTrigger
18 render={
19 <Button
20 size="icon-micro"
21 variant="ghost-muted"
22 aria-label={error ? "Retry loading diff" : "Partial diff preview"}
23 onClick={(event) => {
24 event.stopPropagation();
25 if (error) retry();
26 }}
27 />
28 }
29 >
30 {error ? <RotateCwIcon className="size-3" /> : <InfoIcon className="size-3" />}
31 </TooltipTrigger>
32 <TooltipPopup>
33 {error
34 ? "Retry loading diff"
35 : "This file is too large to show in full. Counts include all changes."}
36 </TooltipPopup>
37 </Tooltip>
38 );
39}
apps/web/src/components/diffs/diffFileTree.logic.test.ts+3-3
...@@ -11,21 +11,21 @@ import {...@@ -11,21 +11,21 @@ import {
11} from "./diffFileTree.logic";11} from "./diffFileTree.logic";
1212
13function file(type: FileDiffMetadata["type"], name: string, prevName = name): FileDiffMetadata {13function file(type: FileDiffMetadata["type"], name: string, prevName = name): FileDiffMetadata {
14 return { type, name: `b/${name}`, prevName: `a/${prevName}` } as FileDiffMetadata;14 return { type, name, prevName } as FileDiffMetadata;
15}15}
1616
17describe("diffFileTreeEntries", () => {17describe("diffFileTreeEntries", () => {
18 it("maps each change type to its git status under the file's current path", () => {18 it("maps each change type to its git status under the file's current path", () => {
19 expect(19 expect(
20 diffFileTreeEntries([20 diffFileTreeEntries([
21 file("new", "src/a.ts"),21 file("new", "a/src/a.ts"),
22 file("deleted", "src/b.ts"),22 file("deleted", "src/b.ts"),
23 file("rename-pure", "src/c.ts", "src/old-c.ts"),23 file("rename-pure", "src/c.ts", "src/old-c.ts"),
24 file("rename-changed", "src/d.ts", "src/old-d.ts"),24 file("rename-changed", "src/d.ts", "src/old-d.ts"),
25 file("change", "README.md"),25 file("change", "README.md"),
26 ]),26 ]),
27 ).toEqual([27 ).toEqual([
28 { path: "src/a.ts", status: "added" },28 { path: "a/src/a.ts", status: "added" },
29 { path: "src/b.ts", status: "deleted" },29 { path: "src/b.ts", status: "deleted" },
30 { path: "src/c.ts", status: "renamed" },30 { path: "src/c.ts", status: "renamed" },
31 { path: "src/d.ts", status: "renamed" },31 { path: "src/d.ts", status: "renamed" },
apps/web/src/components/diffs/useCodeViewFileReveal.ts+8-3
...@@ -8,7 +8,11 @@ interface FileRevealHandle {...@@ -8,7 +8,11 @@ interface FileRevealHandle {
88
9// Wait for a mounted viewer and expanded rows, then apply each tree click once.9// Wait for a mounted viewer and expanded rows, then apply each tree click once.
10// Keep scope stable until the diff or external file selection changes.10// Keep scope stable until the diff or external file selection changes.
11export function useCodeViewFileReveal<TScope>(viewer: FileRevealHandle | null, scope: TScope) {11export function useCodeViewFileReveal<TScope>(
12 viewer: FileRevealHandle | null,
13 scope: TScope,
14 readyFileKeys?: ReadonlyArray<string>,
15) {
12 const [request, setRequest] = useState<{ fileKey: string; scope: TScope } | null>(null);16 const [request, setRequest] = useState<{ fileKey: string; scope: TScope } | null>(null);
13 const handledRequest = useRef<typeof request>(null);17 const handledRequest = useRef<typeof request>(null);
1418
...@@ -18,11 +22,12 @@ export function useCodeViewFileReveal<TScope>(viewer: FileRevealHandle | null, s...@@ -18,11 +22,12 @@ export function useCodeViewFileReveal<TScope>(viewer: FileRevealHandle | null, s
18 handledRequest.current = request;22 handledRequest.current = request;
19 return;23 return;
20 }24 }
21 if (!viewer?.getInstance()) return;25 if (!viewer?.getInstance() || (readyFileKeys && !readyFileKeys.includes(request.fileKey)))
26 return;
2227
23 viewer.scrollTo({ type: "item", id: request.fileKey, align: "start" });28 viewer.scrollTo({ type: "item", id: request.fileKey, align: "start" });
24 handledRequest.current = request;29 handledRequest.current = request;
25 }, [request, scope, viewer]);30 }, [request, scope, viewer, readyFileKeys]);
2631
27 return useCallback((fileKey: string) => setRequest({ fileKey, scope }), [scope]);32 return useCallback((fileKey: string) => setRequest({ fileKey, scope }), [scope]);
28}33}
apps/web/src/components/diffs/useReviewFilePatches.ts created+229
...@@ -0,0 +1,229 @@
1import { RegistryContext, useAtomValue } from "@effect/atom-react";
2import type { FileDiffMetadata } from "@pierre/diffs";
3import type { EnvironmentId, ReviewDiffPreviewSource } from "@t3tools/contracts";
4import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
5import * as Atom from "effect/unstable/reactivity/Atom";
6import { useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
7import { getRenderablePatch, resolveFileDiffPath, type RenderablePatch } from "~/lib/diffRendering";
8import { reviewEnvironment } from "~/state/review";
9
10export function useReviewFilePatches({
11 environmentId,
12 cwd,
13 source,
14 baseRef,
15 ignoreWhitespace,
16 theme,
17 revision,
18 preview,
19}: {
20 environmentId: EnvironmentId | undefined;
21 cwd: string | undefined;
22 source: ReviewDiffPreviewSource | null;
23 baseRef: string | null;
24 ignoreWhitespace: boolean;
25 theme: "light" | "dark";
26 revision: string | undefined;
27 preview: RenderablePatch | null;
28}) {
29 const registry = useContext(RegistryContext);
30 const scope = JSON.stringify([
31 environmentId,
32 cwd,
33 source?.kind,
34 source?.diffHash,
35 baseRef,
36 ignoreWhitespace,
37 ]);
38 const [requested, setRequested] = useState({ scope, indices: [0, 1, 2, 3] });
39 const indices = useMemo(
40 () => (requested.scope === scope ? requested.indices : [0, 1, 2, 3]),
41 [requested, scope],
42 );
43 const files = useMemo(
44 () =>
45 source?.files?.toSorted((a, b) =>
46 a.path.localeCompare(b.path, undefined, { numeric: true, sensitivity: "base" }),
47 ) ?? [],
48 [source?.files],
49 );
50 const queries = useMemo(
51 () =>
52 !environmentId || !cwd || !source
53 ? []
54 : indices
55 .filter((index) => index < files.length)
56 .map((index) => {
57 const file = files[index]!;
58 return {
59 index,
60 query: reviewEnvironment.diffFilePatch({
61 environmentId,
62 input: {
63 cacheKey: scope,
64 request: {
65 cwd,
66 ...(baseRef ? { baseRef } : {}),
67 ignoreWhitespace,
68 file: {
69 path: file.path,
70 previousPath: file.previousPath,
71 sourceKind: source.kind,
72 },
73 },
74 },
75 }),
76 };
77 }),
78 [environmentId, cwd, source, files, indices, scope, baseRef, ignoreWhitespace],
79 );
80 const previousPreview = useRef({ scope, revision, queries: [] as typeof queries });
81 useEffect(() => {
82 const previous = previousPreview.current;
83 previousPreview.current = { scope, revision, queries };
84 for (const { query } of queries) {
85 const changed = previous.scope === scope && previous.revision !== revision;
86 const cached =
87 !previous.queries.some((entry) => entry.query === query) &&
88 registry.get(query)._tag !== "Initial" &&
89 !registry.get(query).waiting;
90 if (changed || cached) registry.refresh(query);
91 }
92 }, [scope, revision, queries, registry]);
93 // Derived atoms parse each query result once, even when another file finishes loading.
94 const parsedQuery = useMemo(
95 () =>
96 Atom.family((query: ReturnType<typeof reviewEnvironment.diffFilePatch>) =>
97 Atom.map(query, (result) =>
98 AsyncResult.map(result, (source) => {
99 let patch = getRenderablePatch(source.diff, `diff-panel:${theme}`, {
100 compactPartialHunkOffsets: true,
101 });
102 if (patch?.kind === "files" && patch.files.length === 1 && source.files?.length === 1) {
103 const stat = source.files[0]!;
104 const file = { ...patch.files[0]!, name: stat.path };
105 if (stat.previousPath !== null) file.prevName = stat.previousPath;
106 else delete file.prevName;
107 patch = { ...patch, files: [file] };
108 }
109 return { source, patch };
110 }),
111 ),
112 ),
113 [theme],
114 );
115 const patches = useAtomValue(
116 useMemo(
117 () =>
118 Atom.make(
119 (get) => new Map(queries.map(({ index, query }) => [index, get(parsedQuery(query))])),
120 ),
121 [queries, parsedQuery],
122 ),
123 );
124 const pendingIndex = files.findIndex((_, index) => {
125 const patch = patches.get(index);
126 return !patch || patch._tag === "Initial";
127 });
128 const settledFileCount = source
129 ? pendingIndex < 0
130 ? files.length
131 : pendingIndex
132 : preview?.kind === "files"
133 ? preview.files.length
134 : 0;
135 const requestFiles = useCallback(
136 (indices: number[]) =>
137 setRequested((current) => {
138 const previous = current.scope === scope ? current.indices : [0, 1, 2, 3];
139 const added = indices.filter((index) => !previous.includes(index));
140 return added.length === 0 ? current : { scope, indices: [...previous, ...added] };
141 }),
142 [scope],
143 );
144 const loadNextFiles = useCallback(
145 () => requestFiles(Array.from({ length: 4 }, (_, index) => settledFileCount + index)),
146 [requestFiles, settledFileCount],
147 );
148 const requestFile = useCallback((index: number) => requestFiles([index]), [requestFiles]);
149 const retry = useCallback(
150 (path: string) => {
151 const query = queries.find(({ index }) => files[index]?.path === path)?.query;
152 if (query) registry.refresh(query);
153 },
154 [queries, files, registry],
155 );
156 const renderableFiles = useMemo(
157 () =>
158 source
159 ? files.map((file, index): FileDiffMetadata => {
160 const result = patches.get(index);
161 if (result?._tag === "Success" && result.value.patch?.kind === "files") {
162 const loaded = result.value.patch.files.find(
163 (candidate) => resolveFileDiffPath(candidate) === file.path,
164 );
165 if (loaded) return loaded;
166 }
167 return {
168 name: file.path,
169 ...(file.previousPath ? { prevName: file.previousPath } : {}),
170 type: file.previousPath ? "rename-changed" : "change",
171 hunks: [],
172 additionLines: [],
173 deletionLines: [],
174 splitLineCount: 0,
175 unifiedLineCount: 0,
176 isPartial: true,
177 cacheKey: `${scope}:${file.path}:pending`,
178 };
179 })
180 : (preview?.kind === "files" ? preview.files : []).toSorted((a, b) =>
181 resolveFileDiffPath(a).localeCompare(resolveFileDiffPath(b), undefined, {
182 numeric: true,
183 sensitivity: "base",
184 }),
185 ),
186 [source, files, patches, scope, preview],
187 );
188 const fileStates = new Map(
189 files.map((file, index) => {
190 const patch = patches.get(index);
191 return [
192 file.path,
193 {
194 error:
195 patch?._tag === "Failure" ||
196 (patch?._tag === "Success" &&
197 (patch.value.patch?.kind !== "files" ||
198 !patch.value.patch.files.some(
199 (candidate) => resolveFileDiffPath(candidate) === file.path,
200 ))),
201 truncated: patch?._tag === "Success" && patch.value.source.truncated,
202 },
203 ] as const;
204 }),
205 );
206 const readyFilePaths = useMemo(
207 () =>
208 new Set(
209 files
210 .filter((_, index) => {
211 const patch = patches.get(index);
212 return patch && patch._tag !== "Initial";
213 })
214 .map((file) => file.path),
215 ),
216 [files, patches],
217 );
218 return {
219 scope,
220 fileStates,
221 isPending: [...patches.values()].some((patch) => patch._tag === "Initial" || patch.waiting),
222 retry,
223 requestFile,
224 readyFilePaths,
225 renderableFiles,
226 settledFileCount,
227 loadNextFiles,
228 };
229}
apps/web/src/lib/diffFileContents.test.ts+2-2
...@@ -18,8 +18,8 @@ const SOURCE = {...@@ -18,8 +18,8 @@ const SOURCE = {
18function fileDiff(type: FileDiffMetadata["type"] = "rename-changed"): FileDiffMetadata {18function fileDiff(type: FileDiffMetadata["type"] = "rename-changed"): FileDiffMetadata {
19 return {19 return {
20 type,20 type,
21 prevName: "a/src/old-name.ts",21 prevName: "src/old-name.ts",
22 name: "b/src/new-name.ts",22 name: "src/new-name.ts",
23 } as FileDiffMetadata;23 } as FileDiffMetadata;
24}24}
2525
apps/web/src/lib/diffRendering.test.ts+30
...@@ -6,6 +6,8 @@ import {...@@ -6,6 +6,8 @@ import {
6 buildPatchCacheKey,6 buildPatchCacheKey,
7 getDiffLineStat,7 getDiffLineStat,
8 getRenderablePatch,8 getRenderablePatch,
9 resolveFileDiffPath,
10 resolveFileDiffPreviousPath,
9} from "./diffRendering";11} from "./diffRendering";
1012
11describe("buildPatchCacheKey", () => {13describe("buildPatchCacheKey", () => {
...@@ -32,6 +34,34 @@ describe("buildPatchCacheKey", () => {...@@ -32,6 +34,34 @@ describe("buildPatchCacheKey", () => {
32});34});
3335
34describe("getRenderablePatch", () => {36describe("getRenderablePatch", () => {
37 it.each([
38 ["a/example.ts", "a/example.ts"],
39 ["b/example.ts", "b/example.ts"],
40 ["a/before.ts", "b/after.ts"],
41 ])("preserves repository paths from %s to %s", (previousPath, path) => {
42 const parsed = getRenderablePatch(
43 [
44 `diff --git a/${previousPath} b/${path}`,
45 ...(previousPath === path
46 ? []
47 : ["similarity index 50%", `rename from ${previousPath}`, `rename to ${path}`]),
48 `--- a/${previousPath}`,
49 `+++ b/${path}`,
50 "@@ -1 +1 @@",
51 "-before",
52 "+after",
53 ].join("\n"),
54 );
55 expect(parsed?.kind).toBe("files");
56 if (parsed?.kind !== "files") return;
57 const file = parsed.files[0];
58 expect(file).toBeDefined();
59 if (!file) return;
60 expect(resolveFileDiffPath(file)).toBe(path);
61 expect(resolveFileDiffPreviousPath(file)).toBe(previousPath);
62 expect(buildFileDiffIdentityKey(file)).toBe(`${previousPath}\0${path}`);
63 });
64
35 it("compacts partial hunk render offsets for virtualized review diffs", () => {65 it("compacts partial hunk render offsets for virtualized review diffs", () => {
36 const patch = [66 const patch = [
37 "diff --git a/example.ts b/example.ts",67 "diff --git a/example.ts b/example.ts",
apps/web/src/lib/diffRendering.ts+2-10
...@@ -144,11 +144,7 @@ export function getRenderablePatch(...@@ -144,11 +144,7 @@ export function getRenderablePatch(
144}144}
145145
146export function resolveFileDiffPath(fileDiff: FileDiffMetadata): string {146export function resolveFileDiffPath(fileDiff: FileDiffMetadata): string {
147 const raw = fileDiff.name ?? fileDiff.prevName ?? "";147 return fileDiff.name ?? fileDiff.prevName ?? "";
148 if (raw.startsWith("a/") || raw.startsWith("b/")) {
149 return raw.slice(2);
150 }
151 return raw;
152}148}
153149
154/**150/**
...@@ -156,11 +152,7 @@ export function resolveFileDiffPath(fileDiff: FileDiffMetadata): string {...@@ -156,11 +152,7 @@ export function resolveFileDiffPath(fileDiff: FileDiffMetadata): string {
156 * path, and the hosts that resolve a diff position against both sides need both names.152 * path, and the hosts that resolve a diff position against both sides need both names.
157 */153 */
158export function resolveFileDiffPreviousPath(fileDiff: FileDiffMetadata): string {154export function resolveFileDiffPreviousPath(fileDiff: FileDiffMetadata): string {
159 const raw = fileDiff.prevName ?? fileDiff.name ?? "";155 return fileDiff.prevName ?? fileDiff.name ?? "";
160 if (raw.startsWith("a/") || raw.startsWith("b/")) {
161 return raw.slice(2);
162 }
163 return raw;
164}156}
165157
166export function buildFileDiffIdentityKey(fileDiff: FileDiffMetadata): string {158export function buildFileDiffIdentityKey(fileDiff: FileDiffMetadata): string {
packages/client-runtime/src/state/review.ts+35-1
...@@ -1,9 +1,17 @@...@@ -1,9 +1,17 @@
1import { WS_METHODS } from "@t3tools/contracts";1import {
2 type ReviewDiffPreviewInput,
3 VcsUnsupportedOperationError,
4 WS_METHODS,
5} from "@t3tools/contracts";
6import * as Effect from "effect/Effect";
7import * as Semaphore from "effect/Semaphore";
8import { request } from "../rpc/client.ts";
2import { Atom } from "effect/unstable/reactivity";9import { Atom } from "effect/unstable/reactivity";
310
4import {11import {
5 createAtomCommandScheduler,12 createAtomCommandScheduler,
6 createEnvironmentRpcCommand,13 createEnvironmentRpcCommand,
14 createEnvironmentQueryAtomFamily,
7 createEnvironmentRpcQueryAtomFamily,15 createEnvironmentRpcQueryAtomFamily,
8} from "./runtime.ts";16} from "./runtime.ts";
9import type { EnvironmentRegistry } from "../connection/registry.ts";17import type { EnvironmentRegistry } from "../connection/registry.ts";
...@@ -11,6 +19,7 @@ import type { EnvironmentRegistry } from "../connection/registry.ts";...@@ -11,6 +19,7 @@ import type { EnvironmentRegistry } from "../connection/registry.ts";
11export function createReviewEnvironmentAtoms<R, E>(19export function createReviewEnvironmentAtoms<R, E>(
12 runtime: Atom.AtomRuntime<EnvironmentRegistry | R, E>,20 runtime: Atom.AtomRuntime<EnvironmentRegistry | R, E>,
13) {21) {
22 const patchReads = Semaphore.makeUnsafe(4);
14 const diffFileScheduler = createAtomCommandScheduler();23 const diffFileScheduler = createAtomCommandScheduler();
15 return {24 return {
16 diffPreview: createEnvironmentRpcQueryAtomFamily(runtime, {25 diffPreview: createEnvironmentRpcQueryAtomFamily(runtime, {
...@@ -18,6 +27,31 @@ export function createReviewEnvironmentAtoms<R, E>(...@@ -18,6 +27,31 @@ export function createReviewEnvironmentAtoms<R, E>(
18 tag: WS_METHODS.reviewGetDiffPreview,27 tag: WS_METHODS.reviewGetDiffPreview,
19 staleTimeMs: 5_000,28 staleTimeMs: 5_000,
20 }),29 }),
30 diffFilePatch: createEnvironmentQueryAtomFamily(runtime, {
31 label: "environment-data:review:diff-file-patch",
32 staleTimeMs: 5 * 60_000,
33 execute: (input: {
34 request: ReviewDiffPreviewInput & { file: NonNullable<ReviewDiffPreviewInput["file"]> };
35 cacheKey: string;
36 }) =>
37 request(WS_METHODS.reviewGetDiffPreview, input.request).pipe(
38 patchReads.withPermit,
39 Effect.flatMap((result) => {
40 const source = result.sources.find(
41 (source) => source.kind === input.request.file.sourceKind,
42 );
43 return source
44 ? Effect.succeed(source)
45 : Effect.fail(
46 new VcsUnsupportedOperationError({
47 operation: "review.diffFilePatch",
48 kind: "git",
49 detail: "Diff no longer available. Refresh the comparison.",
50 }),
51 );
52 }),
53 ),
54 }),
21 diffFileContents: createEnvironmentRpcCommand(runtime, {55 diffFileContents: createEnvironmentRpcCommand(runtime, {
22 label: "environment-data:review:diff-file-contents",56 label: "environment-data:review:diff-file-contents",
23 tag: WS_METHODS.reviewGetDiffFileContents,57 tag: WS_METHODS.reviewGetDiffFileContents,
packages/contracts/src/review.ts+17
...@@ -7,12 +7,27 @@ export const ReviewDiffPreviewInput = Schema.Struct({...@@ -7,12 +7,27 @@ export const ReviewDiffPreviewInput = Schema.Struct({
7 cwd: TrimmedNonEmptyString,7 cwd: TrimmedNonEmptyString,
8 baseRef: Schema.optional(TrimmedNonEmptyString),8 baseRef: Schema.optional(TrimmedNonEmptyString),
9 ignoreWhitespace: Schema.optionalKey(Schema.Boolean),9 ignoreWhitespace: Schema.optionalKey(Schema.Boolean),
10 file: Schema.optionalKey(
11 Schema.Struct({
12 path: Schema.NonEmptyString,
13 previousPath: Schema.NullOr(Schema.NonEmptyString),
14 sourceKind: Schema.Literals(["working-tree", "branch-range"]),
15 }),
16 ),
10});17});
11export type ReviewDiffPreviewInput = typeof ReviewDiffPreviewInput.Type;18export type ReviewDiffPreviewInput = typeof ReviewDiffPreviewInput.Type;
1219
13export const ReviewDiffPreviewSourceKind = Schema.Literals(["working-tree", "branch-range"]);20export const ReviewDiffPreviewSourceKind = Schema.Literals(["working-tree", "branch-range"]);
14export type ReviewDiffPreviewSourceKind = typeof ReviewDiffPreviewSourceKind.Type;21export type ReviewDiffPreviewSourceKind = typeof ReviewDiffPreviewSourceKind.Type;
1522
23export const ReviewDiffFileStat = Schema.Struct({
24 path: Schema.String,
25 previousPath: Schema.NullOr(Schema.String),
26 additions: Schema.Number,
27 deletions: Schema.Number,
28});
29export type ReviewDiffFileStat = typeof ReviewDiffFileStat.Type;
30
16export const ReviewDiffPreviewSource = Schema.Struct({31export const ReviewDiffPreviewSource = Schema.Struct({
17 id: TrimmedNonEmptyString,32 id: TrimmedNonEmptyString,
18 kind: ReviewDiffPreviewSourceKind,33 kind: ReviewDiffPreviewSourceKind,
...@@ -22,6 +37,8 @@ export const ReviewDiffPreviewSource = Schema.Struct({...@@ -22,6 +37,8 @@ export const ReviewDiffPreviewSource = Schema.Struct({
22 diff: Schema.String,37 diff: Schema.String,
23 diffHash: TrimmedNonEmptyString,38 diffHash: TrimmedNonEmptyString,
24 truncated: Schema.Boolean,39 truncated: Schema.Boolean,
40 /** Complete statistics, independent of patch limits. Absent on older servers. */
41 files: Schema.optionalKey(Schema.Array(ReviewDiffFileStat)),
25});42});
26export type ReviewDiffPreviewSource = typeof ReviewDiffPreviewSource.Type;43export type ReviewDiffPreviewSource = typeof ReviewDiffPreviewSource.Type;
2744