[Experimental PR] Benchmark any Frontier Release on the CI #1090
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoves most GitHub Actions workflows and Phoenix SLURM helper scripts, and adds a Frontier-only benchmark workflow triggered by tag/dispatch. No source-level public or exported API declarations were changed. (28 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Trigger (manual/tag)
participant GH as GitHub Actions (Frontier runner)
participant PR as PR repo (checkout)
participant Master as Master repo (checkout@tag)
participant Build as Build script (mfc.sh)
participant Bench as Benchmark runner
participant Art as Artifact storage
Note over User,GH: workflow_dispatch with tag input
User->>GH: dispatch(tag)
GH->>PR: checkout PR repo (path=pr)
GH->>Master: checkout master@tag (path=master)
GH->>Master: prepare build context / copy sources
GH->>Build: run master build (mfc.sh)
Build-->>GH: build logs/artifacts
GH->>Bench: run benchmarks (master build on Frontier)
Bench-->>GH: benchmark logs/results
GH->>Art: upload archived logs/artifacts
Art-->>User: artifacts available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-24T21:50:46.909ZApplied to files:
🪛 actionlint (1.7.9).github/workflows/bench-frontier.yml28-28: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file (runner-label) 36-36: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file (runner-label) 63-63: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue (action) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
codeant-ai
bot
added
the
size:L
|
CodeAnt AI finished reviewing your PR. |
💡 Enhance Your PR ReviewsWe noticed that 3 feature(s) are not configured for this repository. Enabling these features can help improve your code quality and workflow: 🚦 Quality GatesStatus: Quality Gates are not enabled at the organization level 🎫 Jira Ticket ComplianceStatus: Jira credentials file not found. Please configure Jira integration in your settings ⚙️ Custom RulesStatus: No custom rules configured. Add rules via organization settings or .codeant/review.json in your repository Want to enable these features? Contact your organization admin or check our documentation for setup instructions. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
.github/workflows/test.yml
Outdated
There was a problem hiding this comment.
High-level Suggestion
The PR's removal of most CI workflows to support historical benchmarking is a major issue. The new method uses fragile sed and cp commands; a better approach is using containerization for reproducible builds without sacrificing CI quality. [High-level, importance: 10]
Solution Walkthrough:
Before:
# .github/workflows/bench.yml
on:
workflow_dispatch:
inputs:
tag: required
jobs:
benchmark:
steps:
- checkout PR code to 'pr/'
- checkout code for specified 'tag' to 'master/'
# Manually patch the old code in 'master/'
- run: |
cp pr/some_new_script.sh master/
cp pr/some_new_config.patch master/
sed -i 's/old_thing/new_thing/' master/some_config_file
sed -i '/# START_BLOCK/,/# END_BLOCK/{...}' master/CMakeLists.txt
- name: Build & Bench
run: (cd master && ./build.sh)
After:
# .github/workflows/bench.yml
# (Keep existing CI for PRs, e.g., test.yml, lint.yml)
# For historical benchmarking:
on:
workflow_dispatch:
inputs:
tag: required
jobs:
benchmark:
steps:
- name: Build and push container for the specific tag
uses: docker/build-push-action@v6
with:
context: .
# Dockerfile would handle version-specific dependencies
file: ./.github/dockerfiles/Dockerfile.${{ inputs.tag }}
push: true
tags: my-repo/mfc:${{ inputs.tag }}
- name: Run benchmark using the container
run: docker run my-repo/mfc:${{ inputs.tag }} ./run_benchmark.sh
| - name: Clone - PR | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| path: pr | ||
| path: pr | ||
|
|
||
| - name: Clone - Master | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: MFlowCode/MFC | ||
| ref: master | ||
| ref: ${{ github.event.inputs.tag }} | ||
| path: master | ||
|
|
||
| - name: Checkout PR repo | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| path: pr |
There was a problem hiding this comment.
Suggestion: Remove the redundant actions/checkout@v3 step for the pr directory in the bench.yml workflow, as it is already checked out by a preceding actions/checkout@v4 step. [general, importance: 6]
| - name: Clone - PR | |
| uses: actions/checkout@v4 | |
| with: | |
| path: pr | |
| path: pr | |
| - name: Clone - Master | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: MFlowCode/MFC | |
| ref: master | |
| ref: ${{ github.event.inputs.tag }} | |
| path: master | |
| - name: Checkout PR repo | |
| uses: actions/checkout@v3 | |
| with: | |
| path: pr | |
| - name: Clone - PR | |
| uses: actions/checkout@v4 | |
| with: | |
| path: pr | |
| - name: Clone - Master | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: MFlowCode/MFC | |
| ref: ${{ github.event.inputs.tag }} | |
| path: master |
.github/workflows/release.yml
Outdated
| - name: Dispatch bench.yml for each tag | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const tags = '${{ steps.tags.outputs.tags }}'.split(' ').filter(Boolean); | ||
| for (const tag of tags) { | ||
| await github.rest.actions.createWorkflowDispatch({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'bench.yml', | ||
| ref: context.ref.replace('refs/heads/', ''), | ||
| inputs: { tag } | ||
| }); | ||
|
|
||
| let runId = null; | ||
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | ||
| const runs = await github.rest.actions.listWorkflowRuns({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'bench.yml', | ||
| event: 'workflow_dispatch', | ||
| per_page: 5 | ||
| }); | ||
| // Find the most recent queued or in_progress run | ||
| const found = runs.data.workflow_runs.find(run => | ||
| ['queued', 'in_progress'].includes(run.status) | ||
| ); | ||
| if (found) { | ||
| runId = found.id; | ||
| break; | ||
| } | ||
| await new Promise(resolve => setTimeout(resolve, 5000)); | ||
| } | ||
| if (!runId) { | ||
| core.setFailed(`Could not find workflow run for tag ${tag}`); | ||
| return; | ||
| } | ||
|
|
||
| let completed = false; | ||
| while (!completed) { | ||
| const run = await github.rest.actions.getWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: runId | ||
| }); | ||
| if (run.data.status === 'completed') { | ||
| if (run.data.conclusion !== 'success') { | ||
| core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`); | ||
| return; | ||
| } | ||
| completed = true; | ||
| } else { | ||
| await new Promise(resolve => setTimeout(resolve, 15000)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggestion: To prevent a race condition in the release.yml workflow, filter the dispatched workflow runs by actor: context.actor. This ensures the script monitors the correct run, even if multiple workflows are triggered simultaneously. [possible issue, importance: 8]
| - name: Dispatch bench.yml for each tag | |
| uses: actions/github-script@v7 | |
| with: | |
| script: | | |
| const tags = '${{ steps.tags.outputs.tags }}'.split(' ').filter(Boolean); | |
| for (const tag of tags) { | |
| await github.rest.actions.createWorkflowDispatch({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'bench.yml', | |
| ref: context.ref.replace('refs/heads/', ''), | |
| inputs: { tag } | |
| }); | |
| let runId = null; | |
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | |
| const runs = await github.rest.actions.listWorkflowRuns({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'bench.yml', | |
| event: 'workflow_dispatch', | |
| per_page: 5 | |
| }); | |
| // Find the most recent queued or in_progress run | |
| const found = runs.data.workflow_runs.find(run => | |
| ['queued', 'in_progress'].includes(run.status) | |
| ); | |
| if (found) { | |
| runId = found.id; | |
| break; | |
| } | |
| await new Promise(resolve => setTimeout(resolve, 5000)); | |
| } | |
| if (!runId) { | |
| core.setFailed(`Could not find workflow run for tag ${tag}`); | |
| return; | |
| } | |
| let completed = false; | |
| while (!completed) { | |
| const run = await github.rest.actions.getWorkflowRun({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| run_id: runId | |
| }); | |
| if (run.data.status === 'completed') { | |
| if (run.data.conclusion !== 'success') { | |
| core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`); | |
| return; | |
| } | |
| completed = true; | |
| } else { | |
| await new Promise(resolve => setTimeout(resolve, 15000)); | |
| } | |
| } | |
| } | |
| - name: Dispatch bench.yml for each tag | |
| uses: actions/github-script@v7 | |
| with: | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | |
| script: | | |
| const tags = '${{ steps.tags.outputs.tags }}'.split(' ').filter(Boolean); | |
| for (const tag of tags) { | |
| await github.rest.actions.createWorkflowDispatch({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'bench.yml', | |
| ref: context.ref.replace('refs/heads/', ''), | |
| inputs: { tag } | |
| }); | |
| let runId = null; | |
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | |
| await new Promise(resolve => setTimeout(resolve, 5000)); | |
| const runs = await github.rest.actions.listWorkflowRuns({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| workflow_id: 'bench.yml', | |
| event: 'workflow_dispatch', | |
| actor: context.actor, | |
| per_page: 5 | |
| }); | |
| // Find the most recent queued or in_progress run initiated by this actor | |
| const found = runs.data.workflow_runs.find(run => | |
| ['queued', 'in_progress'].includes(run.status) | |
| ); | |
| if (found) { | |
| runId = found.id; | |
| break; | |
| } | |
| } | |
| if (!runId) { | |
| core.setFailed(`Could not find workflow run for tag ${tag}`); | |
| return; | |
| } | |
| let completed = false; | |
| while (!completed) { | |
| await new Promise(resolve => setTimeout(resolve, 15000)); | |
| const run = await github.rest.actions.getWorkflowRun({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| run_id: runId | |
| }); | |
| if (run.data.status === 'completed') { | |
| if (run.data.conclusion !== 'success') { | |
| core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`); | |
| return; | |
| } | |
| completed = true; | |
| } | |
| } | |
| } |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 19 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/workflows/bench.yml">
<violation number="1" location=".github/workflows/bench.yml:71">
P0: Reference to non-existent `mfc/` directory. The workflow checks out to `pr/` and `master/` but these lines try to copy from `mfc/` which doesn't exist. This will cause the workflow to fail.</violation>
<violation number="2" location=".github/workflows/bench.yml:125">
P0: Missing required `${{ matrix.interface }}` argument. The `run_parallel_benchmarks.sh` script requires exactly 3 arguments (device, interface, cluster) and will fail with exit code 1 when only 2 are provided.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
.github/workflows/bench-frontier.yml
Outdated
| sudo mkdir -p master | ||
| sudo chmod 777 master | ||
| rm -rf master/* | ||
| cp -r mfc/* master/ |
There was a problem hiding this comment.
P0: Reference to non-existent mfc/ directory. The workflow checks out to pr/ and master/ but these lines try to copy from mfc/ which doesn't exist. This will cause the workflow to fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/bench.yml, line 71:
<comment>Reference to non-existent `mfc/` directory. The workflow checks out to `pr/` and `master/` but these lines try to copy from `mfc/` which doesn't exist. This will cause the workflow to fail.</comment>
<file context>
@@ -81,44 +41,98 @@ jobs:
+ sudo mkdir -p master
+ sudo chmod 777 master
+ rm -rf master/*
+ cp -r mfc/* master/
+ cp -r mfc/.git master/.git
+ cp pr/.github/Dockerfile master/
</file context>
✅ Addressed in 2a7df55
.github/workflows/bench-frontier.yml
Outdated
| run: | | ||
| (cd pr && . ./mfc.sh load -c ${{ matrix.flag }} -m g) | ||
| (cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml) | ||
| run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }} |
There was a problem hiding this comment.
P0: Missing required ${{ matrix.interface }} argument. The run_parallel_benchmarks.sh script requires exactly 3 arguments (device, interface, cluster) and will fail with exit code 1 when only 2 are provided.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/bench.yml, line 125:
<comment>Missing required `${{ matrix.interface }}` argument. The `run_parallel_benchmarks.sh` script requires exactly 3 arguments (device, interface, cluster) and will fail with exit code 1 when only 2 are provided.</comment>
<file context>
@@ -81,44 +41,98 @@ jobs:
- run: |
- (cd pr && . ./mfc.sh load -c ${{ matrix.flag }} -m g)
- (cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml)
+ run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }}
- name: Print Logs
</file context>
| run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }} | |
| run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.interface }} ${{ matrix.cluster }} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/bench.yml(3 hunks).github/workflows/cleanliness.yml(0 hunks).github/workflows/coverage.yml(0 hunks).github/workflows/deploy-tap.yml(0 hunks).github/workflows/docker.yml(0 hunks).github/workflows/docs.yml(0 hunks).github/workflows/formatting.yml(0 hunks).github/workflows/homebrew.yml(0 hunks).github/workflows/line-count.yml(0 hunks).github/workflows/lint-source.yml(0 hunks).github/workflows/lint-toolchain.yml(0 hunks).github/workflows/phoenix/bench.sh(0 hunks).github/workflows/phoenix/submit-bench.sh(0 hunks).github/workflows/phoenix/submit.sh(0 hunks).github/workflows/phoenix/test.sh(0 hunks).github/workflows/pmd.yml(0 hunks).github/workflows/release.yml(1 hunks).github/workflows/spelling.yml(0 hunks).github/workflows/test.yml(0 hunks)
💤 Files with no reviewable changes (17)
- .github/workflows/phoenix/submit.sh
- .github/workflows/docs.yml
- .github/workflows/line-count.yml
- .github/workflows/phoenix/submit-bench.sh
- .github/workflows/pmd.yml
- .github/workflows/lint-source.yml
- .github/workflows/spelling.yml
- .github/workflows/deploy-tap.yml
- .github/workflows/test.yml
- .github/workflows/coverage.yml
- .github/workflows/cleanliness.yml
- .github/workflows/docker.yml
- .github/workflows/phoenix/bench.sh
- .github/workflows/phoenix/test.sh
- .github/workflows/lint-toolchain.yml
- .github/workflows/homebrew.yml
- .github/workflows/formatting.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: After each successful build step, run focused tests using `./mfc.sh test -j $(nproc) -f EA8FA07E -t 9E2CA336` instead of running all tests
Applied to files:
.github/workflows/bench.yml
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench.yml
🪛 actionlint (1.7.9)
.github/workflows/bench.yml
54-54: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/bench.yml (1)
79-83: The sed pattern for cantera pinning is ineffective and won't modify any dependencies.The sed command on line 81 searches for
"cantera",(without version specification), but:
- In the PR's pyproject.toml: cantera is specified as
"cantera>=3.1.0",(different format)- In releases v4.8.0–v5.1.3: cantera dependency doesn't exist in their pyproject.toml files
As written, the sed command is a no-op and never actually pins any version. Either the pattern should match the actual dependency format (e.g.,
s/"cantera[^"]*",/"cantera==3.0.1",/g), or this sed command can be removed as it has no effect.Likely an incorrect or invalid review comment.
.github/workflows/release.yml
Outdated
| from_tag: | ||
| description: 'Start tag (e.g. v4.3.0)' | ||
| required: false | ||
| default: 'v5.1.3' | ||
| to_tag: | ||
| description: 'End tag (e.g. v5.1.3)' | ||
| required: false | ||
| default: 'v5.1.3' |
There was a problem hiding this comment.
Clarify input descriptions to match defaults.
The from_tag input description says "(e.g. v4.3.0)" but the default is 'v5.1.3'. For consistency, either:
- Update the description to match the default range (e.g., "e.g., v5.1.0"), or
- Adjust the default if v4.3.0 is the intended start of the benchmark range.
The PR summary mentions benchmarking v4.8.0–v5.1.3, so the description may be outdated.
from_tag:
- description: 'Start tag (e.g. v4.3.0)'
+ description: 'Start tag (e.g. v4.8.0)'
required: false
default: 'v5.1.3'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from_tag: | |
| description: 'Start tag (e.g. v4.3.0)' | |
| required: false | |
| default: 'v5.1.3' | |
| to_tag: | |
| description: 'End tag (e.g. v5.1.3)' | |
| required: false | |
| default: 'v5.1.3' | |
| from_tag: | |
| description: 'Start tag (e.g. v4.8.0)' | |
| required: false | |
| default: 'v5.1.3' | |
| to_tag: | |
| description: 'End tag (e.g. v5.1.3)' | |
| required: false | |
| default: 'v5.1.3' |
🤖 Prompt for AI Agents
In .github/workflows/release.yml around lines 10–17 the from_tag/to_tag input
descriptions show examples that don’t match the configured defaults or the PR
benchmark range; update the descriptions and/or defaults so they are consistent.
Specifically either change the example in from_tag to match the default (e.g.,
"e.g., v5.1.3") or, if the intended benchmark range is v4.8.0–v5.1.3, set
from_tag default to 'v4.8.0' and adjust the description examples for both inputs
to reflect that range (e.g., "e.g., v4.8.0" and "e.g., v5.1.3").
.github/workflows/release.yml
Outdated
| let runId = null; | ||
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | ||
| const runs = await github.rest.actions.listWorkflowRuns({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'bench.yml', | ||
| event: 'workflow_dispatch', | ||
| per_page: 5 | ||
| }); | ||
| // Find the most recent queued or in_progress run | ||
| const found = runs.data.workflow_runs.find(run => | ||
| ['queued', 'in_progress'].includes(run.status) | ||
| ); | ||
| if (found) { | ||
| runId = found.id; | ||
| break; | ||
| } | ||
| await new Promise(resolve => setTimeout(resolve, 5000)); | ||
| } | ||
| if (!runId) { | ||
| core.setFailed(`Could not find workflow run for tag ${tag}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "release.yml" -o -name "release.yaml" | head -20Repository: MFlowCode/MFC
Length of output: 170
🏁 Script executed:
fd -e yml -e yaml | grep -E "release|workflow" | head -20Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
git ls-files .github/workflows/ 2>/dev/null || ls -la .github/workflows/ 2>/dev/null || echo "Checking alternative paths..."Repository: MFlowCode/MFC
Length of output: 301
🏁 Script executed:
cat -n .github/workflows/release.ymlRepository: MFlowCode/MFC
Length of output: 3882
Fix run detection to match the correct dispatched run and handle PR contexts.
The current logic has three critical issues:
-
Race condition: When dispatching multiple tags sequentially (lines 42–68), if
tag1's run is still queued whentag2is dispatched, the code detectstag1's run again instead of waiting fortag2's run. ThelistWorkflowRunscall doesn't filter by thetaginput passed to dispatch. -
Missing tag validation: Although
inputs: { tag }is sent with the dispatch (line 47), the run detection (lines 60–62) only checks status and doesn't verify the run matches the current tag. Multiple concurrent or sequential dispatches can cause cross-tag mismatches. -
Broken ref handling for PR contexts (line 46): Using
context.ref.replace('refs/heads/', '')fails for pull requests wherecontext.refisrefs/pull/XXX/merge. This leaves an invalid ref that will cause dispatch to fail.
Filter runs by both status and inputs.tag to ensure the correct dispatched run is detected, and use the full context.ref or detect PR contexts separately for the dispatch parameter.
.github/workflows/release.yml
Outdated
| let completed = false; | ||
| while (!completed) { | ||
| const run = await github.rest.actions.getWorkflowRun({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: runId | ||
| }); | ||
| if (run.data.status === 'completed') { | ||
| if (run.data.conclusion !== 'success') { | ||
| core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`); | ||
| return; | ||
| } | ||
| completed = true; | ||
| } else { | ||
| await new Promise(resolve => setTimeout(resolve, 15000)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the release.yml file
find . -name "release.yml" -type f | head -20Repository: MFlowCode/MFC
Length of output: 88
🏁 Script executed:
# Read the release.yml file to understand the full context around lines 74-90
cat -n .github/workflows/release.ymlRepository: MFlowCode/MFC
Length of output: 3882
Add timeout to run completion polling and clarify failure behavior.
The while loop (lines 74–90) has no maximum iteration count or timeout. If a dispatched run hangs indefinitely, this workflow will also hang, consuming the runner. Add a safety timeout and consider whether stopping on the first failure is the intended behavior (e.g., should all tags be attempted even if one fails?).
Consider revising the completion polling to include a timeout:
let completed = false;
let maxWaitTime = 24 * 60 * 60 * 1000; // 24 hours in milliseconds
let elapsed = 0;
const pollInterval = 15000; // 15 seconds
while (!completed) {
const run = await github.rest.actions.getWorkflowRun({...});
if (run.data.status === 'completed') {
if (run.data.conclusion !== 'success') {
core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`);
return; // Consider: should this stop the entire job, or just this tag?
}
completed = true;
} else {
elapsed += pollInterval;
if (elapsed > maxWaitTime) {
core.setFailed(`Workflow for tag ${tag} timed out after 24 hours`);
return;
}
await new Promise(resolve => setTimeout(resolve, pollInterval));
}
}Also document whether the all-or-nothing failure mode is intentional (stop on first failure) or if all tags should be attempted.
Malmahrouqi3
force-pushed
the
benchmark-all-releases-on-frontier-runner
branch
from
e2af6f8 to
2a7df55
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes most existing CI workflows and introduces a new experimental release.yml dispatcher to benchmark Frontier releases (v4.8.0-v5.1.3 for GPU-ACC and v5.1.0-v5.1.3 for GPU-MP) by orchestrating sequential runs of a simplified bench.yml workflow. The PR focuses on enabling historical release benchmarking without containers, using the existing Frontier CI infrastructure.
Key Changes
- Removed 15+ existing CI workflows (test, lint, docs, coverage, etc.)
- Added new
release.ymlworkflow dispatcher for sequential tag-based benchmarking - Simplified
bench.ymlto benchmark a single release tag on Frontier only - Removed Phoenix-specific test and benchmark scripts
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/test.yml |
Deleted entire test suite workflow (GitHub/Phoenix/Frontier testing) |
.github/workflows/spelling.yml |
Deleted spell check workflow |
.github/workflows/release.yml |
Added dispatcher to orchestrate benchmarks across release tags |
.github/workflows/pmd.yml |
Deleted source code duplication detection workflow |
.github/workflows/phoenix/test.sh |
Deleted Phoenix test execution script |
.github/workflows/phoenix/submit.sh |
Deleted Phoenix job submission script |
.github/workflows/phoenix/submit-bench.sh |
Deleted Phoenix benchmark submission script |
.github/workflows/phoenix/bench.sh |
Deleted Phoenix benchmark execution script |
.github/workflows/lint-toolchain.yml |
Deleted toolchain linting workflow |
.github/workflows/lint-source.yml |
Deleted source code linting workflow |
.github/workflows/line-count.yml |
Deleted lines of code tracking workflow |
.github/workflows/homebrew.yml |
Deleted Homebrew formula testing workflow |
.github/workflows/formatting.yml |
Deleted code formatting check workflow |
.github/workflows/docs.yml |
Deleted documentation build/publish workflow |
.github/workflows/docker.yml |
Deleted container image build workflow |
.github/workflows/deploy-tap.yml |
Deleted Homebrew tap deployment workflow |
.github/workflows/coverage.yml |
Deleted code coverage tracking workflow |
.github/workflows/cleanliness.yml |
Deleted compiler warning cleanliness workflow |
.github/workflows/bench.yml |
Simplified to benchmark single Frontier release tag |
.github/workflows/release.yml
Outdated
| workflow_dispatch: | ||
| inputs: | ||
| from_tag: | ||
| description: 'Start tag (e.g. v4.3.0)' |
There was a problem hiding this comment.
The version format should use 'v5.1.3' consistently. However, line 11 shows 'v4.3.0' as an example in the description which doesn't match the stated range of v4.8.0-v5.1.3 mentioned in the PR description.
| description: 'Start tag (e.g. v4.3.0)' | |
| description: 'Start tag (e.g. v4.8.0)' |
.github/workflows/release.yml
Outdated
| }); | ||
|
|
||
| let runId = null; | ||
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds |
There was a problem hiding this comment.
The comment states 'up to 300 seconds' but the loop runs 60 times with a 5-second delay (line 67), which totals 300 seconds. However, if the workflow is found early, the timeout is not 300 seconds. The comment should clarify this is the maximum timeout.
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | |
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds (maximum), exit earlier if found |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
74-90: Add timeout to run completion polling to prevent indefinite hangs.Lines 74–90 poll for run completion in an infinite loop with no maximum iteration count or timeout. If a dispatched run hangs indefinitely, this workflow will also hang and consume the runner.
Add a timeout safety check:
let completed = false; + let maxWaitTime = 24 * 60 * 60 * 1000; // 24 hours in milliseconds + let startTime = Date.now(); while (!completed) { const run = await github.rest.actions.getWorkflowRun({ owner: 'Malmahrouqi3', repo: 'MFC-mo2', run_id: runId }); if (run.data.status === 'completed') { if (run.data.conclusion !== 'success') { core.setFailed(`Workflow for tag ${tag} failed with conclusion: ${run.data.conclusion}`); return; } completed = true; } else { + if (Date.now() - startTime > maxWaitTime) { + core.setFailed(`Workflow for tag ${tag} timed out after 24 hours`); + return; + } await new Promise(resolve => setTimeout(resolve, 15000)); } }
🧹 Nitpick comments (2)
.github/workflows/bench-frontier.yml (1)
39-40: Update deprecated Node.js version to a supported LTS release.Lines 39–40 force the workflow to use Node 16, which is now outdated. GitHub Actions recommends Node 20 or 22 for modern runtime compatibility and security.
Apply this diff:
- ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 + ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node20 - ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: trueThis also removes the insecure flag, which is no longer needed with a current Node version.
.github/workflows/release.yml (1)
10-17: Align input descriptions and defaults with PR benchmarking range.Line 11 shows example "v4.3.0" but the default is "v5.1.3", and the PR summary states the benchmarking range is v4.8.0–v5.1.3. The mismatch creates confusion about the intended default behavior.
Apply this diff to align the descriptions with the stated benchmarking range:
from_tag: - description: 'Start tag (e.g. v4.3.0)' + description: 'Start tag (e.g. v4.8.0)' required: false default: 'v5.1.3'Alternatively, if v4.8.0 is the intended default start, update line 13 to
default: 'v4.8.0'as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/bench-frontier.yml(1 hunks).github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench-frontier.yml
🪛 actionlint (1.7.9)
.github/workflows/bench-frontier.yml
21-21: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
55-55: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/bench-frontier.yml (1)
128-139: Verify artifact archival paths match expected benchmark outputs.Lines 133–139 archive logs and benchmark results using wildcard paths. Ensure that:
master/bench-${{ matrix.device }}.*actually produces files namedbench-gpu.*(or similar).master/build/benchmarks/*exists and contains the expected output.If the benchmark script doesn't produce these files, the archival will silently skip them. Consider adding a verification step beforehand.
.github/workflows/bench-frontier.yml
Outdated
| - name: Replace old SILO block in CMakeLists.txt | ||
| run: | | ||
| if [ -f master/toolchain/pyproject.toml ]; then | ||
| sed -i 's/"cantera",/"cantera==3.0.1",/g' master/toolchain/pyproject.toml | ||
| fi | ||
|
|
||
| sed -i '/# SILO/,/^endif()/{ | ||
| r /dev/stdin | ||
| d | ||
| }' master/toolchain/dependencies/CMakeLists.txt <<'EOF' | ||
| # SILO | ||
| if (MFC_SILO) | ||
| find_package(SILO) | ||
| if (SILO_FOUND) | ||
| message(STATUS "SILO found.") | ||
| add_custom_target(silo) | ||
| else() | ||
| find_package(Git REQUIRED) | ||
|
|
||
| ExternalProject_Add(silo | ||
| GIT_REPOSITORY "https://github.com/LLNL/Silo" | ||
| GIT_TAG 9af504ef4fb79153e1fbf3bdb75421b6b65f6dc4 | ||
| GIT_PROGRESS ON | ||
| PATCH_COMMAND "${GIT_EXECUTABLE}" stash | ||
| && "${GIT_EXECUTABLE}" apply "${CMAKE_SOURCE_DIR}/Silo.patch" | ||
| CMAKE_ARGS -DSILO_ENABLE_SHARED=OFF | ||
| -DSILO_ENABLE_SILOCK=OFF | ||
| -DSILO_ENABLE_BROWSER=OFF | ||
| -DFIND_LIBRARY_USE_LIB64_PATHS=ON | ||
| "-DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH}" | ||
| "-DCMAKE_FIND_ROOT_PATH=${CMAKE_FIND_ROOT_PATH}" | ||
| "-DCMAKE_FIND_PACKAGE_REDIRECTS_DIR=${CMAKE_FIND_PACKAGE_REDIRECTS_DIR}" | ||
| "-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}" | ||
| "$<$<STREQUAL:${CMAKE_Fortran_COMPILER_ID},Cray>:-DCMAKE_MODULE_PATH=${CMAKE_SOURCE_DIR}/../cmake/cce>" | ||
| ) | ||
| endif() | ||
| endif() | ||
| EOF | ||
| docker buildx create --name mfcbuilder --driver docker-container --use |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the CMakeLists.txt file and check SILO block
fd -t f "CMakeLists.txt" --path "*/toolchain/dependencies/*" | head -20Repository: MFlowCode/MFC
Length of output: 290
🏁 Script executed:
# Search for SILO block pattern in CMakeLists files
rg "# SILO" -A 5 --type cmakeRepository: MFlowCode/MFC
Length of output: 418
🏁 Script executed:
# Check for Silo.patch file location
fd -t f "Silo.patch"Repository: MFlowCode/MFC
Length of output: 90
🏁 Script executed:
# Check pyproject.toml for cantera reference
fd -t f "pyproject.toml" --path "*/toolchain/*"Repository: MFlowCode/MFC
Length of output: 290
🏁 Script executed:
# Verify the pattern in the workflow file itself
cat -n .github/workflows/bench-frontier.yml | sed -n '79,117p'Repository: MFlowCode/MFC
Length of output: 2266
🏁 Script executed:
# Get the full SILO block to verify it ends with endif()
rg "# SILO" -A 50 --type cmake toolchain/dependencies/CMakeLists.txt | head -60Repository: MFlowCode/MFC
Length of output: 2114
🏁 Script executed:
# Check the exact structure around the SILO block
cat toolchain/dependencies/CMakeLists.txt | grep -n "SILO" -A 40 | head -50Repository: MFlowCode/MFC
Length of output: 1735
🏁 Script executed:
# Verify the directory structure for the patch file
ls -la toolchain/dependencies/ | grep -i patchRepository: MFlowCode/MFC
Length of output: 116
🏁 Script executed:
# Check if there's a pyproject.toml in toolchain/
ls -la toolchain/ | grep -i pyprojectRepository: MFlowCode/MFC
Length of output: 121
🏁 Script executed:
# Check if there's a Silo.patch at the root level
find . -name "Silo.patch" -type fRepository: MFlowCode/MFC
Length of output: 92
🏁 Script executed:
# Check the pyproject.toml content to understand Cantera requirement
cat toolchain/pyproject.tomlRepository: MFlowCode/MFC
Length of output: 1196
🏁 Script executed:
# Check how other patches are referenced in CMakeLists.txt
rg "PATCH_COMMAND" -B 5 -A 2 --type cmakeRepository: MFlowCode/MFC
Length of output: 741
Fix two critical issues in the SILO replacement and Cantera version pinning.
-
Silo.patch path is incorrect (line 103): The patch file is located at
toolchain/dependencies/Silo.patch, but the sed replacement uses"${CMAKE_SOURCE_DIR}/Silo.patch", which resolves tomaster/Silo.patch. The PATCH_COMMAND will fail because the file doesn't exist at the root. Change to"${CMAKE_SOURCE_DIR}/toolchain/dependencies/Silo.patch". -
Cantera sed pattern won't match (line 82): The sed pattern
s/"cantera",/"cantera==3.0.1",/gsearches for"cantera",but the actual pyproject.toml contains"cantera>=3.1.0",. The pattern won't match, so the replacement won't occur and the version won't be pinned to 3.0.1.
🤖 Prompt for AI Agents
.github/workflows/bench-frontier.yml lines 79–117: two fixes — update the
PATCH_COMMAND path to point to the actual patch at
toolchain/dependencies/Silo.patch instead of "${CMAKE_SOURCE_DIR}/Silo.patch",
and broaden the sed pattern used to pin Cantera so it matches the existing
version-specifier (e.g. "cantera>=3.1.0",) rather than only "cantera",; change
the sed invocation to match any "cantera..." token and replace it with
"cantera==3.0.1" so the version is actually pinned.
.github/workflows/release.yml
Outdated
| - name: Get tags in range | ||
| id: tags | ||
| run: | | ||
| git clone --quiet https://github.com/MFlowCode/MFC.git mfc | ||
| cd mfc | ||
| git fetch --tags | ||
| TAGS=$(git tag --sort=creatordate) | ||
| TAGS_IN_RANGE=$(awk "/^${{ env.FROM_TAG }}$/,/^${{ env.TO_TAG }}$/" <<< "$TAGS" | grep -v '^$') | ||
| echo "tags=$(echo $TAGS_IN_RANGE | tr '\n' ' ')" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/release.yml | head -50Repository: MFlowCode/MFC
Length of output: 1930
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '35,80p'Repository: MFlowCode/MFC
Length of output: 2172
🏁 Script executed:
#!/bin/bash
# Simulate tag list (sorted by date)
TAGS=$(cat <<'EOF'
v5.0.0
v5.1.0
v5.1.1
v5.1.2
v5.1.3
v5.2.0
EOF
)
echo "=== Test 1: Both tags exist ==="
FROM_TAG="v5.1.1"
TO_TAG="v5.1.3"
RESULT=$(awk "/^$FROM_TAG$/,/^$TO_TAG$/" <<< "$TAGS" | grep -v '^$')
echo "Result: '$RESULT'"
echo "Empty: $([ -z "$RESULT" ] && echo 'YES' || echo 'NO')"
echo ""
echo "=== Test 2: FROM_TAG doesn't exist ==="
FROM_TAG="v5.0.5"
TO_TAG="v5.1.3"
RESULT=$(awk "/^$FROM_TAG$/,/^$TO_TAG$/" <<< "$TAGS" | grep -v '^$')
echo "Result: '$RESULT'"
echo "Empty: $([ -z "$RESULT" ] && echo 'YES' || echo 'NO')"
echo ""
echo "=== Test 3: TO_TAG doesn't exist ==="
FROM_TAG="v5.1.1"
TO_TAG="v5.3.0"
RESULT=$(awk "/^$FROM_TAG$/,/^$TO_TAG$/" <<< "$TAGS" | grep -v '^$')
echo "Result: '$RESULT'"
echo "Empty: $([ -z "$RESULT" ] && echo 'YES' || echo 'NO')"
echo ""
echo "=== Test 4: Both tags don't exist ==="
FROM_TAG="v4.0.0"
TO_TAG="v6.0.0"
RESULT=$(awk "/^$FROM_TAG$/,/^$TO_TAG$/" <<< "$TAGS" | grep -v '^$')
echo "Result: '$RESULT'"
echo "Empty: $([ -z "$RESULT" ] && echo 'YES' || echo 'NO')"Repository: MFlowCode/MFC
Length of output: 338
Add validation to ensure both FROM_TAG and TO_TAG exist before extracting the range.
Lines 33–34 use awk to extract tags between FROM_TAG and TO_TAG, but don't validate that both endpoints exist. The awk range filter has critical failure modes:
- If
FROM_TAGdoesn't exist, it returns an empty result (silent failure—no tags are dispatched). - If
TO_TAGdoesn't exist, it returns all tags fromFROM_TAGto the end of the list (incorrect range—potentially dispatching unintended releases).
Add validation to detect these conditions and fail explicitly:
if ! echo "$TAGS" | grep -q "^${{ env.FROM_TAG }}$"; then
echo "::error::FROM_TAG '${{ env.FROM_TAG }}' not found in repository tags"
exit 1
fi
if ! echo "$TAGS" | grep -q "^${{ env.TO_TAG }}$"; then
echo "::error::TO_TAG '${{ env.TO_TAG }}' not found in repository tags"
exit 1
fiInsert these checks after line 32 (after fetching tags) to catch missing tags before the range extraction.
🤖 Prompt for AI Agents
.github/workflows/release.yml around lines 26 to 34: the workflow extracts a tag
range using awk but doesn’t verify that FROM_TAG and TO_TAG actually exist,
which can produce silent or incorrect results; after fetching tags, add checks
that grep for exact FROM_TAG and TO_TAG in the TAGS list and if either is
missing emit a GitHub Actions error message and exit with non-zero status so the
job fails fast before running the awk range extraction.
.github/workflows/release.yml
Outdated
| await github.rest.actions.createWorkflowDispatch({ | ||
| owner: 'Malmahrouqi3', | ||
| repo: 'MFC-mo2', | ||
| workflow_id: 'bench-frontier.yml', | ||
| ref: 'benchmark-all-releases-on-frontier-runner', | ||
| inputs: { tag } | ||
| }); |
There was a problem hiding this comment.
Fix hardcoded repository references to use dynamic context values.
Lines 43–44 hardcode the owner as 'Malmahrouqi3' and repo as 'MFC-mo2', which appears to be a fork or development repository. This will break on merge to the main MFlowCode/MFC repository. Use dynamic context values instead:
await github.rest.actions.createWorkflowDispatch({
- owner: 'Malmahrouqi3',
- repo: 'MFC-mo2',
+ owner: context.repo.owner,
+ repo: context.repo.repo,
workflow_id: 'bench-frontier.yml',
- ref: 'benchmark-all-releases-on-frontier-runner',
+ ref: context.ref.replace(/^refs\/(heads|pull\/\d+)\//, ''),
inputs: { tag }
});Also fix line 46: the string replace for context.ref doesn't handle PR contexts (e.g., refs/pull/XXX/merge). Use a more robust replacement or detect the event type.
🤖 Prompt for AI Agents
In .github/workflows/release.yml around lines 42–48, replace the hardcoded
owner/repo values and the fragile ref handling: use the GitHub Action context
repo values (github.context.repo.owner and github.context.repo.repo) instead of
'Malmahrouqi3'/'MFC-mo2', and compute the ref robustly by checking
github.context.ref — if it startsWith 'refs/pull/' use
github.context.payload.pull_request.head.ref (or head.sha) otherwise strip
'refs/heads/' from github.context.ref — then pass that computed ref into
createWorkflowDispatch.
.github/workflows/release.yml
Outdated
| let runId = null; | ||
| for (let i = 0; i < 60; i++) { // Try for up to 300 seconds | ||
| const runs = await github.rest.actions.listWorkflowRuns({ | ||
| owner: 'Malmahrouqi3', | ||
| repo: 'MFC-mo2', | ||
| workflow_id: 'bench-frontier.yml', | ||
| event: 'workflow_dispatch', | ||
| per_page: 5 | ||
| }); | ||
| // Find the most recent queued or in_progress run | ||
| const found = runs.data.workflow_runs.find(run => | ||
| ['queued', 'in_progress'].includes(run.status) | ||
| ); | ||
| if (found) { | ||
| runId = found.id; | ||
| break; | ||
| } | ||
| await new Promise(resolve => setTimeout(resolve, 5000)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/release.yml | sed -n '40,75p'Repository: MFlowCode/MFC
Length of output: 1774
🌐 Web query:
GitHub Actions REST API listWorkflowRuns run object inputs parameter
💡 Result:
Short answer: The workflow run objects returned by the REST "list workflow runs" / "get workflow run" endpoints do not include the workflow_dispatch "inputs" values. Those inputs are available inside the running workflow via github.event.inputs (or in job logs) but are not exposed as a run property in the REST responses — you must record them yourself (e.g., set the run-name, write to logs/artifacts, or store them elsewhere) if you need to retrieve them later via the API. [1][2][3]
Sources:
- REST API workflow runs (list/get) — response fields do not include inputs. [1]
- Create a workflow dispatch (shows inputs are sent to workflow via event payload). [2]
- Community/Stack Overflow discussion noting inputs aren’t returned by the API and must be extracted from logs or recorded. [3]
🌐 Web query:
GitHub Actions workflow_dispatch workflow_run_name custom run name filtering
💡 Result:
Short answer
- You can set a custom workflow run title for workflow_dispatch using run-name (supports expressions and inputs). [1][2]
- You cannot filter a workflow_run trigger by that run-name; workflow_run filtering only supports workflow names, types and branch-related filters (not run title). [2]
Workarounds - Have the upstream workflow include a distinct ID in run-name and then use the REST API (List workflow runs) to find runs by name and act on the matching run programmatically. [1][2]
- Or have the upstream job directly trigger the downstream workflow with a payload (repository_dispatch or use an action that discovers the dispatched run-id) so the downstream gets the metadata it needs instead of relying on workflow_run-name filtering. [3]
Sources
[1] GitHub Actions — run-name docs (workflow run naming).
[2] GitHub Actions — events that trigger workflows / workflow_run filtering (shows supported filters).
[3] Marketplace action and community examples for dispatching workflows / discovering run-id (workarounds).
🌐 Web query:
GitHub REST API listWorkflowRuns filter by run name or head_branch
💡 Result:
Short answer: You can filter server‑side by branch (query param branch) but there is no REST query parameter to filter directly by a workflow run's "name" (displayTitle) or a "head_branch" field — you must fetch runs and filter client‑side. You can also list runs for a specific workflow (by workflow_id or filename) and use other supported params like actor, event, head_sha, status, created. [1][2]
Example: list runs for a repo filtered by branch (server side)
curl -H "Accept: application/vnd.github+json"
-H "Authorization: Bearer $TOKEN"
"https://api.github.com/repos/OWNER/REPO/actions/runs?branch=my-branch"
Then filter by run name in your script (jq example):
... | jq '.workflow_runs[] | select(.name=="My run name")'
Docs: REST API parameters and supported filters. [1]
gh CLI notes (shows workflow-name and branch flags / fields like headBranch in output). [2]
Sources:
[1] GitHub REST API — Actions: List workflow runs (parameters list).
[2] GitHub CLI gh run list (flags and JSON fields).
Proposed fix is technically infeasible; the GitHub REST API does not expose workflow dispatch inputs in listWorkflowRuns response.
Lines 50–68 correctly identify a race condition when multiple tags are dispatched sequentially—both runs can queue simultaneously and the detection may match the wrong run. However, the suggested fix using run.inputs?.tag === tag won't work because the GitHub REST API does not include inputs in the run object returned by listWorkflowRuns. Workflow dispatch inputs are only accessible inside the running workflow via github.event.inputs, not via the REST API.
To fix this race condition, consider:
- Setting a unique
run-nameincreateWorkflowDispatch(e.g.,run-name: tag-${tag}) and filtering client-side byrun.name - Or using
head_shaor another unique identifier if available - Or implementing a repository_dispatch with a stored payload to track the dispatched run ID
The issue itself is critical and valid, but the proposed solution needs adjustment.
|
Workflows ( Send off 10 releases at a time with restricted concurrency regardless of order, meaning one will only run while others sit idle till their turn. |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
.github/workflows/test.yml
Outdated
There was a problem hiding this comment.
High-level Suggestion
The PR removes almost all CI/CD workflows for testing, linting, and deployment. It is suggested to restore these critical quality gates and add the new benchmarking workflow separately, rather than replacing the entire CI system. [High-level, importance: 10]
Solution Walkthrough:
Before:
// .github/workflows/ directory structure in this PR
- bench.yml
- cleanliness.yml
- coverage.yml
- docker.yml
- docs.yml
- formatting.yml
- test.yml
- ... (10+ other workflow files removed)
+ bench-frontier.yml // A single, specialized workflow is added
After:
// .github/workflows/ directory structure after applying the suggestion
bench.yml
cleanliness.yml
coverage.yml
docker.yml
docs.yml
formatting.yml
test.yml
... (all existing workflows are kept)
+ bench-frontier.yml // The new workflow is added alongside existing ones
| - name: Clone - PR | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| path: pr | ||
|
|
||
| - name: Clone - Master | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: MFlowCode/MFC | ||
| ref: ${{ env.BENCH_TAG }} | ||
| path: master | ||
|
|
||
| - name: Checkout PR repo | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| path: pr |
There was a problem hiding this comment.
✅ Suggestion: Remove the redundant Checkout PR repo step. The repository is already checked out in the preceding Clone - PR step. [general, importance: 4]
| - name: Clone - PR | |
| uses: actions/checkout@v4 | |
| with: | |
| path: pr | |
| - name: Clone - Master | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: MFlowCode/MFC | |
| ref: ${{ env.BENCH_TAG }} | |
| path: master | |
| - name: Checkout PR repo | |
| uses: actions/checkout@v3 | |
| with: | |
| path: pr | |
| - name: Clone - PR | |
| uses: actions/checkout@v4 | |
| with: | |
| path: pr | |
| - name: Clone - Master | |
| uses: actions/checkout@v4 | |
| with: | |
| repository: MFlowCode/MFC | |
| ref: ${{ env.BENCH_TAG }} | |
| path: master |
| - name: Setup & Build | ||
| run: | | ||
| (cd master && ${{ matrix.build_script }}) & | ||
| wait %1 |
There was a problem hiding this comment.
Suggestion: Simplify the Setup & Build step by removing the unnecessary backgrounding (&) and wait %1. The build script can be run directly in the foreground. [general, importance: 3]
| - name: Setup & Build | |
| run: | | |
| (cd master && ${{ matrix.build_script }}) & | |
| wait %1 | |
| - name: Setup & Build | |
| run: | | |
| cd master && ${{ matrix.build_script }} |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
.github/workflows/bench-frontier.yml (5)
61-64: Remove duplicate and deprecated checkout step.This checkout step is redundant with lines 49-52 and uses the deprecated
actions/checkout@v3. Remove this entire step.- - name: Checkout PR repo - uses: actions/checkout@v3 - with: - path: pr -
79-80: Fix references to non-existentmfc/directory.The workflow checks out code to
pr/andmaster/paths, but these lines attempt to copy from a non-existentmfc/directory. This will cause runtime failure.- cp -r mfc/* master/ - cp -r mfc/.git master/.gitBased on the workflow logic, you likely need to copy from
pr/or themaster/path already contains the checked-out code and these lines should be removed entirely.
89-89: Fix sed pattern to match actual Cantera version specifier.The sed pattern searches for
"cantera",but the actualpyproject.tomlcontains"cantera>=3.1.0",. The pattern won't match, so the version won't be pinned.- sed -i 's/"cantera",/"cantera==3.0.1",/g' master/toolchain/pyproject.toml + sed -i 's/"cantera[^"]*"/"cantera==3.0.1"/g' master/toolchain/pyproject.toml
110-110: Fix incorrect Silo.patch path reference.The patch file is copied to
toolchain/dependencies/Silo.patch(line 83), but this PATCH_COMMAND references${CMAKE_SOURCE_DIR}/Silo.patch(root directory). The build will fail when applying the patch.- PATCH_COMMAND "${GIT_EXECUTABLE}" stash - && "${GIT_EXECUTABLE}" apply "${CMAKE_SOURCE_DIR}/Silo.patch" + PATCH_COMMAND "${GIT_EXECUTABLE}" stash + && "${GIT_EXECUTABLE}" apply "${CMAKE_SOURCE_DIR}/toolchain/dependencies/Silo.patch"
131-131: Add missingmatrix.interfaceargument to benchmark script.The
run_parallel_benchmarks.shscript requires 3 arguments (device, interface, cluster), but only 2 are provided. This will cause the script to fail.- run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }} + run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.interface }} ${{ matrix.cluster }}
🧹 Nitpick comments (1)
.github/workflows/bench-frontier.yml (1)
32-39: Clarify interface naming: "omp" vs "mp".The PR objectives mention benchmarking "gpu-mp" (v5.1.0–v5.1.3), but this commented-out matrix entry uses
interface: omp. Verify whether "mp" and "omp" both refer to OpenMP, or if there's a naming inconsistency that should be corrected before uncommenting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/bench-frontier.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench-frontier.yml
🪛 actionlint (1.7.9)
.github/workflows/bench-frontier.yml
47-47: could not parse as YAML: mapping values are not allowed in this context
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/bench-frontier.yml
[error] 47-47: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (1)
.github/workflows/bench-frontier.yml (1)
44-46: Verify the necessity of forcing Node 16.The workflow forces the use of Node 16 and allows insecure versions. Node 16 reached end-of-life in September 2023. Confirm this is required for Frontier runner compatibility and cannot be upgraded.
If this is a hard requirement due to the self-hosted runner environment, consider documenting the reason in a comment:
env: # Frontier runner requires Node 16 due to [specific constraint] ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
.github/workflows/bench-frontier.yml
Outdated
| sudo mkdir -p master | ||
| sudo chmod 777 master | ||
| rm -rf master/* | ||
| cp -r mfc/* master/ | ||
| cp -r mfc/.git master/.git | ||
| cp pr/.github/Dockerfile master/ | ||
| cp pr/.github/.dockerignore master/ | ||
| cp pr/toolchain/dependencies/Silo.patch master/toolchain/dependencies/Silo.patch | ||
| cp pr/.github/scripts/run_parallel_benchmarks.sh master/.github/scripts/run_parallel_benchmarks.sh |
There was a problem hiding this comment.
Reconsider the master directory preparation logic.
Lines 54-59 check out code to master/, but line 78 immediately deletes all content with rm -rf master/*. This removes the checked-out code before the subsequent copy operations. Combined with the non-existent mfc/ references (lines 79-80), this entire preparation section appears fundamentally broken.
If the master checkout at lines 54-59 already contains the necessary code, consider removing the directory recreation and copy operations, and only copy the additional files needed (Dockerfile, .dockerignore, patches, scripts) from pr/ into master/.
sudo mkdir -p /home/runner/tmp
export TMPDIR=/home/runner/tmp
free -h
-
- sudo mkdir -p master
- sudo chmod 777 master
- rm -rf master/*
- cp -r mfc/* master/
- cp -r mfc/.git master/.git
+
+ # Copy additional files from PR into master
cp pr/.github/Dockerfile master/
cp pr/.github/.dockerignore master/
cp pr/toolchain/dependencies/Silo.patch master/toolchain/dependencies/Silo.patch
cp pr/.github/scripts/run_parallel_benchmarks.sh master/.github/scripts/run_parallel_benchmarks.shCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/bench-frontier.yml lines 76-84: the script recreates and wipes
the master directory and tries to copy from a non-existent mfc/ which removes
the previously checked-out code; instead, remove the mkdir/chmod/rm -rf master/*
and the cp mfc/* / mfc/.git lines, and only copy the specific additional files
from pr/ into the existing master/ (Dockerfile, .dockerignore, patches,
scripts). Ensure the earlier checkout step actually populates master/ and, if
not, adjust the checkout to target master/ before copying the additional files.
| (cd master && ${{ matrix.build_script }}) & | ||
| wait %1 |
There was a problem hiding this comment.
Fix incorrect wait syntax for bash.
Line 128 uses wait %1, which is Windows cmd syntax. In bash, use wait (no arguments) to wait for all background jobs, or wait $! for the last background process.
Apply this diff:
(cd master && ${{ matrix.build_script }}) &
- wait %1
+ wait📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (cd master && ${{ matrix.build_script }}) & | |
| wait %1 | |
| (cd master && ${{ matrix.build_script }}) & | |
| wait |
🤖 Prompt for AI Agents
In .github/workflows/bench-frontier.yml around lines 127 to 128, the workflow
uses Windows cmd style wait syntax `wait %1`; replace it with the correct bash
form—either `wait` to wait for all background jobs or `wait $!` to wait for the
last background process depending on intent—so change the line to use the
appropriate bash wait and ensure the background job invocation remains `(cd
master && ${{ matrix.build_script }}) &` before the wait.
|
The workflow was not recognized for syntax errors which is fixed now. |
|
gotta love the queue for the self-hosted runners. |
Removed unnecessary Docker context preparation steps and simplified the workflow.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
.github/workflows/bench-frontier.yml (5)
61-64: Remove redundant checkout step using deprecatedactions/checkout@v3.Lines 49-52 already clone the PR repository to path
prusingactions/checkout@v4. This step is redundant and uses the deprecated v3 version which actionlint has flagged.- name: Clone - Master uses: actions/checkout@v4 with: repository: MFlowCode/MFC ref: ${{ env.BENCH_TAG }} path: master - - name: Checkout PR repo - uses: actions/checkout@v3 - with: - path: pr - - name: Prepare context
73-75: Cantera sed pattern won't match the actual pyproject.toml content.The sed pattern
s/"cantera",/"cantera==3.0.1",/gsearches for"cantera",but the actualpyproject.tomlcontains"cantera>=3.1.0",. The replacement won't occur.- sed -i 's/"cantera",/"cantera==3.0.1",/g' master/toolchain/pyproject.toml + sed -i 's/"cantera[^"]*",/"cantera==3.0.1",/g' master/toolchain/pyproject.toml
94-95: Silo.patch path in PATCH_COMMAND is incorrect.
CMAKE_SOURCE_DIRtypically resolves to the project root (master/), not the current CMakeLists.txt directory. The patch atmaster/toolchain/dependencies/Silo.patchwon't be found at${CMAKE_SOURCE_DIR}/Silo.patch.Use
CMAKE_CURRENT_SOURCE_DIRinstead:- PATCH_COMMAND "${GIT_EXECUTABLE}" stash - && "${GIT_EXECUTABLE}" apply "${CMAKE_SOURCE_DIR}/Silo.patch" + PATCH_COMMAND "${GIT_EXECUTABLE}" stash + && "${GIT_EXECUTABLE}" apply "${CMAKE_CURRENT_SOURCE_DIR}/Silo.patch"
110-113: Fix incorrectwait %1syntax for non-interactive bash.
wait %1is job control syntax that requires an interactive shell. In GitHub Actions (non-interactive), usewait(all jobs) orwait $!(last background job).- name: Setup & Build run: | (cd master && ${{ matrix.build_script }}) & - wait %1 + wait $!Alternatively, remove the unnecessary backgrounding entirely if sequential execution is acceptable.
115-116: Missing required${{ matrix.interface }}argument.The
run_parallel_benchmarks.shscript requires 3 arguments (device, interface, cluster), but only 2 are provided.- run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }} + run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.interface }} ${{ matrix.cluster }}
🧹 Nitpick comments (1)
.github/workflows/bench-frontier.yml (1)
7-15: Overly broad triggers for a resource-intensive benchmark workflow.Triggering benchmarks on every
pushandpull_requestwill consume significant self-hosted Frontier runner time. Consider restricting to:
workflow_dispatchonly (on-demand)- Or add path filters / branch restrictions to limit runs
on: - push: - pull_request: + push: + branches: [master] + paths: + - 'src/**' + - 'toolchain/**' + pull_request: + branches: [master] + paths: + - 'src/**' + - 'toolchain/**' workflow_dispatch:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/bench-frontier.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench-frontier.yml
🪛 actionlint (1.7.9)
.github/workflows/bench-frontier.yml
27-27: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
62-62: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/workflows/bench-frontier.yml (2)
66-69: LGTM!The file copy operations correctly inject the PR's patch file and benchmark script into the master checkout for the build.
118-129: LGTM!The
if: always()ensures logs are printed even on failure, and the artifact upload correctly captures benchmark results.
| env: | ||
| ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 | ||
| ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true | ||
| BENCH_TAG: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.tag != '' && github.event.inputs.tag || 'v5.1.3' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/bench-frontier.ymlRepository: MFlowCode/MFC
Length of output: 5526
🌐 Web query:
Node.js 16 end of life EOL date GitHub Actions
💡 Result:
- Node.js 16 upstream EOL: September 11, 2023. [1]
- GitHub Actions runner EOL (Node16 removed): November 12, 2024. [2]
🏁 Script executed:
# Search for any comments or documentation about why Node16 is forced
rg -i "node16|node version|unsecure" .github/workflows/ -B 2 -A 2Repository: MFlowCode/MFC
Length of output: 568
🏁 Script executed:
# Check git history for context on why this was added
git log --oneline --all -- .github/workflows/bench-frontier.yml | head -20Repository: MFlowCode/MFC
Length of output: 129
🏁 Script executed:
# Check for any issues, comments, or discussions about Node20 upgrade
rg -i "node20|node version|upgrade.*runner|frontier.*node" --type md --type txt
# Search for any configuration or documentation about runner setup
find . -name "*.md" -o -name "*.txt" | xargs grep -l -i "frontier\|node" 2>/dev/null | head -10Repository: MFlowCode/MFC
Length of output: 522
🏁 Script executed:
# Look for any recent issues or PRs mentioning this
git log --all --oneline --grep="node\|frontier" -i | head -20Repository: MFlowCode/MFC
Length of output: 1099
🏁 Script executed:
# Search specifically for when Node16 forcing was added to this workflow
git log -p --all -- .github/workflows/bench-frontier.yml | grep -A 5 -B 5 "ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION" | head -30Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Check if there are any open issues or discussions about Node version
rg "ACTIONS_ALLOW_USE_UNSECURE" -B 5 -A 5Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Look for Frontier runner documentation or setup notes
find . -path "*frontier*" -name "*.sh" -o -path "*frontier*" -name "*.md" | head -20Repository: MFlowCode/MFC
Length of output: 253
Upgrade the Frontier runner to support Node20 or newer.
Node.js 16 reached end of life on November 12, 2024. GitHub Actions no longer includes Node16 in the runner, making the use of ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 and ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true a security concern. These environment variables force a deprecated, unsupported Node.js version and bypass security protections.
Remove these environment variables and update the self-hosted Frontier runner to support Node20 or later.
🤖 Prompt for AI Agents
In .github/workflows/bench-frontier.yml around lines 44 to 47, remove the
environment variables forcing Node16 (ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION
and ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION) and stop overriding runner Node
versions in the workflow; instead update the self-hosted Frontier runner(s) to
install and support Node20+ (or use a GitHub-hosted runner that already provides
Node20), verify workflows run under Node20, and then delete those env entries
and any references that bypass runner security.
Comment out the Frontier cluster configuration and change checkout reference to master.
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/bench-frontier.yml (4)
44-46: Node.js 16 is EOL and poses a security risk.Lines 45-46 force the use of Node.js 16, which reached end of life in November 2024, and explicitly allow unsecure versions. This is a major security concern that has been flagged in previous reviews.
The self-hosted Frontier runner must be upgraded to support Node20 or later. Remove these environment variable overrides once the runner is updated.
61-64: Remove redundant checkout step using deprecated action version.This step duplicates the checkout at lines 49-52 and uses the deprecated
actions/checkout@v3. The PR repository is already checked out to theprpath in the first step.Remove lines 61-64 entirely:
- name: Clone - Master uses: actions/checkout@v4 with: repository: MFlowCode/MFC ref: master path: master - - name: Checkout PR repo - uses: actions/checkout@v3 - with: - path: pr - - name: Setup & Build
66-69: Fix incorrect wait syntax for bash.Line 69 uses
wait %1, which is Windows cmd syntax. In bash (the shell used by GitHub Actions on Linux runners), the correct syntax iswait(to wait for all background jobs) orwait $!(for the last background process).Apply this diff:
- name: Setup & Build run: | (cd master && ${{ matrix.build_script }}) & - wait %1 + wait
71-72: Add missingmatrix.interfaceargument to benchmark script.The
run_parallel_benchmarks.shscript requires three arguments: device, interface, and cluster. This call only passes two arguments (device and cluster), which will cause the script to fail at runtime.Apply this diff:
- name: Bench (Master) - run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.cluster }} + run: bash master/.github/scripts/run_parallel_benchmarks.sh ${{ matrix.device }} ${{ matrix.interface }} ${{ matrix.cluster }}
🧹 Nitpick comments (1)
.github/workflows/bench-frontier.yml (1)
47-47: Consider simplifying the BENCH_TAG expression.The current ternary-style expression is correct but could be simplified using GitHub Actions' logical OR shorthand. This would make it more readable and idiomatic.
Apply this diff for a cleaner expression:
- BENCH_TAG: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.tag != '' && github.event.inputs.tag || 'v5.1.3' }} + BENCH_TAG: ${{ github.event.inputs.tag || 'v5.1.3' }}Note: This works because
github.event.inputs.tagis only defined whenworkflow_dispatchis triggered, and GitHub Actions treats empty strings as falsy in the||operator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/bench-frontier.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench-frontier.yml
🪛 actionlint (1.7.9)
.github/workflows/bench-frontier.yml
27-27: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
62-62: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Clone - Master | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: MFlowCode/MFC | ||
| ref: master | ||
| path: master |
There was a problem hiding this comment.
Wrong git ref: should checkout the tag, not master branch.
This workflow benchmarks a specific release tag, but line 58 checks out the master branch instead of ${{ env.BENCH_TAG }}. This defeats the entire purpose of the workflow—you'll be benchmarking the master branch every time instead of the specified release.
Apply this diff to fix:
- name: Clone - Master
uses: actions/checkout@v4
with:
repository: MFlowCode/MFC
- ref: master
+ ref: ${{ env.BENCH_TAG }}
path: master🤖 Prompt for AI Agents
.github/workflows/bench-frontier.yml lines 54-59: the checkout step currently
uses ref: master which causes the workflow to benchmark the master branch
instead of the intended release tag; update the checkout ref to use the
BENCH_TAG environment variable (ref: ${{ env.BENCH_TAG }}) so the workflow
checks out the specified tag, leaving repository and path as-is.
Added a new configuration for the Frontier cluster with OpenMP interface and updated the benchmark script call to include the interface parameter.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/bench-frontier.yml (3)
58-58: Fix git ref to checkout the specified release tag, not master.The workflow is designed to benchmark a specific release tag (from
BENCH_TAG), but this line checks out themasterbranch instead. This defeats the purpose of the workflow—every benchmark will test master instead of the intended release.Apply this diff to fix the ref:
- name: Clone - Master uses: actions/checkout@v4 with: repository: MFlowCode/MFC - ref: master + ref: ${{ env.BENCH_TAG }} path: master</comment_end>
69-69: Fix incorrect wait syntax for bash.The syntax
wait %1is Windows cmd syntax and will fail in bash. In bash, usewaitto wait for all background jobs orwait $!to wait for the last background process.Apply this diff:
(cd master && ${{ matrix.build_script }}) & - wait %1 + wait</comment_end>
61-64: Remove redundant and deprecated checkout step.This step duplicates the PR checkout from lines 49-52 and uses the deprecated
actions/checkout@v3which is no longer supported by GitHub Actions. The PR repository is already checked out at the correct path.Apply this diff to remove the redundant step:
- name: Clone - Master uses: actions/checkout@v4 with: repository: MFlowCode/MFC ref: master path: master - - - name: Checkout PR repo - uses: actions/checkout@v3 - with: - path: pr - name: Setup & Build</comment_end>
🧹 Nitpick comments (1)
.github/workflows/bench-frontier.yml (1)
66-69: Consider simplifying by removing unnecessary backgrounding.The build script is backgrounded and then immediately waited for, which is equivalent to running it in the foreground. Unless there's a specific reason for this pattern, the step can be simplified.
- name: Setup & Build run: | - (cd master && ${{ matrix.build_script }}) & - wait + cd master && ${{ matrix.build_script }}</comment_end>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/bench-frontier.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`
Applied to files:
.github/workflows/bench-frontier.yml
🪛 actionlint (1.7.9)
.github/workflows/bench-frontier.yml
27-27: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
35-35: label "frontier" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
62-62: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Oak Ridge | Frontier (CCE) (gpu-omp)
- GitHub Check: Oak Ridge | Frontier (CCE) (gpu-acc)
| env: | ||
| ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16 | ||
| ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true |
There was a problem hiding this comment.
Upgrade runner to support Node20 or remove deprecated Node16 override.
Node.js 16 reached end of life on November 12, 2024 and is no longer supported by GitHub Actions. Forcing Node16 with ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true bypasses security protections and exposes the workflow to known vulnerabilities.
Update the self-hosted Frontier runner to support Node20 or later, then remove these environment variables.
</comment_end>
🤖 Prompt for AI Agents
.github/workflows/bench-frontier.yml lines 44-46: the workflow forces Node16 by
setting ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION and
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION which is deprecated and insecure; either
upgrade the self-hosted Frontier runner to a Node20+ environment and then remove
these two environment variables from the workflow, or if upgrade cannot be
completed immediately, replace the override with a documented temporary comment
and an action that enforces a supported Node version (Node20) on the runner;
ensure the final change removes both ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION
and ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION entries and that the runner is
verified to support Node20 before committing.
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Will resort to old-school bash scripts and take down this PR due to the countless out-of-countrol limitations e.g. CI, scale, debugging, and many others as some versions require specific revival treatments. It was a good learning experience tho. |
1 participant
User description
User description
User description
Description
It has not yet been viable to do so with containers, so I resorted to occupying the CI frontier run to do the job which is easy to debug and monitor without much of manual handling. I recycled the same steps to revive old release herein which should hopefully workflow.
(v4.8.0-v.5.1.3) for gpu-acc, and(v5.1.0-v5.1.3) for gpu-mp.PR Type
Enhancement, Other
Description
Refactored benchmark workflow to support arbitrary Frontier releases
Removed Phoenix cluster testing and multiple CI workflows
Added release dispatcher workflow for batch benchmarking
Simplified benchmark script to focus on Frontier GPU acceleration
Diagram Walkthrough
File Walkthrough
1 files
Refactored benchmark workflow for Frontier releases1 files
New release dispatcher workflow for batch benchmarks17 files
Removed Phoenix-specific benchmark scriptRemoved Phoenix benchmark submission scriptRemoved Phoenix job submission scriptRemoved Phoenix test execution scriptRemoved code cleanliness check workflowRemoved coverage check workflowRemoved Homebrew tap deployment workflowRemoved containerization workflowRemoved documentation build workflowRemoved code formatting check workflowRemoved Homebrew formula test workflowRemoved line count analysis workflowRemoved source linting workflowRemoved toolchain linting workflowRemoved PMD code duplication detection workflowRemoved spell check workflowRemoved comprehensive test suite workflowCodeAnt-AI Description
Benchmark Frontier releases from CI and dispatch across tag ranges
What Changed
Impact
✅ Can benchmark a specific release tag on Frontier from the Actions UI✅ Run benchmarks across a range of release tags automatically✅ Archive Frontier benchmark logs per tag💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.
PR Type
Enhancement, Other
Description
Removed multiple CI workflows (test, coverage, docs, formatting, etc.)
Created new
bench-frontier.ymlfor benchmarking arbitrary Frontier releasesDeleted old Phoenix-specific benchmark scripts and submission utilities
Simplified benchmark infrastructure to focus on Frontier GPU acceleration
Diagram Walkthrough
File Walkthrough
1 files
New Frontier release benchmarking workflow18 files
Removed old benchmark workflowRemoved test suite workflowRemoved code cleanliness check workflowRemoved coverage check workflowRemoved Homebrew tap deployment workflowRemoved Docker containerization workflowRemoved documentation build workflowRemoved code formatting check workflowRemoved Homebrew formula test workflowRemoved lines of code counting workflowRemoved source code linting workflowRemoved toolchain linting workflowRemoved PMD code duplication detection workflowRemoved spell check workflowRemoved Phoenix GPU benchmark scriptRemoved Phoenix benchmark submission scriptRemoved Phoenix job submission scriptRemoved Phoenix test execution script