Skip to content

Quality of life improvements for MFC toolchain#1118

Merged
sbryngelson merged 154 commits intoMFlowCode:masterfrom
sbryngelson:test
Feb 5, 2026
Merged

Quality of life improvements for MFC toolchain#1118
sbryngelson merged 154 commits intoMFlowCode:masterfrom
sbryngelson:test

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Jan 31, 2026

User description

PR Type

Enhancement, Tests, Documentation

Description

Major Quality of Life Improvements for MFC Toolchain

  • Centralized Parameter System: Consolidated ~3,300 MFC case parameters into unified registry with descriptions, constraints, and dependencies (params/definitions.py, params/descriptions.py)

  • Enhanced CLI Architecture: Refactored argument parsing with declarative schema-based command definitions, auto-generated argparse parsers, and shell completion support (cli/schema.py, cli/argparse_gen.py, cli/completion_gen.py)

  • Improved User Experience: Added interactive user guide with topic-based help system, contextual tips, and first-time user onboarding (user_guide.py, completion.py)

  • Case Template System: New case template generator with built-in minimal templates and example-based case creation (init.py)

  • Parameter Discovery: New CLI command for searching and exploring parameters with filtering and statistics (params_cmd.py)

  • Enhanced Validation: Comprehensive physics constraint validation for bubbles, patches, time stepping, and acoustic sources with improved error reporting (case_validator.py)

  • Build System Improvements: Added progress bars, streaming output, and detailed error reporting with contextual hints (build.py)

  • Test Infrastructure: Comprehensive parameter validation test suite including negative tests, mutation testing, snapshot regression testing, and coverage analysis (params_tests/ module)

  • Documentation Generation: Auto-generated CLI reference and parameter documentation from schema definitions (cli/docs_gen.py, params/generators/docs_gen.py)

  • Code Quality: Fixed formatting issues across example cases and benchmarks (PEP 8 compliance for imports and comments)

Diagram Walkthrough

flowchart LR
  A["Parameter Registry<br/>definitions.py<br/>descriptions.py"] -->|"loads"| B["Case Validator<br/>Enhanced Constraints"]
  A -->|"powers"| C["Parameter Search<br/>params_cmd.py"]
  D["CLI Schema<br/>schema.py"] -->|"generates"| E["Argparse Parser<br/>argparse_gen.py"]
  D -->|"generates"| F["Shell Completions<br/>completion_gen.py"]
  D -->|"generates"| G["CLI Docs<br/>docs_gen.py"]
  E -->|"used by"| H["Enhanced Args<br/>args.py"]
  H -->|"provides"| I["User Guide<br/>user_guide.py"]
  J["Build System<br/>build.py"] -->|"reports"| I
  B -->|"tested by"| K["Test Suite<br/>negative/mutation/<br/>snapshot/coverage"]
  L["Case Templates<br/>init.py"] -->|"creates"| M["New Cases"]
Loading

File Walkthrough

Relevant files
Enhancement
15 files
descriptions.py
Centralized parameter descriptions and documentation system

toolchain/mfc/params/descriptions.py

  • New file providing centralized parameter descriptions for ~3,300 MFC
    case parameters
  • Includes manual descriptions dictionary extracted from documentation
  • Implements pattern-based auto-generation for indexed parameters (e.g.,
    patch_icpp(N)%geometry)
  • Provides fallback naming convention inference for undocumented
    parameters
+552/-0 
case_dicts.py
Migrate parameter definitions to central registry               

toolchain/mfc/run/case_dicts.py

  • Refactored to load parameter definitions from central registry instead
    of hardcoded dictionaries
  • Replaced ~500 lines of manual parameter type definitions with dynamic
    loading via _load_from_registry()
  • Updated CASE_OPTIMIZATION to derive from registry metadata instead of
    hardcoded list
  • Added comprehensive docstrings to ParamType enum and utility functions
+61/-523
commands.py
Unified CLI command definitions and schema                             

toolchain/mfc/cli/commands.py

  • New file defining all MFC CLI commands as a single source of truth
  • Includes 20+ commands (build, run, test, clean, validate, new, params,
    etc.) with full argument specifications
  • Defines common argument sets, command aliases, and help topics
  • Provides structured schema for generating argparse parsers,
    completions, and documentation
+1003/-0
case_validator.py
Enhanced case validation with physics constraints               

toolchain/mfc/case_validator.py

  • Added check_parameter_types() to validate logical parameters are 'T'
    or 'F'
  • Enhanced check_bubbles_euler() with validation for bubble physics
    parameters (R0ref, p0ref, viscosities, surface tension)
  • Added check_patch_physics() to validate initial condition constraints
    (positive pressure, valid volume fractions, positive dimensions)
  • Improved check_time_stepping() to handle multiple time stepping modes
    (CFL-based, adaptive, fixed)
  • Enhanced check_acoustic_source() with physics validation (positive
    frequency, wavelength, gaussian parameters)
  • Added _is_numeric() helper and _format_errors() for better error
    reporting with hints
+257/-20
definitions.py
Consolidated parameter definitions module with constraint and
dependency support

toolchain/mfc/params/definitions.py

  • New file consolidating ~3,300 parameter definitions into a single
    compact module using loops instead of a definitions/ directory
  • Defines parameter constraints (choices, min/max values) and
    dependencies (requires/recommends relationships)
  • Implements _load() function that registers all parameters across
    COMMON, PRE_PROCESS, SIMULATION, and POST_PROCESS stages
  • Supports indexed parameters for patches, fluids, boundary conditions,
    acoustic sources, probes, and other multi-instance configurations
+394/-0 
user_guide.py
New user guide with enhanced help and interactive onboarding system

toolchain/mfc/user_guide.py

  • New comprehensive user guide module providing enhanced help system
    with Rich formatting
  • Implements topic-based help system covering GPU configuration, cluster
    setup, batch jobs, and debugging
  • Provides contextual tips after build failures, case errors, test
    failures, and run failures
  • Includes onboarding for first-time users with welcome message and
    interactive menu-driven interface
+594/-0 
init.py
New case template generator with built-in and example-based templates

toolchain/mfc/init.py

  • New case template generator module for creating new case files from
    built-in or example templates
  • Provides three built-in minimal templates: 1D_minimal, 2D_minimal, and
    3D_minimal with documented parameters
  • Implements create_case() function to generate new cases from templates
    or copy from examples directory
  • Includes list_templates() to display available templates and
    get_available_templates() for programmatic access
+500/-0 
args.py
Refactored argument parsing with centralized CLI schema and enhanced
help

toolchain/mfc/args.py

  • Refactored argument parsing to use auto-generated parsers from
    centralized CLI schema instead of manual argparse setup
  • Added enhanced help output with print_help(), print_command_help(),
    and topic-based help system integration
  • Implemented _handle_enhanced_help() to provide rich formatting before
    argparse help
  • Deferred test case loading to avoid slow startup for non-test commands
  • Added support for command aliases and improved help for first-time
    users
+106/-151
build.py
Enhanced build system with progress bars and improved error reporting

