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
37 changes: 27 additions & 10 deletions src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { PREFLIGHT_LIMITS } from "./preflight-limits";
import type { UnifiedCollapsible } from "../review/unified-comment";
import { splitAiReviewNits } from "../review/ai-notes";
import { GITTENSORY_GATE_CHECK_NAME } from "../review/check-names";
import { diffFilePriority } from "../review/review-diff";

export type ParticipationLane = "direct_pr" | "issue_discovery" | "split" | "inactive" | "unknown";
export type SignalFinding = AdvisoryFinding;
Expand Down Expand Up @@ -862,16 +863,23 @@ 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;
// Re-score without path terms: tells us whether title/label overlap ALONE already clears the bar
// (pre-existing behavior, unaffected) or whether changedFiles tokens are what pushed this pair over —
// the two false-positive shapes that creates are guarded separately below.
const titleOnlyOverlap = termOverlap(collisionTerms(left, false), collisionTerms(right, false));
const pathDrivenMatch = titleOnlyOverlap.score < 0.58 || titleOnlyOverlap.shared < 2;
if (pathDrivenMatch) {
// 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 — self-authored path-only overlap is dropped outright.
if (isPullRequestShapedItem(left) && isPullRequestShapedItem(right) && Boolean(left.authorLogin) && sameLogin(left.authorLogin, right.authorLogin ?? "")) {
continue;
}
// Different authors: file paths tokenize into directory segments (src, review, test, unit, ...) that
// recur across nearly every PR in a consistently-organized repo, so shared TOKENS alone are not
// reliable collision evidence — a repo-wide shadow test found this drove the large majority of
// path-only matches with zero actual shared files. Require an ACTUAL shared file (ignoring
// lockfiles/generated artifacts nobody would call a collision over) before clustering.
if (!sharesMeaningfulFile(left.changedFiles, right.changedFiles)) continue;
}
const key = [itemKey(left), itemKey(right)].sort().join("--");
if (clusters.has(key)) continue;
Expand Down Expand Up @@ -5406,6 +5414,15 @@ function isPullRequestShapedItem(item: CollisionItem): boolean {
return item.type === "pull_request" || item.type === "recent_merged_pull_request";
}

/** True when two changed-file lists share at least one path that isn't a lockfile/generated/vendor artifact
* (diffFilePriority's least-useful-to-review bucket) — a shared package-lock.json or dist/ output is touched
* incidentally by unrelated PRs and is not evidence of a real collision. */
function sharesMeaningfulFile(left: string[] | undefined, right: string[] | undefined): boolean {
if (!left || !right || left.length === 0 || right.length === 0) return false;
const rightSet = new Set(right);
return left.some((path) => rightSet.has(path) && diffFilePriority(path) < 4);
}

function sameRepo(left: string | null | undefined, right: string | null | undefined): boolean {
return Boolean(left && right && left.toLowerCase() === right.toLowerCase());
}
Expand Down
84 changes: 84 additions & 0 deletions test/unit/signals-v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,90 @@ describe("v2 signal builders", () => {
expect(cluster).toBeDefined();
});

it("does not flag two open PRs from different authors whose only overlap is generic directory-segment tokens, not a real shared file", () => {
// Both touch src/review/*.ts + test/unit/*.ts, so "src"/"review"/"test"/"unit" tokenize as shared terms
// even though the two PRs share zero actual file paths — a repo-wide shadow test found this drove most
// path-only false positives once changedFiles fed the same bag-of-words scorer as titles/labels.
const ivanPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 223,
title: "CLA check",
state: "open",
authorLogin: "ivan",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["src/review/cla-check.ts", "test/unit/cla-check.test.ts"],
};
const judyPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 224,
title: "RAG builder",
state: "open",
authorLogin: "judy",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["src/review/rag.ts", "test/unit/rag.test.ts"],
};
const report = buildCollisionReport(repo.fullName, [], [ivanPr, judyPr]);
expect(findCluster(report, 223, 224)).toBeUndefined();
});

