diff --git a/apps/mobile/src/features/review/ReviewSheet.tsx b/apps/mobile/src/features/review/ReviewSheet.tsx index a953ca80901fa..777e121c89ead 100644 --- a/apps/mobile/src/features/review/ReviewSheet.tsx +++ b/apps/mobile/src/features/review/ReviewSheet.tsx @@ -6,7 +6,7 @@ import { useNavigation, type StaticScreenProps } from "@react-navigation/native" import { AppActivityIndicator } from "../../components/ui/AppActivityIndicator"; import { AppBar } from "../../components/ui/AppBar"; import type { AppBarMenuItem } from "../../components/ui/AppBar.types"; -import type { ReviewSectionItem } from "./reviewModel"; +import { UNTRACKED_OMITTED_NOTICE, type ReviewSectionItem } from "./reviewModel"; import { useReviewHeaderPresentation } from "./useReviewHeaderPresentation"; import { type Ref, type ReactElement, useImperativeHandle, useRef, useState } from "react"; import { @@ -766,7 +766,9 @@ export function ReviewSheet(props: ReviewSheetProps) { Platform.OS === "android" && "mt-2 text-center", )} > - {selectedSection.subtitle ?? "This diff is empty."} + {selectedSection.source?.untrackedOmitted + ? UNTRACKED_OMITTED_NOTICE + : (selectedSection.subtitle ?? "This diff is empty.")} ) : parsedDiff.kind === "raw" ? ( diff --git a/apps/mobile/src/features/review/reviewModel.ts b/apps/mobile/src/features/review/reviewModel.ts index df3c7c51324b4..960e3a69783ec 100644 --- a/apps/mobile/src/features/review/reviewModel.ts +++ b/apps/mobile/src/features/review/reviewModel.ts @@ -664,14 +664,20 @@ export function buildReviewParsedDiff( } } +export const UNTRACKED_OMITTED_NOTICE = + "Some untracked files are too large to review. They are not shown or counted."; + export function applyReviewDiffMetadata( previewDiff: ReviewParsedDiff, - selectedSection: Pick | null, + selectedSection: Pick | null, ): ReviewParsedDiff { if (previewDiff.kind === "empty") return previewDiff; + const untrackedOmitted = selectedSection?.source?.untrackedOmitted === true; const notice = selectedSection?.truncated - ? `This preview exceeds the size limit. Changes shown are incomplete.${selectedSection.files ? " Counts include all changes." : ""}` - : previewDiff.notice; + ? `This preview exceeds the size limit. Changes shown are incomplete.${selectedSection.files && !untrackedOmitted ? " Counts include all changes." : ""}` + : untrackedOmitted + ? UNTRACKED_OMITTED_NOTICE + : previewDiff.notice; if (previewDiff.kind !== "files" || !selectedSection?.files) return { ...previewDiff, notice }; const totals = selectedSection.files.reduce( (total, file) => ({ diff --git a/apps/mobile/src/features/review/useReviewDiffData.ts b/apps/mobile/src/features/review/useReviewDiffData.ts index d9ec656cbc773..d45ab1ad2524b 100644 --- a/apps/mobile/src/features/review/useReviewDiffData.ts +++ b/apps/mobile/src/features/review/useReviewDiffData.ts @@ -6,6 +6,7 @@ import { getCachedNativeReviewDiffData } from "./nativeReviewDiffAdapter"; import { getCachedReviewParsedDiff } from "./reviewState"; import { applyReviewDiffMetadata, + UNTRACKED_OMITTED_NOTICE, buildReviewParsedDiff, type ReviewParsedDiff, type ReviewRenderableFile, @@ -225,7 +226,7 @@ export function useReviewDiffData(input: { fileCount: files.length, additions: files.reduce((total, file) => total + file.additions, 0), deletions: files.reduce((total, file) => total + file.deletions, 0), - notice: null, + notice: lazySource.untrackedOmitted ? UNTRACKED_OMITTED_NOTICE : null, }; }, [lazySource, previewDiff, selectedSection, patches]); const headerDiffSummary = useMemo( diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index d9a5e7d2b9fde..c26e07834b87b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -2645,6 +2645,42 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("leaves oversized untracked files out of review input", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const fileSystem = yield* FileSystem.FileSystem; + const pathService = yield* Path.Path; + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* writeTextFile(cwd, "README.md", "# test\ntracked edit\n"); + yield* writeTextFile(cwd, "ordinary.txt", "ordinary edit\n"); + yield* writeTextFile(cwd, "large.data", ""); + // Sparse fixture exercises logical size without allocating the input budget. + yield* fileSystem.truncate(pathService.join(cwd, "large.data"), 257 * 1024 * 1024); + if (symlinksSupported) { + yield* fileSystem.symlink("large.data", pathService.join(cwd, "ordinary-link")); + } + + const preview = yield* driver.getReviewDiffPreview({ cwd, metadataOnly: true }); + const source = preview.sources.find((item) => item.kind === "working-tree")!; + assert.isTrue(source.untrackedOmitted); + assert.deepStrictEqual( + source.files?.map((file) => file.path), + symlinksSupported + ? ["README.md", "ordinary-link", "ordinary.txt"] + : ["README.md", "ordinary.txt"], + ); + const status = yield* driver.statusDetailsLocal(cwd, { includeBranchChanges: true }); + assert.isUndefined(status.branchChanges); + const oversized = yield* driver.getReviewDiffPreview({ + cwd, + file: { path: "large.data", previousPath: null, sourceKind: "working-tree" }, + }); + assert.isTrue(oversized.sources[0]!.untrackedOmitted); + assert.isEmpty(oversized.sources[0]!.diff); + }), + ); + it.effect("reviews untracked files outside the sparse-checkout cone", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 4dabf3b493ebe..a5b498bf63981 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -76,6 +76,8 @@ const REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES = 120_000; const REVIEW_METADATA_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; // Temporary-index git add scales poorly (50k files took ~30 s); report incomplete totals above this cap. const REVIEW_UNTRACKED_MAX_FILES = 5_000; +// git diff --numstat reads ~5 ms/MB of untracked text; larger files are left out of the review index. +const REVIEW_UNTRACKED_MAX_TOTAL_BYTES = 256n * 1024n * 1024n; const REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES = 1024 * 1024; const REVIEW_BRANCH_COMMIT_LIMIT = 100; const REVIEW_BRANCH_SUBJECT_LIMIT = 300; @@ -3169,9 +3171,23 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* (candidate) => onlyPath === undefined || candidate === onlyPath || candidate === `${onlyPath}/`, ); - if (paths.length === 0) return { env: undefined }; + if (paths.length === 0) return { env: undefined, omitted: false }; if (paths.length > REVIEW_UNTRACKED_MAX_FILES) return null; - const env = yield* prepareReviewIndex(cwd, paths).pipe( + const boundedPaths: string[] = []; + let bytes = 0n; + for (const relativePath of paths) { + // lstat reads logical size without following symlinks. + const info = yield* fileSystem + .lstat(path.resolve(cwd, relativePath)) + .pipe(Effect.orElseSucceed(() => undefined)); + const size = info?.type === "File" || info?.type === "SymbolicLink" ? info.size : 0n; + if (bytes + size > REVIEW_UNTRACKED_MAX_TOTAL_BYTES) continue; + bytes += size; + boundedPaths.push(relativePath); + } + const omitted = boundedPaths.length < paths.length; + if (boundedPaths.length === 0) return { env: undefined, omitted }; + const env = yield* prepareReviewIndex(cwd, boundedPaths).pipe( Effect.catchTags({ PlatformError: (cause) => Effect.fail( @@ -3185,7 +3201,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ), }), ); - return { env }; + return { env, omitted }; }); const resolveReviewMergeBase = Effect.fn("resolveReviewMergeBase")(function* ( @@ -3262,12 +3278,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* : []; // Empty status can mean untracked files are hidden; retain the full scan. const untracked = yield* prepareUntrackedReviewIndex(cwd, pathspecs); - if (untracked === null) { + if (untracked === null || untracked.omitted) { return yield* new GitCommandError({ operation: "GitVcsDriver.readBranchChangeTotals", command: "git ls-files", cwd, - detail: "Too many untracked files to count.", + detail: "Untracked files exceed the review input limits.", }); } const readNumstat = (ref: string) => @@ -3594,9 +3610,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ); return { ...patch, files: stat.files }; }); - const [dirtyTrackedResult, baseResult] = yield* Effect.gen(function* () { + const [dirtyTrackedResult, baseResult, untrackedOmitted] = yield* Effect.gen(function* () { const untracked = input.trackedOnly - ? { env: undefined } + ? { env: undefined, omitted: false } : yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path); const metadataOnly = input.metadataOnly === true && !input.file && untracked !== null; const [dirty, base] = @@ -3611,10 +3627,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ], { concurrency: 2 }, ); - if (untracked !== null) return [dirty, base] as const; + if (untracked !== null) return [dirty, base, untracked.omitted] as const; const incomplete = (ref: string | null, result: typeof dirty) => ref === null ? result : { ...result, files: undefined, stdoutTruncated: true }; - return [incomplete(dirtyRef, dirty), incomplete(review.mergeBase, base)] as const; + return [incomplete(dirtyRef, dirty), incomplete(review.mergeBase, base), false] as const; }).pipe(Effect.scoped); const dirtyFiles = dirtyTrackedResult.files; const baseFiles = baseResult.files; @@ -3636,6 +3652,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...(dirtyFiles === undefined ? {} : { files: dirtyFiles }), diffHash: dirtyDiffHash, truncated: dirtyTrackedResult.stdoutTruncated, + ...(untrackedOmitted ? { untrackedOmitted } : {}), ...(input.metadataOnly && !input.file && dirtyFiles !== undefined ? { patchesDeferred: true } : {}), @@ -3650,6 +3667,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...(baseFiles === undefined ? {} : { files: baseFiles }), diffHash: baseDiffHash, truncated: baseResult.stdoutTruncated, + ...(untrackedOmitted ? { untrackedOmitted } : {}), ...(input.metadataOnly && !input.file && baseFiles !== undefined ? { patchesDeferred: true } : {}), diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index e7e6d614ef77f..f5d9a01775fc2 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -1308,10 +1308,18 @@ export default function DiffPanel({ : "Showing tracked changes. Checking untracked files..."}

)} + {isGitSelection && selectedGitSource?.untrackedOmitted && ( +

+ Some untracked files are too large to review. They are not shown or counted. +

+ )} {isSelectedPatchTruncated && !lazySource && (

This preview exceeds the size limit. Changes shown are incomplete. - {isGitSelection && selectedGitSource?.files && !preview?.incomplete + {isGitSelection && + selectedGitSource?.files && + !preview?.incomplete && + !selectedGitSource.untrackedOmitted ? " Totals include all changes." : ""}

diff --git a/packages/contracts/src/review.ts b/packages/contracts/src/review.ts index 2608e635ba906..7d5d082ef8b11 100644 --- a/packages/contracts/src/review.ts +++ b/packages/contracts/src/review.ts @@ -65,6 +65,8 @@ export const ReviewDiffPreviewSource = Schema.Struct({ diffHash: TrimmedNonEmptyString, truncated: Schema.Boolean, patchesDeferred: Schema.optionalKey(Schema.Boolean), + // Untracked files past the review size budget are left out of the diff, files and totals. + untrackedOmitted: Schema.optionalKey(Schema.Boolean), files: Schema.optionalKey(Schema.Array(ReviewDiffFileStat)), }); export type ReviewDiffPreviewSource = typeof ReviewDiffPreviewSource.Type;