From 5452058cdfd13293240e9e824424a464e28335b3 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 8 Jul 2026 05:07:53 -0700 Subject: [PATCH] feat(review): reuse review.instructions/pathInstructions for E2E test generation Adds resolveE2eTestGenInstructions, combining a repo's existing review.instructions (repo-wide brief) with any review.pathInstructions entries matching the PR's changed files into one instructions block for AI-generated E2E test coverage (#4189 epic). Reuses the config-as-code mechanism already parsed and already feeding the AI reviewer end to end (resolveReviewPathInstructions), rather than inventing a second, e2e-test-gen-specific instructions schema: a maintainer's repo conventions and path-scoped rules apply equally well to steering an AI reviewer or an AI test generator, so both draw from the same maintainer-authored brief written once. Closes #4200. --- .gittensory.yml.example | 7 ++- config/examples/gittensory.full.yml | 7 ++- .../gittensory-engine/src/focus-manifest.ts | 8 ++- src/services/ai-e2e-test-gen.ts | 24 +++++++- test/unit/ai-e2e-test-gen.test.ts | 57 +++++++++++++++++++ 5 files changed, 96 insertions(+), 7 deletions(-) diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 0a21e37b64..6ffabf2590 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -883,10 +883,13 @@ settings: # security_focus: false # # A repo-level natural-language brief handed to the AI reviewer on EVERY review (vs the per-path # # path_instructions below) -- the maintainer's conventions/voice. Bounded + public-safe at parse time. -# # String or null. Default: null (byte-identical prompt). +# # String or null. Default: null (byte-identical prompt). Also feeds AI-generated E2E test coverage +# # (features.e2eTests, #4190/#4189) when that feature is enabled -- the same conventions brief steers +# # both the AI reviewer and the AI test generator, so you only write it once. # instructions: "Prefer small, focused PRs. Flag any missing test for a bug fix." # # Per-path natural-language guidance handed to the AI reviewer when a changed file matches the glob. -# # Empty/default ⇒ byte-identical prompt. +# # Empty/default ⇒ byte-identical prompt. Also feeds AI-generated E2E test coverage the same way as +# # `instructions` above, for path-scoped rules ("always test the payment-failure retry path"). # path_instructions: # - path: "src/db/**" # instructions: "Flag any migration missing a matching down-path note." diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 09ebb04588..63ef086d0e 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -896,10 +896,13 @@ settings: # security_focus: false # # A repo-level natural-language brief handed to the AI reviewer on EVERY review (vs the per-path # # path_instructions below) -- the maintainer's conventions/voice. Bounded + public-safe at parse time. -# # String or null. Default: null (byte-identical prompt). +# # String or null. Default: null (byte-identical prompt). Also feeds AI-generated E2E test coverage +# # (features.e2eTests, #4190/#4189) when that feature is enabled -- the same conventions brief steers +# # both the AI reviewer and the AI test generator, so you only write it once. # instructions: "Prefer small, focused PRs. Flag any missing test for a bug fix." # # Per-path natural-language guidance handed to the AI reviewer when a changed file matches the glob. -# # Empty/default ⇒ byte-identical prompt. +# # Empty/default ⇒ byte-identical prompt. Also feeds AI-generated E2E test coverage the same way as +# # `instructions` above, for path-scoped rules ("always test the payment-failure retry path"). # path_instructions: # - path: "src/db/**" # instructions: "Flag any migration missing a matching down-path note." diff --git a/packages/gittensory-engine/src/focus-manifest.ts b/packages/gittensory-engine/src/focus-manifest.ts index f8f5a420e6..ea1b8b6699 100644 --- a/packages/gittensory-engine/src/focus-manifest.ts +++ b/packages/gittensory-engine/src/focus-manifest.ts @@ -527,11 +527,15 @@ export type FocusManifestReviewConfig = { * (#1955) knobs. (#2047) */ commentVerbosity: CommentVerbosity | null; /** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's - * changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */ + * changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. Also consumed by + * AI-generated E2E test coverage (`resolveE2eTestGenInstructions` in `ai-e2e-test-gen.ts`, #4200) when + * that feature is enabled — the same maintainer-authored guidance steers both consumers, no separate + * test-generation-specific instructions schema. (#review-path-instructions) */ pathInstructions: ReviewPathInstruction[]; /** `review.instructions`: a repo-level natural-language brief handed to the AI reviewer on EVERY review (vs the * per-path path_instructions) — the maintainer's conventions/voice for this repo. Bounded + public-safe at parse - * time (so it stays cost-cheap, unlike ingesting a whole CLAUDE.md). null (default, absent) ⇒ byte-identical + * time (so it stays cost-cheap, unlike ingesting a whole CLAUDE.md). Also consumed by AI-generated E2E test + * coverage (#4200) for the same reason as pathInstructions above. null (default, absent) ⇒ byte-identical * reviewer prompt. (#review-instructions) */ instructions: string | null; /** `review.exclude_paths`: globs whose matching files are EXCLUDED from the AI review (diff + grounding + RAG) diff --git a/src/services/ai-e2e-test-gen.ts b/src/services/ai-e2e-test-gen.ts index 9f06328433..c03d6b0a56 100644 --- a/src/services/ai-e2e-test-gen.ts +++ b/src/services/ai-e2e-test-gen.ts @@ -1,4 +1,4 @@ -// Gittensory AI-generated E2E test coverage (the `e2eTests` capability, #4191, part of the #4189 epic). +// Gittensory AI-generated E2E test coverage (the `e2eTests` capability, #4191/#4200, part of the #4189 epic). // // Turns a PR's changed-file diffs into a complete Playwright test file, following the SAME shape as // `ai-slop.ts`'s AI-assisted advisory: an opt-in, fail-safe second capability layered on top of the @@ -25,6 +25,7 @@ import { countByokAiEventsForRepoSince, recordAiUsageEvent, sumAiEstimatedNeuron import { convergedFeatureActive } from "../review/feature-activation"; import { defangReviewInput } from "../review/safety"; import { isE2eTestGenerationEnabled } from "../review/e2e-test-gen-wire"; +import { resolveReviewPathInstructions, type FocusManifestReviewConfig } from "../signals/focus-manifest"; import { type AiReviewActualUsage, type AiReviewProviderKey, @@ -112,6 +113,27 @@ export function buildE2eTestGenDiffText(files: E2eTestGenChangedFile[]): string .slice(0, MAX_DIFF_CHARS); } +/** + * Pure: combine a repo's general `review.instructions` (#4200) with any `review.pathInstructions` entries + * matching the PR's changed files into one instructions block for E2E test generation — reusing the + * EXISTING config-as-code mechanism the AI reviewer itself already consumes (`resolveReviewPathInstructions`), + * rather than inventing a second, e2e-test-gen-specific instructions schema. A maintainer's repo-wide + * conventions ("use Playwright with our page-object pattern under test/e2e/pages/") and path-scoped rules + * ("always test the payment-failure retry path for src/checkout/**") apply equally well to steering an AI + * reviewer or an AI test generator, so both draw from the same maintainer-authored brief. Returns null when + * nothing is configured (no repo-wide instructions, no matching path instructions) — never an empty string, + * so a caller can treat "no instructions" and "instructions" as a clean two-way branch. + */ +export function resolveE2eTestGenInstructions( + review: Pick | null | undefined, + changedPaths: string[], +): string | null { + const repoWide = review?.instructions?.trim() || ""; + const pathGuidance = resolveReviewPathInstructions(review?.pathInstructions ?? [], changedPaths).trim(); + const combined = [repoWide, pathGuidance].filter(Boolean).join("\n\n"); + return combined || null; +} + /** * Pure: build the user prompt from an already-assembled (and, if the `safety` feature is on, already * defanged) diff/title/body. Callers that need defanging apply it before calling this — this function diff --git a/test/unit/ai-e2e-test-gen.test.ts b/test/unit/ai-e2e-test-gen.test.ts index ed9cba81c3..4eb02df267 100644 --- a/test/unit/ai-e2e-test-gen.test.ts +++ b/test/unit/ai-e2e-test-gen.test.ts @@ -4,10 +4,12 @@ import { buildE2eTestGenDiffText, buildE2eTestGenPrompt, parseE2eTestGenResponse, + resolveE2eTestGenInstructions, runGittensoryE2eTestGeneration, type E2eTestGenInput, } from "../../src/services/ai-e2e-test-gen"; import { recordAiUsageEvent } from "../../src/db/repositories"; +import type { FocusManifestReviewConfig } from "../../src/signals/focus-manifest"; import { createTestEnv } from "../helpers/d1"; const { runWorkersE2eTestGen } = __aiE2eTestGenInternals; @@ -105,6 +107,61 @@ describe("buildE2eTestGenPrompt", () => { }); }); +function reviewWith(over: Partial>) { + return { instructions: null, pathInstructions: [], ...over }; +} + +describe("resolveE2eTestGenInstructions", () => { + it("returns null when nothing is configured", () => { + expect(resolveE2eTestGenInstructions(reviewWith({}), ["src/a.ts"])).toBeNull(); + }); + + it("returns null for a null/undefined review config (tolerates an absent manifest)", () => { + expect(resolveE2eTestGenInstructions(null, ["src/a.ts"])).toBeNull(); + expect(resolveE2eTestGenInstructions(undefined, ["src/a.ts"])).toBeNull(); + }); + + it("returns the repo-wide instructions alone when no path instructions match", () => { + const result = resolveE2eTestGenInstructions( + reviewWith({ instructions: "Use Playwright with our page-object pattern." }), + ["src/other.ts"], + ); + expect(result).toBe("Use Playwright with our page-object pattern."); + }); + + it("returns matching path instructions alone when no repo-wide instructions are set", () => { + const result = resolveE2eTestGenInstructions( + reviewWith({ pathInstructions: [{ path: "src/checkout/**", instructions: "Always test the payment-failure retry path." }] }), + ["src/checkout/pay.ts"], + ); + expect(result).toContain("Always test the payment-failure retry path."); + }); + + it("combines repo-wide and matching path instructions together", () => { + const result = resolveE2eTestGenInstructions( + reviewWith({ + instructions: "Use Playwright with our page-object pattern.", + pathInstructions: [{ path: "src/checkout/**", instructions: "Always test the payment-failure retry path." }], + }), + ["src/checkout/pay.ts"], + ); + expect(result).toContain("Use Playwright with our page-object pattern."); + expect(result).toContain("Always test the payment-failure retry path."); + }); + + it("omits a path instruction whose glob does not match any changed file", () => { + const result = resolveE2eTestGenInstructions( + reviewWith({ + instructions: "Use Playwright.", + pathInstructions: [{ path: "src/checkout/**", instructions: "Payment-specific guidance." }], + }), + ["src/unrelated.ts"], + ); + expect(result).toBe("Use Playwright."); + expect(result).not.toContain("Payment-specific guidance"); + }); +}); + describe("parseE2eTestGenResponse", () => { it("extracts source from a fenced code block", () => { expect(parseE2eTestGenResponse(fenced(VALID_TEST_SOURCE))).toBe(VALID_TEST_SOURCE);