-
Notifications
You must be signed in to change notification settings - Fork 132
Shell completion auto-install and pre-commit hook improvements #1124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Add lint-gate job to test.yml that runs fast checks (formatting, spelling, linting) before expensive test matrix and HPC jobs start - Add concurrency groups to test.yml, coverage.yml, cleanliness.yml, and bench.yml to cancel superseded runs on new pushes - Add ./mfc.sh precheck command for local CI validation before pushing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add cross-platform hash function (macOS uses md5, Linux uses md5sum) - Validate -j/--jobs argument (require value, must be numeric) - Improve error messages with actionable guidance - Clarify that formatting has been auto-applied when check fails Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add wait-for-tests job that polls GitHub API to ensure: - Lint Gate passes first (fast fail) - All Github test jobs complete successfully - Only then do benchmark jobs start This prevents wasting HPC resources on benchmarking code that fails tests, while preserving the existing maintainer approval gate. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add .githooks/pre-commit that runs ./mfc.sh precheck before commits - Auto-install hook on first ./mfc.sh invocation (symlinks to .git/hooks/) - Hook only installs once; subsequent runs skip if already present - Developers can bypass with: git commit --no-verify Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Auto-detect available CPUs for parallel formatting: - Linux: nproc - macOS: sysctl -n hw.ncpu - Fallback: 4 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Keep both: pre-commit hook auto-install and 'Starting...' log message Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Register precheck in commands.py so it appears in: - Shell tab completion - CLI documentation - ./mfc.sh precheck --help Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When completion scripts are auto-regenerated, also update the installed completions at ~/.local/share/mfc/completions/ if they exist. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When installed shell completions are auto-updated, print a message with the appropriate source command for the user's detected shell. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, installed completions only updated when source files changed and regeneration occurred. Now we also check if the installed completions are older than the generated ones (e.g., after git pull brings new pre-generated completions). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove -o bashdefault from bash complete command to prevent falling back to directory completion when no matches found - Add explicit : (no-op) for zsh commands without arguments to prevent default file/directory completion Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Auto-install completions on first mfc.sh run (via main.py) - Add -o filenames back to bash complete (needed for file completion) - Keep -o bashdefault removed to prevent directory fallback - Simplify code by having __update_installed_completions handle both install and update cases Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move completion auto-install to mfc.sh so it runs for ALL commands including help, precheck, etc. This ensures completions are always set up on first run. - Install completion files to ~/.local/share/mfc/completions/ - Add source line to .bashrc or fpath to .zshrc - Tell user to restart shell or source the file - main.py now only handles updates when generated files change Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- -v/-vv/-vvv: output verbosity levels - --debug: build with debug compiler flags (for MFC Fortran code) - --debug-log/-d: Python toolchain debug logging (not MFC code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Avoid hogging resources on machines with many cores. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously checked if ~/.local/share/mfc/completions/ existed. Now checks if the actual completion file exists for the user's shell. This handles the edge case of an empty completions directory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
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 · |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 8. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
📝 WalkthroughWalkthroughReplaces inline completion installation with a centralized bootstrap script; standardizes quoted argument forwarding to bootstrap scripts; adds verbose handling to the Python bootstrap and CI/runtime invocations; compacts help output and prints full build error outputs; changes bench CI trigger to workflow_run with PR-info collection. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite as Test Suite (workflow)
participant BenchWF as bench.yml (workflow)
participant GitHub as GitHub API
participant Runner as Runner / Job
TestSuite->>BenchWF: Emit workflow_run (conclusion: success)
BenchWF->>GitHub: Request PR info (derive pr_number from workflow_run)
GitHub-->>BenchWF: Return pr_number, pr_author, approvals
BenchWF->>BenchWF: Set outputs (pr_number, pr_author, pr_approved)
BenchWF->>Runner: Start file-changes & matrix jobs (conditional)
Runner->>GitHub: Clone repo with ref = workflow_run.head_sha || github.sha
Runner-->>BenchWF: Job results and statuses
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 finished reviewing your PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements follow-up improvements to the CI and developer experience workflow introduced in #1122, focusing on shell completion auto-installation, completion auto-updates, and pre-commit hook enhancements.
Changes:
- Auto-install shell completions to
~/.local/share/mfc/completions/on first./mfc.shrun, with automatic rc file configuration for bash/zsh - Auto-update installed completions when generated files are newer than installed copies
- Enhanced pre-commit hook with parallelism capped at 12 jobs and visible job count in output
- Fixed bash/zsh tab completion to prevent unwanted directory fallback by removing
-o bashdefault - Clarified documentation distinguishing
--debug(compiler flags),-v/-vv/-vvv(verbosity), and--debug-log(toolchain debugging)
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
mfc.sh |
Auto-install shell completions and pre-commit hook on first run, with rc file modifications |
toolchain/main.py |
Auto-update installed completions when regenerated files are newer |
toolchain/mfc/cli/completion_gen.py |
Remove -o bashdefault from bash completions and add explicit no-op for empty zsh commands |
toolchain/bootstrap/precheck.sh |
New precheck script for local CI validation before commits |
toolchain/mfc/cli/commands.py |
Add PRECHECK_COMMAND definition and clarify flag documentation |
toolchain/mfc/cli/docs_gen.py |
Update documentation for debug and verbosity flags |
.githooks/pre-commit |
New pre-commit hook with capped parallelism (max 12 jobs) |
.github/workflows/test.yml |
Add lint-gate job and concurrency groups |
.github/workflows/coverage.yml |
Add concurrency configuration |
.github/workflows/cleanliness.yml |
Add concurrency configuration |
.github/workflows/bench.yml |
Add concurrency and wait-for-tests job with polling logic |
Comments suppressed due to low confidence (2)
mfc.sh:42
- The shell completion auto-install logic modifies user's shell rc files (.bashrc/.zshrc) without explicit user consent. This could be considered intrusive. Consider either:
- Prompting the user before modifying their shell rc files, or
- Printing a clear message explaining what was added and how to remove it if desired.
Additionally, the check on line 38 only verifies if COMPLETION_DIR exists in the rc file, which could produce false positives if the directory path appears in an unrelated context.
if [ -f "$RC_FILE" ] && ! grep -q "$COMPLETION_DIR" "$RC_FILE" 2>/dev/null; then
echo "" >> "$RC_FILE"
echo "# MFC shell completion" >> "$RC_FILE"
echo "$RC_LINE" >> "$RC_FILE"
fi
mfc.sh:16
- The symlink creation could fail silently if the user doesn't have write permissions to .git/hooks/ or if the target doesn't exist. Consider adding error handling to inform the user if the hook installation fails.
if [ -d "$(pwd)/.git" ] && [ ! -e "$(pwd)/.git/hooks/pre-commit" ] && [ -f "$(pwd)/.githooks/pre-commit" ]; then
ln -sf "$(pwd)/.githooks/pre-commit" "$(pwd)/.git/hooks/pre-commit"
log "Installed git pre-commit hook (runs$MAGENTA ./mfc.sh precheck$COLOR_RESET before commits)."
Check if installed completions are older than source files and update them automatically. Shows message with source command only on install or update, silent otherwise. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace polling-based wait-for-tests job with workflow_run trigger that fires when Test Suite completes (more efficient, no wasted runner minutes) - Extract shell completion setup from mfc.sh to dedicated toolchain/bootstrap/completions.sh script for better maintainability Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/bench.yml:
- Around line 48-51: The current workflow sets author using
github.event.workflow_run.actor.login which can be the re-runner, not the PR
author; when PR_NUMBER is present, call the GitHub API to fetch the PR metadata
(GET /repos/{owner}/{repo}/pulls/{PR_NUMBER}) and extract the pull request
author login (user.login) and write that to GITHUB_OUTPUT instead of
github.event.workflow_run.actor.login; update the block that defines PR_NUMBER
and author (the PR_NUMBER variable and the echo "author=..." line) to detect
PR_NUMBER, query the API (using gh api or curl with GITHUB_TOKEN), parse
.user.login, and echo "author=<pr_author>" >> $GITHUB_OUTPUT so downstream gates
use the real PR author.
In `@toolchain/bootstrap/completions.sh`:
- Around line 60-67: The sourced-detection using BASH_SOURCE in the if condition
is bash-specific and fails silently in zsh; update the detection logic around
the BASH_SOURCE check so it handles zsh too (or use a portable fallback): detect
sourcing by checking both bash's BASH_SOURCE and zsh's $ZSH_VERSION or checking
if the current shell differs from the parent via "${BASH_SOURCE[1]} != ${0}" OR
a zsh-compatible test (e.g., [[ -n $ZSH_VERSION && $ZSH_EVAL_CONTEXT == *:file:*
]]) before sourcing "$COMPLETION_FILE" and calling log "Tab completions
activated.", while preserving the existing else branch that uses SOURCE_CMD and
log for manual instructions.
- Around line 38-56: The script is copying completion files without verifying
their presence; before any cp from "$COMPLETION_SRC/mfc.bash" and
"$COMPLETION_SRC/_mfc" (used in both the fresh-install and update branches where
COMPLETION_FILE or SOURCE_FILE logic runs), check that those source files exist
and are readable, and only perform the copy and set COMPLETIONS_CHANGED=true if
they do; if either is missing, avoid copying and either log a warning to stderr
(e.g., mention COMPLETION_SRC and which file is missing) or skip silently so the
script doesn't fail when toolchain/completions hasn't been generated yet, and
keep the existing RC_FILE/RC_LINE addition behavior only after successful copy.
Filter out individual package lines (+ pkg==1.0) from uv output while keeping progress info (Resolved, Built, Installed). Use -v flag to see full package list. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="toolchain/bootstrap/completions.sh">
<violation number="1" location="toolchain/bootstrap/completions.sh:41">
P2: The completion files are copied without verifying the source files exist. If `toolchain/completions/mfc.bash` or `toolchain/completions/_mfc` haven't been generated yet (e.g., fresh clone before running the toolchain), `cp` will fail with an error message that may confuse users. Add an existence check before copying.</violation>
</file>
<file name=".github/workflows/bench.yml">
<violation number="1" location=".github/workflows/bench.yml:51">
P2: Using `workflow_run.actor.login` returns the user who triggered the workflow run, not the PR author. On re-runs by maintainers or other users, this will return the wrong author, potentially bypassing or incorrectly applying the author-based benchmark gate. Consider fetching the actual PR author from the API when `PR_NUMBER` is available.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Remove decorative boxes and condense layout while keeping all essential information: commands with aliases, descriptions, and quick start guide. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Now ./mfc.sh init -v and similar commands respect verbosity flags during venv/package installation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mfc.sh`:
- Around line 53-71: Several bootstrap invocations (the calls that source
lint.sh, format.sh, spelling.sh and precheck.sh after the shift) use unquoted $@
which breaks arguments with spaces; change each occurrence like shift; .
"$(pwd)/toolchain/bootstrap/lint.sh" $@ to quote the arguments as shift; .
"$(pwd)/toolchain/bootstrap/lint.sh" "$@" (and do the same for format.sh,
spelling.sh, precheck.sh), and also ensure the similar $@ usage near the
python.sh invocation earlier (around the referenced line 51) is consistently
quoted as "$@".
When only flags are given without a command, show help screen instead of passing flags to main.py which would error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use the CLI schema from commands.py instead of hardcoded descriptions for the compact splash screen. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. Fix workflow_run.actor.login returning re-runner instead of PR author by fetching actual PR author from GitHub API 2. Quote all $@ in mfc.sh to handle arguments with spaces correctly 3. Add existence check for completion source files before copying (prevents errors on fresh clones before generation) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mfc.sh`:
- Around line 87-92: The shell wrapper in mfc.sh incorrectly drops flags by
treating any leading option as "no command"; update the conditional so it only
treats the no-argument case as empty (i.e., keep the check for -z "$1" only) and
always forward all arguments to python3 "$(pwd)/toolchain/main.py" when any
arguments exist (so remove the [[ "$1" == -* ]] branch and call main.py with
"$@"); this preserves flags like -v and lets main.py's _get_command_from_args()
parse mixed option/command ordering.
🧹 Nitpick comments (1)
.github/workflows/bench.yml (1)
41-69: Quote variables for robustness (static analysis).Static analysis flags unquoted variable expansions throughout this script block. While
$GITHUB_OUTPUTand other GitHub-provided variables typically don't contain spaces, quoting is still best practice. Additionally, if thegh apicall fails or returns unexpected output,$APPROVEDcould be empty, causing the arithmetic comparison on Line 59 to fail.🛠️ Proposed fix
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - echo "pr_number=" >> $GITHUB_OUTPUT - echo "approved=true" >> $GITHUB_OUTPUT - echo "author=${{ github.actor }}" >> $GITHUB_OUTPUT + echo "pr_number=" >> "$GITHUB_OUTPUT" + echo "approved=true" >> "$GITHUB_OUTPUT" + echo "author=${{ github.actor }}" >> "$GITHUB_OUTPUT" else # Get PR number from workflow_run PR_NUMBER="${{ github.event.workflow_run.pull_requests[0].number }}" if [ -n "$PR_NUMBER" ]; then - echo "pr_number=$PR_NUMBER" >> $GITHUB_OUTPUT + echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" # Fetch actual PR author from API (workflow_run.actor is the re-runner, not PR author) PR_AUTHOR=$(gh api repos/${{ github.repository }}/pulls/$PR_NUMBER --jq '.user.login') - echo "author=$PR_AUTHOR" >> $GITHUB_OUTPUT + echo "author=$PR_AUTHOR" >> "$GITHUB_OUTPUT" # Check if PR is approved APPROVED=$(gh api repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews \ --jq '[.[] | select(.state == "APPROVED")] | length') - if [ "$APPROVED" -gt 0 ]; then - echo "approved=true" >> $GITHUB_OUTPUT + if [ "${APPROVED:-0}" -gt 0 ]; then + echo "approved=true" >> "$GITHUB_OUTPUT" else - echo "approved=false" >> $GITHUB_OUTPUT + echo "approved=false" >> "$GITHUB_OUTPUT" fi else - echo "pr_number=" >> $GITHUB_OUTPUT - echo "approved=false" >> $GITHUB_OUTPUT - echo "author=" >> $GITHUB_OUTPUT + echo "pr_number=" >> "$GITHUB_OUTPUT" + echo "approved=false" >> "$GITHUB_OUTPUT" + echo "author=" >> "$GITHUB_OUTPUT" fi fiThe
${APPROVED:-0}default ensures the arithmetic comparison doesn't fail if the API call returns empty.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1124 +/- ##
=======================================
Coverage 44.03% 44.03%
=======================================
Files 70 70
Lines 20649 20649
Branches 2053 2053
=======================================
Hits 9093 9093
Misses 10368 10368
Partials 1188 1188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Truncation hides important context when diagnosing build failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
--debug-log only enables Python toolchain logging, while --debug enables debug compiler flags which is actually useful for diagnosing build and run failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
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 Incremental review completed. |
- Scan all args for a non-flag to detect if a command is present, so ./mfc.sh -v build works correctly instead of dropping args - Use ZSH_VERSION instead of $SHELL for shell detection (detects the running shell, not the login shell) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@toolchain/bootstrap/completions.sh`:
- Around line 56-61: The update condition only checks a single SOURCE_FILE vs
COMPLETION_FILE and can miss newer source scripts for the other shell; change
the logic to independently compare each source file (e.g.
"$COMPLETION_SRC/mfc.bash" and "$COMPLETION_SRC/_mfc") against their respective
destination files in "$COMPLETION_DIR" and copy each file when its source is
newer, setting COMPLETIONS_CHANGED=true if either copy occurs; locate the block
that references SOURCE_FILE, COMPLETION_FILE, COMPLETION_SRC, COMPLETION_DIR,
mfc.bash and _mfc and replace the single conditional with two checks (one per
file) that perform cp and set the flag when needed.
|
/improve |
The pyrometheus dependency requires Python >= 3.12. The previous minimum of 3.11 would allow bootstrapping but fail at package installation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pin pyrometheus to commit 49833404f (before it added a Python >= 3.12 requirement) so MFC can support Python 3.10+. Verified that bootstrap, build, and chemistry test cases all pass with Python 3.10. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enables verbose output in CI for easier debugging of failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Missed these in the previous commit — adds verbose output to the codecov build/test and the GitHub-hosted runner build/test steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/coverage.yml:
- Around line 41-45: The workflow uses unquoted command substitution for
parallelism in the Build and Test steps (the run lines invoking "mfc.sh build -v
-j $(nproc) --gcov" and "mfc.sh test -v -a -j $(nproc)"), which triggers
actionlint SC2046; fix it by quoting the command substitutions so they read "-j
\"$(nproc)\"" in both the Build (mfc.sh build) and Test (mfc.sh test) run
commands to prevent word-splitting and satisfy actionlint.
In @.github/workflows/frontier_amd/bench.sh:
- Around line 18-21: ShellCheck warns about unquoted variables causing
word-splitting: in the bench invocation inside the if/else branch (condition
checks job_device and calls ./mfc.sh bench), quote the $n_ranks and the command
substitution $(nproc) so they become "$n_ranks" and "$(nproc)"; update the two
occurrences in the bench command lines that call ./mfc.sh so the -j arguments
are quoted, leaving job_device, job_slug and device_opts usage unchanged.
| - name: Build | ||
| run: /bin/bash mfc.sh build -j $(nproc) --gcov | ||
| run: /bin/bash mfc.sh build -v -j $(nproc) --gcov | ||
|
|
||
| - name: Test | ||
| run: /bin/bash mfc.sh test -a -j $(nproc) | ||
| run: /bin/bash mfc.sh test -v -a -j $(nproc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix actionlint SC2046 by quoting $(nproc).
actionlint flags the unquoted command substitution; this can fail the lint gate.
🔧 Suggested fix
- run: /bin/bash mfc.sh build -v -j $(nproc) --gcov
+ run: /bin/bash mfc.sh build -v -j "$(nproc)" --gcov
@@
- run: /bin/bash mfc.sh test -v -a -j $(nproc)
+ run: /bin/bash mfc.sh test -v -a -j "$(nproc)"📝 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.
| - name: Build | |
| run: /bin/bash mfc.sh build -j $(nproc) --gcov | |
| run: /bin/bash mfc.sh build -v -j $(nproc) --gcov | |
| - name: Test | |
| run: /bin/bash mfc.sh test -a -j $(nproc) | |
| run: /bin/bash mfc.sh test -v -a -j $(nproc) | |
| - name: Build | |
| run: /bin/bash mfc.sh build -v -j "$(nproc)" --gcov | |
| - name: Test | |
| run: /bin/bash mfc.sh test -v -a -j "$(nproc)" |
🧰 Tools
🪛 actionlint (1.7.10)
[error] 42-42: shellcheck reported issue in this script: SC2046:warning:1:30: Quote this to prevent word splitting
(shellcheck)
[error] 45-45: shellcheck reported issue in this script: SC2046:warning:1:32: Quote this to prevent word splitting
(shellcheck)
🤖 Prompt for AI Agents
In @.github/workflows/coverage.yml around lines 41 - 45, The workflow uses
unquoted command substitution for parallelism in the Build and Test steps (the
run lines invoking "mfc.sh build -v -j $(nproc) --gcov" and "mfc.sh test -v -a
-j $(nproc)"), which triggers actionlint SC2046; fix it by quoting the command
substitutions so they read "-j \"$(nproc)\"" in both the Build (mfc.sh build)
and Test (mfc.sh test) run commands to prevent word-splitting and satisfy
actionlint.
| if [ "$job_device" = "gpu" ]; then | ||
| ./mfc.sh bench --mem 12 -j $n_ranks -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | ||
| ./mfc.sh bench -v --mem 12 -j $n_ranks -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | ||
| else | ||
| ./mfc.sh bench --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | ||
| ./mfc.sh bench -v --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -nP '\bjob_slug\b' -C2 .github/workflowsRepository: MFlowCode/MFC
Length of output: 10370
🏁 Script executed:
cat -n .github/workflows/frontier_amd/bench.sh | head -50Repository: MFlowCode/MFC
Length of output: 1004
🏁 Script executed:
fd -type f -name "*.sh" .github/workflows | head -10Repository: MFlowCode/MFC
Length of output: 227
🏁 Script executed:
cat -n .github/workflows/frontier_amd/submit-bench.shRepository: MFlowCode/MFC
Length of output: 1552
🏁 Script executed:
cat -n .github/workflows/frontier_amd/bench.shRepository: MFlowCode/MFC
Length of output: 1004
Quote variables to follow shellcheck recommendations.
Lines 19 and 21 use unquoted $n_ranks and $(nproc), which shellcheck flags for potential word-splitting issues. Quote these for consistency: "$n_ranks" and "$(nproc)".
Note: job_slug is reliably set in the parent sbatch context (submit-bench.sh line 45) before this script is sourced, so a guard is unnecessary.
🔧 Suggested fix
if [ "$job_device" = "gpu" ]; then
- ./mfc.sh bench -v --mem 12 -j $n_ranks -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks
+ ./mfc.sh bench -v --mem 12 -j "$n_ranks" -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n "$n_ranks"
else
- ./mfc.sh bench -v --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks
+ ./mfc.sh bench -v --mem 1 -j "$(nproc)" -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n "$n_ranks"
fi📝 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.
| if [ "$job_device" = "gpu" ]; then | |
| ./mfc.sh bench --mem 12 -j $n_ranks -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | |
| ./mfc.sh bench -v --mem 12 -j $n_ranks -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | |
| else | |
| ./mfc.sh bench --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | |
| ./mfc.sh bench -v --mem 1 -j $(nproc) -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n $n_ranks | |
| if [ "$job_device" = "gpu" ]; then | |
| ./mfc.sh bench -v --mem 12 -j "$n_ranks" -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n "$n_ranks" | |
| else | |
| ./mfc.sh bench -v --mem 1 -j "$(nproc)" -o "$job_slug.yaml" -- -c frontier_amd $device_opts -n "$n_ranks" | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 19-19: job_slug is referenced but not assigned.
(SC2154)
[warning] 21-21: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In @.github/workflows/frontier_amd/bench.sh around lines 18 - 21, ShellCheck
warns about unquoted variables causing word-splitting: in the bench invocation
inside the if/else branch (condition checks job_device and calls ./mfc.sh
bench), quote the $n_ranks and the command substitution $(nproc) so they become
"$n_ranks" and "$(nproc)"; update the two occurrences in the bench command lines
that call ./mfc.sh so the -j arguments are quoted, leaving job_device, job_slug
and device_opts usage unchanged.
The Python 3.10 compatibility changes have been merged upstream, so we no longer need to pin to a specific commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="toolchain/pyproject.toml">
<violation number="1">
P2: Dependency is no longer pinned to a specific commit or tag, making builds non-reproducible and susceptible to upstream changes. Pin the Git dependency to a commit hash or release tag.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
User description
User description
Summary
Follow-up improvements to the CI and developer experience work from #1122:
./mfc.shrun, completions are installed to~/.local/share/mfc/completions/and the source line is added to.bashrc/.zshrc-v/-vv/-vvv(verbosity),--debug(compiler flags), and--debug-log(toolchain debugging)Changes
mfc.sh: Auto-install completions with shell rc setuptoolchain/main.py: Auto-update installed completions when regeneratedtoolchain/mfc/cli/completion_gen.py: Fix bash/zsh completion options.githooks/pre-commit: Cap at 12 jobs, show counttoolchain/mfc/cli/commands.py: Clarify flag documentationTest plan
./mfc.shon fresh clone - completions should auto-install~/.local/share/mfc/completions/and run again - should reinstallbuild/-j Nwhere N ≤ 12🤖 Generated with Claude Code
PR Type
Enhancement, Tests
Description
Add CI lint-gate job and local precheck command for early validation
Auto-install git pre-commit hook and shell completions on first run
Gate benchmarks on test suite completion to prevent wasted HPC resources
Improve documentation clarity for debug, verbosity, and logging flags
Add concurrency groups to CI workflows to cancel superseded runs
Diagram Walkthrough
File Walkthrough
7 files
Auto-update installed completions when regeneratedAdd precheck command and clarify flag documentationAuto-install pre-commit hook and shell completionsNew precheck script for local CI validationNew git pre-commit hook running precheckAdd lint-gate job and concurrency groupsAdd wait-for-tests job and concurrency groups1 files
Fix bash/zsh completion options and prevent directory fallback1 files
Clarify debug and verbosity flag documentation2 files
Add concurrency groups to cancel superseded runsAdd concurrency groups to cancel superseded runsCodeAnt-AI Description
Auto-install shell completions, clearer build/run errors, and bench jobs gated on Test Suite
What Changed
Impact
✅ Clearer build and run errors✅ Immediate tab completions on first run✅ Fewer wasted benchmark runs💡 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
New Features
Improved UX
Chores