toolchain/mfc/build.py

  • Added progress bar visualization for build process with ninja/make
    output parsing using regex patterns
  • Implemented _run_build_with_progress() function supporting streaming
    mode (-v) and interactive progress bars
  • Added _show_build_error() to display detailed error information from
    captured subprocess output
  • Enhanced configure, build, and install steps with status indicators
    (✓/✗), timing information, and contextual error tips
  • Improved verbosity levels: default (progress bar), -v (streaming),
    -vv+ (full compiler output)
+339/-7 
completion_gen.py
New shell completion generator for bash and zsh from CLI schema

toolchain/mfc/cli/completion_gen.py

  • New module generating bash and zsh shell completion scripts from CLI
    schema
  • Implements generate_bash_completion() and generate_zsh_completion()
    functions for auto-generated completions
  • Supports completion for commands, options, positional arguments, and
    file types (Python, YAML, pack files)
  • Handles command aliases, mutually exclusive groups, and common
    argument sets in completion generation
+353/-0 
argparse_gen.py
New argparse generator from declarative CLI schema             

toolchain/mfc/cli/argparse_gen.py

  • New module generating argparse parsers from declarative CLI schema
    definitions
  • Implements generate_parser() to convert schema into complete
    ArgumentParser with subcommands and argument groups
  • Handles MFCConfig boolean pair arguments (--flag/--no-flag)
    dynamically from dataclass fields
  • Supports positional arguments, common argument sets, mutually
    exclusive groups, and nested subcommands
+226/-0 
params_cmd.py
Parameter search and discovery CLI command                             

toolchain/mfc/params_cmd.py

  • New module providing CLI command for searching and exploring ~3,300
    MFC case parameters
  • Implements parameter search with filtering by type and collapsing of
    indexed parameters
  • Displays parameter statistics, families, and descriptions with
    configurable output formats
  • Includes helper functions for pattern matching, result formatting, and
    alternative suggestions
+330/-0 
test.py
Enhanced test reporting and failure feedback                         

toolchain/mfc/test/test.py

  • Enhanced test output formatting with improved progress display showing
    test names prominently
  • Added test duration tracking and comprehensive summary report with
    visual panel formatting
  • Implemented real-time failure feedback with error type detection and
    helpful hints
  • Added failed_tests tracking and _print_test_summary() function for
    detailed failure reporting
+136/-18
completion.py
Shell completion installation and management                         

toolchain/mfc/completion.py

  • New module for shell completion installation and management
  • Implements auto-detection and installation for bash and zsh shells
  • Manages completion script installation to
    ~/.local/share/mfc/completions/
  • Provides status checking, uninstall, and RC file configuration
+220/-0 
schema.py
CLI schema dataclass definitions                                                 

toolchain/mfc/cli/schema.py

  • New module defining dataclasses for CLI schema (single source of
    truth)
  • Implements Argument, Positional, Command, CommonArgumentSet, and
    CLISchema dataclasses
  • Defines enums for ArgAction and CompletionType with completion
    configuration
  • Provides helper methods for command lookup and argument flag
    generation
+204/-0 
Formatting
7 files
case.py
Fix comment formatting in example case                                     

examples/2D_shearlayer/case.py

  • Fixed comment formatting from #' to # ' for consistency with Python
    style guidelines
+4/-4     
case.py
Fix import statement formatting                                                   

examples/1D_bubblescreen/case.py

  • Split multi-import statement into separate lines for PEP 8 compliance
+2/-1     
case.py
Code formatting improvement for import statements               

examples/nD_perfect_reactor/case.py

  • Minor formatting change: split import json, argparse into separate
    lines for PEP 8 compliance
+2/-1     
case.py
Code formatting and style improvements                                     

benchmarks/5eq_rk3_weno3_hllc/case.py

  • Reformatted import statements to follow PEP 8 (one import per line)
  • Converted multi-line comments from ## to single # for consistency
+9/-7     
case.py
Comment style normalization                                                           

examples/2D_axisym_shockwatercavity/case.py

  • Converted multi-line comments from ## to single # for consistency
+6/-6     
case.py
Comment style normalization                                                           

examples/2D_ibm_steady_shock/case.py

  • Converted multi-line comment from ## to single # for consistency
+1/-1     
export.py
Import statement formatting                                                           

examples/scaling/export.py

  • Reformatted import statements to follow PEP 8 (one import per line)
+5/-1     
Tests
6 files
negative_tests.py
Negative test case generator for validator constraints     

toolchain/mfc/params_tests/negative_tests.py

  • New test module generating negative test cases that intentionally
    violate validator constraints
  • Defines BASE_CASE with valid parameters and ConstraintTest dataclass
    for test specification
  • Implements generate_constraint_tests() covering simulation domain,
    model equations, time stepping, WENO, boundary conditions, and
    acoustic source constraints
  • Provides run_constraint_tests() and print_test_report() for validation
    and reporting
+358/-0 
mutation_tests.py
Mutation testing for validator coverage analysis                 

toolchain/mfc/params_tests/mutation_tests.py

  • New module for mutation testing that systematically mutates valid
    parameters to verify validator coverage
  • Defines MUTATIONS dictionary with invalid values for ~40 parameters
    across numeric, boolean, physics, and numerics categories
  • Implements mutation application and validation checking with result
    tracking
  • Provides comprehensive reporting on catch rates and missed mutations
    by parameter
+279/-0 
snapshot.py
Validation snapshot tool for regression testing                   

toolchain/mfc/params_tests/snapshot.py

  • New module for capturing and comparing validation snapshots of example
    cases for regression testing
  • Implements CaseSnapshot and ValidationResult dataclasses for storing
    validation state
  • Provides functions to load case parameters, validate across stages,
    and save/load snapshots to JSON
  • Includes comparison logic to detect changes in validation behavior
    across refactoring
+301/-0 
coverage.py
Constraint coverage analysis tool                                               

toolchain/mfc/params_tests/coverage.py

  • New module analyzing constraint coverage by parsing case_validator.py
    AST
  • Extracts all self.prohibit() calls and maps them to validation stages
  • Generates coverage reports showing constraints per method and stage
  • Provides JSON export and console reporting of coverage metrics
+278/-0 
runner.py
Test safety net runner and orchestrator                                   

toolchain/mfc/params_tests/runner.py

  • New main entry point for building and verifying parameter validation
    test suite
  • Implements build_safety_net() to capture parameter inventory,
    validation snapshots, and constraint coverage
  • Provides verify_safety_net() to detect unintended validation behavior
    changes
  • Includes CLI interface supporting build, verify, summary, and report
    generation commands
+258/-0 
inventory.py
Parameter inventory export and analysis                                   

toolchain/mfc/params_tests/inventory.py

  • New module exporting parameter inventory with types and validation
    stages
  • Implements export_parameter_inventory() categorizing parameters by
    stage and type
  • Extracts dynamic parameter patterns with normalization and example
    tracking
  • Provides JSON export and console summary reporting of parameter
    metadata
+176/-0 
Documentation
2 files
docs_gen.py
CLI documentation generator from schema                                   

