Skip to content

ci: streamline Python test matrix - #1131

Merged
bokelley merged 3 commits into
mainfrom
ci-optimize
Sep 5, 2026
Merged

ci: streamline Python test matrix#1131
bokelley merged 3 commits into
mainfrom
ci-optimize

Conversation

@bokelley

@bokelley bokelley commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • run Ruff, mypy, adopter type checks, and the fixture contract only in the Python 3.12 matrix leg
  • keep runtime tests across Python 3.10–3.13 while collecting coverage only on Python 3.12
  • enable setup-python pip caching for every Python job
  • add job-level timeouts so stalled jobs terminate instead of holding PRs indefinitely
  • let superseded CI runs skip the aggregate storyboard gate so concurrency cancellation actually completes

Why

The four-version matrix repeated static analysis and coverage even though those results are not version-specific. Recent successful matrix legs took roughly 16–21 minutes; most of that time is pytest, so this is a conservative cleanup rather than test sharding without measurements.

Static analysis stays inside the already-required Test Python 3.12 check. This preserves the current ruleset gate without requiring a repository-settings change.

The previous storyboard-required-gate used job-level if: always(). A superseding push canceled every other job in the old run, but that gate remained queued and held the workflow concurrency slot, leaving the replacement run pending. if: ${{ !cancelled() }} still runs the aggregate gate after matrix failures, while allowing a canceled workflow to terminate.

Validation

  • actionlint v1.7.7
  • pre-commit YAML and repository hygiene hooks
  • Ruff
  • mypy for src/adcp/
  • strict adopter type-check fixtures
  • type-ignore fixture contract
  • live reproduction of the superseded-run concurrency stall on this PR

Review

Three independent CI/security reviewers found no workflow correctness blocker. Their required-check concern was resolved by keeping the one static-analysis run in the required Python 3.12 matrix context.

Follow-up

This refactor does not change the separate CodeQL workflow. Auditing ci.yml with zizmor and pinning its remaining third-party actions should be handled as a focused security-hardening change because the current audit intentionally excludes this workflow.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean CI-only refactor, no blocking findings.

What I checked

  • Single file touched: .github/workflows/ci.yml (+44/-10), a matrix streamlining + pip caching + added job timeouts.
  • Reviewer verified every Python version still runs tests, the fail_under=80 coverage gate still fires on the 3.12 leg, and gating ruff/mypy to 3.12 loses no coverage since both pin target py310 independent of interpreter.
  • No public-surface / semver, forward-compat, import-layering, generated-code, ctx_metadata, or CI-gate-disarming concerns triggered — this diff touches none of those load-bearing areas.
  • The if: always()→!cancelled() change referenced in the PR body lands in a later commit (0f0dc37) outside the reviewed head, so it is out of scope, not a defect.

Gate checks

  • gated_paths: false — row 2 not triggered.
  • high_risk: false, no deletions — rows 3/5 not triggered.
  • No author no-auto-approve team match — row 7 not triggered.
  • Zero findings (no critical/high/medium) — rows 1, 4, 8 not triggered.

review_decision: REVIEW_REQUIRED does not force a downgrade because gated_paths is false. Falls through to row 9 → approve.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

CI-only refactor to .github/workflows/ci.yml (+47/-11). No blocking findings.

What I checked:

  • if: always()if: ${{ !cancelled() }} on the storyboard-required-gate is correct: the storyboard matrix runs fail-fast: false, so a failing leg still fails the aggregate gate while only run-level cancellation now skips it — closing the superseded-run concurrency stall without weakening the gate.
  • Gating static analysis (ruff/mypy/type-checks) to the 3.12 leg loses no coverage: mypy is pinned to python_version 3.10 and ruff to py310, so results are Python-version-independent.
  • Job timeouts and pip caching are pure hygiene.
  • CI gates remain armed — no test disabled, no ruff rule dropped, no blanket mypy silencing.

No Critical/High/Medium findings. gated_paths is false; no high-risk paths; no author team gate. Falls through to row 9.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

CI-only refactor of .github/workflows/ci.yml (+66/-16, single file, no source or wire surface touched). Verified against the decision table top-down:

  • No critical/high/medium findings (row 1, 4, 5, 6, 8 do not fire).
  • gated_paths: false — row 2 does not apply despite review_decision: REVIEW_REQUIRED.
  • high_risk: false — no deletion/modification escalation (rows 3, 5).
  • No no-auto-approve team match (row 7 does not fire).
  • Falls through to row 9 → approve.

Reviewer confirmed the storyboard gate move from if: always() to if: ${{ !cancelled() }} (correct idiom — still fails the required check on matrix failure while letting concurrency-cancelled superseded runs release the slot), static-analysis folded into the required 3.12 matrix leg with pytest running exactly once per leg, and added job timeouts plus pip caching. CI gates remain armed (no disabled tests, no dropped ruff rules, no blanket mypy silencing).

Medium findings
None.

One non-blocking Low note: mypy now runs only under 3.12 — acceptable follow-up, not a blocker.

@bokelley
bokelley enabled auto-merge (squash) September 5, 2026 19:01
@bokelley
bokelley merged commit 1a9e9a8 into main Sep 5, 2026
28 checks passed
@bokelley
bokelley deleted the ci-optimize branch September 5, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant