Skip to content

Add CI lint gate and local precheck command #1122

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.

Sign up for GitHub

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

Merged
sbryngelson merged 16 commits into from
Feb 5, 2026
+394 −21
Merged

Add CI lint gate and local precheck command #1122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7496ab7
Add CI lint gate and local precheck command
sbryngelson Feb 4, 2026
26f7e17
Fix precheck.sh portability and usability issues
sbryngelson Feb 4, 2026
4b17fa2
Gate benchmarks on Test Suite completion
sbryngelson Feb 4, 2026
1b211a5
Auto-install git pre-commit hook for precheck
sbryngelson Feb 4, 2026
9304137
Use dynamic CPU count in pre-commit hook
sbryngelson Feb 4, 2026
77ac8cc
Show CPU count in pre-commit hook output
sbryngelson Feb 4, 2026
a7cf1d2
Merge upstream master, resolve mfc.sh conflict
sbryngelson Feb 5, 2026
4b55c30
Add precheck command to CLI and autocomplete
sbryngelson Feb 5, 2026
e017cf0
Auto-update installed shell completions on regeneration
sbryngelson Feb 5, 2026
780138b
Show source command when completions auto-update
sbryngelson Feb 5, 2026
06f0425
Always check installed completions on every run
sbryngelson Feb 5, 2026
f65ed30
Prevent directory completion fallback in shell completions
sbryngelson Feb 5, 2026
d23addc
Auto-install completions and fix bash completion options
sbryngelson Feb 5, 2026
33cbd0e
Auto-install completions from mfc.sh with shell rc setup
sbryngelson Feb 5, 2026
1ba4666
Clarify verbose, debug, and debug-log flag documentation
sbryngelson Feb 5, 2026
adfcb7f
Cap pre-commit hook parallelism at 12 jobs
sbryngelson Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .githooks/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/bin/bash

# MFC pre-commit hook
# Runs ./mfc.sh precheck before allowing commits
# Bypass with: git commit --no-verify

# Only run if we're in the MFC repo root (where mfc.sh exists)
if [ ! -f "$(git rev-parse --show-toplevel)/mfc.sh" ]; then
exit 0
fi

cd "$(git rev-parse --show-toplevel)"

# Auto-detect CPU count (capped at 12 to avoid hogging resources)
JOBS=$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4)
[ "$JOBS" -gt 12 ] && JOBS=12

echo ""
echo "mfc: Running precheck before commit (-j $JOBS)..."
echo ""

if ./mfc.sh precheck -j "$JOBS"; then
echo ""
exit 0
else
echo ""
echo "mfc: Commit blocked. Fix issues above or use 'git commit --no-verify' to bypass."
echo ""
exit 1
fi
62 changes: 59 additions & 3 deletions .github/workflows/bench.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ on:
types: [submitted]
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
file-changes:
name: Detect File Changes
runs-on: 'ubuntu-latest'
outputs:
outputs:
checkall: ${{ steps.changes.outputs.checkall }}
steps:
- name: Clone
Expand All @@ -19,13 +23,65 @@ jobs:
- name: Detect Changes
uses: dorny/paths-filter@v3
id: changes
with:
with:
filters: ".github/file-filter.yml"

wait-for-tests:
name: Wait for Test Suite
runs-on: ubuntu-latest
steps:
- name: Wait for Test Suite to Pass
env:
GH_TOKEN: ${{ github.token }}
run: |
Comment on lines +29 to +36
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/bench.yml

Repository: MFlowCode/MFC

Length of output: 8041

🏁 Script executed:

# Check for any actions API calls in the workflow
rg "actions/runs|actions/workflows" .github/workflows/bench.yml

# Also verify the check-runs API endpoint requirements
echo "=== Checking all gh api calls in wait-for-tests job ==="
sed -n '36,79p' .github/workflows/bench.yml | grep -E "gh api|--jq"

Repository: MFlowCode/MFC

Length of output: 704

🏁 Script executed:

# Check if there are any other workflow files that might show permission patterns
find .github/workflows -name "*.yml" -type f | head -10 | xargs grep -l "permissions:" 2>/dev/null

# Also check if this is a public repo to understand default token permissions
git remote -v | head -5

Repository: MFlowCode/MFC

Length of output: 345

🏁 Script executed:

# Check what permissions are used in those files
echo "=== deploy-tap.yml permissions ==="
grep -A 5 "permissions:" .github/workflows/deploy-tap.yml | head -15

echo -e "\n=== line-count.yml permissions ==="
grep -A 5 "permissions:" .github/workflows/line-count.yml | head -15

# Also check if the repo has a default permissions setting at workflow level
grep -n "permissions:" .github/workflows/bench.yml

Repository: MFlowCode/MFC

Length of output: 511

🌐 Web query:

GitHub Actions default GITHUB_TOKEN permissions check-runs API public repository

💡 Result:

  • Default GITHUB_TOKEN permissions (public repo): By default (for newly created repos in a personal account), GITHUB_TOKEN is restricted to read access for contents and packages only; org repos inherit the org’s setting. You can switch the repo/org default to a more permissive “read/write for all scopes” in Settings → Actions → General → Workflow permissions. [1]
  • Check Runs API needs checks: write: Creating/managing check runs via the REST API requires a token with Checks (write) permission, and GitHub notes that write access to Checks via the REST API is only available to GitHub Apps (not OAuth apps/authenticated users). [2]
  • So, why it fails by default: With the restricted default, GITHUB_TOKEN won’t have checks: write, so calls like Create a check run will be denied unless you grant that permission. [1][2]
  • Fix in workflow: explicitly grant it: 
    permissions:
  checks: write
    (Any permission you don’t list is set to none.) [3]
  • Fork PR caveat (public repos): workflows triggered from pull requests from public forks get a read-only token regardless of these settings, so creating/updating check runs may still be blocked in that scenario. [3]

Sources: [1] GitHub Docs – repository Actions settings / default GITHUB_TOKEN permissions; [2] GitHub Docs – REST API “check runs” (Create a check run) permission notes; [3] GitHub Changelog – permissions: syntax + public-fork PR read-only behavior.

Remove unnecessary actions: read permission; add only checks: read.

The job makes read-only calls to the Checks API endpoint (repos/{owner}/{repo}/commits/{ref}/check-runs), not the Actions API. With restricted default GITHUB_TOKEN permissions, the job will fail without explicit read access to checks. Add checks: read only:

Corrected permissions block 
  wait-for-tests:
    name: Wait for Test Suite
    runs-on: ubuntu-latest
+   permissions:
+     checks: read
📝 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.