toolchain/mfc/cli/docs_gen.py

  • New module generating markdown CLI reference documentation from schema
    definitions
  • Formats command sections with usage, arguments, options tables,
    examples, and subcommands
  • Organizes commands by category (core, utility, development, CI, other)
  • Generates quick reference tables and common options documentation
+322/-0 
docs_gen.py
Parameter documentation generator                                               

toolchain/mfc/params/generators/docs_gen.py

  • New module generating markdown documentation for all MFC case
    parameters
  • Organizes parameters by family with descriptions, types, and
    constraints
  • Generates table of contents and detailed family sections with pattern
    grouping for large families
  • Includes command-line reference examples and footer with CLI search
    instructions
+234/-0 
Additional files
74 files
docs.yml +1/-1     
lint-toolchain.yml +3/-0     
settings.json +21/-9   
CMakeLists.txt +1/-1     
README.md +46/-0   
case.py +3/-1     
case.py +3/-1     
case.py +3/-1     
case.py +3/-1     
authors.md +2/-0     
case.md +5/-1     
cli-reference.md +667/-0 
docker.md +2/-0     
expectedPerformance.md +2/-0     
getting-started.md +40/-0   
gpuParallelization.md +2/-0     
papers.md +2/-0     
parameters.md +1000/-0
readme.md +29/-18 
references.md +2/-0     
running.md +2/-0     
testing.md +2/-0     
troubleshooting.md +151/-0 
visualization.md +2/-0     
examples.sh +1/-1     
index.html +1/-1     
plot.py +3/-3     
case.py +1/-1     
case.py +3/-1     
case.py +2/-1     
case.py +2/-1     
case.py +2/-1     
case.py +2/-1     
case.py +2/-1     
case.py +6/-4     
casefile.py +8/-5     
case.py +2/-2     
case.py +9/-7     
case.py +2/-1     
case.py +2/-1     
case.py +6/-4     
case.py +2/-2     
case.py +6/-6     
case.py +6/-6     
analyze.py +2/-1     
case.py +7/-3     
mfc.sh +22/-5   
format.sh +53/-19 
format_python.sh +6/-1     
lint.sh +1/-1     
python.sh +235/-4 
_mfc +449/-0 
mfc.bash +191/-0 
main.py +60/-11 
mfc-case-schema.json +24427/-0
case.py +37/-7   
clean.py +22/-0   
__init__.py +37/-0   
common.py +26/-1   
gen_case_constraints_docs.py +1/-0     
generate.py +116/-0 
ide.py +136/-0 
__init__.py +10/-0   
__init__.py +5/-0     
case_dicts_gen.py +46/-0   
json_schema_gen.py +128/-0 
registry.py +52/-0   
schema.py +88/-0   
validate.py +148/-0 
__init__.py +8/-0     
input.py +20/-5   
run.py +29/-1   
validate.py +58/-0   
pyproject.toml +1/-1     

CodeAnt-AI Description

Centralize parameters, add human-readable parameter descriptions, and unify CLI commands

What Changed

  • Added a complete parameter descriptions registry and pattern-based generators so every case parameter (including indexed ones) now has a clear, human-readable description available.
  • Replaced the ad-hoc per-stage parameter dictionaries with registry-driven stage views and a single source of truth for parameter types, schema, validator, and GPU case-optimization lists — get valid parameter lists and JSON schema from the central registry.
  • Introduced a declarative CLI schema that defines all user-facing commands and options (build, run, test, validate, clean, params, completion, generate, new, packer, etc.), enabling consistent help text, shell completion, examples, and standardized arguments across commands.
  • Added a searchable "params" command and feature groups for exploring parameters by feature/family and for showing descriptions or counts.
  • Added explicit validate and clean commands and surfaced a --case-optimization option in build/run/test flows for producing GPU-optimized builds.

Impact

✅ Clearer parameter descriptions for case authors
✅ Easier pre-flight case validation (validate command + registry schema)
✅ Consistent CLI experience and reliable shell tab-completion

💡 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

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

    • Expanded CLI: init/templates, validate (case-only), params search/describe, generate (completions/docs/schema), completion install/uninstall, clean, interactive mode.
  • Documentation

    • Doxygen-style restructure and many new pages (CLI reference, parameters, troubleshooting, GPU, performance, getting-started) plus README quick-starts and CLI/parameter docs generation.
  • Improvements

    • Centralized parameter registry, richer validation with suggestions, VS Code schema integration, improved build/run progress and verbosity, shell completions and IDE helpers.
  • Tests & CI

    • Extensive new tests and added CI smoke checks, lint/test, and toolchain validation steps.

sbryngelson and others added 30 commits January 27, 2026 20:34
- Add validate command for pre-flight case validation without building
- Add clean command implementation (was previously missing)
- Show detailed CMake/compiler errors on build failure with formatted panels
- Add build progress indicators (Configuring/Building/Installing status)
- Display human-readable test names prominently in test output
- Add real-time test failure feedback with helpful hints
- Enhanced case validator error messages with suggestions
- Add --debug-log flag for troubleshooting
- Create troubleshooting documentation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test Summary:
- Rich panel with formatted pass/fail/skip counts
- Total test duration display
- Failed test details with UUIDs and error types
- Helpful "Next Steps" suggestions for failures

Case Template Generator (./mfc.sh init):
- Create new cases from built-in templates (1D/2D/3D_minimal)
- Copy from any example with --template example:<name>
- List available templates with --list
- Well-documented case files with clear parameter sections

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…oarding

- Add ./mfc.sh completion command to install shell tab-completion
  - Installs to ~/.local/share/mfc/completions/ (works across MFC clones)
  - Automatically configures ~/.bashrc or ~/.zshrc
  - Supports install/uninstall/status subcommands

- Add enhanced help output with Rich formatting
- Add contextual tips after build/test failures
- Add interactive menu mode (./mfc.sh interactive)
- Add welcome message for first-time users with getting started guide
- Add bash and zsh completion scripts
- Update README with toolchain features documentation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused Text import from rich.text
- Add pylint disable for too-many-locals in test()
- Add pylint disable for too-many-arguments in _print_test_summary()
- Rename failed_tests param to failed_test_list to avoid shadowing
- Prefix unused skipped_cases param with underscore
- Add pylint disable for global-statement in setup_debug_logging()

Lint score: 10.00/10

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. Enhanced subcommand help (./mfc.sh build --help):
   - Shows Rich-formatted header with command description
   - Displays practical examples with explanations
   - Lists key options in easy-to-read format
   - Still shows full argparse options below

2. Topic-based help (./mfc.sh help <topic>):
   - gpu: GPU configuration, OpenACC/OpenMP, troubleshooting
   - clusters: HPC cluster setup (Phoenix, Frontier, etc.)
   - batch: Batch job submission options and examples
   - debugging: Debug logging, validation, common issues

3. Command aliases for faster typing:
   - b = build
   - r = run
   - t = test
   - v = validate
   - c = clean

4. Consistent --help behavior:
   - ./mfc.sh --help now shows enhanced help (same as ./mfc.sh)
   - Aliases shown in command table

