Skip to content

Commit 68d9f6a

Browse files
authored
feat(signals): flag open-vs-open PR file-path collisions (#2653) (#2654)
Open PRs carried no changedFiles, so buildCollisionReport's existing termOverlap scoring only ever saw title/label text for open work, silently missing two independently-open PRs touching the same file. Populate changedFiles on PullRequestRecord from the pull_request_files cache (flag-gated GITTENSORY_OPEN_PR_FILE_COLLISION, default off) and add a same-author guard so a contributor's own follow-up PR sharing a file with their prior PR is never flagged as a collision.
1 parent dfba160 commit 68d9f6a

8 files changed

Lines changed: 435 additions & 10 deletions

File tree

src/env.d.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,13 @@ declare global {
310310
* unchanged). Once a winner closes, the next-lowest OPEN sibling becomes the winner on re-eval. See
311311
* src/signals/duplicate-winner.ts. */
312312
GITTENSORY_DUPLICATE_WINNER?: string;
313+
/** Open-PR file-path collision (#2653): when truthy, a live PR review enriches its own and its open
314+
* siblings' `changedFiles` from the `pull_request_files` cache (a plain D1 read — no extra GitHub calls)
315+
* before building the collision report, so two independently-open PRs touching the same file are flagged
316+
* the same way two title-similar PRs already are. A contributor's own two PRs sharing a file are never
317+
* flagged (see the same-author guard in buildCollisionReport). Default OFF — unset/false leaves every
318+
* PullRequestRecord's changedFiles unset, byte-identical to today. See src/signals/engine.ts prItem. */
319+
GITTENSORY_OPEN_PR_FILE_COLLISION?: string;
313320
}
314321
}
315322

src/queue/processors.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5805,6 +5805,33 @@ export function reputationOutcomeFromTerminalState(
58055805
return undefined;
58065806
}
58075807

5808+
/**
5809+
* Open-PR file-path collision (#2653): enrich `changedFiles` on the reviewed PR and its open siblings from the
5810+
* `pull_request_files` cache, so `buildCollisionReport`'s existing termOverlap heuristic (which already tokenizes
5811+
* `changedFiles` for merged PRs, see recentMergedItem) gets real path signal for open-vs-open pairs too — not
5812+
* just title/label/linked-issue text. A single bounded D1 read (no GitHub API calls): siblings are populated by
5813+
* the routine detail-sync backfill independent of this flag, so this is a cache read, not a live fetch. Only
5814+
* `PullRequestRecord`s already carrying no `changedFiles` are overwritten; entries missing from the cache (e.g. a
5815+
* brand-new PR reviewed before its first detail-sync) are left as-is and simply carry no path signal this pass —
5816+
* a fail-safe degrade, not an error, and the next scheduled re-gate sweep picks it up once synced.
5817+
*/
5818+
export async function enrichOpenPullRequestsWithChangedFiles(env: Env, repoFullName: string, pullRequests: PullRequestRecord[]): Promise<PullRequestRecord[]> {
5819+
const openPullNumbers = pullRequests.filter((candidate) => candidate.state === "open").map((candidate) => candidate.number);
5820+
if (openPullNumbers.length === 0) return pullRequests;
5821+
const filePaths = await listRepoPullRequestFilePaths(env, repoFullName, { pullNumbers: openPullNumbers });
5822+
if (filePaths.length === 0) return pullRequests;
5823+
const pathsByPullNumber = new Map<number, string[]>();
5824+
for (const row of filePaths) {
5825+
const paths = pathsByPullNumber.get(row.pullNumber) ?? [];
5826+
paths.push(row.path);
5827+
pathsByPullNumber.set(row.pullNumber, paths);
5828+
}
5829+
return pullRequests.map((candidate) => {
5830+
const paths = pathsByPullNumber.get(candidate.number);
5831+
return paths ? { ...candidate, changedFiles: paths } : candidate;
5832+
});
5833+
}
5834+
58085835
async function maybePublishPrPublicSurface(
58095836
env: Env,
58105837
installationId: number,
@@ -6190,15 +6217,22 @@ async function maybePublishPrPublicSurface(
61906217
listPullRequests(env, repoFullName),
61916218
listBountiesByRepo(env, repoFullName),
61926219
]);
6220+
// Open-PR file-path collision (#2653): flag-gated, byte-identical when OFF (see enrichOpenPullRequestsWithChangedFiles).
6221+
// Scoped to collision/preflight/queue-health inputs only — every OTHER use of repoPullRequests below (e.g. the
6222+
// duplicate-winner adjudication, which is same-linked-issue-based, not path-based) keeps reading the un-enriched array.
6223+
const collisionPullRequests =
6224+
env.GITTENSORY_OPEN_PR_FILE_COLLISION === "true"
6225+
? await enrichOpenPullRequestsWithChangedFiles(env, repoFullName, repoPullRequests)
6226+
: repoPullRequests;
61936227
collisions = buildCollisionReport(
61946228
repoFullName,
61956229
repoIssues,
6196-
repoPullRequests,
6230+
collisionPullRequests,
61976231
);
61986232
queueHealth = buildQueueHealth(
61996233
repo,
62006234
repoIssues,
6201-
repoPullRequests,
6235+
collisionPullRequests,
62026236
collisions,
62036237
);
62046238
preflight = buildPreflightResult(
@@ -6213,7 +6247,7 @@ async function maybePublishPrPublicSurface(
62136247
},
62146248
repo,
62156249
repoIssues,
6216-
repoPullRequests,
6250+
collisionPullRequests,
62176251
repoBounties,
62186252
);
62196253
// Duplicate-winner adjudication (#dup-winner): compute the winner ONCE for this review run from the SAME

