From 9a066240fde7c662a83fb77699fd843aba358d78 Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:46:24 -0400 Subject: [PATCH 1/3] feat(git)!: replace dugite with tsgit, own the unified-diff rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the bundled dugite git binary in favor of @scolladon/tsgit, a pure-TypeScript git implementation with zero native dependencies — no git binary required on any platform, no more Alpine/musl workaround. tsgit's diff() returns structured DiffChange data, not patch text, so smart-diff now materializes blob content via readBlob and renders its own unified diffs (Myers line-diff + hunk grouping), which also fixes a latent bug: getDiff and getDiffSummary previously derived counts from two separate `git diff` passes (--numstat / --name-status) that could in principle disagree; they now share one blob-diff pass. Path filtering (no pathspec support in tsgit) moved client-side to a predicate over the returned DiffChange[]. getCommits now uses tsgit's log() with all-parent ordering, fixing the previous first-parent-only range bug for free. BREAKING CHANGE: createGitClient is now async and returns a tsgit Repository handle instead of a `{ run(args) }` shell wrapper. buildDiffPathspecs, buildDiffShapingGitArgs, and parseDiffSummary are removed in favor of buildPathFilterPredicate, direct renderer options, and buildFileSummary/mergeFileSummariesByPath/summarizeFiles. Node engine requirement bumped to >=22.22.1 (tsgit's actual minimum). Co-Authored-By: Claude Sonnet 5 --- README.md | 50 +- package-lock.json | 237 +---- package.json | 9 +- src/git/diffGitStatus.ts | 21 +- src/git/diffNameStatusParse.ts | 60 -- src/git/diffNumstatParse.ts | 50 -- .../{diffPathspecs.ts => diffPathFilter.ts} | 42 +- src/git/diffRender.ts | 180 ++++ src/git/diffRenderPrimitives.ts | 155 ++++ src/git/diffShaping.ts | 24 +- src/git/diffSummary.ts | 63 ++ src/git/diffSummaryBuild.ts | 68 -- src/git/diffSummaryParse.ts | 86 -- src/git/gitDiff.ts | 15 +- src/git/gitDiffOps.ts | 322 +++---- src/index.ts | 15 +- test/diffGitStatus.spec.ts | 31 +- test/diffNameStatusParse.spec.ts | 94 -- test/diffNumstat.spec.ts | 119 --- test/diffPathFilter.spec.ts | 113 +++ test/diffRender.spec.ts | 115 +++ test/diffRenderPrimitives.spec.ts | 156 ++++ test/diffShaping.spec.ts | 48 +- test/diffSummary.spec.ts | 202 +++++ test/diffSummaryBuild.spec.ts | 70 -- test/gitDiff.async.spec.ts | 821 ++++++------------ test/gitDiff.spec.ts | 259 ------ test/helpers/tsgitFixture.ts | 61 ++ test/index.spec.ts | 195 +++-- test/summarizeGitDiff.spec.ts | 225 ++--- 30 files changed, 1782 insertions(+), 2124 deletions(-) delete mode 100644 src/git/diffNameStatusParse.ts delete mode 100644 src/git/diffNumstatParse.ts rename src/git/{diffPathspecs.ts => diffPathFilter.ts} (53%) create mode 100644 src/git/diffRender.ts create mode 100644 src/git/diffRenderPrimitives.ts create mode 100644 src/git/diffSummary.ts delete mode 100644 src/git/diffSummaryBuild.ts delete mode 100644 src/git/diffSummaryParse.ts delete mode 100644 test/diffNameStatusParse.spec.ts delete mode 100644 test/diffNumstat.spec.ts create mode 100644 test/diffPathFilter.spec.ts create mode 100644 test/diffRender.spec.ts create mode 100644 test/diffRenderPrimitives.spec.ts create mode 100644 test/diffSummary.spec.ts delete mode 100644 test/diffSummaryBuild.spec.ts create mode 100644 test/helpers/tsgitFixture.ts diff --git a/README.md b/README.md index 7bc5199..de0bef8 100644 --- a/README.md +++ b/README.md @@ -28,25 +28,9 @@ Use any LLM provider supported by the [Vercel AI SDK](https://sdk.vercel.ai) (Op ## Requirements -- **Node.js** 22+ +- **Node.js** 22.22.1+ - An LLM provider credential (see [Provider configuration](#provider-configuration)) -- No git binary required for **glibc** systems - Uses [dugite](https://github.com/desktop/dugite) wich ships its own git binary - -### Alpine Linux / musl libc - -Dugite's bundled git binary is compiled against glibc and will not run on Alpine Linux or other musl-based images. Set the two [dugite execution env vars](https://github.com/desktop/dugite/blob/main/docs/environment-variables.md#execution) to point at your system git instead: - -| Variable | Purpose | -|---|---| -| `LOCAL_GIT_DIRECTORY` | Root of your git installation (the directory containing `bin/git`) | -| `GIT_EXEC_PATH` | Directory containing git's subprograms (set if your distro moves them) | - -```sh -export LOCAL_GIT_DIRECTORY=/usr # uses /usr/bin/git -export GIT_EXEC_PATH=/usr/lib/git-core # only needed if subprograms are non-standard -``` - -Install git in your image first if needed (`apk add git`). No code changes are required — these are first-class dugite env vars. +- No git binary required, on any platform — smart-diff reads the repository directly via [`@scolladon/tsgit`](https://github.com/scolladon/tsgit), a pure-TypeScript git implementation with zero native dependencies. Nothing to install, nothing glibc/musl-sensitive. ## Installation @@ -188,9 +172,9 @@ npx smart-diff --help | Option | CLI flag | Description | |--------|----------|-------------| | `from` / `to` | `` `[to]` / `--from` / `--to` | Git refs for the range; `to` defaults to `HEAD`. | -| `cwd` / `git` | `--cwd ` | Working directory path, or inject your own `GitClient` instance (library only). | +| `cwd` / `git` | `--cwd ` | Working directory path, or inject your own `GitClient` instance (library only; see [Lower-level API](#lower-level-api)). | | `includeFolders` | `--include ` | Limit diff to these paths relative to repo root (omit for full repo minus excludes). | -| `excludeFolders` | `--exclude ` | Excluded paths (git `:(exclude)` pathspecs), e.g. `node_modules`. | +| `excludeFolders` | `--exclude ` | Excluded paths, applied client-side to the changed-path list (directory-prefix match), e.g. `node_modules`. | | `commitMessageIncludeRegexes` | `--commit-include ` | If any pattern is non-empty, only commits whose **full message** matches at least one pattern are kept (after excludes). Case-insensitive. | | `commitMessageExcludeRegexes` | `--commit-exclude ` | Drop commits whose message matches **any** of these patterns. | | `teamName` | `--team ` | Adds a `Team:` line to the user payload for the model. | @@ -298,30 +282,32 @@ const md = await summarizeGitDiff({ ### Diff shape: single range vs per-commit - **Single unified diff** for `from..to` when no commit-message filters apply and the filtered commit list matches the full log for that range. -- **Concatenated per-commit patches** (`^!`) when you use include/exclude regexes or when the filtered commit list differs in length from the full range (so the diff reflects only the commits that remain). +- **Concatenated per-commit patches** (each commit diffed against its first parent, or the empty tree for a root commit) when you use include/exclude regexes or when the filtered commit list differs in length from the full range (so the diff reflects only the commits that remain). ### Lower-level API The package also exports helpers for building a custom pipeline on top of the same git and LLM behavior: -- **Git**: `createGitClient(cwd?, timeout?)`, `getRepoRoot`, `getCommits`, `getDiff`, `getDiffSummary`, `getChangedFiles`, `filterCommitsByMessageRegexes`, `buildDiffPathspecs`, `buildDiffShapingGitArgs`, `shapeUnifiedDiff`, `redactSecrets`, `DEFAULT_NOISE_EXCLUDES`, `DEFAULT_SECRET_PATTERNS` — `timeout` is in milliseconds; omit for no timeout +- **Git**: `createGitClient(cwd?, timeout?)` *(async — returns `Promise`, a live [`@scolladon/tsgit`](https://github.com/scolladon/tsgit) repository handle; call `git.dispose()` when done)*, `getRepoRoot`, `getCommits`, `getDiff`, `getDiffSummary`, `getChangedFiles`, `filterCommitsByMessageRegexes`, `buildPathFilterPredicate`, `matchesAnyPath`, `renderFileDiff`, `renderUnifiedDiff`, `buildFileSummary`, `mergeFileSummariesByPath`, `summarizeFiles`, `shapeUnifiedDiff`, `redactSecrets`, `DEFAULT_NOISE_EXCLUDES`, `DEFAULT_SECRET_PATTERNS` — `timeout` is in milliseconds; omit for no timeout - **AI**: `generateSummary`, `generateSummaryWithUsage`, `resolveLlmMaxDiffChars`, `resolveLlmMaxRetries`, `truncateUnifiedDiffForLlm`, `splitUnifiedDiffIntoFileChunks`, `groupDiffChunksByBudget` - **Provider resolution**: `resolveLanguageModel`, `detectLlmProvider`, `isLlmProviderConfigured`, `defaultModelForProvider`, `resolveLlmBaseUrl`, `parseLlmDefaultHeadersFromEnv` - **Constants**: `DEFAULT_GIT_DIFF_SYSTEM_PROMPT`, `DEFAULT_MAP_REDUCE_MAP_SYSTEM_PROMPT`, `DEFAULT_MAP_REDUCE_REDUCE_SYSTEM_PROMPT`, `LLM_GATEWAY_REQUIRED_MESSAGE` -- **Types**: `LlmProviderId`, `LlmModelProvider`, `ResolveLanguageModelOptions`, `GenerateSummaryInput`, `SummarizeFlags`, `LlmUsageReport`, `DiffFileSummary`, `DiffSummary`, `CommitInfo`, `GitClient`, `GitDiffRangeQuery`, `DiffPathFilter`, `DiffShapingOptions`, `SecretRedactionRule` - - `DiffFileSummary.binary?: boolean` is `true` when git reports `-` for additions/deletions (binary file); absent for text files +- **Types**: `LlmProviderId`, `LlmModelProvider`, `ResolveLanguageModelOptions`, `GenerateSummaryInput`, `SummarizeFlags`, `LlmUsageReport`, `DiffFileSummary`, `DiffSummary`, `CommitInfo`, `GitClient`, `GitDiffRangeQuery`, `DiffPathFilter`, `DiffShapingOptions`, `PathFilterPredicate`, `RenderedFileDiff`, `SecretRedactionRule` + - `DiffFileSummary.binary?: boolean` is `true` when either blob sniffs as binary; absent for text files -## Migrating from 2.x → 3.x +## Migrating from 3.x → 4.x -`@ai-sdk/openai` and `@ai-sdk/openai-compatible` are no longer bundled as direct dependencies. If you use either, add them explicitly: +The git backend moved from shelling out to a bundled [dugite](https://github.com/desktop/dugite) git binary to reading the repository directly via the pure-TypeScript [`@scolladon/tsgit`](https://github.com/scolladon/tsgit) — no git binary, no native dependency, on any platform. -```bash -npm install @ai-sdk/openai -# or -npm install @ai-sdk/openai-compatible -``` +Only the [lower-level API](#lower-level-api) is affected: + +- `createGitClient` is now **async**: `const git = await createGitClient(cwd)`. +- `GitClient` is now a live tsgit `Repository` handle, not a `{ run(args) }` shell wrapper. Call `git.dispose()` when you're done with it (or let it be garbage collected after the process exits). +- `buildDiffPathspecs` is removed. Path filtering is now client-side; use `buildPathFilterPredicate(repoRoot, pathFilter)` to get a `(path: string) => boolean` predicate instead of a git pathspec argument list. +- `buildDiffShapingGitArgs` is removed — `contextLines` and `ignoreWhitespace` are now consumed directly by the diff renderer instead of being turned into `git diff` flags. +- `parseDiffSummary` is removed; build `DiffFileSummary` entries from a `DiffChange` + rendered diff via `buildFileSummary`, and aggregate with `mergeFileSummariesByPath` / `summarizeFiles`. -Everything else — env vars, auto-detection, the public API — is unchanged. +`summarizeGitDiff`, `summarizeGitDiffWithUsage`, and every CLI flag are unchanged. ## Used By diff --git a/package-lock.json b/package-lock.json index 27610a2..5e8f338 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,13 +9,9 @@ "version": "3.4.1", "license": "MIT", "dependencies": { - "@ai-sdk/anthropic": "4.0.36", - "@ai-sdk/cohere": "4.0.25", - "@ai-sdk/deepseek": "3.0.26", - "@ai-sdk/groq": "4.0.26", - "@ai-sdk/mistral": "4.0.27", + "@scolladon/tsgit": "^3.2.1", "ai": "7.0.58", - "dugite": "3.2.2" + "diff": "^9.0.0" }, "bin": { "smart-diff": "dist/cli.cjs" @@ -3582,6 +3578,15 @@ "win32" ] }, + "node_modules/@scolladon/tsgit": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/@scolladon/tsgit/-/tsgit-3.2.1.tgz", + "integrity": "sha512-DskPzL5Ix1zPccIzTIa1Gr6uXPVXFmifQ8k0AD2zxmKus7fIpv3owTe9/ZLuKBOEdTTp0mEj5gSSsMf+zzenuA==", + "license": "MIT", + "engines": { + "node": ">=22.22.1" + } + }, "node_modules/@sec-ant/readable-stream": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/@sec-ant/readable-stream/-/readable-stream-0.4.1.tgz", @@ -4416,20 +4421,6 @@ "integrity": "sha512-/djoAN709iY65ETD6LKCtyyEI04XIBP5xVvfmNxsEP0uJB5tyaGBztSryRr4HqMStr9R06PisQE7m9zDTXKu6g==", "optional": true }, - "node_modules/b4a": { - "version": "1.8.1", - "resolved": "https://registry.npmjs.org/b4a/-/b4a-1.8.1.tgz", - "integrity": "sha512-aiqre1Nr0B/6DgE2N5vwTc+2/oQZ4Wh1t4NznYY4E00y8LCt6NqdRv81so00oo27D8MVKTpUa/MwUUtBLXCoDw==", - "license": "Apache-2.0", - "peerDependencies": { - "react-native-b4a": "*" - }, - "peerDependenciesMeta": { - "react-native-b4a": { - "optional": true - } - } - }, "node_modules/balanced-match": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", @@ -4439,98 +4430,6 @@ "node": "18 || 20 || >=22" } }, - "node_modules/bare-events": { - "version": "2.9.1", - "resolved": "https://registry.npmjs.org/bare-events/-/bare-events-2.9.1.tgz", - "integrity": "sha512-Z0oHEHAFDZkffN8Qc39zNZjQlMDkPJRyyyZieU1VH7u8c5S+qHZ2S8ixdKIAxEjfHO7FJxXmJWgteOghVanIsg==", - "license": "Apache-2.0", - "peerDependencies": { - "bare-abort-controller": "*" - }, - "peerDependenciesMeta": { - "bare-abort-controller": { - "optional": true - } - } - }, - "node_modules/bare-fs": { - "version": "4.7.2", - "resolved": "https://registry.npmjs.org/bare-fs/-/bare-fs-4.7.2.tgz", - "integrity": "sha512-aTvMFUWkBmjzKtEQMDGGDNF8bkfpD5N1b/FCwt7A3wrU4t1o/e/85Wzkluh6JlODCjqVESYCkQCdTXqZ9G7VFg==", - "license": "Apache-2.0", - "dependencies": { - "bare-events": "^2.5.4", - "bare-path": "^3.0.0", - "bare-stream": "^2.6.4", - "bare-url": "^2.2.2", - "fast-fifo": "^1.3.2" - }, - "engines": { - "bare": ">=1.16.0" - }, - "peerDependencies": { - "bare-buffer": "*" - }, - "peerDependenciesMeta": { - "bare-buffer": { - "optional": true - } - } - }, - "node_modules/bare-os": { - "version": "3.9.1", - "resolved": "https://registry.npmjs.org/bare-os/-/bare-os-3.9.1.tgz", - "integrity": "sha512-6M5XjcnsygQNPMCMPXSK379xrJFiZ/AEMNBmFEmQW8d/789VQATvriyi5r0HYTL9TkQ26rn3kgdTG3aisbrXkQ==", - "license": "Apache-2.0", - "engines": { - "bare": ">=1.14.0" - } - }, - "node_modules/bare-path": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/bare-path/-/bare-path-3.0.1.tgz", - "integrity": "sha512-ghj2DSK/2e99a1anTVPCV4m4YIYtrbXhfM7V3D7XZLOTsybnYyaJloymGqssQc8l/or0UoDyRtNQkmkEF/ysgQ==", - "license": "Apache-2.0", - "dependencies": { - "bare-os": "^3.0.1" - } - }, - "node_modules/bare-stream": { - "version": "2.13.3", - "resolved": "https://registry.npmjs.org/bare-stream/-/bare-stream-2.13.3.tgz", - "integrity": "sha512-Kc+brLqvEqGkjyfiwJmImAOqLZL7OsoLKuavx+hJjgVV3nLTOjloJyPMFxjUPerGGHrNH0fLU06jjykMLWrERQ==", - "license": "Apache-2.0", - "dependencies": { - "b4a": "^1.8.1", - "streamx": "^2.25.0", - "teex": "^1.0.1" - }, - "peerDependencies": { - "bare-abort-controller": "*", - "bare-buffer": "*", - "bare-events": "*" - }, - "peerDependenciesMeta": { - "bare-abort-controller": { - "optional": true - }, - "bare-buffer": { - "optional": true - }, - "bare-events": { - "optional": true - } - } - }, - "node_modules/bare-url": { - "version": "2.4.5", - "resolved": "https://registry.npmjs.org/bare-url/-/bare-url-2.4.5.tgz", - "integrity": "sha512-K+y9xF1tN+CdPu4qWwr0QiK1Al07eFPGYK5M2pDXcmHdMdgC/tT/bpmMe1hrmRHaidKLkXrC+cRNYf3XVDUhSQ==", - "license": "Apache-2.0", - "dependencies": { - "bare-path": "^3.0.0" - } - }, "node_modules/baseline-browser-mapping": { "version": "2.10.35", "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.35.tgz", @@ -4562,16 +4461,16 @@ } }, "node_modules/brace-expansion": { - "version": "5.0.7", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.7.tgz", - "integrity": "sha512-7oFy703dxfY3/NLxC1fh2SUCQ0H9rmAY+5EpDVfXjUTTs+HEwR2nYaqLv+GWcTsumwxPfiz6CzCNkwXwBUwqCA==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": "20 || >=22" } }, "node_modules/braces": { @@ -5188,6 +5087,15 @@ "node": ">=8" } }, + "node_modules/diff": { + "version": "9.0.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-9.0.0.tgz", + "integrity": "sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==", + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/diff-match-patch": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/diff-match-patch/-/diff-match-patch-1.0.5.tgz", @@ -5207,20 +5115,6 @@ "node": ">=8" } }, - "node_modules/dugite": { - "version": "3.2.2", - "resolved": "https://registry.npmjs.org/dugite/-/dugite-3.2.2.tgz", - "integrity": "sha512-pGTVaxea0WqauGXF5A3GmpmPOim6oTnfYM6dS6s8nHyJxf4pRTjwtFHkqLkT5de87uUPhTI+LtlmcJQ2WkqeGA==", - "hasInstallScript": true, - "license": "MIT", - "dependencies": { - "progress": "^2.0.3", - "tar-stream": "^3.1.7" - }, - "engines": { - "node": ">= 20" - } - }, "node_modules/dunder-proto": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", @@ -5492,15 +5386,6 @@ "integrity": "sha512-mlsTRyGaPBjPedk6Bvw+aqbsXDtoAyAzm5MO7JgU+yVRyMQ5O8bD4Kcci7BS85f93veegeCPkL8R4GLClnjLFw==", "dev": true }, - "node_modules/events-universal": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/events-universal/-/events-universal-1.0.1.tgz", - "integrity": "sha512-LUd5euvbMLpwOF8m6ivPCbhQeSiYVNb8Vs0fQ8QjXo0JTkEHpz8pxdQf0gStltaPpw0Cca8b39KxvK9cfKRiAw==", - "license": "Apache-2.0", - "dependencies": { - "bare-events": "^2.7.0" - } - }, "node_modules/eventsource-parser": { "version": "3.0.8", "resolved": "https://registry.npmjs.org/eventsource-parser/-/eventsource-parser-3.0.8.tgz", @@ -5567,12 +5452,6 @@ "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", "dev": true }, - "node_modules/fast-fifo": { - "version": "1.3.2", - "resolved": "https://registry.npmjs.org/fast-fifo/-/fast-fifo-1.3.2.tgz", - "integrity": "sha512-/d9sfos4yxzpwkDkuN7k2SqFKtYNmCTzgfEpz82x34IM9/zc8KGxQoXg1liNC/izpRM/MBdt44Nmx41ZWqk+FQ==", - "license": "MIT" - }, "node_modules/fast-string-truncated-width": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/fast-string-truncated-width/-/fast-string-truncated-width-3.0.3.tgz", @@ -5591,9 +5470,9 @@ } }, "node_modules/fast-uri": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", - "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz", + "integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==", "dev": true, "funding": [ { @@ -6857,9 +6736,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz", - "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==", + "version": "4.3.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", + "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", "dev": true, "funding": [ { @@ -7913,9 +7792,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "dev": true, "funding": [ { @@ -8524,9 +8403,9 @@ } }, "node_modules/postcss": { - "version": "8.5.16", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.16.tgz", - "integrity": "sha512-vuwillviilfKZsg0VGj5R/YwwcHx4SLsIOI/7K6mQkWx+l5cUHTjj5g0AasTBcyXsbfTgrwsUNmVUb5xVwyPwg==", + "version": "8.5.26", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.26.tgz", + "integrity": "sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==", "dev": true, "funding": [ { @@ -8544,7 +8423,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.17", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -8606,6 +8485,7 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/progress/-/progress-2.0.3.tgz", "integrity": "sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==", + "dev": true, "license": "MIT", "engines": { "node": ">=0.4.0" @@ -9353,17 +9233,6 @@ "node": ">= 0.4" } }, - "node_modules/streamx": { - "version": "2.28.0", - "resolved": "https://registry.npmjs.org/streamx/-/streamx-2.28.0.tgz", - "integrity": "sha512-1Yowhzjf0ivGMrTIkY9hav5TxobO9qIVqUE41fiCGMGgc3CLlf4MY+9AHmZqBWgDTue0fY9zWjYFVyf6Diuobw==", - "license": "MIT", - "dependencies": { - "events-universal": "^1.0.0", - "fast-fifo": "^1.3.2", - "text-decoder": "^1.1.0" - } - }, "node_modules/string-argv": { "version": "0.3.2", "resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.2.tgz", @@ -9638,18 +9507,6 @@ "node": ">=18" } }, - "node_modules/tar-stream": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/tar-stream/-/tar-stream-3.2.0.tgz", - "integrity": "sha512-ojzvCvVaNp6aOTFmG7jaRD0meowIAuPc3cMMhSgKiVWws1GyHbGd/xvnyuRKcKlMpt3qvxx6r0hreCNITP9hIg==", - "license": "MIT", - "dependencies": { - "b4a": "^1.6.4", - "bare-fs": "^4.5.5", - "fast-fifo": "^1.2.0", - "streamx": "^2.15.0" - } - }, "node_modules/tar/node_modules/yallist": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-5.0.0.tgz", @@ -9660,15 +9517,6 @@ "node": ">=18" } }, - "node_modules/teex": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/teex/-/teex-1.0.1.tgz", - "integrity": "sha512-eYE6iEI62Ni1H8oIa7KlDU6uQBtqr4Eajni3wX7rpfXD8ysFx8z0+dri+KWEPWpBsxXfxu58x/0jvTVT1ekOSg==", - "license": "MIT", - "dependencies": { - "streamx": "^2.12.5" - } - }, "node_modules/terser": { "version": "5.46.1", "resolved": "https://registry.npmjs.org/terser/-/terser-5.46.1.tgz", @@ -9703,15 +9551,6 @@ "source-map": "^0.6.0" } }, - "node_modules/text-decoder": { - "version": "1.2.7", - "resolved": "https://registry.npmjs.org/text-decoder/-/text-decoder-1.2.7.tgz", - "integrity": "sha512-vlLytXkeP4xvEq2otHeJfSQIRyWxo/oZGEbXrtEEF9Hnmrdly59sUbzZ/QgyWuLYHctCHxFF4tRQZNQ9k60ExQ==", - "license": "Apache-2.0", - "dependencies": { - "b4a": "^1.6.4" - } - }, "node_modules/tinybench": { "version": "2.9.0", "resolved": "https://registry.npmjs.org/tinybench/-/tinybench-2.9.0.tgz", diff --git a/package.json b/package.json index 852638c..4a1a549 100644 --- a/package.json +++ b/package.json @@ -11,11 +11,9 @@ "smart-diff": "dist/cli.cjs" }, "engines": { - "node": ">=22" + "node": ">=22.22.1" }, - "files": [ - "dist" - ], + "files": ["dist"], "publishConfig": { "access": "public" }, @@ -56,8 +54,9 @@ "vitest": "4.1.5" }, "dependencies": { + "@scolladon/tsgit": "^3.2.1", "ai": "7.0.58", - "dugite": "3.2.2" + "diff": "^9.0.0" }, "optionalDependencies": { "@ai-sdk/amazon-bedrock": "5.0.50", diff --git a/src/git/diffGitStatus.ts b/src/git/diffGitStatus.ts index 4ea97f8..1691e96 100644 --- a/src/git/diffGitStatus.ts +++ b/src/git/diffGitStatus.ts @@ -1,17 +1,18 @@ +import type { DiffChangeType } from "@scolladon/tsgit"; + import type { DiffStatus } from "./diffTypes.js"; -/** First character of git name-status / synthetic tokens (e.g. R100 → R). */ -const GIT_STATUS_BY_FIRST_CHAR: Record = { - A: "added", - D: "deleted", - R: "renamed", - C: "copied", - T: "type-changed", - M: "modified", +const CHANGE_TYPE_TO_STATUS: Record = { + add: "added", + delete: "deleted", + rename: "renamed", + copy: "copied", + "type-change": "type-changed", + modify: "modified", }; -export function mapGitStatus(statusCode: string): DiffStatus { - return GIT_STATUS_BY_FIRST_CHAR[statusCode.charAt(0)] ?? "unknown"; +export function mapChangeTypeToStatus(type: DiffChangeType): DiffStatus { + return CHANGE_TYPE_TO_STATUS[type]; } export function mergeStatus( diff --git a/src/git/diffNameStatusParse.ts b/src/git/diffNameStatusParse.ts deleted file mode 100644 index 5996986..0000000 --- a/src/git/diffNameStatusParse.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { mapGitStatus, mergeStatus } from "./diffGitStatus.js"; -import type { DiffStatus } from "./diffTypes.js"; - -export type ParsedNameEntry = { - path: string; - status: DiffStatus; - oldPath?: string; -}; - -function parseNameStatusLine(line: string): ParsedNameEntry | null { - const parts = line.split("\t"); - let entry: ParsedNameEntry | null = null; - - if (parts.length >= 2) { - const statusToken = parts[0]!; - const status = mapGitStatus(statusToken); - const isRenameOrCopy = - statusToken.startsWith("R") || statusToken.startsWith("C"); - - if (isRenameOrCopy && parts.length >= 3) { - entry = { path: parts[2]!, status, oldPath: parts[1]! }; - } else if (!isRenameOrCopy) { - entry = { path: parts[1]!, status }; - } - } - - return entry; -} - -export function parseNameStatusLines( - nameStatusOutput: string, -): ParsedNameEntry[] { - const entries: ParsedNameEntry[] = []; - for (const rawLine of nameStatusOutput.split(/\r?\n/)) { - const line = rawLine.trim(); - // Stryker disable next-line ConditionalExpression - if (!line) continue; - const entry = parseNameStatusLine(line); - if (entry) entries.push(entry); - } - return entries; -} - -export function mergeNameEntriesByPath( - entries: ParsedNameEntry[], -): Map { - const byPath = new Map(); - for (const e of entries) { - const existing = byPath.get(e.path); - if (!existing) { - byPath.set(e.path, { ...e }); - } else { - existing.status = mergeStatus(existing.status, e.status); - if (e.oldPath) { - existing.oldPath = existing.oldPath ?? e.oldPath; - } - } - } - return byPath; -} diff --git a/src/git/diffNumstatParse.ts b/src/git/diffNumstatParse.ts deleted file mode 100644 index 49057d6..0000000 --- a/src/git/diffNumstatParse.ts +++ /dev/null @@ -1,50 +0,0 @@ -/** Map numstat path field (including `{old => new}` rename form) to the post-change path used as lookup key. */ -function numStatPathToLookupKey(pathField: string): string { - // Stryker disable next-line Regex - const brace = /^(.*?)\{(.+?) => (.+?)\}(.*)$/.exec(pathField); - if (!brace) { - return pathField; - } - return `${brace[1]}${brace[3].trim()}${brace[4]}`; -} - -function parseNumStatLine( - line: string, -): { key: string; additions: number; deletions: number; binary?: true } | null { - const parts = line.split("\t"); - if (parts.length < 3) return null; - - const addStr = parts[0]!; - const delStr = parts[1]!; - // Stryker disable next-line StringLiteral - const pathField = parts.slice(2).join("\t"); - - const binary = addStr === "-" || delStr === "-" ? (true as const) : undefined; - // Stryker disable next-line ConditionalExpression,StringLiteral - const additions = addStr !== "-" ? Number.parseInt(addStr, 10) || 0 : 0; - // Stryker disable next-line ConditionalExpression,StringLiteral - const deletions = delStr !== "-" ? Number.parseInt(delStr, 10) || 0 : 0; - - const key = numStatPathToLookupKey(pathField); - return { key, additions, deletions, ...(binary ? { binary } : {}) }; -} - -export function accumulateNumStat( - numStatOutput: string, - into: Map, -): void { - for (const rawLine of numStatOutput.split(/\r?\n/)) { - const line = rawLine.trim(); - // Stryker disable next-line ConditionalExpression - if (!line) continue; - const parsed = parseNumStatLine(line); - if (!parsed) continue; - const prev = into.get(parsed.key) ?? { additions: 0, deletions: 0 }; - const binary = parsed.binary || prev.binary || undefined; - into.set(parsed.key, { - additions: prev.additions + parsed.additions, - deletions: prev.deletions + parsed.deletions, - ...(binary ? { binary } : {}), - }); - } -} diff --git a/src/git/diffPathspecs.ts b/src/git/diffPathFilter.ts similarity index 53% rename from src/git/diffPathspecs.ts rename to src/git/diffPathFilter.ts index d2f37eb..04789eb 100644 --- a/src/git/diffPathspecs.ts +++ b/src/git/diffPathFilter.ts @@ -19,14 +19,23 @@ function assertPathUnderRepo(repoRoot: string, userPath: string): void { } } +function isUnderOrEqual(path: string, folder: string): boolean { + return path === folder || path.startsWith(`${folder}/`); +} + +/** Repo-root-relative path predicate produced by {@link buildPathFilterPredicate}. */ +export type PathFilterPredicate = (path: string) => boolean; + /** - * Build git pathspec arguments: include paths plus `:(exclude)…` entries. - * Paths are relative to the repository root using forward slashes, as users see them in the repo tree. + * Build a predicate over repo-root-relative diff paths, mirroring the directory-prefix + * semantics of git `:(exclude)` pathspecs (the only pathspec shape this library ever + * produced — no globs). tsgit's `diff` has no pathspec support, so filtering happens + * client-side over the returned `DiffChange[]` instead of being passed to git. */ -export function buildDiffPathspecs( +export function buildPathFilterPredicate( repoRoot: string, pathFilter?: DiffPathFilter, -): string[] { +): PathFilterPredicate { const includeRaw = pathFilter?.includeFolders?.filter((p) => p.trim().length > 0) ?? []; const excludeRaw = @@ -47,16 +56,17 @@ export function buildDiffPathspecs( assertPathUnderRepo(repoRoot, exc); } - const specs: string[] = []; - if (includes.length === 0) { - specs.push("."); - } else { - for (const inc of includes) { - specs.push(inc); - } - } - for (const exc of excludes) { - specs.push(`:(exclude)${exc}`); - } - return specs; + return (path: string): boolean => { + if (excludes.some((exc) => isUnderOrEqual(path, exc))) return false; + if (includes.length === 0) return true; + return includes.some((inc) => isUnderOrEqual(path, inc)); + }; +} + +/** True when at least one defined candidate path (e.g. a rename's old+new path) passes the predicate. */ +export function matchesAnyPath( + predicate: PathFilterPredicate, + paths: ReadonlyArray, +): boolean { + return paths.some((p) => p !== undefined && predicate(p)); } diff --git a/src/git/diffRender.ts b/src/git/diffRender.ts new file mode 100644 index 0000000..10d31e6 --- /dev/null +++ b/src/git/diffRender.ts @@ -0,0 +1,180 @@ +import type { DiffChange, ObjectId, Repository } from "@scolladon/tsgit"; +import { isBinary } from "@scolladon/tsgit"; + +import { + buildHunks, + computeLineOps, + countLineOps, + formatHunks, +} from "./diffRenderPrimitives.js"; + +const textDecoder = new TextDecoder("utf-8", { fatal: false }); + +export type RenderedFileDiff = { + /** Path used for lookups/display: `newPath` for add/rename/copy, `oldPath` for delete, `path` otherwise. */ + path: string; + oldPath?: string; + newPath?: string; + added: number; + deleted: number; + binary: boolean; + /** Full `diff --git ...` block for this file, including headers. Empty hunks are omitted (e.g. a pure rename). */ + text: string; +}; + +export type ChangePaths = { oldPath?: string; newPath?: string; path: string }; + +/** Repo-relative path(s) touched by a change: the pair for rename/copy, a single path otherwise. */ +export function changePaths(change: DiffChange): ChangePaths { + switch (change.type) { + case "add": + return { newPath: change.newPath, path: change.newPath }; + case "delete": + return { oldPath: change.oldPath, path: change.oldPath }; + case "modify": + case "type-change": + return { oldPath: change.path, newPath: change.path, path: change.path }; + case "rename": + case "copy": + return { + oldPath: change.oldPath, + newPath: change.newPath, + path: change.newPath, + }; + } +} + +function changeIds(change: DiffChange): { + oldId?: ObjectId; + newId?: ObjectId; +} { + switch (change.type) { + case "add": + return { newId: change.newId }; + case "delete": + return { oldId: change.oldId }; + default: + return { oldId: change.oldId, newId: change.newId }; + } +} + +function abbrev(id: ObjectId | undefined): string { + return id ? id.slice(0, 7) : "0000000"; +} + +function similarityPercent(similarity: { + score: number; + maxScore: number; +}): number { + // Stryker disable next-line ConditionalExpression,EqualityOperator + if (similarity.maxScore <= 0) return 0; + return Math.round((similarity.score / similarity.maxScore) * 100); +} + +function buildHeaderLines(change: DiffChange, binary: boolean): string[] { + const { oldPath, newPath, path } = changePaths(change); + const { oldId, newId } = changeIds(change); + const displayOld = oldPath ?? path; + const displayNew = newPath ?? path; + const lines: string[] = [`diff --git a/${displayOld} b/${displayNew}`]; + + if (change.type === "add") { + lines.push(`new file mode ${change.newMode}`); + } else if (change.type === "delete") { + lines.push(`deleted file mode ${change.oldMode}`); + } else if (change.oldMode !== change.newMode) { + lines.push(`old mode ${change.oldMode}`); + lines.push(`new mode ${change.newMode}`); + } + + if (change.type === "rename" || change.type === "copy") { + const label = change.type === "rename" ? "rename" : "copy"; + lines.push(`similarity index ${similarityPercent(change.similarity)}%`); + lines.push(`${label} from ${change.oldPath}`); + lines.push(`${label} to ${change.newPath}`); + } + + const modeSuffix = + change.type === "modify" && change.oldMode === change.newMode + ? ` ${change.newMode}` + : ""; + lines.push(`index ${abbrev(oldId)}..${abbrev(newId)}${modeSuffix}`); + + if (binary) { + lines.push(`Binary files a/${displayOld} and b/${displayNew} differ`); + } else { + lines.push(change.type === "add" ? "--- /dev/null" : `--- a/${displayOld}`); + lines.push( + change.type === "delete" ? "+++ /dev/null" : `+++ b/${displayNew}`, + ); + } + return lines; +} + +async function readBlobBytes( + repo: Repository, + id: ObjectId | undefined, +): Promise { + if (id === undefined) return undefined; + const blob = await repo.primitives.readBlob(id); + return blob.content; +} + +/** Materialize both blob sides of a `DiffChange` and render it as a unified-diff text block. */ +export async function renderFileDiff( + repo: Repository, + change: DiffChange, + contextLines: number, +): Promise { + const { oldPath, newPath, path } = changePaths(change); + const { oldId, newId } = changeIds(change); + + const [oldBytes, newBytes] = await Promise.all([ + readBlobBytes(repo, oldId), + readBlobBytes(repo, newId), + ]); + + const binary = Boolean( + (oldBytes && isBinary(oldBytes)) || (newBytes && isBinary(newBytes)), + ); + const headerLines = buildHeaderLines(change, binary); + + if (binary) { + return { + path, + oldPath, + newPath, + added: 0, + deleted: 0, + binary: true, + text: headerLines.join("\n"), + }; + } + + const oldText = oldBytes ? textDecoder.decode(oldBytes) : ""; + const newText = newBytes ? textDecoder.decode(newBytes) : ""; + const ops = computeLineOps(oldText, newText); + const { added, deleted } = countLineOps(ops); + const body = formatHunks(buildHunks(ops, contextLines)); + + return { + path, + oldPath, + newPath, + added, + deleted, + binary: false, + text: body ? `${headerLines.join("\n")}\n${body}` : headerLines.join("\n"), + }; +} + +/** Render every change in a `TreeDiff` and join into one unified-diff document. */ +export async function renderUnifiedDiff( + repo: Repository, + changes: readonly DiffChange[], + contextLines: number, +): Promise { + return Promise.all( + changes.map((change) => renderFileDiff(repo, change, contextLines)), + ); +} diff --git a/src/git/diffRenderPrimitives.ts b/src/git/diffRenderPrimitives.ts new file mode 100644 index 0000000..0cc702c --- /dev/null +++ b/src/git/diffRenderPrimitives.ts @@ -0,0 +1,155 @@ +import { diffLines } from "diff"; + +export type LineOp = { + kind: "context" | "add" | "del"; + text: string; + /** True when this line is the last line of its file and that file has no trailing newline. */ + noEol: boolean; +}; + +function linesFromValue( + value: string, +): Array<{ text: string; noEol: boolean }> { + const endsWithNewline = value.endsWith("\n"); + const body = endsWithNewline ? value.slice(0, -1) : value; + const lines = body.split("\n"); + return lines.map((text, i) => ({ + text, + noEol: !endsWithNewline && i === lines.length - 1, + })); +} + +/** Turn two file contents into a flat line-level op stream via Myers diff (`diff` package). */ +export function computeLineOps(oldText: string, newText: string): LineOp[] { + const parts = diffLines(oldText, newText); + const ops: LineOp[] = []; + for (const part of parts) { + // Stryker disable next-line ConditionalExpression + const kind = part.added ? "add" : part.removed ? "del" : "context"; + for (const line of linesFromValue(part.value)) { + ops.push({ kind, ...line }); + } + } + return ops; +} + +type PositionedOp = LineOp & { oldNo?: number; newNo?: number }; + +function positionOps(ops: LineOp[]): PositionedOp[] { + const positioned: PositionedOp[] = []; + let oldNo = 1; + let newNo = 1; + for (const op of ops) { + if (op.kind === "context") { + positioned.push({ ...op, oldNo, newNo }); + oldNo++; + newNo++; + } else if (op.kind === "del") { + positioned.push({ ...op, oldNo }); + oldNo++; + } else { + positioned.push({ ...op, newNo }); + newNo++; + } + } + return positioned; +} + +/** + * The line number just before `index`, for a side with zero lines in this hunk. + * `index` is always either 0 (start of file) or the position right after a context + * line (buildHunks' clustering merges any adjacent changed op into the same hunk, + * so a zero-length side can only border a context line or the file start) — so a + * single lookback always resolves it. + */ +function nearestNoBefore( + positioned: PositionedOp[], + index: number, + key: "oldNo" | "newNo", +): number { + if (index === 0) return 0; + return positioned[index - 1]![key]!; +} + +export type Hunk = { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + lines: LineOp[]; +}; + +/** + * Group a line-op stream into unified-diff hunks, `contextLines` of context around + * each change run, merging runs whose gap is within `2 * contextLines`. + */ +export function buildHunks(ops: LineOp[], contextLines: number): Hunk[] { + const context = Math.max(0, Math.trunc(contextLines)); + const positioned = positionOps(ops); + const changedIdx: number[] = []; + positioned.forEach((op, i) => { + if (op.kind !== "context") changedIdx.push(i); + }); + if (changedIdx.length === 0) return []; + + const clusters: Array<[number, number]> = []; + let curStart = changedIdx[0]!; + let curEnd = changedIdx[0]!; + for (let k = 1; k < changedIdx.length; k++) { + const idx = changedIdx[k]!; + if (idx - curEnd - 1 <= context * 2) { + curEnd = idx; + } else { + clusters.push([curStart, curEnd]); + curStart = idx; + curEnd = idx; + } + } + clusters.push([curStart, curEnd]); + + return clusters.map(([start, end]) => { + const from = Math.max(0, start - context); + const to = Math.min(positioned.length - 1, end + context); + const lines = positioned.slice(from, to + 1); + const oldLines = lines.filter((l) => l.kind !== "add").length; + const newLines = lines.filter((l) => l.kind !== "del").length; + const firstOld = lines.find((l) => l.oldNo !== undefined)?.oldNo; + const firstNew = lines.find((l) => l.newNo !== undefined)?.newNo; + const oldStart = + oldLines === 0 ? nearestNoBefore(positioned, from, "oldNo") : firstOld!; + const newStart = + newLines === 0 ? nearestNoBefore(positioned, from, "newNo") : firstNew!; + return { oldStart, oldLines, newStart, newLines, lines }; + }); +} + +/** Render hunks as unified-diff body text (`@@ ... @@` headers plus ` `/`-`/`+` lines). */ +export function formatHunks(hunks: Hunk[]): string { + const out: string[] = []; + for (const hunk of hunks) { + out.push( + `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@`, + ); + for (const line of hunk.lines) { + const prefix = + line.kind === "add" ? "+" : line.kind === "del" ? "-" : " "; + out.push(`${prefix}${line.text}`); + if (line.noEol) out.push("\\ No newline at end of file"); + } + } + return out.join("\n"); +} + +/** Net added/deleted line counts for a line-op stream (the numstat-equivalent data). */ +export function countLineOps(ops: LineOp[]): { + added: number; + deleted: number; +} { + let added = 0; + let deleted = 0; + for (const op of ops) { + if (op.kind === "add") added++; + else if (op.kind === "del") deleted++; + } + return { added, deleted }; +} diff --git a/src/git/diffShaping.ts b/src/git/diffShaping.ts index 2a63edc..002b9a4 100644 --- a/src/git/diffShaping.ts +++ b/src/git/diffShaping.ts @@ -11,14 +11,14 @@ import { */ export type DiffShapingOptions = { /** - * Number of context lines around each change. Passed as `-U` to `git diff`. - * Git defaults to 3; lowering to 0 or 1 commonly saves 30–60% of tokens on - * modification-heavy diffs with little loss of fidelity. + * Number of context lines around each change, fed to the renderer's own hunk + * grouping. Defaults to 3 (git's default); lowering to 0 or 1 commonly saves + * 30–60% of tokens on modification-heavy diffs with little loss of fidelity. */ contextLines?: number; /** - * Pass `-w` / `--ignore-all-space` to `git diff`. Pure-whitespace hunks vanish, - * which is usually what you want when the model is reasoning about behavior. + * Ignore whitespace-only differences when diffing (tsgit's native `ignoreWhitespace: 'all'` + * data mode). A file whose only change is whitespace drops out of the diff entirely. */ ignoreWhitespace?: boolean; /** @@ -82,20 +82,6 @@ function normalizeContextLines(raw: number): number { return Math.trunc(raw); } -/** Build the leading args for `git diff` implied by shaping options. */ -export function buildDiffShapingGitArgs( - shaping?: DiffShapingOptions, -): string[] { - const args: string[] = []; - if (shaping?.contextLines !== undefined) { - args.push(`-U${normalizeContextLines(shaping.contextLines)}`); - } - if (shaping?.ignoreWhitespace) { - args.push("-w"); - } - return args; -} - const PREAMBLE_NOISE_PREFIXES = [ "diff --git ", "index ", diff --git a/src/git/diffSummary.ts b/src/git/diffSummary.ts new file mode 100644 index 0000000..a53d06a --- /dev/null +++ b/src/git/diffSummary.ts @@ -0,0 +1,63 @@ +import type { DiffChange } from "@scolladon/tsgit"; + +import { mapChangeTypeToStatus, mergeStatus } from "./diffGitStatus.js"; +import type { RenderedFileDiff } from "./diffRender.js"; +import type { DiffFileSummary, DiffSummary } from "./diffTypes.js"; + +/** + * Build one file's summary entry directly from its `DiffChange` and rendered blob-diff — + * counts and binary detection come from the same line-diff pass `getDiff` uses, so the + * two can never disagree (unlike git's separate `--numstat`/`--name-status` passes). + */ +export function buildFileSummary( + change: DiffChange, + rendered: RenderedFileDiff, +): DiffFileSummary { + const isRenameOrCopy = change.type === "rename" || change.type === "copy"; + return { + path: rendered.path, + status: mapChangeTypeToStatus(change.type), + additions: rendered.added, + deletions: rendered.deleted, + oldPath: isRenameOrCopy ? rendered.oldPath : undefined, + newPath: isRenameOrCopy ? rendered.newPath : undefined, + binary: rendered.binary ? true : undefined, + }; +} + +/** + * Merge summary entries sharing the same post-change path — needed when `filterByCommits` + * aggregates several commits' changes into one summary and the same file was touched more + * than once (git's own numstat/name-status text has the same duplicate-line shape). + */ +export function mergeFileSummariesByPath( + entries: DiffFileSummary[], +): DiffFileSummary[] { + const byPath = new Map(); + for (const entry of entries) { + const existing = byPath.get(entry.path); + if (!existing) { + byPath.set(entry.path, { ...entry }); + continue; + } + existing.additions += entry.additions; + existing.deletions += entry.deletions; + existing.status = mergeStatus(existing.status, entry.status); + // Stryker disable next-line ConditionalExpression + if (entry.oldPath) existing.oldPath = existing.oldPath ?? entry.oldPath; + // Stryker disable next-line ConditionalExpression + if (entry.newPath) existing.newPath = existing.newPath ?? entry.newPath; + // Stryker disable next-line ConditionalExpression + if (entry.binary) existing.binary = true; + } + return [...byPath.values()]; +} + +export function summarizeFiles(files: DiffFileSummary[]): DiffSummary { + return { + files, + totalFiles: files.length, + totalAdditions: files.reduce((sum, f) => sum + f.additions, 0), + totalDeletions: files.reduce((sum, f) => sum + f.deletions, 0), + }; +} diff --git a/src/git/diffSummaryBuild.ts b/src/git/diffSummaryBuild.ts deleted file mode 100644 index 0e2b518..0000000 --- a/src/git/diffSummaryBuild.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { - mergeNameEntriesByPath, - type ParsedNameEntry, - parseNameStatusLines, -} from "./diffNameStatusParse.js"; -import { accumulateNumStat } from "./diffNumstatParse.js"; -import { parseDiffSummary } from "./diffSummaryParse.js"; -import type { DiffStatus, DiffSummary } from "./diffTypes.js"; - -const STATUS_TO_SYNTHETIC_PREFIX: Record = { - added: "A", - deleted: "D", - renamed: "R100", - copied: "C100", - "type-changed": "T", - modified: "M", - unknown: "X", -}; - -function diffStatusToSyntheticPrefix(status: DiffStatus): string { - return STATUS_TO_SYNTHETIC_PREFIX[status]; -} - -function buildSyntheticDiffLine( - meta: ParsedNameEntry, - counts: { additions: number; deletions: number }, -): string { - const prefix = diffStatusToSyntheticPrefix(meta.status); - if (meta.oldPath) { - return `${prefix}\t${counts.additions}\t${counts.deletions}\t${meta.oldPath}\t${meta.path}`; - } - return `${prefix}\t${counts.additions}\t${counts.deletions}\t${meta.path}`; -} - -/** - * Git does not combine `--numstat` and `--name-status` into one machine-readable line; using both flags - * yields name-status only. We run each mode separately and merge into the compact shape `parseDiffSummary` expects. - */ -export function buildDiffSummaryFromGitOutputs( - nameStatusOutput: string, - numStatOutput: string, -): DiffSummary { - const numMap = new Map< - string, - { additions: number; deletions: number; binary?: boolean } - >(); - accumulateNumStat(numStatOutput, numMap); - - const mergedName = mergeNameEntriesByPath( - parseNameStatusLines(nameStatusOutput), - ); - const syntheticLines: string[] = []; - - for (const [path, meta] of mergedName) { - const counts = numMap.get(path) ?? { additions: 0, deletions: 0 }; - syntheticLines.push(buildSyntheticDiffLine(meta, counts)); - } - - const summary = parseDiffSummary(syntheticLines.join("\n")); - - for (const file of summary.files) { - if (numMap.get(file.path)?.binary) { - file.binary = true; - } - } - - return summary; -} diff --git a/src/git/diffSummaryParse.ts b/src/git/diffSummaryParse.ts deleted file mode 100644 index 5594357..0000000 --- a/src/git/diffSummaryParse.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { mapGitStatus, mergeStatus } from "./diffGitStatus.js"; -import type { DiffFileSummary, DiffStatus, DiffSummary } from "./diffTypes.js"; - -type ParsedDiffSummaryLine = { - status: DiffStatus; - additions: number; - deletions: number; - oldPath?: string; - newPath: string; -}; - -function parseTabDiffSummaryLine(line: string): ParsedDiffSummaryLine | null { - const parts = line.split("\t"); - // Stryker disable next-line ConditionalExpression,EqualityOperator - if (parts.length < 3) return null; - - const statusToken = parts.shift()!; - const status = mapGitStatus(statusToken); - const add0 = parts[0]; - const del0 = parts[1]; - // Stryker disable next-line ConditionalExpression,LogicalOperator,StringLiteral - const additions = add0 && add0 !== "-" ? Number.parseInt(add0, 10) || 0 : 0; - // Stryker disable next-line ConditionalExpression,LogicalOperator,StringLiteral - const deletions = del0 && del0 !== "-" ? Number.parseInt(del0, 10) || 0 : 0; - - if (parts.length === 3) { - return { status, additions, deletions, newPath: parts[2]! }; - } - if (parts.length === 4) { - return { - status, - additions, - deletions, - oldPath: parts[2], - newPath: parts[3]!, - }; - } - return null; -} - -function mergeParsedDiffSummaryLine( - fileMap: Map, - p: ParsedDiffSummaryLine, -): void { - const { newPath, status, additions, deletions, oldPath } = p; - const existing = fileMap.get(newPath); - if (existing) { - existing.additions += additions; - existing.deletions += deletions; - existing.status = mergeStatus(existing.status, status); - // Stryker disable next-line ConditionalExpression - if (oldPath) existing.oldPath = existing.oldPath ?? oldPath; - existing.newPath = existing.newPath ?? newPath; - } else { - fileMap.set(newPath, { - path: newPath, - status, - additions, - deletions, - oldPath, - newPath: oldPath ? newPath : undefined, - }); - } -} - -/** Exported for tests; also used to merge synthetic lines when the same path appears more than once. */ -export function parseDiffSummary(diffOutput: string): DiffSummary { - const fileMap = new Map(); - - for (const rawLine of diffOutput.split(/\r?\n/)) { - const line = rawLine.trim(); - // Stryker disable next-line ConditionalExpression - if (!line) continue; - - const parsed = parseTabDiffSummaryLine(line); - if (parsed) mergeParsedDiffSummaryLine(fileMap, parsed); - } - - const files = Array.from(fileMap.values()); - return { - files, - totalFiles: files.length, - totalAdditions: files.reduce((sum, file) => sum + file.additions, 0), - totalDeletions: files.reduce((sum, file) => sum + file.deletions, 0), - }; -} diff --git a/src/git/gitDiff.ts b/src/git/gitDiff.ts index 3c12e76..c2c3755 100644 --- a/src/git/gitDiff.ts +++ b/src/git/gitDiff.ts @@ -1,15 +1,18 @@ export { filterCommitsByMessageRegexes } from "./commitMessageFilter.js"; -export { buildDiffPathspecs } from "./diffPathspecs.js"; +export type { PathFilterPredicate } from "./diffPathFilter.js"; +export { buildPathFilterPredicate, matchesAnyPath } from "./diffPathFilter.js"; export type { SecretRedactionRule } from "./diffRedaction.js"; export { DEFAULT_SECRET_PATTERNS, redactSecrets } from "./diffRedaction.js"; +export type { RenderedFileDiff } from "./diffRender.js"; +export { renderFileDiff, renderUnifiedDiff } from "./diffRender.js"; export type { DiffShapingOptions } from "./diffShaping.js"; +export { DEFAULT_NOISE_EXCLUDES, shapeUnifiedDiff } from "./diffShaping.js"; export { - buildDiffShapingGitArgs, - DEFAULT_NOISE_EXCLUDES, - shapeUnifiedDiff, -} from "./diffShaping.js"; -export { parseDiffSummary } from "./diffSummaryParse.js"; + buildFileSummary, + mergeFileSummariesByPath, + summarizeFiles, +} from "./diffSummary.js"; export type { CommitInfo, DiffFileSummary, diff --git a/src/git/gitDiffOps.ts b/src/git/gitDiffOps.ts index 0f0c826..ddeacd8 100644 --- a/src/git/gitDiffOps.ts +++ b/src/git/gitDiffOps.ts @@ -1,37 +1,44 @@ -import { exec } from "dugite"; -import { buildDiffPathspecs } from "./diffPathspecs.js"; -import { buildDiffShapingGitArgs, shapeUnifiedDiff } from "./diffShaping.js"; -import { buildDiffSummaryFromGitOutputs } from "./diffSummaryBuild.js"; +import type { DiffChange, ObjectId, Repository } from "@scolladon/tsgit"; +import { openRepository } from "@scolladon/tsgit"; +import { + buildPathFilterPredicate, + matchesAnyPath, + type PathFilterPredicate, +} from "./diffPathFilter.js"; +import { changePaths, renderUnifiedDiff } from "./diffRender.js"; +import { shapeUnifiedDiff } from "./diffShaping.js"; +import { + buildFileSummary, + mergeFileSummariesByPath, + summarizeFiles, +} from "./diffSummary.js"; import type { CommitInfo, - DiffPathFilter, DiffSummary, GitDiffRangeQuery, } from "./diffTypes.js"; -export type GitClient = { - run(args: string[]): Promise; -}; +/** A live `@scolladon/tsgit` repository handle. Dispose with `git.dispose()` when done. */ +export type GitClient = Repository; -export function createGitClient( +/** Git's well-known SHA-1 empty-tree object id; tsgit special-cases it without needing the object stored. */ +const EMPTY_TREE_OID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" as ObjectId; + +/** Matches `git diff`'s own default context, applied when `shaping.contextLines` is unset. */ +const DEFAULT_CONTEXT_LINES = 3; + +export async function createGitClient( cwd = process.cwd(), timeout?: number, -): GitClient { - return { - run: async (args) => { - const result = await exec(args, cwd, { - ...(timeout !== undefined - ? { signal: AbortSignal.timeout(timeout) } - : {}), - }); - if (result.exitCode !== 0) { - throw new Error( - result.stderr || `git exited with code ${result.exitCode}`, - ); - } - return result.stdout; - }, - }; +): Promise { + return openRepository({ + cwd, + ...(timeout !== undefined ? { signal: AbortSignal.timeout(timeout) } : {}), + }); +} + +export async function getRepoRoot(git: GitClient): Promise { + return git.primitives.getRepoRoot(); } export async function getCommits( @@ -39,191 +46,146 @@ export async function getCommits( from: string, to: string, ): Promise { - const output = await git.run(["log", "--format=%H%x1f%s", `${from}..${to}`]); - return output - .split("\n") - .filter(Boolean) - .map((line) => { - const sep = line.indexOf("\x1f"); - return { - hash: sep >= 0 ? line.slice(0, sep) : line, - message: sep >= 0 ? line.slice(sep + 1) : "", - }; - }); + const entries = await git.log({ rev: to, excluding: [from] }); + return entries.map((e) => ({ + hash: e.id, + message: e.message.split("\n")[0]!, + })); } -export async function getRepoRoot(git: GitClient): Promise { - const root = await git.run(["rev-parse", "--show-toplevel"]); - return root.trim(); +async function resolveRepoRoot( + git: GitClient, + repoRootOverride?: string, +): Promise { + return repoRootOverride ?? getRepoRoot(git); } -type DiffPathContext = { - repoRoot: string; - specs: string[]; -}; - -async function getDiffPathContext( +async function resolveParentTree( git: GitClient, - pathFilter: DiffPathFilter | undefined, - repoRootOverride?: string, -): Promise { - const repoRoot = repoRootOverride ?? (await getRepoRoot(git)); - const specs = buildDiffPathspecs(repoRoot, pathFilter); - return { repoRoot, specs }; + parentId: ObjectId | undefined, +): Promise { + if (parentId === undefined) return EMPTY_TREE_OID; + const [parentEntry] = await git.log({ rev: parentId, limit: 1 }); + return parentEntry!.tree; } -export async function getDiff( +async function diffRangeChanges( git: GitClient, - query: GitDiffRangeQuery, -): Promise { - const { + from: string, + to: string, + ignoreWhitespace: boolean | undefined, +): Promise { + const result = await git.diff({ from, to, - commits, - filterByCommits, - pathFilter, - repoRootOverride, - shaping, - } = query; - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); - const shapingArgs = buildDiffShapingGitArgs(shaping); - - if (!filterByCommits) { - const raw = await git.run([ - "diff", - ...shapingArgs, - `${from}..${to}`, - "--", - ...specs, - ]); - return shapeUnifiedDiff(raw, shaping); - } + detectRenames: true, + recursive: true, + ...(ignoreWhitespace ? { ignoreWhitespace: "all" as const } : {}), + }); + return result.changes; +} - const patches = await Promise.all( - commits.map((c) => - git.run(["diff", ...shapingArgs, `${c.hash}^!`, "--", ...specs]), - ), - ); +async function diffCommitChanges( + git: GitClient, + hash: string, + ignoreWhitespace: boolean | undefined, +): Promise { + const [entry] = await git.log({ rev: hash, limit: 1 }); + const parentTree = await resolveParentTree(git, entry!.parents[0]); + const result = await git.primitives.diffTrees(parentTree, entry!.tree, { + detectRenames: true, + recursive: true, + ...(ignoreWhitespace ? { ignoreWhitespace: "all" as const } : {}), + }); + return result.changes; +} - return patches - .map((p) => shapeUnifiedDiff(p, shaping)) - .filter(Boolean) - .join("\n"); +function changeMatchesPathFilter( + change: DiffChange, + predicate: PathFilterPredicate, +): boolean { + const { oldPath, newPath, path } = changePaths(change); + if (change.type === "rename" || change.type === "copy") { + return matchesAnyPath(predicate, [oldPath, newPath]); + } + return predicate(path); } -export async function getDiffSummary( +async function collectChanges( git: GitClient, query: GitDiffRangeQuery, -): Promise { - const { - from, - to, - commits, - filterByCommits, - pathFilter, - repoRootOverride, - shaping, - } = query; - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); - const whitespaceArgs = shaping?.ignoreWhitespace ? ["-w"] : []; + predicate: PathFilterPredicate, +): Promise { + const { from, to, commits, filterByCommits, shaping } = query; + const ignoreWhitespace = shaping?.ignoreWhitespace; if (!filterByCommits) { - const [numOutput, nameOutput] = await Promise.all([ - git.run([ - "diff", - ...whitespaceArgs, - "--numstat", - `${from}..${to}`, - "--", - ...specs, - ]), - git.run([ - "diff", - ...whitespaceArgs, - "--name-status", - `${from}..${to}`, - "--", - ...specs, - ]), - ]); - return buildDiffSummaryFromGitOutputs(nameOutput, numOutput); + const changes = await diffRangeChanges(git, from, to, ignoreWhitespace); + return [changes.filter((c) => changeMatchesPathFilter(c, predicate))]; } - const pairs = await Promise.all( + return Promise.all( commits.map(async (c) => { - const range = `${c.hash}^!`; - const [numOutput, nameOutput] = await Promise.all([ - git.run([ - "diff", - ...whitespaceArgs, - "--numstat", - range, - "--", - ...specs, - ]), - git.run([ - "diff", - ...whitespaceArgs, - "--name-status", - range, - "--", - ...specs, - ]), - ]); - return { numOutput, nameOutput }; + const changes = await diffCommitChanges(git, c.hash, ignoreWhitespace); + return changes.filter((ch) => changeMatchesPathFilter(ch, predicate)); }), ); - const nameJoined = pairs - .map((p) => p.nameOutput) - .filter(Boolean) - .join("\n"); - const numJoined = pairs - .map((p) => p.numOutput) - .filter(Boolean) - .join("\n"); - return buildDiffSummaryFromGitOutputs(nameJoined, numJoined); } -export async function getChangedFiles( +export async function getDiff( git: GitClient, query: GitDiffRangeQuery, -): Promise { - const { from, to, commits, filterByCommits, pathFilter, repoRootOverride } = - query; - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); +): Promise { + const { pathFilter, repoRootOverride, shaping } = query; + const repoRoot = await resolveRepoRoot(git, repoRootOverride); + const predicate = buildPathFilterPredicate(repoRoot, pathFilter); + const contextLines = shaping?.contextLines ?? DEFAULT_CONTEXT_LINES; - if (!filterByCommits) { - const output = await git.run([ - "diff", - "--name-only", - `${from}..${to}`, - "--", - ...specs, - ]); - - return output - .split(/\r?\n/) - .map((f) => f.trim()) - .filter(Boolean) - .sort(); - } + const changeGroups = await collectChanges(git, query, predicate); + const patches = await Promise.all( + changeGroups.map(async (changes) => { + const rendered = await renderUnifiedDiff(git, changes, contextLines); + const text = rendered + .map((r) => r.text) + .filter(Boolean) + .join("\n"); + return shapeUnifiedDiff(text, shaping); + }), + ); + return patches.filter(Boolean).join("\n"); +} - const results = await Promise.all( - commits.map(async (c) => { - const output = await git.run([ - "show", - "--name-only", - "--pretty=format:", - c.hash, - "--", - ...specs, - ]); - return output - .split(/\r?\n/) - .map((f) => f.trim()) - .filter(Boolean); +export async function getDiffSummary( + git: GitClient, + query: GitDiffRangeQuery, +): Promise { + const { pathFilter, repoRootOverride } = query; + const repoRoot = await resolveRepoRoot(git, repoRootOverride); + const predicate = buildPathFilterPredicate(repoRoot, pathFilter); + + const changeGroups = await collectChanges(git, query, predicate); + const fileGroups = await Promise.all( + changeGroups.map(async (changes) => { + const rendered = await renderUnifiedDiff(git, changes, 0); + return changes.map((c, i) => buildFileSummary(c, rendered[i]!)); }), ); + const files = + changeGroups.length > 1 + ? mergeFileSummariesByPath(fileGroups.flat()) + : fileGroups.flat(); + return summarizeFiles(files); +} + +export async function getChangedFiles( + git: GitClient, + query: GitDiffRangeQuery, +): Promise { + const { pathFilter, repoRootOverride } = query; + const repoRoot = await resolveRepoRoot(git, repoRootOverride); + const predicate = buildPathFilterPredicate(repoRoot, pathFilter); - return [...new Set(results.flat())].sort(); + const changeGroups = await collectChanges(git, query, predicate); + const paths = changeGroups.flat().map((c) => changePaths(c).path); + return [...new Set(paths)].sort(); } diff --git a/src/index.ts b/src/index.ts index 815251b..63192f0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,7 +37,7 @@ export type GitDiffAiSummaryOptions = { includeFolders?: string[]; /** * Exclude these paths relative to the repo root, e.g. `node_modules`, `dist`. - * Implemented with git `:(exclude)` pathspecs. + * Applied client-side to the diff's changed-path list (directory-prefix match). */ excludeFolders?: string[]; /** @@ -172,7 +172,7 @@ function shouldFilterByCommits( async function prepareSummaryInput( options: GitDiffAiSummaryOptions, ): Promise { - const git = options.git ?? createGitClient(options.cwd); + const git = options.git ?? (await createGitClient(options.cwd)); const from = options.from; const to = options.to ?? "HEAD"; @@ -301,11 +301,13 @@ export type { DiffSummary, GitClient, GitDiffRangeQuery, + PathFilterPredicate, + RenderedFileDiff, SecretRedactionRule, } from "./git/gitDiff.js"; export { - buildDiffPathspecs, - buildDiffShapingGitArgs, + buildFileSummary, + buildPathFilterPredicate, createGitClient, DEFAULT_NOISE_EXCLUDES, DEFAULT_SECRET_PATTERNS, @@ -315,6 +317,11 @@ export { getDiff, getDiffSummary, getRepoRoot, + matchesAnyPath, + mergeFileSummariesByPath, redactSecrets, + renderFileDiff, + renderUnifiedDiff, shapeUnifiedDiff, + summarizeFiles, } from "./git/gitDiff.js"; diff --git a/test/diffGitStatus.spec.ts b/test/diffGitStatus.spec.ts index a3221de..231a795 100644 --- a/test/diffGitStatus.spec.ts +++ b/test/diffGitStatus.spec.ts @@ -1,19 +1,18 @@ -import { mapGitStatus, mergeStatus } from "../src/git/diffGitStatus"; - -describe("mapGitStatus", () => { - it("maps A to added", () => expect(mapGitStatus("A")).toBe("added")); - it("maps D to deleted", () => expect(mapGitStatus("D")).toBe("deleted")); - it("maps R to renamed", () => expect(mapGitStatus("R")).toBe("renamed")); - it("maps R100 to renamed", () => - expect(mapGitStatus("R100")).toBe("renamed")); - it("maps C to copied", () => expect(mapGitStatus("C")).toBe("copied")); - it("maps T to type-changed", () => - expect(mapGitStatus("T")).toBe("type-changed")); - it("maps M to modified", () => expect(mapGitStatus("M")).toBe("modified")); - it("maps unknown code to unknown", () => - expect(mapGitStatus("X")).toBe("unknown")); - it("maps empty string to unknown", () => - expect(mapGitStatus("")).toBe("unknown")); +import { mapChangeTypeToStatus, mergeStatus } from "../src/git/diffGitStatus"; + +describe("mapChangeTypeToStatus", () => { + it("maps add to added", () => + expect(mapChangeTypeToStatus("add")).toBe("added")); + it("maps delete to deleted", () => + expect(mapChangeTypeToStatus("delete")).toBe("deleted")); + it("maps rename to renamed", () => + expect(mapChangeTypeToStatus("rename")).toBe("renamed")); + it("maps copy to copied", () => + expect(mapChangeTypeToStatus("copy")).toBe("copied")); + it("maps type-change to type-changed", () => + expect(mapChangeTypeToStatus("type-change")).toBe("type-changed")); + it("maps modify to modified", () => + expect(mapChangeTypeToStatus("modify")).toBe("modified")); }); describe("mergeStatus", () => { diff --git a/test/diffNameStatusParse.spec.ts b/test/diffNameStatusParse.spec.ts deleted file mode 100644 index d4fee32..0000000 --- a/test/diffNameStatusParse.spec.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { - mergeNameEntriesByPath, - parseNameStatusLines, -} from "../src/git/diffNameStatusParse"; - -describe("parseNameStatusLines", () => { - it("returns empty array for blank input", () => { - expect(parseNameStatusLines("")).toHaveLength(0); - }); - - it("parses a simple modified entry", () => { - const entries = parseNameStatusLines("M\tfile.ts"); - expect(entries).toHaveLength(1); - expect(entries[0]).toMatchObject({ path: "file.ts", status: "modified" }); - }); - - it("ignores a single-token line with no tab", () => { - const entries = parseNameStatusLines("M"); - expect(entries).toHaveLength(0); - }); - - it("ignores a rename with only one path column (incomplete line)", () => { - const entries = parseNameStatusLines("R100\told.ts"); - expect(entries).toHaveLength(0); - }); - - it("parses a full rename with old and new paths", () => { - const entries = parseNameStatusLines("R100\told.ts\tnew.ts"); - expect(entries).toHaveLength(1); - expect(entries[0]).toMatchObject({ - path: "new.ts", - status: "renamed", - oldPath: "old.ts", - }); - }); - - it("uses second column as path for non-rename entries regardless of column count", () => { - const entries = parseNameStatusLines("M\tfile.ts\textra"); - expect(entries).toHaveLength(1); - expect(entries[0]).toMatchObject({ path: "file.ts", status: "modified" }); - }); - - it("trims leading/trailing whitespace from name-status lines", () => { - const entries = parseNameStatusLines(" M\tfile.ts "); - expect(entries).toHaveLength(1); - expect(entries[0]).toMatchObject({ path: "file.ts", status: "modified" }); - }); - - it("skips empty lines", () => { - const entries = parseNameStatusLines("\nM\tfile.ts\n\n"); - expect(entries).toHaveLength(1); - }); - - it("parses CRLF line endings", () => { - const entries = parseNameStatusLines("M\ta.ts\r\nA\tb.ts"); - expect(entries).toHaveLength(2); - expect(entries[0]?.path).toBe("a.ts"); - expect(entries[1]?.path).toBe("b.ts"); - }); -}); - -describe("mergeNameEntriesByPath", () => { - it("preserves first oldPath when same path appears twice with different oldPaths", () => { - const entries = parseNameStatusLines( - ["R100\told/a.ts\tshared.ts", "R100\told/b.ts\tshared.ts"].join("\n"), - ); - const byPath = mergeNameEntriesByPath(entries); - expect(byPath.get("shared.ts")?.oldPath).toBe("old/a.ts"); - }); - - it("fills in oldPath from a later entry when first had none", () => { - const entries = parseNameStatusLines( - ["M\tshared.ts", "R100\told/a.ts\tshared.ts"].join("\n"), - ); - const byPath = mergeNameEntriesByPath(entries); - expect(byPath.get("shared.ts")?.oldPath).toBe("old/a.ts"); - }); - - it("does not overwrite existing oldPath when later entry has no oldPath", () => { - const entries = parseNameStatusLines( - ["R100\told/a.ts\tshared.ts", "M\tshared.ts"].join("\n"), - ); - const byPath = mergeNameEntriesByPath(entries); - expect(byPath.get("shared.ts")?.oldPath).toBe("old/a.ts"); - }); - - it("merges status toward higher precedence when same path appears twice", () => { - const entries = parseNameStatusLines( - ["M\tfile.ts", "D\tfile.ts"].join("\n"), - ); - const byPath = mergeNameEntriesByPath(entries); - expect(byPath.get("file.ts")?.status).toBe("deleted"); - }); -}); diff --git a/test/diffNumstat.spec.ts b/test/diffNumstat.spec.ts deleted file mode 100644 index d021119..0000000 --- a/test/diffNumstat.spec.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { accumulateNumStat } from "../src/git/diffNumstatParse"; - -function accumulate( - input: string, -): Map { - const m = new Map< - string, - { additions: number; deletions: number; binary?: boolean } - >(); - accumulateNumStat(input, m); - return m; -} - -describe("accumulateNumStat", () => { - it("parses a basic line", () => { - const m = accumulate("5\t3\tsrc/foo.ts"); - expect(m.get("src/foo.ts")).toEqual({ additions: 5, deletions: 3 }); - }); - - it("accumulates multiple lines for the same key", () => { - const m = accumulate("2\t1\tsrc/foo.ts\n3\t4\tsrc/foo.ts"); - expect(m.get("src/foo.ts")).toEqual({ additions: 5, deletions: 5 }); - }); - - it("skips empty lines", () => { - const m = accumulate("\n5\t3\tsrc/foo.ts\n\n"); - expect(m.size).toBe(1); - }); - - it("skips lines with fewer than 3 tab-separated fields", () => { - const m = accumulate("5\t3"); - expect(m.size).toBe(0); - }); - - it("handles lines with leading/trailing whitespace", () => { - const m = accumulate(" 5\t3\tsrc/foo.ts "); - expect(m.get("src/foo.ts")).toBeDefined(); - expect(m.get("src/foo.ts")).toMatchObject({ additions: 5, deletions: 3 }); - }); - - it("parses rename path without dir prefix: key is the new name", () => { - const m = accumulate("5\t3\t{old.ts => new.ts}"); - expect(m.get("new.ts")).toEqual({ additions: 5, deletions: 3 }); - expect(m.has("{old.ts => new.ts}")).toBe(false); - }); - - it("parses rename path with dir prefix: key preserves prefix + new name", () => { - const m = accumulate("5\t3\tsrc/prefix/{old.ts => new.ts}"); - expect(m.get("src/prefix/new.ts")).toEqual({ additions: 5, deletions: 3 }); - expect(m.has("src/prefix/{old.ts => new.ts}")).toBe(false); - }); - - it("trims whitespace from new segment in rename path", () => { - const m = accumulate("5\t3\tsrc/{old.ts => new.ts }"); - expect(m.get("src/new.ts")).toEqual({ additions: 5, deletions: 3 }); - }); - - it("uses entire old-part for matching (not just first char)", () => { - const m = accumulate("1\t2\t{longoldname.ts => newname.ts}"); - expect(m.get("newname.ts")).toEqual({ additions: 1, deletions: 2 }); - }); - - it("uses entire new-part for key (not just first char)", () => { - const m = accumulate("1\t2\t{old.ts => longnewname.ts}"); - expect(m.get("longnewname.ts")).toEqual({ additions: 1, deletions: 2 }); - }); - - it("handles non-rename paths with brace-like chars as literal key", () => { - const m = accumulate("1\t1\tsrc/no-rename.ts"); - expect(m.get("src/no-rename.ts")).toEqual({ additions: 1, deletions: 1 }); - }); - - it("parses CRLF line endings", () => { - const m = accumulate("5\t3\tsrc/a.ts\r\n2\t1\tsrc/b.ts"); - expect(m.get("src/a.ts")).toEqual({ additions: 5, deletions: 3 }); - expect(m.get("src/b.ts")).toEqual({ additions: 2, deletions: 1 }); - }); - - it("marks binary flag when both columns are '-'", () => { - const m = accumulate("-\t-\timage.png"); - expect(m.get("image.png")).toEqual({ - additions: 0, - deletions: 0, - binary: true, - }); - }); - - it("marks binary flag when only additions column is '-'", () => { - const m = accumulate("-\t0\tpartial.bin"); - expect(m.get("partial.bin")).toEqual({ - additions: 0, - deletions: 0, - binary: true, - }); - }); - - it("marks binary flag when only deletions column is '-'", () => { - const m = accumulate("0\t-\tdel-only.bin"); - expect(m.get("del-only.bin")).toEqual({ - additions: 0, - deletions: 0, - binary: true, - }); - }); - - it("propagates binary flag when accumulating a binary entry onto a text entry", () => { - const m = accumulate("2\t1\tsrc/foo.ts\n-\t-\tsrc/foo.ts"); - expect(m.get("src/foo.ts")).toMatchObject({ - additions: 2, - deletions: 1, - binary: true, - }); - }); - - it("does not set binary flag for normal numeric entries", () => { - const m = accumulate("5\t3\tsrc/foo.ts"); - expect(m.get("src/foo.ts")).toEqual({ additions: 5, deletions: 3 }); - }); -}); diff --git a/test/diffPathFilter.spec.ts b/test/diffPathFilter.spec.ts new file mode 100644 index 0000000..bc6f718 --- /dev/null +++ b/test/diffPathFilter.spec.ts @@ -0,0 +1,113 @@ +import { join } from "node:path"; + +import { + buildPathFilterPredicate, + matchesAnyPath, +} from "../src/git/diffPathFilter"; + +describe("buildPathFilterPredicate", () => { + const repoRoot = join(__dirname, "fixture-repo-root"); + + it("matches everything when no filter is given", () => { + const predicate = buildPathFilterPredicate(repoRoot); + expect(predicate("anything/at/all.ts")).toBe(true); + }); + + it("excludes an exact excluded path and anything under it", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + excludeFolders: ["node_modules", "dist"], + }); + expect(predicate("node_modules")).toBe(false); + expect(predicate("node_modules/pkg/index.js")).toBe(false); + expect(predicate("dist/bundle.js")).toBe(false); + expect(predicate("src/index.ts")).toBe(true); + }); + + it("restricts to include folders and their subpaths only", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["src", "packages/lib"], + }); + expect(predicate("src/index.ts")).toBe(true); + expect(predicate("src")).toBe(true); + expect(predicate("packages/lib/a.ts")).toBe(true); + expect(predicate("packages/other/a.ts")).toBe(false); + expect(predicate("README.md")).toBe(false); + }); + + it("does not match a sibling path that merely shares a prefix", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["src"], + }); + expect(predicate("src-other/file.ts")).toBe(false); + }); + + it("applies excludes on top of includes", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["src"], + excludeFolders: ["src/generated"], + }); + expect(predicate("src/index.ts")).toBe(true); + expect(predicate("src/generated/a.ts")).toBe(false); + }); + + it("normalizes backslashes and slashes in configured folders", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["src\\app"], + }); + expect(predicate("src/app/main.ts")).toBe(true); + }); + + it("treats a root-like include as the whole repo", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["/"], + excludeFolders: ["tmp"], + }); + expect(predicate("anything.ts")).toBe(true); + expect(predicate("tmp/x")).toBe(false); + }); + + it("ignores blank/whitespace-only entries", () => { + const predicate = buildPathFilterPredicate(repoRoot, { + includeFolders: ["", " ", "src"], + }); + expect(predicate("src/index.ts")).toBe(true); + expect(predicate("other.ts")).toBe(false); + }); + + it("throws when an include path escapes the repository root", () => { + expect(() => + buildPathFilterPredicate(repoRoot, { includeFolders: ["../outside"] }), + ).toThrow(/escapes repository root/); + }); + + it("throws when an exclude path escapes the repository root", () => { + expect(() => + buildPathFilterPredicate(repoRoot, { + excludeFolders: ["../somewhere-else"], + }), + ).toThrow(/escapes repository root/); + }); + + it("throws when include path resolves exactly to the parent directory", () => { + expect(() => + buildPathFilterPredicate(repoRoot, { includeFolders: [".."] }), + ).toThrow(/escapes repository root/); + }); +}); + +describe("matchesAnyPath", () => { + it("is true when at least one defined path matches", () => { + const predicate = (p: string): boolean => p === "b"; + expect(matchesAnyPath(predicate, ["a", "b"])).toBe(true); + }); + + it("is false when no defined path matches", () => { + const predicate = (p: string): boolean => p === "z"; + expect(matchesAnyPath(predicate, ["a", "b"])).toBe(false); + }); + + it("skips undefined candidates", () => { + const predicate = (p: string): boolean => p === "b"; + expect(matchesAnyPath(predicate, [undefined, "b"])).toBe(true); + }); +}); diff --git a/test/diffRender.spec.ts b/test/diffRender.spec.ts new file mode 100644 index 0000000..f89adaf --- /dev/null +++ b/test/diffRender.spec.ts @@ -0,0 +1,115 @@ +import type { DiffChange, ObjectId, Repository } from "@scolladon/tsgit"; + +import { renderFileDiff } from "../src/git/diffRender"; + +const oid = (s: string): ObjectId => s as ObjectId; +const encoder = new TextEncoder(); + +/** Minimal stub satisfying only what renderFileDiff touches — no real repo needed. */ +function stubRepo(blobs: Record): Repository { + return { + primitives: { + readBlob: async (id: ObjectId) => ({ + type: "blob" as const, + id, + content: encoder.encode(blobs[id as unknown as string] ?? ""), + }), + }, + } as unknown as Repository; +} + +describe("renderFileDiff — mode changes", () => { + it("emits old mode/new mode lines for a pure mode change with unchanged content", async () => { + const change: DiffChange = { + type: "modify", + path: "script.sh", + oldId: oid("a"), + newId: oid("a"), + oldMode: "100644", + newMode: "100755", + }; + const repo = stubRepo({ a: "echo hi\n" }); + + const out = await renderFileDiff(repo, change, 3); + + expect(out.text).toContain("old mode 100644"); + expect(out.text).toContain("new mode 100755"); + expect(out.added).toBe(0); + expect(out.deleted).toBe(0); + }); + + it("emits old mode/new mode lines for a rename with a mode change", async () => { + const change: DiffChange = { + type: "rename", + oldPath: "old.sh", + newPath: "new.sh", + oldId: oid("a"), + newId: oid("a"), + oldMode: "100644", + newMode: "100755", + similarity: { score: 100, maxScore: 100 }, + }; + const repo = stubRepo({ a: "echo hi\n" }); + + const out = await renderFileDiff(repo, change, 3); + + expect(out.text).toContain("old mode 100644"); + expect(out.text).toContain("new mode 100755"); + expect(out.text).toContain("rename from old.sh"); + }); + + it("emits copy from/to lines for a copy change", async () => { + const change: DiffChange = { + type: "copy", + oldPath: "src.ts", + newPath: "dup.ts", + oldId: oid("a"), + newId: oid("a"), + oldMode: "100644", + newMode: "100644", + similarity: { score: 100, maxScore: 100 }, + }; + const repo = stubRepo({ a: "shared\n" }); + + const out = await renderFileDiff(repo, change, 3); + + expect(out.text).toContain("copy from src.ts"); + expect(out.text).toContain("copy to dup.ts"); + expect(out.text).toContain("similarity index 100%"); + }); + + it("reports similarity 0% instead of dividing by a zero maxScore", async () => { + const change: DiffChange = { + type: "rename", + oldPath: "old.ts", + newPath: "new.ts", + oldId: oid("a"), + newId: oid("b"), + oldMode: "100644", + newMode: "100644", + similarity: { score: 0, maxScore: 0 }, + }; + const repo = stubRepo({ a: "x\n", b: "y\n" }); + + const out = await renderFileDiff(repo, change, 3); + + expect(out.text).toContain("similarity index 0%"); + }); + + it("omits old mode/new mode lines when the mode is unchanged", async () => { + const change: DiffChange = { + type: "modify", + path: "f.ts", + oldId: oid("a"), + newId: oid("b"), + oldMode: "100644", + newMode: "100644", + }; + const repo = stubRepo({ a: "one\n", b: "two\n" }); + + const out = await renderFileDiff(repo, change, 3); + + expect(out.text).not.toContain("old mode"); + expect(out.text).not.toContain("new mode"); + }); +}); diff --git a/test/diffRenderPrimitives.spec.ts b/test/diffRenderPrimitives.spec.ts new file mode 100644 index 0000000..4aa19af --- /dev/null +++ b/test/diffRenderPrimitives.spec.ts @@ -0,0 +1,156 @@ +import { + buildHunks, + computeLineOps, + countLineOps, + formatHunks, +} from "../src/git/diffRenderPrimitives"; + +describe("computeLineOps", () => { + it("produces a single context run for identical text", () => { + const ops = computeLineOps("a\nb\n", "a\nb\n"); + expect(ops.every((o) => o.kind === "context")).toBe(true); + expect(ops.map((o) => o.text)).toEqual(["a", "b"]); + }); + + it("marks the last line noEol when the new text lacks a trailing newline", () => { + const ops = computeLineOps("a\n", "a"); + const last = ops.at(-1)!; + expect(last.text).toBe("a"); + expect(last.noEol).toBe(true); + }); + + it("treats an empty string as zero lines", () => { + expect(computeLineOps("", "")).toEqual([]); + }); + + it("produces del-only ops for a pure deletion to empty", () => { + const ops = computeLineOps("a\nb\n", ""); + expect(ops).toEqual([ + { kind: "del", text: "a", noEol: false }, + { kind: "del", text: "b", noEol: false }, + ]); + }); + + it("produces add-only ops for a pure addition from empty", () => { + const ops = computeLineOps("", "a\nb\n"); + expect(ops).toEqual([ + { kind: "add", text: "a", noEol: false }, + { kind: "add", text: "b", noEol: false }, + ]); + }); +}); + +describe("countLineOps", () => { + it("counts add and del ops, ignoring context", () => { + const ops = computeLineOps("a\nb\nc\n", "a\nx\nc\n"); + expect(countLineOps(ops)).toEqual({ added: 1, deleted: 1 }); + }); + + it("returns zero counts for identical text", () => { + expect(countLineOps(computeLineOps("same\n", "same\n"))).toEqual({ + added: 0, + deleted: 0, + }); + }); +}); + +describe("buildHunks", () => { + it("returns no hunks when nothing changed", () => { + expect(buildHunks(computeLineOps("a\nb\n", "a\nb\n"), 3)).toEqual([]); + }); + + it("builds one hunk with correct start/length for a single-line change", () => { + const ops = computeLineOps("a\nb\nc\n", "a\nB\nc\n"); + const hunks = buildHunks(ops, 3); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ + oldStart: 1, + oldLines: 3, + newStart: 1, + newLines: 3, + }); + }); + + it("uses oldStart 0 for a pure addition at the very start with zero context", () => { + const ops = computeLineOps("", "new\n"); + const hunks = buildHunks(ops, 0); + expect(hunks[0]).toMatchObject({ + oldStart: 0, + oldLines: 0, + newStart: 1, + newLines: 1, + }); + }); + + it("uses newStart 0 for a pure deletion with zero context", () => { + const ops = computeLineOps("gone\n", ""); + const hunks = buildHunks(ops, 0); + expect(hunks[0]).toMatchObject({ + oldStart: 1, + oldLines: 1, + newStart: 0, + newLines: 0, + }); + }); + + it("derives oldStart from the preceding context line for a mid-file pure addition", () => { + const ops = computeLineOps("line1\n", "line1\nline2\n"); + const hunks = buildHunks(ops, 0); + expect(hunks[0]).toMatchObject({ oldStart: 1, oldLines: 0, newStart: 2 }); + }); + + it("derives newStart from the preceding context line for a mid-file pure deletion", () => { + const ops = computeLineOps("line1\nline2\n", "line1\n"); + const hunks = buildHunks(ops, 0); + expect(hunks[0]).toMatchObject({ newStart: 1, newLines: 0, oldStart: 2 }); + }); + + it("splits distant changes into separate hunks", () => { + const oldText = Array.from({ length: 20 }, (_, i) => `l${i}`).join("\n"); + const newLines = Array.from({ length: 20 }, (_, i) => `l${i}`); + newLines[0] = "CHANGED-0"; + newLines[19] = "CHANGED-19"; + const ops = computeLineOps(`${oldText}\n`, `${newLines.join("\n")}\n`); + const hunks = buildHunks(ops, 1); + expect(hunks).toHaveLength(2); + }); + + it("merges two changes within 2*context of each other into one hunk", () => { + const oldLines = Array.from({ length: 10 }, (_, i) => `l${i}`); + const newLines = [...oldLines]; + newLines[2] = "CHANGED-2"; + newLines[6] = "CHANGED-6"; + const ops = computeLineOps( + `${oldLines.join("\n")}\n`, + `${newLines.join("\n")}\n`, + ); + // gap between changed lines (index 2 and 6) is 3; context 2 => merge window 4 >= gap-1(3) + const hunks = buildHunks(ops, 2); + expect(hunks).toHaveLength(1); + }); +}); + +describe("formatHunks", () => { + it("renders a @@ header and prefixed lines", () => { + const ops = computeLineOps("a\nb\nc\n", "a\nB\nc\n"); + const text = formatHunks(buildHunks(ops, 3)); + expect(text.split("\n")).toEqual([ + "@@ -1,3 +1,3 @@", + " a", + "-b", + "+B", + " c", + ]); + }); + + it("appends the no-newline marker after a noEol line", () => { + const ops = computeLineOps("a\n", "a\nb"); + const text = formatHunks(buildHunks(ops, 3)); + expect(text).toContain("+b"); + expect(text.split("\n").at(-1)).toBe("\\ No newline at end of file"); + }); + + it("returns empty string for no hunks", () => { + expect(formatHunks([])).toBe(""); + }); +}); diff --git a/test/diffShaping.spec.ts b/test/diffShaping.spec.ts index 4e2fa4a..01c343d 100644 --- a/test/diffShaping.spec.ts +++ b/test/diffShaping.spec.ts @@ -1,50 +1,4 @@ -import { - buildDiffShapingGitArgs, - DEFAULT_NOISE_EXCLUDES, - shapeUnifiedDiff, -} from "../src/git/gitDiff"; - -describe("buildDiffShapingGitArgs", () => { - it("returns an empty array when shaping is undefined", () => { - expect(buildDiffShapingGitArgs()).toEqual([]); - }); - - it("returns an empty array when no fields are set", () => { - expect(buildDiffShapingGitArgs({})).toEqual([]); - }); - - it("emits -U for valid contextLines", () => { - expect(buildDiffShapingGitArgs({ contextLines: 1 })).toEqual(["-U1"]); - }); - - it("truncates fractional contextLines", () => { - expect(buildDiffShapingGitArgs({ contextLines: 2.9 })).toEqual(["-U2"]); - }); - - it("clamps negative contextLines to 0", () => { - expect(buildDiffShapingGitArgs({ contextLines: -5 })).toEqual(["-U0"]); - }); - - it("clamps non-finite contextLines to 0", () => { - expect(buildDiffShapingGitArgs({ contextLines: Number.NaN })).toEqual([ - "-U0", - ]); - }); - - it("emits -w when ignoreWhitespace is true", () => { - expect(buildDiffShapingGitArgs({ ignoreWhitespace: true })).toEqual(["-w"]); - }); - - it("omits -w when ignoreWhitespace is false", () => { - expect(buildDiffShapingGitArgs({ ignoreWhitespace: false })).toEqual([]); - }); - - it("combines contextLines and ignoreWhitespace", () => { - expect( - buildDiffShapingGitArgs({ contextLines: 0, ignoreWhitespace: true }), - ).toEqual(["-U0", "-w"]); - }); -}); +import { DEFAULT_NOISE_EXCLUDES, shapeUnifiedDiff } from "../src/git/gitDiff"; describe("DEFAULT_NOISE_EXCLUDES", () => { it("includes common lockfiles and build outputs", () => { diff --git a/test/diffSummary.spec.ts b/test/diffSummary.spec.ts new file mode 100644 index 0000000..6bd81f4 --- /dev/null +++ b/test/diffSummary.spec.ts @@ -0,0 +1,202 @@ +import type { DiffChange, ObjectId } from "@scolladon/tsgit"; + +import type { RenderedFileDiff } from "../src/git/diffRender"; +import { + buildFileSummary, + mergeFileSummariesByPath, + summarizeFiles, +} from "../src/git/diffSummary"; + +const oid = (s: string): ObjectId => s as ObjectId; + +function rendered(over: Partial = {}): RenderedFileDiff { + return { + path: "f.ts", + added: 0, + deleted: 0, + binary: false, + text: "", + ...over, + }; +} + +describe("buildFileSummary", () => { + it("summarizes an add change", () => { + const change: DiffChange = { + type: "add", + newPath: "new.ts", + newId: oid("a"), + newMode: "100644", + }; + const out = buildFileSummary( + change, + rendered({ path: "new.ts", added: 3 }), + ); + expect(out).toMatchObject({ + path: "new.ts", + status: "added", + additions: 3, + deletions: 0, + }); + expect(out.oldPath).toBeUndefined(); + expect(out.newPath).toBeUndefined(); + }); + + it("summarizes a delete change", () => { + const change: DiffChange = { + type: "delete", + oldPath: "gone.ts", + oldId: oid("a"), + oldMode: "100644", + }; + const out = buildFileSummary( + change, + rendered({ path: "gone.ts", deleted: 5 }), + ); + expect(out).toMatchObject({ + path: "gone.ts", + status: "deleted", + deletions: 5, + }); + }); + + it("summarizes a rename change with oldPath/newPath set", () => { + const change: DiffChange = { + type: "rename", + oldPath: "old.ts", + newPath: "new.ts", + oldId: oid("a"), + newId: oid("b"), + oldMode: "100644", + newMode: "100644", + similarity: { score: 100, maxScore: 100 }, + }; + const out = buildFileSummary( + change, + rendered({ path: "new.ts", oldPath: "old.ts", newPath: "new.ts" }), + ); + expect(out).toMatchObject({ + path: "new.ts", + status: "renamed", + oldPath: "old.ts", + newPath: "new.ts", + }); + }); + + it("does not set oldPath/newPath for a plain modify", () => { + const change: DiffChange = { + type: "modify", + path: "f.ts", + oldId: oid("a"), + newId: oid("b"), + oldMode: "100644", + newMode: "100644", + }; + const out = buildFileSummary(change, rendered({ added: 1, deleted: 1 })); + expect(out.oldPath).toBeUndefined(); + expect(out.newPath).toBeUndefined(); + expect(out.status).toBe("modified"); + }); + + it("sets binary true only when the rendered diff is binary", () => { + const change: DiffChange = { + type: "modify", + path: "img.png", + oldId: oid("a"), + newId: oid("b"), + oldMode: "100644", + newMode: "100644", + }; + expect( + buildFileSummary(change, rendered({ path: "img.png", binary: true })) + .binary, + ).toBe(true); + expect( + buildFileSummary(change, rendered({ path: "img.png", binary: false })) + .binary, + ).toBeUndefined(); + }); +}); + +describe("mergeFileSummariesByPath", () => { + it("sums additions/deletions for duplicate paths", () => { + const merged = mergeFileSummariesByPath([ + { path: "a.ts", status: "modified", additions: 1, deletions: 1 }, + { path: "a.ts", status: "modified", additions: 2, deletions: 0 }, + ]); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ additions: 3, deletions: 1 }); + }); + + it("merges statuses toward higher precedence", () => { + const merged = mergeFileSummariesByPath([ + { path: "a.ts", status: "modified", additions: 1, deletions: 1 }, + { path: "a.ts", status: "deleted", additions: 0, deletions: 1 }, + ]); + expect(merged[0]?.status).toBe("deleted"); + }); + + it("fills in oldPath from a later rename entry", () => { + const merged = mergeFileSummariesByPath([ + { path: "shared.ts", status: "modified", additions: 1, deletions: 1 }, + { + path: "shared.ts", + status: "renamed", + additions: 0, + deletions: 0, + oldPath: "old.ts", + newPath: "shared.ts", + }, + ]); + expect(merged[0]).toMatchObject({ + oldPath: "old.ts", + newPath: "shared.ts", + }); + }); + + it("propagates binary true once set", () => { + const merged = mergeFileSummariesByPath([ + { path: "a.bin", status: "modified", additions: 0, deletions: 0 }, + { + path: "a.bin", + status: "modified", + additions: 0, + deletions: 0, + binary: true, + }, + ]); + expect(merged[0]?.binary).toBe(true); + }); + + it("keeps distinct paths separate", () => { + const merged = mergeFileSummariesByPath([ + { path: "a.ts", status: "modified", additions: 1, deletions: 0 }, + { path: "b.ts", status: "added", additions: 2, deletions: 0 }, + ]); + expect(merged).toHaveLength(2); + }); +}); + +describe("summarizeFiles", () => { + it("computes totals from the file list", () => { + const summary = summarizeFiles([ + { path: "a.ts", status: "modified", additions: 2, deletions: 1 }, + { path: "b.ts", status: "added", additions: 5, deletions: 0 }, + ]); + expect(summary).toEqual({ + files: expect.any(Array), + totalFiles: 2, + totalAdditions: 7, + totalDeletions: 1, + }); + }); + + it("handles an empty file list", () => { + expect(summarizeFiles([])).toEqual({ + files: [], + totalFiles: 0, + totalAdditions: 0, + totalDeletions: 0, + }); + }); +}); diff --git a/test/diffSummaryBuild.spec.ts b/test/diffSummaryBuild.spec.ts deleted file mode 100644 index e1396f4..0000000 --- a/test/diffSummaryBuild.spec.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { buildDiffSummaryFromGitOutputs } from "../src/git/diffSummaryBuild"; - -describe("buildDiffSummaryFromGitOutputs", () => { - it("produces status 'copied' for C100 name-status entries", () => { - const summary = buildDiffSummaryFromGitOutputs( - "C100\torig.ts\tcopy.ts", - "1\t0\tcopy.ts", - ); - expect(summary.files.find((f) => f.path === "copy.ts")?.status).toBe( - "copied", - ); - }); - - it("produces status 'type-changed' for T name-status entries", () => { - const summary = buildDiffSummaryFromGitOutputs( - "T\tfile.ext", - "0\t0\tfile.ext", - ); - expect(summary.files.find((f) => f.path === "file.ext")?.status).toBe( - "type-changed", - ); - }); - - it("includes oldPath in the synthetic line for renamed files", () => { - const summary = buildDiffSummaryFromGitOutputs( - "R100\told.ts\tnew.ts", - "5\t3\tnew.ts", - ); - expect(summary.files.find((f) => f.path === "new.ts")).toMatchObject({ - oldPath: "old.ts", - status: "renamed", - }); - }); - - it("uses zero additions and deletions for files absent from numstat", () => { - const summary = buildDiffSummaryFromGitOutputs("M\tfile.ts", ""); - const file = summary.files.find((f) => f.path === "file.ts"); - expect(file).toMatchObject({ additions: 0, deletions: 0 }); - }); - - it("marks binary flag for files reported as binary in numstat", () => { - const summary = buildDiffSummaryFromGitOutputs( - "M\timage.png", - "-\t-\timage.png", - ); - expect(summary.files.find((f) => f.path === "image.png")?.binary).toBe( - true, - ); - }); - - it("does not set binary flag for text files", () => { - const summary = buildDiffSummaryFromGitOutputs( - "M\tfile.ts", - "5\t3\tfile.ts", - ); - expect( - summary.files.find((f) => f.path === "file.ts")?.binary, - ).toBeUndefined(); - }); - - it("reflects numstat additions and deletions in the final summary", () => { - const summary = buildDiffSummaryFromGitOutputs( - "M\tfile.ts", - "10\t4\tfile.ts", - ); - const file = summary.files.find((f) => f.path === "file.ts"); - expect(file?.additions).toBe(10); - expect(file?.deletions).toBe(4); - }); -}); diff --git a/test/gitDiff.async.spec.ts b/test/gitDiff.async.spec.ts index e57dba9..4a75abd 100644 --- a/test/gitDiff.async.spec.ts +++ b/test/gitDiff.async.spec.ts @@ -1,8 +1,4 @@ -import { join } from "node:path"; -import { exec } from "dugite"; -import type { Mock } from "vitest"; import { - type CommitInfo, createGitClient, type GitClient, getChangedFiles, @@ -11,621 +7,362 @@ import { getDiffSummary, getRepoRoot, } from "../src/git/gitDiff"; +import { createFixtureRepo, type FixtureRepo } from "./helpers/tsgitFixture"; -vi.mock("dugite", () => ({ exec: vi.fn() })); +describe("gitDiffOps against a real tsgit repo", () => { + let fx: FixtureRepo; + let git: GitClient; -function makeGit( - impl: (args: string[]) => Promise = () => Promise.resolve(""), -): GitClient & { run: Mock } { - return { run: vi.fn().mockImplementation(impl) }; -} - -describe("createGitClient", () => { - it("returns a GitClient with a run function for the given cwd", () => { - const git = createGitClient(join(__dirname, "..")); - expect(git).toBeDefined(); - expect(typeof git.run).toBe("function"); + beforeEach(async () => { + fx = await createFixtureRepo(); + git = await createGitClient(fx.dir); }); - it("defaults cwd to process.cwd when omitted", () => { - const git = createGitClient(); - expect(git).toBeDefined(); + afterEach(async () => { + await git.dispose(); + await fx.cleanup(); }); - describe("run", () => { - beforeEach(() => { - vi.mocked(exec).mockReset(); + describe("createGitClient", () => { + it("defaults cwd to process.cwd when omitted", async () => { + const cwdGit = await createGitClient(); + await cwdGit.dispose(); }); - it("returns stdout on success", async () => { - vi.mocked(exec).mockResolvedValue({ - exitCode: 0, - stdout: "v2.42.0\n", - stderr: "", - }); - const git = createGitClient("/repo"); - await expect(git.run(["--version"])).resolves.toBe("v2.42.0\n"); - expect(vi.mocked(exec)).toHaveBeenCalledWith(["--version"], "/repo", {}); + it("accepts a timeout and still opens successfully", async () => { + const timedGit = await createGitClient(fx.dir, 5000); + expect(await getRepoRoot(timedGit)).toBe(fx.dir); + await timedGit.dispose(); }); + }); - it("passes AbortSignal when timeout is provided", async () => { - vi.mocked(exec).mockResolvedValue({ - exitCode: 0, - stdout: "ok", - stderr: "", - }); - const git = createGitClient("/repo", 5000); - await git.run(["status"]); - expect(vi.mocked(exec)).toHaveBeenCalledWith(["status"], "/repo", { - signal: expect.any(AbortSignal), - }); + describe("getRepoRoot", () => { + it("returns the repository working-tree root", async () => { + expect(await getRepoRoot(git)).toBe(fx.dir); }); + }); - it("throws stderr when exit code is non-zero and stderr is non-empty", async () => { - vi.mocked(exec).mockResolvedValue({ - exitCode: 1, - stdout: "", - stderr: "fatal: not a git repository", + describe("getCommits", () => { + it("returns commits newest-first with subject-only messages", async () => { + const c1 = await fx.commit("first\n\nlonger body here", { + "a.ts": "1\n", }); - const git = createGitClient("/repo"); - await expect(git.run(["status"])).rejects.toThrow( - "fatal: not a git repository", - ); - }); + const c2 = await fx.commit("second", { "a.ts": "2\n" }); - it("throws generic message when exit code is non-zero and stderr is empty", async () => { - vi.mocked(exec).mockResolvedValue({ - exitCode: 128, - stdout: "", - stderr: "", - }); - const git = createGitClient("/repo"); - await expect(git.run(["status"])).rejects.toThrow( - "git exited with code 128", - ); + const commits = await getCommits(git, c1, c2); + expect(commits).toEqual([{ hash: c2, message: "second" }]); }); - }); -}); -describe("getRepoRoot", () => { - it("trims rev-parse output", async () => { - const git = makeGit(() => Promise.resolve(" /repo/root \n")); - await expect(getRepoRoot(git)).resolves.toBe("/repo/root"); - }); -}); - -describe("getCommits", () => { - it("returns parsed log output as CommitInfo[]", async () => { - const git = makeGit(() => Promise.resolve("aaa\x1fmsg\n")); - await expect(getCommits(git, "from", "to")).resolves.toEqual([ - { hash: "aaa", message: "msg" }, - ]); - expect(git.run).toHaveBeenCalledWith([ - "log", - "--format=%H%x1f%s", - "from..to", - ]); - }); - - it("uses full line as hash and empty message when separator is absent", async () => { - const git = makeGit(() => Promise.resolve("deadbeef\n")); - await expect(getCommits(git, "from", "to")).resolves.toEqual([ - { hash: "deadbeef", message: "" }, - ]); - }); + it("returns every commit in the range for a multi-commit span", async () => { + const c1 = await fx.commit("c1", { "a.ts": "1\n" }); + const c2 = await fx.commit("c2", { "a.ts": "2\n" }); + const c3 = await fx.commit("c3", { "a.ts": "3\n" }); - it("handles separator at position zero (empty hash)", async () => { - const git = makeGit(() => Promise.resolve("\x1fmsg\n")); - await expect(getCommits(git, "from", "to")).resolves.toEqual([ - { hash: "", message: "msg" }, - ]); - }); -}); - -function makeGitWithDiff(): { git: GitClient & { run: Mock }; run: Mock } { - const run = vi.fn().mockImplementation(async (args: string[]) => { - if (args[0] === "rev-parse") { - return `${join(__dirname, "fixture-repo")}\n`; - } - return ""; - }); - const git = { run }; - return { git, run }; -} - -describe("getDiff", () => { - it("uses range diff and repoRootOverride skips rev-parse", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("range-diff"); - const commits: CommitInfo[] = [{ hash: "x", message: "m" }]; - - const out = await getDiff(git, { - from: "a", - to: "b", - commits, - filterByCommits: false, - pathFilter: { excludeFolders: ["out"] }, - repoRootOverride: join(__dirname, "fixture-repo"), + const commits = await getCommits(git, c1, c3); + expect(commits.map((c) => c.hash)).toEqual([c3, c2]); }); - expect(out).toBe("range-diff"); - expect(run).toHaveBeenCalledWith([ - "diff", - "a..b", - "--", - ".", - ":(exclude)out", - ]); - const revParseCalled = run.mock.calls.some( - (c: string[][]) => c[0][0] === "rev-parse", - ); - expect(revParseCalled).toBe(false); - }); - - it("forwards shaping args and post-processes the range diff", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue( - [ - "diff --git a/a.ts b/a.ts", - "index 111..222 100644", - "--- a/a.ts", - "+++ b/a.ts", - "@@ -1,1 +1,1 @@", - "-old", - "+new", - ].join("\n"), - ); - - const out = await getDiff(git, { - from: "a", - to: "b", - commits: [{ hash: "x", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), - shaping: { - contextLines: 1, - ignoreWhitespace: true, - stripDiffPreamble: true, - }, + it("returns an empty array for an empty range", async () => { + const c1 = await fx.commit("c1", { "a.ts": "1\n" }); + expect(await getCommits(git, c1, c1)).toEqual([]); }); - - expect(run).toHaveBeenCalledWith(["diff", "-U1", "-w", "a..b", "--", "."]); - expect(out).not.toContain("diff --git"); - expect(out).not.toContain("index 111..222"); - expect(out).toContain("--- a/a.ts"); - expect(out).toContain("@@ -1,1 +1,1 @@"); - }); - - it("shapes each per-commit patch independently", async () => { - const { git, run } = makeGitWithDiff(); - run - .mockResolvedValueOnce( - [ - "diff --git a/a.ts b/a.ts", - "index 111..222 100644", - "--- a/a.ts", - "+++ b/a.ts", - "@@ -1,1 +1,1 @@", - "-a", - "+b", - ].join("\n"), - ) - .mockResolvedValueOnce(""); - - const out = await getDiff(git, { - from: "f", - to: "t", - commits: [ - { hash: "aaa", message: "1" }, - { hash: "bbb", message: "2" }, - ], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), - shaping: { stripDiffPreamble: true, contextLines: 0 }, - }); - - expect(run).toHaveBeenCalledWith(["diff", "-U0", "aaa^!", "--", "."]); - expect(run).toHaveBeenCalledWith(["diff", "-U0", "bbb^!", "--", "."]); - expect(out).not.toContain("diff --git"); - expect(out).toContain("--- a/a.ts"); }); - it("joins per-commit patches and drops empty", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValueOnce("").mockResolvedValueOnce("patch-b"); - const commits: CommitInfo[] = [ - { hash: "aaa111", message: "a" }, - { hash: "bbb222", message: "b" }, - ]; - - const out = await getDiff(git, { - from: "f", - to: "t", - commits, - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), - }); + describe("getDiff — range mode", () => { + it("renders add, modify, and delete in one document", async () => { + const c1 = await fx.commit("root", { + "a.ts": "one\ntwo\nthree\n", + "b.ts": "keep me\n", + }); + const c2 = await fx.commit( + "changes", + { "a.ts": "one\ntwoo\nthree\n", "c.ts": "new file\n" }, + ["b.ts"], + ); - expect(run).toHaveBeenCalledWith(["diff", "aaa111^!", "--", "."]); - expect(run).toHaveBeenCalledWith(["diff", "bbb222^!", "--", "."]); - expect(out).toBe("patch-b"); - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); - it("joins two non-empty per-commit patches with a newline separator", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValueOnce("patch-a").mockResolvedValueOnce("patch-b"); - - const out = await getDiff(git, { - from: "f", - to: "t", - commits: [ - { hash: "aaa", message: "1" }, - { hash: "bbb", message: "2" }, - ], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("diff --git a/a.ts b/a.ts"); + expect(diff).toContain("-two\n+twoo"); + expect(diff).toContain("new file mode 100644"); + expect(diff).toContain("+new file"); + expect(diff).toContain("deleted file mode 100644"); + expect(diff).toContain("-keep me"); }); - expect(out).toBe("patch-a\npatch-b"); - }); -}); + it("detects renames with unchanged content as a single rename entry", async () => { + const c1 = await fx.commit("root", { "old/name.ts": "same content\n" }); + const c2 = await fx.commit( + "rename", + { "new/name.ts": "same content\n" }, + ["old/name.ts"], + ); -describe("getDiffSummary", () => { - it("aggregates range numstat and name-status", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) { - return [ - "1\t2\tadded.ts", - "-\t-\tempty", - "3\t0\tnew/name", - "1\t1\tdup.ts", - "2\t0\tprefix{a => b}", - ].join("\n"); - } - if (args.includes("--name-status")) { - return [ - "A\tadded.ts", - "D\tgone.ts", - "C100\torig\tcopy.ts", - "T\ttyped.ext", - "R100\told/name\tnew/name", - "M\tdup.ts", - "M\tdup.ts", - "M\tprefixb", - "R99\tonlyonecol", - "X", - "??\tunknown.bin", - ].join("\n"); - } - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); - const summary = await getDiffSummary(git, { - from: "x", - to: "y", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("similarity index 100%"); + expect(diff).toContain("rename from old/name.ts"); + expect(diff).toContain("rename to new/name.ts"); + expect(diff).not.toContain("new file mode"); + expect(diff).not.toContain("deleted file mode"); }); - const paths = new Set(summary.files.map((f) => f.path)); - expect(paths.has("added.ts")).toBe(true); - expect(paths.has("gone.ts")).toBe(true); - expect(paths.has("copy.ts")).toBe(true); - expect(summary.files.find((f) => f.path === "new/name")).toMatchObject({ - status: "renamed", - }); - expect(summary.files.find((f) => f.path === "prefixb")).toBeDefined(); - expect(summary.files.find((f) => f.path === "unknown.bin")?.status).toBe( - "unknown", - ); - }); + it("renders a binary change with no hunks", async () => { + const c1 = await fx.commit("root", { + "img.bin": Buffer.from([0, 1, 2, 3, 0]), + }); + const c2 = await fx.commit("update", { + "img.bin": Buffer.from([0, 9, 9, 9, 0]), + }); - it("tolerates malformed numstat lines and non-numeric counts", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) { - return [ - "single_col_no_tabs", - "two\tcols", - "abc\t1\tnonnum.ts", - "1\tdef\tnonnum2.ts", - ].join("\n"); - } - if (args.includes("--name-status")) { - return ["M\tnonnum.ts", "M\tnonnum2.ts"].join("\n"); - } - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); - const summary = await getDiffSummary(git, { - from: "x", - to: "y", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("Binary files a/img.bin and b/img.bin differ"); + expect(diff).not.toContain("@@"); }); - expect(summary.files.find((f) => f.path === "nonnum.ts")).toMatchObject({ - additions: 0, - deletions: 1, - }); - expect(summary.files.find((f) => f.path === "nonnum2.ts")).toMatchObject({ - additions: 1, - deletions: 0, - }); - }); + it("applies includeFolders and excludeFolders", async () => { + const c1 = await fx.commit("root", { + "src/keep.ts": "a\n", + "src/skip.ts": "a\n", + "docs/readme.md": "a\n", + }); + const c2 = await fx.commit("edit", { + "src/keep.ts": "b\n", + "src/skip.ts": "b\n", + "docs/readme.md": "b\n", + }); - it("merges multiple renames that target the same new path", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) { - return "1\t1\tshared.ts"; - } - if (args.includes("--name-status")) { - return ["R100\told/a.ts\tshared.ts", "R100\told/b.ts\tshared.ts"].join( - "\n", - ); - } - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + pathFilter: { + includeFolders: ["src"], + excludeFolders: ["src/skip.ts"], + }, + }); - const summary = await getDiffSummary(git, { - from: "x", - to: "y", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("src/keep.ts"); + expect(diff).not.toContain("src/skip.ts"); + expect(diff).not.toContain("docs/readme.md"); }); - const shared = summary.files.find((f) => f.path === "shared.ts"); - expect(shared?.status).toBe("renamed"); - expect(shared?.oldPath).toBe("old/a.ts"); - }); + it("respects a smaller contextLines by splitting distant hunks", async () => { + const oldLines = Array.from({ length: 20 }, (_, i) => `l${i}`); + const newLines = [...oldLines]; + newLines[0] = "CHANGED-0"; + newLines[19] = "CHANGED-19"; + const c1 = await fx.commit("root", { + "f.ts": `${oldLines.join("\n")}\n`, + }); + const c2 = await fx.commit("edit", { + "f.ts": `${newLines.join("\n")}\n`, + }); - it("fills in oldPath when a later rename follows a non-rename entry for the same path", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) { - return "1\t1\tshared.ts"; - } - if (args.includes("--name-status")) { - return ["M\tshared.ts", "R100\told/name.ts\tshared.ts"].join("\n"); - } - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + shaping: { contextLines: 1 }, + }); - const summary = await getDiffSummary(git, { - from: "x", - to: "y", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff.match(/@@/g)?.length).toBe(4); // two hunk headers, 2 lines each }); - const shared = summary.files.find((f) => f.path === "shared.ts"); - expect(shared?.oldPath).toBe("old/name.ts"); - }); + it("drops a whitespace-only change when ignoreWhitespace is set", async () => { + const c1 = await fx.commit("root", { "f.ts": "a\nb\n" }); + const c2 = await fx.commit("edit", { "f.ts": "a\nb \n" }); - it("passes -w to numstat and name-status when ignoreWhitespace is set", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue(""); - - await getDiffSummary(git, { - from: "a", - to: "b", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), - shaping: { ignoreWhitespace: true }, - }); - - expect(run).toHaveBeenCalledWith([ - "diff", - "-w", - "--numstat", - "a..b", - "--", - ".", - ]); - expect(run).toHaveBeenCalledWith([ - "diff", - "-w", - "--name-status", - "a..b", - "--", - ".", - ]); - }); + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + shaping: { ignoreWhitespace: true }, + }); - it("passes -w per-commit when ignoreWhitespace is set with filterByCommits", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue(""); - - await getDiffSummary(git, { - from: "a", - to: "b", - commits: [{ hash: "c1", message: "m" }], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), - shaping: { ignoreWhitespace: true }, + expect(diff).toBe(""); }); - - expect(run).toHaveBeenCalledWith([ - "diff", - "-w", - "--numstat", - "c1^!", - "--", - ".", - ]); }); - it("sets binary: true on files reported as '-' in numstat", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) return "-\t-\timage.png"; - if (args.includes("--name-status")) return "M\timage.png"; - return ""; - }); + describe("getDiff — per-commit mode", () => { + it("drops a whitespace-only change when ignoreWhitespace is set", async () => { + const c1 = await fx.commit("root", { "f.ts": "a\nb\n" }); + const c2 = await fx.commit("edit", { "f.ts": "a\nb \n" }); + + const diff = await getDiff(git, { + from: c1, + to: c2, + commits: [{ hash: c2, message: "edit" }], + filterByCommits: true, + shaping: { ignoreWhitespace: true }, + }); - const summary = await getDiffSummary(git, { - from: "a", - to: "b", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toBe(""); }); - const file = summary.files.find((f) => f.path === "image.png"); - expect(file?.binary).toBe(true); - expect(file?.additions).toBe(0); - expect(file?.deletions).toBe(0); - }); + it("diffs the root commit against the empty tree", async () => { + const c1 = await fx.commit("root", { "a.ts": "hello\n" }); - it("aggregates per-commit summaries", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - const range = args.find((a) => a.endsWith("^!")); - if (args.includes("--numstat")) { - return range?.startsWith("111") ? "1\t1\tf.ts" : ""; - } - if (args.includes("--name-status")) { - return range?.startsWith("111") ? "M\tf.ts" : ""; - } - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c1, + commits: [{ hash: c1, message: "root" }], + filterByCommits: true, + }); - const summary = await getDiffSummary(git, { - from: "a", - to: "b", - commits: [ - { hash: "111aaa", message: "m1" }, - { hash: "222bbb", message: "m2" }, - ], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("new file mode 100644"); + expect(diff).toContain("+hello"); }); - expect(summary.files.some((f) => f.path === "f.ts")).toBe(true); - }); + it("joins independent per-commit patches with a newline", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("c2", { "a.ts": "2\n" }); + const c3 = await fx.commit("c3", { "b.ts": "new\n" }); - it("includes per-commit numstat additions in filterByCommits mode", async () => { - const { git, run } = makeGitWithDiff(); - run.mockImplementation(async (args: string[]) => { - if (args.includes("--numstat")) return "5\t3\tf.ts"; - if (args.includes("--name-status")) return "M\tf.ts"; - return ""; - }); + const diff = await getDiff(git, { + from: c1, + to: c3, + commits: [ + { hash: c2, message: "c2" }, + { hash: c3, message: "c3" }, + ], + filterByCommits: true, + }); - const summary = await getDiffSummary(git, { - from: "a", - to: "b", - commits: [{ hash: "c1", message: "m" }], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(diff).toContain("a.ts"); + expect(diff).toContain("b.ts"); + expect(diff.indexOf("a.ts")).toBeLessThan(diff.indexOf("b.ts")); }); - - const file = summary.files.find((f) => f.path === "f.ts"); - expect(file?.additions).toBe(5); - expect(file?.deletions).toBe(3); }); -}); -describe("getChangedFiles", () => { - it("splits range output on CRLF", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("a.ts\r\nb.ts\r\n"); - - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), - }); + describe("getDiffSummary", () => { + it("summarizes range-mode changes with correct counts and statuses", async () => { + const c1 = await fx.commit("root", { + "a.ts": "one\ntwo\n", + "b.ts": "gone\n", + }); + const c2 = await fx.commit( + "edit", + { "a.ts": "one\ntwoo\n", "c.ts": "new\n" }, + ["b.ts"], + ); - expect(files).toEqual(["a.ts", "b.ts"]); - }); + const summary = await getDiffSummary(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); - it("dedupes files from per-commit show output", async () => { - const { git, run } = makeGitWithDiff(); - run - .mockResolvedValueOnce("dup.ts\n") - .mockResolvedValueOnce("dup.ts\nother.ts\n"); - - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [ - { hash: "c1", message: "1" }, - { hash: "c2", message: "2" }, - ], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(summary.totalFiles).toBe(3); + expect(summary.files.find((f) => f.path === "a.ts")).toMatchObject({ + status: "modified", + additions: 1, + deletions: 1, + }); + expect(summary.files.find((f) => f.path === "b.ts")).toMatchObject({ + status: "deleted", + deletions: 1, + }); + expect(summary.files.find((f) => f.path === "c.ts")).toMatchObject({ + status: "added", + additions: 1, + }); }); - expect(files).toEqual(["dup.ts", "other.ts"]); - }); + it("marks a binary file summary entry with binary: true and zero counts", async () => { + const c1 = await fx.commit("root", { "i.bin": Buffer.from([0, 1]) }); + const c2 = await fx.commit("edit", { "i.bin": Buffer.from([0, 2]) }); - it("sorts range-mode changed files alphabetically", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("z.ts\na.ts\n"); + const summary = await getDiffSummary(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(summary.files[0]).toMatchObject({ + binary: true, + additions: 0, + deletions: 0, + }); }); - expect(files).toEqual(["a.ts", "z.ts"]); - }); + it("aggregates additions/deletions across commits touching the same file", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n2\n3\n" }); + const c2 = await fx.commit("edit1", { "a.ts": "1\nX\n3\n" }); + const c3 = await fx.commit("edit2", { "a.ts": "1\nX\nY\n" }); - it("sorts filterByCommits changed files alphabetically", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("z.ts\na.ts\n"); + const summary = await getDiffSummary(git, { + from: c1, + to: c3, + commits: [ + { hash: c2, message: "edit1" }, + { hash: c3, message: "edit2" }, + ], + filterByCommits: true, + }); - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [{ hash: "c1", message: "1" }], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(summary.totalFiles).toBe(1); + expect(summary.files[0]).toMatchObject({ + path: "a.ts", + status: "modified", + additions: 2, + deletions: 2, + }); }); - - expect(files).toEqual(["a.ts", "z.ts"]); }); - it("trims trailing whitespace from filenames in range mode", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("file.ts \n"); + describe("getChangedFiles", () => { + it("returns a sorted, deduped file list in range mode", async () => { + const c1 = await fx.commit("root", { "z.ts": "1\n", "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "z.ts": "2\n", "a.ts": "2\n" }); - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [{ hash: "h", message: "m" }], - filterByCommits: false, - repoRootOverride: join(__dirname, "fixture-repo"), + const files = await getChangedFiles(git, { + from: c1, + to: c2, + commits: [], + filterByCommits: false, + }); + + expect(files).toEqual(["a.ts", "z.ts"]); }); - expect(files).toEqual(["file.ts"]); - }); + it("dedupes a file touched by multiple commits in per-commit mode", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit1", { "a.ts": "2\n" }); + const c3 = await fx.commit("edit2", { "a.ts": "3\n", "b.ts": "1\n" }); - it("trims trailing whitespace from filenames in filterByCommits mode", async () => { - const { git, run } = makeGitWithDiff(); - run.mockResolvedValue("file.ts \n"); + const files = await getChangedFiles(git, { + from: c1, + to: c3, + commits: [ + { hash: c2, message: "edit1" }, + { hash: c3, message: "edit2" }, + ], + filterByCommits: true, + }); - const files = await getChangedFiles(git, { - from: "a", - to: "b", - commits: [{ hash: "c1", message: "1" }], - filterByCommits: true, - repoRootOverride: join(__dirname, "fixture-repo"), + expect(files).toEqual(["a.ts", "b.ts"]); }); - - expect(files).toEqual(["file.ts"]); }); }); diff --git a/test/gitDiff.spec.ts b/test/gitDiff.spec.ts index 50f9dee..90fbedb 100644 --- a/test/gitDiff.spec.ts +++ b/test/gitDiff.spec.ts @@ -1,118 +1,8 @@ -import { join } from "node:path"; - import { - buildDiffPathspecs, type CommitInfo, filterCommitsByMessageRegexes, - parseDiffSummary, } from "../src/git/gitDiff"; -describe("buildDiffPathspecs", () => { - const repoRoot = join(__dirname, "fixture-repo-root"); - - it("returns whole repo when no filter", () => { - expect(buildDiffPathspecs(repoRoot)).toEqual(["."]); - }); - - it("returns whole repo with exclude pathspecs", () => { - expect( - buildDiffPathspecs(repoRoot, { - excludeFolders: ["node_modules", "dist"], - }), - ).toEqual([".", ":(exclude)node_modules", ":(exclude)dist"]); - }); - - it("uses include folders without dot when specified", () => { - expect( - buildDiffPathspecs(repoRoot, { - includeFolders: ["src", "packages/lib"], - excludeFolders: ["src/generated"], - }), - ).toEqual(["src", "packages/lib", ":(exclude)src/generated"]); - }); - - it("normalizes backslashes to forward slashes", () => { - expect( - buildDiffPathspecs(repoRoot, { includeFolders: ["src\\app"] }), - ).toEqual(["src/app"]); - }); - - it("throws when path escapes the repository", () => { - expect(() => - buildDiffPathspecs(repoRoot, { includeFolders: ["../outside"] }), - ).toThrow(/escapes repository root/); - }); - - it("throws on invalid include regex path via parent segments", () => { - expect(() => - buildDiffPathspecs(repoRoot, { includeFolders: ["foo/../../etc"] }), - ).toThrow(/escapes repository root/); - }); - - it("throws when include path resolves exactly to the parent directory", () => { - expect(() => - buildDiffPathspecs(repoRoot, { includeFolders: [".."] }), - ).toThrow(/escapes repository root/); - }); - - it("throws when exclude path escapes the repository", () => { - expect(() => - buildDiffPathspecs(repoRoot, { excludeFolders: ["../somewhere-else"] }), - ).toThrow(/escapes repository root/); - }); - - it("treats root-like include as empty and still applies excludes", () => { - expect( - buildDiffPathspecs(repoRoot, { - includeFolders: ["/"], - excludeFolders: ["tmp"], - }), - ).toEqual([".", ":(exclude)tmp"]); - }); - - it("normalizes multiple leading slashes to no leading slash", () => { - expect(buildDiffPathspecs(repoRoot, { includeFolders: ["//src"] })).toEqual( - ["src"], - ); - }); - - it("normalizes multiple trailing slashes", () => { - expect(buildDiffPathspecs(repoRoot, { includeFolders: ["src//"] })).toEqual( - ["src"], - ); - }); - - it("filters out empty-string include paths", () => { - expect( - buildDiffPathspecs(repoRoot, { includeFolders: ["", "src"] }), - ).toEqual(["src"]); - }); - - it("filters out empty-string exclude paths", () => { - expect( - buildDiffPathspecs(repoRoot, { excludeFolders: ["", "dist"] }), - ).toEqual([".", ":(exclude)dist"]); - }); - - it("filters out whitespace-only include paths", () => { - expect( - buildDiffPathspecs(repoRoot, { includeFolders: [" ", "src"] }), - ).toEqual(["src"]); - }); - - it("normalizes leading/trailing spaces within an include path", () => { - expect( - buildDiffPathspecs(repoRoot, { includeFolders: [" src "] }), - ).toEqual(["src"]); - }); - - it("root-plus-real path does not produce redundant dot", () => { - expect( - buildDiffPathspecs(repoRoot, { includeFolders: ["/", "src"] }), - ).toEqual(["src"]); - }); -}); - describe("filterCommitsByMessageRegexes", () => { const commits: CommitInfo[] = [ { hash: "a1", message: "feat: add login" }, @@ -204,152 +94,3 @@ describe("filterCommitsByMessageRegexes", () => { expect(out.map((c) => c.hash)).toEqual(["a1", "c3"]); }); }); - -describe("parseDiffSummary", () => { - it("parses modified file line", () => { - const summary = parseDiffSummary("M\t10\t2\tsrc/foo.ts"); - expect(summary.totalFiles).toBe(1); - expect(summary.totalAdditions).toBe(10); - expect(summary.totalDeletions).toBe(2); - expect(summary.files[0]).toMatchObject({ - path: "src/foo.ts", - status: "modified", - additions: 10, - deletions: 2, - }); - }); - - it("merges duplicate paths", () => { - const summary = parseDiffSummary( - ["M\t1\t1\tpath/a.ts", "M\t2\t0\tpath/a.ts"].join("\n"), - ); - expect(summary.totalFiles).toBe(1); - expect(summary.files[0]?.additions).toBe(3); - expect(summary.files[0]?.deletions).toBe(1); - }); - - it("parses rename with old and new path", () => { - const summary = parseDiffSummary("R100\t5\t5\told/name.ts\tnew/name.ts"); - expect(summary.files[0]).toMatchObject({ - path: "new/name.ts", - status: "renamed", - oldPath: "old/name.ts", - }); - }); - - it("ignores malformed lines", () => { - const summary = parseDiffSummary("not-a-summary-line\n\nM\t1\t1\tok.ts"); - expect(summary.totalFiles).toBe(1); - }); - - it("parses added, deleted, copied, type-changed, and dash counts", () => { - const raw = [ - "A\t1\t0\tnew.ts", - "D\t0\t5\tdel.ts", - "C100\t1\t1\to\tc.ts", - "T\t0\t0\tt.ext", - "M\t-\t3\tdash.ts", - ].join("\n"); - const s = parseDiffSummary(raw); - expect(s.files.find((f) => f.path === "new.ts")?.status).toBe("added"); - expect(s.files.find((f) => f.path === "del.ts")?.status).toBe("deleted"); - expect(s.files.find((f) => f.path === "c.ts")?.status).toBe("copied"); - expect(s.files.find((f) => f.path === "t.ext")?.status).toBe( - "type-changed", - ); - expect(s.files.find((f) => f.path === "dash.ts")).toMatchObject({ - additions: 0, - deletions: 3, - }); - }); - - it("maps unknown status token to unknown", () => { - const s = parseDiffSummary("X\t1\t1\tweird.bin"); - expect(s.files[0]?.status).toBe("unknown"); - }); - - it("skips lines with too many tab fields", () => { - const s = parseDiffSummary("M\t1\t1\ta\tb\tc\textra"); - expect(s.totalFiles).toBe(0); - }); - - it("merges conflicting statuses on the same path toward higher-precedence status", () => { - const s = parseDiffSummary( - ["M\t1\t1\tshared.ts", "D\t0\t1\tshared.ts"].join("\n"), - ); - expect(s.files[0]?.status).toBe("deleted"); - }); - - it("keeps higher-precedence existing status when next has lower precedence", () => { - const s = parseDiffSummary( - ["D\t0\t5\tshared.ts", "M\t1\t1\tshared.ts"].join("\n"), - ); - expect(s.files[0]?.status).toBe("deleted"); - }); - - it("treats '-' deletions column as zero", () => { - const s = parseDiffSummary("M\t3\t-\tnodash.ts"); - expect(s.files[0]).toMatchObject({ - path: "nodash.ts", - additions: 3, - deletions: 0, - }); - }); - - it("treats empty additions/deletions columns as zero", () => { - const s = parseDiffSummary("M\t\t\tempty-counts.ts"); - expect(s.files[0]).toMatchObject({ - additions: 0, - deletions: 0, - }); - }); - - it("merges two renames to the same new path, preserving first oldPath", () => { - const s = parseDiffSummary( - [ - "R100\t1\t1\told/a.ts\tnew/shared.ts", - "R100\t2\t2\told/b.ts\tnew/shared.ts", - ].join("\n"), - ); - expect(s.totalFiles).toBe(1); - expect(s.files[0]).toMatchObject({ - path: "new/shared.ts", - status: "renamed", - additions: 3, - deletions: 3, - oldPath: "old/a.ts", - }); - }); - - it("fills in oldPath from later rename when earlier entry had none", () => { - const s = parseDiffSummary( - ["M\t1\t1\tshared.ts", "R100\t2\t2\told/name.ts\tshared.ts"].join("\n"), - ); - expect(s.totalFiles).toBe(1); - expect(s.files[0]).toMatchObject({ - path: "shared.ts", - oldPath: "old/name.ts", - }); - }); - - it("fills in newPath from a later rename when first occurrence was not a rename", () => { - const s = parseDiffSummary( - ["M\t1\t1\tshared.ts", "R100\t2\t2\told/name.ts\tshared.ts"].join("\n"), - ); - expect(s.files[0]).toMatchObject({ - newPath: "shared.ts", - oldPath: "old/name.ts", - }); - }); - - it("parses lines that have leading or trailing whitespace", () => { - const s = parseDiffSummary(" M\t1\t1\tsrc/foo.ts "); - expect(s.totalFiles).toBe(1); - expect(s.files[0]).toMatchObject({ - path: "src/foo.ts", - status: "modified", - additions: 1, - deletions: 1, - }); - }); -}); diff --git a/test/helpers/tsgitFixture.ts b/test/helpers/tsgitFixture.ts new file mode 100644 index 0000000..a7e1cb6 --- /dev/null +++ b/test/helpers/tsgitFixture.ts @@ -0,0 +1,61 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import type { Repository } from "@scolladon/tsgit"; +import { openRepository } from "@scolladon/tsgit"; + +type FixtureFileMap = Record; + +export type FixtureRepo = { + repo: Repository; + dir: string; + /** Write/modify/delete files, stage everything, and commit. Returns the new commit id. */ + commit( + message: string, + files?: FixtureFileMap, + deletePaths?: string[], + ): Promise; + cleanup(): Promise; +}; + +function writeFiles(dir: string, files: FixtureFileMap): void { + for (const [rel, content] of Object.entries(files)) { + const abs = join(dir, rel); + mkdirSync(dirname(abs), { recursive: true }); + writeFileSync(abs, content); + } +} + +/** Real temp-directory git repo backed by tsgit's Node adapter — one integer clock tick per commit for determinism. */ +export async function createFixtureRepo(): Promise { + const dir = mkdtempSync(join(tmpdir(), "smart-diff-fixture-")); + const repo = await openRepository({ cwd: dir }); + await repo.init(); + let timestamp = 1_700_000_000; + + return { + repo, + dir, + async commit(message, files = {}, deletePaths = []) { + writeFiles(dir, files); + for (const p of deletePaths) { + rmSync(join(dir, p), { force: true }); + } + await repo.add([], { all: true }); + const result = await repo.commit({ + message, + author: { + name: "Test", + email: "test@example.com", + timestamp: timestamp++, + timezoneOffset: "+0000", + }, + }); + return result.id; + }, + async cleanup() { + await repo.dispose(); + rmSync(dir, { recursive: true, force: true }); + }, + }; +} diff --git a/test/index.spec.ts b/test/index.spec.ts index 6b6debc..dc3321f 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -1,106 +1,155 @@ import type { LanguageModel } from "ai"; -import type { Mock } from "vitest"; import * as gitDiff from "../src/git/gitDiff"; -import { summarizeGitDiff, summarizeGitDiffWithUsage } from "../src/index"; -import { makeMockModel, makeUsageMockProvider } from "./helpers/mockLlm"; +import { summarizeGitDiff } from "../src/index"; +import { extractUserText, makeMockModel } from "./helpers/mockLlm"; +import { createFixtureRepo, type FixtureRepo } from "./helpers/tsgitFixture"; function mockLlmProvider(text: string): () => Promise { return async () => makeMockModel(text).model; } -describe("summarizeGitDiff integration", () => { - const originalEnv = process.env; +describe("summarizeGitDiff", () => { + let fx: FixtureRepo; - afterEach(() => { + beforeEach(async () => { + fx = await createFixtureRepo(); + }); + + afterEach(async () => { vi.restoreAllMocks(); - process.env = originalEnv; + await fx.cleanup(); }); - it("uses createGitClient when git is omitted", async () => { - const mockGit = { - run: vi.fn().mockImplementation(async (args: string[]) => { - if (args[0] === "log") return "h1\x1fm\n"; - if (args[0] === "rev-parse") return "C:\\repo\n"; - return ""; - }), - }; + it("aggregates git calls and returns LLM summary via llmModelProvider", async () => { + const c1 = await fx.commit("root", { + "src/app.ts": "one\n", + "node_modules/pkg.js": "vendored\n", + }); + const c2 = await fx.commit("chore: noise", { + "src/app.ts": "two\n", + "node_modules/pkg.js": "vendored2\n", + }); + + const md = await summarizeGitDiff({ + from: c1, + to: c2, + git: fx.repo, + teamName: "Infra", + excludeFolders: ["node_modules"], + commitMessageExcludeRegexes: ["^chore:"], + llmModelProvider: mockLlmProvider("# Infra Summary\nBody from model"), + }); - const createSpy = vi - .spyOn(gitDiff, "createGitClient") - .mockReturnValue(mockGit as never); + expect(md).toBe("# Infra Summary\nBody from model"); + }); + + it("uses per-commit diff shape when include regexes are set even if all match", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "a.ts": "2\n" }); + const getDiffSpy = vi.spyOn(gitDiff, "getDiff"); await summarizeGitDiff({ - from: "a", - to: "b", - cwd: "C:\\some\\cwd", - llmModelProvider: mockLlmProvider("summary"), + from: c1, + to: c2, + git: fx.repo, + commitMessageIncludeRegexes: ["."], + llmModelProvider: mockLlmProvider("ok"), }); - expect(createSpy).toHaveBeenCalledWith("C:\\some\\cwd"); - expect(mockGit.run).toHaveBeenCalledWith(expect.arrayContaining(["log"])); - createSpy.mockRestore(); + expect(getDiffSpy).toHaveBeenCalledWith( + fx.repo, + expect.objectContaining({ filterByCommits: true }), + ); }); - it("uses per-commit diff shape when filtered commits differ without regex options", async () => { - vi.spyOn(gitDiff, "getCommits").mockResolvedValue([ - { hash: "1", message: "a" }, - { hash: "2", message: "b" }, - ]); - vi.spyOn(gitDiff, "filterCommitsByMessageRegexes").mockReturnValue([ - { hash: "1", message: "a" }, - ]); - - const run = vi.fn().mockImplementation(async (args: string[]) => { - if (args[0] === "rev-parse") return "C:\\repo\n"; - if (args[0] === "show") return "f.ts\n"; - if (args.includes("--numstat")) return "1\t1\tf.ts"; - if (args.includes("--name-status")) return "M\tf.ts"; - if (args.includes("--name-only")) return "f.ts\n"; - return ""; + it("forwards shaping options through to getDiff's query", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n2\n3\n" }); + const c2 = await fx.commit("edit", { "a.ts": "1\n2b\n3\n" }); + const getDiffSpy = vi.spyOn(gitDiff, "getDiff"); + + await summarizeGitDiff({ + from: c1, + to: c2, + git: fx.repo, + contextLines: 0, + ignoreWhitespace: true, + stripDiffPreamble: true, + maxHunkLines: 200, + llmModelProvider: mockLlmProvider("ok"), }); - const mockGit = { run } as never; - vi.spyOn(gitDiff, "createGitClient").mockReturnValue(mockGit); + expect(getDiffSpy).toHaveBeenCalledWith( + fx.repo, + expect.objectContaining({ + shaping: expect.objectContaining({ + contextLines: 0, + ignoreWhitespace: true, + stripDiffPreamble: true, + maxHunkLines: 200, + }), + }), + ); + }); + + it("merges excludeDefaultNoise into excludeFolders and deduplicates user entries", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "a.ts": "2\n" }); + const getDiffSpy = vi.spyOn(gitDiff, "getDiff"); await summarizeGitDiff({ - from: "x", - to: "y", - cwd: ".", + from: c1, + to: c2, + git: fx.repo, + excludeFolders: ["node_modules", "custom-out"], + excludeDefaultNoise: true, llmModelProvider: mockLlmProvider("ok"), }); - expect(run).toHaveBeenCalledWith(expect.arrayContaining(["1^!"])); + const query = getDiffSpy.mock.calls[0]?.[1]; + const excludes = query?.pathFilter?.excludeFolders ?? []; + expect(excludes).toContain("package-lock.json"); + expect(excludes).toContain("custom-out"); + expect(excludes.filter((e) => e === "node_modules")).toHaveLength(1); }); - it("summarizeGitDiffWithUsage returns the summary alongside aggregated token usage", async () => { - const mockGit = { - run: vi.fn().mockImplementation(async (args: string[]) => { - if (args[0] === "log") return "h1\x1fm\n"; - if (args[0] === "rev-parse") return "C:\\repo\n"; - return ""; - }), - }; - vi.spyOn(gitDiff, "createGitClient").mockReturnValue(mockGit as never); - - const { llmModelProvider } = makeUsageMockProvider([ - { text: "summary", inputTokens: 42, outputTokens: 8 }, - ]); - - const { summary, usage } = await summarizeGitDiffWithUsage({ - from: "a", - to: "b", - cwd: "C:\\some\\cwd", - llmModelProvider, + it("leaves path filter untouched when excludeDefaultNoise is not set", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "a.ts": "2\n" }); + const getDiffSpy = vi.spyOn(gitDiff, "getDiff"); + + await summarizeGitDiff({ + from: c1, + to: c2, + git: fx.repo, + llmModelProvider: mockLlmProvider("ok"), }); - expect(summary).toBe("summary"); - expect(usage).toEqual({ - requestCount: 1, - inputTokens: 42, - outputTokens: 8, - totalTokens: 50, - cachedInputTokens: 0, + const query = getDiffSpy.mock.calls[0]?.[1]; + expect(query?.pathFilter).toBeUndefined(); + }); + + it("actually excludes noise paths from the diff sent to the LLM", async () => { + const c1 = await fx.commit("root", { + "package-lock.json": "{}\n", + "src/app.ts": "1\n", + }); + const c2 = await fx.commit("edit", { + "package-lock.json": '{"a":1}\n', + "src/app.ts": "2\n", + }); + + const { model, calls } = makeMockModel("ok"); + await summarizeGitDiff({ + from: c1, + to: c2, + git: fx.repo, + excludeDefaultNoise: true, + llmModelProvider: async () => model, }); + + const userText = extractUserText(calls()[0]!); + expect(userText).not.toContain("package-lock.json"); + expect(userText).toContain("src/app.ts"); }); }); diff --git a/test/summarizeGitDiff.spec.ts b/test/summarizeGitDiff.spec.ts index 508601f..d194a54 100644 --- a/test/summarizeGitDiff.spec.ts +++ b/test/summarizeGitDiff.spec.ts @@ -1,201 +1,88 @@ -import { join } from "node:path"; import type { LanguageModel } from "ai"; -import type { Mock } from "vitest"; -import type { GitClient } from "../src/git/gitDiff"; -import { summarizeGitDiff } from "../src/index"; -import { makeMockModel } from "./helpers/mockLlm"; - -function createMockGit(repoRoot: string): GitClient & { run: Mock } { - const run = vi.fn().mockImplementation(async (args: string[]) => { - if (args[0] === "log") return `aaa111\x1ffeat: one\nbbb222\x1fchore: noise`; - if (args[0] === "rev-parse") return `${repoRoot}\n`; - if (args[0] === "show") return ""; - if (args.includes("--name-only")) return "src/app.ts\n"; - if (args.includes("--numstat")) return "2\t1\tsrc/app.ts\n"; - if (args.includes("--name-status")) return "M\tsrc/app.ts\n"; - return "diff --git a/src/app.ts\n+ok"; - }); - return { run }; -} + +import * as gitDiff from "../src/git/gitDiff"; +import { summarizeGitDiff, summarizeGitDiffWithUsage } from "../src/index"; +import { makeMockModel, makeUsageMockProvider } from "./helpers/mockLlm"; +import { createFixtureRepo, type FixtureRepo } from "./helpers/tsgitFixture"; function mockLlmProvider(text: string): () => Promise { return async () => makeMockModel(text).model; } -describe("summarizeGitDiff", () => { - const repoRoot = join(__dirname, "fixture-repo-root"); - - it("aggregates git calls and returns LLM summary via llmModelProvider", async () => { - const git = createMockGit(repoRoot); - - const md = await summarizeGitDiff({ - from: "main", - to: "topic", - git, - teamName: "Infra", - excludeFolders: ["node_modules"], - commitMessageExcludeRegexes: ["^chore:"], - llmModelProvider: mockLlmProvider("# Infra Summary\nBody from model"), - }); +describe("summarizeGitDiff integration", () => { + let fx: FixtureRepo; - expect(git.run).toHaveBeenCalledWith([ - "log", - "--format=%H%x1f%s", - "main..topic", - ]); - expect(md).toBe("# Infra Summary\nBody from model"); - expect(git.run).toHaveBeenCalled(); + beforeEach(async () => { + fx = await createFixtureRepo(); }); - it("uses per-commit diff shape when include regexes are set even if all match", async () => { - const git = createMockGit(repoRoot); - - await summarizeGitDiff({ - from: "a", - to: "b", - git, - commitMessageIncludeRegexes: ["."], - llmModelProvider: mockLlmProvider("ok"), - }); - - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const hasPerCommitPatch = diffCalls.some((args) => - args.some((x) => /^\w+\^!$/.test(x)), - ); - expect(hasPerCommitPatch).toBe(true); + afterEach(async () => { + vi.restoreAllMocks(); + await fx.cleanup(); }); - const SUMMARY_MODE_FLAGS = ["--numstat", "--name-status", "--name-only"]; - - function findPatchCall(diffCalls: string[][]): string[] | undefined { - return diffCalls.find( - (args) => - args.includes("a..b") && - !SUMMARY_MODE_FLAGS.some((f) => args.includes(f)), - ); - } + it("uses createGitClient when git is omitted", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "a.ts": "2\n" }); - it("forwards flat shaping options to git diff", async () => { - const git = createMockGit(repoRoot); + const createSpy = vi.spyOn(gitDiff, "createGitClient"); - await summarizeGitDiff({ - from: "a", - to: "b", - git, - contextLines: 0, - ignoreWhitespace: true, - stripDiffPreamble: true, - maxHunkLines: 200, - llmModelProvider: mockLlmProvider("ok"), + const md = await summarizeGitDiff({ + from: c1, + to: c2, + cwd: fx.dir, + llmModelProvider: mockLlmProvider("summary"), }); - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const patchCall = findPatchCall(diffCalls); - expect(patchCall).toEqual(["-U0", "-w", "a..b", "--", "."]); - const numstatCall = diffCalls.find( - (args) => args.includes("--numstat") && args.includes("a..b"), - ); - expect(numstatCall?.[0]).toBe("-w"); + expect(createSpy).toHaveBeenCalledWith(fx.dir); + expect(md).toBe("summary"); }); - it("merges excludeDefaultNoise into excludeFolders and deduplicates user entries", async () => { - const git = createMockGit(repoRoot); + it("uses per-commit diff shape when filtered commits differ without regex options", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + await fx.commit("edit1", { "a.ts": "2\n" }); + const c3 = await fx.commit("edit2", { "a.ts": "3\n" }); - await summarizeGitDiff({ - from: "a", - to: "b", - git, - excludeFolders: ["node_modules", "custom-out"], - excludeDefaultNoise: true, - llmModelProvider: mockLlmProvider("ok"), - }); - - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const patchCall = findPatchCall(diffCalls); - expect(patchCall).toBeDefined(); - const excludes = (patchCall ?? []).filter((a) => - a.startsWith(":(exclude)"), + vi.spyOn(gitDiff, "filterCommitsByMessageRegexes").mockImplementation( + (commits) => commits.slice(0, 1), ); - expect(excludes).toContain(":(exclude)package-lock.json"); - expect(excludes).toContain(":(exclude)custom-out"); - const nodeModulesCount = excludes.filter( - (e) => e === ":(exclude)node_modules", - ).length; - expect(nodeModulesCount).toBe(1); - }); - - it("ignores blank/duplicate entries when merging default noise excludes", async () => { - const git = createMockGit(repoRoot); + const getDiffSpy = vi.spyOn(gitDiff, "getDiff"); await summarizeGitDiff({ - from: "a", - to: "b", - git, - excludeFolders: [" ", "custom-out", "custom-out"], - excludeDefaultNoise: true, + from: c1, + to: c3, + git: fx.repo, llmModelProvider: mockLlmProvider("ok"), }); - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const patchCall = findPatchCall(diffCalls); - const customCount = (patchCall ?? []).filter( - (e) => e === ":(exclude)custom-out", - ).length; - expect(customCount).toBe(1); - }); - - it("merges default noise excludes even when no user excludes are supplied", async () => { - const git = createMockGit(repoRoot); - - await summarizeGitDiff({ - from: "a", - to: "b", - git, - excludeDefaultNoise: true, - llmModelProvider: mockLlmProvider("ok"), - }); - - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const patchCall = findPatchCall(diffCalls); - const excludes = (patchCall ?? []).filter((a) => - a.startsWith(":(exclude)"), + expect(getDiffSpy).toHaveBeenCalledWith( + fx.repo, + expect.objectContaining({ filterByCommits: true }), ); - expect(excludes).toContain(":(exclude)package-lock.json"); }); - it("leaves path filter untouched when excludeDefaultNoise is not set", async () => { - const git = createMockGit(repoRoot); + it("summarizeGitDiffWithUsage returns the summary alongside aggregated token usage", async () => { + const c1 = await fx.commit("root", { "a.ts": "1\n" }); + const c2 = await fx.commit("edit", { "a.ts": "2\n" }); - await summarizeGitDiff({ - from: "a", - to: "b", - git, - llmModelProvider: mockLlmProvider("ok"), + const { llmModelProvider } = makeUsageMockProvider([ + { text: "summary", inputTokens: 42, outputTokens: 8 }, + ]); + + const { summary, usage } = await summarizeGitDiffWithUsage({ + from: c1, + to: c2, + git: fx.repo, + llmModelProvider, }); - const runCalls = (git.run as Mock).mock.calls.map((c) => c[0] as string[]); - const diffCalls = runCalls - .filter((a) => a[0] === "diff") - .map((a) => a.slice(1)); - const patchCall = findPatchCall(diffCalls); - expect(patchCall).toBeDefined(); - expect((patchCall ?? []).some((a) => a.startsWith(":(exclude)"))).toBe( - false, - ); + expect(summary).toBe("summary"); + expect(usage).toEqual({ + requestCount: 1, + inputTokens: 42, + outputTokens: 8, + totalTokens: 50, + cachedInputTokens: 0, + }); }); }); From 87295ffbaca95a45ceee7e709f8c09ffa0a00d3a Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:48:42 -0400 Subject: [PATCH 2/3] fix(test): realpath the fixture temp dir for macOS CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getRepoRoot() returns the canonical path, but macOS symlinks /var to /private/var — the raw mkdtemp path never matched on that platform. Co-Authored-By: Claude Sonnet 5 --- test/helpers/tsgitFixture.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/helpers/tsgitFixture.ts b/test/helpers/tsgitFixture.ts index a7e1cb6..77a87f9 100644 --- a/test/helpers/tsgitFixture.ts +++ b/test/helpers/tsgitFixture.ts @@ -1,4 +1,10 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import type { Repository } from "@scolladon/tsgit"; @@ -28,7 +34,9 @@ function writeFiles(dir: string, files: FixtureFileMap): void { /** Real temp-directory git repo backed by tsgit's Node adapter — one integer clock tick per commit for determinism. */ export async function createFixtureRepo(): Promise { - const dir = mkdtempSync(join(tmpdir(), "smart-diff-fixture-")); + // realpath'd because getRepoRoot() returns the canonical path — macOS's + // /var -> /private/var symlink makes the raw mkdtemp path not match otherwise. + const dir = realpathSync(mkdtempSync(join(tmpdir(), "smart-diff-fixture-"))); const repo = await openRepository({ cwd: dir }); await repo.init(); let timestamp = 1_700_000_000; From be52f0933c9b5422d368045589258d1a2e60f33f Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:53:43 -0400 Subject: [PATCH 3/3] fix(test): use native realpath for Windows 8.3 short-name CI paths realpathSync (JS fallback) resolves symlinks but not NTFS 8.3 short-name aliasing, which is what GitHub's Windows runners hit (TEMP resolves under RUNNER~1 but tsgit's getRepoRoot() returns the long form runneradmin). realpathSync.native calls the OS's own canonicalization and handles both. Also re-normalize both sides at assertion time in the two comparisons, on top of normalizing at fixture-creation time. Co-Authored-By: Claude Sonnet 5 --- test/gitDiff.async.spec.ts | 12 ++++++++++-- test/helpers/tsgitFixture.ts | 9 ++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/test/gitDiff.async.spec.ts b/test/gitDiff.async.spec.ts index 4a75abd..fc5a731 100644 --- a/test/gitDiff.async.spec.ts +++ b/test/gitDiff.async.spec.ts @@ -1,3 +1,4 @@ +import { realpathSync } from "node:fs"; import { createGitClient, type GitClient, @@ -9,6 +10,13 @@ import { } from "../src/git/gitDiff"; import { createFixtureRepo, type FixtureRepo } from "./helpers/tsgitFixture"; +/** Re-normalize both sides at comparison time — belt-and-braces against any + * platform-specific canonical-path quirk (Windows 8.3 short names, macOS + * /var -> /private/var) the fixture's own realpath call didn't fully iron out. */ +function canonical(p: string): string { + return realpathSync.native(p); +} + describe("gitDiffOps against a real tsgit repo", () => { let fx: FixtureRepo; let git: GitClient; @@ -31,14 +39,14 @@ describe("gitDiffOps against a real tsgit repo", () => { it("accepts a timeout and still opens successfully", async () => { const timedGit = await createGitClient(fx.dir, 5000); - expect(await getRepoRoot(timedGit)).toBe(fx.dir); + expect(canonical(await getRepoRoot(timedGit))).toBe(canonical(fx.dir)); await timedGit.dispose(); }); }); describe("getRepoRoot", () => { it("returns the repository working-tree root", async () => { - expect(await getRepoRoot(git)).toBe(fx.dir); + expect(canonical(await getRepoRoot(git))).toBe(canonical(fx.dir)); }); }); diff --git a/test/helpers/tsgitFixture.ts b/test/helpers/tsgitFixture.ts index 77a87f9..96a1af8 100644 --- a/test/helpers/tsgitFixture.ts +++ b/test/helpers/tsgitFixture.ts @@ -34,9 +34,12 @@ function writeFiles(dir: string, files: FixtureFileMap): void { /** Real temp-directory git repo backed by tsgit's Node adapter — one integer clock tick per commit for determinism. */ export async function createFixtureRepo(): Promise { - // realpath'd because getRepoRoot() returns the canonical path — macOS's - // /var -> /private/var symlink makes the raw mkdtemp path not match otherwise. - const dir = realpathSync(mkdtempSync(join(tmpdir(), "smart-diff-fixture-"))); + // realpath'd (native — resolves Windows 8.3 short names too, not just symlinks) + // because getRepoRoot() returns the canonical path: macOS aliases /var to + // /private/var, and CI Windows runners' TEMP can be an 8.3 short-name path. + const dir = realpathSync.native( + mkdtempSync(join(tmpdir(), "smart-diff-fixture-")), + ); const repo = await openRepository({ cwd: dir }); await repo.init(); let timestamp = 1_700_000_000;