Updated bash/zsh completion scripts to support new commands.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The 'load' command must be sourced to set environment variables in the
current shell. Running it directly (./mfc.sh load) now shows a clear
error message with the correct usage:

  source ./mfc.sh load -c p -m g

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Unicode progress bar during first-time pip package installation
  Shows "│████████░░░░░░░░░░░░│ 53% (37 pkgs, 1m23s)" in terminals
  Falls back to milestone updates when output is piped

- Defer list_cases() call in args.py until test command is actually used
  Previously generated all ~200 test cases on every startup

- Make pyrometheus and cantera imports lazy in run/input.py
  These heavy chemistry packages are now only imported when needed

These changes fix the apparent "hang" on first run where pip install
showed no progress, and significantly speed up startup time for
non-test commands.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Progress bar now tracks three phases: resolving (0-60%),
  building wheels (60-80%), and installing (80-100%)
- Add "Starting..." message in mfc.sh for immediate user feedback
- Non-TTY mode now shows phase transitions (building, installing)
- Better status messages showing current operation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Display the currently processing package (collecting, downloading, or
building) on a second line below the progress bar. The display updates
in real-time as pip processes each package.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use lazy imports in main.py to defer loading heavy modules
- Remove heavy imports from args.py (run.run, build, test.cases)
- Hardcode target names and template names to avoid import chain
- Import test.cases only when running test command

This reduces startup time from ~1.4s to ~1.0s by avoiding loading
mako, pygments, and other heavy dependencies until they're needed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
uv is a blazing fast Python package installer (10-100x faster than pip).
When uv is detected, use it for package installation (~7 seconds vs
several minutes with pip). Falls back to pip with progress bar when
uv is not available.

Users can install uv with: pip install uv

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Instead of requiring users to manually install uv, automatically
bootstrap it in the venv using pip. This gives all users the speed
benefit of uv (~23 seconds for fresh install vs several minutes
with pip alone).

The bootstrap process:
1. Create venv
2. Install uv via pip (~2-3 seconds)
3. Use uv to install all ~70 packages (~7 seconds)

Falls back to pip with progress bar if uv installation fails.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The exact number of packages varies, so just say "Installing Python
packages" without specifying a count.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Instead of hiding uv's output, show its native progress display when
running in an interactive terminal. This lets users see what's happening
during installation instead of appearing to hang.

For non-interactive (CI) environments, output is still captured for logs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
On HPC systems where cache and venv are on different filesystems,
uv's hardlink attempts fail and fall back to copying. Setting
UV_LINK_MODE=copy skips the failed hardlink attempts, reducing
overhead and eliminating the warning message.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove early exit for 'init' command in mfc.sh that prevented help
  from showing
- Fix _handle_enhanced_help to properly show both enhanced help and
  argparse help for all subcommands
- Add subparser map to print argparse help for specific commands

Now ./mfc.sh init --help (and other subcommands) properly show help.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Make build imports lazy in case.py and run/input.py to break circular
import chains that were exposed by the lazy import optimizations in
main.py and args.py.

- case.py: lazy import build in get_inp() and get_fpp()
- run/input.py: lazy import build in generate_inp(), validate_constraints(),
  and clean()

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Pass --help through to Python for bash-handled commands (clean, lint,
  format, load, spelling) so enhanced help is shown
- Add missing commands to COMMANDS dict in user_guide.py:
  bench, completion, lint, format, spelling

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use argparse's built-in aliases= parameter instead of separate parsers
for command aliases (b, r, t, v, c). This ensures aliases inherit all
arguments from their parent commands.

Previously, ./mfc.sh t would fail because the 't' parser didn't have
test-specific arguments like --rdma-mpi.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Parse ninja's [X/Y] output to show compile progress
- Display progress bar with: file count, percentage, current file, elapsed time
- Falls back to spinner with elapsed time for non-TTY environments
- Shows "(build took Xs)" for builds longer than 5 seconds

In interactive terminals, users now see:
  Building simulation [████████░░░░] 42/156 • 0:02:34 m_rhs.fpp.f90

In non-interactive mode:
  Building simulation...
  (build took 234.5s)
  ✓ Built simulation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The 'init' command was originally used to bootstrap the Python environment
without running any command. This restores that behavior and renames the
case template creation to 'new':

- ./mfc.sh init     -> Just bootstrap venv/environment, then exit
- ./mfc.sh new      -> Create a new case from a template

Updated files:
- mfc.sh: Restore original init behavior (exit after python.sh)
- args.py: Rename init parser to new
- main.py: Handle 'new' command instead of 'init'
- user_guide.py: Update all references from init to new
- init.py: Update usage messages to use 'new'

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When --verbose is used, bypass the progress bar and show raw cmake/compiler
output. This applies to both configure and build steps.

Previously, verbose mode would still capture all output in the progress bar,
making the --verbose flag ineffective.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace black with autopep8 due to Python 3.12.5 memory safety issues
- Fix bash completion to allow file path navigation after commands
- Fix format.sh to accept custom path arguments
- Fix pylint errors in build.py and main.py
- Reformat all Python files with autopep8 for CI consistency

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement a declarative CLI schema that eliminates the need to manually
update 8+ files when adding commands or options:

- Add cli/schema.py with dataclass definitions for CLI structure
- Add cli/commands.py as SINGLE SOURCE OF TRUTH for all commands
- Add cli/argparse_gen.py to generate argparse parsers from schema
- Add cli/completion_gen.py to generate bash/zsh completions
- Add generate.py and ./mfc.sh generate command

Modified files to use the schema:
- args.py: Now uses generated parser from CLI schema
- user_guide.py: Imports COMMANDS from cli/commands.py
- Completion scripts are now auto-generated, not hand-edited

Adding a new command now requires editing only cli/commands.py,
then running ./mfc.sh generate to update completion scripts.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract helper functions to reduce complexity in completion_gen.py and
argparse_gen.py. Add pylint disable for dataclasses that legitimately
need many attributes. Code now scores 10.00/10 on pylint.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add cli/docs_gen.py to generate CLI reference markdown from schema
- Update generate.py to also produce docs/cli-reference.md
- Add generate --check step to lint-toolchain CI workflow
- Generated documentation includes quick reference table, all commands
  with options, examples, and common options section

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This adds toolchain/mfc/params_tests/ with tools to capture and verify
validation behavior before refactoring case_validator.py:

- inventory.py: Export all 3300 parameters with types and stages
- snapshot.py: Capture validation results from 134 example cases
- coverage.py: Analyze 306 constraints across 56 check methods
- runner.py: CLI for build/verify/summary commands

Run `python -m mfc.params_tests.runner build` to capture baseline,
then `python -m mfc.params_tests.runner verify` after changes to
detect any unintended changes to validation behavior.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extends the safety net with two new testing approaches:

1. negative_tests.py - Hand-crafted test cases that intentionally
   violate constraints (20 tests, 100% trigger rate)

2. mutation_tests.py - Automatically mutates valid example cases
   to test validator coverage (830 mutations, 82.8% catch rate)

