Skip to content

Refactor/cli#111

Merged
ryanmccann1024 merged 30 commits intorelease/6.0.0from
refactor/cli
Sep 9, 2025
Merged

Refactor/cli#111
ryanmccann1024 merged 30 commits intorelease/6.0.0from
refactor/cli

Conversation

@ryanmccann1024
Copy link
Copy Markdown
Collaborator

@ryanmccann1024 ryanmccann1024 commented Aug 13, 2025

📋 Pull Request Summary

PR Title: refactor(cli): implement modular CLI architecture with comprehensive configuration system

Related Issue(s):

Description:
This PR implements a comprehensive refactoring of the CLI architecture to support modular design patterns and improved configuration management. The refactor includes:

  • Modular CLI Architecture: Migrated from monolithic CLI scripts to a centralized registry-based system with modular parsers
  • Comprehensive Configuration Management: Implemented unified configuration system with INI fallback support and validation
  • Package Restructuring: Reorganized codebase into logical modules (core/, cli/, modules/, etc.) for better maintainability
  • Integration Improvements: Enhanced GUI-CLI integration and improved test coverage across all modules
  • Infrastructure Updates: Added local development support, PR validation workflows, and improved exception handling

🔧 Type of Change

Primary Change Type:

  • 🐛 Bug Fix - Non-breaking change that fixes an issue
  • New Feature - Non-breaking change that adds functionality
  • 💥 Breaking Change - Change that would cause existing functionality to break
  • 🔄 Refactor - Code change that neither fixes a bug nor adds a feature
  • 📚 Documentation - Documentation only changes
  • 🧪 Tests - Adding missing tests or correcting existing tests
  • 🏗️ Build/CI - Changes to build process or CI configuration
  • 🎨 Style - Code style changes (formatting, missing semicolons, etc.)
  • Performance - Performance improvements
  • 🔒 Security - Security vulnerability fixes

Component(s) Affected:

  • CLI Interface (fusion/cli/)
  • Configuration System (fusion/configs/)
  • Simulation Core (fusion/core/)
  • ML/RL Modules (fusion/modules/rl/, fusion/modules/ml/)
  • Routing Algorithms (fusion/modules/routing/)
  • Spectrum Assignment (fusion/modules/spectrum/)
  • SNR Calculations (fusion/modules/snr/)
  • Visualization (fusion/visualization/)
  • GUI Interface (fusion/gui/)
  • Unity/HPC Integration (fusion/unity/)
  • Testing Framework (tests/)
  • Documentation
  • GitHub Workflows (.github/)
  • Build/Dependencies

🧪 Testing

Test Coverage:

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • Existing tests still pass
  • Performance impact assessed

Test Details:
All test files have been updated to work with the new modular architecture. Key testing areas include:

  • CLI argument parsing and configuration loading
  • Module import resolution and package structure
  • RL pipeline integration and environment reset handling
  • GUI-CLI integration and runner functionality
  • Cross-platform compatibility and workflow validation

Test Configuration Used:

# Standard test configuration for simulation runs
[general_settings]
algorithm = dijkstra
node_file = TopologyZoo/GtsCe.graphml
num_requests = 1000
run_id = test_refactor_cli

Commands to Reproduce Testing:

# Run full test suite
python -m pytest tests/

# Test CLI functionality
python -m fusion.cli.run_sim run_sim --config_path test_config.ini --run_id test

# Test specific modules
python -m pytest tests/test_cli/ -v
python -m pytest tests/test_modules/ -v

Test Results:

  • Operating System: macOS Darwin 24.5.0
  • Python Version: 3.x (compatible with project requirements)
  • Test Environment: Local development with CI validation

📊 Impact Analysis

Performance Impact:

  • No performance impact
  • Performance improved
  • Minor performance decrease (acceptable)
  • Significant performance impact (needs discussion)

Memory Usage:

  • No change in memory usage
  • Memory usage optimized
  • Minor increase in memory usage
  • Significant memory impact

Backward Compatibility:

  • Fully backward compatible
  • Minor breaking changes with migration path
  • Major breaking changes (requires version bump)