it("does not flag two open PRs whose only shared file is a lockfile (touched incidentally, not real overlap)", () => {
const kenPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 225,
title: "Rotate secrets",
state: "open",
authorLogin: "ken",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["package-lock.json"],
};
const lisaPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 226,
title: "Trim caches",
state: "open",
authorLogin: "lisa",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["package-lock.json"],
};
const report = buildCollisionReport(repo.fullName, [], [kenPr, lisaPr]);
expect(findCluster(report, 225, 226)).toBeUndefined();
});

it("flags two open PRs from different authors once a real (non-lockfile) shared file joins otherwise-generic path tokens", () => {
const ivanPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 227,
title: "CLA check",
state: "open",
authorLogin: "ivan",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["src/review/cla-check.ts", "test/unit/cla-check.test.ts", "src/shared/helper.ts"],
};
const judyPr: PullRequestRecord = {
repoFullName: repo.fullName,
number: 228,
title: "RAG builder",
state: "open",
authorLogin: "judy",
authorAssociation: "NONE",
labels: [],
linkedIssues: [],
changedFiles: ["src/review/rag.ts", "test/unit/rag.test.ts", "src/shared/helper.ts"],
};
const report = buildCollisionReport(repo.fullName, [], [ivanPr, judyPr]);
expect(findCluster(report, 227, 228)).toBeDefined();
});

it("flags an open PR against a recently-merged PR from a different author sharing a file (extends to merged history)", () => {
const openPr: PullRequestRecord = {
repoFullName: repo.fullName,
Expand Down
4 changes: 2 additions & 2 deletions worker-configuration.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable */
// Generated by Wrangler by running `wrangler types` (hash: ed445a7a260a431e4c5988e63c82a35a)
// Generated by Wrangler by running `wrangler types` (hash: 11efd919605a2b914db02ea113814228)
// Runtime types generated with workerd@1.20260617.1 2026-05-28 nodejs_compat
interface __BaseEnv_Env {
DB: D1Database;
Expand Down Expand Up @@ -35,7 +35,7 @@ interface __BaseEnv_Env {
GITTENSORY_PUBLIC_STATS: "true";
GITTENSORY_PUBLIC_STATS_REPOS: "JSONbored/gittensory,JSONbored/awesome-claude,JSONbored/metagraphed";
GITTENSORY_DUPLICATE_WINNER: "true";
GITTENSORY_OPEN_PR_FILE_COLLISION: "false";
GITTENSORY_OPEN_PR_FILE_COLLISION: "true";
RATE_LIMITER: DurableObjectNamespace<import("./src/index").RateLimiter>;
}
declare namespace Cloudflare {
Expand Down
10 changes: 7 additions & 3 deletions wrangler.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@
"GITTENSORY_DUPLICATE_WINNER": "true",
// Open-PR file-path collision (#2653): enrich changedFiles on the reviewed PR and its open siblings from
// the pull_request_files cache before building the collision report, so two independently-open PRs on the
// same file get flagged the way two title-similar PRs already are. Default OFF — unset/false leaves every
// PullRequestRecord's changedFiles unset (byte-identical to today, no extra D1 reads).
"GITTENSORY_OPEN_PR_FILE_COLLISION": "false",
// same file get flagged the way two title-similar PRs already are. ENABLED: a repo-wide shadow test against
// the live open-PR queue found path-only term overlap alone drove 12 of 17 newly-flagged pairs to false
// positives (shared generic directory-segment tokens like src/review/test/unit, zero actual shared files) —
// fixed by requiring an actual shared, non-lockfile file path before a path-driven match clusters (see the
// sharesMeaningfulFile guard in buildCollisionReport). Re-validated post-fix: every remaining flagged pair is
// backed by a real shared file.
"GITTENSORY_OPEN_PR_FILE_COLLISION": "true",
},
"routes": [
{
Expand Down
Loading