[MINOR][CI] Stop unrelated PR comments from cancelling Delta and ANSI runs

[LuciferYang]

What changes are proposed in this pull request?

delta_spark_ut.yml triggers on pull_request and on issue_comment: types: [created], which fires for every comment posted on an issue or a pull request (review comments on the diff and commit comments use other events). For an issue_comment event github.event.issue.number is that issue's number, which for a comment on a PR is the PR's own number, so the comment run lands in the same concurrency group as that PR's in-flight pull_request run and cancel-in-progress: true kills it. The comment run then does not replace it: delta-test-requested skips anything that is not a /delta-test command, so the suite is lost and nothing takes its place.

The same shape is in velox_backend_ansi.yml: its group is ${{ github.repository }}-ansi-${{ github.event.issue.number || inputs.pr_number }} with cancel-in-progress: true, and its command test lives in jobs.check-comment.if, which is again evaluated after the group. Both files are fixed here the same way.

Fixes #12908.

Neither case is hypothetical. On #12902 I posted an unrelated review comment at 03:37:39Z. It started issue_comment run 33036933287 at 03:37:43Z, which cancelled pull_request run 33034774777 forty-three minutes into the tests. From shard 7's log:

2026-08-27T03:37:41.4970618Z [info] - merge-metrics: insert-only - Partitioned = true, CDF = false
2026-08-27T03:37:43.9917792Z context canceled
2026-08-27T03:37:44.0351639Z ##[error]The operation was canceled.

All eight shards and the aggregate job died within thirty seconds of each other, and the run that killed them concluded skipped. The signal lost was a real one: every failing test in that shard up to the cancellation was in the baseline at the time, so nothing had regressed and the shard was on course to pass.

For ANSI, @felipepessoto reproduced it deliberately on this PR while reviewing it: a /ansi-test comment at 05:38:58Z started run 33043165559, an unrelated follow-up comment at 05:39:40Z created run 33043202388 in the same group two seconds later, and the original run's build-native-lib was cancelled at 05:39:59Z while every job in the replacement was skipped. #12908 has the full trace.

The fix appends a discriminator to the group for that case only. When the comment is not a command the group gains a -comment-<run_id> suffix and the run therefore neither cancels nor queues; for every other trigger the appended segment evaluates to the empty string, so the group name stays byte-identical to what it is today and cancel-in-progress stays true. workflow_dispatch keeps its group too, so a manual ANSI re-dispatch still supersedes an older one for the same PR, which is one of the behaviours #12908 asked to preserve.

In each file the negated condition is the gate job's if repeated verbatim: delta-test-requested for Delta, check-comment for ANSI. One difference between the two is worth naming: check-comment tests the body with contains, not a first-token match, so a comment that merely mentions /ansi-test counts as a command. Copying the condition verbatim keeps the grouping and the gate in agreement on that, rather than making them disagree in a new way. Whether the contains test itself should be tightened is a separate question and not changed here.

Two alternatives I discarded, both of which I had implemented before settling here, in case a reviewer wonders why the expression is shaped this way.

Making cancel-in-progress conditional instead leaves the group alone but does not let the comment run proceed: runs in one group never overlap, so it sits pending until the whole suite finishes, then starts and skips anyway. It also occupies the group's single pending slot in the meantime.

Emitting a -shared suffix for the normal cases rather than an empty one reads better but renames the group for every trigger. pull_request runs use the workflow file from the PR head while issue_comment runs always use the default branch's, so for PRs already open at merge time a push run and a /delta-test run would sit in differently-named groups and stop superseding each other until the PR picks the change up. Appending nothing in those cases avoids the window entirely.

How was this patch tested?

The behaviour cannot be exercised from this PR. Per the issue_comment docs, "This event will only trigger a workflow run if the workflow file exists on the default branch", with GITHUB_REF set to the default branch, so a comment posted here runs main's copy of the workflow no matter what this branch says. That is also why the run that cancelled #12902's suite used the unfixed version. It takes effect once merged.

Three properties are worth checking mechanically rather than by eye, and I checked them for both files by parsing the YAML rather than reading it:

property	result
file parses, concurrency.cancel-in-progress	true, unchanged from main
group with the appended segment removed	byte-identical to main's group
text inside !( ... ) vs the gate job's if	equal after collapsing whitespace

Resolved group per trigger, worked through by hand from the precedence rules (&& binds tighter than ||, and both yield values): pull_request, workflow_dispatch and a real command comment all take the || '' branch and keep today's group; any other comment takes format('-comment-{0}', github.run_id). A comment on a plain issue also gets the suffix, since github.event.issue.pull_request is absent, which is harmless because such a run skips too.

I cannot run GitHub's expression evaluator locally, so evaluation itself rests on the docs plus the fact that each file's gate job already evaluates the identical condition on every comment. A malformed expression fails the run with a workflow syntax error rather than mis-grouping silently. If it evaluated to the wrong branch for pull_request events, the symptom would be a group suffixed with -comment-<id>, visible in the Actions UI, and the cost would be pushes no longer superseding older runs rather than a wrong result.

