Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ declare global {
* unchanged). Once a winner closes, the next-lowest OPEN sibling becomes the winner on re-eval. See
* src/signals/duplicate-winner.ts. */
GITTENSORY_DUPLICATE_WINNER?: string;
/** Open-PR file-path collision (#2653): when truthy, a live PR review enriches its own and its open
* siblings' `changedFiles` from the `pull_request_files` cache (a plain D1 read — no extra GitHub calls)
* before building the collision report, so two independently-open PRs touching the same file are flagged
* the same way two title-similar PRs already are. A contributor's own two PRs sharing a file are never
* flagged (see the same-author guard in buildCollisionReport). Default OFF — unset/false leaves every
* PullRequestRecord's changedFiles unset, byte-identical to today. See src/signals/engine.ts prItem. */
GITTENSORY_OPEN_PR_FILE_COLLISION?: string;
}
}

Expand Down
40 changes: 37 additions & 3 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5795,6 +5795,33 @@ export function reputationOutcomeFromTerminalState(
return undefined;
}

/**
* Open-PR file-path collision (#2653): enrich `changedFiles` on the reviewed PR and its open siblings from the
* `pull_request_files` cache, so `buildCollisionReport`'s existing termOverlap heuristic (which already tokenizes
* `changedFiles` for merged PRs, see recentMergedItem) gets real path signal for open-vs-open pairs too — not
* just title/label/linked-issue text. A single bounded D1 read (no GitHub API calls): siblings are populated by
* the routine detail-sync backfill independent of this flag, so this is a cache read, not a live fetch. Only
* `PullRequestRecord`s already carrying no `changedFiles` are overwritten; entries missing from the cache (e.g. a
* brand-new PR reviewed before its first detail-sync) are left as-is and simply carry no path signal this pass —
* a fail-safe degrade, not an error, and the next scheduled re-gate sweep picks it up once synced.
*/
export async function enrichOpenPullRequestsWithChangedFiles(env: Env, repoFullName: string, pullRequests: PullRequestRecord[]): Promise<PullRequestRecord[]> {
const openPullNumbers = pullRequests.filter((candidate) => candidate.state === "open").map((candidate) => candidate.number);
if (openPullNumbers.length === 0) return pullRequests;
const filePaths = await listRepoPullRequestFilePaths(env, repoFullName, { pullNumbers: openPullNumbers });
if (filePaths.length === 0) return pullRequests;
const pathsByPullNumber = new Map<number, string[]>();
for (const row of filePaths) {
const paths = pathsByPullNumber.get(row.pullNumber) ?? [];
paths.push(row.path);
pathsByPullNumber.set(row.pullNumber, paths);
}
return pullRequests.map((candidate) => {
const paths = pathsByPullNumber.get(candidate.number);
return paths ? { ...candidate, changedFiles: paths } : candidate;
});
}

