ci: PR-feedback quick wins — auto-cancel, drop dead job, restore module cache#986
Merged
oskarszoon merged 4 commits intoMay 29, 2026
Merged
Conversation
Contributor
|
🤖 Claude Code Review Status: Complete SummaryThis PR implements low-risk CI optimizations to reduce PR feedback time. The changes are configuration-only and well-reasoned. Key Changes:
Analysis: ✅ Concurrency configuration: Correctly applied to PR-triggered workflows with appropriate grouping keys. The sonar workflow uses a fallback pattern to handle both regular and fork PRs. ✅ Benchmark exclusion: Confirmed that ✅ Dead job removal: Verified that ✅ Cache strategy: Module cache ( ✅ Security: All action versions pinned to commit SHAs. Observations: The sonar-pr-analyze concurrency group expression at .github/workflows/sonar-pr-analyze.yaml:21 uses a fallback pattern that handles both cases (PR number when available, branch name as fallback), which is correct for workflow_run triggers that may have empty pull_requests arrays for fork PRs. Verdict: No issues found. Changes align with stated goals and follow best practices. |
|
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-29 15:08 UTC |
sugh01
approved these changes
May 29, 2026
ordishs
approved these changes
May 29, 2026
ordishs
left a comment
Collaborator
There was a problem hiding this comment.
Approve — well-scoped, config-only CI change. Verified:
sequential-sqliteremoval is safe: zero references under.github/, smoketests has noneeds:graph, andsonar-pr-analyzeonly consumescoverage-report/golangci-lint-report/sonar-filename-report(none from smoketests).- benchmark-compare exclusion holds: it lives in its own workflow file, so the workflow-level concurrency on the gates can't reach it — partial benchmark runs are preserved.
- All four PR gate workflows covered, no concurrency-group name collisions, and the sonar
pull_requests[0].number || head_branchkey matches the existingclaude-code-reviewpattern. - Cancelling superseded gate runs is safe downstream: both sonar and claude-code-review guard on
conclusion == 'success', so a cancelled run just doesn't trigger them. - Module cache is correctness-safe: mod-dir-only, go.sum-keyed, SHA-pinned; Go checksum-verifies against go.sum.
Non-blocking follow-ups:
- Confirm
sequential-sqliteis not listed as a required status check in branch protection (only thing not verifiable from the repo). - Fork PRs fall back to
head_branchin the sonar key — same-branch-name collisions across forks could cross-cancel (sonar-only, matches existing pattern). - Consider extending the module cache to the PR smoketests jobs in a later phase.
- "Small module cache" is really "stable/infrequently-written" (674 modules) — the size argument is weaker than the write-frequency one.
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
3 participants
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.
First of three phases from a CI-speed investigation. The PR-feedback gate has drifted up over the last quarter (
pr-smoke+33% in 9 weeks,main-build+19%); these are the low-risk, broad-benefit changes that don't touch test behaviour.Changes
Auto-cancel superseded runs. Added a
concurrencygroup withcancel-in-progress: trueto the PR-triggered gate workflows (teranode_pr_tests,teranode_pr_smoketests,teranode_pr_init,dashboard_pr_smoke) and tosonar-pr-analyze(keyed on the originating PR, since it'sworkflow_run-triggered). Pushing a new commit to a PR now cancels the prior in-flight run instead of running both to completion — less runner saturation, less queue time on busy days. Not applied to main/push workflows.benchmark-compareis deliberately excluded: cancelling a partial benchmark run would drop comparison results.Removed the no-op
sequential-sqlitejob. It had no run step — it checked out, set up Go, and uploaded a results file that was never produced. Zero coverage behind an always-green check. Confirmed separately that the--db sqlitename filter matches zero tests, so it ran nothing regardless. Not a required status check.Restored a Go module cache.
actions/cacheon~/go/pkg/modonly (keyed ongo.sum) for the compile-heavy jobs interanode_pr_testsandteranode_main_tests. Deliberately NOT the build cache (~/.cache/go-build) — that is what #3352 disabled, for thrashing the 10 GB Actions cache limit with large-racebuild artifacts. The module cache is small and stable, andsetup-go's own caching stays off.Risk
Low — config only. The module cache reverts cleanly if it regresses. Validated with
actionlint(only pre-existing custom-runner-label warnings).