Dependencies:

  • No new dependencies
  • New dependencies added (list in Additional Notes)
  • Dependencies removed/updated

🔄 Migration Guide

Breaking Changes (if any):

  • Import paths have changed due to package restructuring (e.g., fusion.cli.args instead of top-level imports)
  • CLI entry points now use modular architecture with centralized registry
  • Configuration loading now supports INI fallback with --config_path parameter

Migration Steps:

  1. Update import statements to use new package structure (fusion.cli.*, fusion.core.*, etc.)
  2. Update CLI invocations to use new modular entry points
  3. Update configuration files to use new schema (if applicable)
  4. Update any custom scripts to reference new module locations

Before/After Examples:

# Before (old usage)
from run_sim import main
from setup_helpers import generate_input_files

# After (new usage)  
from fusion.cli.run_sim import main
from fusion.sim.input_setup import generate_input_files

✅ Code Quality Checklist

Architecture & Design:

  • Follows established architecture patterns
  • Code is modular and follows separation of concerns
  • Interfaces are well-defined and documented
  • Error handling is comprehensive
  • Logging is appropriate and informative

Code Standards:

  • Code follows project style guidelines
  • Variable and function names are descriptive
  • Code is properly commented
  • Complex logic is documented
  • No dead code or unused imports

Configuration & CLI:

  • CLI arguments follow established patterns
  • Configuration validation updated (if needed)
  • Schema updated for new config options
  • Backward compatibility maintained for configs

Security:

  • No sensitive information hardcoded
  • Input validation performed where needed
  • No security vulnerabilities introduced
  • Dependencies scanned for vulnerabilities

📚 Documentation

Documentation Updates:

  • Code comments added/updated
  • API documentation updated
  • User guide/tutorial updated
  • Configuration reference updated
  • CHANGELOG.md updated
  • README updated (if needed)

Examples Added:

  • Usage examples in docstrings
  • Configuration examples
  • CLI usage examples
  • Integration examples

🚀 Deployment

Deployment Considerations:

  • Safe to deploy to all environments
  • Requires environment-specific configuration
  • Needs database migration (if applicable)
  • Requires manual steps (document below)

Manual Steps Required:
No manual steps required for deployment.

🔍 Review Guidelines

For Reviewers:

  • PR description is clear and complete
  • Code changes align with described functionality
  • Tests are comprehensive and pass
  • Documentation is adequate
  • No obvious security issues
  • Performance impact is acceptable

Review Focus Areas:

  • CLI architecture and modular design patterns
  • Configuration system integration and validation
  • Package structure and import resolution
  • Test coverage for refactored modules
  • Backward compatibility and migration path

📝 Additional Notes

This refactor represents a significant architectural improvement that establishes a foundation for future development. The modular design makes the codebase more maintainable and extensible.

Open Questions:

  • Should we add additional configuration validation for edge cases?
  • Do we need more comprehensive documentation for the new architecture?

Future Work:

  • Complete Unity/HPC integration updates for new architecture
  • Add comprehensive API documentation
  • Implement configuration schema validation
  • Add integration examples and tutorials

Related PRs:
This PR consolidates work from multiple feature branches and sets the foundation for future modular enhancements.


🏁 Final Checklist

Before submitting this PR, confirm:

  • I have followed the contributing guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

@ryanmccann1024 ryanmccann1024 changed the base branch from main to release/6.0.0 August 13, 2025 20:05
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli branch 2 times, most recently from 4335174 to 7082809 Compare August 14, 2025 14:03
@ryanmccann1024 ryanmccann1024 self-assigned this Aug 14, 2025
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli branch 3 times, most recently from f0f13d3 to 06bd53b Compare August 14, 2025 19:11
ryanmccann1024 and others added 12 commits August 14, 2025 14:12
- Implement comprehensive commit message validation with detailed feedback
- Add support for release branches in cross-platform pipeline
- Update pylint workflow configuration
- Remove deprecated slack notifications workflow

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…d forms