async function maybePublishPrPublicSurface(
env: Env,
installationId: number,
Expand Down Expand Up @@ -6180,15 +6207,22 @@ async function maybePublishPrPublicSurface(
listPullRequests(env, repoFullName),
listBountiesByRepo(env, repoFullName),
]);
// Open-PR file-path collision (#2653): flag-gated, byte-identical when OFF (see enrichOpenPullRequestsWithChangedFiles).
// Scoped to collision/preflight/queue-health inputs only — every OTHER use of repoPullRequests below (e.g. the
// duplicate-winner adjudication, which is same-linked-issue-based, not path-based) keeps reading the un-enriched array.
const collisionPullRequests =
env.GITTENSORY_OPEN_PR_FILE_COLLISION === "true"
? await enrichOpenPullRequestsWithChangedFiles(env, repoFullName, repoPullRequests)
: repoPullRequests;
collisions = buildCollisionReport(
repoFullName,
repoIssues,
repoPullRequests,
collisionPullRequests,
);
queueHealth = buildQueueHealth(
repo,
repoIssues,
repoPullRequests,
collisionPullRequests,
collisions,
);
preflight = buildPreflightResult(
Expand All @@ -6203,7 +6237,7 @@ async function maybePublishPrPublicSurface(
},
repo,
repoIssues,
repoPullRequests,
collisionPullRequests,
repoBounties,
);
// Duplicate-winner adjudication (#dup-winner): compute the winner ONCE for this review run from the SAME
Expand Down
24 changes: 20 additions & 4 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,17 @@ export function buildCollisionReport(
}
const overlap = termOverlap(itemTerms.get(itemKey(left)) ?? collisionTerms(left), itemTerms.get(itemKey(right)) ?? collisionTerms(right));
if (overlap.score < 0.58 || overlap.shared < 2) continue;
// A contributor iterating on their own work (e.g. a follow-up PR touching the same file as their still-open
// prior PR) is not duplicate effort. Title/label overlap between a contributor's own items is today's
// established behavior (unchanged, e.g. a self-filed issue and its own PR); what's new here is that
// `changedFiles` now also feeds this same heuristic, and two of a contributor's own PRs sharing a file is
// exactly the false-positive path-overlap creates. Re-score without paths: if the pair only clears the bar
// WITH file-path terms, paths alone drove the match — self-authored, so skip it. If title/label terms alone
// already clear the bar, this is pre-existing behavior and still clusters.
if (isPullRequestShapedItem(left) && isPullRequestShapedItem(right) && Boolean(left.authorLogin) && sameLogin(left.authorLogin, right.authorLogin ?? "")) {
const titleOnlyOverlap = termOverlap(collisionTerms(left, false), collisionTerms(right, false));
if (titleOnlyOverlap.score < 0.58 || titleOnlyOverlap.shared < 2) continue;
}
const key = [itemKey(left), itemKey(right)].sort().join("--");
if (clusters.has(key)) continue;
clusters.set(key, {
Expand Down Expand Up @@ -5156,6 +5167,7 @@ function prItem(pr: PullRequestRecord): CollisionItem {
labels: pr.labels,
linkedIssues: pr.linkedIssues,
linkedIssueClaimedAt: pr.linkedIssueClaimedAt,
changedFiles: pr.changedFiles,
body: pr.body,
};
}
Expand Down Expand Up @@ -5217,8 +5229,8 @@ type CollisionTerms = {

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

function collisionTerms(item: CollisionItem): CollisionTerms {
const terms = new Set(tokenize(collisionItemText(item)));
function collisionTerms(item: CollisionItem, includePaths = true): CollisionTerms {
const terms = new Set(tokenize(collisionItemText(item, includePaths)));
return { terms, size: terms.size };
}

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

function collisionItemText(item: CollisionItem): string {
function collisionItemText(item: CollisionItem, includePaths = true): string {
return [
truncateText(item.title, PREFLIGHT_LIMITS.titleChars),
...boundedTextItems(item.labels, PREFLIGHT_LIMITS.labels, PREFLIGHT_LIMITS.labelChars),
...boundedTextItems(item.changedFiles, PREFLIGHT_LIMITS.changedFiles, PREFLIGHT_LIMITS.changedFileChars),
...(includePaths ? boundedTextItems(item.changedFiles, PREFLIGHT_LIMITS.changedFiles, PREFLIGHT_LIMITS.changedFileChars) : []),
]
.filter(Boolean)
.join(" ");
Expand Down Expand Up @@ -5390,6 +5402,10 @@ function sameLogin(value: string | null | undefined, login: string): boolean {
return value?.toLowerCase() === login.toLowerCase();
}

function isPullRequestShapedItem(item: CollisionItem): boolean {
return item.type === "pull_request" || item.type === "recent_merged_pull_request";
}

function sameRepo(left: string | null | undefined, right: string | null | undefined): boolean {
return Boolean(left && right && left.toLowerCase() === right.toLowerCase());
}
Expand Down
5 changes: 5 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,11 @@ export type PullRequestRecord = {
* stale-surface diagnostics, not as a hard re-review skip: GitHub comments/checks can still be stale or partial
* while this marker matches headSha. Publish-written; read straight from the row. */
lastPublishedSurfaceSha?: string | null | undefined;
/** File paths changed by this open PR, when the caller has already resolved them (e.g. from the
* `pull_request_files` cache). Absent/undefined when not resolved — callers must not assume an empty array
* means "no files changed". Mirrors {@link RecentMergedPullRequestRecord.changedFiles} so the same
* collision/preflight path-overlap scoring works for open PRs, not just merged history. */
changedFiles?: string[] | undefined;
};

export type IssueRecord = {
Expand Down
Loading
Loading