diff --git a/Makefile b/Makefile index d61da1e..b274025 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: init build run install uninstall check check-workflows typecheck test clean help models-list +.PHONY: init build run install uninstall check check-workflows typecheck test evals clean help models-list # Default target help: @@ -13,6 +13,7 @@ help: @echo " make check-workflows Validate workflows with actionlint" @echo " make typecheck Run TypeScript type checking" @echo " make test Run tests" + @echo " make evals Run deterministic synthetic harness evals (no inference or network)" @echo " make models-list Regenerate models.md from the model registry" @echo " make clean Remove dist/" @echo "" @@ -60,6 +61,9 @@ check: test: pnpm test +evals: + pnpm exec vitest run evals/synthetics + models-list: build pnpm run models-list diff --git a/README.md b/README.md index 23c3ff6..4ad52b9 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,8 @@ codegenie review --pr 123 --post-github-comments # publish inline comments (ex Posting to GitHub is a single `COMMENT`-type review with inline comments anchored to changed lines — it never approves or requests changes, and only happens when you pass the flag. Interactive runs show a stderr progress spinner (auto-disabled in CI; `--no-progress` disables it explicitly). Non-posting Markdown/JSON runs emit the full report to stdout; posting runs emit a concise posting summary instead. Action mode separately renders the full report into the status comment, step summary, and report artifact. +For prose files (`.md`, `.mdx`, `.rst`, `.txt`), unchanged inline comment content is deduplicated across shifted anchors on the same file and diff side. Matching uses the complete sanitized body before truncation; changed wording remains eligible for posting. Executable examples under `docs/` retain code fingerprint matching. Separate findings are not merged merely because they occur in the same document. + ## GitHub Action codegenie ships as a reusable GitHub Action: reviews run automatically on PR open/update, or on demand when a collaborator comments `codegenie review` on a PR. The run posts a single status comment ("Reviewing ...") that live-updates through the pipeline stages and finishes as the full markdown report; inline finding comments post as a PR review alongside it (on by default, `post-inline-comments: "false"` disables). @@ -280,6 +282,8 @@ Verification assesses proposed fixes and tests independently from the defect, us Run `pnpm run check` for TypeScript and GitHub workflow validation, then `pnpm test` and `pnpm build` for the full suite. Workflow validation uses [`actionlint`](https://github.com/rhysd/actionlint); `pnpm test` runs it automatically so GitHub expression/context errors cannot pass while unit tests remain green. +Run `make evals` for the deterministic [synthetic harness evals](evals/synthetics/README.md). These scenarios exercise harness behavior without network requests or model inference, and also run in `pnpm test`. + ## Status codegenie is a pre-1.0 CLI being hardened through live evals. Full specifications live in [`specs/project/`](specs/project/): diff --git a/evals/synthetics/README.md b/evals/synthetics/README.md new file mode 100644 index 0000000..693ef70 --- /dev/null +++ b/evals/synthetics/README.md @@ -0,0 +1,22 @@ +# Synthetic harness evals + +Run `make evals` from the repository root. These deterministic scenarios use real +harness components with scripted model/GitHub boundaries: no credentials, network +requests, or paid inference. They also run under `pnpm test`. + +The documentation comment suite simulates successive pushes through diff parsing, +publication, persisted comment markers, GitHub response parsing, and duplicate +matching. It checks shifted lines, changed anchor text, legacy comments, unrelated +issues on nearby and distant lines, different files, and capped comment bodies. +It also checks anchors supplied by the posting plan and makes the conservative +wording policy explicit: paraphrased findings remain eligible for posting. + +The original PR #28 cases also live in `tests/pipeline-phase5.test.ts`: an unchanged +finding survives a seven-line shift at publication, while distinct same-category +findings in a document remain separate. The latter deliberately reverses PR #28's +file-wide merging tradeoff. Existing code fingerprint tests, including executable +examples under `docs/`, retain symbol-based identity. + +These evaluate harness behavior, not whether a model identifies or phrases a +finding consistently. Use `codegenie eval --eval-dir ...` for live model comparisons. +Add additional `*.test.ts` scenarios here as harness failure patterns are found. diff --git a/evals/synthetics/docs-comment-identity.test.ts b/evals/synthetics/docs-comment-identity.test.ts new file mode 100644 index 0000000..10034b6 --- /dev/null +++ b/evals/synthetics/docs-comment-identity.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vitest"; +import { defaultConfig } from "../../src/config/schema.js"; +import { parseDiff } from "../../src/git/diff-parser.js"; +import { createGitHubClient } from "../../src/github/github-client.js"; +import { parseCodegenieMarker } from "../../src/github/duplicate-detector.js"; +import { maybePublishToGitHub } from "../../src/github/publisher.js"; +import { sha256Hex } from "../../src/util/hashing.js"; +import type { FinalFinding, InlineCommentInput, ResolvedReviewInput, ReviewResult } from "../../src/types.js"; +import type { runGh } from "../../src/git/subprocess.js"; +import { nullTelemetry } from "../../tests/helpers/git.js"; + +const enumAdvice = "The table lists `widgetCreated`, but `EventKind` does not define it. Use a supported event value."; +const retryAdvice = "These retry instructions can submit the same order twice. Require an idempotency key before retrying."; + +// Exercise the real diff parser, publisher, marker serialization, GitHub client, +// and duplicate detector across pushes. Only the GitHub transport is scripted. +describe("synthetic: documentation comment identity across pushes", () => { + it.each([ + { name: "unchanged finding after a seven-line shift", first: enumAdvice, second: enumAdvice, line: 74, expected: 0 }, + { name: "same finding on a table row instead of its prose bullet", first: enumAdvice, second: enumAdvice, line: 40, expected: 0 }, + { name: "unchanged finding using its validated posting-plan anchor", first: enumAdvice, second: enumAdvice, line: 74, expected: 0, plannedAnchor: true }, + { name: "unrelated finding far away in the same document", first: enumAdvice, second: retryAdvice, line: 180, expected: 1 }, + { name: "unrelated finding within the old five-line fuzzy window", first: enumAdvice, second: retryAdvice, line: 82, expected: 1 }, + { name: "changed advice at the same location", first: enumAdvice, second: retryAdvice, line: 81, expected: 1 }, + { name: "paraphrased finding stays eligible without a proven content match", first: enumAdvice, second: "EventKind has no widgetCreated member; correct the documented event value.", line: 74, expected: 1 }, + { name: "legacy marker and an unchanged body", first: enumAdvice, second: enumAdvice, line: 74, expected: 0, legacy: true }, + { name: "legacy sanitized body with a mention", first: `${enumAdvice} Ask @maintainer.`, second: `${enumAdvice} Ask @maintainer.`, line: 74, expected: 0, legacy: true }, + { name: "same published text in a different file", first: enumAdvice, second: enumAdvice, line: 74, expected: 1, path: "docs/other.md" }, + { name: "unchanged long report despite the inline body cap", first: enumAdvice.repeat(130), second: enumAdvice.repeat(130), line: 74, expected: 0 }, + { name: "truncated legacy report cannot prove a full-content match", first: enumAdvice.repeat(130), second: enumAdvice.repeat(130), line: 74, expected: 1, legacy: true }, + { name: "different conclusions after an identical capped prefix", first: enumAdvice.repeat(130) + "\nFirst conclusion.", second: enumAdvice.repeat(130) + "\nDifferent conclusion.", line: 81, expected: 1 } + ])("$name", async scenario => { + const remote = scriptedGitHub(); + const first = snapshot("docs/api.md", 81, "- The event is `widgetCreated`.", scenario.first); + const firstResult = await maybePublishToGitHub(first.review, first.resolved, defaultConfig, nullTelemetry(), { github: remote.client }); + expect(firstResult?.inlinePosted).toBe(1); + expect(remote.comments).toHaveLength(1); + expect(parseCodegenieMarker(remote.comments[0]!.body)?.contentFingerprint).toMatch(/^[0-9a-f]{64}$/u); + if (scenario.legacy) remote.comments[0]!.body = remote.comments[0]!.body.replace(/;content=[0-9a-f]{64}/u, ""); + // GitHub returns original_line for an outdated anchor after another push. + remote.comments[0]!.line = null; + remote.advance(); + const second = snapshot(scenario.path ?? "docs/api.md", scenario.line, "| event | `widgetCreated` |", scenario.second, "2".repeat(40)); + if (scenario.plannedAnchor) delete second.review.findings[0]!.anchor; + const result = await maybePublishToGitHub(second.review, second.resolved, defaultConfig, nullTelemetry(), { github: remote.client }); + expect(result?.inlinePosted).toBe(scenario.expected); + expect(result?.skippedDuplicates).toBe(1 - scenario.expected); + expect(remote.comments).toHaveLength(1 + scenario.expected); + expect(result?.duplicateDecisions?.[0]?.action).toBe(scenario.expected ? "post" : "skip_unchanged_content"); + }); +}); + +function snapshot(path: string, line: number, changedText: string, body: string, headSha = "1".repeat(40)) { + const rawDiff = `diff --git a/${path} b/${path}\n--- a/${path}\n+++ b/${path}\n@@ -${line},1 +${line},1 @@\n-old text\n+${changedText}\n`; + const hunk = parseDiff(rawDiff).files[0]!.hunks[0]!; + const finding: FinalFinding = { + id: "f1", path, title: "Documentation contradicts the supported behavior", severity: "medium", confidence: "high", + category: "correctness", changedLine: true, anchor: { path, line, side: "RIGHT", hunkId: hunk.id }, + evidence: { changedCode: changedText }, failureMode: body, whyThisMatters: body, verification: "Checked against the source contract.", + producedBy: { kind: "packet", stage: 7, packetId: `packet-${hunk.id}`, lensId: "core/code-review", skillIds: [] }, + fingerprint: sha256Hex([path, hunk.id, "correctness", "core/code-review"].join("\0")), + finalBody: body, publication: "inline", mergedCandidateIds: ["f1"] + }; + const review: ReviewResult = { + summary: "One finding.", findings: [finding], summaryOnlyFindings: [], needsHumanAttention: [], noFindings: false, + coverage: { totalHunks: 1, reviewedHunks: 1, skippedHunks: 0, failedHunks: 0, + coverageByLevel: { deep: 0, normal: 1, light: 0, skip: 0 }, degradedPlanning: false, budgetStopped: false, + verificationIncompleteCount: 0, partial: false, reasons: [] }, + postingPlan: { inline: [{ findingId: finding.id, anchor: finding.anchor! }], reviewBody: "One finding." } + }; + const resolved: ResolvedReviewInput = { + mode: "github_pr", repoRoot: "/synthetic", commits: [], rawDiff, + pr: { owner: "fixture", repo: "fixture", number: 1, title: "Docs update", body: "", url: "https://example.invalid/pr/1", + baseRefName: "main", baseSha: "b".repeat(40), headRefName: "feature", headSha } + }; + return { review, resolved }; +} + +function scriptedGitHub() { + type Comment = { id: number; path: string; side: "RIGHT" | "LEFT"; line: number | null; original_line: number; body: string; user: { login: string } }; + const comments: Comment[] = []; + let headSha = "1".repeat(40); + const gh: typeof runGh = async (_cwd, args, opts) => { + if (args[0] === "--version" || args.join(" ") === "auth status") return ""; + if (args[0] === "repo") return JSON.stringify({ owner: { login: "fixture" }, name: "fixture" }); + if (args[0] === "pr") return JSON.stringify({ number: 1, baseRefOid: "b".repeat(40), headRefOid: headSha }); + if (args[1] === "user") return "codegenie-bot"; + if (args[1]?.includes("/comments?")) return JSON.stringify(comments); + if (args[1]?.endsWith("/reviews")) { + const payload = JSON.parse(String(opts?.input)) as { comments: InlineCommentInput[] }; + for (const input of payload.comments) comments.push({ id: comments.length + 1, path: input.path, side: input.side, + line: input.line, original_line: input.line, body: input.body, user: { login: "codegenie-bot" } }); + return "{}"; + } + throw new Error(`Unexpected GitHub transport request: ${args.join(" ")}`); + }; + return { comments, client: createGitHubClient("/synthetic", { runGh: gh }), advance: () => { headSha = "2".repeat(40); } }; +} diff --git a/src/github/duplicate-detector.ts b/src/github/duplicate-detector.ts index c6a6a11..2243b9d 100644 --- a/src/github/duplicate-detector.ts +++ b/src/github/duplicate-detector.ts @@ -3,13 +3,17 @@ import type { FinalFinding, FindingDuplicateDecision } from "../types.js"; +import { isProsePath } from "../util/path-roles.js"; +import { sha256Hex } from "../util/hashing.js"; +import { sanitizeGitHubCommentBody } from "./comment-sanitizer.js"; export const CODEGENIE_MARKER_PATTERN = - //u; + //u; export type CodegenieMarker = { fingerprint: string; runId: string; + contentFingerprint?: string; }; export function parseCodegenieMarker(body: string): CodegenieMarker | undefined { @@ -19,12 +23,19 @@ export function parseCodegenieMarker(body: string): CodegenieMarker | undefined } return { fingerprint: match[1] ?? "", - runId: match[2] ?? "" + runId: match[2] ?? "", + ...(match[3] ? { contentFingerprint: match[3] } : {}) }; } -export function formatCodegenieMarker(fingerprint: string, runId: string): string { - return ``; +export function formatCodegenieMarker(fingerprint: string, runId: string, contentFingerprint?: string): string { + return ``; +} + +/** Match unchanged published text, not model wording similarity or diff geometry. */ +export function proseContentFingerprint(body: string): string | undefined { + const content = sanitizeGitHubCommentBody(body).replace(/\r\n?/gu, "\n").trim(); + return content ? sha256Hex(content) : undefined; } export function detectDuplicateFindings( @@ -38,6 +49,21 @@ export function detectDuplicateFindings( .map((comment) => [comment.fingerprint as string, comment]) ); return findings.map((finding): FindingDuplicateDecision => { + if (isProsePath(finding.path)) { + const contentFingerprint = proseContentFingerprint(finding.finalBody); + const unchanged = finding.anchor !== undefined && contentFingerprint && codegenieComments.find(comment => + comment.path === finding.path && comment.side === finding.anchor?.side && + (comment.contentFingerprint ?? proseContentFingerprint(comment.body ?? "")) === contentFingerprint + ); + // Geometry-based fingerprints and proximity alone cannot distinguish prose defects. + // Legacy comments can match by their full body; missing or changed content stays eligible. + return unchanged ? { + findingId: finding.id, + action: "skip_unchanged_content", + matchedCommentId: unchanged.id, + reason: "unchanged codegenie comment content already exists on the same path and side" + } : { findingId: finding.id, action: "post", reason: "no unchanged codegenie prose comment found" }; + } const exact = fingerprints.get(finding.fingerprint); if (exact !== undefined) { return { diff --git a/src/github/github-client.ts b/src/github/github-client.ts index 6b72869..ed8f20f 100644 --- a/src/github/github-client.ts +++ b/src/github/github-client.ts @@ -217,6 +217,8 @@ export function createGitHubClient(repoRoot: string, opts: CreateGitHubClientOpt } if (marker !== undefined) { thread.fingerprint = marker.fingerprint; + if (marker.contentFingerprint !== undefined) thread.contentFingerprint = marker.contentFingerprint; + if (comment.body !== undefined) thread.body = comment.body; } own.push(thread); } diff --git a/src/github/publisher.ts b/src/github/publisher.ts index 3e7827f..6c35c05 100644 --- a/src/github/publisher.ts +++ b/src/github/publisher.ts @@ -17,7 +17,8 @@ import { inlineCode, severityBadge } from "../util/markdown.js"; import { codegenieVersionInfo, workflowRunUrl } from "../util/version-info.js"; import { sanitizeGitHubCommentBody } from "./comment-sanitizer.js"; import { createGitHubClient } from "./github-client.js"; -import { detectDuplicateFindings, formatCodegenieMarker } from "./duplicate-detector.js"; +import { detectDuplicateFindings, formatCodegenieMarker, proseContentFingerprint } from "./duplicate-detector.js"; +import { isProsePath } from "../util/path-roles.js"; type PublishOptions = { github?: GitHubClient; @@ -116,7 +117,9 @@ export async function maybePublishToGitHub( } const comments = await github.listOwnComments(resolved.pr.number); - const duplicateDecisions = detectDuplicateFindings(inlineCandidates.map((candidate) => candidate.finding), comments); + const duplicateDecisions = detectDuplicateFindings( + inlineCandidates.map(({ finding, anchor }) => ({ ...finding, anchor })), comments + ); const duplicateById = new Map(duplicateDecisions.map((decision) => [decision.findingId, decision])); const prepared = inlineCandidates .filter(({ finding }) => duplicateById.get(finding.id)?.action === "post") @@ -280,11 +283,13 @@ function prepareInlineComment( runId: string, deletedFileAnchor: boolean ): PreparedInlineComment { + const contentFingerprint = isProsePath(finding.path) ? proseContentFingerprint(finding.finalBody) : undefined; + const marker = formatCodegenieMarker(finding.fingerprint, runId, contentFingerprint); const input: InlineCommentInput = { path: anchor.path, line: anchor.line, side: anchor.side, - body: `${capBody(sanitizeGitHubCommentBody(finding.finalBody), INLINE_BODY_CAP)}\n\n${formatCodegenieMarker(finding.fingerprint, runId)}` + body: `${capBody(sanitizeGitHubCommentBody(finding.finalBody), INLINE_BODY_CAP)}\n\n${marker}` }; if (anchor.startLine !== undefined && anchor.startLine !== anchor.line) { input.start_line = anchor.startLine; diff --git a/src/types.ts b/src/types.ts index 5a73502..69da0d9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -171,6 +171,9 @@ export type ExistingReviewThread = { author: string; isCodegenie: boolean; fingerprint?: string; + /** Stable identity of the complete published prose, before the inline size cap. */ + contentFingerprint?: string; + body?: string; }; export interface GitHubClient { @@ -1553,7 +1556,7 @@ export type EvalCompareReport = { export type FindingDuplicateDecision = { findingId: string; - action: "post" | "skip_exact_fingerprint" | "skip_fuzzy_proximity"; + action: "post" | "skip_exact_fingerprint" | "skip_fuzzy_proximity" | "skip_unchanged_content"; matchedCommentId?: string; reason: string; }; diff --git a/src/util/path-roles.ts b/src/util/path-roles.ts index 9813619..19727ef 100644 --- a/src/util/path-roles.ts +++ b/src/util/path-roles.ts @@ -45,6 +45,11 @@ export function isDocsPath(filePath: string): boolean { return /(?:^|\/)(?:docs?|documentation|postmortems?)(?:\/|$)|\.(?:md|mdx|rst|txt)$/u.test(normalized); } +/** Prose extensions, not directory roles: executable examples under docs/ remain code. */ +export function isProsePath(filePath: string): boolean { + return /\.(?:md|mdx|rst|txt)$/u.test(normalizePathRoleInput(filePath)); +} + function normalizePathRoleInput(filePath: string): string { return filePath.toLowerCase().replace(/\\/gu, "/"); } diff --git a/tests/github-client.test.ts b/tests/github-client.test.ts index b13805b..2d376cc 100644 --- a/tests/github-client.test.ts +++ b/tests/github-client.test.ts @@ -91,7 +91,8 @@ describe("GitHub client", () => { side: "RIGHT", author: "codebot", isCodegenie: true, - fingerprint + fingerprint, + body: `` } ]); }); diff --git a/tests/github-duplicate-detector.test.ts b/tests/github-duplicate-detector.test.ts index 4737721..52a6bea 100644 --- a/tests/github-duplicate-detector.test.ts +++ b/tests/github-duplicate-detector.test.ts @@ -2,7 +2,8 @@ import { describe, expect, it } from "vitest"; import { detectDuplicateFindings, formatCodegenieMarker, - parseCodegenieMarker + parseCodegenieMarker, + proseContentFingerprint } from "../src/github/duplicate-detector.js"; import type { ExistingReviewThread, FinalFinding } from "../src/types.js"; @@ -15,6 +16,54 @@ describe("GitHub duplicate detector", () => { expect(parseCodegenieMarker("body only")).toBeUndefined(); }); + it("round trips content markers while retaining legacy marker support", () => { + const fingerprint = "a".repeat(64), contentFingerprint = "b".repeat(64); + expect(parseCodegenieMarker(formatCodegenieMarker(fingerprint, "run-1", contentFingerprint))) + .toEqual({ fingerprint, runId: "run-1", contentFingerprint }); + expect(proseContentFingerprint("\r\nSome unchanged advice.\r\n")).toBe(proseContentFingerprint("Some unchanged advice.")); + expect(proseContentFingerprint(" ")).toBeUndefined(); + }); + + it.each(["docs/design.md", "README.MD", "guide.mdx", "guide.rst", "notes.txt"])("matches legacy prose content across distant or different anchors in %s", path => { + const body = "The table lists widgetCreated, but EventKind does not define it."; + const f = { ...finding({ line: 74 }), path, finalBody: body, + anchor: { path, side: "RIGHT" as const, hunkId: "new-hunk", line: 74 } }; + expect(detectDuplicateFindings([f], [{ id: "previous", path: f.path, line: 81, side: "RIGHT", author: "bot", isCodegenie: true, + fingerprint: "b".repeat(64), body: `${body}\n\n${formatCodegenieMarker("b".repeat(64), "old-run")}` }])) + .toEqual([expect.objectContaining({ action: "skip_unchanged_content" })]); + }); + + it.each([20, 21, 180])("does not hide a different prose defect at line %i under an old fingerprint or proximity", line => { + const f = { ...finding({ line }), path: "docs/api.md", finalBody: "Retrying this request creates duplicate orders.", + anchor: { path: "docs/api.md", side: "RIGHT" as const, hunkId: "new", line } }; + expect(detectDuplicateFindings([f], [{ id: "old", path: f.path, line: 20, side: "RIGHT", author: "bot", isCodegenie: true, + fingerprint: f.fingerprint, body: "The enum value is not defined." }])[0]?.action).toBe("post"); + }); + + it("does not match absent content, other files, other sides, or foreign comments", () => { + const f = { ...finding(), path: "docs/api.md", finalBody: "An actionable issue.", + anchor: { path: "docs/api.md", side: "RIGHT" as const, hunkId: "h1", line: 1 } }; + const base = { id: "old", path: f.path, side: "RIGHT" as const, author: "bot", isCodegenie: true, + fingerprint: f.fingerprint, body: f.finalBody }; + for (const comment of [{ ...base, body: "" }, { ...base, path: "docs/other.md" }, + { ...base, side: "LEFT" as const }, { ...base, isCodegenie: false }]) { + expect(detectDuplicateFindings([f], [comment])[0]?.action).toBe("post"); + } + }); + + it("preserves code fingerprint behavior for executable examples under docs", () => { + const f = { ...finding(), path: "docs/examples/server.ts" }; + expect(detectDuplicateFindings([f], [{ id: "old", author: "bot", isCodegenie: true, fingerprint: f.fingerprint }])[0]?.action) + .toBe("skip_exact_fingerprint"); + }); + + it("does not treat two missing anchor sides as a proven prose match", () => { + const { anchor: _anchor, ...unanchored } = finding(); + const f = { ...unanchored, path: "docs/api.md" }; + expect(detectDuplicateFindings([f], [{ id: "old", path: f.path, author: "bot", isCodegenie: true, + body: f.finalBody }])[0]?.action).toBe("post"); + }); + it("skips exact fingerprint and fuzzy nearby codegenie comments only", () => { const exact = finding({ id: "exact", fingerprint: "b".repeat(64), line: 10 }); const nearby = finding({ id: "nearby", fingerprint: "c".repeat(64), line: 105 }); diff --git a/tests/pipeline-phase5.test.ts b/tests/pipeline-phase5.test.ts index 9dede83..3385b58 100644 --- a/tests/pipeline-phase5.test.ts +++ b/tests/pipeline-phase5.test.ts @@ -10438,11 +10438,11 @@ describe("phase 5 pipeline regressions", () => { expect(result.findings[0]?.mergedCandidateIds.sort()).toEqual(["finding-1", "finding-2", "finding-3"]); }); - it("uses the canonical normalized delimiter-based final fingerprint", async () => { + it.each(["APP.ts", "docs/examples/server.ts"])("uses the canonical normalized delimiter-based final fingerprint for %s", async filePath => { const finding: CandidateFinding = { ...fakeFinding(), - path: "APP.ts", - anchor: { path: "APP.ts", line: 1, side: "RIGHT", hunkId: "h1" }, + path: filePath, + anchor: { path: filePath, line: 1, side: "RIGHT", hunkId: "h1" }, producedBy: { ...fakeFinding().producedBy, lensId: "CORE/Code-Review" } }; const result = await dedupeRankAndComposeReview( @@ -10475,11 +10475,12 @@ describe("phase 5 pipeline regressions", () => { } }, promptBuilder: fakePromptBuilder(), - packets: [packetWithSymbol("packet-1", " Checkout Flow ")] + packets: [{ ...packetWithSymbol("packet-1", " Checkout Flow "), path: filePath, + symbolFacts: packetWithSymbol("packet-1", " Checkout Flow ").symbolFacts.map(fact => ({ ...fact, path: filePath })) }] } ); - const expected = sha256Hex(["app.ts", "checkout flow", "correctness", "core/code-review"].join("\0")); + const expected = sha256Hex([filePath.toLowerCase(), "checkout flow", "correctness", "core/code-review"].join("\0")); const [final] = [...result.findings, ...result.summaryOnlyFindings]; expect(final?.fingerprint).toBe(expected); }); @@ -10566,6 +10567,161 @@ describe("phase 5 pipeline regressions", () => { expect(secondFinal?.fingerprint).toBe(expected); }); + it("keeps docs comment identity stable when a later push shifts the hunk", async () => { + const docsPath = "docs/plans/service-rollout/design.md"; + const tableRow = "| `POST /widgets` | write | `widgetCreated` |"; + const docsPacket = (hunkId: string, line: number): ReviewPacket => ({ + ...fakePacket({ path: docsPath }), + id: `packet-${hunkId}`, + language: "markdown", + hunks: [ + { + hunkId, + oldStart: line, + oldLines: 1, + newStart: line, + newLines: 1, + contentWithLineNumbers: ` ${line} ${line} +${tableRow}`, + lines: [{ kind: "add", content: tableRow, newLine: line }], + changedNewLineNumbers: [line], + changedOldLineNumbers: [] + } + ] + }); + const docsFinding = (hunkId: string, line: number): CandidateFinding => ({ + ...fakeFinding(), + path: docsPath, + anchor: { path: docsPath, line, side: "RIGHT", hunkId }, + evidence: { changedCode: tableRow }, + producedBy: { ...fakeFinding().producedBy, packetId: `packet-${hunkId}` } + }); + const compose = async (hunkId: string, line: number) => dedupeRankAndComposeReview( + { verified: [docsFinding(hunkId, line)], verdicts: [] }, + fakePlanForHunks([hunkId], docsPath), + { + mode: "branch", + repoRoot: "/tmp/repo", + commits: [], + rawDiff: "" + }, + { + totalHunks: 1, + reviewedHunks: 1, + skippedHunks: 0, + failedHunks: 0, + coverageByLevel: { deep: 0, normal: 1, light: 0, skip: 0 }, + degradedPlanning: false, + budgetStopped: false, + verificationIncompleteCount: 0, + partial: false, + reasons: [] + }, + config(), + nullTelemetry(), + { + runner: { + runStructured: async () => { + throw composerTransientError(); + } + }, + promptBuilder: fakePromptBuilder(), + packets: [docsPacket(hunkId, line)], + diff: { files: [{ ...fakeDiffFile(docsPath), language: "markdown", hunks: [{ + ...fakeDiffFile(docsPath).hunks[0]!, id: hunkId, oldStart: line, newStart: line, + lines: [{ kind: "add", content: tableRow, newLineNumber: line }] + }] }] } + } + ); + + const beforePush = await compose("hunk-at-81", 81); + const afterPush = await compose("hunk-at-74", 74); + + const [before] = [...beforePush.findings, ...beforePush.summaryOnlyFindings]; + const [after] = [...afterPush.findings, ...afterPush.summaryOnlyFindings]; + expect(before).toBeDefined(); + expect(after).toBeDefined(); + expect(after!.anchor).toEqual(docsFinding("hunk-at-74", 74).anchor); + // Keep grouping identities distinct; publication identity survives shifted geometry. + expect(before!.fingerprint).not.toBe(after!.fingerprint); + const { detectDuplicateFindings, proseContentFingerprint } = await import("../src/github/duplicate-detector.js"); + expect(proseContentFingerprint(before!.finalBody)).toBe(proseContentFingerprint(after!.finalBody)); + expect(detectDuplicateFindings([after!], [{ id: "old-comment", path: docsPath, side: "RIGHT", line: 81, + author: "bot", isCodegenie: true, fingerprint: before!.fingerprint, body: before!.finalBody }])) + .toEqual([expect.objectContaining({ action: "skip_unchanged_content", matchedCommentId: "old-comment" })]); + }); + + it("keeps distinct same-category docs findings in one file separate", async () => { + const docsPath = "docs/design.md"; + const docsPacket: ReviewPacket = { + ...fakePacket({ path: docsPath }), + kind: "coalesced-hunks", + language: "markdown", + hunks: [ + { + hunkId: "h1", + oldStart: 1, + oldLines: 1, + newStart: 1, + newLines: 1, + contentWithLineNumbers: " 1 1 +alpha claim", + lines: [{ kind: "add", content: "alpha claim", newLine: 1 }], + changedNewLineNumbers: [1], + changedOldLineNumbers: [] + }, + { + hunkId: "h2", + oldStart: 20, + oldLines: 1, + newStart: 20, + newLines: 1, + contentWithLineNumbers: " 20 20 +beta claim", + lines: [{ kind: "add", content: "beta claim", newLine: 20 }], + changedNewLineNumbers: [20], + changedOldLineNumbers: [] + } + ] + }; + const { anchor: _anchor, ...base } = fakeFinding(); + const first = { ...base, path: docsPath, changedLine: false, evidence: { changedCode: "alpha claim" } }; + const second = { ...first, id: "finding-2", title: "second docs claim", evidence: { changedCode: "beta claim" } }; + const result = await dedupeRankAndComposeReview( + { verified: [first, second], verdicts: [] }, + fakePlanForHunks(["h1", "h2"], docsPath), + { + mode: "branch", + repoRoot: "/tmp/repo", + commits: [], + rawDiff: "" + }, + { + totalHunks: 2, + reviewedHunks: 2, + skippedHunks: 0, + failedHunks: 0, + coverageByLevel: { deep: 0, normal: 2, light: 0, skip: 0 }, + degradedPlanning: false, + budgetStopped: false, + verificationIncompleteCount: 0, + partial: false, + reasons: [] + }, + { ...config(), review: { ...config().review, maxFindings: 100, softCommentCap: 100 } }, + nullTelemetry(), + { + runner: { + runStructured: async () => { + throw composerTransientError(); + } + }, + promptBuilder: fakePromptBuilder(), + packets: [docsPacket] + } + ); + + expect(result.summaryOnlyFindings).toHaveLength(2); + expect(result.summaryOnlyFindings.map(finding => finding.mergedCandidateIds)).toEqual([["finding-1"], ["finding-2"]]); + }); + it("does not merge unanchored findings from different hunks in one coalesced packet", async () => { const coalescedPacket: ReviewPacket = { ...fakePacket(), diff --git a/tsconfig.json b/tsconfig.json index 8439cb9..c7d877e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -17,6 +17,6 @@ "noUncheckedIndexedAccess": true, "exactOptionalPropertyTypes": true }, - "include": ["src/**/*.ts", "tests/**/*.ts", "vitest.config.ts"], + "include": ["src/**/*.ts", "tests/**/*.ts", "evals/synthetics/**/*.ts", "vitest.config.ts"], "exclude": ["dist", "node_modules"] } diff --git a/vitest.config.ts b/vitest.config.ts index 556728c..4032d9a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { globals: true, - include: ["tests/**/*.test.ts"], + include: ["tests/**/*.test.ts", "evals/synthetics/**/*.test.ts"], restoreMocks: true, clearMocks: true }