diff --git a/src/clawsweeper.ts b/src/clawsweeper.ts index 582139c5a5..2a6b8a714d 100644 --- a/src/clawsweeper.ts +++ b/src/clawsweeper.ts @@ -54,6 +54,7 @@ import { } from "./github-retry.js"; import { parseGhJson, parseGhJsonLinesWithRetry, parseGhJsonWithRetry } from "./github-json.js"; import { stableJson } from "./stable-json.js"; +import { reviewPullChecksDigestParts } from "./review-checks-digest.js"; import { coverageTrackedItemIdsFromManifest } from "./review-coverage-manifest.js"; import { LEGACY_FIXED_CLOSE_SKIP_ACTIONS, @@ -3184,7 +3185,7 @@ function itemContentDigest(item: Item, context: ItemContext, git?: GitInfo): str ? (context.pullReviewCommentsRevision ?? reviewCommentDigestParts(context.pullReviewComments)) : null, - checks: isPull ? (context.pullChecks ?? null) : null, + checks: isPull ? reviewPullChecksDigestParts(context.pullChecks ?? null) : null, }), ); } @@ -9165,7 +9166,7 @@ function fetchReviewStructuralRecord(options: { if (!headSha) return null; const pullChecks = pullChecksContext(options.item.number, headSha); if (!completePullChecksContext(pullChecks)) return null; - pullChecksDigest = sha256(stableJson(pullChecks)); + pullChecksDigest = sha256(stableJson(reviewPullChecksDigestParts(pullChecks))); } return reviewStructuralRecordFromGraphql({ response, diff --git a/src/review-checks-digest.ts b/src/review-checks-digest.ts new file mode 100644 index 0000000000..762c50dea7 --- /dev/null +++ b/src/review-checks-digest.ts @@ -0,0 +1,17 @@ +import { stableJsonCodeUnit } from "./stable-json.js"; + +export function reviewPullChecksDigestParts(value: unknown): unknown { + if (!value || typeof value !== "object" || Array.isArray(value)) return value; + const checks = value as Record; + if (!Array.isArray(checks.checkRuns)) return value; + + const seen = new Set(); + const checkRuns = checks.checkRuns.filter((checkRun) => { + const identity = stableJsonCodeUnit(checkRun); + if (seen.has(identity)) return false; + seen.add(identity); + return true; + }); + + return checkRuns.length === checks.checkRuns.length ? value : { ...checks, checkRuns }; +} diff --git a/src/review-semantic-cache.ts b/src/review-semantic-cache.ts index dfc1d14614..3de09f1eec 100644 --- a/src/review-semantic-cache.ts +++ b/src/review-semantic-cache.ts @@ -6,6 +6,7 @@ import { createVirtualFileSystem, type FileSystem } from "typescript/unstable/fs import { API } from "typescript/unstable/sync"; import { REVIEW_CACHE_MAX_AGE_DAYS } from "./scheduler-policy.js"; +import { reviewPullChecksDigestParts } from "./review-checks-digest.js"; import { stableJsonCodeUnit as stableJson } from "./stable-json.js"; export const REVIEW_SEMANTIC_CACHE_VERSION = 10; @@ -782,7 +783,7 @@ function semanticContext(input: ReviewSemanticInput): { reviewComments: input.context.pullReviewCommentsRevision ?? normalizedComments(input.context.pullReviewComments), - checks: input.context.pullChecks ?? null, + checks: reviewPullChecksDigestParts(input.context.pullChecks ?? null), completeness: { commentsTruncated: input.context.counts?.commentsTruncated ?? null, timelineTruncated: input.context.counts?.timelineTruncated ?? null, diff --git a/test/review-comment-rendering.test.ts b/test/review-comment-rendering.test.ts index 503623e689..8dc2156609 100644 --- a/test/review-comment-rendering.test.ts +++ b/test/review-comment-rendering.test.ts @@ -166,7 +166,10 @@ test("structural cache probes before hydration but acquires a lease before carry source.indexOf("function collectItemContext"), ); assert.match(structuralProbeSource, /pullChecksContext\(options\.item\.number, headSha\)/); - assert.match(structuralProbeSource, /pullChecksDigest = sha256\(stableJson\(pullChecks\)\)/); + assert.match( + structuralProbeSource, + /pullChecksDigest = sha256\(stableJson\(reviewPullChecksDigestParts\(pullChecks\)\)\)/, + ); assert.match(structuralProbeSource, /if \(!options\.git\.releaseStateComplete\) return null/); const gitInfoBlock = source.slice( source.indexOf("function gitInfo("), diff --git a/test/review-content-cache.test.ts b/test/review-content-cache.test.ts index d4750439b5..c75169a6b5 100644 --- a/test/review-content-cache.test.ts +++ b/test/review-content-cache.test.ts @@ -294,6 +294,101 @@ test("content digest busts when bounded PR check state changes", () => { assert.notEqual(passing, failing); }); +test("content digest ignores duplicate compacted PR check runs", () => { + const pull = item({ kind: "pull_request", number: 200 }); + const notify = { + name: "notify", + status: "completed", + conclusion: "success", + app: "github-actions", + }; + const once = itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns: [notify], + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + const repeated = itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns: [notify, notify], + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + + assert.equal(once, repeated); +}); + +test("content digest keeps check runs that differ only in name or app", () => { + const pull = item({ kind: "pull_request", number: 200 }); + const check = { + name: "smoke", + status: "completed", + conclusion: "success", + app: "github-actions", + }; + const digest = (checkRuns) => + itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns, + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + + const alone = digest([check]); + const byName = digest([check, { ...check, name: "smoke (windows)" }]); + const byApp = digest([check, { ...check, app: "blacksmith-sh" }]); + + assert.notEqual(alone, byName); + assert.notEqual(alone, byApp); + assert.notEqual(byName, byApp); +}); + +test("content digest busts when one of several repeated check runs newly fails", () => { + const pull = item({ kind: "pull_request", number: 200 }); + const notify = { + name: "notify", + status: "completed", + conclusion: "success", + app: "github-actions", + }; + const digest = (checkRuns) => + itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns, + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + + const allPassing = digest([notify, notify, notify]); + const oneFailing = digest([notify, notify, { ...notify, conclusion: "failure" }]); + + assert.notEqual(allPassing, oneFailing); +}); + test("review comment revision covers comments outside the bounded prompt window", () => { const comments = Array.from({ length: 81 }, (_, index) => ({ id: index + 1, @@ -353,6 +448,48 @@ test("cache hits when content is unchanged, fresh, complete, and policy matches" assert.equal(cacheHit(), true); }); +test("cache hits after an equivalent check run is repeated on an unchanged head", () => { + const pull = item({ kind: "pull_request", number: 200 }); + const notify = { + name: "notify", + status: "completed", + conclusion: "success", + app: "github-actions", + }; + const priorDigest = itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns: [notify], + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + const currentDigest = itemContentDigestForTest( + pull, + pullContext({ + pullChecks: { + complete: true, + checkRuns: [notify, notify], + checkRunsTruncated: false, + statuses: [], + statusesTruncated: false, + }, + }), + ); + + assert.equal( + cacheHit({ + review: freshReview({ contentDigest: priorDigest }), + contentDigest: currentDigest, + }), + true, + ); +}); + test("cache misses when the content digest differs", () => { assert.equal(cacheHit({ contentDigest: "digest-2" }), false); }); diff --git a/test/review-semantic-cache.test.ts b/test/review-semantic-cache.test.ts index 13486796d5..a3bbdac613 100644 --- a/test/review-semantic-cache.test.ts +++ b/test/review-semantic-cache.test.ts @@ -1079,6 +1079,22 @@ test("changed discussion, reviews, checks, or target context busts the context d } }); +test("duplicate compacted check runs do not perturb the semantic context digest", () => { + const prior = record(); + const checkRuns = input().context.pullChecks.checkRuns; + const repeated = record({ + context: { + pullChecks: { + ...input().context.pullChecks, + checkRuns: [...checkRuns, ...checkRuns], + }, + }, + }); + + assert.equal(prior.contextDigest, repeated.contextDigest); + assert.equal(decision({ priorRecord: prior, currentRecord: repeated }).reason, "hit"); +}); + test("head SHA churn alone does not perturb semantic or context digests", () => { const prior = record(); const rebased = record({