Skip to content

fix(pull-request): prioritize failed checks - #12274

Open
akash3444 wants to merge 7 commits into
pingdotgg:mainfrom
akash3444:fix/pr-check-failures-first
Open

akash3444 wants to merge 7 commits into
pingdotgg:mainfrom
akash3444:fix/pr-check-failures-first

Conversation

@akash3444

@akash3444 akash3444 commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Failed and cancelled checks can be buried among passing checks in the PR Summary tab.

The shared check mapper now puts failures and cancellations first after selecting the newest run of each check. It preserves provider order within both groups and retains timestamp comparison and workflow-name disambiguation. GitHub and Forgejo use this mapper; Bitbucket also orders the final combined pages, and GitCafe uses the same ordering without deduplicating its checks.

Credit to @akash3444 for the original fix in #12274. This maintainer takeover retains the original commits and merges current main.

Verification:

  • Six focused suites: 255 → 256 tests. One new core regression checks cancellation priority, stable groups, and a passing rerun replacing an older failure. Existing timestamp coverage is retained. Existing Bitbucket pagination and GitCafe mapping fixtures were strengthened without adding tests.
  • Core/GitHub/Bitbucket JSON: 150 passed / 5 expected failures → 155 passed. The two revised API/mapping fixtures gave 76 passed / 2 expected failures → 78 passed. Forgejo adds 23 passing tests, for 256 total.
  • Broader verification: 615 tests across 22 scoped source-control files passed. The existing GitCafe provider pagination expectation was updated for failure-first ordering (24 passed / 1 failed → 25 passed), with no added tests.
  • Scoped source-control lint, formatting, and typechecks passed; one existing lint warning remains in the GitCafe provider test.
  • Linux web verification used the running app's actual PR Summary tab with isolated cached fixture data. The same entries were passed through the actual base and head helpers, then loaded through the app's existing detail-snapshot cache. This verifies the visible ordering; it does not claim live provider API sessions. The web popover and mobile apply their existing independent status grouping. Mobile's ranking still places pending checks before cancelled checks; this PR makes no mobile display-order claim. GitLab supplies one pipeline check and Azure DevOps supplies no checks, so neither needs list ordering.

Before — base helper output:

Before: failed checks mixed with successful and running checks

After — head helper output:

After: failed and cancelled checks appear first

Not exercised: macOS, Windows, iOS, Android, Electron desktop, or a second remote environment.

Implemented with Codex gpt-6.1-sol; tests by Codex gpt-6-astra; independent reviews by Claude claude-opus-5-5 and Codex gpt-6-sol through T3 Code delegate_task.

- Show failed and cancelled checks before other pull request checks
- Preserve ordering within failing and remaining check groups
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 17, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 03abb02

Macroscope's review found this PR approvable — This is a small, focused fix that reorders existing pull-request checks so failed and cancelled results appear first, without changing check data or workflow behavior. The shared helper and provider expectations are covered by focused tests, and no sensitive, configuration, schema, or deployment files are involved.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dd6f6f6e-5129-4e55-a2b8-44f8e7a99bae
📥 Commits

Reviewing files that changed from the base of the PR and between 2491227 and bed9a43.

📒 Files selected for processing: 9
  • packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts
  • packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts
  • packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.test.ts
  • packages/source-control-core/src/server/pullRequestChecks.test.ts
  • packages/source-control-core/src/server/pullRequestChecks.ts
  • packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts
  • packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts
  • packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts
  • packages/source-control-github/src/server/gitHubPullRequestJson.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

The shared pull-request check utilities now sort failed and cancelled checks before other statuses. Bitbucket and GitCafe apply this ordering to returned checks. Tests across core and provider packages cover ordering, status mapping, pagination, and check names.

Changes: Check ordering

Layer / File(s) Summary
Prioritize failed and cancelled checks
packages/source-control-core/src/server/pullRequestChecks.ts, packages/source-control-core/src/server/pullRequestChecks.test.ts
The exported sortChecks helper places failed and cancelled checks before other statuses. dedupeChecks applies the ordering to its results. Core tests cover status priority and check ordering.
Apply ordering in provider check results
packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts, packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts, packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.test.ts, packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts, packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts, packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts, packages/source-control-github/src/server/gitHubPullRequestJson.test.ts
Bitbucket and GitCafe sort their returned checks with sortChecks. Provider tests update expected ordering and cover status and name mappings. The GitHub test expects a failed check before a pending check.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to bed9a

Failed and cancelled checks are prioritized as intended, with no identified issue requiring resolution before merge.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description provides clear problem, change, and verification details. It does not include a dedicated Scope and approval section or provide the required triaged issue or explicit maintainer approv… Add a Scope and approval section. Link the triaged issue or approval discussion, include the explicit maintainer approval comment, and explain why the change is within the approved scope.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: failed checks receive priority in pull request check ordering.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description provides clear problem, change, and verification details. It does not include a dedicated Scope and approval section or provide the required triaged issue or explicit maintainer approval for the behavior change.

Resolution

Add a Scope and approval section. Link the triaged issue or approval discussion, include the explicit maintainer approval comment, and explain why the change is within the approved scope.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

akash3444 and others added 2 commits September 17, 2026 22:55
- Sort failed and cancelled checks before passing checks while preserving relative order

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The screenshots show the visibility problem: failed checks sit among many successful checks. The patch replaces dedupeChecks’ documented provider-order behavior with failed/cancelled-first ordering for every consumer, and updates the provider expectations accordingly.

Can a maintainer confirm that status priority should replace provider order, including cancelled checks? No direction is linked or recorded here. The prior-approval rule asks for agreement on intentional behavior changes. Leaving this open for that focused ordering decision; the supplied screenshots and stable-within-group regression coverage are useful.

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-5.6-sol). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

@maria-rcks maria-rcks closed this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch misc-01. Failed and cancelled checks should appear first, preserving provider order within each group. I will adapt the shared implementation to the current source-control packages, retain the original commits, and run focused tests and independent reviews. This work will remain unmerged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants