fix: add concurrency group to codegraph-impact workflow#785
Conversation
Without a concurrency group, every push to every open PR queued a new run without cancelling stale ones. With many PRs active simultaneously, this starved the runner pool and left jobs queued for over an hour.
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewed the concurrency group addition for the codegraph-impact workflow. Root Cause AnalysisProblem Being Solved: The Why This Approach: Adds the missing concurrency control using the same proven pattern already implemented in Risk Assessment: LOW RISK — This is a hardening change that adds resource management without modifying workflow logic or relaxing any rules. Backlog Compliance
Critical ConcernsNone. This is a clean, minimal infrastructure improvement that follows established patterns. Implementation verified:
Final Recommendation
|
Greptile SummaryThis PR adds a The fix uses Confidence Score: 5/5Safe to merge — a minimal, correct, and well-precedented change with no logic or data concerns. The only change is a 4-line No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Runner as Runner Pool
Note over Dev,Runner: Before fix — no concurrency group
Dev->>GH: Push commit 1 to PR
GH->>Runner: Queue run A (codegraph-impact)
Dev->>GH: Push commit 2 to PR
GH->>Runner: Queue run B (no cancellation)
Dev->>GH: Push commit 3 to PR
GH->>Runner: Queue run C (no cancellation)
Note over Runner: All 3 runs queue up — pool starvation
Note over Dev,Runner: After fix — concurrency group per github.ref
Dev->>GH: Push commit 1 to PR
GH->>Runner: Start run A (group: codegraph-impact-refs/pull/N/merge)
Dev->>GH: Push commit 2 to PR
GH->>GH: cancel-in-progress cancels run A
GH->>Runner: Start run B (only latest runs)
Dev->>GH: Push commit 3 to PR
GH->>GH: cancel-in-progress cancels run B
GH->>Runner: Start run C (only latest runs)
Reviews (2): Last reviewed commit: "fix: use github.ref for concurrency key ..." | Re-trigger Greptile |
| on: [pull_request] | ||
|
|
||
| concurrency: | ||
| group: codegraph-impact-${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
Inconsistent concurrency key vs. sibling workflows
The PR description says this "matches the pattern already used by ci.yml, embedding-regression.yml, and publish.yml", but those three all use github.ref as the key:
# ci.yml
group: ci-${{ github.ref }}
# embedding-regression.yml
group: embedding-regression-${{ github.ref }}This PR uses github.event.pull_request.number instead. For a workflow that only triggers on pull_request the two are functionally equivalent (both are unique per PR), so this will work correctly as written.
However, using github.ref would be more consistent with the rest of the repo and more resilient — if a push or workflow_dispatch trigger is ever added, github.event.pull_request.number would resolve to an empty string and every such run would share a single unnamed group, potentially cancelling unrelated jobs.
Consider aligning with the existing pattern:
| group: codegraph-impact-${{ github.event.pull_request.number }} | |
| group: codegraph-impact-${{ github.ref }} |
There was a problem hiding this comment.
Good catch — switched to github.ref to match the pattern used by ci.yml, embedding-regression.yml, and the other workflows. This is also more future-proof if additional triggers are ever added. Fixed in a9d00e0.
Summary
codegraph-impact.ymlworkflow was the only CI workflow missing aconcurrencygroupconcurrencywithcancel-in-progress: true, scoped per PR number — matching the pattern already used byci.yml,embedding-regression.yml, andpublish.ymlTest plan