From 038d0d21de4d17213a66b6dce46f3fc55e64c2b4 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 19 Aug 2025 19:36:33 +0000 Subject: [PATCH] feat: optimize large file scanning by pre-filtering git-lfs patterns - Modified getLargeFileAutoExcludePatterns to accept git-lfs patterns as parameter - Use ripgrep exclusion flags to pre-filter git-lfs managed files before size checking - This avoids unnecessary file system operations on already-ignored files - Added comprehensive unit tests for the optimization As suggested by @adamhill, this leverages the existing git-lfs filter to improve performance --- .../checkpoints/__tests__/excludes.spec.ts | 114 +++++++++++++++++- src/services/checkpoints/excludes.ts | 44 ++++++- 2 files changed, 153 insertions(+), 5 deletions(-) diff --git a/src/services/checkpoints/__tests__/excludes.spec.ts b/src/services/checkpoints/__tests__/excludes.spec.ts index 923b3d478e..f194fa68b5 100644 --- a/src/services/checkpoints/__tests__/excludes.spec.ts +++ b/src/services/checkpoints/__tests__/excludes.spec.ts @@ -3,12 +3,14 @@ import { join } from "path" import fs from "fs/promises" import { fileExistsAtPath } from "../../../utils/fs" -import { getExcludePatterns } from "../excludes" +import { getExcludePatterns, getExcludePatternsWithStats } from "../excludes" +import { executeRipgrep } from "../../search/file-search" // Mock fs/promises vi.mock("fs/promises", () => ({ default: { readFile: vi.fn(), + stat: vi.fn(), }, })) @@ -17,6 +19,12 @@ vi.mock("../../../utils/fs", () => ({ fileExistsAtPath: vi.fn(), })) +// Mock executeRipgrep +vi.mock("../../search/file-search", () => ({ + executeRipgrep: vi.fn(), + executeRipgrepForFiles: vi.fn(), +})) + describe("getExcludePatterns", () => { const testWorkspacePath = "/test/workspace" @@ -152,4 +160,108 @@ readme.md text expect(excludePatterns).toContain("*.log") // log }) }) + + describe("getLargeFileAutoExcludePatterns with LFS pre-filtering", () => { + it("should pre-filter git-lfs patterns when scanning for large files", async () => { + // Mock .gitattributes file exists with LFS patterns + vi.mocked(fileExistsAtPath).mockResolvedValue(true) + const gitAttributesContent = `*.psd filter=lfs diff=lfs merge=lfs -text +*.zip filter=lfs diff=lfs merge=lfs -text +*.mp4 filter=lfs diff=lfs merge=lfs -text +` + vi.mocked(fs.readFile).mockResolvedValue(gitAttributesContent) + + // Mock executeRipgrep to return some files + vi.mocked(executeRipgrep).mockResolvedValue([ + { path: "file1.txt", type: "file", label: "file1.txt" }, + { path: "large.bin", type: "file", label: "large.bin" }, + { path: "code.js", type: "file", label: "code.js" }, + ]) + + // Mock file stats + vi.mocked(fs.stat).mockImplementation(async (path) => { + const pathStr = path.toString() + if (pathStr.includes("large.bin")) { + return { size: 20 * 1024 * 1024 } as any // 20MB + } + return { size: 1024 } as any // 1KB + }) + + // Get exclude patterns with stats + const result = await getExcludePatternsWithStats(testWorkspacePath) + + // Verify executeRipgrep was called with LFS patterns as exclusions + expect(executeRipgrep).toHaveBeenCalledWith( + expect.objectContaining({ + args: expect.arrayContaining(["-g", "!*.psd", "-g", "!*.zip", "-g", "!*.mp4"]), + workspacePath: testWorkspacePath, + }), + ) + + // Verify large.bin was detected and included + expect(result.stats.largeFilesExcluded).toBe(1) + expect(result.stats.sample).toContain("large.bin") + }) + + it("should handle empty LFS patterns gracefully", async () => { + // Mock no .gitattributes file + vi.mocked(fileExistsAtPath).mockResolvedValue(false) + + // Mock executeRipgrep to return some files + vi.mocked(executeRipgrep).mockResolvedValue([ + { path: "file1.txt", type: "file", label: "file1.txt" }, + { path: "large.bin", type: "file", label: "large.bin" }, + ]) + + // Mock file stats + vi.mocked(fs.stat).mockImplementation(async (path) => { + const pathStr = path.toString() + if (pathStr.includes("large.bin")) { + return { size: 20 * 1024 * 1024 } as any // 20MB + } + return { size: 1024 } as any // 1KB + }) + + // Get exclude patterns with stats + const result = await getExcludePatternsWithStats(testWorkspacePath) + + // Verify executeRipgrep was called without LFS patterns + expect(executeRipgrep).toHaveBeenCalledWith( + expect.objectContaining({ + args: expect.not.arrayContaining(["-g", "!*.psd", "-g", "!*.zip", "-g", "!*.mp4"]), + workspacePath: testWorkspacePath, + }), + ) + + // Verify large file was still detected + expect(result.stats.largeFilesExcluded).toBe(1) + expect(result.stats.sample).toContain("large.bin") + }) + + it("should not exclude code files even if they are large", async () => { + // Mock no .gitattributes file + vi.mocked(fileExistsAtPath).mockResolvedValue(false) + + // Mock executeRipgrep to return some files including large code files + vi.mocked(executeRipgrep).mockResolvedValue([ + { path: "huge.js", type: "file", label: "huge.js" }, + { path: "large.bin", type: "file", label: "large.bin" }, + { path: "big.ts", type: "file", label: "big.ts" }, + ]) + + // Mock file stats - all files are large + vi.mocked(fs.stat).mockImplementation(async () => { + return { size: 20 * 1024 * 1024 } as any // 20MB + }) + + // Get exclude patterns with stats + const result = await getExcludePatternsWithStats(testWorkspacePath) + + // Verify only non-code file was excluded + expect(result.stats.largeFilesExcluded).toBe(1) + expect(result.stats.sample).toContain("large.bin") + expect(result.stats.sample).not.toContain("huge.js") + expect(result.stats.sample).not.toContain("big.ts") + }) + }) }) diff --git a/src/services/checkpoints/excludes.ts b/src/services/checkpoints/excludes.ts index 240b745bd6..6225a5be39 100644 --- a/src/services/checkpoints/excludes.ts +++ b/src/services/checkpoints/excludes.ts @@ -2,7 +2,7 @@ import fs from "fs/promises" import * as path from "path" import { fileExistsAtPath } from "../../utils/fs" -import { executeRipgrepForFiles } from "../search/file-search" +import { executeRipgrepForFiles, executeRipgrep } from "../search/file-search" const DEFAULT_LARGE_FILE_THRESHOLD_BYTES = 10 * 1024 * 1024 // 10 MB @@ -268,14 +268,42 @@ const getGameEnginePatterns = () => [ /** * Scan the workspace for very large non-code files and exclude them automatically. + * Pre-filters out git-lfs managed files to avoid unnecessary file system operations. * Uses ripgrep for fast file listing, then fs.stat for sizes. */ async function getLargeFileAutoExcludePatterns( workspacePath: string, thresholdBytes: number = DEFAULT_LARGE_FILE_THRESHOLD_BYTES, + lfsPatterns: string[] = [], ): Promise { try { - const items = await executeRipgrepForFiles(workspacePath, 50000) + // Create a custom ripgrep execution that excludes git-lfs patterns + const args = [ + "--files", + "--follow", + "--hidden", + "-g", + "!**/node_modules/**", + "-g", + "!**/.git/**", + "-g", + "!**/out/**", + "-g", + "!**/dist/**", + ] + + // Add git-lfs patterns as exclusions to ripgrep + // This pre-filters files before we check their sizes + for (const pattern of lfsPatterns) { + // Convert git-lfs patterns to ripgrep glob patterns + // Git patterns like "*.psd" need to be "!*.psd" for ripgrep + const rgPattern = pattern.startsWith("!") ? pattern.substring(1) : `!${pattern}` + args.push("-g", rgPattern) + } + + args.push(workspacePath) + + const items = await executeRipgrep({ args, workspacePath, limit: 50000 }) const large: string[] = [] for (const item of items) { @@ -311,6 +339,9 @@ export async function getExcludePatternsWithStats(workspacePath: string): Promis patterns: string[] stats: { largeFilesExcluded: number; thresholdBytes: number; sample: string[] } }> { + // Get git-lfs patterns first + const lfsPatterns = await getLfsPatterns(workspacePath) + const base = [ ".git/", ...getBuildArtifactPatterns(), @@ -322,10 +353,15 @@ export async function getExcludePatternsWithStats(workspacePath: string): Promis ...getGeospatialPatterns(), ...getLogFilePatterns(), ...getGameEnginePatterns(), - ...(await getLfsPatterns(workspacePath)), + ...lfsPatterns, ] - const dynamicLarge = await getLargeFileAutoExcludePatterns(workspacePath) + // Pass lfs patterns to the large file scanner to pre-filter them + const dynamicLarge = await getLargeFileAutoExcludePatterns( + workspacePath, + DEFAULT_LARGE_FILE_THRESHOLD_BYTES, + lfsPatterns, + ) const patterns = Array.from(new Set([...base, ...dynamicLarge]))