fix: report only genuinely current CI state - #2
Conversation
Naively taking the newest run per workflow produced 38 "failures" across the two organisations, of which fewer than half described anything actionable. Four distinct causes, each verified against the live API: - Orphaned runs. GitHub retains run history after a workflow file is deleted, so a long-removed "Update pre-commit hooks" reported a permanent failure in 10 repositories. Runs are now intersected with the repository's live workflow list. - Path-named runs. A run created before its workflow gained a `name:` reports the file path in the name field, splitting one workflow into two series. Workflow identity is now the file path, with the display name resolved from the live list. - Pull-request runs. The API's `branch=` filter matches a pull request's HEAD branch, not its base, so PR runs leaked into what is meant to be default-branch state. A merged PR's last pre-merge failure was reported as the branch's CI state indefinitely. Only push, schedule, and workflow_dispatch now count. - Stale runs. Workflows that fire only on pull_request leave a branch-state run months old; `pre-commit-checks` Lint last ran on a push in December 2025. Past 90 days a run reports conclusion="stale" and sets `workflow_run_stale`, rather than an unclearable failure. Disabled workflows are no longer filtered out. Excluding them hid a failing `update-flakes` in fred-cal that GitHub had auto-disabled for repository inactivity, which is precisely the condition worth alerting on. `workflow_enabled` now carries the state, distinguishing an inactivity auto-disable from a deliberate manual one. Net effect: 209 workflow series to 126, and 38 failures to 16 real plus 7 aged out.
|
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 now caches complete workflow definitions, tracks enabled state, filters runs by live workflow paths and branch-state events, marks runs older than 90 days as stale, and exposes metrics and documentation for these states. ChangesWorkflow State Observability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Collector
participant GitHubAPI
participant RunReducer
participant Metrics
Collector->>GitHubAPI: list workflow definitions
GitHubAPI-->>Collector: return live and disabled workflows
Collector->>GitHubAPI: fetch workflow runs
GitHubAPI-->>RunReducer: return run entries
RunReducer->>RunReducer: filter events and match workflow paths
RunReducer->>Metrics: report conclusions and stale status
Collector->>Metrics: publish workflow enabled state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
README.md (1)
119-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the values of the
statelabel.The section explains why disabled workflows are kept, but it does not list the values
github_workflow_enabledcan carry. An operator writing an alert needs to know they areactive,disabled_inactivity, anddisabled_manually, and that onlydisabled_inactivityindicates a fault.📝 Proposed addition
| Disabled workflows are kept | A workflow auto-disabled by GitHub after 60 days of inactivity has stopped running silently. That is the fault worth alerting on, so it is reported rather than filtered out. | + +`github_workflow_enabled` is `1` when GitHub will run the workflow. Its +`state` label is `active`, `disabled_inactivity`, or `disabled_manually`. +Only `disabled_inactivity` indicates a fault; a manual disable is deliberate.🤖 Prompt for 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. In `@README.md` around lines 119 - 131, Update the “What counts as the current CI state” section to document the possible github_workflow_enabled state values: active, disabled_inactivity, and disabled_manually. Clarify that only disabled_inactivity represents a fault requiring alerting.github-ci-exporter/src/collector.rs (1)
417-447: 🩺 Stability & Availability | 🔵 TrivialConfirm that alert rules are updated for the new
conclusion="stale"value.A stale run no longer reports
conclusion="failure". Any recording rule or alert that selectsgithub_workflow_run_status{conclusion="failure"}silently stops matching those 7 runs. Update the rules together with this change, or the transition looks like the failures were fixed.🤖 Prompt for 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. In `@github-ci-exporter/src/collector.rs` around lines 417 - 447, Update the recording and alert rules that select github_workflow_run_status with conclusion="failure" to also account for conclusion="stale", preserving failure alerts while ensuring stale runs remain represented in the alerting behavior introduced by the status assignment above.
🤖 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 166-177: Update the repository loop around
`record_workflow_states` and `rest::fetch_runs` to skip fetching runs when
`live` is empty, and emit a warning explaining that the run fetch was skipped
because no cached workflows are available. Preserve the existing fetch and
error-handling flow for repositories with non-empty `live` data.
In `@github-ci-exporter/src/github/rest.rs`:
- Around line 241-263: Use workflow path as the identity throughout reduce_runs:
key latest and last_success by path, while retaining the display name inside
LatestRun for presentation. In github-ci-exporter/src/collector.rs lines
310-327, update the workflow_enabled labels to include a path-based
disambiguator so workflows with duplicate names remain distinct; both sites
require changes.
- Around line 186-199: Update fetch_runs and its get_cached usage so cache
entries vary with the current live workflow paths and names, preventing stale
RepoRuns after deletions or renames. Either incorporate a stable fingerprint of
live into the cache key while preserving URL-based request behavior, or cache
raw workflow runs and reapply reduce_runs on each fetch.
---
Nitpick comments:
In `@github-ci-exporter/src/collector.rs`:
- Around line 417-447: Update the recording and alert rules that select
github_workflow_run_status with conclusion="failure" to also account for
conclusion="stale", preserving failure alerts while ensuring stale runs remain
represented in the alerting behavior introduced by the status assignment above.
In `@README.md`:
- Around line 119-131: Update the “What counts as the current CI state” section
to document the possible github_workflow_enabled state values: active,
disabled_inactivity, and disabled_manually. Clarify that only
disabled_inactivity represents a fault requiring alerting.
🪄 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: bb16dc6d-bddc-41ca-bb20-4ab7bedc1863
📒 Files selected for processing (4)
README.mdgithub-ci-exporter/src/collector.rsgithub-ci-exporter/src/github/rest.rsgithub-ci-exporter/src/metrics.rs
| fn reduce_runs(runs: Vec<RunEntry>, live: &[Workflow]) -> RepoRuns { | ||
| // Workflow identity is the file path, not the display name: a run created | ||
| // before the workflow gained a `name:` reports the path in the name field, | ||
| // and a renamed workflow would otherwise split into two series. | ||
| let live_by_path: HashMap<&str, &Workflow> = | ||
| live.iter().map(|w| (w.path.as_str(), w)).collect(); | ||
|
|
||
| let mut latest: HashMap<String, LatestRun> = HashMap::new(); | ||
| let mut last_success: HashMap<String, DateTime<Utc>> = HashMap::new(); | ||
|
|
||
| for run in runs { | ||
| if run.event == "dynamic" { | ||
| if !is_branch_state_event(&run.event) { | ||
| continue; | ||
| } | ||
| // A run whose workflow file was deleted still appears in history; | ||
| // without a path there is no stable identity to key on. | ||
| let Some(name) = run.name.filter(|n| !n.is_empty()) else { | ||
| // Runs of a since-deleted workflow linger in history forever. Only | ||
| // workflows still present in the repository are reported. | ||
| let Some(path) = run.path.as_deref() else { | ||
| continue; | ||
| }; | ||
| if run | ||
| .path | ||
| .as_ref() | ||
| .is_some_and(|p| !p.starts_with(".github/workflows")) | ||
| { | ||
| let Some(workflow) = live_by_path.get(path) else { | ||
| continue; | ||
| } | ||
| }; | ||
| let name = workflow.name.clone(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Workflow identity is the file path in the lookup but the display name in every key and label. The PR establishes the path as workflow identity, yet both the reduction maps and the metric labels use name. Two workflow files that declare the same name: therefore merge into one series in both places.
github-ci-exporter/src/github/rest.rs#L241-L263: keylatestandlast_successby workflow path, and carry the display name insideLatestRun.github-ci-exporter/src/collector.rs#L310-L327: add a disambiguator to theworkflow_enabledlabels, or document that duplicate workflow names collapse.
📍 Affects 2 files
github-ci-exporter/src/github/rest.rs#L241-L263(this comment)github-ci-exporter/src/collector.rs#L310-L327
🤖 Prompt for 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.
In `@github-ci-exporter/src/github/rest.rs` around lines 241 - 263, Use workflow
path as the identity throughout reduce_runs: key latest and last_success by
path, while retaining the display name inside LatestRun for presentation. In
github-ci-exporter/src/collector.rs lines 310-327, update the workflow_enabled
labels to include a path-based disambiguator so workflows with duplicate names
remain distinct; both sites require changes.
There was a problem hiding this comment.
Pull request overview
This PR improves the accuracy of the exporter’s “current CI state” reporting by ensuring only relevant, genuinely current GitHub Actions data contributes to branch health, while still surfacing important non-running conditions (like inactivity auto-disables).
Changes:
- Filter workflow runs to branch-state events, intersect run history with the live workflow list, and key run identity off workflow
path(with display name resolved from the live list). - Add staleness handling (90-day horizon) so ancient branch-state runs report
conclusion="stale"and emit a dedicatedgithub_workflow_run_stalemetric. - Stop filtering out disabled workflows; expose workflow enablement state via a new
github_workflow_enabledmetric (includingstate).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| README.md | Documents the new metrics and defines what counts as “current CI state.” |
| github-ci-exporter/src/metrics.rs | Adds metric families and registrations for workflow enablement and staleness. |
| github-ci-exporter/src/github/rest.rs | Updates workflow/run fetching and reduction logic to eliminate orphan/PR/stale noise and normalize identity by workflow path. |
| github-ci-exporter/src/collector.rs | Caches live workflow sets, records workflow enabled state, and publishes stale-run signals/labels during run recording. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if workflows.is_empty() && config.skip_repos_without_workflows { | ||
| cache.workflows.remove(&key); | ||
| skipped.push((repo, SkipReason::NoWorkflows)); | ||
| continue; | ||
| } | ||
| cache.workflows.insert(key.clone(), workflows.clone()); | ||
|
|
||
| // Cron schedules change rarely, so they are resolved once per set. | ||
| if let std::collections::hash_map::Entry::Vacant(entry) = cache.intervals.entry(key) { | ||
| entry.insert(resolve_cron_intervals(client, &repo, &workflows).await); | ||
| } |
| // Workflow identity is the file path, not the display name: a run created | ||
| // before the workflow gained a `name:` reports the path in the name field, | ||
| // and a renamed workflow would otherwise split into two series. | ||
| let live_by_path: HashMap<&str, &Workflow> = | ||
| live.iter().map(|w| (w.path.as_str(), w)).collect(); |
Three issues raised in review, all confirmed against the code: - The projection cache could revive the orphaned-run bug. `get_cached` keys on the request URL and replays the stored reduction on 304, but the reduction also depends on the live workflow list. Deleting or renaming a workflow leaves the runs listing unchanged, so the request answers 304 and the stale reduction is served indefinitely. Cache entries for runs now include an order-independent fingerprint of the workflow set's paths and names, via a new `get_cached_as` that decouples the cache key from the request path. Warm-cache reuse is unaffected: 310 of 379 requests still answer 304. - An empty workflow set silently emptied a repository. When `list_workflows` fails and nothing is cached, every run is discarded as orphaned, so the repository publishes no series at all while still spending a request. That is the "CI vanished" appearance the budget pre-flight exists to avoid. The run fetch is now skipped with a warning. - Documented that lookup is by path while output is keyed by display name, so two workflow files sharing a `name:` collapse into one series. Accepted deliberately: it does not occur in the monitored organisations, and keying metrics by file path would make dashboards and alerts harder to read.
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/client.rs`:
- Around line 407-417: Update the cache insertion in the request flow to store
entries under the computed cache_key, matching the lookup used by get_cached_as
and custom workflow-fingerprint requests. Extend the existing cache test around
the repeated `#v1` request to issue a second request with the cached ETag, assert
If-None-Match is sent, and verify the result is CacheOutcome::NotModified.
🪄 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: 48bedf1e-4537-4c9c-b925-2fdd86628605
📒 Files selected for processing (3)
github-ci-exporter/src/collector.rsgithub-ci-exporter/src/github/client.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
Entries were read by `cache_key` but written by `url`, so every `get_cached_as` caller stored under a key it would never look up. Since `fetch_runs` always uses a workflow-fingerprint key, the runs endpoint -- the largest consumer of the rate-limit budget -- refetched and reprojected on every cycle instead of revalidating. The regression was masked in testing: the warm-cache run still showed 310 of 379 requests answered 304, because the workflow-list and file-contents lookups were cached normally. Only the 62 run fetches were affected. With the write corrected, warm cycles answer 372 of 379 from cache. Adds a test asserting that a repeated custom key revalidates and yields `CacheOutcome::NotModified`, which the previous test did not cover: it checked only that two *different* keys stay separate.
Naively taking the newest run per workflow produced 38 "failures" across
sdr-enthusiastsandfredsystems, of which fewer than half described anything actionable.Four distinct causes, each verified against the live API:
Update pre-commit hooksreported a permanent failure in 10 repos.name:reports the file path instead, splitting one workflow into two series.path; resolve display name from the live list.branch=filter matches a PR's head branch, not its base. A merged PR's last pre-merge failure was reported as the branch's CI state indefinitely.push,schedule,workflow_dispatch.pull_requestleave a branch-state run months old —pre-commit-checksLint last ran on a push in Dec 2025.conclusion="stale"and setworkflow_run_stale.Disabled workflows are no longer filtered out. Excluding them hid a failing
update-flakesinfred-calthat GitHub had auto-disabled for repository inactivity — precisely the condition worth alerting on.workflow_enablednow carries the state, distinguishing an inactivity auto-disable from a deliberate manual one.Result
Testing
cargo xtask cigreen: fmt, clippy (strict lint set), test, deny, machetepublic_reporead tokenSummary by CodeRabbit
New Features
Bug Fixes
Documentation