From 396e120755622ce990e749043d4aba219b455dba Mon Sep 17 00:00:00 2001 From: Matt Carvin <90224411+mcarvin8@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:35:04 -0400 Subject: [PATCH] fix(quality): fix quality checks in ai folder --- src/ai/aiConstants.ts | 20 ++++++++++ src/ai/aiSummary.ts | 64 ++++++++++-------------------- src/ai/aiTypes.ts | 29 ++++++++++++++ src/index.ts | 22 ++++++----- test/aiSummary.spec.ts | 89 +++++++++++++++++++++++++++++++----------- 5 files changed, 148 insertions(+), 76 deletions(-) create mode 100644 src/ai/aiConstants.ts create mode 100644 src/ai/aiTypes.ts diff --git a/src/ai/aiConstants.ts b/src/ai/aiConstants.ts new file mode 100644 index 0000000..e921c45 --- /dev/null +++ b/src/ai/aiConstants.ts @@ -0,0 +1,20 @@ +/** + * Cap on unified-diff characters sent to the LLM (only the diff body; preamble is extra). + * Tuned for ~128k-token context models; override with `LLM_MAX_DIFF_CHARS` or `maxDiffChars` in options. + */ +export const DEFAULT_LLM_MAX_DIFF_CHARS = 120_000; + +/** Default system prompt when summarizing a git diff for any repository. */ +export const DEFAULT_GIT_DIFF_SYSTEM_PROMPT = `You are a senior software engineer helping developers understand code and configuration changes from the git context they supplied. +You receive: commit subject lines (when available), changed file paths, and unified git patch(es)—either one range diff or concatenated per-commit patches, depending on how the diff was produced. Patches may be truncated mid-section with an explicit marker—do not infer changes beyond visible lines. +Explain what changed in terms of behavior, APIs, data, configuration, security, and operational risk. Tie claims to the patch when possible. +Produce a concise, developer-focused summary in Markdown. +Use sections that fit the change (for example: Highlights, Breaking or risky changes, API / contract changes, Data & schema, Configuration & infra, Security & auth, Tests & quality). Omit empty sections. +Group related changes; do not list every individual file. When multiple commits appear in the context, briefly separate notable themes by commit when helpful. +If the user message includes a Team line, use that exact team name in the summary title (for example: "## – Change summary" or similar).`; + +/** 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."; diff --git a/src/ai/aiSummary.ts b/src/ai/aiSummary.ts index c3a561e..622dce3 100644 --- a/src/ai/aiSummary.ts +++ b/src/ai/aiSummary.ts @@ -4,12 +4,16 @@ import { shouldUseLlmGateway, type OpenAiLikeClient, } from "./openAIConfig.js"; - -/** - * Cap on unified-diff characters sent to the LLM (only the diff body; preamble is extra). - * Tuned for ~128k-token context models; override with `LLM_MAX_DIFF_CHARS` or `maxDiffChars` in options. - */ -const DEFAULT_LLM_MAX_DIFF_CHARS = 120_000; +import { + DEFAULT_LLM_MAX_DIFF_CHARS, + DEFAULT_GIT_DIFF_SYSTEM_PROMPT, + LLM_GATEWAY_REQUIRED_MESSAGE, +} from "./aiConstants.js"; +import type { + GenerateSummaryInput, + OpenAiClientProvider, + SummarizeFlags, +} from "./aiTypes.js"; /** Resolve max unified-diff characters for the LLM path. CLI wins, then env, then default. */ export function resolveLlmMaxDiffChars(cliOverride?: number): number { @@ -41,46 +45,18 @@ export function truncateUnifiedDiffForLlm( return diffText.slice(0, maxChars) + marker; } -/** Default system prompt when summarizing a git diff for any repository. */ -export const DEFAULT_GIT_DIFF_SYSTEM_PROMPT = `You are a senior software engineer helping developers understand code and configuration changes from the git context they supplied. -You receive: commit subject lines (when available), changed file paths, and unified git patch(es)—either one range diff or concatenated per-commit patches, depending on how the diff was produced. Patches may be truncated mid-section with an explicit marker—do not infer changes beyond visible lines. -Explain what changed in terms of behavior, APIs, data, configuration, security, and operational risk. Tie claims to the patch when possible. -Produce a concise, developer-focused summary in Markdown. -Use sections that fit the change (for example: Highlights, Breaking or risky changes, API / contract changes, Data & schema, Configuration & infra, Security & auth, Tests & quality). Omit empty sections. -Group related changes; do not list every individual file. When multiple commits appear in the context, briefly separate notable themes by commit when helpful. -If the user message includes a Team line, use that exact team name in the summary title (for example: "## – Change summary" or similar).`; - -/** 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."; - -export type SummarizeFlags = { - /** Start ref for the diff. */ - from: string; - to?: string; - model?: string; - /** Optional team or squad label for the summary title and context. */ - team?: string; - /** Max characters of unified diff sent to the LLM; see `resolveLlmMaxDiffChars`. */ - maxDiffChars?: number; - /** When set, replaces {@link DEFAULT_GIT_DIFF_SYSTEM_PROMPT} for the chat completion. */ - systemPrompt?: string; - commitMessageIncludeRegexes?: string[]; - commitMessageExcludeRegexes?: string[]; -}; - -type OpenAiClientProvider = () => Promise; - export async function generateSummary( - diffText: string, - fileNames: string[], - commits: CommitInfo[], - flags: SummarizeFlags, - openAiClientProvider?: OpenAiClientProvider, - diffSummary?: DiffSummary, + input: GenerateSummaryInput, ): Promise { + const { + diffText, + fileNames, + commits, + flags, + openAiClientProvider, + diffSummary, + } = input; + if (!shouldUseLlmGateway() && openAiClientProvider === undefined) { throw new Error(LLM_GATEWAY_REQUIRED_MESSAGE); } diff --git a/src/ai/aiTypes.ts b/src/ai/aiTypes.ts new file mode 100644 index 0000000..0c804ce --- /dev/null +++ b/src/ai/aiTypes.ts @@ -0,0 +1,29 @@ +import type { CommitInfo, DiffSummary } from "../git/gitDiff.js"; +import { type OpenAiLikeClient } from "./openAIConfig.js"; + +export type SummarizeFlags = { + /** Start ref for the diff. */ + from: string; + to?: string; + model?: string; + /** Optional team or squad label for the summary title and context. */ + team?: string; + /** Max characters of unified diff sent to the LLM; see `resolveLlmMaxDiffChars`. */ + maxDiffChars?: number; + /** When set, replaces {@link DEFAULT_GIT_DIFF_SYSTEM_PROMPT} for the chat completion. */ + systemPrompt?: string; + commitMessageIncludeRegexes?: string[]; + commitMessageExcludeRegexes?: string[]; +}; + +export type OpenAiClientProvider = () => Promise; + +/** Input object for `generateSummary` (see `aiSummary.ts`). */ +export type GenerateSummaryInput = { + diffText: string; + fileNames: string[]; + commits: CommitInfo[]; + flags: SummarizeFlags; + openAiClientProvider?: OpenAiClientProvider; + diffSummary?: DiffSummary; +}; diff --git a/src/index.ts b/src/index.ts index 71f6370..182c390 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ import type { SimpleGit } from "simple-git"; -import { generateSummary, type SummarizeFlags } from "./ai/aiSummary.js"; +import { generateSummary } from "./ai/aiSummary.js"; +import type { SummarizeFlags } from "./ai/aiTypes.js"; import type { OpenAiLikeClient } from "./ai/openAIConfig.js"; import { createGitClient, @@ -127,14 +128,14 @@ export async function summarizeGitDiff( commitMessageExcludeRegexes: options.commitMessageExcludeRegexes, }; - return generateSummary( + return generateSummary({ diffText, fileNames, - filteredCommits, - summarizeFlags, - options.openAiClientProvider, + commits: filteredCommits, + flags: summarizeFlags, + openAiClientProvider: options.openAiClientProvider, diffSummary, - ); + }); } export type { @@ -155,15 +156,18 @@ export { getRepoRoot, } from "./git/gitDiff.js"; -export type { SummarizeFlags } from "./ai/aiSummary.js"; +export type { GenerateSummaryInput, SummarizeFlags } from "./ai/aiTypes.js"; export { - DEFAULT_GIT_DIFF_SYSTEM_PROMPT, generateSummary, - LLM_GATEWAY_REQUIRED_MESSAGE, resolveLlmMaxDiffChars, truncateUnifiedDiffForLlm, } from "./ai/aiSummary.js"; +export { + DEFAULT_GIT_DIFF_SYSTEM_PROMPT, + LLM_GATEWAY_REQUIRED_MESSAGE, +} from "./ai/aiConstants.js"; + export type { OpenAiLikeClient, OpenAiLikeClientInit, diff --git a/test/aiSummary.spec.ts b/test/aiSummary.spec.ts index f9112f4..f27e134 100644 --- a/test/aiSummary.spec.ts +++ b/test/aiSummary.spec.ts @@ -1,12 +1,14 @@ 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"; +import { + DEFAULT_GIT_DIFF_SYSTEM_PROMPT, + LLM_GATEWAY_REQUIRED_MESSAGE, +} from "../src/ai/aiConstants"; function mockLlmClient( content: string, @@ -98,26 +100,31 @@ describe("generateSummary", () => { jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(false); await expect( - generateSummary("+added line", ["src/a.ts"], commits, flagsBase), + generateSummary({ + diffText: "+added line", + fileNames: ["src/a.ts"], + commits, + flags: flagsBase, + }), ).rejects.toThrow(LLM_GATEWAY_REQUIRED_MESSAGE); }); it("uses openAiClientProvider when gateway env is off", async () => { jest.spyOn(openAIConfig, "shouldUseLlmGateway").mockReturnValue(false); - const md = await generateSummary( - "diff...", - ["f.ts"], + const md = await generateSummary({ + diffText: "diff...", + fileNames: ["f.ts"], commits, - { + flags: { ...flagsBase, team: "QA", systemPrompt: "You are a test bot.", model: "gpt-test", maxDiffChars: 1000, }, - mockLlmClient(" **Summary** from inject "), - ); + openAiClientProvider: mockLlmClient(" **Summary** from inject "), + }); expect(md).toBe("**Summary** from inject"); }); @@ -135,12 +142,17 @@ describe("generateSummary", () => { }, } as Awaited>); - const md = await generateSummary("diff...", ["f.ts"], commits, { - ...flagsBase, - team: "QA", - systemPrompt: "You are a test bot.", - model: "gpt-test", - maxDiffChars: 1000, + const md = await generateSummary({ + diffText: "diff...", + fileNames: ["f.ts"], + commits, + flags: { + ...flagsBase, + team: "QA", + systemPrompt: "You are a test bot.", + model: "gpt-test", + maxDiffChars: 1000, + }, }); expect(md).toBe("**Summary** from model"); @@ -170,7 +182,12 @@ describe("generateSummary", () => { chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary("d", [], [], flagsBase); + await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: flagsBase, + }); expect(completionCreate).toHaveBeenCalledWith( expect.objectContaining({ model: "gpt-4o-mini" }), ); @@ -185,9 +202,14 @@ describe("generateSummary", () => { chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary("d", [], [], { - ...flagsBase, - commitMessageExcludeRegexes: ["^WIP"], + await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: { + ...flagsBase, + commitMessageExcludeRegexes: ["^WIP"], + }, }); const userMsg = completionCreate.mock.calls[0]![0].messages.find( (m: { role: string }) => m.role === "user", @@ -205,7 +227,12 @@ describe("generateSummary", () => { chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary("d", [], [], { ...flagsBase, team: " " }); + await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: { ...flagsBase, team: " " }, + }); const userMsg = completionCreate.mock.calls[0]![0].messages.find( (m: { role: string }) => m.role === "user", )?.content as string; @@ -227,7 +254,13 @@ describe("generateSummary", () => { totalAdditions: 0, totalDeletions: 0, }; - await generateSummary("d", [], [], flagsBase, undefined, diffSummary); + await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: flagsBase, + diffSummary, + }); const userMsg = completionCreate.mock.calls[0]![0].messages.find( (m: { role: string }) => m.role === "user", )?.content as string; @@ -247,7 +280,12 @@ describe("generateSummary", () => { }, } as Awaited>); - const md = await generateSummary("d", [], [], flagsBase); + const md = await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: flagsBase, + }); expect(md).toBe("No summary generated by OpenAI."); }); @@ -262,7 +300,12 @@ describe("generateSummary", () => { chat: { completions: { create: completionCreate } }, } as Awaited>); - await generateSummary("d", [], [], flagsBase); + await generateSummary({ + diffText: "d", + fileNames: [], + commits: [], + flags: flagsBase, + }); expect(completionCreate).toHaveBeenCalledWith( expect.objectContaining({ max_tokens: 4000 }), );