mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
15f8d4fe4c
* feat(review): collapse linguist-generated files by default (#1317) Code review now respects linguist-generated (and linguist-generated=true) from .gitattributes, collapsing those diffs by default the way GitHub does. Server (Bun + Pi mirror): a generatedFiles sidecar rides /api/diff and /api/diff/switch, resolved through git's own attribute machinery — one batched 'git check-attr --stdin -z' over the served patch's paths at the review cwd, so stacked and negated rules land exactly as git resolves them. Plain local git sessions only; PR worktrees, workspace multi-repo, jj, GitButler, and P4 omit the sidecar (degrade to no-collapse). Shared logic in packages/shared/generated-files.ts, vendored to Pi. Client: generated files SEED their CodeView item collapsed (the existing Pierre collapse state — same mechanism as commit-diff folding), render the one-line FileHeader bar with a 'generated' tag next to the +/- counts, and expand per file on click. Expansion is session-local App state so it survives remounts and diff switches. Presentation-only: the diff data, annotations, search, and Edit Mode are untouched; the file tree and single-file tabs list generated files normally (tag, no auto-collapse). Guide viewer manifest pin regenerated (AllFilesCodeView/FileHeader are bundled into the guides.show viewer) from a clean frozen-lockfile install. * feat(review): built-in generated defaults, visible collapsed strip, review-round fixes (#1317) Round 2 on PR #1346, per maintainer review. Built-in generated defaults (industry-standard two-layer detection): packages/shared/generated-files.ts (vendored to Pi) now carries DEFAULT_GENERATED_PATTERNS — lockfiles (package-lock.json, yarn.lock, bun.lock, Cargo.lock, go.sum, ...) plus *.min.js / *.min.css / *.map — matched against the path's last segment only. Explicit .gitattributes wins in BOTH directions: linguist-generated (set/true) marks any file, -linguist-generated / =false un-marks even a built-in name, unspecified falls through to the defaults. In plain local git sessions check-attr refines the defaults; the non-git degrade modes (piped patches, PR worktrees, workspace, jj, GitButler, P4) now emit the sidecar from the name-based defaults alone instead of omitting it. Visible collapsed state: a collapsed generated card no longer renders as a bare header — a GeneratedFileNotice strip ('Generated file collapsed', +N/-N, 'Click to view') styled like the other below-header notices sits in the card, and clicking it expands through the SAME reportFileCollapsed funnel as the chevron. Review findings: - F1: search-match and sidebar-comment navigation expanded items without reporting through the funnel, so those expansions died on diff switch. Both now call syncAllCollapsedMirror + reportFileCollapsed; the funnel invariant comment lists the navigation-driven sites. - F2: the check-attr call gets the same 5000ms timeout as review-core's stdin git callers, and Pi's vcs.ts stdin write gets the one-line EPIPE guard (call-flow.ts shape) a timeout kill makes reachable. - F3: removed the dead prevGeneratedRef + collectSetDelta leg — a changed generated set always remounts via fileSetKey, so the delta path was unreachable. Tests: default-list matching (glob + directory-named-bun.lock), both- direction precedence, non-git sidecar from defaults (dual-runtime), the placeholder strip through the funnel, and search expansion surviving a re-seed round-trip. AGENTS.md payload docs updated. Guide viewer manifest pin regenerated from this clean frozen-lockfile worktree.
235 lines
7.4 KiB
TypeScript
235 lines
7.4 KiB
TypeScript
import { afterEach, describe, expect, test } from "bun:test";
|
|
import { spawnSync } from "node:child_process";
|
|
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
|
import { tmpdir } from "node:os";
|
|
import { join, resolve as resolvePath } from "node:path";
|
|
import { readFileSync } from "node:fs";
|
|
import {
|
|
detectGeneratedFiles,
|
|
detectGeneratedFilesByName,
|
|
isDefaultGeneratedPath,
|
|
parseCheckAttrStates,
|
|
} from "./generated-files";
|
|
import type { ReviewGitRuntime } from "./review-core";
|
|
|
|
// Same minimal git harness as review-core.test.ts (per-file test harnesses
|
|
// are this package's style — each suite stays runnable in isolation).
|
|
const tempDirs: string[] = [];
|
|
|
|
function makeTempDir(prefix: string): string {
|
|
const dir = mkdtempSync(join(tmpdir(), prefix));
|
|
tempDirs.push(dir);
|
|
return dir;
|
|
}
|
|
|
|
afterEach(() => {
|
|
while (tempDirs.length > 0) {
|
|
const dir = tempDirs.pop();
|
|
if (dir) rmSync(dir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|
|
function git(cwd: string, args: string[]): string {
|
|
const result = spawnSync("git", args, { cwd, encoding: "utf-8" });
|
|
if (result.status !== 0) {
|
|
throw new Error(result.stderr || `git ${args.join(" ")} failed`);
|
|
}
|
|
return result.stdout.trim();
|
|
}
|
|
|
|
function makeRuntime(baseCwd: string): ReviewGitRuntime {
|
|
return {
|
|
async getFileInfo() {
|
|
return null;
|
|
},
|
|
async readLink() {
|
|
return null;
|
|
},
|
|
async runGit(args: string[], options?: { cwd?: string; stdin?: string }) {
|
|
const result = spawnSync("git", args, {
|
|
cwd: options?.cwd ?? baseCwd,
|
|
encoding: "utf-8",
|
|
input: options?.stdin,
|
|
});
|
|
return {
|
|
stdout: result.stdout ?? "",
|
|
stderr: result.stderr ?? "",
|
|
exitCode: result.status ?? (result.error ? 1 : 0),
|
|
};
|
|
},
|
|
async readTextFile(path: string) {
|
|
try {
|
|
const fullPath = path.startsWith("/") ? path : resolvePath(baseCwd, path);
|
|
return readFileSync(fullPath, "utf-8");
|
|
} catch {
|
|
return null;
|
|
}
|
|
},
|
|
};
|
|
}
|
|
|
|
function initRepo(): string {
|
|
const repoDir = makeTempDir("plannotator-generated-files-");
|
|
git(repoDir, ["init", "-q"]);
|
|
return repoDir;
|
|
}
|
|
|
|
describe("detectGeneratedFiles", () => {
|
|
test("resolves bare, =true, negated, and =false rules exactly as git does", async () => {
|
|
const repo = initRepo();
|
|
writeFileSync(
|
|
join(repo, ".gitattributes"),
|
|
[
|
|
"/CLAUDE.md linguist-generated=true",
|
|
"gen/** linguist-generated",
|
|
// Negated rule stacked after the glob: git must win this resolution,
|
|
// not a naive first-match parser.
|
|
"gen/keep.ts -linguist-generated",
|
|
"docs/api.md linguist-generated=false",
|
|
].join("\n") + "\n",
|
|
);
|
|
mkdirSync(join(repo, "gen"), { recursive: true });
|
|
|
|
const generated = await detectGeneratedFiles(makeRuntime(repo), repo, [
|
|
"CLAUDE.md",
|
|
"gen/schema.sql",
|
|
"gen/keep.ts",
|
|
"docs/api.md",
|
|
"src/app.ts",
|
|
]);
|
|
|
|
expect(generated).toEqual(["CLAUDE.md", "gen/schema.sql"]);
|
|
});
|
|
|
|
test("dedupes paths and preserves diff order in the result", async () => {
|
|
const repo = initRepo();
|
|
writeFileSync(join(repo, ".gitattributes"), "*.lock linguist-generated\n");
|
|
|
|
const generated = await detectGeneratedFiles(makeRuntime(repo), repo, [
|
|
"b.lock",
|
|
"a.lock",
|
|
"b.lock",
|
|
]);
|
|
|
|
expect(generated).toEqual(["b.lock", "a.lock"]);
|
|
});
|
|
|
|
test("handles paths with spaces via NUL-terminated stdin", async () => {
|
|
const repo = initRepo();
|
|
// Quoted-pattern support for spaces in .gitattributes is inconsistent
|
|
// across git versions; a directory rule covers the spaced filename.
|
|
writeFileSync(join(repo, ".gitattributes"), "generated/** linguist-generated\n");
|
|
|
|
const generated = await detectGeneratedFiles(makeRuntime(repo), repo, [
|
|
"generated/weird name.md",
|
|
"src/ok.ts",
|
|
]);
|
|
|
|
expect(generated).toEqual(["generated/weird name.md"]);
|
|
});
|
|
|
|
test("outside a git work tree the name-based defaults still apply", async () => {
|
|
const plainDir = makeTempDir("plannotator-generated-nogit-");
|
|
const generated = await detectGeneratedFiles(makeRuntime(plainDir), plainDir, [
|
|
"a.md",
|
|
"bun.lock",
|
|
]);
|
|
// check-attr fails (not a work tree) — the built-in list stands alone.
|
|
expect(generated).toEqual(["bun.lock"]);
|
|
});
|
|
|
|
test("explicit .gitattributes wins over the built-in list in BOTH directions", async () => {
|
|
const repo = initRepo();
|
|
writeFileSync(
|
|
join(repo, ".gitattributes"),
|
|
[
|
|
// Un-mark a built-in default: the lockfile must render expanded.
|
|
"yarn.lock -linguist-generated",
|
|
"package-lock.json linguist-generated=false",
|
|
// Mark a file the built-in list knows nothing about.
|
|
"src/schema.ts linguist-generated",
|
|
].join("\n") + "\n",
|
|
);
|
|
|
|
const generated = await detectGeneratedFiles(makeRuntime(repo), repo, [
|
|
"yarn.lock",
|
|
"package-lock.json",
|
|
"bun.lock", // unspecified — falls through to the built-in default
|
|
"src/schema.ts",
|
|
"src/app.ts",
|
|
]);
|
|
|
|
expect(generated).toEqual(["bun.lock", "src/schema.ts"]);
|
|
});
|
|
|
|
test("returns empty for an empty path list without spawning git", async () => {
|
|
let spawned = false;
|
|
const runtime: ReviewGitRuntime = {
|
|
async getFileInfo() {
|
|
return null;
|
|
},
|
|
async readLink() {
|
|
return null;
|
|
},
|
|
async runGit() {
|
|
spawned = true;
|
|
return { stdout: "", stderr: "", exitCode: 0 };
|
|
},
|
|
async readTextFile() {
|
|
return null;
|
|
},
|
|
};
|
|
expect(await detectGeneratedFiles(runtime, undefined, ["", ""])).toEqual([]);
|
|
expect(spawned).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe("parseCheckAttrStates", () => {
|
|
test("maps set/true to set, unset/false to unset, everything else to unspecified", () => {
|
|
const stdout = [
|
|
"a.md", "linguist-generated", "true",
|
|
"b.sql", "linguist-generated", "set",
|
|
"c.ts", "linguist-generated", "unset",
|
|
"d.md", "linguist-generated", "false",
|
|
"e.ts", "linguist-generated", "unspecified",
|
|
].join("\0") + "\0";
|
|
expect(parseCheckAttrStates(stdout)).toEqual(
|
|
new Map([
|
|
["a.md", "set"],
|
|
["b.sql", "set"],
|
|
["c.ts", "unset"],
|
|
["d.md", "unset"],
|
|
["e.ts", "unspecified"],
|
|
]),
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("built-in generated defaults", () => {
|
|
test("matches exact lockfile names and *.min/*.map globs on the LAST path segment only", () => {
|
|
// Exact names, at the root and nested.
|
|
expect(isDefaultGeneratedPath("bun.lock")).toBe(true);
|
|
expect(isDefaultGeneratedPath("apps/web/package-lock.json")).toBe(true);
|
|
// Glob suffixes.
|
|
expect(isDefaultGeneratedPath("vendor/app.min.js")).toBe(true);
|
|
expect(isDefaultGeneratedPath("dist/styles.min.css")).toBe(true);
|
|
expect(isDefaultGeneratedPath("dist/app.js.map")).toBe(true);
|
|
// A DIRECTORY named bun.lock never marks the files inside it.
|
|
expect(isDefaultGeneratedPath("bun.lock/README.md")).toBe(false);
|
|
// Ordinary sources stay unmarked.
|
|
expect(isDefaultGeneratedPath("src/app.ts")).toBe(false);
|
|
expect(isDefaultGeneratedPath("src/min.js")).toBe(false);
|
|
});
|
|
|
|
test("detectGeneratedFilesByName dedupes and preserves input order (the non-git sidecar path)", () => {
|
|
expect(
|
|
detectGeneratedFilesByName([
|
|
"folder/yarn.lock",
|
|
"src/app.ts",
|
|
"bun.lock",
|
|
"folder/yarn.lock",
|
|
]),
|
|
).toEqual(["folder/yarn.lock", "bun.lock"]);
|
|
});
|
|
});
|