feat: surface pull requests that need action - #9
Merged
Conversation
Default-branch health said nothing about whether an open PR was stuck. Renovate PRs sitting green because automerge is off on that repo, and PRs that are flat broken, were both invisible. Adds `github_pull_needs_attention` and `github_pull_ready_to_merge`, plus `checks`, `mergeable`, and `auto_merge` labels on the existing PR metric. `checks` is the head commit's statusCheckRollup, which unlike on a default-branch commit is reliable: PR-triggered workflows attach to the head commit by construction. `mergeable="unknown"` is deliberately not treated as mergeable. GitHub computes mergeability lazily, so a first query often returns UNKNOWN for a PR that is in fact conflicting -- two PRs sampled here both resolved to CONFLICTING on re-query. Treating UNKNOWN as mergeable would report conflicted PRs as ready to merge. Also fixes a false positive in default-branch CI state: a run's event must still be a trigger the workflow currently declares. frext and bike-fitter-1000 both reported failing `push` runs for a ci.yml that declares only pull_request; the workflow used to run on push, and GitHub keeps those runs forever. The trigger list is parsed from the workflow file during the existing cron lookup, so it costs no additional request, and an unparsable file leaves the list empty which accepts any event -- a failed lookup degrades to the previous behaviour rather than blanking a repository. Verified against the live API: all 16 previously-reported failures were individually confirmed as still matching a declared trigger, so nothing was over-filtered. frext and bike-fitter-1000 now report their real pull_request state.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe exporter validates workflow runs against declared workflow events and exposes pull-request checks, mergeability, auto-merge, attention, and readiness metrics. Workflow definitions provide trigger and cron metadata, while interval caching uses workflow file paths. ChangesPull metrics and workflow validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubGraphQL
participant OpenPull
participant Collector
participant PrometheusMetrics
GitHubGraphQL->>OpenPull: provide pull request state
OpenPull->>Collector: provide labels and classifications
Collector->>PrometheusMetrics: publish pull request metrics
sequenceDiagram
participant Collector
participant GitHubREST
participant WorkflowDefinition
participant RunReducer
Collector->>GitHubREST: fetch workflow definition
GitHubREST->>WorkflowDefinition: parse triggers and cron schedules
WorkflowDefinition->>Collector: return workflow metadata
Collector->>RunReducer: provide declared triggers
RunReducer->>Collector: retain valid workflow runs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@github-ci-exporter/src/github/rest.rs`:
- Around line 321-329: Update fingerprint_workflows to include Workflow.triggers
in the cache fingerprint, sorting the triggers deterministically before hashing
so trigger-only changes produce a new key. Add a regression test demonstrating
that changing only the workflow triggers changes the fingerprint.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c9be23e-55c0-437d-8aa2-32bf1abe4e7a
📒 Files selected for processing (6)
README.mdgithub-ci-exporter/src/collector.rsgithub-ci-exporter/src/github/graphql.rsgithub-ci-exporter/src/github/rest.rsgithub-ci-exporter/src/metrics.rsgithub-ci-exporter/src/model.rs
There was a problem hiding this comment.
Pull request overview
This PR improves observability of “stuck” open pull requests by exporting additional PR-state labels and introducing two new PR actionability metrics, and it reduces default-branch CI false positives by filtering workflow runs to only events still declared by the current workflow triggers.
Changes:
- Add PR-level state extraction via GraphQL (
checks,mergeable,auto_merge) and export them as labels on existing PR metrics. - Introduce
github_pull_needs_attentionandgithub_pull_ready_to_mergegauges for actionable PR states. - Parse workflow
on:triggers from workflow YAML and discard historical runs whose events are no longer declared, preventing false positives after trigger changes.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new PR actionability metrics and new PR metric labels; documents the new “event must still be declared” workflow-run filtering rule. |
| github-ci-exporter/src/model.rs | Adds ChecksState and MergeableState enums to model PR check-rollup and mergeability states. |
| github-ci-exporter/src/metrics.rs | Extends PR label set and registers new pull_needs_attention / pull_ready_to_merge metric families. |
| github-ci-exporter/src/github/rest.rs | Adds workflow trigger storage, parses triggers from workflow YAML, and filters runs by currently-declared triggers. |
| github-ci-exporter/src/github/graphql.rs | Extends PR GraphQL query to fetch check rollup, mergeability, and auto-merge, and computes actionability helpers. |
| github-ci-exporter/src/collector.rs | Populates new PR labels/metrics and resolves workflow definitions (crons + triggers) for run filtering. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+310
to
+312
| let intervals = resolve_definitions(client, &repo, &mut workflows).await; | ||
| cache.workflows.insert(key.clone(), workflows); | ||
| cache.intervals.insert(key, intervals); |
Comment on lines
+101
to
+103
| /// `merge_group` accepts `push` too: a merge-queue run reports | ||
| /// `event = "merge_group"`, but the queue exists to gate what lands on the | ||
| /// branch, so treating it as branch state is correct. |
reduce_runs now filters on the trigger list, which makes triggers part of the reduction's input. The fingerprint covered only path and name, so a trigger-only edit kept the same cache key: the runs listing is unchanged by such an edit, the request answers 304, and the stale reduction computed against the old trigger set is replayed. That is the same stale-reduction bug the fingerprint was introduced to prevent, reintroduced by the trigger filter. Triggers are sorted before hashing so a reordering of the `on:` mapping cannot spuriously invalidate every entry and force a full uncached sweep. Verified by reverting the fix: two of the three new tests fail without it. An earlier attempt at that verification used a revert whose string did not match, so the tests passed and appeared to prove nothing -- worth stating because "the test passes with the fix removed" is the only signal that distinguishes a real regression guard from a decorative one.
Two review findings, both confirmed: - `cache.intervals.insert` replaced the whole per-repo interval map every cycle. `resolve_definitions` omits any workflow whose contents fetch failed, so a transient error dropped previously-known intervals, making workflow_expected_interval_seconds disappear and reappear. That flaps GitHubScheduledWorkflowStale, which compares against it. Now merged, with entries for workflows that no longer exist retained-then- pruned so a deleted cron workflow still stops being expected to run. - The same failure emptied the trigger list, and an empty list means "accept any event", so one bad cycle would silently re-admit runs from a superseded trigger -- the frext/bike-fitter false positive returning intermittently. Triggers are now carried forward from the previous cycle, matched on file path so a rename keeps them, while a freshly resolved list always wins over the cached one. Also removes a false claim from `declares`: the doc comment described a merge_group/push equivalence that the code did not implement. It should not: merge-queue runs execute on a gh-readonly-queue branch, so the runs request's `branch=` filter excludes them before reduction. Verified against nixos and freminal -- 24 and 6 merge_group runs respectively, none under `branch=main`. The comment now records why the case cannot arise instead of describing behaviour that does not exist. Both fixes are extracted into pure functions so they are testable. Verified by reverting: 4 of the 7 new tests fail without them.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@github-ci-exporter/src/collector.rs`:
- Around line 357-365: Update merge_intervals and its callers to retain cached
intervals using workflow path identity, migrating entries when a workflow is
renamed instead of deleting them by the old name. Preserve intervals restored by
carry_forward_triggers when definition lookup fails, while still removing
intervals after a successful lookup confirms the workflow no longer has a cron
schedule.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b3143f2-8838-4e58-bc94-3d3a426008ab
📒 Files selected for processing (2)
github-ci-exporter/src/collector.rsgithub-ci-exporter/src/github/rest.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- github-ci-exporter/src/github/rest.rs
The interval cache was keyed by display name while workflow identity is the file path -- the same inconsistency already fixed in the run reducer. Renaming a workflow whose contents lookup then failed orphaned its cached interval: carry_forward_triggers restored the triggers by path, but the retain step pruned the interval because the old display name was no longer in the live set. workflow_expected_interval_seconds vanished for that cycle, flapping GitHubScheduledWorkflowStale. Intervals are now keyed by path throughout, and record_runs maps the run's display name to its path through the cached workflow set, so the exported metric keeps its human-readable workflow label. Verified by reverting to name-keying: 4 of the interval tests fail. Confirmed against the live API that the metric still populates -- 36 series, unchanged in shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Default-branch health said nothing about whether an open PR is stuck. Renovate PRs sitting green because automerge is off, and PRs that are flat broken, were both invisible.
New metrics
github_pull_needs_attentiongithub_pull_ready_to_mergePlus
checks,mergeable, andauto_mergelabels on the existing PR metric.checksis the head commit'sstatusCheckRollup. Unlike on a default-branch commit (null 30/31 times), it is reliable here — PR-triggered workflows attach to the head commit by construction. Measured 7/10 populated; the 3 nulls are repos with no PR CI, reported aschecks="none"rather than stuckpending.mergeable="unknown"is not treated as mergeable. GitHub computes it lazily, so a first query often returnsUNKNOWNfor a PR that is actually conflicting — two sampled PRs both resolved toCONFLICTINGon re-query. Treating it as mergeable would report conflicted PRs as ready to merge.Also: a default-branch false positive
A run's event must still be a trigger the workflow currently declares.
frextandbike-fitter-1000both reported failingpushruns for aci.ymlthat declares onlypull_request. The workflow used to run on push; GitHub keeps those runs forever. Confirmed: since the lastci.ymlchange, frext produced onlypull_requestruns.The trigger list is parsed from the workflow file during the existing cron lookup, so it costs no additional request. An unparsable file leaves the list empty, which accepts any event — a failed lookup degrades to previous behaviour rather than blanking a repository.
Verification against live data
Each of the 16 previously-reported failures was individually re-checked against its workflow's declared triggers — all 16
KEPT, nothing over-filtered. The count fell to 1 because theupdate-flakesfailures were genuinely fixed upstream in the meantime, not because they were hidden.Currently surfaced as needing attention:
Testing
93 tests (6 new for triggers, 9 for PR state), covering: automerge on vs off, conflicting-but-green,
UNKNOWNmergeability, drafts, repos with no PR CI, all three YAMLon:forms, and the exact frext workflow shape.cargo xtask cigreen; all 28 pre-commit hooks pass.Summary by CodeRabbit
New Features
Bug Fixes
Documentation