From 578c55cfff2775ce99d26d3afd622f71e9597d38 Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Tue, 14 Apr 2026 10:48:56 -0400 Subject: [PATCH 1/3] fix(quality): reduce overall complexity of git functions --- src/ai/aiSummary.ts | 106 ++++--- src/ai/openAIConfig.ts | 48 +++- src/git/commitMessageFilter.ts | 52 ++++ src/git/diffGitStatus.ts | 31 +++ src/git/diffNameStatusParse.ts | 64 +++++ src/git/diffNumstatParse.ts | 44 +++ src/git/diffPathspecs.ts | 68 +++++ src/git/diffSummaryBuild.ts | 57 ++++ src/git/diffSummaryParse.ts | 81 ++++++ src/git/diffTypes.ts | 37 +++ src/git/gitDiff.ts | 488 ++------------------------------- src/git/gitDiffOps.ts | 149 ++++++++++ src/index.ts | 66 +++-- test/aiSummary.spec.ts | 227 ++++++++------- test/gitDiff.async.spec.ts | 217 +++++++++------ test/gitDiff.spec.ts | 187 ++++++++----- test/index.spec.ts | 68 ++--- test/openAIConfig.spec.ts | 167 ++++++----- test/openAiSdk.spec.ts | 15 +- test/summarizeGitDiff.spec.ts | 68 +++-- 20 files changed, 1294 insertions(+), 946 deletions(-) create mode 100644 src/git/commitMessageFilter.ts create mode 100644 src/git/diffGitStatus.ts create mode 100644 src/git/diffNameStatusParse.ts create mode 100644 src/git/diffNumstatParse.ts create mode 100644 src/git/diffPathspecs.ts create mode 100644 src/git/diffSummaryBuild.ts create mode 100644 src/git/diffSummaryParse.ts create mode 100644 src/git/diffTypes.ts create mode 100644 src/git/gitDiffOps.ts diff --git a/src/ai/aiSummary.ts b/src/ai/aiSummary.ts index db21ff8..c3a561e 100644 --- a/src/ai/aiSummary.ts +++ b/src/ai/aiSummary.ts @@ -1,5 +1,9 @@ -import type { CommitInfo, DiffSummary } from '../git/gitDiff.js'; -import { createOpenAiLikeClient, shouldUseLlmGateway, type OpenAiLikeClient } from './openAIConfig.js'; +import type { CommitInfo, DiffSummary } from "../git/gitDiff.js"; +import { + createOpenAiLikeClient, + shouldUseLlmGateway, + type OpenAiLikeClient, +} from "./openAIConfig.js"; /** * Cap on unified-diff characters sent to the LLM (only the diff body; preamble is extra). @@ -9,7 +13,11 @@ const DEFAULT_LLM_MAX_DIFF_CHARS = 120_000; /** Resolve max unified-diff characters for the LLM path. CLI wins, then env, then default. */ export function resolveLlmMaxDiffChars(cliOverride?: number): number { - if (cliOverride !== undefined && Number.isFinite(cliOverride) && cliOverride > 0) { + if ( + cliOverride !== undefined && + Number.isFinite(cliOverride) && + cliOverride > 0 + ) { return Math.trunc(cliOverride); } const raw = process.env.LLM_MAX_DIFF_CHARS?.trim(); @@ -22,7 +30,10 @@ export function resolveLlmMaxDiffChars(cliOverride?: number): number { return DEFAULT_LLM_MAX_DIFF_CHARS; } -export function truncateUnifiedDiffForLlm(diffText: string, maxChars: number): string { +export function truncateUnifiedDiffForLlm( + diffText: string, + maxChars: number, +): string { if (diffText.length <= maxChars) { return diffText; } @@ -41,9 +52,9 @@ If the user message includes a Team line, use that exact team name in the summar /** Thrown when no LLM gateway is configured and no `openAiClientProvider` was passed. */ export const LLM_GATEWAY_REQUIRED_MESSAGE = - 'No LLM gateway configured. Set OPENAI_API_KEY or LLM_API_KEY, and/or LLM_BASE_URL or OPENAI_BASE_URL, ' + - 'and/or JSON in OPENAI_DEFAULT_HEADERS or LLM_DEFAULT_HEADERS. ' + - 'Alternatively pass openAiClientProvider to generateSummary or summarizeGitDiff.'; + "No LLM gateway configured. Set OPENAI_API_KEY or LLM_API_KEY, and/or LLM_BASE_URL or OPENAI_BASE_URL, " + + "and/or JSON in OPENAI_DEFAULT_HEADERS or LLM_DEFAULT_HEADERS. " + + "Alternatively pass openAiClientProvider to generateSummary or summarizeGitDiff."; export type SummarizeFlags = { /** Start ref for the diff. */ @@ -68,7 +79,7 @@ export async function generateSummary( commits: CommitInfo[], flags: SummarizeFlags, openAiClientProvider?: OpenAiClientProvider, - diffSummary?: DiffSummary + diffSummary?: DiffSummary, ): Promise { if (!shouldUseLlmGateway() && openAiClientProvider === undefined) { throw new Error(LLM_GATEWAY_REQUIRED_MESSAGE); @@ -76,36 +87,47 @@ export async function generateSummary( const maxDiffChars = resolveLlmMaxDiffChars(flags.maxDiffChars); const diffForLlm = truncateUnifiedDiffForLlm(diffText, maxDiffChars); - const userContent = buildOpenAiUserContent(flags, commits, fileNames, diffForLlm, diffSummary); + const userContent = buildOpenAiUserContent( + flags, + commits, + fileNames, + diffForLlm, + diffSummary, + ); return callOpenAi( userContent, - flags.model ?? 'gpt-4o-mini', + flags.model ?? "gpt-4o-mini", flags.systemPrompt ?? DEFAULT_GIT_DIFF_SYSTEM_PROMPT, openAiClientProvider ?? - /* istanbul ignore next */ (async (): Promise => createOpenAiLikeClient()) + /* istanbul ignore next */ (async (): Promise => + createOpenAiLikeClient()), ); } function formatRegexFilterLines(flags: SummarizeFlags): string { - const includes = (flags.commitMessageIncludeRegexes ?? []).map((s) => s.trim()).filter(Boolean); - const excludes = (flags.commitMessageExcludeRegexes ?? []).map((s) => s.trim()).filter(Boolean); + const includes = (flags.commitMessageIncludeRegexes ?? []) + .map((s) => s.trim()) + .filter(Boolean); + const excludes = (flags.commitMessageExcludeRegexes ?? []) + .map((s) => s.trim()) + .filter(Boolean); const incLine = includes.length > 0 - ? `Commit message include regexes (OR): ${includes.map((r) => JSON.stringify(r)).join(', ')}\n` - : ''; + ? `Commit message include regexes (OR): ${includes.map((r) => JSON.stringify(r)).join(", ")}\n` + : ""; const excLine = excludes.length > 0 - ? `Commit message exclude regexes: ${excludes.map((r) => JSON.stringify(r)).join(', ')}\n` - : ''; + ? `Commit message exclude regexes: ${excludes.map((r) => JSON.stringify(r)).join(", ")}\n` + : ""; if (!incLine && !excLine) { - return 'Commit message filters: none.\nGit context shape: single unified diff for the full ref range.\n'; + return "Commit message filters: none.\nGit context shape: single unified diff for the full ref range.\n"; } return ( `${incLine}${excLine}` + - 'Git context shape: concatenated per-commit unified patches for commits that pass the message filters.\n' + "Git context shape: concatenated per-commit unified patches for commits that pass the message filters.\n" ); } @@ -114,37 +136,43 @@ function buildOpenAiUserContent( commits: CommitInfo[], fileNames: string[], diffText: string, - diffSummary?: DiffSummary + diffSummary?: DiffSummary, ): string { const from = flags.from; - const to = flags.to ?? 'HEAD'; + const to = flags.to ?? "HEAD"; const team = flags.team?.trim(); const ts = new Date().toISOString(); - const teamLine = team ? `Team: ${team}\n` : ''; + const teamLine = team ? `Team: ${team}\n` : ""; const filterBlock = formatRegexFilterLines(flags); const commitBlock = commits.length > 0 - ? commits.map((c) => `- ${c.hash.slice(0, 7)} ${c.message.replace(/\r?\n/g, ' ')}`).join('\n') - : '- (no commits in range after filtering)'; - - const pathsBlock = fileNames.length > 0 ? fileNames.join('\n') : '(no paths in diff scope)'; + ? commits + .map( + (c) => + `- ${c.hash.slice(0, 7)} ${c.message.replace(/\r?\n/g, " ")}`, + ) + .join("\n") + : "- (no commits in range after filtering)"; + + const pathsBlock = + fileNames.length > 0 ? fileNames.join("\n") : "(no paths in diff scope)"; const structuredDiffSection = diffSummary ? `=== Structured git context (JSON summary) ===\n${JSON.stringify(diffSummary, null, 2)}\n\n` - : ''; + : ""; return ( `${teamLine}` + `Date: ${ts}\n\n` + `Git refs: ${from}..${to}\n` + filterBlock + - '\n' + - '=== Included commits (subject lines) ===\n' + + "\n" + + "=== Included commits (subject lines) ===\n" + `${commitBlock}\n\n` + - '=== Changed paths ===\n' + + "=== Changed paths ===\n" + `${pathsBlock}\n\n` + structuredDiffSection + - '=== Git context (unified diff(s); patches may be truncated with an explicit marker) ===\n' + + "=== Git context (unified diff(s); patches may be truncated with an explicit marker) ===\n" + diffText ); } @@ -153,22 +181,24 @@ async function callOpenAi( userContent: string, model: string, systemPrompt: string, - openAiClientProvider: OpenAiClientProvider + openAiClientProvider: OpenAiClientProvider, ): Promise { const client = await openAiClientProvider(); - const maxTokensRaw = process.env.LLM_MAX_TOKENS ?? process.env.OPENAI_MAX_TOKENS; - const parsed = maxTokensRaw !== undefined ? Number.parseInt(maxTokensRaw, 10) : 4000; + const maxTokensRaw = + process.env.LLM_MAX_TOKENS ?? process.env.OPENAI_MAX_TOKENS; + const parsed = + maxTokensRaw !== undefined ? Number.parseInt(maxTokensRaw, 10) : 4000; const maxTokens = Number.isFinite(parsed) && parsed > 0 ? parsed : 4000; const response = await client.chat.completions.create({ model, messages: [ { - role: 'system', + role: "system", content: systemPrompt, }, { - role: 'user', + role: "user", content: userContent, }, ], @@ -182,6 +212,6 @@ async function callOpenAi( choices?: Array<{ message?: { content?: string } }>; }; - const text = typedResponse.choices?.[0]?.message?.content?.trim() ?? ''; - return text.length > 0 ? text : 'No summary generated by OpenAI.'; + const text = typedResponse.choices?.[0]?.message?.content?.trim() ?? ""; + return text.length > 0 ? text : "No summary generated by OpenAI."; } diff --git a/src/ai/openAIConfig.ts b/src/ai/openAIConfig.ts index 4af31ec..0684d15 100644 --- a/src/ai/openAIConfig.ts +++ b/src/ai/openAIConfig.ts @@ -5,20 +5,28 @@ /** `LLM_BASE_URL` overrides `OPENAI_BASE_URL` when set. */ export function resolveLlmBaseUrl(): string | undefined { - return process.env.LLM_BASE_URL?.trim() ?? process.env.OPENAI_BASE_URL?.trim(); + return ( + process.env.LLM_BASE_URL?.trim() ?? process.env.OPENAI_BASE_URL?.trim() + ); } -function parseHeaderJsonObject(raw: string | undefined): Record { +function parseHeaderJsonObject( + raw: string | undefined, +): Record { const trimmed = raw?.trim(); if (!trimmed) return {}; try { const parsed = JSON.parse(trimmed) as unknown; - if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + if ( + typeof parsed !== "object" || + parsed === null || + Array.isArray(parsed) + ) { return {}; } const out: Record = {}; for (const [key, value] of Object.entries(parsed)) { - if (typeof value === 'string' && value.length > 0) { + if (typeof value === "string" && value.length > 0) { out[key] = value; } } @@ -31,15 +39,19 @@ function parseHeaderJsonObject(raw: string | undefined): Record /** * Merged default headers: `OPENAI_DEFAULT_HEADERS` first, then `LLM_DEFAULT_HEADERS` overrides. */ -export function parseLlmDefaultHeadersFromEnv(): Record | undefined { +export function parseLlmDefaultHeadersFromEnv(): + | Record + | undefined { const base = parseHeaderJsonObject(process.env.OPENAI_DEFAULT_HEADERS); const override = parseHeaderJsonObject(process.env.LLM_DEFAULT_HEADERS); const merged = { ...base, ...override }; return Object.keys(merged).length > 0 ? merged : undefined; } -function findAuthorizationHeaderName(headers: Record): string | undefined { - return Object.keys(headers).find((k) => k.toLowerCase() === 'authorization'); +function findAuthorizationHeaderName( + headers: Record, +): string | undefined { + return Object.keys(headers).find((k) => k.toLowerCase() === "authorization"); } /** Strip a single `Bearer ` prefix; otherwise return the trimmed value. */ @@ -56,7 +68,9 @@ function stripBearerPrefix(value: string): string { * (`param: api_key`). When no `LLM_API_KEY` / `OPENAI_API_KEY` is set, promote recognizable * tokens from `Authorization` into `apiKey` and drop that header from `defaultHeaders`. */ -export function splitPromotableAuthorizationFromHeaders(headers: Record): { +export function splitPromotableAuthorizationFromHeaders( + headers: Record, +): { defaultHeaders: Record; apiKeyFromAuthHeader?: string; } { @@ -80,7 +94,8 @@ export function splitPromotableAuthorizationFromHeaders(headers: Record | undefined; let apiKey = envApiKey; @@ -116,13 +132,17 @@ export function resolveOpenAiLikeClientInit(): OpenAiLikeClientInit { if (split.apiKeyFromAuthHeader) { apiKey = split.apiKeyFromAuthHeader; } - defaultHeaders = Object.keys(split.defaultHeaders).length > 0 ? split.defaultHeaders : undefined; + defaultHeaders = + Object.keys(split.defaultHeaders).length > 0 + ? split.defaultHeaders + : undefined; } else { - defaultHeaders = Object.keys(mergedHeaders).length > 0 ? mergedHeaders : undefined; + defaultHeaders = + Object.keys(mergedHeaders).length > 0 ? mergedHeaders : undefined; } return { - apiKey: apiKey.length > 0 ? apiKey : 'unused', + apiKey: apiKey.length > 0 ? apiKey : "unused", ...(baseURL ? { baseURL } : {}), ...(defaultHeaders ? { defaultHeaders } : {}), }; @@ -130,6 +150,6 @@ export function resolveOpenAiLikeClientInit(): OpenAiLikeClientInit { /** Build options for `new OpenAI(...)` (official OpenAI Node SDK). */ export async function createOpenAiLikeClient(): Promise { - const { default: OpenAI } = await import('openai'); + const { default: OpenAI } = await import("openai"); return new OpenAI(resolveOpenAiLikeClientInit()) as OpenAiLikeClient; } diff --git a/src/git/commitMessageFilter.ts b/src/git/commitMessageFilter.ts new file mode 100644 index 0000000..178a2c7 --- /dev/null +++ b/src/git/commitMessageFilter.ts @@ -0,0 +1,52 @@ +import type { CommitInfo } from "./diffTypes.js"; + +function compileRegex(pattern: string, label: string): RegExp { + try { + return new RegExp(pattern, "i"); + } catch { + throw new Error( + `Invalid ${label} regular expression: ${JSON.stringify(pattern)}`, + ); + } +} + +function commitMessagePassesFilters( + message: string, + includeRes: RegExp[], + excludeRes: RegExp[], +): boolean { + for (const ex of excludeRes) { + if (ex.test(message)) return false; + } + if (includeRes.length > 0 && !includeRes.some((r) => r.test(message))) + return false; + return true; +} + +/** + * Filter commits by message. Excludes are applied first; then if `includePatterns` is non-empty, + * the message must match at least one include pattern. + */ +export function filterCommitsByMessageRegexes( + commits: CommitInfo[], + includePatterns?: string[], + excludePatterns?: string[], +): CommitInfo[] { + const includes = (includePatterns ?? []) + .map((p) => p.trim()) + .filter((p) => p.length > 0); + const excludes = (excludePatterns ?? []) + .map((p) => p.trim()) + .filter((p) => p.length > 0); + + const includeRes = includes.map((p, i) => + compileRegex(p, `commit message include pattern[${i}]`), + ); + const excludeRes = excludes.map((p, i) => + compileRegex(p, `commit message exclude pattern[${i}]`), + ); + + return commits.filter((c) => + commitMessagePassesFilters(c.message, includeRes, excludeRes), + ); +} diff --git a/src/git/diffGitStatus.ts b/src/git/diffGitStatus.ts new file mode 100644 index 0000000..5f58461 --- /dev/null +++ b/src/git/diffGitStatus.ts @@ -0,0 +1,31 @@ +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", +}; + +export function mapGitStatus(statusCode: string): DiffStatus { + return GIT_STATUS_BY_FIRST_CHAR[statusCode.charAt(0)] ?? "unknown"; +} + +export function mergeStatus(existing: DiffStatus, next: DiffStatus): DiffStatus { + if (existing === next) return existing; + const precedence: DiffStatus[] = [ + "deleted", + "added", + "renamed", + "copied", + "type-changed", + "modified", + "unknown", + ]; + return precedence.indexOf(existing) <= precedence.indexOf(next) + ? existing + : next; +} diff --git a/src/git/diffNameStatusParse.ts b/src/git/diffNameStatusParse.ts new file mode 100644 index 0000000..b489852 --- /dev/null +++ b/src/git/diffNameStatusParse.ts @@ -0,0 +1,64 @@ +import type { DiffStatus } from "./diffTypes.js"; +import { mapGitStatus, mergeStatus } from "./diffGitStatus.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) { + const oldPath = parts[1]; + const newPath = parts[2]; + if (oldPath !== undefined && newPath !== undefined) { + entry = { path: newPath, status, oldPath }; + } + } else if (!isRenameOrCopy) { + const pathOnly = parts[1]; + if (pathOnly !== undefined) { + entry = { path: pathOnly, status }; + } + } + } + + return entry; +} + +export function parseNameStatusLines(nameStatusOutput: string): ParsedNameEntry[] { + const entries: ParsedNameEntry[] = []; + for (const rawLine of nameStatusOutput.split(/\r?\n/)) { + const line = rawLine.trim(); + 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 new file mode 100644 index 0000000..9239545 --- /dev/null +++ b/src/git/diffNumstatParse.ts @@ -0,0 +1,44 @@ +/** Map numstat path field (including `{old => new}` rename form) to the post-change path used as lookup key. */ +function numStatPathToLookupKey(pathField: string): string { + const brace = /^(.*)\{(.+) => (.+)\}$/.exec(pathField); + if (!brace) { + return pathField; + } + const dirRaw = brace[1]; + const toSeg = brace[3].trim(); + return `${dirRaw}${toSeg}`; +} + +function parseNumStatLine( + line: string, +): { key: string; additions: number; deletions: number } | null { + const parts = line.split("\t"); + if (parts.length < 3) return null; + + const addStr = parts[0] ?? ""; + const delStr = parts[1] ?? ""; + const pathField = parts.slice(2).join("\t"); + + const additions = addStr !== "-" ? Number.parseInt(addStr, 10) || 0 : 0; + const deletions = delStr !== "-" ? Number.parseInt(delStr, 10) || 0 : 0; + + const key = numStatPathToLookupKey(pathField); + return { key, additions, deletions }; +} + +export function accumulateNumStat( + numStatOutput: string, + into: Map, +): void { + for (const rawLine of numStatOutput.split(/\r?\n/)) { + const line = rawLine.trim(); + if (!line) continue; + const parsed = parseNumStatLine(line); + if (!parsed) continue; + const prev = into.get(parsed.key) ?? { additions: 0, deletions: 0 }; + into.set(parsed.key, { + additions: prev.additions + parsed.additions, + deletions: prev.deletions + parsed.deletions, + }); + } +} diff --git a/src/git/diffPathspecs.ts b/src/git/diffPathspecs.ts new file mode 100644 index 0000000..5dd764c --- /dev/null +++ b/src/git/diffPathspecs.ts @@ -0,0 +1,68 @@ +import { resolve, relative } from "node:path"; + +import type { DiffPathFilter } from "./diffTypes.js"; + +function normalizeRepoRelativePath(p: string): string { + const trimmed = p.trim().replace(/\\/g, "/"); + const noLeading = trimmed.replace(/^\/+/, ""); + const noTrailingSlash = noLeading.replace(/\/+$/, ""); + return noTrailingSlash.length > 0 ? noTrailingSlash : "."; +} + +function assertPathUnderRepo(repoRoot: string, userPath: string): void { + const abs = resolve(repoRoot, userPath); + const rel = relative(repoRoot, abs); + if (rel === "..") { + throw new Error( + `Path escapes repository root: ${JSON.stringify(userPath)}`, + ); + } + const segments = rel.split(/[/\\]/); + if (segments.includes("..")) { + throw new Error( + `Path escapes repository root: ${JSON.stringify(userPath)}`, + ); + } +} + +/** + * 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. + */ +export function buildDiffPathspecs( + repoRoot: string, + pathFilter?: DiffPathFilter, +): string[] { + const includeRaw = + pathFilter?.includeFolders?.filter((p) => p.trim().length > 0) ?? []; + const excludeRaw = + pathFilter?.excludeFolders?.filter((p) => p.trim().length > 0) ?? []; + + const includes = includeRaw + .map(normalizeRepoRelativePath) + .filter((p) => p !== "." && p !== ""); + const excludes = excludeRaw + .map(normalizeRepoRelativePath) + .filter((p) => p !== "." && p !== ""); + + const toValidate = includes.length > 0 ? includes : ["."]; + for (const inc of toValidate) { + assertPathUnderRepo(repoRoot, inc); + } + for (const exc of excludes) { + 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; +} diff --git a/src/git/diffSummaryBuild.ts b/src/git/diffSummaryBuild.ts new file mode 100644 index 0000000..e91307a --- /dev/null +++ b/src/git/diffSummaryBuild.ts @@ -0,0 +1,57 @@ +import type { DiffStatus, DiffSummary } from "./diffTypes.js"; +import { + mergeNameEntriesByPath, + parseNameStatusLines, + type ParsedNameEntry, +} from "./diffNameStatusParse.js"; +import { accumulateNumStat } from "./diffNumstatParse.js"; +import { parseDiffSummary } from "./diffSummaryParse.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(); + 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)); + } + + return parseDiffSummary(syntheticLines.join("\n")); +} diff --git a/src/git/diffSummaryParse.ts b/src/git/diffSummaryParse.ts new file mode 100644 index 0000000..39ba065 --- /dev/null +++ b/src/git/diffSummaryParse.ts @@ -0,0 +1,81 @@ +import type { DiffFileSummary, DiffStatus, DiffSummary } from "./diffTypes.js"; +import { mapGitStatus, mergeStatus } from "./diffGitStatus.js"; + +type ParsedDiffSummaryLine = { + status: DiffStatus; + additions: number; + deletions: number; + oldPath?: string; + newPath: string; +}; + +function parseTabDiffSummaryLine(line: string): ParsedDiffSummaryLine | null { + const parts = line.split("\t"); + if (parts.length < 3) return null; + + const statusToken = parts.shift() ?? ""; + const status = mapGitStatus(statusToken); + const add0 = parts[0]; + const del0 = parts[1]; + const additions = add0 && add0 !== "-" ? Number.parseInt(add0, 10) || 0 : 0; + 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); + 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(); + 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/diffTypes.ts b/src/git/diffTypes.ts new file mode 100644 index 0000000..f2c0e91 --- /dev/null +++ b/src/git/diffTypes.ts @@ -0,0 +1,37 @@ +export type CommitInfo = { + hash: string; + message: string; +}; + +export type DiffStatus = + | "added" + | "deleted" + | "modified" + | "renamed" + | "copied" + | "type-changed" + | "unknown"; + +export type DiffFileSummary = { + path: string; + status: DiffStatus; + additions: number; + deletions: number; + oldPath?: string; + newPath?: string; +}; + +export type DiffSummary = { + files: DiffFileSummary[]; + totalFiles: number; + totalAdditions: number; + totalDeletions: number; +}; + +/** Restrict or exclude paths for `git diff` / `git show`, relative to repository root as users see them (e.g. `src`, `node_modules`). */ +export type DiffPathFilter = { + /** If set and non-empty, only these paths (under the repo root) are included. If omitted or empty, the whole repository is included (subject to `excludeFolders`). */ + includeFolders?: string[]; + /** Paths under the repo root excluded from the diff (git `:(exclude)` pathspecs). */ + excludeFolders?: string[]; +}; diff --git a/src/git/gitDiff.ts b/src/git/gitDiff.ts index 4586f6f..187be87 100644 --- a/src/git/gitDiff.ts +++ b/src/git/gitDiff.ts @@ -1,469 +1,19 @@ -import { resolve, relative } from 'node:path'; -import { simpleGit } from 'simple-git'; -import type { SimpleGit } from 'simple-git'; - -export type CommitInfo = { - hash: string; - message: string; -}; - -export type DiffStatus = 'added' | 'deleted' | 'modified' | 'renamed' | 'copied' | 'type-changed' | 'unknown'; - -export type DiffFileSummary = { - path: string; - status: DiffStatus; - additions: number; - deletions: number; - oldPath?: string; - newPath?: string; -}; - -export type DiffSummary = { - files: DiffFileSummary[]; - totalFiles: number; - totalAdditions: number; - totalDeletions: number; -}; - -/** Restrict or exclude paths for `git diff` / `git show`, relative to repository root as users see them (e.g. `src`, `node_modules`). */ -export type DiffPathFilter = { - /** If set and non-empty, only these paths (under the repo root) are included. If omitted or empty, the whole repository is included (subject to `excludeFolders`). */ - includeFolders?: string[]; - /** Paths under the repo root excluded from the diff (git `:(exclude)` pathspecs). */ - excludeFolders?: string[]; -}; - -export function createGitClient(cwd = process.cwd()): SimpleGit { - return simpleGit(cwd); -} - -export async function getCommits(git: SimpleGit, from: string, to: string): Promise { - const logResult = await git.log({ from, to }); - return logResult.all as unknown as CommitInfo[]; -} - -function compileRegex(pattern: string, label: string): RegExp { - try { - return new RegExp(pattern, 'i'); - } catch { - throw new Error(`Invalid ${label} regular expression: ${JSON.stringify(pattern)}`); - } -} - -function commitMessagePassesFilters(message: string, includeRes: RegExp[], excludeRes: RegExp[]): boolean { - for (const ex of excludeRes) { - if (ex.test(message)) return false; - } - if (includeRes.length > 0 && !includeRes.some((r) => r.test(message))) return false; - return true; -} - -/** - * Filter commits by message. Excludes are applied first; then if `includePatterns` is non-empty, - * the message must match at least one include pattern. - */ -export function filterCommitsByMessageRegexes( - commits: CommitInfo[], - includePatterns?: string[], - excludePatterns?: string[] -): CommitInfo[] { - const includes = (includePatterns ?? []).map((p) => p.trim()).filter((p) => p.length > 0); - const excludes = (excludePatterns ?? []).map((p) => p.trim()).filter((p) => p.length > 0); - - const includeRes = includes.map((p, i) => compileRegex(p, `commit message include pattern[${i}]`)); - const excludeRes = excludes.map((p, i) => compileRegex(p, `commit message exclude pattern[${i}]`)); - - return commits.filter((c) => commitMessagePassesFilters(c.message, includeRes, excludeRes)); -} - -export async function getRepoRoot(git: SimpleGit): Promise { - const root = await git.revparse(['--show-toplevel']); - return root.trim(); -} - -function normalizeRepoRelativePath(p: string): string { - const trimmed = p.trim().replace(/\\/g, '/'); - const noLeading = trimmed.replace(/^\/+/, ''); - const noTrailingSlash = noLeading.replace(/\/+$/, ''); - return noTrailingSlash.length > 0 ? noTrailingSlash : '.'; -} - -function assertPathUnderRepo(repoRoot: string, userPath: string): void { - const abs = resolve(repoRoot, userPath); - const rel = relative(repoRoot, abs); - if (rel === '..') { - throw new Error(`Path escapes repository root: ${JSON.stringify(userPath)}`); - } - const segments = rel.split(/[/\\]/); - if (segments.includes('..')) { - throw new Error(`Path escapes repository root: ${JSON.stringify(userPath)}`); - } -} - -/** - * 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. - */ -export function buildDiffPathspecs(repoRoot: string, pathFilter?: DiffPathFilter): string[] { - const includeRaw = pathFilter?.includeFolders?.filter((p) => p.trim().length > 0) ?? []; - const excludeRaw = pathFilter?.excludeFolders?.filter((p) => p.trim().length > 0) ?? []; - - const includes = includeRaw.map(normalizeRepoRelativePath).filter((p) => p !== '.' && p !== ''); - const excludes = excludeRaw.map(normalizeRepoRelativePath).filter((p) => p !== '.' && p !== ''); - - const toValidate = includes.length > 0 ? includes : ['.']; - for (const inc of toValidate) { - assertPathUnderRepo(repoRoot, inc); - } - for (const exc of excludes) { - 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; -} - -type DiffPathContext = { - repoRoot: string; - specs: string[]; -}; - -async function getDiffPathContext(git: SimpleGit, pathFilter: DiffPathFilter | undefined, repoRootOverride?: string): Promise { - const repoRoot = repoRootOverride ?? (await getRepoRoot(git)); - const specs = buildDiffPathspecs(repoRoot, pathFilter); - return { repoRoot, specs }; -} - -export async function getDiff( - git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string -): Promise { - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); - - if (!filterByCommits) { - return git.diff([`${from}..${to}`, '--', ...specs]); - } - - const patches = await Promise.all(commits.map((c) => git.diff([`${c.hash}^!`, '--', ...specs]))); - - return patches.filter(Boolean).join('\n'); -} - -export async function getDiffSummary( - git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string -): Promise { - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); - - if (!filterByCommits) { - const [numOutput, nameOutput] = await Promise.all([ - git.diff(['--numstat', `${from}..${to}`, '--', ...specs]), - git.diff(['--name-status', `${from}..${to}`, '--', ...specs]), - ]); - return buildDiffSummaryFromGitOutputs(nameOutput, numOutput); - } - - const pairs = await Promise.all( - commits.map(async (c) => { - const range = `${c.hash}^!`; - const [numOutput, nameOutput] = await Promise.all([ - git.diff(['--numstat', range, '--', ...specs]), - git.diff(['--name-status', range, '--', ...specs]), - ]); - return { numOutput, nameOutput }; - }) - ); - 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( - git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string -): Promise { - const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); - - if (!filterByCommits) { - const output = await git.diff(['--name-only', `${from}..${to}`, '--', ...specs]); - - return output - .split(/\r?\n/) - .map((f) => f.trim()) - .filter(Boolean); - } - - const fileSet = new Set(); - - await Promise.all( - commits.map(async (c) => { - const output = await git.show(['--name-only', '--pretty=format:', c.hash, '--', ...specs]); - - output - .split(/\r?\n/) - .map((f) => f.trim()) - .filter(Boolean) - .forEach((f) => fileSet.add(f)); - }) - ); - - return Array.from(fileSet); -} - -/** 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', -}; - -function mapGitStatus(statusCode: string): DiffStatus { - return GIT_STATUS_BY_FIRST_CHAR[statusCode.charAt(0)] ?? 'unknown'; -} - -function mergeStatus(existing: DiffStatus, next: DiffStatus): DiffStatus { - if (existing === next) return existing; - const precedence: DiffStatus[] = ['deleted', 'added', 'renamed', 'copied', 'type-changed', 'modified', 'unknown']; - return precedence.indexOf(existing) <= precedence.indexOf(next) ? existing : next; -} - -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) { - const oldPath = parts[1]; - const newPath = parts[2]; - if (oldPath !== undefined && newPath !== undefined) { - entry = { path: newPath, status, oldPath }; - } - } else if (!isRenameOrCopy) { - const pathOnly = parts[1]; - if (pathOnly !== undefined) { - entry = { path: pathOnly, status }; - } - } - } - - return entry; -} - -function parseNameStatusLines(nameStatusOutput: string): ParsedNameEntry[] { - const entries: ParsedNameEntry[] = []; - for (const rawLine of nameStatusOutput.split(/\r?\n/)) { - const line = rawLine.trim(); - if (!line) continue; - const entry = parseNameStatusLine(line); - if (entry) entries.push(entry); - } - return entries; -} - -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; -} - -/** Map numstat path field (including `{old => new}` rename form) to the post-change path used as lookup key. */ -function numStatPathToLookupKey(pathField: string): string { - const brace = /^(.*)\{(.+) => (.+)\}$/.exec(pathField); - if (!brace) { - return pathField; - } - const dirRaw = brace[1]; - const toSeg = brace[3].trim(); - return `${dirRaw}${toSeg}`; -} - -function parseNumStatLine(line: string): { key: string; additions: number; deletions: number } | null { - const parts = line.split('\t'); - if (parts.length < 3) return null; - - const addStr = parts[0] ?? ''; - const delStr = parts[1] ?? ''; - const pathField = parts.slice(2).join('\t'); - - const additions = addStr !== '-' ? Number.parseInt(addStr, 10) || 0 : 0; - const deletions = delStr !== '-' ? Number.parseInt(delStr, 10) || 0 : 0; - - const key = numStatPathToLookupKey(pathField); - return { key, additions, deletions }; -} - -function accumulateNumStat(numStatOutput: string, into: Map): void { - for (const rawLine of numStatOutput.split(/\r?\n/)) { - const line = rawLine.trim(); - if (!line) continue; - const parsed = parseNumStatLine(line); - if (!parsed) continue; - const prev = into.get(parsed.key) ?? { additions: 0, deletions: 0 }; - into.set(parsed.key, { additions: prev.additions + parsed.additions, deletions: prev.deletions + parsed.deletions }); - } -} - -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. - */ -function buildDiffSummaryFromGitOutputs(nameStatusOutput: string, numStatOutput: string): DiffSummary { - const numMap = new Map(); - 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)); - } - - return parseDiffSummary(syntheticLines.join('\n')); -} - -type ParsedDiffSummaryLine = { - status: DiffStatus; - additions: number; - deletions: number; - oldPath?: string; - newPath: string; -}; - -function parseTabDiffSummaryLine(line: string): ParsedDiffSummaryLine | null { - const parts = line.split('\t'); - if (parts.length < 3) return null; - - const statusToken = parts.shift() ?? ''; - const status = mapGitStatus(statusToken); - const add0 = parts[0]; - const del0 = parts[1]; - const additions = add0 && add0 !== '-' ? Number.parseInt(add0, 10) || 0 : 0; - 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); - 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(); - 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), - }; -} +export type { + CommitInfo, + DiffFileSummary, + DiffPathFilter, + DiffStatus, + DiffSummary, +} from "./diffTypes.js"; + +export { buildDiffPathspecs } from "./diffPathspecs.js"; +export { filterCommitsByMessageRegexes } from "./commitMessageFilter.js"; +export { + createGitClient, + getChangedFiles, + getCommits, + getDiff, + getDiffSummary, + getRepoRoot, +} from "./gitDiffOps.js"; +export { parseDiffSummary } from "./diffSummaryParse.js"; diff --git a/src/git/gitDiffOps.ts b/src/git/gitDiffOps.ts new file mode 100644 index 0000000..ba72eec --- /dev/null +++ b/src/git/gitDiffOps.ts @@ -0,0 +1,149 @@ +import { simpleGit } from "simple-git"; +import type { SimpleGit } from "simple-git"; + +import type { CommitInfo, DiffPathFilter, DiffSummary } from "./diffTypes.js"; +import { buildDiffPathspecs } from "./diffPathspecs.js"; +import { buildDiffSummaryFromGitOutputs } from "./diffSummaryBuild.js"; + +export function createGitClient(cwd = process.cwd()): SimpleGit { + return simpleGit(cwd); +} + +export async function getCommits( + git: SimpleGit, + from: string, + to: string, +): Promise { + const logResult = await git.log({ from, to }); + return logResult.all as unknown as CommitInfo[]; +} + +export async function getRepoRoot(git: SimpleGit): Promise { + const root = await git.revparse(["--show-toplevel"]); + return root.trim(); +} + +type DiffPathContext = { + repoRoot: string; + specs: string[]; +}; + +async function getDiffPathContext( + git: SimpleGit, + pathFilter: DiffPathFilter | undefined, + repoRootOverride?: string, +): Promise { + const repoRoot = repoRootOverride ?? (await getRepoRoot(git)); + const specs = buildDiffPathspecs(repoRoot, pathFilter); + return { repoRoot, specs }; +} + +export async function getDiff( + git: SimpleGit, + from: string, + to: string, + commits: CommitInfo[], + filterByCommits: boolean, + pathFilter?: DiffPathFilter, + repoRootOverride?: string, +): Promise { + const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); + + if (!filterByCommits) { + return git.diff([`${from}..${to}`, "--", ...specs]); + } + + const patches = await Promise.all( + commits.map((c) => git.diff([`${c.hash}^!`, "--", ...specs])), + ); + + return patches.filter(Boolean).join("\n"); +} + +export async function getDiffSummary( + git: SimpleGit, + from: string, + to: string, + commits: CommitInfo[], + filterByCommits: boolean, + pathFilter?: DiffPathFilter, + repoRootOverride?: string, +): Promise { + const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); + + if (!filterByCommits) { + const [numOutput, nameOutput] = await Promise.all([ + git.diff(["--numstat", `${from}..${to}`, "--", ...specs]), + git.diff(["--name-status", `${from}..${to}`, "--", ...specs]), + ]); + return buildDiffSummaryFromGitOutputs(nameOutput, numOutput); + } + + const pairs = await Promise.all( + commits.map(async (c) => { + const range = `${c.hash}^!`; + const [numOutput, nameOutput] = await Promise.all([ + git.diff(["--numstat", range, "--", ...specs]), + git.diff(["--name-status", range, "--", ...specs]), + ]); + return { numOutput, nameOutput }; + }), + ); + 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( + git: SimpleGit, + from: string, + to: string, + commits: CommitInfo[], + filterByCommits: boolean, + pathFilter?: DiffPathFilter, + repoRootOverride?: string, +): Promise { + const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); + + if (!filterByCommits) { + const output = await git.diff([ + "--name-only", + `${from}..${to}`, + "--", + ...specs, + ]); + + return output + .split(/\r?\n/) + .map((f) => f.trim()) + .filter(Boolean); + } + + const fileSet = new Set(); + + await Promise.all( + commits.map(async (c) => { + const output = await git.show([ + "--name-only", + "--pretty=format:", + c.hash, + "--", + ...specs, + ]); + + output + .split(/\r?\n/) + .map((f) => f.trim()) + .filter(Boolean) + .forEach((f) => fileSet.add(f)); + }), + ); + + return Array.from(fileSet); +} diff --git a/src/index.ts b/src/index.ts index b3fa3fc..0ebd579 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ -import type { SimpleGit } from 'simple-git'; +import type { SimpleGit } from "simple-git"; -import { generateSummary, type SummarizeFlags } from './ai/aiSummary.js'; -import type { OpenAiLikeClient } from './ai/openAIConfig.js'; +import { generateSummary, type SummarizeFlags } from "./ai/aiSummary.js"; +import type { OpenAiLikeClient } from "./ai/openAIConfig.js"; import { createGitClient, filterCommitsByMessageRegexes, @@ -11,7 +11,7 @@ import { getDiffSummary, type CommitInfo, type DiffPathFilter, -} from './git/gitDiff.js'; +} from "./git/gitDiff.js"; export type GitDiffAiSummaryOptions = { /** Start ref (older side of the range). */ @@ -56,9 +56,15 @@ function hasNonEmptyTrimmed(arr?: string[]): boolean { function shouldFilterByCommits( allCommits: CommitInfo[], filtered: CommitInfo[], - opts: Pick + opts: Pick< + GitDiffAiSummaryOptions, + "commitMessageIncludeRegexes" | "commitMessageExcludeRegexes" + >, ): boolean { - if (hasNonEmptyTrimmed(opts.commitMessageIncludeRegexes) || hasNonEmptyTrimmed(opts.commitMessageExcludeRegexes)) { + if ( + hasNonEmptyTrimmed(opts.commitMessageIncludeRegexes) || + hasNonEmptyTrimmed(opts.commitMessageExcludeRegexes) + ) { return true; } return filtered.length !== allCommits.length; @@ -68,13 +74,16 @@ function shouldFilterByCommits( * Produce an AI-assisted Markdown summary of the git changes between `from` and `to`, * honoring path filters, commit message include/exclude regexes, optional team label, and optional system prompt. */ -export async function summarizeGitDiff(options: GitDiffAiSummaryOptions): Promise { +export async function summarizeGitDiff( + options: GitDiffAiSummaryOptions, +): Promise { const git = options.git ?? createGitClient(options.cwd); const from = options.from; - const to = options.to ?? 'HEAD'; + const to = options.to ?? "HEAD"; const pathFilter: DiffPathFilter | undefined = - hasNonEmptyTrimmed(options.includeFolders) || hasNonEmptyTrimmed(options.excludeFolders) + hasNonEmptyTrimmed(options.includeFolders) || + hasNonEmptyTrimmed(options.excludeFolders) ? { includeFolders: options.includeFolders, excludeFolders: options.excludeFolders, @@ -85,13 +94,24 @@ export async function summarizeGitDiff(options: GitDiffAiSummaryOptions): Promis const filteredCommits = filterCommitsByMessageRegexes( allCommits, options.commitMessageIncludeRegexes, - options.commitMessageExcludeRegexes + options.commitMessageExcludeRegexes, + ); + const filterByCommits = shouldFilterByCommits( + allCommits, + filteredCommits, + options, ); - const filterByCommits = shouldFilterByCommits(allCommits, filteredCommits, options); const [diffText, fileNames, diffSummary] = await Promise.all([ getDiff(git, from, to, filteredCommits, filterByCommits, pathFilter), - getChangedFiles(git, from, to, filteredCommits, filterByCommits, pathFilter), + getChangedFiles( + git, + from, + to, + filteredCommits, + filterByCommits, + pathFilter, + ), getDiffSummary(git, from, to, filteredCommits, filterByCommits, pathFilter), ]); @@ -112,11 +132,16 @@ export async function summarizeGitDiff(options: GitDiffAiSummaryOptions): Promis filteredCommits, summarizeFlags, options.openAiClientProvider, - diffSummary + diffSummary, ); } -export type { CommitInfo, DiffFileSummary, DiffPathFilter, DiffSummary } from './git/gitDiff.js'; +export type { + CommitInfo, + DiffFileSummary, + DiffPathFilter, + DiffSummary, +} from "./git/gitDiff.js"; export { buildDiffPathspecs, createGitClient, @@ -126,18 +151,21 @@ export { getDiff, getDiffSummary, getRepoRoot, -} from './git/gitDiff.js'; +} from "./git/gitDiff.js"; -export type { SummarizeFlags } from './ai/aiSummary.js'; +export type { SummarizeFlags } from "./ai/aiSummary.js"; export { DEFAULT_GIT_DIFF_SYSTEM_PROMPT, generateSummary, LLM_GATEWAY_REQUIRED_MESSAGE, resolveLlmMaxDiffChars, truncateUnifiedDiffForLlm, -} from './ai/aiSummary.js'; +} from "./ai/aiSummary.js"; -export type { OpenAiLikeClient, OpenAiLikeClientInit } from './ai/openAIConfig.js'; +export type { + OpenAiLikeClient, + OpenAiLikeClientInit, +} from "./ai/openAIConfig.js"; export { createOpenAiLikeClient, parseLlmDefaultHeadersFromEnv, @@ -145,4 +173,4 @@ export { resolveOpenAiLikeClientInit, shouldUseLlmGateway, splitPromotableAuthorizationFromHeaders, -} from './ai/openAIConfig.js'; +} from "./ai/openAIConfig.js"; diff --git a/test/aiSummary.spec.ts b/test/aiSummary.spec.ts index 3c948da..f9112f4 100644 --- a/test/aiSummary.spec.ts +++ b/test/aiSummary.spec.ts @@ -1,14 +1,16 @@ -import type { CommitInfo } from '../src/git/gitDiff'; -import * as openAIConfig from '../src/ai/openAIConfig'; +import type { CommitInfo } from "../src/git/gitDiff"; +import * as openAIConfig from "../src/ai/openAIConfig"; import { DEFAULT_GIT_DIFF_SYSTEM_PROMPT, generateSummary, LLM_GATEWAY_REQUIRED_MESSAGE, resolveLlmMaxDiffChars, truncateUnifiedDiffForLlm, -} from '../src/ai/aiSummary'; +} from "../src/ai/aiSummary"; -function mockLlmClient(content: string): () => Promise { +function mockLlmClient( + content: string, +): () => Promise { return async () => ({ chat: { @@ -18,10 +20,10 @@ function mockLlmClient(content: string): () => Promise { +describe("resolveLlmMaxDiffChars", () => { const original = process.env.LLM_MAX_DIFF_CHARS; afterEach(() => { @@ -29,101 +31,103 @@ describe('resolveLlmMaxDiffChars', () => { else process.env.LLM_MAX_DIFF_CHARS = original; }); - it('uses positive cli override', () => { + it("uses positive cli override", () => { expect(resolveLlmMaxDiffChars(50_000)).toBe(50_000); }); - it('truncates float cli override', () => { + it("truncates float cli override", () => { expect(resolveLlmMaxDiffChars(99.7)).toBe(99); }); - it('ignores non-positive cli override and reads env', () => { - process.env.LLM_MAX_DIFF_CHARS = '8000'; + it("ignores non-positive cli override and reads env", () => { + process.env.LLM_MAX_DIFF_CHARS = "8000"; expect(resolveLlmMaxDiffChars(0)).toBe(8000); expect(resolveLlmMaxDiffChars(-1)).toBe(8000); }); - it('falls back to default when env invalid', () => { - process.env.LLM_MAX_DIFF_CHARS = 'not-a-number'; + it("falls back to default when env invalid", () => { + process.env.LLM_MAX_DIFF_CHARS = "not-a-number"; expect(resolveLlmMaxDiffChars()).toBe(120_000); }); - it('ignores NaN cli override', () => { - process.env.LLM_MAX_DIFF_CHARS = '500'; + it("ignores NaN cli override", () => { + process.env.LLM_MAX_DIFF_CHARS = "500"; expect(resolveLlmMaxDiffChars(Number.NaN)).toBe(500); }); }); -describe('truncateUnifiedDiffForLlm', () => { - it('returns input unchanged when under limit', () => { - expect(truncateUnifiedDiffForLlm('abc', 10)).toBe('abc'); +describe("truncateUnifiedDiffForLlm", () => { + it("returns input unchanged when under limit", () => { + expect(truncateUnifiedDiffForLlm("abc", 10)).toBe("abc"); }); - it('truncates and appends marker when over limit', () => { - const long = 'x'.repeat(100); + it("truncates and appends marker when over limit", () => { + const long = "x".repeat(100); const out = truncateUnifiedDiffForLlm(long, 20); - expect(out.startsWith('x'.repeat(20))).toBe(true); - expect(out).toContain('TRUNCATED'); + expect(out.startsWith("x".repeat(20))).toBe(true); + expect(out).toContain("TRUNCATED"); expect(out.length).toBeGreaterThan(20); }); }); -describe('DEFAULT_GIT_DIFF_SYSTEM_PROMPT', () => { - it('is a non-empty markdown-oriented prompt', () => { +describe("DEFAULT_GIT_DIFF_SYSTEM_PROMPT", () => { + it("is a non-empty markdown-oriented prompt", () => { expect(DEFAULT_GIT_DIFF_SYSTEM_PROMPT.length).toBeGreaterThan(100); - expect(DEFAULT_GIT_DIFF_SYSTEM_PROMPT).toContain('git'); + expect(DEFAULT_GIT_DIFF_SYSTEM_PROMPT).toContain("git"); }); }); -describe('LLM_GATEWAY_REQUIRED_MESSAGE', () => { - it('is a stable exported string for callers', () => { - expect(LLM_GATEWAY_REQUIRED_MESSAGE).toContain('OPENAI_API_KEY'); - expect(LLM_GATEWAY_REQUIRED_MESSAGE).toContain('openAiClientProvider'); +describe("LLM_GATEWAY_REQUIRED_MESSAGE", () => { + it("is a stable exported string for callers", () => { + expect(LLM_GATEWAY_REQUIRED_MESSAGE).toContain("OPENAI_API_KEY"); + expect(LLM_GATEWAY_REQUIRED_MESSAGE).toContain("openAiClientProvider"); }); }); -describe('generateSummary', () => { - const commits: CommitInfo[] = [{ hash: 'deadbeef', message: 'feat: example' }]; - const flagsBase = { from: 'main', to: 'HEAD' }; +describe("generateSummary", () => { + const commits: CommitInfo[] = [ + { hash: "deadbeef", message: "feat: example" }, + ]; + const flagsBase = { from: "main", to: "HEAD" }; afterEach(() => { jest.restoreAllMocks(); }); - it('throws when LLM gateway is off and no client provider', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(false); + it("throws when LLM gateway is off and no client provider", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(false); - await expect(generateSummary('+added line', ['src/a.ts'], commits, flagsBase)).rejects.toThrow( - LLM_GATEWAY_REQUIRED_MESSAGE - ); + await expect( + generateSummary("+added line", ["src/a.ts"], commits, flagsBase), + ).rejects.toThrow(LLM_GATEWAY_REQUIRED_MESSAGE); }); - it('uses openAiClientProvider when gateway env is off', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(false); + it("uses openAiClientProvider when gateway env is off", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(false); const md = await generateSummary( - 'diff...', - ['f.ts'], + "diff...", + ["f.ts"], commits, { ...flagsBase, - team: 'QA', - systemPrompt: 'You are a test bot.', - model: 'gpt-test', + team: "QA", + systemPrompt: "You are a test bot.", + model: "gpt-test", maxDiffChars: 1000, }, - mockLlmClient(' **Summary** from inject ') + mockLlmClient(" **Summary** from inject "), ); - expect(md).toBe('**Summary** from inject'); + expect(md).toBe("**Summary** from inject"); }); - it('calls OpenAI-compatible client when gateway is on', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + it("calls OpenAI-compatible client when gateway is on", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: ' **Summary** from model ' } }], + choices: [{ message: { content: " **Summary** from model " } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate, @@ -131,114 +135,137 @@ describe('generateSummary', () => { }, } as Awaited>); - const md = await generateSummary('diff...', ['f.ts'], commits, { + const md = await generateSummary("diff...", ["f.ts"], commits, { ...flagsBase, - team: 'QA', - systemPrompt: 'You are a test bot.', - model: 'gpt-test', + team: "QA", + systemPrompt: "You are a test bot.", + model: "gpt-test", maxDiffChars: 1000, }); - expect(md).toBe('**Summary** from model'); + expect(md).toBe("**Summary** from model"); expect(completionCreate).toHaveBeenCalledWith( expect.objectContaining({ - model: 'gpt-test', + model: "gpt-test", messages: expect.arrayContaining([ - expect.objectContaining({ role: 'system', content: 'You are a test bot.' }), - expect.objectContaining({ role: 'user', content: expect.stringContaining('Team: QA') }), + expect.objectContaining({ + role: "system", + content: "You are a test bot.", + }), + expect.objectContaining({ + role: "user", + content: expect.stringContaining("Team: QA"), + }), ]), - }) + }), ); }); - it('defaults model to gpt-4o-mini when omitted', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + it("defaults model to gpt-4o-mini when omitted", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'ok' } }], + choices: [{ message: { content: "ok" } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary('d', [], [], flagsBase); - expect(completionCreate).toHaveBeenCalledWith(expect.objectContaining({ model: 'gpt-4o-mini' })); + await generateSummary("d", [], [], flagsBase); + expect(completionCreate).toHaveBeenCalledWith( + expect.objectContaining({ model: "gpt-4o-mini" }), + ); }); - it('includes exclude-only commit filter copy in user message', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + it("includes exclude-only commit filter copy in user message", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'x' } }], + choices: [{ message: { content: "x" } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary('d', [], [], { + await generateSummary("d", [], [], { ...flagsBase, - commitMessageExcludeRegexes: ['^WIP'], + commitMessageExcludeRegexes: ["^WIP"], }); - const userMsg = completionCreate.mock.calls[0]![0].messages.find((m: { role: string }) => m.role === 'user')?.content as string; - expect(userMsg).toContain('Commit message exclude regexes'); - expect(userMsg).not.toContain('include regexes'); + const userMsg = completionCreate.mock.calls[0]![0].messages.find( + (m: { role: string }) => m.role === "user", + )?.content as string; + expect(userMsg).toContain("Commit message exclude regexes"); + expect(userMsg).not.toContain("include regexes"); }); - it('omits team line when team is blank', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + it("omits team line when team is blank", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'x' } }], + choices: [{ message: { content: "x" } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary('d', [], [], { ...flagsBase, team: ' ' }); - const userMsg = completionCreate.mock.calls[0]![0].messages.find((m: { role: string }) => m.role === 'user')?.content as string; + await generateSummary("d", [], [], { ...flagsBase, team: " " }); + const userMsg = completionCreate.mock.calls[0]![0].messages.find( + (m: { role: string }) => m.role === "user", + )?.content as string; expect(userMsg).not.toMatch(/^Team:/m); }); - it('embeds diffSummary JSON when provided', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + it("embeds diffSummary JSON when provided", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'x' } }], + choices: [{ message: { content: "x" } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate } }, } as Awaited>); - const diffSummary = { files: [], totalFiles: 0, totalAdditions: 0, totalDeletions: 0 }; - await generateSummary('d', [], [], flagsBase, undefined, diffSummary); - const userMsg = completionCreate.mock.calls[0]![0].messages.find((m: { role: string }) => m.role === 'user')?.content as string; - expect(userMsg).toContain('Structured git context'); + const diffSummary = { + files: [], + totalFiles: 0, + totalAdditions: 0, + totalDeletions: 0, + }; + await generateSummary("d", [], [], flagsBase, undefined, diffSummary); + const userMsg = completionCreate.mock.calls[0]![0].messages.find( + (m: { role: string }) => m.role === "user", + )?.content as string; + expect(userMsg).toContain("Structured git context"); expect(userMsg).toContain('"totalFiles": 0'); }); - it('returns placeholder when model returns empty content', async () => { - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + it("returns placeholder when model returns empty content", async () => { + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { - create: jest.fn().mockResolvedValue({ choices: [{ message: { content: ' ' } }] }), + create: jest + .fn() + .mockResolvedValue({ choices: [{ message: { content: " " } }] }), }, }, } as Awaited>); - const md = await generateSummary('d', [], [], flagsBase); - expect(md).toBe('No summary generated by OpenAI.'); + const md = await generateSummary("d", [], [], flagsBase); + expect(md).toBe("No summary generated by OpenAI."); }); - it('uses 4000 max_tokens when OPENAI_MAX_TOKENS is invalid', async () => { + it("uses 4000 max_tokens when OPENAI_MAX_TOKENS is invalid", async () => { const prev = process.env.OPENAI_MAX_TOKENS; - process.env.OPENAI_MAX_TOKENS = 'not-int'; - jest.spyOn(openAIConfig, 'shouldUseLlmGateway').mockReturnValue(true); + process.env.OPENAI_MAX_TOKENS = "not-int"; + jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(true); const completionCreate = jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'ok' } }], + choices: [{ message: { content: "ok" } }], }); - jest.spyOn(openAIConfig, 'createOpenAiLikeClient').mockResolvedValue({ + jest.spyOn(openAIConfig, "createOpenAiLikeClient").mockResolvedValue({ chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary('d', [], [], flagsBase); - expect(completionCreate).toHaveBeenCalledWith(expect.objectContaining({ max_tokens: 4000 })); + await generateSummary("d", [], [], flagsBase); + expect(completionCreate).toHaveBeenCalledWith( + expect.objectContaining({ max_tokens: 4000 }), + ); if (prev === undefined) delete process.env.OPENAI_MAX_TOKENS; else process.env.OPENAI_MAX_TOKENS = prev; }); diff --git a/test/gitDiff.async.spec.ts b/test/gitDiff.async.spec.ts index 75496d7..5f34597 100644 --- a/test/gitDiff.async.spec.ts +++ b/test/gitDiff.async.spec.ts @@ -1,5 +1,5 @@ -import { join } from 'node:path'; -import type { SimpleGit } from 'simple-git'; +import { join } from "node:path"; +import type { SimpleGit } from "simple-git"; import { createGitClient, @@ -9,193 +9,228 @@ import { getDiffSummary, getRepoRoot, type CommitInfo, -} from '../src/git/gitDiff'; +} from "../src/git/gitDiff"; -describe('createGitClient', () => { - it('returns a simple-git instance for the given cwd', () => { - const git = createGitClient(join(__dirname, '..')); +describe("createGitClient", () => { + it("returns a simple-git instance for the given cwd", () => { + const git = createGitClient(join(__dirname, "..")); expect(git).toBeDefined(); - expect(typeof git.log).toBe('function'); + expect(typeof git.log).toBe("function"); }); - it('defaults cwd to process.cwd when omitted', () => { + it("defaults cwd to process.cwd when omitted", () => { const git = createGitClient(); expect(git).toBeDefined(); }); }); -describe('getRepoRoot', () => { - it('trims revparse output', async () => { +describe("getRepoRoot", () => { + it("trims revparse output", async () => { const git = { - revparse: jest.fn().mockResolvedValue(' /repo/root \n'), + revparse: jest.fn().mockResolvedValue(" /repo/root \n"), } as unknown as SimpleGit; - await expect(getRepoRoot(git)).resolves.toBe('/repo/root'); + await expect(getRepoRoot(git)).resolves.toBe("/repo/root"); }); }); -describe('getCommits', () => { - it('returns log.all as CommitInfo[]', async () => { +describe("getCommits", () => { + it("returns log.all as CommitInfo[]", async () => { const git = { log: jest.fn().mockResolvedValue({ - all: [{ hash: 'aaa', message: 'msg' }], + all: [{ hash: "aaa", message: "msg" }], }), } as unknown as SimpleGit; - await expect(getCommits(git, 'from', 'to')).resolves.toEqual([{ hash: 'aaa', message: 'msg' }]); - expect(git.log).toHaveBeenCalledWith({ from: 'from', to: 'to' }); + await expect(getCommits(git, "from", "to")).resolves.toEqual([ + { hash: "aaa", message: "msg" }, + ]); + expect(git.log).toHaveBeenCalledWith({ from: "from", to: "to" }); }); }); function makeGitWithDiff(): { git: SimpleGit; diff: jest.Mock } { const diff = jest.fn(); const git = { - revparse: jest.fn().mockResolvedValue(`${join(__dirname, 'fixture-repo')}\n`), + revparse: jest + .fn() + .mockResolvedValue(`${join(__dirname, "fixture-repo")}\n`), diff, show: jest.fn(), } as unknown as SimpleGit; return { git, diff }; } -describe('getDiff', () => { - it('uses range diff and repoRootOverride skips revparse', async () => { +describe("getDiff", () => { + it("uses range diff and repoRootOverride skips revparse", async () => { const { git, diff } = makeGitWithDiff(); - diff.mockResolvedValue('range-diff'); - const commits: CommitInfo[] = [{ hash: 'x', message: 'm' }]; + diff.mockResolvedValue("range-diff"); + const commits: CommitInfo[] = [{ hash: "x", message: "m" }]; - const out = await getDiff(git, 'a', 'b', commits, false, { excludeFolders: ['out'] }, join(__dirname, 'fixture-repo')); + const out = await getDiff( + git, + "a", + "b", + commits, + false, + { excludeFolders: ["out"] }, + join(__dirname, "fixture-repo"), + ); - expect(out).toBe('range-diff'); - expect(diff).toHaveBeenCalledWith(['a..b', '--', '.', ':(exclude)out']); - expect((git as unknown as { revparse: jest.Mock }).revparse).not.toHaveBeenCalled(); + expect(out).toBe("range-diff"); + expect(diff).toHaveBeenCalledWith(["a..b", "--", ".", ":(exclude)out"]); + expect( + (git as unknown as { revparse: jest.Mock }).revparse, + ).not.toHaveBeenCalled(); }); - it('joins per-commit patches and drops empty', async () => { + it("joins per-commit patches and drops empty", async () => { const { git, diff } = makeGitWithDiff(); - diff.mockResolvedValueOnce('').mockResolvedValueOnce('patch-b'); + diff.mockResolvedValueOnce("").mockResolvedValueOnce("patch-b"); const commits: CommitInfo[] = [ - { hash: 'aaa111', message: 'a' }, - { hash: 'bbb222', message: 'b' }, + { hash: "aaa111", message: "a" }, + { hash: "bbb222", message: "b" }, ]; - const out = await getDiff(git, 'f', 't', commits, true, undefined, join(__dirname, 'fixture-repo')); + const out = await getDiff( + git, + "f", + "t", + commits, + true, + undefined, + join(__dirname, "fixture-repo"), + ); - expect(diff).toHaveBeenCalledWith(['aaa111^!', '--', '.']); - expect(diff).toHaveBeenCalledWith(['bbb222^!', '--', '.']); - expect(out).toBe('patch-b'); + expect(diff).toHaveBeenCalledWith(["aaa111^!", "--", "."]); + expect(diff).toHaveBeenCalledWith(["bbb222^!", "--", "."]); + expect(out).toBe("patch-b"); }); }); -describe('getDiffSummary', () => { - it('aggregates range numstat and name-status', async () => { +describe("getDiffSummary", () => { + it("aggregates range numstat and name-status", async () => { const { git, diff } = makeGitWithDiff(); diff.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("--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')) { + 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'); + "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 ''; + return ""; }); const summary = await getDiffSummary( git, - 'x', - 'y', - [{ hash: 'h', message: 'm' }], + "x", + "y", + [{ hash: "h", message: "m" }], false, undefined, - join(__dirname, 'fixture-repo') + join(__dirname, "fixture-repo"), ); 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'); + 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('aggregates per-commit summaries', async () => { + it("aggregates per-commit summaries", async () => { const { git, diff } = makeGitWithDiff(); let call = 0; diff.mockImplementation(async (args: string[]) => { - const range = args.find((a) => a.endsWith('^!')); + const range = args.find((a) => a.endsWith("^!")); call += 1; - if (args.includes('--numstat')) { - return range?.startsWith('111') ? '1\t1\tf.ts' : ''; + if (args.includes("--numstat")) { + return range?.startsWith("111") ? "1\t1\tf.ts" : ""; } - if (args.includes('--name-status')) { - return range?.startsWith('111') ? 'M\tf.ts' : ''; + if (args.includes("--name-status")) { + return range?.startsWith("111") ? "M\tf.ts" : ""; } - return ''; + return ""; }); const summary = await getDiffSummary( git, - 'a', - 'b', + "a", + "b", [ - { hash: '111aaa', message: 'm1' }, - { hash: '222bbb', message: 'm2' }, + { hash: "111aaa", message: "m1" }, + { hash: "222bbb", message: "m2" }, ], true, undefined, - join(__dirname, 'fixture-repo') + join(__dirname, "fixture-repo"), ); - expect(summary.files.some((f) => f.path === 'f.ts')).toBe(true); + expect(summary.files.some((f) => f.path === "f.ts")).toBe(true); }); }); -describe('getChangedFiles', () => { - it('splits range output on CRLF', async () => { +describe("getChangedFiles", () => { + it("splits range output on CRLF", async () => { const { git, diff } = makeGitWithDiff(); - diff.mockResolvedValue('a.ts\r\nb.ts\r\n'); + diff.mockResolvedValue("a.ts\r\nb.ts\r\n"); const files = await getChangedFiles( git, - 'a', - 'b', - [{ hash: 'h', message: 'm' }], + "a", + "b", + [{ hash: "h", message: "m" }], false, undefined, - join(__dirname, 'fixture-repo') + join(__dirname, "fixture-repo"), ); - expect(files).toEqual(['a.ts', 'b.ts']); + expect(files).toEqual(["a.ts", "b.ts"]); }); - it('dedupes files from per-commit show output', async () => { + it("dedupes files from per-commit show output", async () => { const { git } = makeGitWithDiff(); - const show = jest.fn().mockResolvedValueOnce('dup.ts\n').mockResolvedValueOnce('dup.ts\nother.ts\n'); + const show = jest + .fn() + .mockResolvedValueOnce("dup.ts\n") + .mockResolvedValueOnce("dup.ts\nother.ts\n"); (git as unknown as { show: typeof show }).show = show; const files = await getChangedFiles( git as unknown as SimpleGit, - 'a', - 'b', + "a", + "b", [ - { hash: 'c1', message: '1' }, - { hash: 'c2', message: '2' }, + { hash: "c1", message: "1" }, + { hash: "c2", message: "2" }, ], true, undefined, - join(__dirname, 'fixture-repo') + join(__dirname, "fixture-repo"), ); - expect(files.sort()).toEqual(['dup.ts', 'other.ts']); + expect(files.sort()).toEqual(["dup.ts", "other.ts"]); }); }); diff --git a/test/gitDiff.spec.ts b/test/gitDiff.spec.ts index 2e3db7f..ac7c9df 100644 --- a/test/gitDiff.spec.ts +++ b/test/gitDiff.spec.ts @@ -1,153 +1,190 @@ -import { join } from 'node:path'; +import { join } from "node:path"; import { buildDiffPathspecs, filterCommitsByMessageRegexes, parseDiffSummary, type CommitInfo, -} from '../src/git/gitDiff'; +} from "../src/git/gitDiff"; -describe('buildDiffPathspecs', () => { - const repoRoot = join(__dirname, 'fixture-repo-root'); +describe("buildDiffPathspecs", () => { + const repoRoot = join(__dirname, "fixture-repo-root"); - it('returns whole repo when no filter', () => { - expect(buildDiffPathspecs(repoRoot)).toEqual(['.']); + 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("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', () => { + 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']); + 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("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 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 on invalid include regex path via parent segments", () => { + expect(() => + buildDiffPathspecs(repoRoot, { includeFolders: ["foo/../../etc"] }), + ).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("treats root-like include as empty and still applies excludes", () => { + expect( + buildDiffPathspecs(repoRoot, { + includeFolders: ["/"], + excludeFolders: ["tmp"], + }), + ).toEqual([".", ":(exclude)tmp"]); }); }); -describe('filterCommitsByMessageRegexes', () => { +describe("filterCommitsByMessageRegexes", () => { const commits: CommitInfo[] = [ - { hash: 'a1', message: 'feat: add login' }, - { hash: 'b2', message: 'chore: bump deps' }, - { hash: 'c3', message: 'fix: handle edge case' }, + { hash: "a1", message: "feat: add login" }, + { hash: "b2", message: "chore: bump deps" }, + { hash: "c3", message: "fix: handle edge case" }, ]; - it('returns all commits when no patterns', () => { + it("returns all commits when no patterns", () => { expect(filterCommitsByMessageRegexes(commits)).toEqual(commits); expect(filterCommitsByMessageRegexes(commits, [], [])).toEqual(commits); }); - it('applies exclude before include', () => { - const out = filterCommitsByMessageRegexes(commits, ['feat:', 'fix:'], ['chore:']); - expect(out.map((c) => c.hash)).toEqual(['a1', 'c3']); + it("applies exclude before include", () => { + const out = filterCommitsByMessageRegexes( + commits, + ["feat:", "fix:"], + ["chore:"], + ); + expect(out.map((c) => c.hash)).toEqual(["a1", "c3"]); }); - it('requires OR match across include patterns', () => { - const out = filterCommitsByMessageRegexes(commits, ['^feat:', '^fix:']); + it("requires OR match across include patterns", () => { + const out = filterCommitsByMessageRegexes(commits, ["^feat:", "^fix:"]); expect(out).toHaveLength(2); - expect(out[0]?.hash).toBe('a1'); - expect(out[1]?.hash).toBe('c3'); + expect(out[0]?.hash).toBe("a1"); + expect(out[1]?.hash).toBe("c3"); }); - it('drops commits matching exclude only', () => { - const out = filterCommitsByMessageRegexes(commits, undefined, ['chore:']); - expect(out.map((c) => c.hash)).toEqual(['a1', 'c3']); + it("drops commits matching exclude only", () => { + const out = filterCommitsByMessageRegexes(commits, undefined, ["chore:"]); + expect(out.map((c) => c.hash)).toEqual(["a1", "c3"]); }); - it('is case-insensitive for regex', () => { - const out = filterCommitsByMessageRegexes([{ hash: 'x', message: 'FEAT: caps' }], ['feat:']); + it("is case-insensitive for regex", () => { + const out = filterCommitsByMessageRegexes( + [{ hash: "x", message: "FEAT: caps" }], + ["feat:"], + ); expect(out).toHaveLength(1); }); - it('throws on invalid include pattern', () => { - expect(() => filterCommitsByMessageRegexes(commits, ['('], [])).toThrow(/include pattern\[0\]/); + it("throws on invalid include pattern", () => { + expect(() => filterCommitsByMessageRegexes(commits, ["("], [])).toThrow( + /include pattern\[0\]/, + ); }); - it('throws on invalid exclude pattern', () => { - expect(() => filterCommitsByMessageRegexes(commits, [], ['('])).toThrow(/exclude pattern\[0\]/); + it("throws on invalid exclude pattern", () => { + expect(() => filterCommitsByMessageRegexes(commits, [], ["("])).toThrow( + /exclude pattern\[0\]/, + ); }); }); -describe('parseDiffSummary', () => { - it('parses modified file line', () => { - const summary = parseDiffSummary('M\t10\t2\tsrc/foo.ts'); +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', + 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')); + 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'); + 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', + 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'); + 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'); + 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 }); + 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("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'); + 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("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"); }); }); diff --git a/test/index.spec.ts b/test/index.spec.ts index 7e21b9b..31e589b 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -1,8 +1,8 @@ -import * as gitDiff from '../src/git/gitDiff'; -import type { OpenAiLikeClient } from '../src/ai/openAIConfig'; -import { summarizeGitDiff } from '../src/index'; +import * as gitDiff from "../src/git/gitDiff"; +import type { OpenAiLikeClient } from "../src/ai/openAIConfig"; +import { summarizeGitDiff } from "../src/index"; -describe('summarizeGitDiff integration', () => { +describe("summarizeGitDiff integration", () => { const originalEnv = process.env; afterEach(() => { @@ -10,79 +10,83 @@ describe('summarizeGitDiff integration', () => { process.env = originalEnv; }); - it('uses createGitClient when git is omitted', async () => { + it("uses createGitClient when git is omitted", async () => { const mockGit = { - log: jest.fn().mockResolvedValue({ all: [{ hash: 'h1', message: 'm' }] }), - revparse: jest.fn().mockResolvedValue('C:\\repo\n'), - diff: jest.fn().mockResolvedValue(''), - show: jest.fn().mockResolvedValue(''), + log: jest.fn().mockResolvedValue({ all: [{ hash: "h1", message: "m" }] }), + revparse: jest.fn().mockResolvedValue("C:\\repo\n"), + diff: jest.fn().mockResolvedValue(""), + show: jest.fn().mockResolvedValue(""), }; - const createSpy = jest.spyOn(gitDiff, 'createGitClient').mockReturnValue(mockGit as never); + const createSpy = jest + .spyOn(gitDiff, "createGitClient") + .mockReturnValue(mockGit as never); const openAiClientProvider = async (): Promise => ({ chat: { completions: { create: jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'summary' } }], + choices: [{ message: { content: "summary" } }], }), }, }, }) as OpenAiLikeClient; await summarizeGitDiff({ - from: 'a', - to: 'b', - cwd: 'C:\\some\\cwd', + from: "a", + to: "b", + cwd: "C:\\some\\cwd", openAiClientProvider, }); - expect(createSpy).toHaveBeenCalledWith('C:\\some\\cwd'); + expect(createSpy).toHaveBeenCalledWith("C:\\some\\cwd"); expect(mockGit.log).toHaveBeenCalled(); createSpy.mockRestore(); }); - it('uses per-commit diff when filtered commits differ without regex options', async () => { - jest.spyOn(gitDiff, 'getCommits').mockResolvedValue([ - { hash: '1', message: 'a' }, - { hash: '2', message: 'b' }, + it("uses per-commit diff when filtered commits differ without regex options", async () => { + jest.spyOn(gitDiff, "getCommits").mockResolvedValue([ + { hash: "1", message: "a" }, + { hash: "2", message: "b" }, ]); - jest.spyOn(gitDiff, 'filterCommitsByMessageRegexes').mockReturnValue([{ hash: '1', message: 'a' }]); + jest + .spyOn(gitDiff, "filterCommitsByMessageRegexes") + .mockReturnValue([{ hash: "1", message: "a" }]); const diff = jest.fn().mockImplementation(async (args: string[]) => { - 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 ''; + 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 ""; }); const mockGit = { log: jest.fn(), - revparse: jest.fn().mockResolvedValue('C:\\repo\n'), + revparse: jest.fn().mockResolvedValue("C:\\repo\n"), diff, - show: jest.fn().mockResolvedValue('f.ts\n'), + show: jest.fn().mockResolvedValue("f.ts\n"), } as never; - jest.spyOn(gitDiff, 'createGitClient').mockReturnValue(mockGit); + jest.spyOn(gitDiff, "createGitClient").mockReturnValue(mockGit); const openAiClientProvider = async (): Promise => ({ chat: { completions: { create: jest.fn().mockResolvedValue({ - choices: [{ message: { content: 'ok' } }], + choices: [{ message: { content: "ok" } }], }), }, }, }) as OpenAiLikeClient; await summarizeGitDiff({ - from: 'x', - to: 'y', - cwd: '.', + from: "x", + to: "y", + cwd: ".", openAiClientProvider, }); - expect(diff).toHaveBeenCalledWith(expect.arrayContaining(['1^!'])); + expect(diff).toHaveBeenCalledWith(expect.arrayContaining(["1^!"])); }); }); diff --git a/test/openAIConfig.spec.ts b/test/openAIConfig.spec.ts index def8496..195929c 100644 --- a/test/openAIConfig.spec.ts +++ b/test/openAIConfig.spec.ts @@ -4,7 +4,7 @@ import { resolveOpenAiLikeClientInit, shouldUseLlmGateway, splitPromotableAuthorizationFromHeaders, -} from '../src/ai/openAIConfig'; +} from "../src/ai/openAIConfig"; const originalEnv = process.env; @@ -12,7 +12,7 @@ function resetEnv(): void { process.env = { ...originalEnv }; } -describe('openAIConfig', () => { +describe("openAIConfig", () => { beforeEach(() => { resetEnv(); delete process.env.LLM_BASE_URL; @@ -27,144 +27,167 @@ describe('openAIConfig', () => { process.env = originalEnv; }); - describe('resolveLlmBaseUrl', () => { - it('prefers LLM_BASE_URL over OPENAI_BASE_URL', () => { - process.env.OPENAI_BASE_URL = 'https://openai.example'; - process.env.LLM_BASE_URL = ' https://llm.example '; - expect(resolveLlmBaseUrl()).toBe('https://llm.example'); + describe("resolveLlmBaseUrl", () => { + it("prefers LLM_BASE_URL over OPENAI_BASE_URL", () => { + process.env.OPENAI_BASE_URL = "https://openai.example"; + process.env.LLM_BASE_URL = " https://llm.example "; + expect(resolveLlmBaseUrl()).toBe("https://llm.example"); }); - it('falls back to OPENAI_BASE_URL', () => { - process.env.OPENAI_BASE_URL = 'https://only-openai'; - expect(resolveLlmBaseUrl()).toBe('https://only-openai'); + it("falls back to OPENAI_BASE_URL", () => { + process.env.OPENAI_BASE_URL = "https://only-openai"; + expect(resolveLlmBaseUrl()).toBe("https://only-openai"); }); - it('returns undefined when unset', () => { + it("returns undefined when unset", () => { expect(resolveLlmBaseUrl()).toBeUndefined(); }); }); - describe('parseLlmDefaultHeadersFromEnv', () => { - it('returns undefined when no headers set', () => { + describe("parseLlmDefaultHeadersFromEnv", () => { + it("returns undefined when no headers set", () => { expect(parseLlmDefaultHeadersFromEnv()).toBeUndefined(); }); - it('merges OPENAI_DEFAULT_HEADERS with LLM_DEFAULT_HEADERS override', () => { - process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ 'X-A': '1', 'X-B': 'old' }); - process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ 'X-B': 'new', 'X-C': '3' }); + it("merges OPENAI_DEFAULT_HEADERS with LLM_DEFAULT_HEADERS override", () => { + process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ + "X-A": "1", + "X-B": "old", + }); + process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ + "X-B": "new", + "X-C": "3", + }); expect(parseLlmDefaultHeadersFromEnv()).toEqual({ - 'X-A': '1', - 'X-B': 'new', - 'X-C': '3', + "X-A": "1", + "X-B": "new", + "X-C": "3", }); }); - it('returns undefined for invalid JSON', () => { - process.env.OPENAI_DEFAULT_HEADERS = '{not json'; + it("returns undefined for invalid JSON", () => { + process.env.OPENAI_DEFAULT_HEADERS = "{not json"; expect(parseLlmDefaultHeadersFromEnv()).toBeUndefined(); }); - it('ignores non-object JSON (arrays)', () => { - process.env.OPENAI_DEFAULT_HEADERS = '[1,2,3]'; + it("ignores non-object JSON (arrays)", () => { + process.env.OPENAI_DEFAULT_HEADERS = "[1,2,3]"; expect(parseLlmDefaultHeadersFromEnv()).toBeUndefined(); }); - it('ignores non-string header values', () => { - process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ 'X-Num': 42, 'X-Ok': 'yes' }); - expect(parseLlmDefaultHeadersFromEnv()).toEqual({ 'X-Ok': 'yes' }); + it("ignores non-string header values", () => { + process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ + "X-Num": 42, + "X-Ok": "yes", + }); + expect(parseLlmDefaultHeadersFromEnv()).toEqual({ "X-Ok": "yes" }); }); }); - describe('splitPromotableAuthorizationFromHeaders', () => { - it('returns headers unchanged when no Authorization', () => { - const h = { 'X-Custom': 'v' }; - expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ defaultHeaders: h }); + describe("splitPromotableAuthorizationFromHeaders", () => { + it("returns headers unchanged when no Authorization", () => { + const h = { "X-Custom": "v" }; + expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ + defaultHeaders: h, + }); }); - it('promotes raw sk- token from Authorization', () => { - const sk = 'sk-test123456789012345678901234567890'; - const result = splitPromotableAuthorizationFromHeaders({ Authorization: sk }); + it("promotes raw sk- token from Authorization", () => { + const sk = "sk-test123456789012345678901234567890"; + const result = splitPromotableAuthorizationFromHeaders({ + Authorization: sk, + }); expect(result.apiKeyFromAuthHeader).toBe(sk); expect(result.defaultHeaders).toEqual({}); }); - it('promotes Bearer sk- token and strips header', () => { - const sk = 'sk-abc'; - const result = splitPromotableAuthorizationFromHeaders({ Authorization: `Bearer ${sk}` }); + it("promotes Bearer sk- token and strips header", () => { + const sk = "sk-abc"; + const result = splitPromotableAuthorizationFromHeaders({ + Authorization: `Bearer ${sk}`, + }); expect(result.apiKeyFromAuthHeader).toBe(sk); expect(result.defaultHeaders).toEqual({}); }); - it('does not promote non-key Authorization values', () => { - const h = { Authorization: 'Basic dXNlcjpwYXNz' }; - expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ defaultHeaders: h }); + it("does not promote non-key Authorization values", () => { + const h = { Authorization: "Basic dXNlcjpwYXNz" }; + expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ + defaultHeaders: h, + }); }); - it('returns headers unchanged when Authorization header is empty', () => { - const h = { Authorization: '' }; - expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ defaultHeaders: h }); + it("returns headers unchanged when Authorization header is empty", () => { + const h = { Authorization: "" }; + expect(splitPromotableAuthorizationFromHeaders(h)).toEqual({ + defaultHeaders: h, + }); }); }); - describe('shouldUseLlmGateway', () => { - it('is true when OPENAI_API_KEY is set', () => { - process.env.OPENAI_API_KEY = 'sk-key'; + describe("shouldUseLlmGateway", () => { + it("is true when OPENAI_API_KEY is set", () => { + process.env.OPENAI_API_KEY = "sk-key"; expect(shouldUseLlmGateway()).toBe(true); }); - it('is true when LLM_API_KEY is set', () => { - process.env.LLM_API_KEY = 'sk-llm'; + it("is true when LLM_API_KEY is set", () => { + process.env.LLM_API_KEY = "sk-llm"; expect(shouldUseLlmGateway()).toBe(true); }); - it('is true when base URL is set without key', () => { - process.env.OPENAI_BASE_URL = 'https://proxy.local/v1'; + it("is true when base URL is set without key", () => { + process.env.OPENAI_BASE_URL = "https://proxy.local/v1"; expect(shouldUseLlmGateway()).toBe(true); }); - it('is true when default headers JSON is non-empty', () => { - process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ Authorization: 'Bearer sk-from-header' }); + it("is true when default headers JSON is non-empty", () => { + process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ + Authorization: "Bearer sk-from-header", + }); expect(shouldUseLlmGateway()).toBe(true); }); - it('is false when nothing configured', () => { + it("is false when nothing configured", () => { expect(shouldUseLlmGateway()).toBe(false); }); }); - describe('resolveOpenAiLikeClientInit', () => { - it('uses env API key and optional base URL', () => { - process.env.OPENAI_API_KEY = 'sk-real'; - process.env.LLM_BASE_URL = 'https://custom'; + describe("resolveOpenAiLikeClientInit", () => { + it("uses env API key and optional base URL", () => { + process.env.OPENAI_API_KEY = "sk-real"; + process.env.LLM_BASE_URL = "https://custom"; expect(resolveOpenAiLikeClientInit()).toEqual({ - apiKey: 'sk-real', - baseURL: 'https://custom', + apiKey: "sk-real", + baseURL: "https://custom", }); }); - it('uses unused placeholder when no key and no promotable auth in headers', () => { - expect(resolveOpenAiLikeClientInit()).toEqual({ apiKey: 'unused' }); + it("uses unused placeholder when no key and no promotable auth in headers", () => { + expect(resolveOpenAiLikeClientInit()).toEqual({ apiKey: "unused" }); }); - it('prefers LLM_API_KEY over OPENAI_API_KEY', () => { - process.env.OPENAI_API_KEY = 'sk-openai'; - process.env.LLM_API_KEY = 'sk-llm-wins'; - expect(resolveOpenAiLikeClientInit().apiKey).toBe('sk-llm-wins'); + it("prefers LLM_API_KEY over OPENAI_API_KEY", () => { + process.env.OPENAI_API_KEY = "sk-openai"; + process.env.LLM_API_KEY = "sk-llm-wins"; + expect(resolveOpenAiLikeClientInit().apiKey).toBe("sk-llm-wins"); }); - it('promotes sk- token from default headers when env API key is empty', () => { - process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ Authorization: 'sk-from-headers-only' }); + it("promotes sk- token from default headers when env API key is empty", () => { + process.env.LLM_DEFAULT_HEADERS = JSON.stringify({ + Authorization: "sk-from-headers-only", + }); expect(resolveOpenAiLikeClientInit()).toEqual({ - apiKey: 'sk-from-headers-only', + apiKey: "sk-from-headers-only", }); }); - it('uses defaultHeaders with env API key when both are set', () => { - process.env.OPENAI_API_KEY = 'sk-main'; - process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ 'X-Custom': '1' }); + it("uses defaultHeaders with env API key when both are set", () => { + process.env.OPENAI_API_KEY = "sk-main"; + process.env.OPENAI_DEFAULT_HEADERS = JSON.stringify({ "X-Custom": "1" }); expect(resolveOpenAiLikeClientInit()).toEqual({ - apiKey: 'sk-main', - defaultHeaders: { 'X-Custom': '1' }, + apiKey: "sk-main", + defaultHeaders: { "X-Custom": "1" }, }); }); }); diff --git a/test/openAiSdk.spec.ts b/test/openAiSdk.spec.ts index b05bc7b..4feb140 100644 --- a/test/openAiSdk.spec.ts +++ b/test/openAiSdk.spec.ts @@ -1,18 +1,21 @@ -jest.mock('openai', () => ({ +jest.mock("openai", () => ({ __esModule: true, default: jest.fn().mockImplementation(() => ({ constructed: true })), })); -import OpenAI from 'openai'; +import OpenAI from "openai"; -import { createOpenAiLikeClient, resolveOpenAiLikeClientInit } from '../src/ai/openAIConfig'; +import { + createOpenAiLikeClient, + resolveOpenAiLikeClientInit, +} from "../src/ai/openAIConfig"; const originalEnv = process.env; -describe('createOpenAiLikeClient', () => { +describe("createOpenAiLikeClient", () => { beforeEach(() => { process.env = { ...originalEnv }; - process.env.OPENAI_API_KEY = 'sk-test-openai-sdk-spec'; + process.env.OPENAI_API_KEY = "sk-test-openai-sdk-spec"; delete process.env.LLM_BASE_URL; }); @@ -20,7 +23,7 @@ describe('createOpenAiLikeClient', () => { process.env = originalEnv; }); - it('loads the OpenAI SDK and constructs a client with resolved init', async () => { + it("loads the OpenAI SDK and constructs a client with resolved init", async () => { const init = resolveOpenAiLikeClientInit(); const client = await createOpenAiLikeClient(); expect(client).toEqual({ constructed: true }); diff --git a/test/summarizeGitDiff.spec.ts b/test/summarizeGitDiff.spec.ts index 87b92ee..4453886 100644 --- a/test/summarizeGitDiff.spec.ts +++ b/test/summarizeGitDiff.spec.ts @@ -1,31 +1,33 @@ -import { join } from 'node:path'; -import type { SimpleGit } from 'simple-git'; +import { join } from "node:path"; +import type { SimpleGit } from "simple-git"; -import type { OpenAiLikeClient } from '../src/ai/openAIConfig'; -import { summarizeGitDiff } from '../src/index'; +import type { OpenAiLikeClient } from "../src/ai/openAIConfig"; +import { summarizeGitDiff } from "../src/index"; function createMockGit(repoRoot: string): SimpleGit { const diff = jest.fn().mockImplementation(async (args: string[]) => { - 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'; + 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 { log: jest.fn().mockResolvedValue({ all: [ - { hash: 'aaa111', message: 'feat: one' }, - { hash: 'bbb222', message: 'chore: noise' }, + { hash: "aaa111", message: "feat: one" }, + { hash: "bbb222", message: "chore: noise" }, ], }), revparse: jest.fn().mockResolvedValue(`${repoRoot}\n`), diff, - show: jest.fn().mockResolvedValue(''), + show: jest.fn().mockResolvedValue(""), } as unknown as SimpleGit; } -function mockOpenAiClient(summaryMarkdown: string): () => Promise { +function mockOpenAiClient( + summaryMarkdown: string, +): () => Promise { return async () => ({ chat: { @@ -38,40 +40,46 @@ function mockOpenAiClient(summaryMarkdown: string): () => Promise { - const repoRoot = join(__dirname, 'fixture-repo-root'); +describe("summarizeGitDiff", () => { + const repoRoot = join(__dirname, "fixture-repo-root"); - it('aggregates git calls and returns LLM summary via openAiClientProvider', async () => { + it("aggregates git calls and returns LLM summary via openAiClientProvider", async () => { const git = createMockGit(repoRoot); const md = await summarizeGitDiff({ - from: 'main', - to: 'topic', + from: "main", + to: "topic", git, - teamName: 'Infra', - excludeFolders: ['node_modules'], - commitMessageExcludeRegexes: ['^chore:'], - openAiClientProvider: mockOpenAiClient('# Infra Summary\nBody from model'), + teamName: "Infra", + excludeFolders: ["node_modules"], + commitMessageExcludeRegexes: ["^chore:"], + openAiClientProvider: mockOpenAiClient( + "# Infra Summary\nBody from model", + ), }); - expect(git.log).toHaveBeenCalledWith({ from: 'main', to: 'topic' }); - expect(md).toBe('# Infra Summary\nBody from model'); + expect(git.log).toHaveBeenCalledWith({ from: "main", to: "topic" }); + expect(md).toBe("# Infra Summary\nBody from model"); expect(git.diff).toHaveBeenCalled(); }); - it('uses per-commit diff shape when include regexes are set even if all match', async () => { + 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', + from: "a", + to: "b", git, - commitMessageIncludeRegexes: ['.'], - openAiClientProvider: mockOpenAiClient('ok'), + commitMessageIncludeRegexes: ["."], + openAiClientProvider: mockOpenAiClient("ok"), }); - const diffCalls = (git.diff as jest.Mock).mock.calls.map((c) => c[0] as string[]); - const hasPerCommitPatch = diffCalls.some((args) => args.some((x) => /^\w+\^!$/.test(x))); + const diffCalls = (git.diff as jest.Mock).mock.calls.map( + (c) => c[0] as string[], + ); + const hasPerCommitPatch = diffCalls.some((args) => + args.some((x) => /^\w+\^!$/.test(x)), + ); expect(hasPerCommitPatch).toBe(true); }); }); From d5141d49adec52c02f16f3627a6ec2f539a24f35 Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Tue, 14 Apr 2026 10:53:56 -0400 Subject: [PATCH 2/3] chore: format new files --- src/git/diffGitStatus.ts | 5 ++++- src/git/diffNameStatusParse.ts | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/git/diffGitStatus.ts b/src/git/diffGitStatus.ts index 5f58461..e56a44f 100644 --- a/src/git/diffGitStatus.ts +++ b/src/git/diffGitStatus.ts @@ -14,7 +14,10 @@ export function mapGitStatus(statusCode: string): DiffStatus { return GIT_STATUS_BY_FIRST_CHAR[statusCode.charAt(0)] ?? "unknown"; } -export function mergeStatus(existing: DiffStatus, next: DiffStatus): DiffStatus { +export function mergeStatus( + existing: DiffStatus, + next: DiffStatus, +): DiffStatus { if (existing === next) return existing; const precedence: DiffStatus[] = [ "deleted", diff --git a/src/git/diffNameStatusParse.ts b/src/git/diffNameStatusParse.ts index b489852..fb4e8ae 100644 --- a/src/git/diffNameStatusParse.ts +++ b/src/git/diffNameStatusParse.ts @@ -34,7 +34,9 @@ function parseNameStatusLine(line: string): ParsedNameEntry | null { return entry; } -export function parseNameStatusLines(nameStatusOutput: string): ParsedNameEntry[] { +export function parseNameStatusLines( + nameStatusOutput: string, +): ParsedNameEntry[] { const entries: ParsedNameEntry[] = []; for (const rawLine of nameStatusOutput.split(/\r?\n/)) { const line = rawLine.trim(); From 607aaee64555c5a9c0df781375e15205b37cd077 Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:01:24 -0400 Subject: [PATCH 3/3] refactor: reduce parameters --- src/git/diffTypes.ts | 11 +++++ src/git/gitDiff.ts | 1 + src/git/gitDiffOps.ts | 34 ++++++-------- src/index.ts | 22 +++++---- test/gitDiff.async.spec.ts | 93 +++++++++++++++++--------------------- 5 files changed, 80 insertions(+), 81 deletions(-) diff --git a/src/git/diffTypes.ts b/src/git/diffTypes.ts index f2c0e91..898266f 100644 --- a/src/git/diffTypes.ts +++ b/src/git/diffTypes.ts @@ -35,3 +35,14 @@ export type DiffPathFilter = { /** Paths under the repo root excluded from the diff (git `:(exclude)` pathspecs). */ excludeFolders?: string[]; }; + +/** Arguments shared by `getDiff`, `getDiffSummary`, and `getChangedFiles`. */ +export type GitDiffRangeQuery = { + from: string; + to: string; + commits: CommitInfo[]; + filterByCommits: boolean; + pathFilter?: DiffPathFilter; + /** When set, skips `git rev-parse` and uses this as the repo root for pathspecs. */ + repoRootOverride?: string; +}; diff --git a/src/git/gitDiff.ts b/src/git/gitDiff.ts index 187be87..755485f 100644 --- a/src/git/gitDiff.ts +++ b/src/git/gitDiff.ts @@ -4,6 +4,7 @@ export type { DiffPathFilter, DiffStatus, DiffSummary, + GitDiffRangeQuery, } from "./diffTypes.js"; export { buildDiffPathspecs } from "./diffPathspecs.js"; diff --git a/src/git/gitDiffOps.ts b/src/git/gitDiffOps.ts index ba72eec..350c66f 100644 --- a/src/git/gitDiffOps.ts +++ b/src/git/gitDiffOps.ts @@ -1,7 +1,12 @@ import { simpleGit } from "simple-git"; import type { SimpleGit } from "simple-git"; -import type { CommitInfo, DiffPathFilter, DiffSummary } from "./diffTypes.js"; +import type { + CommitInfo, + DiffPathFilter, + DiffSummary, + GitDiffRangeQuery, +} from "./diffTypes.js"; import { buildDiffPathspecs } from "./diffPathspecs.js"; import { buildDiffSummaryFromGitOutputs } from "./diffSummaryBuild.js"; @@ -40,13 +45,10 @@ async function getDiffPathContext( export async function getDiff( git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string, + query: GitDiffRangeQuery, ): Promise { + const { from, to, commits, filterByCommits, pathFilter, repoRootOverride } = + query; const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); if (!filterByCommits) { @@ -62,13 +64,10 @@ export async function getDiff( export async function getDiffSummary( git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string, + query: GitDiffRangeQuery, ): Promise { + const { from, to, commits, filterByCommits, pathFilter, repoRootOverride } = + query; const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); if (!filterByCommits) { @@ -102,13 +101,10 @@ export async function getDiffSummary( export async function getChangedFiles( git: SimpleGit, - from: string, - to: string, - commits: CommitInfo[], - filterByCommits: boolean, - pathFilter?: DiffPathFilter, - repoRootOverride?: string, + query: GitDiffRangeQuery, ): Promise { + const { from, to, commits, filterByCommits, pathFilter, repoRootOverride } = + query; const { specs } = await getDiffPathContext(git, pathFilter, repoRootOverride); if (!filterByCommits) { diff --git a/src/index.ts b/src/index.ts index 0ebd579..71f6370 100644 --- a/src/index.ts +++ b/src/index.ts @@ -102,17 +102,18 @@ export async function summarizeGitDiff( options, ); + const rangeQuery = { + from, + to, + commits: filteredCommits, + filterByCommits, + pathFilter, + }; + const [diffText, fileNames, diffSummary] = await Promise.all([ - getDiff(git, from, to, filteredCommits, filterByCommits, pathFilter), - getChangedFiles( - git, - from, - to, - filteredCommits, - filterByCommits, - pathFilter, - ), - getDiffSummary(git, from, to, filteredCommits, filterByCommits, pathFilter), + getDiff(git, rangeQuery), + getChangedFiles(git, rangeQuery), + getDiffSummary(git, rangeQuery), ]); const summarizeFlags: SummarizeFlags = { @@ -141,6 +142,7 @@ export type { DiffFileSummary, DiffPathFilter, DiffSummary, + GitDiffRangeQuery, } from "./git/gitDiff.js"; export { buildDiffPathspecs, diff --git a/test/gitDiff.async.spec.ts b/test/gitDiff.async.spec.ts index 5f34597..52d0035 100644 --- a/test/gitDiff.async.spec.ts +++ b/test/gitDiff.async.spec.ts @@ -65,15 +65,14 @@ describe("getDiff", () => { diff.mockResolvedValue("range-diff"); const commits: CommitInfo[] = [{ hash: "x", message: "m" }]; - const out = await getDiff( - git, - "a", - "b", + const out = await getDiff(git, { + from: "a", + to: "b", commits, - false, - { excludeFolders: ["out"] }, - join(__dirname, "fixture-repo"), - ); + filterByCommits: false, + pathFilter: { excludeFolders: ["out"] }, + repoRootOverride: join(__dirname, "fixture-repo"), + }); expect(out).toBe("range-diff"); expect(diff).toHaveBeenCalledWith(["a..b", "--", ".", ":(exclude)out"]); @@ -90,15 +89,13 @@ describe("getDiff", () => { { hash: "bbb222", message: "b" }, ]; - const out = await getDiff( - git, - "f", - "t", + const out = await getDiff(git, { + from: "f", + to: "t", commits, - true, - undefined, - join(__dirname, "fixture-repo"), - ); + filterByCommits: true, + repoRootOverride: join(__dirname, "fixture-repo"), + }); expect(diff).toHaveBeenCalledWith(["aaa111^!", "--", "."]); expect(diff).toHaveBeenCalledWith(["bbb222^!", "--", "."]); @@ -137,15 +134,13 @@ describe("getDiffSummary", () => { return ""; }); - const summary = await getDiffSummary( - git, - "x", - "y", - [{ hash: "h", message: "m" }], - false, - undefined, - join(__dirname, "fixture-repo"), - ); + const summary = await getDiffSummary(git, { + from: "x", + to: "y", + commits: [{ hash: "h", message: "m" }], + filterByCommits: false, + repoRootOverride: join(__dirname, "fixture-repo"), + }); const paths = new Set(summary.files.map((f) => f.path)); expect(paths.has("added.ts")).toBe(true); @@ -175,18 +170,16 @@ describe("getDiffSummary", () => { return ""; }); - const summary = await getDiffSummary( - git, - "a", - "b", - [ + const summary = await getDiffSummary(git, { + from: "a", + to: "b", + commits: [ { hash: "111aaa", message: "m1" }, { hash: "222bbb", message: "m2" }, ], - true, - undefined, - join(__dirname, "fixture-repo"), - ); + filterByCommits: true, + repoRootOverride: join(__dirname, "fixture-repo"), + }); expect(summary.files.some((f) => f.path === "f.ts")).toBe(true); }); @@ -197,15 +190,13 @@ describe("getChangedFiles", () => { const { git, diff } = makeGitWithDiff(); diff.mockResolvedValue("a.ts\r\nb.ts\r\n"); - const files = await getChangedFiles( - git, - "a", - "b", - [{ hash: "h", message: "m" }], - false, - undefined, - join(__dirname, "fixture-repo"), - ); + const files = await getChangedFiles(git, { + from: "a", + to: "b", + commits: [{ hash: "h", message: "m" }], + filterByCommits: false, + repoRootOverride: join(__dirname, "fixture-repo"), + }); expect(files).toEqual(["a.ts", "b.ts"]); }); @@ -218,18 +209,16 @@ describe("getChangedFiles", () => { .mockResolvedValueOnce("dup.ts\nother.ts\n"); (git as unknown as { show: typeof show }).show = show; - const files = await getChangedFiles( - git as unknown as SimpleGit, - "a", - "b", - [ + const files = await getChangedFiles(git as unknown as SimpleGit, { + from: "a", + to: "b", + commits: [ { hash: "c1", message: "1" }, { hash: "c2", message: "2" }, ], - true, - undefined, - join(__dirname, "fixture-repo"), - ); + filterByCommits: true, + repoRootOverride: join(__dirname, "fixture-repo"), + }); expect(files.sort()).toEqual(["dup.ts", "other.ts"]); });