From a5865923a2b7b08d9f8a26d6fbec290ae66a75ec Mon Sep 17 00:00:00 2001 From: Saffron Date: Mon, 29 Jun 2026 09:36:39 -0600 Subject: [PATCH] feat(groomer): add title rewriting and body enrichment to grooming pipeline Extend the hosted groomer to rewrite bad issue titles and enrich sparse issue bodies, with guardrail logic to prevent unnecessary changes. Title rewriting: - Only triggers when title is <10 chars, single-word generic token (P0, TODO, bug, fix), or clearly a priority/label token - Proposes imperative verb form, 10-200 chars, based on body content Body enrichment: - Only triggers when body is missing, empty, or <100 chars (excluding HTML/markdown comments) - Adds structured context without clobbering existing content Schema validation: - proposedTitle must be 10-200 chars - proposedBody must be under 10000 chars - Both optional with null tolerance Implementation: - Added updateIssueTitleAndBody() to GitHub API helpers - Updated LLM prompt with title/body enrichment instructions - Guardrail checks in run pipeline before applying mutations - Mutation plan tracks originalTitle, proposedTitle, proposedBody - appliedMutations includes titleUpdated and bodyUpdated flags - Full test coverage for guardrails and mutation application --- src/lib/github.ts | 28 +++++ src/lib/groomer/llm.ts | 19 +++- src/lib/groomer/run.test.ts | 186 +++++++++++++++++++++++++++++++++ src/lib/groomer/run.ts | 94 ++++++++++++++++- src/lib/groomer/schema.test.ts | 94 +++++++++++++++++ src/lib/groomer/schema.ts | 28 ++++- 6 files changed, 444 insertions(+), 5 deletions(-) diff --git a/src/lib/github.ts b/src/lib/github.ts index e30aa8b..7d71b98 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -398,6 +398,34 @@ export async function addIssueLabel( } } + +export interface UpdateIssueFields { + title?: string; + body?: string | null; +} + +/** + * Update issue title and/or body via GitHub API. + */ +export async function updateIssueTitleAndBody( + repoFullName: string, + issueNumber: number, + fields: UpdateIssueFields, +): Promise { + const [owner, repo] = repoFullName.split("/"); + const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}`; + + const response = await fetch(url, { + method: "PATCH", + headers: await getHeadersAsync(), + body: JSON.stringify(fields), + }); + + if (!response.ok) { + const text = await response.text(); + throw new Error(`GitHub API error updating issue #${issueNumber}: ${response.status} ${text}`); + } +} export async function removeIssueLabel( repoFullName: string, issueNumber: number, diff --git a/src/lib/groomer/llm.ts b/src/lib/groomer/llm.ts index 00e75ed..6e96580 100644 --- a/src/lib/groomer/llm.ts +++ b/src/lib/groomer/llm.ts @@ -30,7 +30,9 @@ Return ONLY valid JSON with this exact schema: "githubComment": "optional comment to post on the issue (omit if nothing to say)", "needsInfoReason": "optional reason if info is needed", "blockedReason": "optional reason if blocked", - "nextGroomingAction": "optional: promote_to_ready|escalate|mark_not_ready|mark_needs_info|mark_blocked" + "nextGroomingAction": "optional: promote_to_ready|escalate|mark_not_ready|mark_needs_info|mark_blocked", + "proposedTitle": "optional: rewritten title if current one is bad", + "proposedBody": "optional: enriched body if current one is sparse" } Rules: @@ -40,7 +42,20 @@ Rules: - Valid type labels: ${typeLabels} - Never remove agent/* labels - Lane must be one of the configured lane ids -- Be concise in summary and reason fields`; +- Be concise in summary and reason fields + +Title rewriting rules: +- Only propose a new title when the current title is bad: length < 10 chars, matches generic patterns (single word like "P0", "TODO", "bug", "fix"), or is clearly just a priority/label token +- If the title is already descriptive (>= 10 chars and looks like a real sentence/phrase), omit proposedTitle +- The new title should be 10-200 chars, imperative verb form, specific and actionable +- Base the rewritten title on body content, labels, and comments + +Body enrichment rules: +- Only propose an enriched body when the current body is missing, empty, or < 100 characters (excluding markdown/HTML comments) +- If the body already has substantial content, omit proposedBody +- The enriched body should add structure: brief context, what's known, suggested approach based on labels/body/comments +- Do NOT clobber existing body content — if there's any meaningful body, append rather than replace; if empty/missing, create from scratch +- Keep enriched body under 10000 characters`; } export async function callGroomerLLM(options: CallLlmOptions): Promise { diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index 04c2ca9..33482fc 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -23,6 +23,7 @@ const { mocks } = vi.hoisted(() => ({ getHostedGroomerConfig: vi.fn(), updateIssueLabels: vi.fn(), addIssueComment: vi.fn(), + updateIssueTitleAndBody: vi.fn(), findActiveLeasesForIssue: vi.fn(), upsertLease: vi.fn(), releaseLease: vi.fn(), @@ -64,6 +65,7 @@ vi.mock("./config", () => ({ vi.mock("@/lib/github", () => ({ updateIssueLabels: mocks.updateIssueLabels, addIssueComment: mocks.addIssueComment, + updateIssueTitleAndBody: mocks.updateIssueTitleAndBody, addIssueLabel: mocks.addIssueLabel, removeIssueLabel: mocks.removeIssueLabel, })); @@ -131,6 +133,7 @@ describe("runHostedGroomer", () => { mocks.getHostedGroomerConfig.mockReturnValue(mockConfig); mocks.callGroomerLLM.mockResolvedValue(mockOutput); mocks.updateIssueLabels.mockResolvedValue(undefined); + mocks.updateIssueTitleAndBody.mockResolvedValue(undefined); mocks.addIssueComment.mockResolvedValue({ url: null }); mocks.findActiveLeasesForIssue.mockResolvedValue([]); mocks.upsertLease.mockResolvedValue({ created: true, lease: { id: "lease-1" } }); @@ -469,4 +472,187 @@ describe("runHostedGroomer", () => { }), ); }); + + // ─── Title rewriting tests ─── + + it("does not rewrite a good title", async () => { + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Fix the login bug" }, + }); + + const result = await runHostedGroomer(); + + // "Fix login bug" (13 chars) is a good title — should not be rewritten + expect(result!.mutationPlan?.titleRewritten).toBe(false); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("rewrites a bad short title", async () => { + const badCandidate = { ...mockCandidate, title: "P0" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { + ...mockOutput, + proposedTitle: "Fix SSO/OIDC callback state verification mismatch causing 400 errors", + }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(result!.mutationPlan?.originalTitle).toBe("P0"); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ title: "Fix SSO/OIDC callback state verification mismatch causing 400 errors" }), + ); + expect(result!.appliedMutations?.titleUpdated).toBe(true); + }); + + it("rewrites a single-word generic title like TODO", async () => { + const badCandidate = { ...mockCandidate, title: "TODO" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Implement user authentication flow" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + it("rewrites an empty title", async () => { + const badCandidate = { ...mockCandidate, title: "" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Add missing error handling for database connections" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + // ─── Body enrichment tests ─── + + it("does not enrich a substantial body", async () => { + const goodCandidate = { + ...mockCandidate, + body: "This is a detailed issue description that explains the problem clearly with enough context and detail for developers to understand what needs to be done.", + }; + mocks.selectGroomingCandidate.mockResolvedValue(goodCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: "Enriched body content." }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(false); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("enriches a sparse body", async () => { + const sparseCandidate = { ...mockCandidate, body: "Broken." }; + mocks.selectGroomingCandidate.mockResolvedValue(sparseCandidate); + const enrichedBody = `## Context +This issue relates to the login flow. + +## What's known +- Login fails after password reset + +## Suggested approach +Investigate session handling in auth module.`; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: enrichedBody }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ body: enrichedBody }), + ); + expect(result!.appliedMutations?.bodyUpdated).toBe(true); + }); + + it("enriches a null body", async () => { + const noBodyCandidate = { ...mockCandidate, body: null }; + mocks.selectGroomingCandidate.mockResolvedValue(noBodyCandidate); + const enrichedBody = "## Description\nMore detail needed.\n\n## Labels\npriority/p0"; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: enrichedBody }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + it("applies both title rewrite and body enrichment together", async () => { + const badCandidate = { ...mockCandidate, title: "P0", body: "Fix." }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + const enrichedBody = "## Context\nSSO login is broken.\n\n## What's known\nState verification fails on callback."; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { + ...mockOutput, + proposedTitle: "Fix SSO callback state mismatch", + proposedBody: enrichedBody, + }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ + title: "Fix SSO callback state mismatch", + body: enrichedBody, + }), + ); + expect(result!.appliedMutations?.titleUpdated).toBe(true); + expect(result!.appliedMutations?.bodyUpdated).toBe(true); + }); + + it("dry-run includes title/body plan but does not call GitHub API", async () => { + const badCandidate = { ...mockCandidate, title: "P0" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.getHostedGroomerConfig.mockReturnValue({ ...mockConfig, dryRun: true }); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Fix the thing" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.dryRun).toBe(true); + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("skips title/body update when LLM does not propose changes", async () => { + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: mockOutput, // no proposedTitle or proposedBody + }); + + await runHostedGroomer(); + + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index 7c0d40b..afd13a3 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -1,10 +1,10 @@ import { prisma } from "@/lib/prisma"; -import { addIssueComment, updateIssueLabels } from "@/lib/github"; +import { addIssueComment, updateIssueLabels, updateIssueTitleAndBody } from "@/lib/github"; import { findActiveLeasesForIssue, releaseLease, upsertLease } from "@/lib/lease"; import { selectGroomingCandidate } from "./selector"; import { buildIssueContext, fetchIssueComments } from "./context"; import { callGroomerLLM } from "./llm"; -import { validateGroomerOutput } from "./schema"; +import { validateGroomerOutput, type GroomerOutput } from "./schema"; import { getHostedGroomerConfig } from "./config"; import { buildRepositoryContext } from "./repository-context"; import type { RepositoryContextInput, RepositoryContextConfig } from "./repository-context"; @@ -41,6 +41,7 @@ export interface GroomerDeps { getConfig: typeof getHostedGroomerConfig; updateLabels: typeof updateIssueLabels; addComment: typeof addIssueComment; + updateTitleAndBody: typeof updateIssueTitleAndBody; findActiveLeases: typeof findActiveLeasesForIssue; upsertLease: typeof upsertLease; releaseLease: typeof releaseLease; @@ -57,6 +58,7 @@ const defaultDeps: GroomerDeps = { getConfig: getHostedGroomerConfig, updateLabels: updateIssueLabels, addComment: addIssueComment, + updateTitleAndBody: updateIssueTitleAndBody, findActiveLeases: findActiveLeasesForIssue, upsertLease, releaseLease, @@ -185,6 +187,9 @@ export async function runHostedGroomer( const newLabels = applyLabelChanges(candidate.labels, output.labelsToAdd, output.labelsToRemove); + // Compute title/body enrichment decisions + const titleBodyMutations = computeTitleBodyMutations(candidate, output); + // Build mutationPlan const mutationPlan: Record = { labelsToAdd: output.labelsToAdd, @@ -192,6 +197,11 @@ export async function runHostedGroomer( lane: output.lane, summary: output.summary ?? null, willComment: Boolean(output.githubComment?.trim()), + titleRewritten: titleBodyMutations.shouldRewrite, + originalTitle: titleBodyMutations.shouldRewrite ? candidate.title : undefined, + proposedTitle: titleBodyMutations.proposedTitle, + bodyEnriched: titleBodyMutations.shouldEnrich, + proposedBody: titleBodyMutations.proposedBody, }; // Persist stage planned @@ -231,6 +241,20 @@ export async function runHostedGroomer( await deps.updateLabels(candidate.repoFullName, candidate.number, newLabels); appliedMutations.labelsUpdated = true; + // Apply title and/or body updates if guardrails pass + const titleBodyFields: Record = {}; + if (titleBodyMutations.shouldRewrite && titleBodyMutations.proposedTitle) { + titleBodyFields.title = titleBodyMutations.proposedTitle; + } + if (titleBodyMutations.shouldEnrich && titleBodyMutations.proposedBody) { + titleBodyFields.body = titleBodyMutations.proposedBody; + } + if (Object.keys(titleBodyFields).length > 0) { + await deps.updateTitleAndBody(candidate.repoFullName, candidate.number, titleBodyFields as Parameters[2]); + appliedMutations.titleUpdated = titleBodyMutations.shouldRewrite; + appliedMutations.bodyUpdated = titleBodyMutations.shouldEnrich; + } + // Comment with cooldown enforcement if (output.githubComment?.trim()) { let commentPosted = false; @@ -400,6 +424,72 @@ function getCurrentStage(run: { stage?: string }): string { return run.stage ?? "selected"; } +/** + * Check if a title is "bad" and should be rewritten. + * Bad titles: length < 10 chars, or matches generic patterns (single word like "P0", "TODO", etc.), + * or is clearly just a priority/label token. + */ +function shouldRewriteTitle(title: string): boolean { + const trimmed = title.trim(); + if (trimmed.length === 0) return true; + if (trimmed.length < 10) return true; + + // Single word that looks like a generic token + const words = trimmed.split(/\s+/); + if (words.length === 1) { + const lower = trimmed.toLowerCase(); + const GENERIC_TOKENS = ["p0", "p1", "p2", "p3", "p4", "todo", "bug", "fix", "fixme", "wip", "help", "urgent", "critical"]; + if (GENERIC_TOKENS.includes(lower)) return true; + + // Priority/label-like tokens: starts with a letter/digit, no spaces, looks like a label prefix + if (/^[a-z0-9]+$/i.test(trimmed) && trimmed.length <= 6) return true; + } + + return false; +} + +/** + * Check if a body is "sparse" and should be enriched. + * Sparse bodies: missing, empty, or < 100 chars (excluding markdown/HTML comments). + */ +function shouldEnrichBody(body: string | null): boolean { + if (body === null || body.trim().length === 0) return true; + + // Strip HTML comments + let stripped = body.replace(//g, ""); + // Strip markdown-style block comments (if any) + stripped = stripped.replace(/^/gm, ""); + + if (stripped.trim().length < 100) return true; + return false; +} + +/** + * Compute title/body enrichment decisions and build the mutation plan entries. + */ +function computeTitleBodyMutations( + candidate: { title: string; body: string | null }, + output: GroomerOutput, +): { + shouldRewrite: boolean; + shouldEnrich: boolean; + proposedTitle?: string; + proposedBody?: string; +} { + const proposedTitle = typeof output.proposedTitle === "string" ? output.proposedTitle : undefined; + const proposedBody = typeof output.proposedBody === "string" ? output.proposedBody : undefined; + + const shouldRewrite = proposedTitle !== undefined && shouldRewriteTitle(candidate.title); + const shouldEnrich = proposedBody !== undefined && shouldEnrichBody(candidate.body); + + return { + shouldRewrite, + shouldEnrich, + ...(shouldRewrite ? { proposedTitle } : {}), + ...(shouldEnrich ? { proposedBody } : {}), + }; +} + function applyLabelChanges( current: string[], toAdd: string[], diff --git a/src/lib/groomer/schema.test.ts b/src/lib/groomer/schema.test.ts index 706b50b..489b74e 100644 --- a/src/lib/groomer/schema.test.ts +++ b/src/lib/groomer/schema.test.ts @@ -401,4 +401,98 @@ describe("groomer schema validation", () => { const result = validateGroomerOutput(null as any); expect(result.valid).toBe(false); }); + + // ─── proposedTitle validation ─── + + it("accepts valid proposedTitle (10-200 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix SSO/OIDC callback state verification mismatch", + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedTitle).toBe("Fix SSO/OIDC callback state verification mismatch"); + }); + + it("rejects proposedTitle that is too short (< 10 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix bug", + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be between 10 and 200 characters"); + }); + + it("rejects proposedTitle that is too long (> 200 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "A".repeat(201), + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be between 10 and 200 characters"); + }); + + it("accepts null proposedTitle (treated as omitted)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: null as any, + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedTitle).toBeUndefined(); + }); + + // ─── proposedBody validation ─── + + it("accepts valid proposedBody (under 10000 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: "## Context\nThis issue relates to the login flow.", + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedBody).toBe("## Context\nThis issue relates to the login flow."); + }); + + it("rejects proposedBody that is too long (> 10000 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: "A".repeat(10_001), + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedBody must be under 10000 characters"); + }); + + it("accepts null proposedBody (treated as omitted)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: null as any, + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedBody).toBeUndefined(); + }); + + it("accepts both proposedTitle and proposedBody together", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix SSO callback state mismatch in auth module", + proposedBody: "## Context\nThe SSO login flow has a bug.", + }); + expect(result.valid).toBe(true); + }); + + it("rejects non-string proposedTitle when provided", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: 123 as any, + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be a string"); + }); + + it("rejects non-string proposedBody when provided", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: 123 as any, + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedBody must be a string"); + }); }); diff --git a/src/lib/groomer/schema.ts b/src/lib/groomer/schema.ts index 5cc9c47..6d41c24 100644 --- a/src/lib/groomer/schema.ts +++ b/src/lib/groomer/schema.ts @@ -17,6 +17,8 @@ export interface GroomerOutput { needsInfoReason?: string; blockedReason?: string; nextGroomingAction?: GroomAction; + proposedTitle?: string; + proposedBody?: string; } export interface ValidationResult { @@ -113,11 +115,13 @@ const baseSchema = z.object({ // ─── Optional String Fields ─────────────────────────────────────────────────── -const OPTIONAL_STRING_FIELDS: (keyof Pick)[] = [ +const OPTIONAL_STRING_FIELDS: (keyof Pick)[] = [ "summary", "githubComment", "needsInfoReason", "blockedReason", + "proposedTitle", + "proposedBody", ]; // ─── Main Validator ─────────────────────────────────────────────────────────── @@ -277,6 +281,28 @@ export function validateGroomerOutput(data: unknown): ValidationResult { parsed.confidence = resolvedConfidence as GroomerOutput["confidence"]; } + // proposedTitle — length guardrails (10-200 chars) + if ("proposedTitle" in obj) { + const rawTitle = obj.proposedTitle; + if (rawTitle !== null && typeof rawTitle === "string") { + if (rawTitle.length < 10 || rawTitle.length > 200) { + errors.push(`proposedTitle must be between 10 and 200 characters, got ${rawTitle.length}`); + return { valid: false, errors, resolutions }; + } + } + } + + // proposedBody — length guardrails (< 10000 chars) + if ("proposedBody" in obj) { + const rawBody = obj.proposedBody; + if (rawBody !== null && typeof rawBody === "string") { + if (rawBody.length > 10_000) { + errors.push(`proposedBody must be under 10000 characters, got ${rawBody.length}`); + return { valid: false, errors, resolutions }; + } + } + } + // Optional string fields — null tolerance (null treated as absent) for (const field of OPTIONAL_STRING_FIELDS) { if (!(field in obj)) continue;