Skip to content

Commit 08296dd

Browse files
committed
refactor: use ignore package instead of picomatch for .stageignore
Per review feedback: the ignore package implements gitignore semantics natively (comments, blank lines, negation, anchoring), so the hand-rolled compile/match layer is no longer needed. - Replace picomatch + @types/picomatch with ignore@^7.0.5 - Rename loadStageIgnorePatterns -> loadStageIgnore (returns Ignore | null) - Drop CompiledPattern, compileIgnorePatterns, isIgnoredByPatterns - Update prep.ts and show.ts callers - Update tests to construct Ignore instances directly; add empty-file test - README: clarify ".gitignore-style patterns" instead of "glob patterns" Minor behavior change: leading whitespace on a pattern is no longer stripped (the previous code did line.trim()). This aligns with the gitignore spec, which strips only trailing whitespace.
1 parent cdb1c80 commit 08296dd

7 files changed

Lines changed: 65 additions & 103 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Examples:
7979

8080
### `.stageignore`
8181

82-
Add a `.stageignore` file to your repo root to exclude files from the diff analysis. Uses glob patterns, one per line:
82+
Add a `.stageignore` file to your repo root to exclude files from the diff analysis. Uses `.gitignore`-style patterns, one per line:
8383

8484
```
8585
# Build artifacts

packages/cli/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,15 @@
4949
"better-sqlite3": "^12.9.0",
5050
"commander": "^14.0.3",
5151
"drizzle-orm": "^0.45.2",
52+
"ignore": "^7.0.5",
5253
"open": "^11.0.0",
5354
"parse-diff": "^0.11.1",
54-
"picomatch": "^4.0.4",
5555
"zod": "^4.3.6"
5656
},
5757
"devDependencies": {
5858
"@stagereview/types": "workspace:*",
5959
"@types/better-sqlite3": "^7.6.13",
6060
"@types/node": "^25.6.0",
61-
"@types/picomatch": "^4.0.3",
6261
"drizzle-kit": "^0.31.10",
6362
"tsdown": "^0.21.10",
6463
"typescript": "^5.6.3",

packages/cli/src/__tests__/filter-files.test.ts

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ import { tmpdir } from "node:os";
33
import path from "node:path";
44
import type { Hunk, PullRequestFile } from "@stagereview/types/parsed-diff";
55
import { LINE_TYPE } from "@stagereview/types/parsed-diff";
6+
import ignore from "ignore";
67
import { describe, expect, it } from "vitest";
7-
import { filterFilesForLlm, loadStageIgnorePatterns, shouldIncludeFile } from "../filter-files.js";
8+
import { filterFilesForLlm, loadStageIgnore, shouldIncludeFile } from "../filter-files.js";
89

910
function makeHunk(lineCount: number, overrides?: Partial<Hunk>): Hunk {
1011
return {
@@ -35,6 +36,10 @@ function makeFile(overrides?: Partial<PullRequestFile>): PullRequestFile {
3536
};
3637
}
3738

39+
function ig(patterns: string[]) {
40+
return ignore().add(patterns);
41+
}
42+
3843
describe("shouldIncludeFile", () => {
3944
const denylistedFilenames = [
4045
"package-lock.json",
@@ -144,7 +149,7 @@ describe("filterFilesForLlm", () => {
144149
makeFile({ path: "build/config.gypi" }),
145150
makeFile({ path: "dist/bundle.js" }),
146151
];
147-
const result = filterFilesForLlm(files, ["build/**", "dist/**"]);
152+
const result = filterFilesForLlm(files, ig(["build/**", "dist/**"]));
148153
expect(result.files).toHaveLength(1);
149154
expect(result.files[0]?.path).toBe("src/app.ts");
150155
expect(result.excludedByPath).toEqual(["build/config.gypi", "dist/bundle.js"]);
@@ -156,32 +161,32 @@ describe("filterFilesForLlm", () => {
156161
makeFile({ path: "pnpm-lock.yaml" }),
157162
makeFile({ path: "generated/schema.ts" }),
158163
];
159-
const result = filterFilesForLlm(files, ["generated/**"]);
164+
const result = filterFilesForLlm(files, ig(["generated/**"]));
160165
expect(result.files).toHaveLength(1);
161166
expect(result.files[0]?.path).toBe("src/app.ts");
162167
expect(result.excludedByPath).toEqual(["pnpm-lock.yaml", "generated/schema.ts"]);
163168
});
164169

165-
it("works normally when stageIgnorePatterns is undefined", () => {
170+
it("works normally when stageIgnore is undefined", () => {
166171
const files = [makeFile({ path: "src/app.ts" }), makeFile({ path: "pnpm-lock.yaml" })];
167172
const result = filterFilesForLlm(files, undefined);
168173
expect(result.files).toHaveLength(1);
169174
expect(result.files[0]?.path).toBe("src/app.ts");
170175
});
171176

172-
it("works normally when stageIgnorePatterns is empty", () => {
177+
it("works normally when stageIgnore is null", () => {
173178
const files = [makeFile({ path: "src/app.ts" }), makeFile({ path: "src/utils.ts" })];
174-
const result = filterFilesForLlm(files, []);
179+
const result = filterFilesForLlm(files, null);
175180
expect(result.files).toHaveLength(2);
176181
});
177182

178-
it("matches slashless globs against nested paths via matchBase", () => {
183+
it("slashless globs match nested paths", () => {
179184
const files = [
180185
makeFile({ path: "src/app.ts" }),
181186
makeFile({ path: "src/schema.generated.ts" }),
182187
makeFile({ path: "lib/deep/nested/types.generated.ts" }),
183188
];
184-
const result = filterFilesForLlm(files, ["*.generated.ts"]);
189+
const result = filterFilesForLlm(files, ig(["*.generated.ts"]));
185190
expect(result.files).toHaveLength(1);
186191
expect(result.files[0]?.path).toBe("src/app.ts");
187192
});
@@ -192,35 +197,34 @@ describe("filterFilesForLlm", () => {
192197
makeFile({ path: "build/important.js" }),
193198
makeFile({ path: "src/app.ts" }),
194199
];
195-
const result = filterFilesForLlm(files, ["build/**", "!build/important.js"]);
200+
const result = filterFilesForLlm(files, ig(["build/**", "!build/important.js"]));
196201
expect(result.files).toHaveLength(2);
197202
expect(result.files.map((f) => f.path)).toEqual(["build/important.js", "src/app.ts"]);
198203
});
199204

200205
it("last matching pattern wins with negation", () => {
201206
const files = [makeFile({ path: "dist/bundle.js" })];
202-
// exclude, re-include, exclude again
203-
const result = filterFilesForLlm(files, ["dist/**", "!dist/bundle.js", "*.js"]);
207+
const result = filterFilesForLlm(files, ig(["dist/**", "!dist/bundle.js", "*.js"]));
204208
expect(result.files).toHaveLength(0);
205209
});
206210

207-
it("strips leading slash from root-anchored patterns", () => {
211+
it("leading slash anchors a pattern to the repo root", () => {
208212
const files = [makeFile({ path: "dist/bundle.js" }), makeFile({ path: "src/app.ts" })];
209-
const result = filterFilesForLlm(files, ["/dist/**"]);
213+
const result = filterFilesForLlm(files, ig(["/dist/**"]));
210214
expect(result.files).toHaveLength(1);
211215
expect(result.files[0]?.path).toBe("src/app.ts");
212216
});
213217

214218
it("root-anchored pattern does not match nested paths", () => {
215219
const files = [makeFile({ path: "foo/bar.js" }), makeFile({ path: "src/foo/bar.js" })];
216-
const result = filterFilesForLlm(files, ["/foo/**"]);
220+
const result = filterFilesForLlm(files, ig(["/foo/**"]));
217221
expect(result.files).toHaveLength(1);
218222
expect(result.files[0]?.path).toBe("src/foo/bar.js");
219223
});
220224

221225
it("trailing slash matches directory contents", () => {
222226
const files = [makeFile({ path: "build/output.js" }), makeFile({ path: "src/app.ts" })];
223-
const result = filterFilesForLlm(files, ["build/"]);
227+
const result = filterFilesForLlm(files, ig(["build/"]));
224228
expect(result.files).toHaveLength(1);
225229
expect(result.files[0]?.path).toBe("src/app.ts");
226230
});
@@ -230,26 +234,30 @@ describe("filterFilesForLlm", () => {
230234
makeFile({ path: "generated/schema.ts" }),
231235
makeFile({ path: "generated/keep-this.ts" }),
232236
];
233-
const result = filterFilesForLlm(files, ["generated/**", "!keep-this.ts"]);
237+
const result = filterFilesForLlm(files, ig(["generated/**", "!keep-this.ts"]));
234238
expect(result.files).toHaveLength(1);
235239
expect(result.files[0]?.path).toBe("generated/keep-this.ts");
236240
});
237241
});
238242

239-
describe("loadStageIgnorePatterns", () => {
243+
describe("loadStageIgnore", () => {
240244
function makeTempDir(): string {
241245
return mkdtempSync(path.join(tmpdir(), "stage-test-"));
242246
}
243247

244-
it("returns empty array when .stageignore does not exist", () => {
248+
it("returns null when .stageignore does not exist", () => {
245249
const dir = makeTempDir();
246-
expect(loadStageIgnorePatterns(dir)).toEqual([]);
250+
expect(loadStageIgnore(dir)).toBeNull();
247251
});
248252

249253
it("parses patterns from .stageignore", () => {
250254
const dir = makeTempDir();
251255
writeFileSync(path.join(dir, ".stageignore"), "build/**\ndist/**\n");
252-
expect(loadStageIgnorePatterns(dir)).toEqual(["build/**", "dist/**"]);
256+
const matcher = loadStageIgnore(dir);
257+
expect(matcher).not.toBeNull();
258+
expect(matcher?.ignores("build/config.gypi")).toBe(true);
259+
expect(matcher?.ignores("dist/bundle.js")).toBe(true);
260+
expect(matcher?.ignores("src/app.ts")).toBe(false);
253261
});
254262

255263
it("ignores comments and blank lines", () => {
@@ -258,12 +266,18 @@ describe("loadStageIgnorePatterns", () => {
258266
path.join(dir, ".stageignore"),
259267
"# Build artifacts\nbuild/**\n\n# Output\ndist/**\n\n",
260268
);
261-
expect(loadStageIgnorePatterns(dir)).toEqual(["build/**", "dist/**"]);
269+
const matcher = loadStageIgnore(dir);
270+
expect(matcher?.ignores("build/config.gypi")).toBe(true);
271+
expect(matcher?.ignores("dist/bundle.js")).toBe(true);
272+
expect(matcher?.ignores("src/app.ts")).toBe(false);
262273
});
263274

264-
it("trims whitespace from patterns", () => {
275+
it("empty .stageignore matches nothing", () => {
265276
const dir = makeTempDir();
266-
writeFileSync(path.join(dir, ".stageignore"), " build/** \n dist/** \n");
267-
expect(loadStageIgnorePatterns(dir)).toEqual(["build/**", "dist/**"]);
277+
writeFileSync(path.join(dir, ".stageignore"), "");
278+
const matcher = loadStageIgnore(dir);
279+
expect(matcher).not.toBeNull();
280+
expect(matcher?.ignores("src/app.ts")).toBe(false);
281+
expect(matcher?.ignores("build/anything.js")).toBe(false);
268282
});
269283
});

packages/cli/src/filter-files.ts

Lines changed: 10 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { existsSync, readFileSync } from "node:fs";
22
import path from "node:path";
33
import type { PullRequestFile } from "@stagereview/types/parsed-diff";
4-
import picomatch from "picomatch";
4+
import ignore, { type Ignore } from "ignore";
55

66
const IGNORED_FILENAMES = new Set([
77
"package-lock.json",
@@ -48,59 +48,15 @@ export function shouldIncludeFile(filePath: string): boolean {
4848
}
4949

5050
/**
51-
* Load exclusion patterns from a `.stageignore` file in the repo root.
52-
* Format follows `.gitignore` conventions: one glob pattern per line,
53-
* blank lines and `#` comments are ignored. Prefix a pattern with `!`
54-
* to negate (re-include) a previously excluded match.
51+
* Load a `.stageignore` file from the repo root into an `Ignore` matcher.
52+
* Returns `null` when the file is absent. Parsing, comments, blank lines,
53+
* negation, and anchoring semantics all follow `.gitignore` via the
54+
* `ignore` package.
5555
*/
56-
export function loadStageIgnorePatterns(repoRoot: string): string[] {
56+
export function loadStageIgnore(repoRoot: string): Ignore | null {
5757
const ignorePath = path.join(repoRoot, ".stageignore");
58-
if (!existsSync(ignorePath)) return [];
59-
const content = readFileSync(ignorePath, "utf8");
60-
return content
61-
.split("\n")
62-
.map((line) => line.trim())
63-
.filter((line) => line !== "" && !line.startsWith("#"));
64-
}
65-
66-
interface CompiledPattern {
67-
negated: boolean;
68-
matcher: picomatch.Matcher;
69-
}
70-
71-
/**
72-
* Pre-compile `.stageignore` patterns into matchers. Patterns without a
73-
* slash use `matchBase` so bare globs like `*.generated.ts` match in
74-
* subdirectories. Patterns with a slash match the full path. Patterns
75-
* prefixed with `!` negate a previous match (`.gitignore` semantics:
76-
* last matching pattern wins).
77-
*/
78-
export function compileIgnorePatterns(patterns: string[]): CompiledPattern[] {
79-
const compiled: CompiledPattern[] = [];
80-
for (const raw of patterns) {
81-
const negated = raw.startsWith("!");
82-
const stripped = negated ? raw.slice(1) : raw;
83-
const rooted = stripped.startsWith("/");
84-
const unrooted = rooted ? stripped.slice(1) : stripped;
85-
const glob = unrooted.endsWith("/") ? `${unrooted}**` : unrooted;
86-
if (!glob) continue;
87-
const useMatchBase = !rooted && !glob.includes("/");
88-
compiled.push({
89-
negated,
90-
matcher: picomatch(glob, { dot: true, matchBase: useMatchBase }),
91-
});
92-
}
93-
return compiled;
94-
}
95-
96-
function isIgnoredByPatterns(filePath: string, compiled: CompiledPattern[]): boolean {
97-
let ignored = false;
98-
for (const { negated, matcher } of compiled) {
99-
if (matcher(filePath)) {
100-
ignored = !negated;
101-
}
102-
}
103-
return ignored;
58+
if (!existsSync(ignorePath)) return null;
59+
return ignore().add(readFileSync(ignorePath, "utf8"));
10460
}
10561

10662
export interface FilterFilesResult {
@@ -110,18 +66,13 @@ export interface FilterFilesResult {
11066

11167
export function filterFilesForLlm(
11268
files: PullRequestFile[],
113-
stageIgnorePatterns?: string[],
69+
stageIgnore?: Ignore | null,
11470
): FilterFilesResult {
115-
const compiled =
116-
stageIgnorePatterns && stageIgnorePatterns.length > 0
117-
? compileIgnorePatterns(stageIgnorePatterns)
118-
: null;
119-
12071
const excludedByPath: string[] = [];
12172
const reviewable: PullRequestFile[] = [];
12273

12374
for (const file of files) {
124-
if (!shouldIncludeFile(file.path) || (compiled && isIgnoredByPatterns(file.path, compiled))) {
75+
if (!shouldIncludeFile(file.path) || stageIgnore?.ignores(file.path)) {
12576
excludedByPath.push(file.path);
12677
continue;
12778
}

packages/cli/src/prep.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { tmpdir } from "node:os";
33
import path from "node:path";
44
import type { Hunk, PullRequestFile } from "@stagereview/types/parsed-diff";
55
import { parseGitDiff } from "./diff-parser.js";
6-
import { filterFilesForLlm, loadStageIgnorePatterns } from "./filter-files.js";
6+
import { filterFilesForLlm, loadStageIgnore } from "./filter-files.js";
77
import { formatHunkDiffWithLineNumbers } from "./format-diff.js";
88
import { getCommitMessages, type ResolveScopeOptions, readRepoRoot, resolveScope } from "./git.js";
99
import type { WorkingTreeRef } from "./schema.js";
@@ -29,8 +29,8 @@ export function runPrep(
2929
const { scope, rawDiff, mergeBaseSha } = resolveScope(options);
3030

3131
const allFiles = parseGitDiff(rawDiff);
32-
const stageIgnorePatterns = loadStageIgnorePatterns(readRepoRoot());
33-
const { files } = filterFilesForLlm(allFiles, stageIgnorePatterns);
32+
const stageIgnore = loadStageIgnore(readRepoRoot());
33+
const { files } = filterFilesForLlm(allFiles, stageIgnore);
3434

3535
const formattedHunks = files
3636
.flatMap((file) => file.hunks.map((hunk) => formatHunkForPrompt(file, hunk)))

packages/cli/src/show.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import open from "open";
44
import { buildOtherChangesChapter } from "./build-other-changes.js";
55
import { closeDb, getDb } from "./db/client.js";
66
import { parseGitDiff } from "./diff-parser.js";
7-
import { filterFilesForLlm, loadStageIgnorePatterns } from "./filter-files.js";
7+
import { filterFilesForLlm, loadStageIgnore } from "./filter-files.js";
88
import { type ResolveScopeOptions, readRepoContext, readRepoRoot, resolveScope } from "./git.js";
99
import { diffRoutes } from "./routes/diff.js";
1010
import { runRoutes } from "./routes/runs.js";
@@ -90,8 +90,8 @@ function assembleChaptersFile(
9090
};
9191
const { scope, rawDiff } = resolveScope(options);
9292
const allFiles = parseGitDiff(rawDiff);
93-
const stageIgnorePatterns = loadStageIgnorePatterns(readRepoRoot());
94-
const { files: filteredFiles, excludedByPath } = filterFilesForLlm(allFiles, stageIgnorePatterns);
93+
const stageIgnore = loadStageIgnore(readRepoRoot());
94+
const { files: filteredFiles, excludedByPath } = filterFilesForLlm(allFiles, stageIgnore);
9595

9696
validateHunkCoverage(filteredFiles, agentOutput.chapters);
9797
const sanitized = sanitizeLineRefs(agentOutput.chapters, filteredFiles);

0 commit comments

Comments
 (0)