Mutation testing found real validator gaps:
- t_step_save: No validation for 0 or negative values
- mpp_lim/cyl_coord: Invalid strings like "X" not caught
- x_domain%beg/end: Missing value not validated

Run with:
  python -m mfc.params_tests.runner negative
  python -m mfc.params_tests.runner mutation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mutation testing found several gaps where the validator allowed
invalid parameter values. This commit fixes them:

1. Add check_parameter_types() - validates Fortran logicals are 'T'/'F'
   - mpp_lim, cyl_coord, bubbles_euler, etc. now reject 'X', 'yes', etc.

2. Add x_domain validation - x_domain%beg/end required when m > 0

3. Fix t_step_save - now validates t_step_save > 0

4. Fix dt validation - now validates dt > 0 in all modes where dt is set

5. Improve time stepping validation - properly handles cfl_dt, cfl_adap_dt,
   and adap_dt modes

Mutation test results improved from 82.8% to 99.9% catch rate.
All 134 example cases still pass validation.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement a central parameter registry that serves as the single source
of truth for MFC's ~3,300 case parameters. This eliminates manual
synchronization between case_dicts.py, case_validator.py, and documentation.

Key changes:
- Create mfc/params/ package with schema definitions and registry
- Refactor case_dicts.py from ~680 lines to ~115 lines (uses registry)
- Add parameter definitions organized by domain (core, weno, patches, etc.)
- Add code generators for case_dicts, validator, and documentation
- Add physics validation for patches, acoustics, and bubbles

The registry provides:
- ParamDef dataclass for parameter metadata (name, type, stages, description)
- ConstraintRule dataclass for validation constraints (26 defined)
- Generators that produce case_dicts-compatible output
- Documentation generator (~194K chars of markdown)
- Comparison tool to verify registry matches original

All tests pass:
- Safety net: 134/134 cases unchanged
- Mutation tests: 99.9% catch rate
- JSON schema validation: working

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comment on lines 784 to 801
for pattern in patterns:
if pattern.endswith("%"):
# Prefix pattern: match anything starting with this
# Strip index from param name for comparison
base_name = re.sub(r"\(\d+\)", "", param_name)
if base_name.startswith(pattern[:-1]):
matches.append(param_name)
break
elif "%" in pattern:
# Partial pattern like "patch_icpp%Bx"
# Match patch_icpp(N)%Bx
base_name = re.sub(r"\(\d+\)", "", param_name)
if base_name == pattern or base_name.startswith(pattern):
matches.append(param_name)
break
else:
# Exact match
if param_name == pattern:
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Suggestion: The feature-parameter matching logic only normalizes parameter names (stripping numeric indices in parentheses) for patterns containing '%', so feature entries that use plain base names like "chem_wrt_Y" or "alpha_wrt" will not match indexed parameters such as "chem_wrt_Y(1)" or "alpha_wrt(1)", causing those parameters to be omitted from the corresponding feature groups. Normalizing every parameter name once per iteration and comparing against both the raw and normalized name for exact-match patterns fixes this and makes the matching consistent. [logic error]

Severity Level: Major ⚠️
- ❌ params_cmd.py feature discovery omits indexed parameters
- ⚠️ CLI parameter search returns incomplete results
- ⚠️ Auto-completion may miss indexed parameters
Suggested change
for pattern in patterns:
if pattern.endswith("%"):
# Prefix pattern: match anything starting with this
# Strip index from param name for comparison
base_name = re.sub(r"\(\d+\)", "", param_name)
if base_name.startswith(pattern[:-1]):
matches.append(param_name)
break
elif "%" in pattern:
# Partial pattern like "patch_icpp%Bx"
# Match patch_icpp(N)%Bx
base_name = re.sub(r"\(\d+\)", "", param_name)
if base_name == pattern or base_name.startswith(pattern):
matches.append(param_name)
break
else:
# Exact match
if param_name == pattern:
# Strip numeric indices in parentheses for consistent matching
base_name = re.sub(r"\(\d+\)", "", param_name)
for pattern in patterns:
if pattern.endswith("%"):
# Prefix pattern: match anything starting with this
if base_name.startswith(pattern[:-1]):
matches.append(param_name)
break
elif "%" in pattern:
# Partial pattern like "patch_icpp%Bx"
# Match patch_icpp(N)%Bx
if base_name == pattern or base_name.startswith(pattern):
matches.append(param_name)
break
else:
# Exact match (also allow match against base name without indices)
if param_name == pattern or base_name == pattern:
Steps of Reproduction ✅
1. Open the repository Python REPL or run a small script that imports the function defined
at toolchain/mfc/params/descriptions.py:766 (function get_feature_params).

2. Call get_feature_params("chemistry", ["chem_wrt_Y(1)"]) from the REPL (this simulates a
registry containing an indexed parameter chem_wrt_Y(1)). Expectation: the feature group
"chemistry" (FEATURE_GROUPS entry) lists "chem_wrt_Y" but the function returns [] (missing
the indexed parameter).

3. Trace execution inside toolchain/mfc/params/descriptions.py between lines 766–805: the
loop over all_param_names runs, and for the pattern "chem_wrt_Y" the code reaches the
exact-match branch (the final else) which compares param_name == pattern (existing code)
and does not compare the normalized base name without "(1)". As a result, "chem_wrt_Y(1)"
is not considered a match and is omitted.

4. Verify corrected behavior by applying the improved code: re-run
get_feature_params("chemistry", ["chem_wrt_Y(1)"]) and observe the result contains
["chem_wrt_Y(1)"] because the improved code normalizes base_name once per loop and
compares base_name == pattern, causing the indexed parameter to be included.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** toolchain/mfc/params/descriptions.py
**Line:** 784:801
**Comment:**
	*Logic Error: The feature-parameter matching logic only normalizes parameter names (stripping numeric indices in parentheses) for patterns containing '%', so feature entries that use plain base names like "chem_wrt_Y" or "alpha_wrt" will not match indexed parameters such as "chem_wrt_Y(1)" or "alpha_wrt(1)", causing those parameters to be omitted from the corresponding feature groups. Normalizing every parameter name once per iteration and comparing against both the raw and normalized name for exact-match patterns fixes this and makes the matching consistent.

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 +21 to +23
show_features = ARG("features")
feature_name = ARG("feature")
names_only = ARG("names_only")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: The new optional CLI flags ("features", "feature", and "names_only") are read via ARG without defaults, so if the argument registry does not define these keys (e.g., in older tests or non-CLI callers), calling params() will raise a KeyError instead of simply treating the options as disabled; they should default to False/None to avoid spurious crashes. [possible bug]

Severity Level: Major ⚠️
- ❌ params CLI command crashes with KeyError when flags missing.
- ⚠️ Direct Python callers importing params_cmd may raise KeyError.
- ⚠️ Batch scripts/tests invoking params() programmatically affected.
Suggested change
show_features = ARG("features")
feature_name = ARG("feature")
names_only = ARG("names_only")
show_features = ARG("features", False)
feature_name = ARG("feature", None)
names_only = ARG("names_only", False)
Steps of Reproduction ✅
1. Import and call the params() function directly: run Python and execute:

   from toolchain.mfc.params_cmd import params; params()

   This executes the ARG lookups inside params() at toolchain/mfc/params_cmd.py lines
   ~18-30 (the ARG calls at lines 21-23).

