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
2 changes: 1 addition & 1 deletion ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ server/ # webhook server (GitHub App)
- **incremental processing**: only re-embeds new/changed items. crash-recoverable via sqlite
- **read-only default**: every GitHub mutation (labels, comments, closes, issue creation) funnels through one `write-gate.ts` gate that defaults to dry-run. CLI writes only under `--apply-labels`; the webhook server writes only when `PRISM_APPLY=1`. `--dry-run` always wins. (Fixes the prior leak where `ensureLabelsExist` created labels even under `--dry-run`, and the server writing unconditionally.)
- **cross-repo**: config accepts multiple repos, dupe detection works across repo boundaries
- **canonical selection**: one `selectCanonical()` (src/canonical.ts) picks each cluster's source of truth for the report, the starmap payload, and the live triage bot alike. issue-majority clusters resolve to the earliest report (the original bug); PR-majority to the highest-quality item. fully deterministic - every tie bottoms out at item number - so re-runs name the same canonical
- **canonical selection**: one `selectCanonical()` (src/canonical.ts) picks each cluster's source of truth for the report, the starmap payload, and the live triage bot alike. issue-majority clusters resolve to the earliest report (the original bug); PR-majority ranks by lifecycle state (merged > open > closed), then a CI veto (a known-red build never outranks a same-state green sibling, before score), then quality score. the veto stops a high-scored PR with failing checks from becoming bestPick over the green fix that actually landed; only `ciStatus === "failure"` demotes, so a not-yet-reported PR is never penalized. fully deterministic - every tie bottoms out at item number - so re-runs name the same canonical
- **cluster confidence**: clustering is single-linkage (BFS over pairs >= threshold) with a centroid-refinement pass to break chained mega-clusters. because single-linkage can still chain in loosely-related members, each cluster reports both `avgSimilarity` and `minSimilarity` (lowest pairwise). the report/dupes output surfaces min as a confidence tier (high >= 90%, solid >= 80%, loose < 80%) so a low-min "loose" cluster gets eyeballed before anything is closed. avg and min are computed exactly over all pairs (no sampling), so the tier a maintainer sees is reproducible run to run

## database
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

all notable changes to pr-prism are documented here.

## [unreleased]

### changed
- canonical/bestPick selection now vetoes a red build: a PR with failing CI never outranks a same-state sibling with a non-failing build, regardless of quality score. stops a high-scored PR (e.g. one that added a test file) from being named bestPick over the green fix that actually landed, when the added test fails. only `ciStatus === "failure"` demotes, so a PR whose checks have not reported yet is never penalized; state (merged > open > closed) still dominates. starmap `canonical`/`contested` for such clusters shift accordingly (schema unchanged, still v1)

## [3.0.1] — 2026-07-13

### fixed
Expand Down
76 changes: 76 additions & 0 deletions src/__tests__/canonical.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,82 @@ describe("selectCanonical state preference (merged PRs are the source of truth)"
});
});

describe("selectCanonical CI veto (a red build never becomes bestPick over a green sibling)", () => {
it("the odysseus #5207 case: a failing higher-scored PR loses to a passing lower-scored sibling", () => {
// Both open at scan time. #5358 scored higher (a test file it added inflated
// its quality score) but its CI is red; #5208 is the minimal green fix that
// was actually merged. Without the veto, score picks #5358 (the wrong one).
const items = [
c({ number: 5358, type: "pr", state: "open", score: 0.752, ciStatus: "failure" }),
c({ number: 5208, type: "pr", state: "open", score: 0.532, ciStatus: "success" }),
];
expect(selectCanonical(items).number).toBe(5208);
});

it("a red build loses to a green sibling no matter how large its score lead", () => {
const items = [
c({ number: 1, type: "pr", state: "open", score: 0.99, ciStatus: "failure" }),
c({ number: 2, type: "pr", state: "open", score: 0.01, ciStatus: "success" }),
];
expect(selectCanonical(items).number).toBe(2);
});

it("only failure demotes: pending/unknown/absent CI are never penalized, score still decides", () => {
const pending = [
c({ number: 1, type: "pr", state: "open", score: 0.9, ciStatus: "pending" }),
c({ number: 2, type: "pr", state: "open", score: 0.5, ciStatus: "success" }),
];
expect(selectCanonical(pending).number).toBe(1); // pending not demoted -> higher score wins
const unknownVsAbsent = [
c({ number: 3, type: "pr", state: "open", score: 0.9, ciStatus: "unknown" }),
c({ number: 4, type: "pr", state: "open", score: 0.5 }),
];
expect(selectCanonical(unknownVsAbsent).number).toBe(3); // neither demoted -> higher score wins
});

it("when every sibling is failing, the CI gate ties and score decides (no bottomless demotion)", () => {
const items = [
c({ number: 1, type: "pr", state: "open", score: 0.4, ciStatus: "failure" }),
c({ number: 2, type: "pr", state: "open", score: 0.9, ciStatus: "failure" }),
];
expect(selectCanonical(items).number).toBe(2); // both red -> highest score
});

it("state still dominates CI: a merged PR wins even if its build is red", () => {
const items = [
c({ number: 1, type: "pr", state: "open", score: 0.9, ciStatus: "success" }),
c({ number: 2, type: "pr", state: "merged", score: 0.4, ciStatus: "failure" }),
];
expect(selectCanonical(items).number).toBe(2); // merged is in main = source of truth
});

it("the CI gate applies within a state: a green closed PR beats a red closed PR", () => {
const items = [
c({ number: 1, type: "pr", state: "closed", score: 0.9, ciStatus: "failure" }),
c({ number: 2, type: "pr", state: "closed", score: 0.4, ciStatus: "success" }),
];
expect(selectCanonical(items).number).toBe(2);
});

it("a pick decided by the CI gate is NOT contested, even when scores are within the tie margin", () => {
const items = [
c({ number: 1, type: "pr", state: "open", score: 0.52, ciStatus: "failure" }),
c({ number: 2, type: "pr", state: "open", score: 0.5, ciStatus: "success" }),
];
const d = decideCanonical(items);
expect(d.canonical.number).toBe(2); // green wins despite lower score
expect(d.contested).toBe(false); // CI decided it, not a coin flip
});

it("issue-majority clusters ignore CI (earliest report stays canonical)", () => {
const items = [
c({ number: 1, type: "issue", createdAt: "2026-01-01T00:00:00Z", score: 0.1, ciStatus: "failure" }),
c({ number: 2, type: "issue", createdAt: "2026-02-01T00:00:00Z", score: 0.9, ciStatus: "success" }),
];
expect(selectCanonical(items).number).toBe(1); // earliest report, CI is a PR-mode concept
});
});