Suggested change
wait-for-tests:
name: Wait for Test Suite
runs-on: ubuntu-latest
steps:
- name: Wait for Test Suite to Pass
env:
GH_TOKEN: ${{ github.token }}
run: |
wait-for-tests:
name: Wait for Test Suite
runs-on: ubuntu-latest
permissions:
checks: read
steps:
- name: Wait for Test Suite to Pass
env:
GH_TOKEN: ${{ github.token }}
run: |
🤖 Prompt for AI Agents 
In @.github/workflows/bench.yml around lines 29 - 36, Update the workflow job
"wait-for-tests" to remove the unnecessary permissions entry granting actions:
read and instead grant only checks: read so the "Wait for Test Suite to Pass"
step can call the Checks API; locate the permissions block for the
wait-for-tests job (associated with the job name wait-for-tests and the step
named "Wait for Test Suite to Pass") and replace actions: read with checks:
read.

echo "Waiting for Test Suite workflow to complete..."
SHA="${{ github.event.pull_request.head.sha || github.sha }}"

# Poll every 60 seconds for up to 3 hours
for i in $(seq 1 180); do
# Get the Test Suite workflow runs for this commit
STATUS=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
--jq '.check_runs[] | select(.name == "Lint Gate") | .conclusion' | head -1)

if [ "$STATUS" = "success" ]; then
echo "Lint Gate passed. Checking test jobs..."

# Check if any Github test jobs failed
FAILED=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
--jq '[.check_runs[] | select(.name | startswith("Github")) | select(.conclusion == "failure")] | length')

if [ "$FAILED" != "0" ]; then
echo "::error::Test Suite has failing jobs. Benchmarks will not run."
exit 1
fi

# Check if Github tests are still running
PENDING=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
--jq '[.check_runs[] | select(.name | startswith("Github")) | select(.conclusion == null)] | length')

if [ "$PENDING" = "0" ]; then
echo "All Test Suite jobs completed successfully!"
exit 0
fi

echo "Tests still running ($PENDING pending)..."
elif [ "$STATUS" = "failure" ]; then
echo "::error::Lint Gate failed. Benchmarks will not run."
exit 1
else
echo "Lint Gate status: ${STATUS:-pending}..."
fi

sleep 60
done

echo "::error::Timeout waiting for Test Suite to complete."
exit 1
Comment on lines +37 to +79
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/test.yml | head -100

Repository: MFlowCode/MFC

Length of output: 3596

🏁 Script executed:

rg -n "name:" .github/workflows/test.yml | grep -E "(Lint|Github|GitHub)"

Repository: MFlowCode/MFC

Length of output: 286

🏁 Script executed:

cat -n .github/workflows/bench.yml | sed -n '37,79p'

Repository: MFlowCode/MFC

Length of output: 2205

Scope polling to the latest Test Suite run and handle terminal non-success states.

The check-runs endpoint aggregates across reruns for the same SHA, so a prior failed run can block benchmarks even if a newer run succeeds. Additionally, skipped, cancelled, timed_out, or other non-success states for the Lint Gate job will currently hang until timeout instead of failing fast, as they fall through to the status logging logic and continue polling. Switch to the workflow runs and jobs endpoints to scope to the latest Test Suite run, and explicitly handle all terminal non-success states.

The job names are confirmed as correct: "Lint Gate" (line 11 of test.yml) and "Github" (line 64 of test.yml).

🛠️ Suggested update to poll the latest Test Suite run 
-            # Get the Test Suite workflow runs for this commit
-            STATUS=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
-              --jq '.check_runs[] | select(.name == "Lint Gate") | .conclusion' | head -1)
+            # Get the latest Test Suite workflow run for this commit
+            RUN_ID=$(gh api repos/${{ github.repository }}/actions/workflows/test.yml/runs?head_sha=$SHA \
+              --jq '.workflow_runs | sort_by(.created_at) | last | .id')
+
+            if [ -z "$RUN_ID" ]; then
+              echo "No Test Suite run found yet..."
+              sleep 60
+              continue
+            fi
+
+            STATUS=$(gh api repos/${{ github.repository }}/actions/runs/$RUN_ID/jobs \
+              --jq '.jobs[] | select(.name == "Lint Gate") | .conclusion' | head -1)

-              FAILED=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
-                --jq '[.check_runs[] | select(.name | startswith("Github")) | select(.conclusion == "failure")] | length')
+              FAILED=$(gh api repos/${{ github.repository }}/actions/runs/$RUN_ID/jobs \
+                --jq '[.jobs[] | select(.name | startswith("Github")) | select(.conclusion == "failure")] | length')

-              PENDING=$(gh api repos/${{ github.repository }}/commits/$SHA/check-runs \
-                --jq '[.check_runs[] | select(.name | startswith("Github")) | select(.conclusion == null)] | length')
+              PENDING=$(gh api repos/${{ github.repository }}/actions/runs/$RUN_ID/jobs \
+                --jq '[.jobs[] | select(.name | startswith("Github")) | select(.conclusion == null)] | length')

-            elif [ "$STATUS" = "failure" ]; then
+            elif [ "$STATUS" = "failure" ] || [ "$STATUS" = "cancelled" ] || [ "$STATUS" = "timed_out" ] || [ "$STATUS" = "skipped" ]; then
               echo "::error::Lint Gate failed. Benchmarks will not run."
               exit 1
🤖 Prompt for AI Agents 
In @.github/workflows/bench.yml around lines 37 - 79, Replace the current
check-runs polling with logic that scopes to the latest Test Suite workflow run
and inspects that run's jobs: use the Actions workflow-runs endpoint to find the
most recent run for the Test Suite (scoped to the PR/head SHA stored in SHA),
then call the workflow run jobs endpoint to get job-level conclusions for that
run; look up the "Lint Gate" job conclusion and all jobs whose name starts with
"Github" from that run (not aggregated check-runs), and immediately fail if
"Lint Gate" is any terminal non-success (failure, cancelled, timed_out, skipped,
neutral, etc.), or if any "Github"-prefixed job has a terminal non-success
conclusion; only wait/poll while the latest run has non-terminal
in-progress/null conclusions, and loop by re-querying the latest workflow run id
each iteration so reruns don’t get blocked by earlier runs.


self:
name: "${{ matrix.name }} (${{ matrix.device }}${{ matrix.interface != 'none' && format('-{0}', matrix.interface) || '' }})"
if: ${{ github.repository=='MFlowCode/MFC' && needs.file-changes.outputs.checkall=='true' && ((github.event_name=='pull_request_review' && github.event.review.state=='approved') || (github.event_name=='pull_request' && (github.event.pull_request.user.login=='sbryngelson' || github.event.pull_request.user.login=='wilfonba'))) }}
needs: file-changes
needs: [file-changes, wait-for-tests]
strategy:
fail-fast: false
matrix:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/cleanliness.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ name: Cleanliness