2. Inside params(), the code calls ARG("features"), ARG("feature"), ARG("names_only")
(toolchain/mfc/params_cmd.py:21-23). ARG delegates to the global argument registry.

3. If gARG does not contain those keys, state.ARG raises KeyError per its implementation
at toolchain/mfc/state.py:85-93 ("raise KeyError(f\"{arg} is not an argument.\")").

4. Observe a KeyError traceback originating from state.ARG (toolchain/mfc/state.py:85-93)
when params() is invoked without those keys set. This reproduces the crash
deterministically — calling params() without the three keys present triggers the error.
(This is not hypothetical: ARG explicitly raises KeyError when the key is absent.)
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** toolchain/mfc/params_cmd.py
**Line:** 21:23
**Comment:**
	*Possible Bug: The new optional CLI flags ("features", "feature", and "names_only") are read via ARG without defaults, so if the argument registry does not define these keys (e.g., in older tests or non-CLI callers), calling params() will raise a KeyError instead of simply treating the options as disabled; they should default to False/None to avoid spurious crashes.

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.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 3, 2026

CodeAnt AI Incremental review completed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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/mfc/params/descriptions.py`:
- Around line 766-804: The partial-pattern branch in get_feature_params is too
permissive: when a pattern contains "%" but doesn't end with it (patterns
variable), the code currently allows base_name.startswith(pattern) which can
wrongly match longer names; change the elif "%" in pattern branch to only
compare the stripped base_name for exact equality to the pattern (using the same
re.sub(r"\(\d+\)", "", param_name) logic already used) and remove the startswith
check so that non-wildcard patterns only match exact names.
🧹 Nitpick comments (2)
toolchain/mfc/params_cmd.py (2)

15-16: Remove unused noqa directive.

The static analysis tool flagged that the noqa: F401 directive on line 16 is unnecessary since F401 is not enabled. The import of definitions appears intentional (likely for side effects), but the noqa comment serves no purpose.

🔧 Suggested fix
     from .params import REGISTRY
-    from .params import definitions  # noqa: F401  pylint: disable=unused-import
+    from .params import definitions  # pylint: disable=unused-import

167-179: Redundant conditional branches produce identical output.

Lines 168-171 and 173-176 have identical branches - both produce the same range string format regardless of whether the indices are contiguous:

if idx1_vals == list(range(min(idx1_vals), max(idx1_vals) + 1)):
    range_parts.append(f"N={min(idx1_vals)}..{max(idx1_vals)}")
else:
    range_parts.append(f"N={min(idx1_vals)}..{max(idx1_vals)}")  # Same!

Either remove the contiguity check or differentiate the output (e.g., show individual values for non-contiguous ranges).

🔧 Suggested fix (simplify)
             if idx1_vals:
-                if idx1_vals == list(range(min(idx1_vals), max(idx1_vals) + 1)):
-                    range_parts.append(f"N={min(idx1_vals)}..{max(idx1_vals)}")
-                else:
-                    range_parts.append(f"N={min(idx1_vals)}..{max(idx1_vals)}")
+                range_parts.append(f"N={min(idx1_vals)}..{max(idx1_vals)}")
             if idx2_vals:
-                if idx2_vals == list(range(min(idx2_vals), max(idx2_vals) + 1)):
-                    range_parts.append(f"M={min(idx2_vals)}..{max(idx2_vals)}")
-                else:
-                    range_parts.append(f"M={min(idx2_vals)}..{max(idx2_vals)}")
+                range_parts.append(f"M={min(idx2_vals)}..{max(idx2_vals)}")
             if idx3_vals:
                 range_parts.append(f"K={min(idx3_vals)}..{max(idx3_vals)}")

- Add tags field to ParamDef schema for feature-based grouping
- Add get_params_by_tag() and get_all_tags() to registry
- Register tags directly at parameter definition time (single source of truth)
- Remove stage-based organization, make all parameters universal
- Reorganize definitions.py by feature groups instead of stages
- Simplify descriptions.py to only contain feature descriptions

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 `@toolchain/mfc/params_cmd.py`:
- Around line 15-16: Remove the redundant noqa directive from the side-effect
import of definitions in params_cmd.py: keep the import statement "from .params
import definitions" (so the module is still imported for side effects) but
delete the trailing "# noqa: F401" while retaining the "pylint:
disable=unused-import" if you want to silence pylint; ensure REGISTRY import and
the definitions import remain otherwise unchanged.

In `@toolchain/mfc/params/definitions.py`:
- Around line 205-206: The _load function triggers Pylint R0912 (too many
branches); to unblock CI either explicitly silence that specific check on the
function or refactor into smaller helpers: add a function-level disable for
R0912 to the _load definition (e.g., keep def _load(): but add the pylint
disable for R0912) or extract independent logical blocks from _load into named
helper functions (e.g., _load_section_x, _load_validations, _load_defaults) and
call them from _load so the original function's branch count drops below the
limit; update any imports/locals used by those helpers accordingly.

In `@toolchain/mfc/params/schema.py`:
- Around line 50-72: ParamDef currently triggers Pylint R0902 for too many
instance attributes; to unblock CI add a class-level pylint disable for R0902
immediately above the dataclass definition of ParamDef (i.e., annotate the
ParamDef declaration to suppress R0902) so the linter ignores this class, or
alternatively refactor by grouping metadata into a nested dataclass used by
ParamDef (e.g., move constraints, dependencies, tags into a ParamMetadata
dataclass and replace those fields with a single metadata: ParamMetadata field)
and update references to ParamDef accordingly.

Comment on lines +15 to +16
from .params import REGISTRY
from .params import definitions # noqa: F401 pylint: disable=unused-import
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused noqa directive.

Ruff flags # noqa: F401 on Line 16 as unused (RUF100). Keep the side-effect import, but drop the redundant noqa.

🔧 Suggested fix
-from .params import definitions  # noqa: F401  pylint: disable=unused-import
+from .params import definitions  # pylint: disable=unused-import
📝 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
from .params import REGISTRY
from .params import definitions # noqa: F401 pylint: disable=unused-import
from .params import REGISTRY
from .params import definitions # pylint: disable=unused-import
🧰 Tools
🪛 Ruff (0.14.14)

[warning] 16-16: Unused noqa directive (non-enabled: F401)

Remove unused noqa directive

(RUF100)

🤖 Prompt for AI Agents
In `@toolchain/mfc/params_cmd.py` around lines 15 - 16, Remove the redundant noqa
directive from the side-effect import of definitions in params_cmd.py: keep the
import statement "from .params import definitions" (so the module is still
imported for side effects) but delete the trailing "# noqa: F401" while
retaining the "pylint: disable=unused-import" if you want to silence pylint;
ensure REGISTRY import and the definitions import remain otherwise unchanged.

The Stage categorization (COMMON, PRE_PROCESS, SIMULATION, POST_PROCESS)
added complexity without benefit since users run all three codes anyway.

- Remove Stage enum from schema.py
- Remove stages field from ParamDef
- Remove _by_stage tracking from registry
- Simplify case_dicts.py to just export ALL params
- Update all tests to remove stage references
- Update inventory.py to use tags instead of stages

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 `@toolchain/mfc/params_cmd.py`:
- Around line 164-173: The current logic in params_cmd.py building range_parts
uses min..max for both contiguous and non-contiguous idx1_vals/idx2_vals, hiding
gaps; modify the branches that handle idx1_vals and idx2_vals so that when the
set is contiguous (idx1_vals == list(range(min(...), max(...)+1))) you keep the
"N=min..max" / "M=min..max" format, but when non-contiguous you emit an explicit
list (e.g., "N={v1,v2,...}" or comma-separated values) instead of min..max;
update the code around the idx1_vals/idx2_vals checks and the range_parts append
logic to generate the explicit list for non-contiguous cases so users see gaps.

In `@toolchain/mfc/run/case_dicts.py`:
- Around line 68-71: ARG may raise KeyError when "case_optimization" is missing;
change the check in the block using ARG, target_name, ALL and CASE_OPTIMIZATION
so it defaults to False when the argument is absent (e.g., call ARG with a
default or catch KeyError) and then return list(ALL.keys()) when case
optimization is not enabled or target_name != "simulation"; otherwise filter
ALL.keys() by excluding CASE_OPTIMIZATION as before.

Descriptions are now auto-generated at registration time, providing
a single source of truth in definitions.py.

- Add _auto_describe() function that parses param names
- Add _PREFIX_DESCS for indexed family descriptions (patch_icpp, etc.)
- Add _ATTR_DESCS for common attribute descriptions (geometry, vel, etc.)
- Add _SIMPLE_DESCS for simple parameter descriptions (~80 params)
- Update _r() to accept optional desc= override
- Update get_description() to use ParamDef.description first

DESCRIPTIONS dict and PATTERNS list in descriptions.py are kept as
fallback but can be removed once all params have good auto-descriptions.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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 2 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/mfc/params/definitions.py">

<violation number="1" location="toolchain/mfc/params/definitions.py:239">
P3: The attribute regex in `_auto_describe` only matches lowercase letters, so uppercase attributes like `Re(1)` never use the `_ATTR_DESCS` mapping and produce degraded descriptions. Expand the regex to include uppercase letters so parameter docs resolve to the intended descriptions.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

sbryngelson and others added 2 commits February 3, 2026 17:24
- Add descriptions for grid stretching, body forces, output fields
- Remove duplicate mpp_lim entry
- Change params command default limit from 25 to unlimited

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Tab completion: Add backward-scanning for multi-value arguments
  (nargs="+" or "*") so completing works after the first value
  e.g., ./mfc.sh b -t pre_process sim<TAB> now completes to simulation

- Fortran namelist: Convert T/F to .true./.false. for compatibility
  with strict compilers (some Mac compilers reject bare T/F)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 `@toolchain/mfc/cli/completion_gen.py`:
- Around line 300-345: The zsh generator (_generate_zsh_command_args) omits
several mfc_config_flags present in the bash completions; update the
mfc_config_flags branch (the block guarded by if common_set.mfc_config_flags) to
include the missing paired flags --gcov/--no-gcov, --unified/--no-unified,
--single/--no-single, --mixed/--no-mixed, and --fastmath/--no-fastmath (use the
same pattern as existing --mpi/--no-mpi and --gpu/--no-gpu entries), or better
yet refactor to centralize the common flag definitions so both bash and zsh use
the same source (e.g., move the literal flag list into a shared
constant/function and reference it from _generate_zsh_command_args and the bash
generator) so completions remain in sync.
- Around line 159-177: The current _generate_bash_command_case builds only
top-level subcommand name completion and returns early, so option/positional
completions for the chosen subcommand never run; replace the early return when
cmd.subcommands is truthy with a nested case on ${COMP_WORDS[2]} that dispatches
to the selected subcommand's completion block. Concretely, keep the subcmd_names
pattern but instead of returning, append a line like '            case
"${COMP_WORDS[2]}" in' and for each subcommand in cmd.subcommands call the same
generator logic (e.g., reuse _generate_bash_command_case or inline its logic for
that subcommand) to produce its options/positionals, then close the nested case
and only after that append the ';;' for the parent pattern so option/positional
completion of the selected subcommand runs. Ensure variables referenced
(patterns, options, cmd.subcommands, subcmd_names) match existing names and
remove the early return.

In `@toolchain/mfc/params/definitions.py`:
- Around line 282-338: The function _auto_describe has too many return
statements causing Pylint R0911; to quickly fix CI either extract chunks into
small helpers (e.g., _describe_indexed_prefix, _describe_percent_attr,
_describe_suffix_indexed) and call them from _auto_describe to reduce returns,
or add an explicit Pylint disable for this function by placing a disable comment
for R0911 on the _auto_describe definition; update references to helper names
(_describe_indexed_prefix, _describe_percent_attr, _describe_suffix_indexed) if
you choose refactoring so logic and tests remain unchanged.
🧹 Nitpick comments (2)
toolchain/mfc/cli/completion_gen.py (1)

66-70: Make the Optional type explicit in the signature.
This avoids implicit Optional per PEP 484 and aligns with typing best practices.

💡 Proposed typing fix
-from typing import List, Set
+from typing import List, Set, Optional
...
-def _bash_completion_for_type(comp_type: CompletionType, choices: List[str] = None) -> str:
+def _bash_completion_for_type(comp_type: CompletionType, choices: Optional[List[str]] = None) -> str:

Please confirm the project’s minimum Python version and typing policy to ensure this aligns with your standards.

toolchain/mfc/case.py (1)

87-117: Use PEP 604 union syntax and preserve exception context with chaining.

The codebase already uses str | None syntax in gen_case_constraints_docs.py, confirming Python 3.10+ support. Additionally, exception chaining with from e is a pattern used throughout the codebase (run/input.py, common.py), yet it's missing in the current error handling. Apply both changes for consistency:

🔧 Proposed changes
-    def validate_params(self, origin_txt: str = None):
+    def validate_params(self, origin_txt: str | None = None):
@@
-        except fastjsonschema.JsonSchemaException as e:
-            if origin_txt:
-                raise common.MFCException(f"{origin_txt}: {e}")
-            raise common.MFCException(f"{e}")
+        except fastjsonschema.JsonSchemaException as e:
+            if origin_txt:
+                raise common.MFCException(f"{origin_txt}: {e}") from e
+            raise common.MFCException(f"{e}") from e

sbryngelson and others added 2 commits February 3, 2026 18:29
Use ARG("case_optimization", dflt=False) so the function works
in test environments where CLI args aren't initialized.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The T/F format is correct - the namelist issue was due to
stale binaries, not the format of logical values.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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/mfc/case.py">

<violation number="1">
P1: Boolean parameters are no longer converted to Fortran logical literals. Emitting `T`/`F` directly makes the generated .inp invalid for Fortran logicals. Restore the `.true.`/`.false.` conversion for string booleans before falling back to the generic handling.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

sbryngelson and others added 2 commits February 3, 2026 18:39
Added --gcov, --unified, --single, --mixed, --fastmath and their
--no-* variants to zsh completion for consistency with bash.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When we removed the Stage system, all params were written to all targets.
But some params (like run_time_info) only exist in simulation's Fortran
namelist, not pre_process. This caused "Invalid line in namelist" errors.

Added _SIMULATION_ONLY and _POST_PROCESS_ONLY sets to filter params
that should not be written to other targets' .inp files.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 `@toolchain/mfc/cli/completion_gen.py`:
- Around line 275-292: The _zsh_completion_for_arg function omits
CompletionType.FILES_YAML handling; add a branch for CompletionType.FILES_YAML
(similar to the FILES_PY/FILES_PACK cases) that returns f'{label}:_files -g
"*.yaml"' (and possibly "*.yml" if desired) so argument completions support YAML
file globs; update the condition checks in _zsh_completion_for_arg to include
CompletionType.FILES_YAML.
- Around line 256-272: The _zsh_completion_for_positional function is missing
handling for CompletionType.FILES_YAML, so positional args that expect YAML
files get no zsh completion; update _zsh_completion_for_positional to include an
elif branch for CompletionType.FILES_YAML (similar to FILES_PY/FILES_PACK) that
sets completion to a file glob for YAML (e.g. ':_files -g "*.yml" -g "*.yaml"'),
ensure help_text processing remains the same and reference
CompletionType.FILES_YAML and the _zsh_completion_for_positional function when
making the change.
🧹 Nitpick comments (3)
toolchain/mfc/cli/completion_gen.py (2)

66-70: Use explicit Optional or union syntax for nullable parameter.

The choices parameter defaults to None but the type hint doesn't reflect this. PEP 484 prohibits implicit Optional.

♻️ Suggested fix
-def _bash_completion_for_type(comp_type: CompletionType, choices: List[str] = None) -> str:
+def _bash_completion_for_type(comp_type: CompletionType, choices: List[str] | None = None) -> str:

395-404: Minor style improvement for list construction.

Similar to line 164, line 397 can use unpacking syntax per Ruff's suggestion.

♻️ Suggested fix
-        all_names = [cmd.name] + cmd.aliases
+        all_names = [cmd.name, *cmd.aliases]
toolchain/mfc/case.py (1)

89-95: Preserve schema error context with exception chaining.
Use raise ... from e to keep the original traceback for debugging.

🔧 Proposed change
-        except fastjsonschema.JsonSchemaException as e:
-            if origin_txt:
-                raise common.MFCException(f"{origin_txt}: {e}")
-            raise common.MFCException(f"{e}")
+        except fastjsonschema.JsonSchemaException as e:
+            if origin_txt:
+                raise common.MFCException(f"{origin_txt}: {e}") from e
+            raise common.MFCException(f"{e}") from e

sbryngelson and others added 2 commits February 3, 2026 18:53
Split exclusions into three categories:
- _SIMULATION_ONLY: params only in simulation (not pre or post)
- _SIM_AND_POST: params in simulation AND post_process (not pre)
- _POST_PROCESS_ONLY: params only in post_process

pre_process now correctly excludes all non-pre_process params.
post_process excludes only simulation-only params.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added __ensure_generated_files() check at startup that compares mtimes
of source files (commands.py, completion_gen.py, definitions.py) against
generated files (mfc.bash, mfc-case-schema.json). If sources are newer,
automatically regenerates the files with a brief message.

This means users no longer need to manually run ./mfc.sh generate after
pulling changes that modify CLI commands or parameters.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a 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/main.py">

<violation number="1" location="toolchain/main.py:48">
P2: The regeneration check ignores the zsh completion file (_mfc), so missing or outdated zsh completions will not trigger regeneration. Include _mfc in the generated list so zsh completions stay in sync.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

sbryngelson and others added 6 commits February 3, 2026 19:18
The auto-generated description function in definitions.py
legitimately has many branches and return statements for
pattern matching. While these R-messages are informational
(score remains 10.00/10), pylint returns exit code 8 when
any refactor messages are present, causing CI to fail with
set -e enabled.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add _PRE_PROCESS_ONLY set containing params like num_patches,
grid stretching, mixing layer, perturbation, and other initial
condition setup params that only exist in pre_process's Fortran
namelist.

Also add prefix matching for indexed params like patch_icpp(N)%...
and patch_bc(N)%... which are pre_process-only.

This fixes "Invalid line in namelist: num_patches = 2" errors
when running simulation after pre_process.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add _SIMULATION_ONLY_PREFIXES for indexed params like acoustic(N)%...,
  probe(N)%..., integral(N)%..., lag_params%, chem_params%
- Add _PRE_AND_POST set for sigR (in pre+post but not simulation)
- Add body force params (bf_x, k_x, w_x, etc.) to simulation-only
- Add hyper_cleaning_speed/tau to simulation-only
- Create _is_simulation_only() helper for prefix matching

This fixes "Invalid line in namelist" errors for acoustic, sigR, and
other params that were incorrectly written to targets that don't
accept them.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add _POST_PROCESS_ONLY_PREFIXES for omega_wrt(, schlieren_alpha(,
  x_output%, y_output%, z_output%
- Add _is_post_process_only() helper function
- Add missing post_process-only params: alpha_rho_e_wrt, chem_wrt_Y,
  chem_wrt_T, lag_r0_wrt, lag_betaT_wrt, lag_betaC_wrt, G
- Update filtering logic to use prefix matching for post_process-only

Fixes "Invalid line in namelist: omega_wrt(1) = T" errors when running
pre_process.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
n_start is in ALL THREE Fortran namelists (pre_process, simulation,
post_process), but was incorrectly placed in _SIM_AND_POST which
excluded it from pre_process.

This caused test failures like "No reference to D/cons.1.00.000000.dat"
because pre_process wasn't getting the correct n_start value.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Instead of maintaining error-prone blacklists of which params belong
to which targets, parse the actual Fortran namelist definitions from
src/*/m_start_up.fpp to determine valid params for each target.

This approach:
- Uses Fortran source as the single source of truth
- Automatically handles indexed params (patch_icpp, acoustic, etc.)
- Eliminates the need to manually update Python when Fortran changes
- Is simpler and more maintainable

New file: toolchain/mfc/params/namelist_parser.py
- Parses Fortran namelist blocks from each target's m_start_up.fpp
- Extracts parameter names, handling line continuations and preprocessor directives
- Caches results for performance

Updated: toolchain/mfc/run/case_dicts.py
- Removed all manual exclusion sets (_PRE_PROCESS_ONLY, _SIMULATION_ONLY, etc.)
- Now uses namelist_parser to determine valid params for each target
- Much simpler implementation (~100 lines vs ~230 lines)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sbryngelson sbryngelson merged commit 1d3286a into MFlowCode:master Feb 5, 2026
56 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 4/5 size:XXL This PR changes 1000+ lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

1 participant