Repository navigation
fix(health): ignore unexecuted workflow runs - #69225
Conversation
Exclude waiting runs and non-dispatched conclusions from health metrics and revision evidence while preserving pagination to executed runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Probe the sampled run attempt through the budgeted Jobs API and omit failures with no jobs from metrics, revisions, and failure clusters. Preserve executed failures, paginate to older evidence, and surface unavailable or malformed job responses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
pelikhan
changed the title
Continue bounded health sampling beyond the fixed page cap and roll over GitHub's 1,000-result search windows without skipping tied timestamps. Keep explicit time bounds and API budgets, and report incomplete boundary coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Ponytail Reviewer completed successfully!
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
Assert numbered pages keep the search window stable until rollover, while preserving coverage that filtered batches do not hide older dispatched runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
L7: yagni: 12-line pagination helper in a separate file with one caller. Inline the boundary check in sampleHealthWorkflow.
net: -8 lines possible.
Generated by ✂️ Ponytail Reviewer for #69225 · codex · gpt56 · 8.59 AIC · ⌖ 5.44 AIC · ⊞ 14.6K
Comment /ponytail to run again
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (477 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. 🔍 Evidence used
❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
There was a problem hiding this comment.
Review verdict
I found two actionable issues in the new health sampling path: the rollover logic re-probes duplicate failed runs, and the new waiting filter looks broader than the approval-only case this patch is trying to exclude.
Themes
- Boundary-overlap pagination now intentionally duplicates a timestamp window, but the jobs probe runs before dedupe, so redundant calls can burn the request budget and force partial reports.
waitingis being treated as equivalent to a never-dispatched run, which can hide real pending current-revision evidence if that status appears outside the approval-only shape.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 52 AIC · ⌖ 5.94 AIC · ⊞ 21.7K
Comment /review to run again
Confirm waiting attempts have no jobs before excluding them, cache successful job-presence results per run and attempt across overlap, and document bounded evidence sampling in ADR-69225. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Replying to #69225 (comment): No code change for this status-only comment. The two substantive sampling findings from the completed review are fixed in e573435 and answered in their respective threads. |
|
Replying to #69225 (comment): Agreed that reported_no_action does not establish test quality or completed validation, so no implementation change is inferred from this status. Focused health regressions and impacted Go tests passed, and the final formatting/lint/schema/workflow-drift gate passed after correcting the help function size. |
|
Replying to #69225 (comment): No implementation change is requested by this completion notice. The helper-inlining suggestion was evaluated and declined: the named boundary helper keeps the sampling loop readable and within repository function-size limits. |
|
Replying to #69225 (comment): No code change for this review-in-progress notification because it contains no actionable finding. The substantive findings supplied in this batch have been addressed separately. |
There was a problem hiding this comment.
Skills-Based Review ð�§
Applied /diagnosing-bugs, /grill-with-docs, and /tdd — observations only, no blocking issues.
� Key Themes & Highlights
Key Themes
- Redundant Jobs API calls at window boundaries: healthRunHasJobs runs before run de-duplication in sampleHealthWorkflow, so a failing run re-fetched across the 1s-overlapping search-window boundary gets checked twice before the duplicate is dropped. Minor API budget waste, not correctness-breaking.
- Doc duplication: The new paragraph on excluded run types (waiting/action_required/skipped/no-job failures) and the pre-existing paragraph on 'runs that never dispatched a job' now both describe overlapping behavior with slightly different scope, inviting drift.
- Test gap: healthRunHasJobs's Attempt == 0 fallback path (endpoint without /attempts/N) has no direct test.
Positive Highlights
- � Thorough regression coverage: pagination rollover at the 1,000-result boundary, timestamp-tie dedup, request-budget exhaustion, malformed/missing Jobs API responses, and caller-data immutability are all explicitly tested.
- � isIgnoredHealthRun is applied consistently across metrics, revisions, and sampling, avoiding a half-filtered state.
- � errHealthAPIBudget and partial-report semantics are preserved and extended correctly to the new Jobs API probe.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 139.1 AIC · ⌖ 15.1 AIC · ⊞ 11.7K
Comment /matt to run again
|
Replying to #69225 (comment): No separate implementation change is needed for this gate-completion notification. The subsequent ADR requirement is addressed with ADR-69225 and a link in the PR body. |
|
Replying to #69225 (comment): Accepted: added and linked ADR-69225, documenting context, decision, alternatives, consequences, attempt-scoped caching, and evidence-based waiting-run handling. The claimed draft was absent locally and on the branch, so this is a new Proposed ADR; maintainer approval and deciders are explicitly not asserted. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved health-classification, collection-coverage, and regression-test issues prevent approval.
1 open finding
What changed in this PR
Updates gh aw health to exclude unexecuted runs and continue sampling for usable workflow evidence.
Changes:
- Filters approval-waiting, skipped, and confirmed zero-job failed runs.
- Adds search-window rollover with quota and partial-report safeguards.
- Extends regression coverage, CLI help, and reference documentation.
| File | Description |
|---|---|
pkg/cli/health_sampling.go |
Filters runs, probes jobs, and extends pagination. |
pkg/cli/health_sampling_test.go |
Tests filtering and request counts. |
pkg/cli/health_revisions.go |
Excludes ignored runs from revision evidence. |
pkg/cli/health_revisions_test.go |
Tests exclusions and input preservation. |
pkg/cli/health_pagination.go |
Adds timestamp-based search-window rollover. |
pkg/cli/health_pagination_test.go |
Covers rollover, boundaries, and request limits. |
pkg/cli/health_no_jobs_test.go |
Covers job probes and insufficient evidence. |
pkg/cli/health_metrics.go |
Filters ignored runs without modifying caller data. |
pkg/cli/health_command.go |
Documents exclusions in CLI help. |
docs/src/content/docs/setup/cli.md |
Explains filtering and sampling behavior. |
🧠 Review effort: Max
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
🎉 This pull request is included in a new release. Release: |
Runs that never execute should not reduce workflow health or create pending revision evidence. GitHub can mark expired approval requests as
failure, so filtering onlyaction_requiredis insufficient.This change excludes skipped and
action_requiredruns. Waiting and failed runs are excluded only after an attempt-specific Jobs API lookup confirms no jobs. Job-backed waiting runs remain pending revision evidence. Successful job-presence results, including empty results, are cached by run ID and attempt within the collection. Jobs API errors and malformed responses are surfaced and never cached as zero jobs.Sampling continues backward past ignored runs without the unrelated fixed 20-page cap. At GitHub's 1,000-result search ceiling, health narrows the upper timestamp boundary and restarts pagination, overlapping the boundary second and deduplicating run IDs. Explicit start/end bounds and API quotas remain enforced. Saturated or unavailable timestamps that prevent safe advancement produce an explicitly partial report, never a healthy verdict based on truncated evidence.
The job-presence probe requests only one job, uses the existing quota guard, and avoids revision lookups for ignored runs. CLI help and reference documentation describe the behavior.
Regression coverage verifies more than 20 ignored pages, search-window rollover across 1,000 results, timestamp overlap and deduplication, reuse of empty and nonempty job probes across overlap, distinct attempts and collections, preservation of job-backed waiting revision evidence, conservative direct calculations without API evidence, explicit date bounds, saturation/missing timestamps, request-budget exhaustion, executed failures and caller-owned data, enterprise-host lookups, malformed/unavailable responses and uncached errors.
ADR: ADR-69225: Ignore Unexecuted Workflow Runs in Health Sampling
Validation: local CLI build, targeted health/time-window/pagination regressions, impacted Go tests, and the change-scoped formatting/lint/schema/workflow-drift gate passed. An earlier targeted AI Moderator recheck over the original 24-hour window produced zero failures and no failure clusters; its revision remained unknown, so this was not a healthy verdict. No workflows were dispatched.