src/signals/engine.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,17 @@ export function buildCollisionReport(
862862
}
863863
const overlap = termOverlap(itemTerms.get(itemKey(left)) ?? collisionTerms(left), itemTerms.get(itemKey(right)) ?? collisionTerms(right));
864864
if (overlap.score < 0.58 || overlap.shared < 2) continue;
865+
// A contributor iterating on their own work (e.g. a follow-up PR touching the same file as their still-open
866+
// prior PR) is not duplicate effort. Title/label overlap between a contributor's own items is today's
867+
// established behavior (unchanged, e.g. a self-filed issue and its own PR); what's new here is that
868+
// `changedFiles` now also feeds this same heuristic, and two of a contributor's own PRs sharing a file is
869+
// exactly the false-positive path-overlap creates. Re-score without paths: if the pair only clears the bar
870+
// WITH file-path terms, paths alone drove the match — self-authored, so skip it. If title/label terms alone
871+
// already clear the bar, this is pre-existing behavior and still clusters.
872+
if (isPullRequestShapedItem(left) && isPullRequestShapedItem(right) && Boolean(left.authorLogin) && sameLogin(left.authorLogin, right.authorLogin ?? "")) {
873+
const titleOnlyOverlap = termOverlap(collisionTerms(left, false), collisionTerms(right, false));
874+
if (titleOnlyOverlap.score < 0.58 || titleOnlyOverlap.shared < 2) continue;
875+
}
865876
const key = [itemKey(left), itemKey(right)].sort().join("--");
866877
if (clusters.has(key)) continue;
867878
clusters.set(key, {
@@ -5156,6 +5167,7 @@ function prItem(pr: PullRequestRecord): CollisionItem {
51565167
labels: pr.labels,
51575168
linkedIssues: pr.linkedIssues,
51585169
linkedIssueClaimedAt: pr.linkedIssueClaimedAt,
5170+
changedFiles: pr.changedFiles,
51595171
body: pr.body,
51605172
};
51615173
}
@@ -5217,8 +5229,8 @@ type CollisionTerms = {
52175229

52185230
const collisionReportTermCache = new WeakMap<CollisionReport, Map<string, CollisionTerms>>();
52195231

5220-
function collisionTerms(item: CollisionItem): CollisionTerms {
5221-
const terms = new Set(tokenize(collisionItemText(item)));
5232+
function collisionTerms(item: CollisionItem, includePaths = true): CollisionTerms {
5233+
const terms = new Set(tokenize(collisionItemText(item, includePaths)));
52225234
return { terms, size: terms.size };
52235235
}
52245236

@@ -5251,11 +5263,11 @@ function termOverlap(left: CollisionTerms, right: CollisionTerms): { score: numb
52515263
return { score: shared / Math.min(left.size, right.size), shared };
52525264
}
52535265

5254-
function collisionItemText(item: CollisionItem): string {
5266+
function collisionItemText(item: CollisionItem, includePaths = true): string {
52555267
return [
52565268
truncateText(item.title, PREFLIGHT_LIMITS.titleChars),
52575269
...boundedTextItems(item.labels, PREFLIGHT_LIMITS.labels, PREFLIGHT_LIMITS.labelChars),
5258-
...boundedTextItems(item.changedFiles, PREFLIGHT_LIMITS.changedFiles, PREFLIGHT_LIMITS.changedFileChars),
5270+
...(includePaths ? boundedTextItems(item.changedFiles, PREFLIGHT_LIMITS.changedFiles, PREFLIGHT_LIMITS.changedFileChars) : []),
52595271
]
52605272
.filter(Boolean)
52615273
.join(" ");
@@ -5390,6 +5402,10 @@ function sameLogin(value: string | null | undefined, login: string): boolean {
53905402
return value?.toLowerCase() === login.toLowerCase();
53915403
}
53925404

5405+
function isPullRequestShapedItem(item: CollisionItem): boolean {
5406+
return item.type === "pull_request" || item.type === "recent_merged_pull_request";
5407+
}
5408+
53935409
function sameRepo(left: string | null | undefined, right: string | null | undefined): boolean {
53945410
return Boolean(left && right && left.toLowerCase() === right.toLowerCase());
53955411
}

src/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,11 @@ export type PullRequestRecord = {
470470
* stale-surface diagnostics, not as a hard re-review skip: GitHub comments/checks can still be stale or partial
471471
* while this marker matches headSha. Publish-written; read straight from the row. */
472472
lastPublishedSurfaceSha?: string | null | undefined;
473+
/** File paths changed by this open PR, when the caller has already resolved them (e.g. from the
474+
* `pull_request_files` cache). Absent/undefined when not resolved — callers must not assume an empty array
475+
* means "no files changed". Mirrors {@link RecentMergedPullRequestRecord.changedFiles} so the same
476+
* collision/preflight path-overlap scoring works for open PRs, not just merged history. */
477+
changedFiles?: string[] | undefined;
473478
};
474479

475480
export type IssueRecord = {

0 commit comments

Comments
 (0)