- Replace markdown templates with structured YAML forms for better data collection
- Add comprehensive bug report template with environment details
- Create feature request template with clear requirements sections
- Organize templates into dedicated directories for better maintainability
- Add commit message guide for contributor reference

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add JSON schema validation for configuration files
- Create configuration registry and profile management
- Implement CLI argument to configuration mapping
- Add configuration templates for common use cases
- Migrate INI files from ini/ to fusion/configs/templates/
- Add comprehensive documentation for configuration system

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…gistry

- Create centralized argument registry system for consistent parsing
- Implement modular argument groups for reusability across commands
- Add clean entry points that delegate to appropriate modules
- Refactor config setup with proper path normalization
- Add comprehensive argument modules for simulation, training, and analysis
- Ensure all CLI components follow architectural best practices

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…cture

- Add core properties module for centralized configuration
- Refactor simulation pipeline for better modularity and error handling
- Enhance training pipeline with improved agent type dispatch
- Improve separation of concerns between CLI and business logic

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…nner

- Update GUI runner to work with new CLI argument system
- Enhance style argument processing for better customization
- Improve process arguments and file pathing for button images
- Add proper error handling and user feedback

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix environment reset issue in RL workflow runner
- Enhance general utilities for better error handling
- Improve modular structure alignment with new architecture

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ules

- Fix duplicate processing and normalization issues in metrics module
- Update import statements across core modules for new architecture
- Resolve module path dependencies and circular import issues
- Ensure compatibility with refactored package structure

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…cture

- Update import statements in visualization utilities
- Fix module paths in plotting and Excel statistics
- Add properties module for visualization configuration
- Ensure compatibility with refactored fusion package structure

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ation

- Update Architecture Plan v2 with one-week completion strategy
- Enhance unity manifest generation for improved deployment
- Document refactored CLI and configuration system approach

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Update import statements across all test files for new package structure
- Fix test modules for CLI, configuration, and core components
- Resolve module path dependencies in test suite
- Ensure all tests work with refactored fusion architecture
- Maintain test coverage for critical functionality

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Create add_run_sim_args and register_run_sim_args functions in simulation_args.py
- Add missing CLI arguments: snapshot_step, print_step, save_snapshots, save_step, save_start_end_slots, file_type, filter_mods
- Fix import paths from non-existent run_sim_args module to simulation_args
- Add legacy compatibility functions for removed argument modules
- Ensure run_sim subcommand includes all required config, debug, and output arguments

Fixes cross-platform compatibility tests that were failing due to ModuleNotFoundError.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli branch 7 times, most recently from b72b329 to e71629f Compare August 14, 2025 21:17
@ryanmccann1024 ryanmccann1024 force-pushed the refactor/cli branch 3 times, most recently from 34830ce to 188c416 Compare August 14, 2025 21:44
- Update run_comparison.py imports to use new CLI argument module structure
- Fix test_get_start_time timing race condition by validating format instead of exact timestamps
- Replace brittle time comparison with robust format validation for sim_start field

Resolves test failures caused by CLI refactoring and eliminates flaky timing-dependent assertions.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
ryanmccann1024 and others added 2 commits August 15, 2025 07:41
Add comprehensive tooling for local development workflow that mirrors
CI/CD pipeline checks, making it easier for developers to validate
changes before submitting PRs.

Changes include:
- Makefile with convenient commands for validation, testing, and setup
- setup.py for proper package installation and development workflow
- tools/ directory with PR validation scripts and documentation
- Local validation script that runs all CI/CD checks locally
- Support for quick validation during development

This enables developers to run 'make validate' to ensure their
changes will pass GitHub Actions before submitting PRs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add comprehensive tooling for local development workflow that mirrors
CI/CD pipeline checks, making it easier for developers to validate
changes before submitting PRs.

Changes include:
- Makefile with convenient commands for validation, testing, and setup
- setup.py for proper package installation and development workflow
- tools/ directory with PR validation scripts and documentation
- Local validation script that runs all CI/CD checks locally
- Support for quick validation during development