describe("selectTracker (tracker substrate: original bug + role-tagged candidates)", () => {
it("mixed cluster: tracker = earliest issue, PRs are fixes, later issue is a duplicate", () => {
const items = [
Expand Down
27 changes: 25 additions & 2 deletions src/canonical.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
// Semantics (the cli.ts rule, now shared):
// - issue-majority cluster -> the earliest report is canonical (the original
// bug), quality score then item number as tiebreaks.
// - PR-majority cluster -> the highest-quality item is canonical, then
// earliest, then most-reviewed, then item number.
// - PR-majority cluster -> ranked in order: lifecycle state (merged >
// open > closed), then a CI veto (a known-red build never outranks a
// same-state non-failing sibling), then quality score, then earliest, then
// most-reviewed, then item number. state and the CI veto both sit BEFORE
// score so neither a stale open PR nor a failing one wins on score alone.
// `mode` lets a caller pin the rule instead of deriving it from the set — the
// triage bot passes the incoming item's type so its behavior is unchanged.

Expand All @@ -23,6 +26,8 @@ export interface CanonicalCandidate {
number: number;
/** "open" | "closed" | "merged"; absent for callers with no state (triage). */
state?: string;
/** CI rollup; absent for callers with no CI signal (triage). Only "failure" demotes. */
ciStatus?: "success" | "failure" | "pending" | "unknown";
}

function createdMs(item: CanonicalCandidate): number {
Expand Down Expand Up @@ -63,11 +68,25 @@ function statePriority(state: string | undefined): number {
}
}

/**
* Rank a PR by CI outcome: only a KNOWN-red build demotes. A failing PR must not
* become bestPick over a same-state green sibling however high its quality score
* (the score rewards e.g. having a test file, blind to whether that test passes).
* success / pending / unknown / absent all rank equal (1) so a PR whose checks
* simply have not reported yet is never penalized, and an all-failing set ties
* here and falls through to score unchanged.
*/
function ciRank(status: string | undefined): number {
return status === "failure" ? 0 : 1;
}

function isNearTie<T extends CanonicalCandidate>(mode: "issue" | "pr", canonical: T, runnerUp: T): boolean {
if (mode === "pr") {
// A different lifecycle state decided the pick (e.g. merged over open) -> a
// clear winner, not a coin flip. Only a same-state pair is a score near-tie.
if (statePriority(canonical.state) !== statePriority(runnerUp.state)) return false;
// A green build chosen over a red sibling is a decisive reason, not a coin flip.
if (ciRank(canonical.ciStatus) !== ciRank(runnerUp.ciStatus)) return false;
return canonical.score - runnerUp.score < TIE_MARGIN;
}
// The pick is on createdAt, so nearness is a time window (a huge score gap
Expand Down Expand Up @@ -109,6 +128,10 @@ export function decideCanonical<T extends CanonicalCandidate>(
// BEFORE score - a merged fix often does not have the top computed score.
const sp = statePriority(b.state) - statePriority(a.state);
if (sp !== 0) return sp;
// Then veto a known-red build: a same-state green sibling always outranks it,
// BEFORE score, so a high quality score cannot promote a failing PR.
const cr = ciRank(b.ciStatus) - ciRank(a.ciStatus);
if (cr !== 0) return cr;
Comment on lines +133 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict the CI veto to PR candidates

When a PR-majority cluster includes its tracker issue and same-state PRs whose CI is failure, this comparison gives the issue an absent-CI rank of 1 while the failing PRs rank 0, so the issue is selected before score is considered. That regresses the existing PR-majority behavior that the issue stays the tracker and the best PR is the canonical/bestPick; the veto should only compare CI between PR candidates rather than treating issues as non-failing siblings.

Useful? React with 👍 / 👎.

if (b.score !== a.score) return b.score - a.score; // then highest score first
const d = createdMs(a) - createdMs(b);
if (d !== 0) return d;
Expand Down
Loading