on: [push, pull_request, workflow_dispatch]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
file-changes:
name: Detect File Changes
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/coverage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ name: Coverage Check

on: [push, pull_request, workflow_dispatch]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
file-changes:
name: Detect File Changes
Expand Down
48 changes: 45 additions & 3 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,50 @@
name: 'Test Suite'

on: [push, pull_request, workflow_dispatch]


concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
lint-gate:
name: Lint Gate
runs-on: ubuntu-latest
steps:
- name: Clone
uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.12'

- name: Initialize MFC
run: ./mfc.sh init

- name: Check Formatting
run: |
./mfc.sh format -j $(nproc)
git diff --exit-code || (echo "::error::Code is not formatted. Run './mfc.sh format' locally." && exit 1)

- name: Spell Check
run: ./mfc.sh spelling

- name: Lint Toolchain
run: ./mfc.sh lint

- name: Lint Source - No Raw Directives
run: |
! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*

- name: Lint Source - No Double Precision Intrinsics
run: |
! grep -iR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/*
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern 'double\ precision' (with backslash before space) will match a literal backslash followed by a space, not just a space. In grep basic regex without -E flag, the backslash before a space has no special meaning. The pattern should be 'double precision' (without backslash) to match "double precision" with a space. This affects both the CI workflow and local precheck script.

Suggested change
! grep -iR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/*
! grep -iR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/*

Copilot uses AI. Check for mistakes.

- name: Lint Source - No Junk Code
run: |
! grep -iR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/*

file-changes:
name: Detect File Changes
runs-on: 'ubuntu-latest'
Expand All @@ -21,7 +63,7 @@ jobs:
github:
name: Github
if: needs.file-changes.outputs.checkall == 'true'
needs: file-changes
needs: [lint-gate, file-changes]
strategy:
matrix:
os: ['ubuntu', 'macos']
Expand Down Expand Up @@ -95,7 +137,7 @@ jobs:
self:
name: "${{ matrix.cluster_name }} (${{ matrix.device }}${{ matrix.interface != 'none' && format('-{0}', matrix.interface) || '' }})"
if: github.repository == 'MFlowCode/MFC' && needs.file-changes.outputs.checkall == 'true'
needs: file-changes
needs: [lint-gate, file-changes]
continue-on-error: false
timeout-minutes: 480
strategy:
Expand Down
37 changes: 37 additions & 0 deletions mfc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,39 @@ fi
# Load utility script
. "$(pwd)/toolchain/util.sh"

# Auto-install git pre-commit hook (once, silently)
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)."
fi
Comment on lines +14 to +17
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The auto-install hook check only supports repositories where .git is a directory; in worktrees .git is a file, so the hook never installs and precheck won’t run on commit. Consider resolving the git dir via git rev-parse so it works in worktrees/submodules too.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At mfc.sh, line 14:

<comment>The auto-install hook check only supports repositories where `.git` is a directory; in worktrees `.git` is a file, so the hook never installs and precheck won’t run on commit. Consider resolving the git dir via `git rev-parse` so it works in worktrees/submodules too.</comment>

<file context>
@@ -10,6 +10,12 @@ fi
 . "$(pwd)/toolchain/util.sh"
 
+# Auto-install git pre-commit hook (once, silently)
+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)."
</file context>
Suggested change
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)."
fi
git_dir="$(git rev-parse --git-dir 2>/dev/null)"
hooks_dir="$(git rev-parse --git-path hooks 2>/dev/null)"
if [ -n "$hooks_dir" ] && [ -d "$git_dir" ] && [ ! -e "$hooks_dir/pre-commit" ] && [ -f "$(pwd)/.githooks/pre-commit" ]; then
ln -sf "$(pwd)/.githooks/pre-commit" "$hooks_dir/pre-commit"
log "Installed git pre-commit hook (runs$MAGENTA ./mfc.sh precheck$COLOR_RESET before commits)."
fi
Fix with Cubic


# Auto-install shell completions (once)
COMPLETION_DIR="$HOME/.local/share/mfc/completions"
if [ ! -d "$COMPLETION_DIR" ]; then
mkdir -p "$COMPLETION_DIR"
cp "$(pwd)/toolchain/completions/mfc.bash" "$COMPLETION_DIR/"
cp "$(pwd)/toolchain/completions/_mfc" "$COMPLETION_DIR/"

# Add to shell rc file based on current shell
if [[ "$SHELL" == *"zsh"* ]]; then
RC_FILE="$HOME/.zshrc"
RC_LINE="fpath=(\"$COMPLETION_DIR\" \$fpath)"
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: For zsh, the completion directory is added to fpath but compinit is not rerun, so on a typical .zshrc where compinit is called earlier in the file the newly added directory will not actually be used for completions despite the log message claiming they are installed. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Zsh tab completions not active after installation.
- ⚠️ Users misled by completion-installed success message.
Suggested change
RC_LINE="fpath=(\"$COMPLETION_DIR\" \$fpath)"
RC_LINE="fpath=(\"$COMPLETION_DIR\" \$fpath); autoload -Uz compinit; compinit"
Steps of Reproduction ✅ 
1. Run ./mfc.sh from the repo root on a system without the completions directory; the
script creates completions and sets RC_LINE at mfc.sh:29 (PR hunk).

2. mfc.sh appends RC_LINE to the user's ~/.zshrc (code path around mfc.sh:37-41) but does
not re-run compinit in the current shell.

3. If the user's ~/.zshrc contains an earlier compinit invocation (common setup: "autoload
-Uz compinit; compinit"), the newly appended fpath entry will be processed only on next
shell start. The current shell will not pick up the new fpath, so completions are
effectively inactive.

4. The script logs "Installed tab completions. Restart shell or run: source ..." at
mfc.sh:43, which can mislead users into thinking sourcing the completion file alone is
sufficient; without running compinit after modifying fpath the completion will still not
be activated in zsh.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** mfc.sh
**Line:** 29:29
**Comment:**
	*Logic Error: For zsh, the completion directory is added to `fpath` but `compinit` is not rerun, so on a typical `.zshrc` where `compinit` is called earlier in the file the newly added directory will not actually be used for completions despite the log message claiming they are installed.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

SOURCE_CMD="source $COMPLETION_DIR/_mfc"
else
RC_FILE="$HOME/.bashrc"
RC_LINE="[ -f \"$COMPLETION_DIR/mfc.bash\" ] && source \"$COMPLETION_DIR/mfc.bash\""
SOURCE_CMD="source $COMPLETION_DIR/mfc.bash"
fi

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

log "Installed tab completions. Restart shell or run:$MAGENTA $SOURCE_CMD$COLOR_RESET"
fi

# Print startup message immediately for user feedback
log "Starting..."

Expand Down Expand Up @@ -56,6 +89,10 @@ elif [ "$1" '==' "spelling" ] && [ "$2" != "--help" ] && [ "$2" != "-h" ]; then
. "$(pwd)/toolchain/bootstrap/python.sh"

shift; . "$(pwd)/toolchain/bootstrap/spelling.sh" $@; exit 0
elif [ "$1" '==' "precheck" ]; then
. "$(pwd)/toolchain/bootstrap/python.sh"

shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The precheck command always exits with status 0, ignoring any non-zero status from the precheck script or Python bootstrap, so failures in the underlying checks will not cause this wrapper to fail, breaking its use as a gating/pre-push check. [logic error]

Severity Level: Critical 🚨 
- ❌ ./mfc.sh precheck reports success despite failures.
- ⚠️ Local pre-push validation may be misleading.
- ⚠️ Developers may push changes with lint errors.
Suggested change
shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0
shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit $?
Steps of Reproduction ✅ 
1. From the repository root, run the wrapper command: `./mfc.sh precheck`. This enters the
precheck branch in `mfc.sh` (see `mfc.sh` hunk lines 43-46: the `elif [ "$1" '=='
"precheck" ]` block).

2. `mfc.sh` sources the Python bootstrap at `mfc.sh` line 44 (`.
"$(pwd)/toolchain/bootstrap/python.sh"`), then executes the precheck script by sourcing
`toolchain/bootstrap/precheck.sh` at `mfc.sh` line 46 (`shift; .
"$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0`).

3. If `toolchain/bootstrap/precheck.sh` detects failures and returns/non-zero (for example
a linter or formatting check exits 1), that non-zero status is lost because `mfc.sh`
immediately executes `exit 0` on line 46.

4. Verify by running a deliberately-failing precheck (or a small wrapper
`toolchain/bootstrap/precheck.sh` that does `exit 1`): after `./mfc.sh precheck` observe
the shell exit code is 0 (`echo $?`), proving the wrapper masks failures and reports
success.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** mfc.sh
**Line:** 46:46
**Comment:**
	*Logic Error: The precheck command always exits with status 0, ignoring any non-zero status from the precheck script or Python bootstrap, so failures in the underlying checks will not cause this wrapper to fail, breaking its use as a gating/pre-push check.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Comment on lines +92 to +95
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Quote $@ to preserve argument boundaries.

The unquoted $@ on line 68 will re-split arguments containing spaces (e.g., ./mfc.sh precheck -j "4 cores" would incorrectly become three separate arguments). While this follows the existing pattern used by lint, format, and spelling commands, it's still a latent bug.

Proposed fix 
 elif [ "$1" '==' "precheck" ]; then
     . "$(pwd)/toolchain/bootstrap/python.sh"

-    shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0
+    shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" "$@"; exit 0
 fi

Note: Consider applying this fix to the other commands (lint, format, spelling) for consistency, though that's outside the scope of this PR.

🧰 Tools
🪛 Shellcheck (0.11.0)

[error] 68-68: Double quote array expansions to avoid re-splitting elements.

(SC2068)

🤖 Prompt for AI Agents 
In `@mfc.sh` around lines 65 - 68, The precheck branch in mfc.sh currently calls
precheck.sh with unquoted $@ which can split arguments containing spaces; update
the invocation in the elif block that sources toolchain/bootstrap/precheck.sh to
use "$@" (preserve argument boundaries) so the call is .
"$(pwd)/toolchain/bootstrap/precheck.sh" "$@"; also consider applying the same
change to the similar invocations for lint, format and spelling for consistency.

Copy link
Contributor

@codeant-ai codeant-ai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The arguments to the precheck subcommand are expanded as $@ instead of "$@", so any argument containing whitespace will be split into multiple arguments when passed to precheck.sh, causing its CLI parsing to misbehave for such inputs. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Local precheck CLI parsing can break with spaced arguments.
- ⚠️ Developers using filenames/paths with spaces affected.
Suggested change
shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0
shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" "$@"; exit 0
Steps of Reproduction ✅ 
1. From the repository root, run the precheck command with an argument that contains a
space, e.g.:

   ./mfc.sh precheck "path/with space/file.txt"

   (mfc.sh branch handling starts at mfc.sh:92 in the PR new hunk).

2. Execution enters the precheck branch at mfc.sh:92 and sources python bootstrap at
mfc.sh:93.

3. mfc.sh then executes (mfc.sh:95) the line:

   shift; . "$(pwd)/toolchain/bootstrap/precheck.sh" $@; exit 0

   Because $@ is unquoted, the single argument "path/with space/file.txt" is split into
   two words by the shell.

4. The sourced script toolchain/bootstrap/precheck.sh receives two separate arguments
instead of one, causing its argument parsing to behave incorrectly (incorrect
option/filename boundaries) or to error. This reproduces the described misbehavior.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** mfc.sh
**Line:** 95:95
**Comment:**
	*Logic Error: The arguments to the `precheck` subcommand are expanded as `$@` instead of `"$@"`, so any argument containing whitespace will be split into multiple arguments when passed to `precheck.sh`, causing its CLI parsing to misbehave for such inputs.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

fi

mkdir -p "$(pwd)/build"
Expand Down
138 changes: 138 additions & 0 deletions toolchain/bootstrap/precheck.sh
View file
Open in desktop
Copy link
Contributor

@qodo-code-review qodo-code-review bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-level Suggestion

The linting logic is duplicated in both the test.yml CI workflow and the new precheck.sh script. To improve maintainability and prevent divergence, the CI workflow should call ./mfc.sh precheck directly, making the script the single source of truth. [High-level, importance: 8]

Solution Walkthrough:

Before:

# In .github/workflows/test.yml
jobs:
  lint-gate:
    steps:
      - name: Check Formatting
        run: |
          ./mfc.sh format ...
          git diff --exit-code ...
      - name: Spell Check
        run: ./mfc.sh spelling
      - name: Lint Toolchain
        run: ./mfc.sh lint
      - name: Lint Source - No Raw Directives
        run: ! grep ...
      # ... and other source lint steps

# In toolchain/bootstrap/precheck.sh
# ... similar checks for formatting, spelling, toolchain, and source linting are defined again.

After:

# In .github/workflows/test.yml
jobs:
  lint-gate:
    steps:
      - name: Clone
        ...
      - name: Initialize MFC
        run: ./mfc.sh init
      - name: Run Pre-push Checks
        run: ./mfc.sh precheck

# In toolchain/bootstrap/precheck.sh
# This script becomes the single source of truth for all checks.
# ... checks for formatting, spelling, linting, and source code ...

Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
#!/bin/bash
set -e
set -o pipefail
Comment on lines +1 to +3
Copy link
Contributor

@qodo-code-review qodo-code-review bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Source the logging.sh script at the beginning of precheck.sh. This is necessary to define the log, ok, and error helper functions used throughout the script. [possible issue, importance: 9]

Suggested change
#!/bin/bash
set -e
set -o pipefail
#!/bin/bash
set -e
set -o pipefail
. "$(pwd)/toolchain/bootstrap/logging.sh"


Copy link
Contributor

@codeant-ai codeant-ai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The script assumes it is always run from the repository root by hardcoding ./mfc.sh and ./src/* paths, so invoking precheck.sh from any other working directory (for example, running it directly from toolchain/bootstrap/) will cause it to fail to find mfc.sh or to skip source checks; computing the repo root from the script location and changing to it makes these relative paths reliable. [possible bug]

Severity Level: Major ⚠️ 
- ❌ Local precheck fails when run outside repository root.
- ⚠️ Formatting check errors due to missing ./mfc.sh file.
- ⚠️ Source lint searches miss files if cwd incorrect.
Suggested change
# Ensure we run from the repository root so relative paths like ./mfc.sh and ./src/* resolve correctly
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
cd "$REPO_ROOT"
Steps of Reproduction ✅ 
1. From a clone of the repository, change to the toolchain/bootstrap directory: cd
toolchain/bootstrap. This changes the cwd away from the repo root.

2. Execute the script directly: ./precheck.sh. The script later invokes ./mfc.sh in
toolchain/bootstrap/precheck.sh:67 (if ! ./mfc.sh format -j "$JOBS" > /dev/null 2>&1;
then). Because cwd=toolchain/bootstrap, ./mfc.sh does not exist and bash reports "No such
file or directory" from that line.

3. The script is run with set -e (toolchain/bootstrap/precheck.sh:2), so the missing
./mfc.sh call causes an immediate exit and the formatting check never runs.

4. Additionally, source checks use relative paths such as ./src/* in
toolchain/bootstrap/precheck.sh:107 (grep ... ./src/*) which will not point to repository
sources when cwd is not repo root, causing skipped or failing source lint checks. These
steps reproduce the issue using concrete file and line references.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** toolchain/bootstrap/precheck.sh
**Line:** 4:4
**Comment:**
	*Possible Bug: The script assumes it is always run from the repository root by hardcoding `./mfc.sh` and `./src/*` paths, so invoking `precheck.sh` from any other working directory (for example, running it directly from `toolchain/bootstrap/`) will cause it to fail to find `mfc.sh` or to skip source checks; computing the repo root from the script location and changing to it makes these relative paths reliable.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

# Function to display help message
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The script relies on helper functions (log, ok, error) and color variables (MAGENTA, CYAN, COLOR_RESET) that are not defined anywhere in this file, so if precheck.sh is executed directly in a clean shell (rather than being sourced from a wrapper that defines them), the first call to log will result in "command not found" and the script will exit due to set -e. Adding safe fallback definitions for these helpers ensures the script runs correctly both standalone and when invoked via a parent script that may already define them. [possible bug]

Severity Level: Major ⚠️ 
- ❌ Local `./mfc.sh precheck` may abort with "command not found".
- ⚠️ Developer convenience: standalone usage inconsistent.
Suggested change
# Function to display help message
# Provide fallbacks for logging functions and color variables when not pre-defined
: "${MAGENTA:=}"
: "${CYAN:=}"
: "${COLOR_RESET:=}"
if ! declare -f log >/dev/null 2>&1; then
log() { printf '%s\n' "$*"; }
fi
if ! declare -f ok >/dev/null 2>&1; then
ok() { printf '%s\n' "$*"; }
fi
if ! declare -f error >/dev/null 2>&1; then
error() { printf '%s\n' "$*" >&2; }
fi
Steps of Reproduction ✅ 
1. Invoke the script directly in a clean shell: `./toolchain/bootstrap/precheck.sh` (file
`toolchain/bootstrap/precheck.sh`, execution starts at line 1; `set -e` and `set -o
pipefail` are on lines 2-3).

2. Execution reaches the first logging call at line 40: `log "Running$MAGENTA
precheck$COLOR_RESET (same checks as CI lint-gate)..."` (see final file lines around 40).
The script expects `log`, `ok`, `error`, and color variables to exist.

3. If those functions/variables are not defined by the higher-level wrapper (e.g.
`./mfc.sh`) and the script is executed standalone, the shell reports `./precheck.sh: line
40: log: command not found` and, because `set -e` is enabled (line 2), the script exits
immediately.

4. Observable outcome: precheck aborts with "command not found" instead of running checks.
The suggested fallback definitions ensure the script prints sensible messages when run
standalone.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** toolchain/bootstrap/precheck.sh
**Line:** 5:5
**Comment:**
	*Possible Bug: The script relies on helper functions (`log`, `ok`, `error`) and color variables (`MAGENTA`, `CYAN`, `COLOR_RESET`) that are not defined anywhere in this file, so if `precheck.sh` is executed directly in a clean shell (rather than being sourced from a wrapper that defines them), the first call to `log` will result in "command not found" and the script will exit due to `set -e`. Adding safe fallback definitions for these helpers ensures the script runs correctly both standalone and when invoked via a parent script that may already define them.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

show_help() {
echo "Usage: ./mfc.sh precheck [OPTIONS]"
echo "Run the same fast checks that CI runs before expensive tests start."
echo "Use this locally before pushing to catch issues early."
echo ""
echo "Options:"
echo " -h, --help Display this help message and exit."
echo " -j, --jobs JOBS Runs JOBS number of parallel jobs for formatting."
echo ""
exit 0
}

# Cross-platform hash function (macOS uses md5, Linux uses md5sum)
compute_hash() {
if command -v md5sum > /dev/null 2>&1; then
md5sum | cut -d' ' -f1
elif command -v md5 > /dev/null 2>&1; then
md5 -q
else
# Fallback: use cksum if neither available
cksum | cut -d' ' -f1
fi
}

JOBS=1

while [[ $# -gt 0 ]]; do
case "$1" in
-j|--jobs)
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for the --jobs argument value. If the user runs './mfc.sh precheck -j' without providing a number, JOBS will be set to an empty string, potentially causing issues when passed to './mfc.sh format'. Add a check to ensure $2 is provided and is a valid number before using it.

Suggested change
-j|--jobs)
-j|--jobs)
if [[ -z "$2" || "$2" == -* ]]; then
echo "Precheck: option '$1' requires a positive integer argument."
exit 1
fi
if ! [[ "$2" =~ ^[1-9][0-9]*$ ]]; then
echo "Precheck: jobs value '$2' is not a valid positive integer."
exit 1
fi

Copilot uses AI. Check for mistakes.
if [[ -z "$2" || "$2" == -* ]]; then
echo "Precheck: -j/--jobs requires a value."
exit 1
fi
if ! [[ "$2" =~ ^[0-9]+$ ]]; then
echo "Precheck: jobs value '$2' is not a valid number."
exit 1
fi
JOBS="$2"
shift
;;
-h | --help)
Comment on lines +43 to +46
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 4, 2026 •
edited by qodo-code-review bot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Suggestion: The -j/--jobs argument handling assumes a value is always present; running the script with "-j" (without a value) will cause an extra shift on an empty argument list, triggering "shift: shift count out of range" and immediate exit due to set -e, instead of a clear error about the missing jobs value. The fix is to validate that a value exists for -j/--jobs and to shift exactly two arguments (option and value) when it is provided. [logic error]

Severity Level: Major ⚠️ 
- ❌ Local precheck crashes on `./mfc.sh precheck -j` misuse.
- ⚠️ Developer pre-push validation interrupted, confusing error messages.
Suggested change
JOBS="$2"
shift
;;
-h | --help)
if [[ $# -lt 2 ]] || [[ "$2" == -* ]]; then
echo "Precheck: -j|--jobs requires a positive integer argument."
exit 1
fi
JOBS="$2"
shift 2
continue
;;
-h|--help)
Steps of Reproduction ✅ 
1. From the repository root, run the precheck script directly:
`./toolchain/bootstrap/precheck.sh -j` (this file is `toolchain/bootstrap/precheck.sh`,
see lines 1-4 for `set -e` and the argument loop at lines 20-36).

2. Execution enters the while-case at line 20; the `-j|--jobs)` branch executes lines
22-25 which assign `JOBS="$2"` and perform a single `shift` (line 24), expecting a value
in `$2`.

3. Because no value was provided, `$2` is empty; the script continues and reaches the
unconditional `shift` at line 35. Bash then raises `shift: shift count out of range`
(shift with no positional parameters), and because `set -e` is active (line 2), the script
exits immediately with an error.

4. Observable outcome: abrupt exit with shell `shift` error message instead of a clear,
actionable error about the missing `-j` argument. The fix in the suggestion prevents the
out-of-range shift by validating `$2` and shifting by 2 (lines 22-25 area).
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** toolchain/bootstrap/precheck.sh
**Line:** 23:26
**Comment:**
	*Logic Error: The -j/--jobs argument handling assumes a value is always present; running the script with "-j" (without a value) will cause an extra shift on an empty argument list, triggering "shift: shift count out of range" and immediate exit due to `set -e`, instead of a clear error about the missing jobs value. The fix is to validate that a value exists for -j/--jobs and to shift exactly two arguments (option and value) when it is provided.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

show_help
;;
*)
echo "Precheck: unknown argument: $1."
exit 1
;;
esac

shift
done

FAILED=0
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The script calls logging helpers log, ok, and error without defining them locally or sourcing any shared library, so running this script directly (or from an environment that does not export these functions) will immediately fail with "command not found" when the first log is executed; providing local fallbacks when these functions are absent makes the script robust in all invocation contexts. [possible bug]

Severity Level: Major ⚠️ 
- ❌ Local precheck command fails immediately when run directly.
- ⚠️ Developers cannot run prechecks without wrapper functions.
- ⚠️ CI-wrapper may mask this but direct execution breaks.
Suggested change
FAILED=0
# Provide basic logging helpers if they are not already defined by the caller
if ! declare -F log >/dev/null 2>&1; then
log() { echo "$@"; }
fi
if ! declare -F ok >/dev/null 2>&1; then
ok() { echo "$@"; }
fi
if ! declare -F error >/dev/null 2>&1; then
error() { echo "$@" >&2; }
fi
Steps of Reproduction ✅ 
1. Open the file toolchain/bootstrap/precheck.sh and inspect for logging helpers: there
are no definitions for log/ok/error in this file (verify entire file content shown in PR).

2. Run the script directly from the repository root: bash toolchain/bootstrap/precheck.sh.
Execution reaches the first logging call on toolchain/bootstrap/precheck.sh:60 where the
script runs:

   log "Running$MAGENTA precheck$COLOR_RESET (same checks as CI lint-gate)..."

3. Because log is not defined, bash prints "toolchain/bootstrap/precheck.sh: line 60: log:
command not found" and, due to set -e at toolchain/bootstrap/precheck.sh:2, the script
immediately exits with non-zero status.

4. Confirm by searching file for other uses of ok/error (e.g.,
toolchain/bootstrap/precheck.sh:80, :87, :96, :125, :133) — these will not be reached if
the first log call fails. This reproduces the failure with concrete file:line references.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** toolchain/bootstrap/precheck.sh
**Line:** 58:58
**Comment:**
	*Possible Bug: The script calls logging helpers `log`, `ok`, and `error` without defining them locally or sourcing any shared library, so running this script directly (or from an environment that does not export these functions) will immediately fail with "command not found" when the first `log` is executed; providing local fallbacks when these functions are absent makes the script robust in all invocation contexts.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.


log "Running$MAGENTA precheck$COLOR_RESET (same checks as CI lint-gate)..."
echo ""

# 1. Check formatting
log "[$CYAN 1/4$COLOR_RESET] Checking$MAGENTA formatting$COLOR_RESET..."
# Capture state before formatting
BEFORE_HASH=$(git diff -- '*.f90' '*.fpp' '*.py' 2>/dev/null | compute_hash)
if ! ./mfc.sh format -j "$JOBS" > /dev/null 2>&1; then
error "Formatting check failed to run."
FAILED=1
else
# Check if formatting changed any Fortran/Python files
AFTER_HASH=$(git diff -- '*.f90' '*.fpp' '*.py' 2>/dev/null | compute_hash)
if [ "$BEFORE_HASH" != "$AFTER_HASH" ]; then
error "Code was not formatted. Files have been auto-formatted; review and stage the changes."
echo ""
git diff --stat -- '*.f90' '*.fpp' '*.py' 2>/dev/null || true
echo ""
FAILED=1
else
ok "Formatting check passed."
fi
fi
Comment on lines 63 to 82
Copy link
Contributor

@qodo-code-review qodo-code-review bot Feb 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Suggestion: Simplify the formatting check by replacing the md5sum hash comparison with a git diff --exit-code check. This removes a dependency and makes the script more portable and robust. [general, importance: 7]

Suggested change
# 1. Check formatting
log "[$CYAN 1/4$COLOR_RESET] Checking$MAGENTA formatting$COLOR_RESET..."
# Capture state before formatting
BEFORE_HASH=$(git diff -- '*.f90' '*.fpp' '*.py' 2>/dev/null | md5sum | cut -d' ' -f1)
if ! ./mfc.sh format -j "$JOBS" > /dev/null 2>&1; then
error "Formatting check failed to run."
FAILED=1
else
# Check if formatting changed any Fortran/Python files
AFTER_HASH=$(git diff -- '*.f90' '*.fpp' '*.py' 2>/dev/null | md5sum | cut -d' ' -f1)
if [ "$BEFORE_HASH" != "$AFTER_HASH" ]; then
error "Code is not formatted. Run$MAGENTA ./mfc.sh format$COLOR_RESET to fix."
echo ""
git diff --stat -- '*.f90' '*.fpp' '*.py' 2>/dev/null || true
echo ""
FAILED=1
else
ok "Formatting check passed."
fi
fi
# 1. Check formatting
log "[$CYAN 1/4$COLOR_RESET] Checking$MAGENTA formatting$COLOR_RESET..."
if ! ./mfc.sh format -j "$JOBS" > /dev/null 2>&1; then
error "Formatting check failed to run."
FAILED=1
elif ! git diff -- '*.f90' '*.fpp' '*.py' > /dev/null; then
error "Code is not formatted. Run$MAGENTA ./mfc.sh format$COLOR_RESET to fix."
echo ""
git diff --stat -- '*.f90' '*.fpp' '*.py' 2>/dev/null || true
echo ""
FAILED=1
else
ok "Formatting check passed."
fi


# 2. Spell check
log "[$CYAN 2/4$COLOR_RESET] Running$MAGENTA spell check$COLOR_RESET..."
if ./mfc.sh spelling > /dev/null 2>&1; then
ok "Spell check passed."
else
error "Spell check failed. Run$MAGENTA ./mfc.sh spelling$COLOR_RESET for details."
FAILED=1
fi

# 3. Lint toolchain (Python)
log "[$CYAN 3/4$COLOR_RESET] Running$MAGENTA toolchain lint$COLOR_RESET..."
if ./mfc.sh lint > /dev/null 2>&1; then
ok "Toolchain lint passed."
else
error "Toolchain lint failed. Run$MAGENTA ./mfc.sh lint$COLOR_RESET for details."
FAILED=1
fi

# 4. Source code lint checks
log "[$CYAN 4/4$COLOR_RESET] Running$MAGENTA source lint$COLOR_RESET checks..."
SOURCE_FAILED=0

# Check for raw OpenACC/OpenMP directives
if grep -qiR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/* 2>/dev/null; then
error "Found raw OpenACC/OpenMP directives. Use macros instead."
SOURCE_FAILED=1
fi

# Check for double precision intrinsics
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern 'double\ precision' (with backslash before space) will match a literal backslash followed by a space, not just a space. In grep basic regex, the backslash before a space has no special meaning. The pattern should be 'double precision' (without backslash) to match "double precision" with a space. This matches the same issue in the CI workflow.

Suggested change
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then

Copilot uses AI. Check for mistakes.
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
Comment on lines +113 to +115
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The double-precision intrinsic check uses grep -qiR ... ./src/* 2>/dev/null inside an if without inspecting grep's exit status, so if grep fails for reasons like a missing ./src directory or read errors (exit status >1), the script silently treats it as "no matches" and later reports "Source lint passed," giving a false sense of success. The fix is to distinguish between "no matches" (exit status 1) and real grep errors (>1), and to mark the source lint as failed in the latter case instead of ignoring them. [logic error]

Severity Level: Major ⚠️ 
- ❌ Source lint can falsely report success when scanning fails.
- ⚠️ Local precheck trustworthiness reduced for wrong directory.
Suggested change
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/*; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
else
grep_status=$?
if [ "$grep_status" -gt 1 ]; then
error "Source lint failed: could not scan files for double precision intrinsics."
SOURCE_FAILED=1
FAILED=1
fi
Steps of Reproduction ✅ 
1. From repository root, rename or remove the `src` directory: `mv src src.bak` (this
causes `./src/*` glob to fail). The double-precision check is at
`toolchain/bootstrap/precheck.sh` lines 92-96 (`# Check for double precision intrinsics`
through `fi`).

2. Run `./toolchain/bootstrap/precheck.sh`. The script executes the `grep -qiR ... ./src/*
2>/dev/null` command at line 93; because `./src/*` doesn't expand to files, `grep` exits
with status 2 (error). stderr was redirected to /dev/null, so no error message is shown.

3. The `if` condition tests only for a successful match (exit status 0). On grep exit
status 2, the `if` body is skipped and `SOURCE_FAILED` remains unchanged. Later (lines
104-108) the script reports "Source lint passed." despite the scan failing.

4. Observable outcome: false-positive "Source lint passed" when files were not scanned.
The suggested change inspects grep's exit status and treats scanning errors as failures.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** toolchain/bootstrap/precheck.sh
**Line:** 93:95
**Comment:**
	*Logic Error: The double-precision intrinsic check uses `grep -qiR ... ./src/* 2>/dev/null` inside an `if` without inspecting grep's exit status, so if grep fails for reasons like a missing `./src` directory or read errors (exit status >1), the script silently treats it as "no matches" and later reports "Source lint passed," giving a false sense of success. The fix is to distinguish between "no matches" (exit status 1) and real grep errors (>1), and to mark the source lint as failed in the latter case instead of ignoring them.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

fi
Comment on lines +113 to +116
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, locate and read the precheck.sh file
find . -name "precheck.sh" -type f

Repository: MFlowCode/MFC

Length of output: 90

🏁 Script executed:

# Read the specific lines mentioned in the review
sed -n '113,116p' ./toolchain/bootstrap/precheck.sh

Repository: MFlowCode/MFC

Length of output: 433

🏁 Script executed:

# Check context around those lines
sed -n '100,130p' ./toolchain/bootstrap/precheck.sh

Repository: MFlowCode/MFC

Length of output: 1229

🏁 Script executed:

# Check for potential false positives from 'd0' pattern in fortran source files
echo "=== Potential false positives (d0 in variable names or non-literal contexts) ==="
grep -riH 'd0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src --include="*.f90" --include="*.fpp" 2>/dev/null | grep -v '\.[0-9]*[dD]' | head -30 || echo "No false positive candidates found"

Repository: MFlowCode/MFC

Length of output: 197

🏁 Script executed:

# Check for actual double precision literals (what the pattern should catch)
echo "=== Actual double precision literals (intended matches) ==="
grep -rEH '[0-9]+\.[0-9]*[dD][+-]?[0-9]+|[0-9]+[dD][+-]?[0-9]+' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src --include="*.f90" --include="*.fpp" 2>/dev/null | head -20 || echo "No double precision literals found"

Repository: MFlowCode/MFC

Length of output: 701

🏁 Script executed:

# Count total d0 matches to understand the scope
echo "=== Total 'd0' matches in source ==="
grep -ric 'd0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src --include="*.f90" --include="*.fpp" 2>/dev/null | awk -F: '{sum+=$2} END {print "Total d0 matches:", sum}'

Repository: MFlowCode/MFC

Length of output: 113

The d0 pattern is too broad and fails to catch most double precision literals.

The pattern d0 is intended to catch hardcoded double precision literals like 1.0d0, but it fails to match actual double precision literals in the codebase such as 1d-2 and 5.0d-11 (which contain d- not d0). Additionally, the pattern would match variable names like reserved0 if they weren't already excluded.

The codebase contains double precision literals that should be flagged:

  • src/pre_process/m_start_up.fpp: 1d-2
  • src/simulation/m_bubbles_EL.fpp: 5.0d-11

Use a more specific regex to match floating-point literals with d exponent notation and ensure all violations are caught:

Proposed fix 
-if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
+if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
     error "Found double precision intrinsics. Use generic intrinsics."
     SOURCE_FAILED=1
 fi
+# Separate check for double precision literals (e.g., 1.0d0, 1d-2, 5.0d-11)
+if grep -qER '[0-9]+\.?[0-9]*[dD][+-]?[0-9]+' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
+    error "Found double precision literals (e.g., 1.0d0). Use wp kind parameter."
+    SOURCE_FAILED=1
+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.

Suggested change
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
# Separate check for double precision literals (e.g., 1.0d0, 1d-2, 5.0d-11)
if grep -qER '[0-9]+\.?[0-9]*[dD][+-]?[0-9]+' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision literals (e.g., 1.0d0). Use wp kind parameter."
SOURCE_FAILED=1
fi
🤖 Prompt for AI Agents 
In `@toolchain/bootstrap/precheck.sh` around lines 113 - 116, The existing grep
pattern uses the token "d0" which is too narrow; update the check in the
if-condition (the grep command that currently searches for
'double_precision\|... \|d0') to use an extended regex that matches Fortran
double-precision literals with a 'd' exponent (e.g. 1d-2, 5.0d-11) and switch to
extended regex mode (grep -E or --extended-regexp). Replace the 'd0' fragment
with a pattern such as '([0-9]+(\.[0-9]*)?|\.[0-9]+)d[+-]?[0-9]+' so hardcoded
double literals like 1d-2 and 5.0d-11 are detected while avoiding accidental
matches to non-literals.


# Check for junk code patterns
if grep -qiR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/* 2>/dev/null; then
Comment on lines +107 to +119
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stderr redirection '2>/dev/null' on grep commands differs from the CI implementation in test.yml (lines 38, 42, 46), which doesn't suppress stderr. This could cause the local precheck to silently hide errors that would be visible in CI, creating inconsistent behavior. Remove the '2>/dev/null' redirection to match CI behavior.

Suggested change
if grep -qiR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/* 2>/dev/null; then
error "Found raw OpenACC/OpenMP directives. Use macros instead."
SOURCE_FAILED=1
fi
# Check for double precision intrinsics
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
# Check for junk code patterns
if grep -qiR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/* 2>/dev/null; then
if grep -qiR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*; then
error "Found raw OpenACC/OpenMP directives. Use macros instead."
SOURCE_FAILED=1
fi
# Check for double precision intrinsics
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/*; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
# Check for junk code patterns
if grep -qiR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/*; then

Copilot uses AI. Check for mistakes.
Comment on lines +107 to +119
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error messages for source lint failures don't provide enough actionable information. Users are told that raw directives or forbidden patterns were found, but not shown where they are or how to find them. Consider either showing the grep output or providing guidance on how to locate the offending code.

Suggested change
if grep -qiR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/* 2>/dev/null; then
error "Found raw OpenACC/OpenMP directives. Use macros instead."
SOURCE_FAILED=1
fi
# Check for double precision intrinsics
if grep -qiR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
# Check for junk code patterns
if grep -qiR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/* 2>/dev/null; then
if grep -niR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="acc_macros.fpp" --exclude="omp_macros.fpp" --exclude="shared_parallel_macros.fpp" --exclude="syscheck.fpp" ./src/* 2>/dev/null; then
error "Found raw OpenACC/OpenMP directives. Use macros instead."
SOURCE_FAILED=1
fi
# Check for double precision intrinsics
if grep -niR 'double_precision\|dsqrt\|dexp\|dlog\|dble\|dabs\|double\ precision\|real(8)\|real(4)\|dprod\|dmin\|dmax\|dfloat\|dreal\|dcos\|dsin\|dtan\|dsign\|dtanh\|dsinh\|dcosh\|d0' --exclude-dir=syscheck --exclude="*nvtx*" --exclude="*precision_select*" ./src/* 2>/dev/null; then
error "Found double precision intrinsics. Use generic intrinsics."
SOURCE_FAILED=1
fi
# Check for junk code patterns
if grep -niR -e '\.\.\.' -e '\-\-\-' -e '===' ./src/* 2>/dev/null; then

Copilot uses AI. Check for mistakes.
error "Found junk code patterns (..., ---, ===) in source."
SOURCE_FAILED=1
fi

if [ $SOURCE_FAILED -eq 0 ]; then
ok "Source lint passed."
else
FAILED=1
fi

echo ""

if [ $FAILED -eq 0 ]; then
ok "All precheck tests passed! Safe to push."
exit 0
else
error "Some precheck tests failed. Fix issues before pushing."
exit 1
fi
Loading
Loading