Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f24e057
fix(review): return complete diff statistics and per-file previews
tris203 Sep 8, 2026
1284623
fix(review): load large diffs progressively in clients
tris203 Sep 8, 2026
37b10ed
fix(web): use shared buttons for diff status actions
tris203 Sep 8, 2026
66230d0
fix(review): preserve loaded patches during background refresh
tris203 Sep 8, 2026
29eb764
test(review): decode file requests through Effect
tris203 Sep 8, 2026
3ae8971
fix(review): collect complete metadata independently of preview limits
tris203 Sep 8, 2026
7230273
fix(review): resolve nested project diffs from the worktree root
tris203 Sep 8, 2026
8a6a05a
fix(web): use shared ghost button interaction colors
tris203 Sep 8, 2026
9190f8e
fix(review): preserve upstream working-tree rename detection
tris203 Sep 12, 2026
bbc0d74
test(mobile): preserve diff excerpts alongside truncation notices
tris203 Sep 13, 2026
de11048
fix(review): extend progressive diffs to pull request code tabs
tris203 Sep 15, 2026
0424865
fix(review): retain oversized Forgejo file notices across CLI paths
tris203 Sep 15, 2026
00c8f63
fix(review): preserve file metadata and bound diff loading work
tris203 Sep 15, 2026
9d18580
fix(review): preserve Forgejo preview error reasons
tris203 Sep 15, 2026
f3afebd
fix(review): include omitted files in completed diff totals
tris203 Sep 15, 2026
b7dd566
fix(review): use the selected thread's repository
tris203 Sep 8, 2026
b9804a9
fix(web): use an icon for diff retry actions
tris203 Sep 15, 2026
571d5a3
fix(review): handle final pages and binary diff previews
tris203 Sep 16, 2026
53e34b4
fix(review): align lazy previews with file metadata
tris203 Sep 16, 2026
413398b
refactor(review): simplify large local diff loading
maria-rcks Sep 16, 2026
4835fbf
perf(review): avoid duplicate refreshes and repeated file processing
maria-rcks Sep 16, 2026
f4f1f2e
fix(review): preserve literal paths and refresh position
maria-rcks Sep 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions apps/mobile/src/features/review/ReviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -382,24 +382,40 @@ export function ReviewSheet(props: ReviewSheetProps) {
useEffect(() => {
showAuxiliaryPane("inspector");
}, [environmentId, showAuxiliaryPane, threadId]);
const { error, reviewSections, selectedSection, refreshSelectedSection, selectSection } =
useReviewSections({
enabled: isEnvironmentReady,
environmentId,
threadId,
reviewCache,
});
const {
error,
reviewSections,
selectedSection,
refreshSelectedSection,
selectSection,
isSelectedSectionPending,
diffPreviewRevision,
} = useReviewSections({
enabled: isEnvironmentReady,
environmentId,
threadId,
reviewCache,
});
useReviewDiffPrewarming({
threadKey: reviewCache.threadKey,
sections: reviewSections,
selectedSectionId: selectedSection?.id ?? null,
});
const { headerDiffSummary, nativeReviewDiffData, parsedDiff, pendingReviewCommentCount } =
useReviewDiffData({
threadKey: reviewCache.threadKey,
selectedSection,
draftMessage,
});
const {
headerDiffSummary,
nativeReviewDiffData,
parsedDiff,
pendingReviewCommentCount,
loadVisibleFile,
isPending: areFilePatchesPending,
} = useReviewDiffData({
threadKey: reviewCache.threadKey,
environmentId,
cwd: selectedThreadCwd,
selectedSection,
revision: diffPreviewRevision,
draftMessage,
});
// Resolution returns null while Expo registers the native view (or forever
// when the binary lacks it). Rendering a null component type crashes the
// app, so callers must fall back — ThreadFeed's ReviewCommentCard does the
Expand Down Expand Up @@ -469,6 +485,7 @@ export function ReviewSheet(props: ReviewSheetProps) {

const handleSelectFile = useCallback(
(fileId: string | null) => {
loadVisibleFile(fileId, true);
commentSelection.clearSelection();
if (fileId !== null && collapsedFileIds.includes(fileId)) {
toggleExpandedFile(fileId);
Expand All @@ -481,13 +498,14 @@ export function ReviewSheet(props: ReviewSheetProps) {
console.error("[review] Failed to navigate to diff file", error);
});
},
[collapsedFileIds, commentSelection, toggleExpandedFile],
[collapsedFileIds, commentSelection, toggleExpandedFile, loadVisibleFile],
);
const handleVisibleFileChange = useCallback(
(event: NativeSyntheticEvent<{ readonly fileId?: string | null }>) => {
loadVisibleFile(event.nativeEvent.fileId ?? null);
reviewFileNavigatorRef.current?.setVisibleFile(event.nativeEvent.fileId ?? null);
},
[],
[loadVisibleFile],
);
const renderInspector = useCallback(
() => (
Expand All @@ -508,10 +526,11 @@ export function ReviewSheet(props: ReviewSheetProps) {
(event: NativeSyntheticEvent<{ readonly fileId?: string }>) => {
const { fileId } = event.nativeEvent;
if (fileId) {
loadVisibleFile(fileId, true);
toggleExpandedFile(fileId);
}
},
[toggleExpandedFile],
[toggleExpandedFile, loadVisibleFile],
);

const handleNativeToggleViewedFile = useCallback(
Expand Down Expand Up @@ -574,12 +593,12 @@ export function ReviewSheet(props: ReviewSheetProps) {
(event: { nativeEvent: { event: string } }) => {
const id = event.nativeEvent.event;
if (id === "refresh") {
void refreshSelectedSection();
void handlePullToRefresh();
} else if (id.startsWith("section:")) {
selectSection(id.slice("section:".length));
}
},
[refreshSelectedSection, selectSection],
[handlePullToRefresh, selectSection],
);
const handleRetryEnvironment = useCallback(() => {
void retryEnvironment(environmentId);
Expand Down Expand Up @@ -818,7 +837,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
<NativeReviewDiffView
collapsable={false}
testID="review-native-diff-view"
refreshing={isPullRefreshing}
refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
onPullToRefresh={() => void handlePullToRefresh()}
style={StyleSheet.absoluteFill}
appearanceScheme={selectedTheme}
Expand Down Expand Up @@ -862,7 +881,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
// iOS has no other refresh affordance here (the explicit
// "Refresh current diff" menu is Android-only).
<RefreshControl
refreshing={isPullRefreshing}
refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
onRefresh={() => void handlePullToRefresh()}
/>
}
Expand Down
23 changes: 23 additions & 0 deletions apps/mobile/src/features/review/nativeReviewDiffAdapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,29 @@ function appTheme(themeId: MobileThemeId, appearance: MobileThemeAppearance) {
}

describe("getCachedNativeReviewDiffData", () => {
it.each([true, false])(
"preserves available diff rows before a notice (has excerpt: %s)",
(hasExcerpt) => {
if (parsedDiff.kind !== "files") throw new Error("Expected a parsed file diff");
const notice = "This file preview was truncated.";
const result = buildNativeReviewDiffData({
parsedDiff: {
...parsedDiff,
files: parsedDiff.files.map((file) => ({
...file,
rows: hasExcerpt ? file.rows : [],
notice,
})),
},
});
const original = buildNativeReviewDiffData({ parsedDiff });
expect(result.rows.slice(0, -1)).toEqual(
hasExcerpt ? original.rows : original.rows.filter((row) => row.kind === "file"),
);
expect(result.rows.at(-1)).toMatchObject({ kind: "notice", text: notice });
},
);

it("reuses the row model for equivalent empty comment arrays", () => {
const first = getCachedNativeReviewDiffData(buildInput([]));
const second = getCachedNativeReviewDiffData(buildInput([]));
Expand Down
32 changes: 23 additions & 9 deletions apps/mobile/src/features/review/nativeReviewDiffAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ interface PreparedNativeReviewFileRows {
readonly filePath: string;
readonly lineCount: number;
readonly rows: ReadonlyArray<NativeReviewDiffRow>;
readonly commentTargetsByRowId: ReadonlyMap<string, NativeReviewDiffCommentTarget>;
readonly rowIdByCommentLineId: ReadonlyMap<string, string>;
commentedRows: {
readonly commentsKey: string;
readonly rows: ReadonlyArray<NativeReviewDiffRow>;
Expand All @@ -136,6 +138,7 @@ interface PreparedNativeReviewDiffData extends Omit<NativeReviewDiffData, "rows"
}

const nativeReviewDiffDataCache = new WeakMap<ReviewParsedDiff, CachedNativeReviewDiffData>();
const nativeReviewFileRowsCache = new WeakMap<ReviewRenderableFile, PreparedNativeReviewFileRows>();

function buildReviewCommentsCacheKey(comments: ReadonlyArray<ReviewInlineComment>): string {
if (comments.length === 0) {
Expand Down Expand Up @@ -261,6 +264,7 @@ function createNoticeRow(fileId: string, suffix: string, text: string): NativeRe
}

function noticeRowsForFile(file: ReviewRenderableFile): ReadonlyArray<NativeReviewDiffRow> {
if (file.notice) return [createNoticeRow(file.id, "loading", file.notice)];
if (file.rows.length > 0) {
return [];
}
Comment thread
tris203 marked this conversation as resolved.
Expand Down Expand Up @@ -406,11 +410,11 @@ function mapLineRow(
};
}

function prepareFileRows(
file: ReviewRenderableFile,
commentTargetsByRowId: Map<string, NativeReviewDiffCommentTarget>,
rowIdByCommentLineId: Map<string, string>,
): PreparedNativeReviewFileRows {
function prepareFileRows(file: ReviewRenderableFile): PreparedNativeReviewFileRows {
const cached = nativeReviewFileRowsCache.get(file);
if (cached) return cached;
const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
const rowIdByCommentLineId = new Map<string, string>();
const rows: NativeReviewDiffRow[] = [
{
kind: "file",
Expand Down Expand Up @@ -449,14 +453,18 @@ function prepareFileRows(
});

rows.push(...noticeRowsForFile(file));
return {
const prepared: PreparedNativeReviewFileRows = {
fileId: file.id,
filePath: file.path,
lineCount: lineRows.length,
// Comments must not split the source deletion/addition runs used for word matching.
rows: addNativeWordDiffRanges(rows),
commentTargetsByRowId,
rowIdByCommentLineId,
commentedRows: null,
};
nativeReviewFileRowsCache.set(file, prepared);
return prepared;
}

function insertFileComments(
Expand Down Expand Up @@ -526,9 +534,15 @@ function prepareNativeReviewDiffData(parsedDiff: ReviewParsedDiff): PreparedNati
}));
const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
const rowIdByCommentLineId = new Map<string, string>();
const fileRows = parsedDiff.files.map((file) =>
prepareFileRows(file, commentTargetsByRowId, rowIdByCommentLineId),
);
const fileRows = parsedDiff.files.map(prepareFileRows);
for (const file of fileRows) {
for (const [rowId, target] of file.commentTargetsByRowId) {
commentTargetsByRowId.set(rowId, target);
}
for (const [lineId, rowId] of file.rowIdByCommentLineId) {
rowIdByCommentLineId.set(lineId, rowId);
}
}

return {
fileRows,
Expand Down
61 changes: 61 additions & 0 deletions apps/mobile/src/features/review/reviewModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "@t3tools/contracts";

import {
applyReviewDiffMetadata,
buildReviewParsedDiff,
buildReviewSectionItems,
getDefaultReviewSectionId,
Expand Down Expand Up @@ -135,6 +136,35 @@ describe("buildReviewSectionItems", () => {
});

describe("buildReviewParsedDiff", () => {
it.each([
["a/example.ts", "a/example.ts"],
["b/example.ts", "b/example.ts"],
["a/before.ts", "b/after.ts"],
])("preserves repository paths from %s to %s", (previousPath, path) => {
const parsed = buildReviewParsedDiff(
[
`diff --git a/${previousPath} b/${path}`,
...(previousPath === path
? []
: ["similarity index 50%", `rename from ${previousPath}`, `rename to ${path}`]),
`--- a/${previousPath}`,
`+++ b/${path}`,
"@@ -1 +1 @@",
"-before",
"+after",
].join("\n"),
"repository-paths",
);
expect(parsed.kind).toBe("files");
if (parsed.kind !== "files") return;
expect(parsed.files[0]).toMatchObject({
path,
previousPath: previousPath === path ? null : previousPath,
additions: 1,
deletions: 1,
});
});

it("builds renderable rows from a unified patch", () => {
const parsed = buildReviewParsedDiff(
[
Expand Down Expand Up @@ -271,3 +301,34 @@ describe("buildReviewParsedDiff", () => {
});
});
});

describe("applyReviewDiffMetadata", () => {
it("uses complete counts even when the preview contains only part of one file", () => {
const parsed = buildReviewParsedDiff(
[
"diff --git a/large.txt b/large.txt",
"--- a/large.txt",
"+++ b/large.txt",
"@@ -1 +1 @@",
"-before",
"+after",
].join("\n"),
"partial",
);
const result = applyReviewDiffMetadata(parsed, {
truncated: true,
files: [
{ path: "large.txt", previousPath: null, additions: 4000, deletions: 3000 },
{ path: "unseen.txt", previousPath: null, additions: 100, deletions: 20 },
],
});
expect(result.kind).toBe("files");
if (result.kind !== "files") return;
expect(result.fileCount).toBe(2);
expect(result.additions).toBe(4100);
expect(result.deletions).toBe(3020);
expect(result.files[0]?.additions).toBe(4000);
expect(result.notice).toContain("Counts include all changes");
expect(applyReviewDiffMetadata(parsed, null)).toEqual(parsed);
});
});
45 changes: 33 additions & 12 deletions apps/mobile/src/features/review/reviewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ export interface ReviewSectionItem {
readonly subtitle: string | null;
readonly diff: string | null;
readonly isLoading: boolean;
readonly files?: ReviewDiffPreviewSource["files"];
readonly truncated?: boolean;
readonly source?: ReviewDiffPreviewSource;
}

export interface ReviewRenderableHunkRow {
Expand Down Expand Up @@ -47,6 +50,7 @@ export type ReviewRenderableRow = ReviewRenderableHunkRow | ReviewRenderableLine
export interface ReviewRenderableFile {
readonly id: string;
readonly cacheKey: string;
readonly notice?: string;
readonly path: string;
readonly previousPath: string | null;
readonly changeType: ChangeTypes;
Expand Down Expand Up @@ -126,16 +130,6 @@ function gitSubtitle(section: ReviewDiffPreviewSource): string | null {
return "Base branch unavailable";
}

function stripGitPrefix(pathValue: string | undefined): string | null {
if (!pathValue) {
return null;
}
if (pathValue.startsWith("a/") || pathValue.startsWith("b/")) {
return pathValue.slice(2);
}
return pathValue;
}

function stripTrailingNewline(value: string): string {
return value.endsWith("\n") ? value.slice(0, -1) : value;
}
Expand Down Expand Up @@ -378,8 +372,8 @@ function buildRenderableRows(file: FileDiffMetadata): ReadonlyArray<ReviewRender
}

function mapRenderableFile(file: FileDiffMetadata): ReviewRenderableFile {
const path = stripGitPrefix(file.name) ?? stripGitPrefix(file.prevName) ?? file.name;
const previousPath = stripGitPrefix(file.prevName);
const path = file.name || file.prevName || "";
const previousPath = file.prevName || null;
const additions = file.hunks.reduce((total, hunk) => total + hunk.additionLines, 0);
const deletions = file.hunks.reduce((total, hunk) => total + hunk.deletionLines, 0);
const cacheKey = file.cacheKey ?? `${previousPath ?? "none"}:${path}:${file.type}`;
Expand Down Expand Up @@ -442,6 +436,9 @@ export function buildReviewSectionItems(input: {
title: section.title,
subtitle: gitSubtitle(section),
diff: section.diff,
source: section,
...(section.files ? { files: section.files } : {}),
truncated: section.truncated,
isLoading: false,
}));
const hasDirtyWorktreeItem = gitItems.some((item) => item.id === DIRTY_WORKTREE_SECTION_ID);
Expand Down Expand Up @@ -527,3 +524,27 @@ export function buildReviewParsedDiff(
};
}
}

export function applyReviewDiffMetadata(
previewDiff: ReviewParsedDiff,
selectedSection: Pick<ReviewSectionItem, "files" | "truncated"> | null,
): ReviewParsedDiff {
if (previewDiff.kind === "empty") return previewDiff;
const notice = selectedSection?.truncated
? `This preview exceeds the size limit. Changes shown are incomplete.${selectedSection.files ? " Counts include all changes." : ""}`
: previewDiff.notice;
if (previewDiff.kind !== "files" || !selectedSection?.files) return { ...previewDiff, notice };
const totals = selectedSection.files.reduce(
(total, file) => ({
additions: total.additions + file.additions,
deletions: total.deletions + file.deletions,
}),
{ additions: 0, deletions: 0 },
);
const stats = new Map(selectedSection.files.map((file) => [file.path, file]));
const files = previewDiff.files.map((file) => {
const stat = stats.get(file.path);
return stat ? { ...file, additions: stat.additions, deletions: stat.deletions } : file;
});
return { ...previewDiff, ...totals, files, fileCount: selectedSection.files.length, notice };
}
Loading
Loading