The Delta suite is green on this head: run 33148690783 on 6eec515cd, all eight shards plus the aggregate. Earlier heads of this branch showed four red shards for a reason unrelated to the change, recorded here so the history reads correctly: known-failures.txt listed 32 column-mapping tests as expected failures that had started passing, and fail_on_fixed resolves to true for pull_request events by design, so the gate reported them as NOW-PASSING. @felipepessoto removed exactly those 32 entries on main in #12907, so this branch does not touch the baseline.

One flake to know about, seen once in four complete runs on this branch: DeltaCDCStreamWithCatalogManagedBatch100Suite#maxFilesPerTrigger with Trigger.AvailableNow respects read limits was reported as a regression by run 33044870485 and by none of 33043547622, 33141742435 or 33148690783. flaky-tests.txt has no active entries, so if it recurs it will red a shard.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude claude-opus-5

[Copilot]

Pull request overview

This PR fixes an Actions concurrency edge case in delta_spark_ut.yml where any issue_comment event (including normal review comments) could land in the same concurrency group as a PR’s in-flight Delta Spark UT run and cancel it due to cancel-in-progress: true. The change makes cancellation conditional so only a real /delta-test command comment can cancel an in-flight run for that PR, preserving the intended “new push cancels old run” and “explicit rerun command cancels old run” behavior.

Changes:

• Replace unconditional concurrency.cancel-in-progress: true with a conditional expression that only enables cancellation for real /delta-test comment commands.
• Keep the concurrency group key unchanged, but align the cancellation condition with the existing jobs.delta-test-requested.if logic to avoid unintended cancellations.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[felipepessoto]

Thanks for fixing it @LuciferYang. I wonder if the same happens to .github/workflows/velox_backend_ansi.yml. If I start a /ansi-test, any following comment in the same PR would cancel it?

I think we don't have a good way to tests your changes before merging it. But it looks good to me

[github-actions]

🔄 ANSI mode analysis started by @felipepessoto. View run

[felipepessoto]

Test checking if ansi is cancelled

UPDATE: yes it does: https://github.com/apache/gluten/actions/runs/33043165559

[github-actions]

ANSI Mode Test Analysis Report (Spark 4.1)

Note

Expression-level ANSI mode offload coverage analysis.
Test config: spark.sql.ansi.enabled=true, spark.gluten.sql.ansiFallback.enabled=false.

• Passed (🟢): Velox correctly handles ANSI semantics
• Fallback (🔴): Expression falls back to Spark execution, needs ANSI support in Velox
• Failed (🟡): Velox executes but ANSI error behavior differs from Spark, needs exception handling fix

ANSI Offload suites: 0 tests, 0 records | Other suites: 0 tests

ANSI Offload

Overview (ANSI Offload Expression Records)

Classification	Count	%

[felipepessoto]

Created the issue #12908 in case you are interested in fixing it as well

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[LuciferYang]

Good catch, thanks. Fixed in this PR too, at .github/workflows/velox_backend_ansi.yml, and the title now covers both workflows.

I took the second of your two options for both files, for the reason you gave: a unique group fully isolates the unrelated comment, while conditioning cancel-in-progress only downgrades the damage. Runs in one group never overlap, so with cancellation off the comment run does not proceed alongside the ANSI run, it sits pending until that run finishes and then skips anyway, holding the group's single pending slot in the meantime. I had it that way first and replaced it.

The suffix is appended only when the comment is not a command, so for workflow_dispatch, for pull_request and for a real /ansi-test or /ansi-analyze the group name stays byte-identical to what main has today. That matters beyond tidiness: pull_request runs use the workflow file from the PR head while issue_comment runs always use the default branch's, so renaming the group for every trigger would have put PRs that are already open into a window where a push run and a command run sit in different groups and stop superseding each other. Appending nothing in those cases avoids it.

The negated condition is check-comment's if copied verbatim, so the grouping and the gate cannot disagree. That includes its contains test, which means a comment merely mentioning /ansi-test counts as a command in both places. Whether that test should be tightened to a first-token match, the way delta-test-requested does it, seemed like a separate question from this one, so I left it alone.

Three properties are checkable without running anything, and I verified them for both files by parsing the YAML rather than reading it: cancel-in-progress is still true, removing the appended ${{ }} segment leaves a group string equal to main's, and the text inside !( ... ) equals the gate job's if after collapsing whitespace.

You are right that neither fix can be exercised before merging. issue_comment only ever runs the default branch's copy of a workflow, which is also why your reproduction ran main's unfixed version.

[github-actions]

🔄 ANSI mode analysis started by @LuciferYang. View run

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[LuciferYang]

cc @jackylee-ch CI paased

[jackylee-ch]

👍

[LuciferYang]

Thank you @jackylee-ch