Refactor(Tools): Implementation of Options & Strategies with Type-Stability focus#255
Merged
Refactor(Tools): Implementation of Options & Strategies with Type-Stability focus#255
Conversation
- Add OptionValue{T} struct for value+source tracking
- Add OptionSchema struct with validation and aliases support
- Implement extract_option and extract_options functions
- Add comprehensive test suite (122 tests passing)
- Include full documentation with docstrings
- Support Vector and NamedTuple schema interfaces
- Handle validation errors via CTBase.IncorrectArgument
- Add type checking with warnings for mismatches
BREAKING CHANGE: New Options module replaces legacy option handling
- Flatten Options module structure (no api/contract subdirs) - Move files directly to src/Options/ root level - Remove validation.jl (redundant with OptionSchema.validator) - Update include paths in Options.jl - Keep Options as simple utility module (types + extraction) This prepares for Strategies module to have proper contract/api structure while Options remains a lightweight toolkit.
…affolding - Create unified OptionDefinition type replacing OptionSchema and OptionSpecification - Implement StrategyMetadata with varargs constructor using OptionDefinition - Update extraction API to work with OptionDefinition - Scaffold complete Strategies module structure (contract/api) - Add comprehensive tests for OptionDefinition and StrategyMetadata - Remove legacy options_schema.jl file - Add documentation for OptionDefinition unification - Update module dependencies and exports This unifies the option system while maintaining clean separation between Options (extraction tools) and Strategies (contract definition) modules.
- Add and to OptionDefinition docstrings - Improve extraction API docstrings with OptionDefinition unification context - Add custom show method for OptionDefinition with aliases display - Simplify StrategyMetadata show method to use OptionDefinition display - Add comprehensive display tests for OptionDefinition and StrategyMetadata - Remove obsolete OptionSchema and OptionSpecification types and tests - Update StrategyMetadata to use Dict instead of NamedTuple for flexibility - Add audit reports for test coverage and documentation improvements
…tion - Refactor StrategyOptions to use OptionValue struct for provenance tracking - Update all tests to reflect new API with OptionValue instances - Update extraction API to use OptionDefinition instead of OptionSchema - Refine validator contract to use || throw pattern - Implement @error + rethrow() for better exception handling - Suppress @error logs in tests with redirect_stderr - Add comprehensive docstrings with DocStringExtensions macros - Add detailed documentation for StrategyMetadata indexability - Update abstract strategy contract to use id instead of symbol - Fix test expectations for original exception types - All tests passing with clean output
- Add StrategyRegistry struct with explicit passing architecture - Implement create_registry(), strategy_ids(), type_from_id() functions - Add comprehensive introspection API for strategy metadata - Refactor to remove instance overloads for metadata functions - Fix encapsulation by using StrategyMetadata public interface - Remove misplaced strategy_registry.jl from contract/ - Update module exports with organized categories Features: - Registry validation (ID uniqueness, type hierarchy) - Type-level introspection (option_names, option_type, etc.) - Instance-level state access (option_value, option_source, etc.) - Complete documentation with examples - SOLID principles compliance
- Implement complete test suite for StrategyRegistry (38 tests) * Registry creation with validation * strategy_ids() and type_from_id() lookups * Error handling for duplicate IDs, wrong hierarchy, unknown families * Display methods * Integration tests for multiple families and round-trips - Implement complete test suite for introspection API (70 tests) * Type-level metadata access (option_names, option_type, etc.) * Instance-level state access (option_value, option_source, etc.) * Provenance tracking predicates (is_user, is_default, is_computed) * Integration tests for consistency and workflows - Fix create_registry signature to accept abstract types from tests - Fix option_source to access OptionValue.source field directly - Update tests to expect FieldError instead of KeyError for NamedTuple All tests passing: 108/108 total (38 registry + 70 introspection)
Breaking change: OptionDefinition is now parameterized by default value type
## Changes
### Core refactoring
- Change OptionDefinition from non-parametric to OptionDefinition{T}
- default::Any → default::T for type stability
- Constructor automatically infers T from default value
- Special handling for nothing defaults (uses OptionDefinition{Any})
### API updates
- Update extract_options signature: Vector{OptionDefinition} → Vector{<:OptionDefinition}
- Add type-stable get(opts, Val(:key)) method to StrategyOptions
- Document type-unstable vs type-stable access patterns
### Tests
- Add comprehensive type stability tests (14 new tests)
- Test parametric type inference
- Test @inferred for type-stable access
- Test heterogeneous collections
- Test type narrowing in loops
- All 146 options tests passing
## Performance impact
- ~2.5x faster access to default values
- Zero allocation in type-stable paths
- Eliminates boxing in loops over defaults
## Migration
Existing code continues to work - constructor infers type automatically
- Refactor StrategyMetadata from Dict to NamedTuple for type stability
- Add parametric StrategyMetadata{NT <: NamedTuple} type
- Implement complete collection interface (getindex, keys, values, pairs, iterate)
- Add 10 type stability tests for StrategyMetadata
- Simplify Base.getindex with direct delegation to NamedTuple
- Update introspection tests to expect FieldError instead of KeyError
- Add comprehensive type stability tests (38 total across Options/Strategies)
- Update TODO report to reflect 70% completion of Strategies module
- Update type stability report with complete refactorization results
Performance improvements:
- 2.5x faster option access with zero allocations
- Type-stable metadata storage and access
- All core structures now type-stable (OptionDefinition, StrategyOptions, StrategyMetadata)
Tests: 295 total tests passing, 38 type stability tests validated
Contributor
|
Breakage test results
|
- Add strategy builders with method tuple support - Replace error() with CTBase.IncorrectArgument - Add development standards reference - All tests passing
- Add build_strategy_options with Options API integration - Add utilities: filter_options, suggest_options, levenshtein_distance - 99 tests passing (47 config + 52 utilities) - Update TODO.md: Strategies now 80% complete
Add comprehensive validation for AbstractStrategy contracts including: - Metadata-options consistency verification - Constructor behavior validation - Advanced test coverage (51 tests total) - Enhanced documentation and error messages - Support for edge cases and complex types
…updates - Update todo.md to reflect 85% completion of Strategies module - Add remaining_work_report.md: Detailed analysis of ~85% complete architecture - Add documentation_update_report.md: Professional plan for post-implementation docs Key findings: - Options: 100% complete (147 tests) - Strategies: 85% complete (~323 tests) - functionally complete - Orchestration: 0% complete (needs implementation) Documentation plan includes: - 20 new/updated files across 5 phases - Step-by-step tutorials for strategy creation - Migration guide from AbstractOCPTool to AbstractStrategy - Professional examples and API reference updates Estimated timeline: 2-3 weeks total
- Change triple backticks to quadruple backticks for code block examples - Fix markdown formatting to prevent rendering issues - Add proper spacing around code fences - Ensure proper markdown syntax for documentation templates
- Add complete Orchestration module with routing, disambiguation, and method builders - Implement route_all_options() with auto-routing and disambiguation support - Add extract_strategy_ids() for strategy-based disambiguation syntax - Add build_strategy_to_family_map() and build_option_ownership_map() helpers - Add method builder wrappers for strategy construction - Add comprehensive test suite: 79 tests covering all scenarios - Support single-strategy disambiguation: backend = (:sparse, :adnlp) - Support multi-strategy routing: backend = ((:sparse, :adnlp), (:cpu, :ipopt)) - Full CTBase exception handling with user-friendly error messages - Complete documentation with and examples - Update status reports: Tools architecture 100% complete (649 tests) - All modules production-ready with full standards compliance Results: - Options: 147 tests ✅ - Strategies: 323 tests ✅ - Orchestration: 79 tests ✅ - Total: 649 tests passing ✅
- Add comprehensive Orchestration routing guide - Add detailed routing examples with disambiguation - Separate public/private APIs for all modules - Reorganize documentation structure - Remove empty types.md file - Fix public=true, private=true confusion - Activate Orchestration module documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 PR Description
🎯 Objective
This PR initiates the refactoring of the tools architecture (
Tools) into a 3-module system (Options, Strategies, Orchestration). The goal is to eliminate global mutable state, unify option management, and ensure maximum type stability for solver performance.✨ Key Changes
1.
OptionsModule (Foundational)OptionSchemaandOptionSpecificationinto a singleOptionDefinitiontype.OptionValuefor source tracking (:user,:default,:computed).2.
StrategiesModule (Core)AbstractStrategycontract (Type-level vs. Instance-level).StrategyRegistrystructure passed explicitly (Thread-safe and testable).3. Performance & Type Stability 🚀
StrategyMetadataandStrategyOptionsusing parameterizedNamedTuplesto guarantee perfect inference (Zero-overhead access).@inferred.📊 Status Overview
🔗 References
reports/2026-01-22_tools/reference/.reports/2026-01-22_tools/type_stability/report.md.💡 Note to Reviewer
The focus of this PR was on building robust foundations. The shift to an explicit registry and specialized option types significantly simplifies maintenance and paves the way for the
Orchestrationmodule and multi-strategy support (e.g.,backend = (:sparse, :adnlp)).