This enables developers to run 'make validate' to ensure their
changes will pass GitHub Actions before submitting PRs.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unnecessary else after raise in fusion/core/request.py
- Fix __cause__ attribute access errors in fusion/cli/run_sim.py with proper hasattr checks and pylint disables
- Move traceback import to module level in fusion/cli/run_sim.py
- Fix callable comparison warnings in fusion/cli/config_setup.py by using 'is' instead of '=='
- Change CLI argument defaults to None in simulation_args.py to properly defer to config file values
- Update test expectations to match actual behavior (config file values when CLI args not provided)
- Add validation check for empty nodes in generate_data.py
- Remove unnecessary bandwidth validation call in network_simulator.py
- Update cross_platform.ini template with simplified configuration

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024
Copy link
Copy Markdown
Collaborator Author

Apologies for the 50 pipeline failures...You could argue this needs two branches but I might keep it at one.

Ready for your reviews @bozo-bud @arashr88

Copy link
Copy Markdown
Collaborator Author

@ryanmccann1024 ryanmccann1024 left a comment

Choose a reason for hiding this comment

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

I think after local run sim comparison tests pass and other comments I put down, this is good to go.

Also, the PR description needs to be filled in.

Let me know what you guys think.

Comment thread .github/issue_template/config.yml Outdated
Comment thread fusion/cli/args/analysis_args.py
Comment thread fusion/cli/args/gui_args.py Outdated
Comment thread fusion/cli/args/registry.py
Comment thread fusion/cli/args/simulation_args.py Outdated
Comment thread fusion/cli/run_sim.py
Comment thread fusion/cli/run_train.py
Comment thread fusion/configs/templates/default.ini
Comment thread fusion/sim/network_simulator.py
Comment thread fusion/sim/run_simulation.py Outdated
ryanmccann1024 and others added 2 commits August 15, 2025 10:56
- Add TODOs for future custom error module implementation
- Make exception catches more specific where possible
- Extract helper functions to reduce load_config complexity
- Improve error specificity in config processing
- Add context for future error handling improvements

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Update GitHub issue template URLs from SDON_simulator to FUSION
- Expand CLI argument choices to match actual implementation options
- Remove unimplemented GUI arguments
- Add missing choices for modulation, routing, and ML algorithms

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
ryanmccann1024 and others added 4 commits August 27, 2025 08:45
This commit addresses multiple issues with comparison tests and improves
code quality throughout the CLI configuration system.

## Configuration System Improvements

### Added Missing Parameters
- Added XT-related parameters to snr_settings: beta, theta, xt_type, xt_noise, requested_xt, phi
- Added simulation parameters: fixed_grid, pre_calc_mod_selection, max_segments
- Added comprehensive RL/ML parameter support in OTHER_OPTIONS
- Enhanced dictionary parameter parsing for requested_xt and phi alongside request_distribution

### Code Quality Fixes
- Fixed pylint R1702 error (too many nested blocks) by extracting helper functions:
  - _convert_dict_params(): Handles dictionary parameter conversion
  - _process_cli_override(): Manages CLI argument overrides
- Reduced nesting complexity from 7 to 5 levels

## Test Infrastructure

### Comparison Test Fixes
- Fixed xtar_slicing_pff test by ensuring XT parameters are properly loaded
- This enables XT-aware routing to calculate cross-talk weights instead of distance-based weights
- Removed all debug statements from run_comparison.py for cleaner production code

### Unit Test Updates
- Updated test_config_args.py to handle new config-only parameters
- Added proper categorization for RL/ML, SNR/XT, and general simulation parameters
- Prevents false test failures for parameters that don't need CLI equivalents

## Technical Impact
- XT-aware routing simulations now work correctly with proper cross-talk calculations
- Improved config parameter validation and type conversion
- Better separation of concerns in configuration processing
- Cleaner, more maintainable test code

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Move data_scripts functionality to new io/ module structure
- Create io/generate.py from data_scripts/generate_data.py
- Create io/structure.py from data_scripts/structure_data.py
- Add io/exporter.py with JSON and CSV export capabilities
- Update all imports throughout codebase
- Fix test mocks to use new module paths
- Update documentation references
- Add comprehensive tests for new exporter functionality
- Fix pylint errors (unnecessary-pass, too-few-public-methods, redefined-builtin)

