feat(canonical): red CI vetoes bestPick over a green sibling - #15
Conversation
a PR with a known-failing build must never become a cluster's canonical/ bestPick over a same-state sibling whose build is not red, regardless of quality score. the quality score rewards signals like 'has a test file' without checking that the test passes, so a scope-creep PR with a failing added test could outrank the minimal green fix that actually landed (odysseus #5207: bestPick was the closed, CI-failing #5358 over the merged #5208). - ciRank() in canonical.ts: only ciStatus==='failure' demotes; success/ pending/unknown/absent rank equal, so a not-yet-reported PR is never penalized and an all-failing set falls through to score unchanged - inserted into PR-mode compare after statePriority, before score, so state (merged>open>closed) still dominates and the veto acts within a state - isNearTie: a green-over-red pick is decisive, so contested=false - 8 tests incl. the live #5207 case; starmap value change only, schema v1 flows through the single selectCanonical source to report, starmap payload, cli, and identity; triage's DupeMatch has no ciStatus so it is unaffected.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cd0efdb3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cr = ciRank(b.ciStatus) - ciRank(a.ciStatus); | ||
| if (cr !== 0) return cr; |
There was a problem hiding this comment.
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 👍 / 👎.
the problem (live, on the star map)
odysseus cluster 4 = bug #5207 (ntfy reminder dies on non-ASCII title) with two fix PRs. ground truth: #5208 merged (3-line fix, pytest green), #5358 closed (pytest failing). pr-prism's bestPick was #5358 — the rejected one — at score 0.752 vs 0.532, contested=false.
why: #5358 added a test file, which inflated its quality score via the
hasTests/descriptionQuality/diffSizesignals. the signals can't see that the test fails, is a tautology, and covers unrelated scope. so a red, closed, scope-creep PR outranked the green fix that actually landed.the fix
a known-red build never becomes canonical/bestPick over a same-state sibling whose build isn't red, regardless of quality score.
ciRank()in canonical.ts: onlyciStatus === "failure"demotes. success/pending/unknown/absent rank equal, so a PR whose checks haven't reported yet is never penalized, and an all-failing set falls through to score unchangedcompareafter statePriority, before score: state (merged > open > closed) still dominates, and the veto acts within a stateisNearTie: a green-over-red pick is decisive, socontested=falseselectCanonicalsource to report, starmap payload, cli, identity. triage's DupeMatch has no ciStatus, so it's unaffectedtests + gates
star-map compatibility
value change only —
canonical/contestedshift for affected clusters.STARMAP_SCHEMA_VERSIONstays 1, old snapshots keep parsing. this is the first of the three calibration fixes surfaced by dogfooding cluster 4.