This refactoring aligns with the modular architecture plan by consolidating
data I/O functionality into a dedicated module.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit introduces a comprehensive interface layer for FUSION and
resolves multiple pylint errors across the codebase.

Interface Layer:
- Add AbstractRouter, AbstractSpectrumAssigner, AbstractSNRMeasurer,
  and AbstractAgent interfaces in fusion/interfaces/
- Create factory module for dynamic algorithm instantiation
- Update all algorithm implementations to inherit from new interfaces
- Add comprehensive interface documentation

Pylint Fixes:
- Fix too-many-nested-blocks (R1702) in first_fit.py and last_fit.py
  by extracting nested logic into separate helper methods
- Fix consider-using-enumerate (C0200) by replacing range(len()) with
  enumerate() in spectrum assignment modules
- Remove unnecessary-pass statements (W0107) from interface methods
  that already have docstrings

Testing:
- Add comprehensive test suite for interface implementations
- Add basic and integration tests for the new interface layer
- Include test summary module for interface validation

This refactoring improves code maintainability, enables easier
algorithm swapping, and ensures consistent API across all modules.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…luation pipeline

## Phase 2: Core Decoupling & Simulation Pipeline

### New Features
- **BatchRunner**: Comprehensive batch simulation orchestrator
  - Single and parallel execution support
  - Multiple Erlang load handling
  - Progress tracking and result aggregation
  - Configurable range parsing (e.g., "100-300", "300,600,900")

- **EvaluationPipeline**: Complete evaluation workflow orchestrator
  - Model performance evaluation and comparison
  - Algorithm benchmarking with rankings
  - RL agent evaluation with episode statistics
  - Multi-format report generation (JSON, Excel, Markdown)

### Integration
- Updated `run_simulation.py` to use BatchRunner while maintaining backward compatibility
- Unity HPC scripts work seamlessly through existing CLI interface
- CLI supports new parallel execution options

### Testing & Quality
- Comprehensive test suites for both orchestrators (173 tests total)
- Clean pylint compliance (9.98/10 rating)
- Professional error handling and documentation
- Placeholder implementations for future ML/RL integration

### Architecture Impact
- Establishes modular orchestration layer per architecture plan
- Separates simulation logic from execution orchestration
- Foundation for remaining migration phases
- Clean interfaces for pluggable algorithm modules

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ryanmccann1024 ryanmccann1024 removed the request for review from bozo-bud September 3, 2025 16:10
ryanmccann1024 and others added 7 commits September 3, 2025 11:18
Architecture Plan Phase 2 - Core Decoupling & Simulation Pipeline Implementation
refactor(interfaces): implement abstract interfaces and complete modular architecture migration
- Fixed TypeError in cross-platform compatibility test
- Changed create_input call to use proper parameter names (base_fp, engine_props)
- Added default 'data' value for base_fp parameter

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Added thread_num, date, and sim_start fields to sim_params
- Fixes KeyError in cross-platform CI tests
- Uses default values when fields are not present in config

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Create multiprocessing.Event stop_flag when not provided
- Fixes AttributeError in cross-platform CI tests
- Ensures SimulationEngine has required stop_flag for proper execution

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Added 'seeds' parameter to engine_props (defaults to None)
- Removed Unicode emojis from error messages to fix Windows encoding
- Fixes KeyError and UnicodeEncodeError in cross-platform CI tests

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@arashr88 arashr88 left a comment

Choose a reason for hiding this comment

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

Great work @ryanmccann1024!

@ryanmccann1024 ryanmccann1024 merged commit ff51ad4 into release/6.0.0 Sep 9, 2025
9 checks passed
ryanmccann1024 added a commit that referenced this pull request Sep 9, 2025
@ryanmccann1024 ryanmccann1024 deleted the refactor/cli branch January 19, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants