feat: auto-apply agent hyperparameter recommendations (#460) by ooples · Pull Request #828 · ooples/AiDotNet

ooples · 2026-02-07T23:44:18Z

Summary

Implements Issue #460: Auto-Apply Agent Hyperparameter Recommendations. When the AI agent analyzes user data and recommends hyperparameters, those recommendations are now parsed from the LLM response and applied to model options automatically (opt-in via EnableAutoApplyHyperparameters).

Core Infrastructure

HyperparameterResponseParser: Multi-strategy parser (JSON, markdown bold, colon-separated) that extracts structured key-value pairs from free-form LLM text
HyperparameterRegistry: Maps LLM parameter names (e.g., n_estimators, learning_rate) to C# property names (e.g., NumberOfTrees, LearningRate) with validation ranges for tree, NN, linear, neighbor/kernel, and time series model families
AgentHyperparameterApplicator: Reflection-based service that sets parsed values on model options, with type conversion and result reporting (Applied/Skipped/Failed/Warnings)
IConfigurableModel<T>: Interface exposing GetOptions() for post-construction hyperparameter access

Options Hierarchy Standardization

All options classes now inherit from ModelOptions (the base with Seed property)
Added NeuralNetworkOptions base and domain-specific bases (Audio, Document, Financial, Physics-Informed)
Fixed ClusteringOptions, ReinforcementLearningOptions, 17 MetaLearning options, 3 standalone NN options, and 4 financial options to inherit from ModelOptions
Added IConfigurableModel<T> + GetOptions() to 11 base classes covering ~460+ models

Model Wiring

Wired Options = _options in 45 model constructors (28 financial + 17 remaining NN models) so GetOptions() returns the correct derived options type

Pipeline Integration

AiModelBuilder now uses HyperparameterResponseParser instead of placeholder text
ApplyAgentRecommendationsCore applies hyperparameters when EnableAutoApplyHyperparameters is true
Added EnableAutoApplyHyperparameters to AgentAssistanceOptions (default: false, enabled in Comprehensive preset)
Results stored on AgentRecommendation.HyperparameterApplicationResult

Bug Fix

Fixed HyperparameterResponseParser.TryParseColonSeparated regex where \s* after [:=] could span newlines, causing header lines like "Settings:" to consume the next line's first token. Fixed by using [^\S\n]* for same-line whitespace only.

Test plan

98 unit tests: Parser strategies, registry lookups/validation, applicator type conversion
31 integration tests: Full pipeline (LLM text -> parse -> registry -> apply -> real model options) with DecisionTree, GradientBoosting, RandomForest, KNN, and SVR models
Tests verify GetOptions returns correct derived type, type conversions work, validation warnings propagate, mixed known/unknown params are reported, and realistic LLM responses are handled
dotnet build --no-restore passes with 0 errors
All 129 new tests pass
Backward compatible: auto-apply is opt-in (false by default)

Closes #460

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Agents can parse hyperparameter suggestions from model outputs and (optional) auto-apply them; application results are attached to recommendations.
Improvements
- Models expose a unified options accessor for consistent inspection and mutation.
- Added a toggle to enable/disable auto-apply of agent-recommended hyperparameters.
- Standardized reproducibility option name from RandomState to Seed.
- Many option types now share a common base for consistency.
Bug Fixes & Maintenance
- Clearer summaries and reporting for applied, skipped, failed parameters and warnings.

Add data classes, parser, registry, interface, and applicator service for the agent hyperparameter auto-apply feature. This includes: - HyperparameterApplicationResult for tracking applied/skipped/failed params - HyperparameterDefinition and HyperparameterValidationResult data classes - HyperparameterRegistry mapping ModelType -> LLM param names -> C# properties - HyperparameterResponseParser with multi-strategy LLM output extraction - IConfigurableModel<T> interface for post-construction hyperparameter access - AgentHyperparameterApplicator<T> service for reflection-based application Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add EnableAutoApplyHyperparameters option to AgentAssistanceOptions - Update Comprehensive preset to include auto-apply - Add Enable/Disable methods to AgentAssistanceOptionsBuilder - Add HyperparameterApplicationResult property to AgentRecommendation - Replace placeholder hyperparameter parsing with HyperparameterResponseParser - Replace stub in ApplyAgentRecommendationsCore with real application logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ons hierarchy (#460) - Add IConfigurableModel<T> interface + GetOptions() to all 11 model base classes (RegressionBase, NonLinearRegressionBase, DecisionTreeRegressionBase, AsyncDecisionTreeRegressionBase, TimeSeriesModelBase, ClassifierBase, ClusteringBase, ReinforcementLearningAgentBase, MetaLearnerBase, NeuralNetworkBase, DiffusionModelBase) - Add protected ModelOptions Options property to NeuralNetworkBase - Create base options hierarchy: NeuralNetworkOptions, AudioNeuralNetworkOptions, DocumentNeuralNetworkOptions, FinancialNeuralNetworkOptions, ForecastingModelOptions, FinancialNLPOptions, PhysicsInformedOptions - Update PortfolioOptimizerOptions and RiskModelOptions to inherit from FinancialNeuralNetworkOptions - Set domain-specific Options in intermediate NN base class constructors (AudioNeuralNetworkBase, DocumentNeuralNetworkBase, FinancialModelBase, ForecastingModelBase, PortfolioOptimizerBase, RiskModelBase, FinancialNLPModelBase) - Standardize ClusteringOptions to inherit from ModelOptions (RandomState wraps Seed) - Standardize ReinforcementLearningOptions to inherit from ModelOptions - Standardize all 17 MetaLearning Options to inherit from ModelOptions (RandomSeed wraps Seed for backward compatibility) - Standardize SpiralNetOptions, MeshCNNOptions, MixtureOfExpertsOptions to inherit from NeuralNetworkOptions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add Options = _options in all 28 financial model constructors (56 constructors total - ONNX + native mode each) so GetOptions() returns the model-specific options with all hyperparameters - Fix 4 options classes to inherit from ModelOptions: FactorVAEOptions, FactorTransformerOptions, AlphaFactorOptions, FinBERTOptions Models updated: RealizedVolatilityTransformer, NeuralGARCH, CSDI, MTGNN, DCRNN, TemporalGCN, TSDiff, FactorVAE, TabTransformer, GraphWaveNet, RelationalGCN, STGNN, TimeGrad, FactorTransformer, TabNet, AttentionAllocation, FinBERT, BlackLittermanNeural, AlphaFactorModel, ScoreGrad, DiffusionTS, SAINT, HierarchicalRiskParity, NeuralCVaR, NeuralStressTest, Hippo, S4, TimeMachine Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Wire up Options = _options in TimeSeries deep learning models, Audio models, PhysicsInformed models, and core NN models. Fix supporting options classes to inherit from ModelOptions where needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add 98 unit tests covering: - HyperparameterResponseParser: JSON, markdown bold, colon-separated parsing, type inference, strategy priority, edge cases - HyperparameterRegistry: property name lookups for all model families, alias normalization, validation ranges, custom registration, shared params - AgentHyperparameterApplicator: application of known/unknown params, type conversion, validation warnings, case-insensitive matching, result summary reporting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

) Add 32 integration tests exercising the full pipeline from LLM response parsing through registry lookup to application on real model classes: - Full pipeline tests with DecisionTree, GradientBoosting, RandomForest, KNN, SVR - GetOptions type verification (explicit and null constructor scenarios) - Bug detection tests for null-options constructor pattern - Bug detection test for colon parser header line issue - Validation warning propagation through pipeline - Mixed known/unknown parameter handling - Type conversion (int<->double, scientific notation) - Realistic verbose LLM response formats (JSON, markdown, colon-separated) - AgentAssistanceOptions and AgentRecommendation integration - Registry-to-options property name verification against real options classes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

vercel · 2026-02-07T23:44:23Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
aidotnet-playground-api	Ready	Preview, Comment	Feb 8, 2026 11:40pm

coderabbitai · 2026-02-07T23:44:54Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Walkthrough

Adds LLM hyperparameter parsing, a HyperparameterRegistry and HyperparameterDefinition, and a reflection-based generic AgentHyperparameterApplicator to apply suggested hyperparameters to model options. Exposes IConfigurableModel.GetOptions(), wires auto-apply into AiModelBuilder, and adds many ModelOptions-derived option types while renaming RandomState → Seed.

Changes

Cohort / File(s)	Summary
Hyperparameter infra `src/Agents/AgentHyperparameterApplicator.cs`, `src/Agents/HyperparameterDefinition.cs`, `src/Agents/HyperparameterRegistry.cs`, `src/Agents/HyperparameterResponseParser.cs`, `src/Agents/HyperparameterValidationResult.cs`	New multi-strategy LLM response parser, registry of normalized aliases/definitions and validation, and a reflection-based applicator that converts/validates values (nullable, enums, primitives) and records Applied/Skipped/Failed/Warnings. Review conversion/nullable/enum edge-cases and validation messages.
Agent config & results `src/Models/AgentAssistanceOptions.cs`, `src/Models/AgentAssistanceOptionsBuilder.cs`, `src/Models/AgentRecommendation.cs`, `src/Models/HyperparameterApplicationResult.cs`	Adds EnableAutoApplyHyperparameters and builder toggles; AgentRecommendation carries HyperparameterApplicationResult; new result model summarizes outcomes. Warning: duplicate builder methods (Enable/Disable) appear — compile-blocking.
AiModelBuilder integration `src/AiModelBuilder.cs`, `src/Interfaces/IConfigurableModel.cs`	Parses suggested hyperparameters, derives ModelType heuristically, and conditionally auto-applies via AgentHyperparameterApplicator when model implements IConfigurableModel and option is enabled. Check opt-in guard, error handling, and model-type derivation.
Per-model options plumbing many files across `src/Audio/...`, `src/Document/...`, `src/Finance/...`, `src/Clustering/...`, `src/Classification/...`, `src/MetaLearning/...`	Large, repetitive pattern: add private `_options` fields, optional constructor parameter `...Options? options = null`, `Options = _options` assignments and public `GetOptions()` overrides. Verify consistent null/default handling and that public signatures are intended.
Options type additions & seed rename `src/Models/Options/`, many `src//Options.cs`, `samples/*`	Introduces many new Options subclasses (Document/Audio/Finance/...) deriving from ModelOptions; removes or shadows some Seed/RandomState fields and consolidates randomness property to `Seed`. Search for any remaining `RandomState` references and consider compatibility implications.
New document/audio options files `src/Document/Options/*`, `src/Audio/...Options.cs`	Many new empty/marker options classes added as public types (e.g., `DocBankOptions`, `TRIEOptions`, `DiTOptions`, audio options). Confirm these are intentionally public and production-ready (no TODO stubs).
Misc / scratch & CI guidance `C:\...scratchpad*.json`, `.coderabbit.yaml`	Adds local scratchpad JSON files and a stricter `.coderabbit.yaml` with blocking rules for stubs/tests. Remove local temp files if unintended; review blocking rules for adoption.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Agent as AI Agent
    participant Builder as AiModelBuilder
    participant Parser as HyperparameterResponseParser
    participant Registry as HyperparameterRegistry
    participant Applicator as AgentHyperparameterApplicator<T>
    participant Model as IConfigurableModel

    Agent->>Builder: send recommendation (text)
    Builder->>Parser: Parse(llmResponse)
    Parser-->>Builder: Dictionary<string, object>
    Builder->>Registry: Resolve definitions / aliases
    Builder->>Applicator: new Applicator(registry)
    alt EnableAutoApplyHyperparameters && model implements IConfigurableModel
        Builder->>Applicator: Apply(model, modelType, params)
        Applicator->>Registry: GetDefinition(modelType, paramName)
        Registry-->>Applicator: HyperparameterDefinition
        Applicator->>Applicator: Validate & ConvertValue (handles nullable/enums/primitives)
        Applicator->>Model: Model.GetOptions() -> set property via reflection
        Applicator-->>Builder: HyperparameterApplicationResult (Applied/Skipped/Failed/Warnings)
    else
        Builder-->>Builder: Skip auto-apply
    end
    Builder-->>Agent: return AgentRecommendation with HyperparameterApplicationResult

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

refactor: upgrade time series models to production-ready tensor pattern #533 — Overlaps on time-series model options exposure and ModelType usage; potential merge conflicts in the same model wiring.
test: add clustering integration tests (#617) #679 — Changes to ClusteringBase/GetOptions overlap with the IConfigurableModel/GetOptions additions here.
feat(document): add comprehensive Document AI module #706 — Large, overlapping edits to document-model files and options plumbing; likely same constructor/GetOptions modifications.

Blocking issues / attention items

Compile-blocking duplicate methods: AgentAssistanceOptionsBuilder contains duplicate EnableAutoApplyHyperparameters()/DisableAutoApplyHyperparameters() declarations — remove duplicates.
Conversion robustness: ensure AgentHyperparameterApplicator.ConvertValue handles boxed numeric types, nullable enums, culture-invariant parsing, and returns clear failures rather than throwing. Add unit tests for edge cases.
Read-only / user overrides: applicator must not override user-provided non-default values — verify GetDefaultPropertyValue logic and behavior across value types.
New public types: many empty/marker Options classes were added; treat as BLOCKING if they are placeholders/stubs without intended production semantics (per repo policy in .coderabbit.yaml).
Seed migration: RandomState → Seed is widespread; run a global search for leftover RandomState and confirm public API compatibility or provide migration shims/documentation.
Local artifacts: remove unintended local scratchpad files (paths under C:\Users...) from the PR if not meant to be committed.
.coderabbit.yaml: introduces strict BLOCKING rules for stubs/tests; if any added classes or TODOs are incomplete, mark them FIX/BLOCKING and either complete or document intention.

Poem

🎛️ Agent whispers numbers, names, and schemes,
Parser gleans structure from ambiguous dreams,
Registry maps aliases, types aligned,
Applicator applies — unless the user signed,
Seed steadies randomness; fix dupe builders — ship pristine.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly summarizes the main change: auto-apply agent hyperparameter recommendations, directly reflecting the core feature added across the codebase.
Linked Issues check	✅ Passed	PR implements all major coding requirements from `#460`: multi-strategy parser, registry mapping, reflection-based applicator, IConfigurableModel interface, options hierarchy standardization, pipeline integration, and 129 tests covering parser, registry, applicator, and end-to-end scenarios.
Out of Scope Changes check	✅ Passed	All changes align with `#460` objectives: hyperparameter parsing/registry/applicator infrastructure, IConfigurableModel exposure, options inheritance standardization, and pipeline wiring. Extensive documentation updates (.coderabbit.yaml) support the feature. No unrelated scope creep detected.
Docstring Coverage	✅ Passed	Docstring coverage is 94.87% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/agent-hyperparameter-auto-apply

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Add 31 integration tests that exercise the full pipeline: LLM response parsing -> registry lookup -> applicator -> real model options. Tests cover DecisionTree, GradientBoosting, RandomForest, KNN, and SVR models. Fix bug in HyperparameterResponseParser.TryParseColonSeparated where the regex \s* after [:=] could span newlines, causing a header line like "Settings:" to consume the first token of the next line as its value. Fixed by using [^\S\n]* to match only same-line whitespace. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Implements Issue #460 by parsing agent/LLM hyperparameter recommendations and auto-applying them to configured models (opt-in), using a registry-backed name mapping and reflection to mutate live model options via a new IConfigurableModel<T> interface.

Changes:

Added core hyperparameter infrastructure: response parser, registry/definitions + validation, reflection-based applicator, and application-result reporting.
Standardized options access across model families by introducing IConfigurableModel<T> and aligning many option types under ModelOptions / NeuralNetworkOptions (plus related base option hierarchies).
Wired the pipeline into AiModelBuilder and added extensive unit + integration test coverage.

Reviewed changes

Copilot reviewed 118 out of 118 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/AiDotNet.Tests/UnitTests/Agents/HyperparameterResponseParserTests.cs	Unit tests for multi-strategy LLM hyperparameter parsing and type inference.
tests/AiDotNet.Tests/UnitTests/Agents/HyperparameterRegistryTests.cs	Unit tests for registry mapping, alias normalization, and validation warnings.
tests/AiDotNet.Tests/UnitTests/Agents/AgentHyperparameterApplicatorTests.cs	Unit tests for reflection-based application, conversion, warnings, and summaries.
tests/AiDotNet.Tests/IntegrationTests/Agents/HyperparameterAutoApplyIntegrationTests.cs	End-to-end integration tests across parser → registry → applicator → real model options.
src/Agents/AgentHyperparameterApplicator.cs	Applies parsed hyperparameters to `ModelOptions` via reflection + registry mapping.
src/Agents/HyperparameterDefinition.cs	Defines alias-to-property mapping metadata and normalization.
src/Agents/HyperparameterRegistry.cs	Central registry for model-type-specific + shared hyperparameter mappings and validation.
src/Agents/HyperparameterResponseParser.cs	Extracts hyperparameters from JSON/markdown/colon formats with type inference.
src/Agents/HyperparameterValidationResult.cs	Validation result model used by the registry.
src/Interfaces/IConfigurableModel.cs	New interface exposing live `ModelOptions` for post-construction configuration.
src/Models/HyperparameterApplicationResult.cs	Captures applied/skipped/failed/warning results and produces a summary string.
src/Models/AgentRecommendation.cs	Stores hyperparameter application results on the agent recommendation.
src/Models/AgentAssistanceOptions.cs	Adds `EnableAutoApplyHyperparameters` flag (default false) + preset/clone propagation.
src/Models/AgentAssistanceOptionsBuilder.cs	Builder helpers to enable/disable hyperparameter auto-apply (enables tuning too).
src/AiModelBuilder.cs	Parses hyperparameters from LLM output and auto-applies them when enabled.
src/Regression/RegressionBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Regression/NonLinearRegressionBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Regression/DecisionTreeRegressionBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Regression/DecisionTreeAsyncRegressionBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Classification/ClassifierBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Clustering/Base/ClusteringBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/Clustering/Options/ClusteringOptions.cs	Standardizes clustering options under `ModelOptions` and delegates random state to `Seed`.
src/ReinforcementLearning/Agents/ReinforcementLearningAgentBase.cs	Implements `IConfigurableModel<T>`; RL options now derive from `ModelOptions` for `Seed`.
src/MetaLearning/MetaLearnerBase.cs	Implements `IConfigurableModel<T>` and exposes options via `GetOptions()`.
src/MetaLearning/Options/ANILOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/BOILOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/CNAPOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/GNNMetaOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/iMAMLOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/LEOOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/MAMLOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/MANNOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/MatchingNetworksOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/MetaOptNetOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/MetaSGDOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/NTMOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/ProtoNetsOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/RelationNetworkOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/ReptileOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/SEALOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/MetaLearning/Options/TADAMOptions.cs	Meta-learning options now derive from `ModelOptions` and delegate `RandomSeed` to `Seed`.
src/TimeSeries/TimeSeriesModelBase.cs	Implements `IConfigurableModel<T>` and exposes mutable options via `GetOptions()`.
src/TimeSeries/TemporalFusionTransformer.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/NHiTSModel.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/NBEATSModel.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/InformerModel.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/DeepARModel.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/ChronosFoundationModel.cs	Ensures base `Options` points at derived `_options` instance.
src/TimeSeries/AutoformerModel.cs	Ensures base `Options` points at derived `_options` instance.
src/NeuralNetworks/NeuralNetworkBase.cs	Adds standardized `ModelOptions Options` + `IConfigurableModel<T>` implementation.
src/NeuralNetworks/MixtureOfExpertsNeuralNetwork.cs	Wires derived options into base `Options` for consistent access.
src/NeuralNetworks/MeshCNN.cs	Wires derived options into base `Options` for consistent access.
src/NeuralNetworks/Diffusion/DiffusionModelBase.cs	Implements `IConfigurableModel<T>` and exposes diffusion options as `ModelOptions`.
src/PhysicsInformed/PINNs/MultiScalePINN.cs	Wires training options into base `Options` for configurability.
src/PhysicsInformed/PINNs/InverseProblemPINN.cs	Wires training options into base `Options` for configurability.
src/PhysicsInformed/Interfaces/IMultiScalePDE.cs	Makes multi-scale training options derive from `ModelOptions`.
src/PhysicsInformed/Interfaces/IInverseProblem.cs	Makes inverse-problem training options derive from `ModelOptions`.
src/Models/Options/NeuralNetworkOptions.cs	New base options type for NN models, inheriting `Seed` from `ModelOptions`.
src/Models/Options/PhysicsInformedOptions.cs	New base options type for PINN/physics-informed models.
src/Models/Options/FinancialNeuralNetworkOptions.cs	New base options type for financial NN models.
src/Models/Options/ForecastingModelOptions.cs	New base options type for financial forecasting models.
src/Models/Options/FinancialNLPOptions.cs	New base options type for financial NLP models.
src/Models/Options/DocumentNeuralNetworkOptions.cs	New base options type for document NN models.
src/Models/Options/AudioNeuralNetworkOptions.cs	New base options type for audio NN models.
src/Models/Options/MixtureOfExpertsOptions.cs	Makes MoE options derive from `NeuralNetworkOptions` and maps `RandomSeed` to `Seed`.
src/Models/Options/MeshCNNOptions.cs	Makes MeshCNN options derive from `NeuralNetworkOptions`.
src/Models/Options/SpiralNetOptions.cs	Makes SpiralNet options derive from `NeuralNetworkOptions`.
src/Models/Options/RiskModelOptions.cs	Aligns risk model options under financial NN options hierarchy.
src/Models/Options/PortfolioOptimizerOptions.cs	Aligns portfolio optimizer options under financial NN options hierarchy.
src/Models/Options/FinBERTOptions.cs	Standardizes FinBERT options to derive from `ModelOptions`.
src/Models/Options/AlphaFactorOptions.cs	Standardizes AlphaFactor options to derive from `ModelOptions`.
src/Models/Options/FactorTransformerOptions.cs	Standardizes FactorTransformer options to derive from `ModelOptions`.
src/Models/Options/FactorVAEOptions.cs	Standardizes FactorVAE options to derive from `ModelOptions` (but see review comments on `Seed`).
src/Audio/AudioNeuralNetworkBase.cs	Sets standardized base `Options` for audio NN models.
src/Audio/AudioLDM/AudioLDMOptions.cs	Makes AudioLDM options derive from `ModelOptions` and removes duplicate seed.
src/Audio/AudioLDM/AudioLDMModel.cs	Wires AudioLDM options into base `Options`.
src/Audio/StableAudio/StableAudioOptions.cs	Makes StableAudio options derive from `ModelOptions` and removes duplicate seed.
src/Audio/StableAudio/StableAudioModel.cs	Wires StableAudio options into base `Options`.
src/Audio/MusicGen/MusicGenOptions.cs	Makes MusicGen options derive from `ModelOptions` and removes duplicate seed.
src/Audio/MusicGen/MusicGenModel.cs	Wires MusicGen options into base `Options`.
src/Audio/LanguageIdentification/LanguageIdentifierOptions.cs	Makes language ID options derive from `ModelOptions`.
src/Audio/LanguageIdentification/Wav2Vec2LanguageIdentifier.cs	Wires language ID options into base `Options`.
src/Audio/LanguageIdentification/VoxLingua107Identifier.cs	Wires language ID options into base `Options`.
src/Audio/LanguageIdentification/ECAPATDNNLanguageIdentifier.cs	Wires language ID options into base `Options`.
src/Document/DocumentNeuralNetworkBase.cs	Sets standardized base `Options` for document NN models.
src/Finance/Base/FinancialModelBase.cs	Sets standardized base `Options` for financial NN base models.
src/Finance/Base/ForecastingModelBase.cs	Sets standardized base `Options` for forecasting model base.
src/Finance/Base/PortfolioOptimizerBase.cs	Sets standardized base `Options` for portfolio optimizer base.
src/Finance/Base/RiskModelBase.cs	Sets standardized base `Options` for risk model base.
src/Finance/Base/FinancialNLPModelBase.cs	Sets standardized base `Options` for financial NLP base.
src/Finance/NLP/FinBERT.cs	Wires FinBERT options into base `Options`.
src/Finance/Trading/Factors/FactorVAE.cs	Wires FactorVAE options into base `Options`.
src/Finance/Trading/Factors/FactorTransformer.cs	Wires FactorTransformer options into base `Options`.
src/Finance/Trading/Factors/AlphaFactorModel.cs	Wires AlphaFactor options into base `Options`.
src/Finance/Volatility/NeuralGARCH.cs	Wires NeuralGARCH options into base `Options`.
src/Finance/Volatility/RealizedVolatilityTransformer.cs	Wires realized-volatility transformer options into base `Options`.
src/Finance/Risk/TabTransformer.cs	Wires TabTransformer options into base `Options`.
src/Finance/Risk/TabNet.cs	Wires TabNet options into base `Options`.
src/Finance/Risk/SAINT.cs	Wires SAINT options into base `Options`.
src/Finance/Risk/NeuralStressTest.cs	Wires NeuralStressTest options into base `Options`.
src/Finance/Risk/NeuralCVaR.cs	Wires NeuralCVaR options into base `Options`.
src/Finance/Portfolio/HierarchicalRiskParity.cs	Wires HRP options into base `Options`.
src/Finance/Portfolio/BlackLittermanNeural.cs	Wires Black-Litterman options into base `Options`.
src/Finance/Portfolio/AttentionAllocation.cs	Wires attention allocation options into base `Options`.
src/Finance/Graph/TemporalGCN.cs	Wires TemporalGCN options into base `Options`.
src/Finance/Graph/STGNN.cs	Wires STGNN options into base `Options`.
src/Finance/Graph/RelationalGCN.cs	Wires RelationalGCN options into base `Options`.
src/Finance/Graph/MTGNN.cs	Wires MTGNN options into base `Options`.
src/Finance/Graph/GraphWaveNet.cs	Wires GraphWaveNet options into base `Options`.
src/Finance/Graph/DCRNN.cs	Wires DCRNN options into base `Options`.
src/Finance/Forecasting/StateSpace/TimeMachine.cs	Wires TimeMachine options into base `Options`.
src/Finance/Forecasting/StateSpace/S4.cs	Wires S4 options into base `Options`.
src/Finance/Forecasting/StateSpace/Hippo.cs	Wires Hippo options into base `Options`.
src/Finance/Probabilistic/TimeGrad.cs	Wires TimeGrad options into base `Options`.
src/Finance/Probabilistic/TSDiff.cs	Wires TSDiff options into base `Options`.
src/Finance/Probabilistic/ScoreGrad.cs	Wires ScoreGrad options into base `Options`.
src/Finance/Probabilistic/DiffusionTS.cs	Wires DiffusionTS options into base `Options`.
src/Finance/Probabilistic/CSDI.cs	Wires CSDI options into base `Options`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            catch (Exception ex)
+            {
+                result.Failed[paramName] = $"Unexpected error: {ex.Message}";
+            }


To fix the problem, narrow the generic catch (Exception ex) in Apply to only catch the exception types that are expected and reasonably recoverable when applying a single hyperparameter. For example, reflection-related exceptions (TargetInvocationException, TargetException, ArgumentException), invalid cast or conversion issues (InvalidCastException, FormatException, OverflowException), and perhaps KeyNotFoundException or ArgumentNullException if they can arise from the registry or dictionary usage. More severe runtime exceptions should be allowed to propagate so they are not silently converted into a simple failure message.

The best minimal-impact change is to replace the single generic catch (Exception ex) block around ApplyParameter with several specific catch blocks that map to the same failure recording behavior. This preserves the existing functional behavior for expected failures (they are still reported in result.Failed[paramName]) while avoiding swallowing arbitrary exceptions. Concretely, in src/Agents/AgentHyperparameterApplicator.cs, lines 59–66 inside the foreach loop of Apply should be updated to multiple typed catch clauses. No new methods are required. We can do this using only types from System and System.Reflection, which are already available (System.Reflection is imported at the top), so no additional imports are needed.

+        catch (Exception ex)
+        {
+            result.Failed[paramName] = $"Failed to set property: {ex.Message}";
+        }


General approach: Replace the generic catch (Exception ex) with one or more specific catch blocks for the exceptions that are realistically expected from property.SetValue(options, convertedValue);, such as ArgumentException, TargetException, TargetParameterCountException, MethodAccessException, InvalidOperationException, and TargetInvocationException. Let all other exceptions propagate so they can be logged/handled at a higher level.

Best concrete fix here:

Leave the try block and success path unchanged.

Add a first catch (TargetInvocationException ex) that records the inner exception message when available, because SetValue wraps exceptions thrown by the property setter in TargetInvocationException.

Add a second catch (Exception ex) when (ex is ArgumentException || ex is TargetException || ex is TargetParameterCountException || ex is MethodAccessException || ex is InvalidOperationException) to handle other expected reflection/configuration errors while still avoiding a fully generic catch.

In both catches, continue to populate result.Failed[paramName] with a similar message so existing behavior for configuration failures is preserved.

Do not add new imports: TargetInvocationException, TargetException, and TargetParameterCountException live in System.Reflection, which is already imported.

All changes are within src/Agents/AgentHyperparameterApplicator.cs around lines 112–120; no additional methods or dependencies are required.

+        foreach (var prop in type.GetProperties(BindingFlags.Public | BindingFlags.Instance))
+        {
+            if (prop.CanWrite && HyperparameterDefinition.NormalizeName(prop.Name) == normalized)
+            {
+                return prop;
+            }
+        }


In general, to fix this kind of issue you replace a foreach that iterates an entire sequence and conditionally skips elements using if with a LINQ Where (and possibly FirstOrDefault, Any, etc.) that filters the sequence before iteration or selection. This makes the filtering explicit and improves readability.

Here, in FindProperty, the second phase tries to find a property whose normalized name matches the normalized target name and that is writable. Instead of:

foreach (var prop in type.GetProperties(BindingFlags.Public | BindingFlags.Instance)) { if (prop.CanWrite && HyperparameterDefinition.NormalizeName(prop.Name) == normalized) { return prop; } }

we can use LINQ to filter properties and then pick the first match:

using System.Linq; // needed at top of file // ... var property = type .GetProperties(BindingFlags.Public | BindingFlags.Instance) .FirstOrDefault(prop => prop.CanWrite && HyperparameterDefinition.NormalizeName(prop.Name) == normalized); return property;

This preserves behavior: if a matching property is found, it is returned; otherwise null is returned. We’ll also need to add using System.Linq; at the top of AgentHyperparameterApplicator.cs so FirstOrDefault is available, without modifying other imports or code outside the shown snippet.

+        catch
+        {
+            return null;
+        }


In general, the fix is to replace the bare catch with one or more specific exception types that are expected from type conversion, and let other exceptions bubble up. For numeric and general Convert.* calls, the common expected exceptions are InvalidCastException, FormatException, and OverflowException. For Convert.ChangeType, the same set applies. Narrowing to these maintains the intended behavior—returning null when conversion cannot be performed for normal reasons—while not swallowing unrelated or critical runtime exceptions.

The best concrete fix here is:

Replace the generic catch at lines 204–207 with a catch that lists the expected conversion exceptions and returns null in that case.

Add a second catch (Exception) only if you want to preserve the “never throw” contract. However, this would still be a generic catch and likely keep the CodeQL warning. To both satisfy CodeQL and improve robustness, we should only catch the specific conversion exceptions and not have a generic catch at all.

No additional imports are needed, since InvalidCastException, FormatException, and OverflowException are in System, which is implicitly available.

Concretely, in src/Agents/AgentHyperparameterApplicator.cs, within ConvertValue, replace the catch block starting at line 204 with a multi-type catch:

catch (InvalidCastException) { return null; } catch (FormatException) { return null; } catch (OverflowException) { return null; }

This preserves the original behavior for expected conversion problems while allowing any other unexpected exceptions to surface.

+            foreach (var def in definitions)
+            {
+                if (def.NormalizedAliases.Contains(normalized))
+                {
+                    return def.PropertyName;
+                }
+            }


In general, to fix this kind of issue you replace a foreach loop that performs an if check on each element (implicitly filtering) with an explicit LINQ filter (e.g., .Where(...), or directly .FirstOrDefault(...) / .SingleOrDefault(...)), then operate only on the filtered sequence or matching element. This makes the existence of a filter explicit and often reduces nesting.

Here, in GetPropertyName, the loop:

if (_registry.TryGetValue(modelType, out var definitions)) { foreach (var def in definitions) { if (def.NormalizedAliases.Contains(normalized)) { return def.PropertyName; } } }

is logically “return the PropertyName for the first definition whose NormalizedAliases contains normalized”. The cleanest LINQ equivalent is:

if (_registry.TryGetValue(modelType, out var definitions)) { var match = definitions.FirstOrDefault(d => d.NormalizedAliases.Contains(normalized)); if (match != null) { return match.PropertyName; } }

This preserves early‑exit semantics and does not alter behavior. It also aligns with the rule’s recommendation by making the filter predicate explicit. We should similarly consider the _sharedDefinitions loop; however, that loop also just returns on first match and is structurally identical, so the same pattern could be applied. The alert is specifically on the first loop, so we will minimally change that loop only, to stay within the reported scope.

To implement this, we only need LINQ extension methods. We can safely add using System.Linq; at the top of HyperparameterRegistry.cs (if not already present in unseen code) because it’s a standard BCL namespace and introduces no behavioral changes to existing code. All other functionality remains the same.

+        foreach (var def in _sharedDefinitions)
+        {
+            if (def.NormalizedAliases.Contains(normalized))
+            {
+                return def;
+            }
+        }


In general, to fix this kind of issue you replace an explicit foreach loop that checks a condition on each element with a LINQ query that filters the sequence (Where) and then selects the first matching element (FirstOrDefault). This makes the intent (“find the first definition whose aliases contain this normalized name”) explicit and removes manual loop control.

For this file, the best approach is:

In GetPropertyName, replace the two loops over definitions and _sharedDefinitions with LINQ calls that:

Filter each collection by def.NormalizedAliases.Contains(normalized).

Take the first matching element (or null if none).

Return its PropertyName if found; otherwise fall back to the shared list, then null.

In GetDefinition, replace the two loops with LINQ calls that:

Directly return the first matching HyperparameterDefinition from the model-specific list, or from _sharedDefinitions if none match.

Preserve the null return when no definitions match.

To use LINQ extension methods on List<T>, add using System.Linq; at the top of this file.

We do not change anything else in the class; Validate and other members keep working as before.

These changes must be done in src/Agents/HyperparameterRegistry.cs:

Add the required using System.Linq;.

Replace the body of GetPropertyName.

Replace the body of GetDefinition.

+        catch
+        {
+            result = 0;
+            return false;
+        }


In general, to fix a generic catch clause you should replace catch with one or more catch blocks that target the specific, expected exception types, and allow unexpected exceptions to propagate. This avoids inadvertently swallowing critical errors and makes behavior clearer.

Here, TryConvertToDouble calls Convert.ToDouble(object), which can throw several documented exceptions for invalid conversion scenarios: commonly FormatException, InvalidCastException, OverflowException, and ArgumentNullException. We want TryConvertToDouble to return false (and set result to a default) for these “normal conversion failure” cases, while letting other exceptions bubble up.

The single best fix is to change the bare catch block in TryConvertToDouble to a catch that explicitly lists the expected exceptions using a C# exception filter. This preserves existing semantics (return false and set result = 0 whenever the conversion fails for normal reasons) without adding logging or changing callers. No new imports are needed because these exception types are in System, which is already implicitly available in C# projects; we are not modifying the using directives. The changes are confined to the TryConvertToDouble method in src/Agents/HyperparameterRegistry.cs, around lines 143–155.

+        foreach (Match match in matches)
+        {
+            if (TryParseJsonObject(match.Groups[1].Value.Trim(), result))
+            {
+                return result;
+            }
+        }


+        foreach (Match match in matches)
+        {
+            if (TryParseJsonObject(match.Value.Trim(), result))
+            {
+                return result;
+            }
+        }


+        catch
+        {
+            return false;
+        }


In general, to fix a generic catch clause, you replace catch { ... } with one or more specific catch (SomeExceptionType) blocks that cover only the expected failure modes. Unexpected exceptions are then allowed to propagate, making debugging easier and avoiding masking serious issues.

For this code, the best minimal change is to narrow the catch in TryParseJsonObject to JSON/parsing-related exceptions thrown by Newtonsoft.Json. The main candidate is JsonReaderException (the typical exception for malformed JSON), and more broadly JsonException, both from Newtonsoft.Json. This preserves the existing behavior—returning false when the JSON cannot be parsed or interpreted—while no longer swallowing unrelated exceptions. Concretely:

Add an appropriate using Newtonsoft.Json; at the top of HyperparameterResponseParser.cs so that JsonException/JsonReaderException are available without fully qualified names.

Replace the generic catch block (lines 104–107) in TryParseJsonObject with specific catch blocks for JsonReaderException and JsonException. These should still just return false; to maintain the current external behavior.

Do not otherwise alter logic or signatures.

All changes are confined to src/Agents/HyperparameterResponseParser.cs within the provided snippet.

coderabbitai

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

src/Clustering/Base/ClusteringBase.cs (1)
21-39: ⚠️ Potential issue | 🟠 Major

New public GetOptions/IConfigurableModel expands facade surface

Lines 21-39: Adding a public interface plus public GetOptions introduces a new user-facing entry point outside AiModelBuilder/AiModelResult. If this is only for internal hyperparameter plumbing, consider making the interface internal and/or implementing it explicitly to reduce surface area.
🔧 Suggested adjustment to hide GetOptions from the public surface
-    public virtual ModelOptions GetOptions() => Options;
+    ModelOptions IConfigurableModel<T>.GetOptions() => Options;
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API".
src/Regression/DecisionTreeAsyncRegressionBase.cs (1)
27-51: ⚠️ Potential issue | 🟠 Major

Avoid expanding the public API with GetOptions() on base types.

If this is only for internal hyperparameter plumbing, consider explicit interface implementation to keep the method off the public surface (or make the interface internal).
💡 Suggested change
-public virtual ModelOptions GetOptions() => Options;
+ModelOptions IConfigurableModel<T>.GetOptions() => Options;
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API".
src/Classification/ClassifierBase.cs (1)
31-69: ⚠️ Potential issue | 🟠 Major

Avoid adding new public API on base types; consider explicit interface implementation.

If GetOptions() is only needed by internal hyperparameter plumbing, keep it off the public surface via explicit interface implementation (or make the interface internal).
💡 Suggested change
-public virtual ModelOptions GetOptions() => Options;
+ModelOptions IConfigurableModel<T>.GetOptions() => Options;
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API".
src/ReinforcementLearning/Agents/ReinforcementLearningAgentBase.cs (1)
34-88: ⚠️ Potential issue | 🟠 Major

Hide GetOptions() from the public surface unless this is an intentional API expansion.

This adds a public member to a non‑facade type. If external callers shouldn’t access options directly, implement IConfigurableModel<T>.GetOptions() explicitly (or make the interface/internal accessor internal) so it’s only reachable via the interface.
🔐 Example explicit implementation
-    /// <inheritdoc/>
-    public virtual ModelOptions GetOptions() => Options;
+    /// <inheritdoc/>
+    ModelOptions IConfigurableModel<T>.GetOptions() => Options;
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public unless part of the facade API.”

🤖 Fix all issues with AI agents

In `@src/Agents/AgentHyperparameterApplicator.cs`:
- Around line 26-49: The AgentHyperparameterApplicator<T> class is public but
should be internal to avoid expanding the public API; change the type
declaration from public to internal (and ensure any related members like the
constructor AgentHyperparameterApplicator(HyperparameterRegistry) and the Apply
method remain accessible within the assembly), and, if external usage is
required, expose the functionality only via the facade (e.g., AiModelBuilder)
rather than making this class public.
- Around line 46-69: Add explicit null-argument validation at the start of
Apply: validate that the model and hyperparameters parameters are not null and
throw ArgumentNullException for each (e.g., check model and hyperparameters in
the Apply method), then retrieve options via model.GetOptions() and guard
against null (throw ArgumentNullException or InvalidOperationException with a
clear message if options is null) before iterating; this prevents downstream
NullReferenceExceptions from ApplyParameter and makes failures explicit.

In `@src/Agents/HyperparameterDefinition.cs`:
- Around line 19-71: Make HyperparameterDefinition an internal class and stop
exposing a mutable NormalizedAliases HashSet; change the class accessibility to
internal, make the NormalizedAliases field private (or private readonly) and
expose it only via a read-only view or method (e.g., a public
IReadOnlyCollection<string> GetNormalizedAliases() or a property
IReadOnlyCollection<string> NormalizedAliases { get; }) so callers cannot mutate
it; ensure BuildNormalizedAliases still populates the private set using
NormalizeName(PropertyName) and NormalizeName(alias) from Aliases and keep
PropertyName, Aliases, ValueType, MinValue, MaxValue unchanged.

In `@src/Agents/HyperparameterRegistry.cs`:
- Around line 20-141: Change the public surface of the internal utility by
making the HyperparameterRegistry class internal (replace "public class
HyperparameterRegistry" with "internal class HyperparameterRegistry"); keep all
members as-is (they can remain public/private as needed) and rebuild to surface
any external references—if external assemblies need access, add an
InternalsVisibleTo attribute instead of making the class public. Ensure the
constructor HyperparameterRegistry() and usages like GetPropertyName,
GetDefinition, Validate, and Register still compile after the visibility change.

In `@src/AiModelBuilder.cs`:
- Around line 5865-5883: When auto-applying hyperparameters, don't fall back to
ModelType.SimpleRegression when `_model` is already configured; instead derive
the modelType from the actual configured model and only proceed if it can be
resolved—use `_model`'s concrete type or an explicit mapping to determine
`modelType` before calling `AgentHyperparameterApplicator<T>.Apply`; if the
model type cannot be resolved, skip auto-apply and leave
`recommendation.HyperparameterApplicationResult` unset or marked as skipped.
Locate the block gated by `_agentOptions.EnableAutoApplyHyperparameters` that
checks `_model is IConfigurableModel<T> configurableModel` and
`recommendation.SuggestedHyperparameters`, replace the unconditional
`recommendation.SuggestedModelType ?? ModelType.SimpleRegression` with
resolution logic that prefers `recommendation.SuggestedModelType` or derives
from `configurableModel` (or aborts), then call `applicator.Apply` only when a
valid `modelType` is available; keep `HyperparameterRegistry` and
`AgentHyperparameterApplicator` internal as implementation details per the API
guidelines.

In `@src/Interfaces/IConfigurableModel.cs`:
- Around line 22-28: Change the visibility of the IConfigurableModel<T>
interface from public to internal to hide internal configuration mechanics from
consumers; update its declaration (IConfigurableModel<T>) so the GetOptions()
method and returned ModelOptions remain internal-only, ensuring the public
facade surface still exposes only AiModelBuilder and AiModelResult; run
build/tests to confirm no external usages break and adjust any internal
consumers or tests to use the internal interface.

In `@src/MetaLearning/Options/MANNOptions.cs`:
- Line 28: MANNOptions<T, TInput, TOutput> currently inherits from ModelOptions
which exposes ModelOptions' members (e.g., Seed) in the public API; change this
by either making MANNOptions internal or removing the public inheritance from
ModelOptions and instead surface any required configuration through the facade
(AiModelBuilder / AiModelResult) or an internal wrapper; specifically update the
declaration of MANNOptions<T, TInput, TOutput> (and any related
constructors/properties) so it does not publicly expose ModelOptions, and ensure
any code that relied on ModelOptions members accesses them via AiModelBuilder or
an internal API instead.

In `@src/MetaLearning/Options/ReptileOptions.cs`:
- Line 31: ReptileOptions<T, TInput, TOutput> currently publicly inherits
ModelOptions which widens the public API; to fix, remove the public inheritance
from ModelOptions and either make ModelOptions internal or refactor
ReptileOptions to use composition (hold an internal ModelOptions instance) and
implement only IMetaLearnerOptions<T>, exposing only the minimal properties
needed by AiModelBuilder.cs/AiModelResult.cs; ensure no unintended ModelOptions
members are public on ReptileOptions and update references to use the internal
ModelOptions helper where necessary.

In `@src/Models/HyperparameterApplicationResult.cs`:
- Around line 22-47: The HyperparameterApplicationResult type is public but
should be internal per the facade-only API rule; change the class declaration
for HyperparameterApplicationResult from public to internal (and, if desired,
make its properties internal or keep them public if used across the same
assembly) and update any usages in AiModelResult or AiModelBuilder to ensure
they reference the internal type in the same assembly (adjust test/access
modifiers or move/type-forward if needed) so the class is not part of the public
surface anymore.

In `@src/Models/Options/AudioNeuralNetworkOptions.cs`:
- Line 6: The new public class AudioNeuralNetworkOptions expands the public API
surface; if it isn’t intended for direct consumption, change its visibility from
public to internal (AudioNeuralNetworkOptions) and ensure any consumers access
it through the existing facade (AiModelBuilder/AiModelResult) instead—update
AiModelBuilder to accept or construct AudioNeuralNetworkOptions internally (or
return a facade type) and keep NeuralNetworkOptions minimal/public only if it is
part of the documented facade; also review any properties or methods on
AudioNeuralNetworkOptions and make them internal or expose equivalent
configuration methods on AiModelBuilder so no new public types leak from the
assembly.

In `@src/Models/Options/DocumentNeuralNetworkOptions.cs`:
- Around line 1-7: The new options class DocumentNeuralNetworkOptions is public
but should be internal per the API-surface guideline; change its accessibility
from public to internal for DocumentNeuralNetworkOptions and ensure its base
type NeuralNetworkOptions has at least internal visibility so wiring still
compiles, or alternatively expose this type only through the facade
(AiModelBuilder/AiModelResult) if it must remain public.

In `@src/Models/Options/FinancialNLPOptions.cs`:
- Around line 1-7: The FinancialNLPOptions class is declared public but is
intended for internal model wiring only; change its accessibility to internal by
updating the class declaration for FinancialNLPOptions (which inherits
FinancialNeuralNetworkOptions) so it is internal instead of public, ensuring
only the facade types (AiModelBuilder and AiModelResult) remain public.

In `@src/Models/Options/NeuralNetworkOptions.cs`:
- Around line 17-18: NeuralNetworkOptions is declared public but is not part of
the facade API; change its accessibility to internal by replacing "public class
NeuralNetworkOptions : ModelOptions" with "internal class NeuralNetworkOptions :
ModelOptions". Verify usages of NeuralNetworkOptions (and any constructors or
factory methods) are internal or within the same assembly and update any public
surface that accidentally exposes this type (ensure AiModelBuilder and
AiModelResult remain the only public-facing model types). If this class truly
needs to be public for external consumers, leave it public but add a comment
documenting why it’s part of the facade.

In `@src/NeuralNetworks/NeuralNetworkBase.cs`:
- Line 25: NeuralNetworkBase<T> currently exposes GetOptions via the public
IConfigurableModel<T> implementation, which makes it part of the non‑facade
public API; change the implementation of GetOptions on NeuralNetworkBase<T> to
be explicit (i.e., implement IConfigurableModel<T>.GetOptions explicitly) or
make the member internal so it is not publicly visible, and apply the same
change to the related members referenced around lines 106-116 to ensure all
configurable/options accessors are not public on the base class.

In `@src/PhysicsInformed/Interfaces/IInverseProblem.cs`:
- Line 262: InverseProblemOptions<T> was made public and now inherits
AiDotNet.Models.Options.ModelOptions which widens the public API surface; to
fix, either make InverseProblemOptions<T> internal (reverting its visibility) or
keep it public but remove/avoid inheriting ModelOptions and instead encapsulate
only the required settings, ensuring users still only interact via
AiModelBuilder.cs and AiModelResult.cs; update the declaration of
InverseProblemOptions<T> (or its base) accordingly and run
serialization/inheritance tests to confirm no breaking changes.

In `@src/Regression/NonLinearRegressionBase.cs`:
- Around line 29-30: NonLinearRegressionBase<T> currently exposes
IConfigurableModel<T> and a public GetOptions(), expanding the public API;
implement IConfigurableModel<T> explicitly instead: remove public
IConfigurableModel<T> from the class signature (keep INonLinearRegression<T>),
move the public GetOptions() into an explicit interface implementation
(IConfigurableModel<T>.GetOptions) so it isn’t accessible from the concrete
type, and ensure any callers in internal plumbing access it via the interface;
alternatively, if the interface is truly internal-only, change
IConfigurableModel<T> to internal to keep the configuration surface off the
facade.

In `@src/TimeSeries/TimeSeriesModelBase.cs`:
- Line 43: The TimeSeriesModelBase<T> currently exposes IConfigurableModel<T>
and its GetOptions publicly; change IConfigurableModel<T> to internal (or keep
it public but implement it explicitly on TimeSeriesModelBase<T>) so GetOptions
is not part of the public surface of TimeSeriesModelBase<T>; update the class
declaration in TimeSeriesModelBase<T> to remove the public interface surface
(implement IConfigurableModel<T>.GetOptions as an explicit interface
implementation) and ensure AiModelBuilder can still access the interface by
keeping AiModelBuilder in the same assembly or using InternalsVisibleTo as
needed; apply the same explicit-implementation/internal approach to the other
affected base types referenced around lines 63-66.

In
`@tests/AiDotNet.Tests/IntegrationTests/Agents/HyperparameterAutoApplyIntegrationTests.cs`:
- Around line 245-272: The test currently uses conditional assertions that never
fail; update the test for GetOptions on GradientBoostingRegression to enforce a
single expected outcome instead of branching: decide whether the desired
behavior is that GetOptions returns GradientBoostingRegressionOptions or
DecisionTreeOptions, then replace the conditional isCorrectType block with a
direct Assert.IsType<...>(returnedOptions) for the chosen type (or mark the test
with a skip/Explicit attribute and include a short known-issue message if this
is intentionally unresolved). Modify the test referencing
GradientBoostingRegression, IConfigurableModel.GetOptions,
GradientBoostingRegressionOptions and DecisionTreeOptions accordingly (and apply
the same change to the similar test at the later block) so the test fails when
behavior diverges from the decided expectation.

🧹 Nitpick comments (12)

src/PhysicsInformed/Interfaces/IMultiScalePDE.cs (1)

181-181: Consider whether ModelOptions members should be exposed here.

Inheriting ModelOptions adds its public members to a public interface-layer options type. If those members aren’t meant to be part of the facade surface, consider making this type internal or relocating it outside Interfaces. As per coding guidelines: Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs; Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.
src/Clustering/Options/ClusteringOptions.cs (1)
63-67: Consider deprecating RandomState in favor of the inherited Seed property.

The delegation to Seed maintains backward compatibility, but having two properties for the same value could confuse users. Consider adding an [Obsolete] attribute to guide users toward the canonical Seed property.
💡 Optional: Mark RandomState as obsolete
+    [Obsolete("Use the inherited Seed property instead. RandomState will be removed in a future version.")]
     public int? RandomState
     {
         get => Seed;
         set => Seed = value;
     }
src/Agents/HyperparameterResponseParser.cs (3)
21-21: Consider making this class internal to maintain the facade pattern.

Per the coding guidelines, implementation details should be hidden behind the AiModelBuilder facade. This parser is an internal infrastructure component that users shouldn't interact with directly.
-public class HyperparameterResponseParser
+internal class HyperparameterResponseParser
As per coding guidelines: "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

63-63: Regex patterns with Compiled flag should be static fields for efficiency.

These regex patterns are created inside methods but use RegexOptions.Compiled. This means the regex is recompiled on every method invocation, negating the performance benefit of compilation. Consider extracting to static readonly fields.
♻️ Suggested refactor
+private static readonly Regex JsonBlockPattern = new(@"```(?:json)?\s*\n?([\s\S]*?)\n?\s*```", RegexOptions.Compiled);
+private static readonly Regex RawJsonPattern = new(@"\{[^{}]*(?:\{[^{}]*\}[^{}]*)*\}", RegexOptions.Compiled);
+private static readonly Regex MarkdownBoldPattern = new(@"\*\*(\w[\w_]*?):\*\*\s*([^\s(]+)", RegexOptions.Compiled);
+private static readonly Regex ColonSeparatedPattern = new(@"^\s*[-\*\s]*(\w[\w_]*?)\s*[:=][^\S\n]*([^\s(,]+)", RegexOptions.Multiline | RegexOptions.Compiled);

 internal Dictionary<string, object> TryParseJson(string text)
 {
     var result = new Dictionary<string, object>();

-    var jsonBlockPattern = new Regex(@"```(?:json)?\s*\n?([\s\S]*?)\n?\s*```", RegexOptions.Compiled);
-    var matches = jsonBlockPattern.Matches(text);
+    var matches = JsonBlockPattern.Matches(text);
Also applies to: 75-75, 116-116, 138-138

210-218: Consider making the non-parameter set a static readonly field.

The HashSet is recreated on every call. Since the contents are constant, this can be a static readonly field to avoid repeated allocations.
♻️ Suggested refactor
+private static readonly HashSet<string> NonParameterNames = new(StringComparer.OrdinalIgnoreCase)
+{
+    "step", "note", "example", "reason", "because", "since",
+    "model", "algorithm", "method", "approach", "result",
+    "summary", "recommendation", "suggestion", "tip"
+};

 private static bool IsCommonNonParameter(string name)
 {
-    var nonParams = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
-    {
-        "step", "note", "example", "reason", "because", "since",
-        "model", "algorithm", "method", "approach", "result",
-        "summary", "recommendation", "suggestion", "tip"
-    };
-    return nonParams.Contains(name);
+    return NonParameterNames.Contains(name);
 }
src/Models/Options/PortfolioOptimizerOptions.cs (1)
1-3: Redundant using directive for own namespace.

Line 1 imports AiDotNet.Models.Options, but the file is declared in that same namespace on line 3. This using can be safely removed.
🧹 Suggested cleanup
-using AiDotNet.Models.Options;
-
 namespace AiDotNet.Models.Options;
src/Models/AgentRecommendation.cs (1)
405-429: Consider keeping detailed application results internal if not user-facing.

If end users only need a summary, consider exposing a simple string/summary instead and keeping HyperparameterApplicationResult internal to limit surface area.
💡 Suggested change (if not meant to be public)
-public HyperparameterApplicationResult? HyperparameterApplicationResult { get; set; }
+internal HyperparameterApplicationResult? HyperparameterApplicationResult { get; set; }
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API".
src/Regression/RegressionBase.cs (1)
31-68: Consider explicit interface implementation to avoid expanding the public surface.
If IConfigurableModel<T> stays non-public, this can be hidden from consumers and keep RegressionBase<T>’s public API stable.
♻️ Suggested refactor
-    public virtual ModelOptions GetOptions() => Options;
+    ModelOptions IConfigurableModel<T>.GetOptions() => Options;
As per coding guidelines: "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."
tests/AiDotNet.Tests/UnitTests/Agents/HyperparameterRegistryTests.cs (1)

200-236: Validation behavior aligns with lenient-by-design approach.

The tests confirm that out-of-range values produce warnings rather than failures, allowing the applicator to proceed with caution. This matches the opt-in, non-blocking philosophy of the feature.

Consider adding a test that verifies Validate returns IsValid = false for truly invalid scenarios (if any exist), to ensure the Invalid() factory method gets exercised.

src/Agents/HyperparameterValidationResult.cs (1)

41-46: Minor: Invalid() reuses Warning property for the error reason.

The Invalid() factory sets HasWarning = true and stores the reason in Warning, which slightly conflates warnings and errors. This works but could be clearer with an ErrorMessage property or renamed to Message.

Not blocking — the current design is functional and the intent is clear.
src/MetaLearning/Options/MetaSGDOptions.cs (1)
712-713: Consider extracting magic number 10.0 as a named constant.

The validation MaxLearningRate > 10.0 uses a hardcoded value. A named constant like MaxAllowedLearningRate would improve readability and make the limit discoverable/adjustable.
private const double MaxAllowedLearningRate = 10.0;
Not blocking — the current validation is functional.
tests/AiDotNet.Tests/IntegrationTests/Agents/HyperparameterAutoApplyIntegrationTests.cs (1)

22-76: Prefer exercising the facade in end‑to‑end tests.

These tests directly instantiate parser/registry/applicator. For the true end‑to‑end path, consider routing at least one integration test through AiModelBuilder (with AgentAssistanceOptions.EnableAutoApplyHyperparameters) to keep tests aligned to the facade and reduce coupling to internal components.

As per coding guidelines, “Prefer testing through the public facade (AiModelBuilder) when possible.”

sonarqubecloud · 2026-02-08T01:18:42Z

Quality Gate failed

Failed conditions
4 Security Hotspots
1.5% Coverage on New Code (required ≥ 80%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

…itance (#460) - Add GetOptions() override to all 49 RL agent classes - Make 65+ standalone options classes inherit from ModelOptions (via background agent) - Remove duplicate Seed properties from 11 options classes that conflict with ModelOptions.Seed - Make ConsensusClusteringOptions inherit from ModelOptions, rename RandomSeed to Seed - Add GetOptions to remaining clustering, meta-learning, and time series models - Fix MultiLabelClassifierBase to support IConfigurableModel<T> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)

src/Models/Options/NASOptions.cs (1)
371-404: ⚠️ Potential issue | 🟡 Minor

With method does not copy the inherited Seed property from ModelOptions.

The copy logic manually assigns all local properties but misses the base class Seed property. This means calling With(...) on an options instance that has Seed set will produce a copy where Seed is reset to its default value.
Proposed fix
         var copy = new NASOptions<T>
         {
+            Seed = Seed,
             Strategy = Strategy,
             TargetPlatform = TargetPlatform,
src/Models/Options/FinMAOptions.cs (1)
38-60: ⚠️ Potential issue | 🟠 Major

Copy base ModelOptions state when cloning.

After inheriting ModelOptions, the copy ctor does not propagate base properties (e.g., Seed), so cloning can silently drop determinism settings.
🔧 Suggested fix
 public FinMAOptions(FinMAOptions<T> other)
 {
     if (other == null) throw new ArgumentNullException(nameof(other));
+    Seed = other.Seed;
     MaxSequenceLength = other.MaxSequenceLength;
     VocabularySize = other.VocabularySize;
     HiddenDimension = other.HiddenDimension;
src/Clustering/Ensemble/ConsensusClusteringOptions.cs (1)
34-38: ⚠️ Potential issue | 🟠 Major

Validate NumBaseClusterings to avoid divide‑by‑zero.
BuildCoAssociationMatrix divides by clusterings.Count; a value ≤ 0 will crash at runtime.
✅ Suggested guarded setter
-    public int NumBaseClusterings { get; set; } = 10;
+    private int _numBaseClusterings = 10;
+    public int NumBaseClusterings
+    {
+        get => _numBaseClusterings;
+        set => _numBaseClusterings = value < 1
+            ? throw new ArgumentOutOfRangeException(nameof(NumBaseClusterings), "Must be >= 1.")
+            : value;
+    }
src/Clustering/Density/Denclue.cs (1)
49-62: ⚠️ Potential issue | 🟠 Major

Ensure GetOptions() returns the same instance the model actually uses.

When options is null, the constructor creates two separate DenclueOptions instances (one for base Options, one for _options). GetOptions() exposes _options, but HillClimb reads Options.MaxIterations, so auto-applied changes can be ignored and the model becomes inconsistent. Use a single options instance.
🔧 Proposed fix
 public Denclue(DenclueOptions<T>? options = null)
     : base(options ?? new DenclueOptions<T>())
 {
-    _options = options ?? new DenclueOptions<T>();
+    _options = (DenclueOptions<T>)Options;
 }
src/Clustering/Neural/SelfOrganizingMap.cs (1)
50-65: ⚠️ Potential issue | 🟠 Major

Keep GetOptions() aligned with the base Options instance.

When options is null, _options and Options are different objects. Training reads Options.RandomState/MaxIterations, while auto-apply mutates _options via GetOptions(), so applied values can be silently ignored. Point _options to Options to ensure a single source of truth.
🔧 Suggested fix
 public SelfOrganizingMap(SOMOptions<T>? options = null)
     : base(options ?? new SOMOptions<T>())
 {
-    _options = options ?? new SOMOptions<T>();
+    _options = (SOMOptions<T>)Options;
 }

🟠 Major comments (30)

src/Models/Options/REINFORCEOptions.cs-25-25 (1)
25-25: ⚠️ Potential issue | 🟠 Major

Consider reducing public API surface for REINFORCEOptions<T>.

Unless this is intentionally exposed via AiModelBuilder/AiModelResult, make it internal to preserve the facade boundary.
Suggested change
-public class REINFORCEOptions<T> : ModelOptions
+internal class REINFORCEOptions<T> : ModelOptions
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”
src/Audio/Fingerprinting/SpectrogramFingerprintOptions.cs-8-8 (1)

8-8: ⚠️ Potential issue | 🟠 Major

Consider reducing public API surface for options types.

Line 8 keeps this options class public, but the facade guidelines require users to interact only with AiModelBuilder/AiModelResult. If this type isn’t explicitly part of the facade API, make it internal or ensure the facade is the only exposure point.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public unless part of the facade API.”

src/Regression/RobustRegression.cs-46-47 (1)

46-47: ⚠️ Potential issue | 🟠 Major

Public GetOptions() may violate the facade-only API surface.

This adds a new public method on a model type, which conflicts with the “users should only interact with AiModelBuilder/AiModelResult” guideline. If this is only for internal pipeline use, consider an explicit interface implementation (or internal visibility in the base/interface) to avoid expanding the public API. As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs… Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”
src/Models/Options/TradingAgentOptions.cs-67-70 (1)
67-70: ⚠️ Potential issue | 🟠 Major

Property shadowing with new undermines the ModelOptions consolidation.

Using new int? Seed hides the inherited Seed from ModelOptions rather than reusing it. This creates two separate Seed storage slots:

Accessing Seed via a TradingAgentOptions<T> reference uses this property

Accessing Seed via a ModelOptions reference uses the base property

The reflection-based AgentHyperparameterApplicator may set the wrong one depending on which type it reflects over

Since the PR objective is to consolidate Seed into the base class, this shadowing appears unintentional. Remove the redeclaration and let the class inherit Seed from ModelOptions.
Proposed fix
-    /// <summary>
-    /// Random seed for reproducibility.
-    /// </summary>
-    public new int? Seed { get; set; }
src/Audio/Classification/SceneClassifierOptions.cs-9-9 (1)

9-9: ⚠️ Potential issue | 🟠 Major

Public API surface expanded via inheritance — confirm intent.
Line 9 makes a public options type inherit ModelOptions, which likely exposes additional public members (e.g., Seed) to consumers. If this type isn’t meant to be part of the public surface, consider making it internal or otherwise hiding the inherited members. As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”
src/Models/Options/AiModelResultOptions.cs-83-83 (1)
83-83: 🛠️ Refactor suggestion | 🟠 Major

Avoid expanding public API outside the facade.

Inheriting ModelOptions adds its public members (e.g., Seed) onto a public, non-facade type, widening the API surface beyond AiModelBuilder/AiModelResult. Consider making this type internal or moving/encapsulating the options behind the facade to keep the surface minimal.
Possible direction
-public class AiModelResultOptions<T, TInput, TOutput> : ModelOptions
+internal class AiModelResultOptions<T, TInput, TOutput> : ModelOptions
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs”.
src/Audio/AudioGen/AudioGenOptions.cs-73-73 (1)
73-73: ⚠️ Potential issue | 🟠 Major

Property shadowing with new can cause subtle bugs in reflection-based code.

Using new to shadow Seed means:

When accessed via a ModelOptions reference, the base class Seed is read/written—not this one.

The reflection-based AgentHyperparameterApplicator may set the base property while this derived property remains unset (or vice versa).

The PR objective mentioned "removed duplicate Seed properties" to favor base-type handling. If the base ModelOptions.Seed is int?, consider removing this shadowed property entirely and inheriting. If the base is non-nullable int, consider making the base nullable instead to avoid shadowing across all derived options classes.
#!/bin/bash
# Description: Check the type of Seed in ModelOptions to understand the shadowing necessity.

# Find ModelOptions base class definition and inspect Seed property
ast-grep --pattern 'class ModelOptions {
  $$$
  Seed { $$ }
  $$$
}'

# Alternative: search for Seed property definition in ModelOptions
rg -n -A2 -B2 'public.*Seed' --glob '**/ModelOptions.cs'
src/Clustering/Streaming/OnlineKMeans.cs-52-53 (1)

52-53: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands API beyond the facade

This adds a new public method on a model type, which appears to conflict with the facade-only public surface. If possible, keep GetOptions off the public API (e.g., internal or explicit interface implementation) or otherwise confirm that this is intended to be part of the facade. As per coding guidelines: "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."
src/Models/Options/StratifiedKFoldCrossValidationFitDetectorOptions.cs-34-34 (1)
34-34: ⚠️ Potential issue | 🟠 Major

Facade pattern: avoid expanding public API with options types.

This class is public in src/** and now inherits ModelOptions, which further exposes base members (e.g., Seed). If it’s not intended to be part of the facade, please make it internal or route access through AiModelBuilder/AiModelResult only.
🔧 Suggested change (if not facade-exposed)
-public class StratifiedKFoldCrossValidationFitDetectorOptions : ModelOptions
+internal class StratifiedKFoldCrossValidationFitDetectorOptions : ModelOptions
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."
src/Models/Options/DDPGOptions.cs-9-9 (1)
9-9: 🛠️ Refactor suggestion | 🟠 Major

Narrow the public API surface for options types.
Since this line changes the class declaration, please re-check whether DDPGOptions<T> must be public. If it’s not explicitly exposed via the facade, make it internal to avoid expanding the public API.
Proposed change
-public class DDPGOptions<T> : ModelOptions
+internal class DDPGOptions<T> : ModelOptions
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”
src/Models/Options/PatchTSTOptions.cs-33-33 (2)
33-33: ⚠️ Potential issue | 🟠 Major

Reduce public API surface for options types.

PatchTSTOptions<T> is public, which broadens the surface outside the facade. Unless this is intentionally exposed via AiModelBuilder/AiModelResult, make it internal.
💡 Suggested change
-public class PatchTSTOptions<T> : ModelOptions
+internal class PatchTSTOptions<T> : ModelOptions
As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes… unless they are part of the facade API.”

33-33: ⚠️ Potential issue | 🟠 Major

Avoid dual seed sources after inheriting ModelOptions.

With ModelOptions now in the inheritance chain, keeping a separate RandomSeed property risks conflicting values (e.g., agent sets Seed while training reads RandomSeed). Consider removing RandomSeed or shimming it to the base Seed to keep a single source of truth.
💡 Suggested shim to avoid divergence
-    public int? RandomSeed { get; set; }
+    public int? RandomSeed
+    {
+        get => Seed;
+        set => Seed = value;
+    }
src/Models/Options/FineTuningOptions.cs-18-18 (1)

18-18: ⚠️ Potential issue | 🟠 Major

Public API surface likely widened beyond the facade.

Inheriting ModelOptions on a public options type exposes additional members (e.g., Seed) that sit outside the AiModelBuilder/AiModelResult facade. Please confirm this is intended or make FineTuningOptions<T> internal (and expose via the facade) to keep the API surface tight.
As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs … Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”
src/Models/Options/ModelStatsOptions.cs-30-30 (1)
30-30: 🛠️ Refactor suggestion | 🟠 Major

Consider making this options type internal to preserve the facade-only API.

Inheriting ModelOptions adds public members to a public type outside the facade. If this isn’t intended for direct user consumption, mark it internal and access via AiModelBuilder/AiModelResult.
♻️ Suggested change
-public class ModelStatsOptions : ModelOptions
+internal class ModelStatsOptions : ModelOptions
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API”.
src/Models/Options/DuelingDQNOptions.cs-9-9 (1)
9-9: 🛠️ Refactor suggestion | 🟠 Major

Re-check public exposure now that ModelOptions is inherited.

This keeps a non-facade options type public while adding base members to its surface. If it’s not meant to be user-facing, prefer internal and expose configuration through the facade.
♻️ Suggested change
-public class DuelingDQNOptions<T> : ModelOptions
+internal class DuelingDQNOptions<T> : ModelOptions
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API”.
src/Models/Options/FeatureImportanceFitDetectorOptions.cs-19-19 (1)

19-19: ⚠️ Potential issue | 🟠 Major

Confirm facade compliance for this newly-expanded public options type.

Line 19 now exposes ModelOptions members (e.g., Seed) on a public options class, expanding the public API surface. If this type isn’t meant to be directly user-facing, consider making it internal (or otherwise restricting exposure) to keep the facade boundaries intact; if it must be public, please confirm that the added base members are intentional and documented. As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs.”

src/Clustering/Density/Denclue.cs-49-50 (1)

49-50: ⚠️ Potential issue | 🟠 Major

Public GetOptions() expands API surface outside the facade.

This introduces a new public method on a non-facade class, which conflicts with the facade-only API policy. Consider making this internal (or exposing it only through AiModelBuilder/AiModelResult) if external callers don’t need it. As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”

src/ReinforcementLearning/Agents/DynaQPlusAgent.cs-16-18 (1)

16-18: ⚠️ Potential issue | 🟠 Major

Public GetOptions() expands non-facade API surface.
This adds a new public method on a model class; the facade guideline says users should only interact with AiModelBuilder/AiModelResult. If this is only for internal hyperparameter application, consider making the access internal (e.g., internal interface or builder-mediated access).

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for non-facade members.”

src/ReinforcementLearning/Agents/EveryVisitMonteCarloAgent.cs-16-17 (1)

16-17: 🛠️ Refactor suggestion | 🟠 Major

Facade API leakage via public GetOptions.

This adds a new public entry point on a non‑facade class. If this is only meant for internal hyperparameter plumbing, consider making it internal or an explicit interface implementation (so it doesn’t surface on the public API), or document why it must be public.
As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public unless part of the facade API.”

src/Clustering/Hierarchical/BisectingKMeans.cs-45-47 (1)

45-47: ⚠️ Potential issue | 🟠 Major

Public GetOptions() expands API beyond the facade.

This exposes internal configuration on a non-facade type; please keep this internal/explicitly-implemented (or route through AiModelBuilder/AiModelResult) unless there’s a strong public API requirement.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for implementation details.”

src/Clustering/Density/DBSCAN.cs-51-52 (1)

51-52: ⚠️ Potential issue | 🟠 Major

Facade pattern/API surface: avoid new public entry points on model classes.

Line 51-52 adds a public GetOptions() on a non‑facade type. This expands the public API surface beyond AiModelBuilder/AiModelResult. Can this be made internal/explicit‑interface (behind an internal interface) or routed through the facade instead?

As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”

src/ReinforcementLearning/Agents/DreamerAgent.cs-46-47 (1)

46-47: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This adds a public method on a non‑facade type, which conflicts with the facade-only public surface guidance. Please consider making options access internal/explicit to the facade (or otherwise justify why this must be publicly exposed).
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/MetaLearning/Algorithms/MANNAlgorithm.cs-96-97 (1)

96-97: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This exposes options via a public method on a non‑facade class. Please consider keeping option access internal/behind the facade (or otherwise justify the public exposure).
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/MetaLearning/Algorithms/NTMAlgorithm.cs-101-102 (1)

101-102: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This adds a public options accessor on a non‑facade class. Please consider keeping it internal/behind the facade or justify why it needs to be public.
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/ReinforcementLearning/Agents/A2CAgent.cs-41-42 (1)

41-42: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This adds public options exposure on a non‑facade type. Please consider making it internal/explicitly facade‑only or provide justification for the public surface.
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/ReinforcementLearning/Agents/DDPGAgent.cs-48-49 (1)

48-49: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This introduces a public options accessor on a non‑facade class. Consider keeping it internal/behind the facade or justify the public surface.
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/ReinforcementLearning/Agents/DQNAgent.cs-44-45 (1)

44-45: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This exposes options via a public method on a non‑facade type. Please keep this internal/behind the facade or justify the public exposure.
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/Classification/MultiLabel/MLkNNClassifier.cs-36-37 (1)

36-37: ⚠️ Potential issue | 🟠 Major

Public GetOptions expands non‑facade API surface

This exposes options publicly on a non‑facade class. Please consider keeping it internal/behind the facade or justify why this must be public.
As per coding guidelines, "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

src/ReinforcementLearning/Agents/A3CAgent.cs-41-43 (1)

41-43: ⚠️ Potential issue | 🟠 Major

Facade pattern: avoid introducing new public API on non-facade types.

GetOptions() is a new public method on a non-facade class, expanding the public surface. If this is required for hyperparameter application, consider making the interface internal or using explicit interface implementation so the concrete type’s public API stays minimal.

As per coding guidelines: "Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs" and "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."
src/Models/Options/ObjectDetectionOptions.cs-15-15 (1)
15-15: ⚠️ Potential issue | 🟠 Major

Consider making ObjectDetectionOptions<T> internal to preserve the facade.

Unless this type is intentionally exposed by AiModelBuilder/AiModelResult, keep it internal to avoid expanding the public API surface.
🔧 Suggested change
-public class ObjectDetectionOptions<T> : ModelOptions
+internal class ObjectDetectionOptions<T> : ModelOptions
As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”

🧹 Nitpick comments (17)

src/Models/Options/ActiveLearningOptions.cs (1)

119-119: Consider making ActiveLearningOptions internal unless it’s part of the facade surface.

The facade guideline says only AiModelBuilder / AiModelResult (and types they directly expose) should be public. If this options type isn’t intentionally exposed, prefer internal to keep the API surface minimal.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs… Prefer internal over public unless part of the facade API.”

src/Models/Options/PrecisionRecallCurveFitDetectorOptions.cs (1)

40-40: Consider reducing API surface: make this internal unless facade needs it.

This options class is public outside the facade types. Per the architecture guidelines, confirm it must be public; otherwise prefer internal to keep the public surface minimal.

If you want a quick check, I can help generate a script to locate direct public exposure or usage from AiModelBuilder/AiModelResult.
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes…unless they are part of the facade API.”

src/Classification/OrdinalRegression.cs (1)

53-54: Consider reducing public API exposure of GetOptions if not strictly required.

This adds a public method on a non-facade class; if GetOptions is only needed internally, consider explicit interface implementation or narrowing visibility (where possible) to keep the facade pattern intact.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs… Any new public methods or classes that users might call directly should be scrutinized.”

src/Models/Options/PortfolioOptions.cs (1)

7-7: Confirm this options type should remain public.
Given the facade pattern, consider making this internal unless it’s part of the intended public surface via AiModelBuilder/AiModelResult. As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes unless they are part of the facade API.”
src/Models/Options/NASOptions.cs (1)
287-290: Consider consolidating RandomSeed with inherited Seed property.

The class now inherits a Seed property from ModelOptions, but also defines its own RandomSeed property at line 290. Having two seed-related properties with different names could confuse users about which one to set.

Based on the PR objective to "removed duplicate Seed properties," consider whether RandomSeed should be removed in favor of the inherited Seed, or if there's a semantic distinction that warrants keeping both (in which case, the XML documentation should clarify the difference).
Option A: Remove duplicate and use inherited property
-    /// <summary>
-    /// Gets or sets the random seed for reproducibility.
-    /// </summary>
-    /// <remarks>
-    /// <para><b>For Reproducibility:</b> Set a seed to get repeatable results.</para>
-    /// </remarks>
-    public int? RandomSeed { get; set; }
Then update any internal usages to reference Seed instead.
src/Models/Options/IQLOptions.cs (1)
67-77: Consider extending validation to cover IQL-specific hyperparameters.

Given the PR's focus on hyperparameter infrastructure and validation, you might enhance Validate() to cover constraints already documented in comments (e.g., Expectile typically 0.7-0.9) and standard RL invariants (learning rates > 0, DiscountFactor ∈ (0, 1]).
💡 Example validation additions
 public void Validate()
 {
     if (StateSize <= 0)
         throw new ArgumentException("StateSize must be greater than 0", nameof(StateSize));
     if (ActionSize <= 0)
         throw new ArgumentException("ActionSize must be greater than 0", nameof(ActionSize));
     if (BatchSize <= 0)
         throw new ArgumentException("BatchSize must be greater than 0", nameof(BatchSize));
     if (BufferSize <= 0)
         throw new ArgumentException("BufferSize must be greater than 0", nameof(BufferSize));
+    if (Expectile <= 0 || Expectile >= 1)
+        throw new ArgumentException("Expectile must be in (0, 1), typically 0.7-0.9", nameof(Expectile));
 }
src/Models/Options/ShapleyValueFitDetectorOptions.cs (1)
1-1: Remove redundant using if covered by global usings.

If AiDotNet.Models.Options is already in the project-level global usings (as per repo convention), this file-scoped using adds noise and can be dropped.
♻️ Suggested cleanup
-using AiDotNet.Models.Options;
Based on learnings: global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*); prefer relying on global usings to reduce boilerplate.
src/Audio/Classification/GenreClassifierOptions.cs (1)

9-9: Architectural consideration (optional): Per the facade pattern guidelines, domain-specific options classes like this could potentially be internal if users only configure them indirectly through AiModelBuilder. However, given this PR standardizes 60+ options classes with public visibility for direct configuration access, keeping consistency here is reasonable. This is something to consider holistically in a future API surface review rather than piecemeal.
src/Audio/Whisper/WhisperOptions.cs (1)
25-25: Confirm public API expansion from ModelOptions inheritance.

Line 25 now exposes ModelOptions members (e.g., Seed) on a public options type. If WhisperOptions isn’t meant to be part of the facade API surface, consider making it internal (or hiding the base members) to keep users on AiModelBuilder/AiModelResult.
As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public.”
🔧 Optional change if WhisperOptions should not be public
-public class WhisperOptions : ModelOptions
+internal class WhisperOptions : ModelOptions
src/Models/Options/SECBERTOptions.cs (1)

108-128: Consider calling base validation if ModelOptions has or gains a Validate() method.

If ModelOptions provides (or will provide) its own Validate() method for properties like Seed, this override should chain to base.Validate(). This future-proofs the code against base class validation additions.
src/Audio/Speaker/SpeakerEmbeddingOptions.cs (1)
1-1: Consider relying on global usings to avoid redundant imports.

Line 1 may be redundant if AiDotNet.* is already globally imported; consider removing it to keep the file consistent with the project’s global-using setup.
♻️ Optional cleanup
-using AiDotNet.Models.Options;
Based on learnings, "global using directives are declared in AiDotNet.csproj for core namespaces (AiDotNet.Tensors.* and AiDotNet.*) ... Prefer relying on global usings to reduce boilerplate."
src/Clustering/Ensemble/ConsensusClustering.cs (1)

49-50: Consider limiting the new public GetOptions() exposure.
If this accessor is only for internal auto-apply plumbing, prefer an internal/explicit interface implementation to keep end‑users on the facade API.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs.”

src/Clustering/Ensemble/ConsensusClusteringOptions.cs (1)

32-32: Double‑check public exposure of options types.
If end users should only interact with the facade, consider making options internal or otherwise hiding them from the public surface.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs.”

src/Models/Options/BloombergGPTOptions.cs (1)

43-43: Consider making this type internal unless it’s part of the facade API.

If this options class isn’t directly exposed by AiModelBuilder/AiModelResult, keeping it public expands the public surface area unnecessarily.

As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs … Prefer internal over public unless part of the facade API.”

src/Models/Options/SACOptions.cs (1)

30-30: Confirm public API exposure from inheriting ModelOptions.

This adds a new public base type (and its members, e.g., Seed) to a public options class, which can broaden the public surface and be breaking for downstream subclasses. Please confirm this aligns with the facade design; if not, consider keeping options internal or exposing them only via AiModelBuilder.

As per coding guidelines: Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs, and prefer internal over public for other types.
src/MetaLearning/Algorithms/MetaSGDAlgorithm.cs (2)
687-687: Consider making PerParameterOptimizer internal.

Per the architecture guidelines, implementation details should prefer internal over public. PerParameterOptimizer appears to be an internal helper for the Meta-SGD algorithm rather than part of the public facade API. Users interact with MetaSGDAlgorithm and shouldn't need direct access to this optimizer class.
♻️ Suggested change
-public class PerParameterOptimizer<T, TInput, TOutput>
+internal class PerParameterOptimizer<T, TInput, TOutput>
As per coding guidelines: "Prefer internal over public for all classes, methods, and properties unless they are part of the facade API."

1086-1086: Consider making MetaSGDAdaptedModel internal.

Similar to PerParameterOptimizer, this wrapper model is an implementation detail. The Adapt() method returns IModel<TInput, TOutput, ModelMetadata<T>>, so consumers work through the interface rather than the concrete type. Making this class internal would reduce the public API surface without breaking functionality.
♻️ Suggested change
-public class MetaSGDAdaptedModel<T, TInput, TOutput> : IModel<TInput, TOutput, ModelMetadata<T>>
+internal class MetaSGDAdaptedModel<T, TInput, TOutput> : IModel<TInput, TOutput, ModelMetadata<T>>
As per coding guidelines: "Helper classes, utilities, and implementation details should be internal."

…rmed models (#460) Add GetOptions() overrides to 45 model classes across: - Core Neural Networks: MeshCNN, MixtureOfExperts - Audio: AudioLDM, MusicGen, StableAudio, MusicSourceSeparator, VoxLingua107, Wav2Vec2LID, ECAPA-TDNN, SpeakerEmbedding, SpeakerDiarizer, AudioEventDetector, SceneClassifier, GenreClassifier - Finance Graph: TemporalGCN, STGNN, RelationalGCN, MTGNN, GraphWaveNet, DCRNN - Finance Risk: NeuralCVaR, NeuralStressTest, SAINT, TabNet, TabTransformer - Finance Portfolio: HierarchicalRiskParity, BlackLittermanNeural, AttentionAllocation - Finance NLP: FinBERT - Finance Trading: AlphaFactor, FactorTransformer, FactorVAE, MarketMaking - Finance Probabilistic: DiffusionTS, ScoreGrad, TimeGrad, TSDiff, CSDI - Finance StateSpace: TimeMachine, S4, Hippo - Finance Volatility: NeuralGARCH, RealizedVolatilityTransformer - PhysicsInformed: InverseProblemPINN, MultiScalePINN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Finance/NLP/FinBERT.cs (1)
193-205: ⚠️ Potential issue | 🟠 Major

Options mutations won’t update cached fields/layers.
Options = _options exposes mutable options after construction, but _maxSequenceLength, _numLayers, _dropoutRate, etc. are copied once and layers are initialized from those values. If auto-apply mutates options later, FinBERT continues using stale values and recommendations won’t actually take effect. Consider syncing cached fields (and reinitializing layers when safe) after recommendations, or skip auto-apply for this model.
♻️ Suggested refactor to centralize option sync
+    private void ApplyOptionsSnapshot()
+    {
+        _maxSequenceLength = _options.MaxSequenceLength;
+        _vocabularySize = _options.VocabularySize;
+        _hiddenDimension = _options.HiddenDimension;
+        _numAttentionHeads = _options.NumAttentionHeads;
+        _intermediateDimension = _options.IntermediateDimension;
+        _numLayers = _options.NumLayers;
+        _numSentimentClasses = _options.NumSentimentClasses;
+        _dropoutRate = _options.DropoutRate;
+    }
-        _options = options ?? new FinBERTOptions<T>();
-        Options = _options;
+        _options = options ?? new FinBERTOptions<T>();
+        Options = _options;
+        ApplyOptionsSnapshot();
-        _options = options ?? new FinBERTOptions<T>();
-        Options = _options;
+        _options = options ?? new FinBERTOptions<T>();
+        Options = _options;
+        ApplyOptionsSnapshot();
Also applies to: 236-248

🤖 Fix all issues with AI agents

In `@src/Audio/Classification/AudioEventDetector.cs`:
- Around line 62-64: The AudioEventDetector class's constructors do not assign
the backing field to the base Options property, causing GetOptions() (which
returns _options) to diverge from the inherited Options; update each constructor
(the ONNX constructor, the native constructor, and ensure the legacy constructor
which delegates to native inherits this) to set Options = _options after
_options is initialized so the base Options property and the GetOptions()
override remain synchronized (refer to AudioEventDetector, GetOptions, _options,
and Options).

In `@src/Audio/Classification/SceneClassifier.cs`:
- Around line 48-52: The class exposes _options via GetOptions but never wires
the base Options or refreshes derived state; update each constructor to assign
Options = _options and call a new RefreshDerivedState() method that rebuilds
SampleRate, ClassLabels and feature extractors from _options, and also invoke
RefreshDerivedState() whenever Options/_options is changed (e.g., via an
OnOptionsChanged or setter hook) so runtime option changes reinitialize
dependent state in SceneClassifier (referencing SceneClassifierOptions,
GetOptions, Options, SampleRate, ClassLabels, and the feature extractor fields).

In `@src/Audio/SourceSeparation/MusicSourceSeparator.cs`:
- Around line 48-51: The public override GetOptions() on MusicSourceSeparator
exposes internal configuration (SourceSeparationOptions/ModelOptions); change
its accessibility to a non-public surface—either make the override internal
(internal override ModelOptions GetOptions()) or remove the public member and
implement it as an explicit interface or internal helper (e.g., an explicit
IInternalModelOptions.GetOptions implementation or a private/internal
GetOptionsInternal) so only internal components (AiModelBuilder/AiModelResult)
can access it; update all internal callers to use the new non-public API and
ensure the method signature still returns ModelOptions.SourceSeparationOptions
mapping as before.

In `@src/Finance/Forecasting/StateSpace/S4.cs`:
- Around line 87-91: The S4 class currently snapshots S4Options<T> into private
fields at construction (private field _options and usage in layer-building) so
later changes via GetOptions or exposed options won't affect the cached fields
or the built layer stack; add a refresh hook (e.g., a public method
RefreshConfiguration or ApplyOptions) that copies current values from _options
back into the internal cached fields and rebuilds the dependent layers (call the
existing layer construction routine such as BuildLayers/RebuildLayerStack or the
constructor logic), and invoke this hook whenever options are mutated (or expose
a setter that calls it); ensure you update references to _options, S4Options<T>,
and any cached parameters used in forward/initialization to use the refreshed
cached values and add minimal synchronization (lock) around the rebuild to be
thread-safe.

In `@src/Finance/Graph/GraphWaveNet.cs`:
- Around line 99-103: The class currently snapshots GraphWaveNetOptions<T> into
the private field _options and precomputes adjacency/transpose and layer
configuration in the constructor, so later changes to options won’t propagate;
add a refresh hook such as a public method RefreshOptions(GraphWaveNetOptions<T>
newOptions) (or ApplyOptions) that updates the private _options, recomputes
adjacency/transpose and reinitializes layer configuration (the same logic
currently in the constructor), and ensure callers invoke this after
auto-applying hyperparameters; reference the existing private field _options and
the constructor logic that builds adjacency/transpose and layers to locate where
to extract and reuse the recomputation code.

In `@src/Finance/Graph/MTGNN.cs`:
- Around line 97-101: The class caches MTGNNOptions<T> in the private field
_options and builds embeddings, adjacency matrices and layers at construction,
so subsequent mutations via GetOptions() do not update those cached structures;
add a refresh path (e.g., a public RefreshOptions or ApplyOptionsChanges method)
that takes the current _options (or a new MTGNNOptions<T>), re-syncs the cached
fields, disposes/rebuilds embeddings and adjacency structures and reinitializes
layers (referencing the constructor logic that creates
embeddings/adjacency/layers) and call this method from any external setter or
option-wiring point so runtime hyperparameter changes take effect.

In `@src/Finance/Probabilistic/CSDI.cs`:
- Around line 96-99: GetOptions currently returns the live _options instance but
the CSDI class snapshots _options into private fields and builds diffusion
schedule/layers in the constructor, so post-construction mutations to _options
(via auto-apply) leave cached state stale; either change GetOptions to return an
immutable deep copy of _options to prevent external mutation, or implement a
refresh method (e.g., RefreshOptions or RebuildDependentState) that re-reads
_options and reconstructs the cached schedule/layers and any other snapshot
fields (called from the constructor and any option-apply hook) so GetOptions can
safely expose the current configuration without desyncing inference (references:
GetOptions, _options, the CSDI constructor, diffusion schedule/layers private
fields).

In `@src/Finance/Risk/NeuralCVaR.cs`:
- Around line 32-36: NeuralCVaR<T> stores a readonly _options and returns it via
GetOptions(), so post-construction changes to those options won't rebuild the
network; add a public Refresh(NeuralCVaROptions<T> newOptions) (or
ReloadOptions) on NeuralCVaR<T> that replaces the current options and calls the
existing network reinitialization routine (e.g., invoke the method that builds
layers—add a RebuildNetworkLayers/ReinitializeLayers helper if none exists) to
reconstruct layers and refresh any cached config, and change GetOptions() to
return a defensive copy (deep clone) of _options rather than the internal
reference to prevent external mutation; update any constructors/usage to call
Refresh when live option updates are applied.

In `@src/Finance/Risk/TabNet.cs`:
- Around line 28-32: The class caches TabNetOptions<T> in the private field
_options and exposes them via GetOptions(), but updating those live options does
not rebuild the cached config or layer stack the constructor creates; add an
explicit refresh/update path (e.g., a public UpdateOptions(TabNetOptions<T>
newOptions) or OnOptionsChanged method) that assigns _options = newOptions,
re-syncs any cached fields derived from _options, and calls the layer
reinitialization routine (implement or call a RebuildLayers/InitializeLayers
private method that encapsulates the constructor's layer-creation logic used in
the class constructor); ensure the update method is thread-safe (lock or
Interlocked) and used wherever options can change so new options take effect
immediately.

In `@src/Finance/Trading/Agents/MarketMakingAgent.cs`:
- Around line 23-28: The public GetOptions() method on MarketMakingAgent<T>
expands the public API surface; change its accessibility to internal (or make
the MarketMakingAgent<T> class internal) so only the facade types
(AiModelBuilder / AiModelResult) are exposed; specifically, update the signature
of GetOptions() (and any interface/base it implements, e.g., ModelOptions
retrieval) to internal and ensure MarketMakingOptions<T> usage remains
accessible internally without exposing the method publicly.

🧹 Nitpick comments (2)

src/Finance/Portfolio/BlackLittermanNeural.cs (1)

32-34: Consider reducing public API exposure of GetOptions.

Line 32-34 adds/solidifies a public override that exposes internal configuration. If this is only needed for internal auto-apply plumbing, consider making it internal or an explicit interface member to keep the public surface minimal and aligned with the facade model.

As per coding guidelines: “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public for all classes, methods, and properties unless they are part of the facade API.”

src/Finance/NLP/FinBERT.cs (1)

104-108: Re-check the public GetOptions() exposure against the facade API.
If this exists only for internal auto-apply, consider reducing visibility (e.g., explicit interface implementation or internal in the base) to avoid expanding the public surface of model classes.
As per coding guidelines, “Users should ONLY interact with AiModelBuilder.cs and AiModelResult.cs” and “Prefer internal over public”.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

) Create domain-specific options classes for models that don't yet have private _options fields. Each class inherits from the appropriate base (NeuralNetworkOptions, DocumentNeuralNetworkOptions, PhysicsInformedOptions) and can be extended with model-specific hyperparameters over time. - 79 core NeuralNetworks/Options/ files - 29 Document/Options/ files - 30 Video/Options/ files - 9 PhysicsInformed/Options/ files - 2 PointCloud/Options/ files - 1 ProgramSynthesis/Options/ file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add _options field, GetOptions() override, and constructor parameter to all core neural network model files. Also add 12 new options classes for models that were missing them (BGE, ColBERT, ConditionalGAN, DCGAN, InstructorEmbedding, MatryoshkaEmbedding, SGPT, SimCSE, SPLADE, and 3 PhysicsInformed PINNs). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

… NeuralRadianceFields, and ProgramSynthesis models (#460) Wire _options fields, GetOptions() overrides, and constructor parameters into 78 remaining model classes across all domains: - 29 Document models (OCR, layout-aware, graph-based, pixel-to-sequence, vision-language) - 30 Video models (action recognition, depth, enhancement, generation, segmentation, tracking, etc.) - 12 PhysicsInformed models (PINNs, neural operators, scientific ML) - 3 PointCloud models (DGCNN, PointNet, PointNet++) - 3 NeuralRadianceFields models (GaussianSplatting, InstantNGP, NeRF) - 1 ProgramSynthesis model (NeuralProgramSynthesizer) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Create a fresh default options instance of the same concrete type to compare property values, instead of comparing against default(T). This correctly handles non-zero defaults (e.g., LearningRate=0.001) and reference types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/Agents/AgentHyperparameterApplicator.cs`:
- Around line 194-210: Replace the current enum handling block
(targetType.IsEnum / Enum.Parse) to validate and log failures: use Enum.TryParse
for string inputs and if it fails call _logger?.LogWarning with the string and
targetType.Name and return null; for numeric inputs convert to the enum's
underlying type using Convert.ChangeType with CultureInfo.InvariantCulture,
check membership with Enum.IsDefined before calling Enum.ToObject, log a warning
if not defined, and catch/ log any conversion exceptions via _logger?.LogWarning
then return null; remove the silent catch that returns null after Enum.Parse.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

- Use Enum.IsDefined after parsing to reject undefined enum values - Handle numeric-to-enum conversion via Enum.ToObject with validation - Catch specific exceptions instead of generic catch for enum parsing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…engine ops [prototype] Rewrites the inference-only cached GQA layer to apply RoPE via Engine.ApplyRoPEInterleaved and grouped-query attention via Engine.ScaledDotProductAttentionGqa (unexpanded K/V) — a device-agnostic, GPU-graph-recordable forward that drops the two managed unrecordable hotspots (managed RoPE + ExpandKVHeads). RotaryPositionalEncodingLayer exposes its cos/sin caches (GetInterleavedCaches) for the fused op. ALiBi keeps the expand + FlashAttention path. Verified: the optimized (non-paged) model's forward matches the original managed GQA decoder within 1e-3 (InferenceOptimizer_CachedGroupedQueryAttention_RewrittenForward_MatchesOriginal), run against a local AiDotNet.Tensors probe. PROTOTYPE / RELEASE-GATED: the pin is a LOCAL probe (0.117.200-devexec) of the unreleased AiDotNet.Tensors PR #828 (device-agnostic execution). It MUST be bumped to the released AiDotNet.Tensors version before this lands / any AiDotNet PR — CI cannot restore the probe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…ecode/prefill perf (#1917) * feat(device): model.To(DeviceInfo) / layer.To(DeviceInfo) placement API PyTorch-style device placement, enum-only (no magic-string overloads). LayerBase.To moves registered parameters + buffers and recurses sublayers; NeuralNetworkBase.To moves every layer. Skips zero-length (deferred) params. First user-facing piece of the device-agnostic execution model; builds net10.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(device): facade ConfigureModel(source, DeviceInfo) overload (Level 1) Load a pretrained checkpoint and place the whole model on a device in one call — the common "load onto my GPU" case — via the type-safe enum DeviceInfo (no device strings). Additive overload: existing ConfigureModel(source) callers are unchanged. Delegates to model.To(device). Builds net10.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(device): regression test for model.To(OpenCL()) placement -> 7042 Proves the PyTorch-style placement API (model.To(DeviceInfo.OpenCL())) drives correct GPU execution to llama.cpp's greedy token. Uses AutoDetectAndConfigureGpu so placement (Tensor.To -> global backend) and execution (Current dispatcher) share one backend; skips when no GPU is wired in-process (validates on DevHost/CUDA). Surfaced that To() and Current use different backend acquisition — unify in Phase 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(device): phase-2 fusion acceptance test (skipped) + document the gap Runs the whole decoder forward inside a DeferredScope (BeginDeferredScope -> Predict -> Execute) as the acceptance check for transparent fusion. Today it returns token 0 (all-zero logits) instead of 7042: the graph capture/replay does not materialize the decoder's output, so transparent fusion needs the graph path (output binding + RoPE/GQA-SDPA/RMSNorm recording) debugged before it can wrap Predict. Skipped with that reason; un-skip when the graph path is fixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serving): inferenceOptimizer recurses into PreLNTransformerBlock for GQA KV-cache Adds PreLNTransformerBlock.ReplaceAttention (mirrors TransformerEncoderBlock) and a PreLNTransformerBlock case in ApplyAttentionOptimizations that swaps the block's nested GroupedQueryAttentionLayer for a KV-cached CachedGroupedQueryAttention (shared helper BuildCachedGqaReplacement, reused by the top-level GQA case). This is the first piece of wiring the GGUF/LLaMA decoder (GQA nested in PreLNTransformerBlock) to incremental KV-cache decode. Still inert end-to-end: InitializeGQAKVCache's collection scan is also top-level (won't yet find the nested cached GQA), and ServableModelWrapper requires a paged cache that GQA lacks — so the incremental clone is still discarded and serving falls back to the eager model (no regression). Remaining: recurse InitializeGQAKVCache; accept the non-paged GQA cache as the incremental cache. Builds net10.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serving): enumerateAttentionHosts reaches PreLNTransformerBlock attention The shared attention-host enumerator (used by the GQA/paged KV-cache init, attention-optimizability detection, and quantization scans) recursed into TransformerEncoderBlock/TransformerDecoderBlock but not PreLNTransformerBlock, where LLaMA/GGUF decoders host their grouped-query attention. Add that case so InitializeGQAKVCache (and the other scans) find the nested CachedGroupedQueryAttention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serving): make PagedAttentionKernel grouped-query-attention aware Add PagedAttentionConfig.NumQueryHeads (0 => same as NumHeads = MHA). NumHeads is now the KV/cache head count; query heads may outnumber it under GQA, with each KV head shared by NumQueryHeads/NumHeads query heads (kvHead = qHead / group). Threaded through every compute path — ComputeAttention, ComputeTiledPagedAttention (decode), ComputeContiguousCausalPrefill (prefill), ComputeBatchedAttention, and the fused Forward/ForwardQuantized (asymmetric q_proj vs k_proj/v_proj, matching HF). K/V buffers size to the KV heads the cache stores; per-query-head online-softmax scratch. MHA stays the group==1 special case (all 43 PagedAttention tests unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serving): make PagedCachedMultiHeadAttention grouped-query-attention aware Add kvHeadCount ctor param (0 => headCount = MHA). K/V weights are now [embDim, kvHeadCount*headDim] (narrower under GQA); Q/O stay [embDim, embDim]. Threaded kvProjDim through every projection + cache write in the per-token, batched-GEMM, and contiguous-prefill paths; RoPE applies to Q over query heads and K over KV heads; param serialization and int8 quant use the real per-weight widths. Expose KVHeadCount for the optimizer to build the paged config/kernel. Head split/merge and repeat_kv now use vectorized library ops (Reshape/Transpose and Engine.TensorGather on the head axis) instead of scalar nested loops. MHA is the kvHeadCount==headCount special case; all 43 PagedAttention tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serving): wire grouped-query decoders onto the paged KV-cache path InitializePagedKVCache now sizes the PagedKVCache by the KV-head count and passes NumQueryHeads to the kernel so it repeats each KV head across its query-head group. BuildPagedGqaReplacement converts a GroupedQueryAttentionLayer (top-level or nested in PreLNTransformerBlock) to a PagedCachedMultiHeadAttention with the source KV-head count, copying its [Q][K][V][O][outBias] parameters and RoPE/ALiBi config; the GQA branches prefer it when paged KV is enabled. Guarded by GroupedQueryAttentionLayer. UsesProjectionBias: models with Q/K/V projection bias (Qwen2-style) fall back to the contiguous CachedGroupedQueryAttention, which the paged layer cannot represent. 200 paged/optimizer/KVCache/GQA tests green net10.0; both TFMs build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(serving): paged grouped-query attention matches the original decoder Numerical-parity test: a tiny GQA decoder (numHeads=4, numKVHeads=2, RoPE) nested in a PreLNTransformerBlock is optimized with the paged KV cache enabled, and the optimized forward matches the original within 1e-3 — proving the paged-GQA kernel repeat-KV, the layer's narrow K/V projections, the [Q][K][V][O][outBias] weight copy, and the interleaved-RoPE convention are all faithful. Also asserts the GQA is rewritten to PagedCachedMultiHeadAttention and the paged KV cache is live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(serialize): groupedQueryAttentionLayer round-trips RoPE, mask, bias, softcap A cloned GQA layer (serialize -> deserialize) silently lost its RoPE positional encoding, causal mask, custom head dimension, Q/K/V projection bias, and attention logit soft-cap: GetMetadata never persisted them and the deserializer defaulted the bools to false with no ConfigurePositionalEncoding call. So a cloned decoder computed bidirectional, RoPE-less attention and diverged — which is exactly what breaks the incremental-serving clone of a GGUF/LLaMA decoder. Persist all of them in GetMetadata and restore them (incl. RoPE via the layer's ConfigurePositionalEncoding) on deserialize. New test clones a tiny RoPE+causal GQA model and asserts forward parity within 1e-4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(serialize): make PreLNTransformerBlock cloneable so paged serving engages The block hosts a polymorphic (T5/MHA/GQA) attention sublayer, so it had no deserialization constructor and cloning threw — which silently disabled the paged incremental-generation clone (ServableModelWrapper.BuildIncrementalModel) for every GGUF/LLaMA decoder. GetMetadata now persists the block dims + FFN activation + the nested attention as an 'Attn.'-prefixed self-contained sub-blob (type + its metadata + shapes); a new DeserializationHelper branch rebuilds the attention recursively via CreateLayerFromType (E1 keeps its RoPE/mask) and constructs the block. OnFirstForward resolves the lazy norms + FFN in forward order (deser/resolve path only, so no RNG perturbation on a normal forward) so ParameterCount is correct before SetParameters. The paged-GQA parity test now runs with cloneModel: true (the real serving path) and still matches the original within 1e-3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(serving): grouped-query decoder builds the paged incremental path End-to-end proof that paged-GQA engages in serving: a numHeads>numKVHeads PreLN decoder (RoPE) wrapped in ServableModelWrapper now reports SupportsIncrementalGeneration true and generates in-range tokens. Exercises the full chain — GQA-aware paged kernel, narrow-K/V paged attention layer, optimizer GQA->paged rewrite, and PreLNTransformerBlock + GQA serialization (the clone BuildIncrementalModel performs). Before this the clone threw and every LLaMA/GGUF-style decoder silently fell back to the stateless path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(inference): cachedGroupedQueryAttention forward onto recordable engine ops [prototype] Rewrites the inference-only cached GQA layer to apply RoPE via Engine.ApplyRoPEInterleaved and grouped-query attention via Engine.ScaledDotProductAttentionGqa (unexpanded K/V) — a device-agnostic, GPU-graph-recordable forward that drops the two managed unrecordable hotspots (managed RoPE + ExpandKVHeads). RotaryPositionalEncodingLayer exposes its cos/sin caches (GetInterleavedCaches) for the fused op. ALiBi keeps the expand + FlashAttention path. Verified: the optimized (non-paged) model's forward matches the original managed GQA decoder within 1e-3 (InferenceOptimizer_CachedGroupedQueryAttention_RewrittenForward_MatchesOriginal), run against a local AiDotNet.Tensors probe. PROTOTYPE / RELEASE-GATED: the pin is a LOCAL probe (0.117.200-devexec) of the unreleased AiDotNet.Tensors PR #828 (device-agnostic execution). It MUST be bumped to the released AiDotNet.Tensors version before this lands / any AiDotNet PR — CI cannot restore the probe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: bump probe pin to 0.117.201-devexec (GQA-SDPA causal KV-cache offset fix) Local probe of the Tensors branch after the OpenCL GQA-SDPA causal offset fix. Still release-gated — bumps to the released AiDotNet.Tensors before any AiDotNet PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gguf): serve Q8_0 weights quantized via block-Q8_0 GEMM (decode) + profiling harness keeps GGUF Q8_0 linear weights in their native int8 layout instead of expanding to fp32 at load, and runs the block-Q8_0 GEMM (Tensors Q8BlockGemm, ggml q8_0 parity) directly on them for float inference. GgufFile.TryReadQ8_0Raw reads the native blocks (int8 + per-32 fp16 scale); GgufModelSource.TryReadQ8_0 maps the HF name; LlamaModelBuilder.LoadDense installs them on the FFN + lm_head DenseLayers; DenseLayer runs the quantized GEMM on the float inference path. gated to small M (decode/small-batch, where the naive int8 kernel is bandwidth-bound and beats fp32 BLAS); large-M prefill stays on fp32 until the int8 GEMM is register-tiled, so prefill is unchanged (451 tok/s) while decode runs on int8. gguf 7042 llama.cpp parity preserved. also adds the DEVHOST_PROFILE steady-state loop used to profile the forward. NOTE: release-gated on Tensors Q8BlockGemm (probe pin 0.118.10-q8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * perf(gguf): gate Q8_0 quantized forward on VNNI hardware the block-Q8_0 GEMM only beats the tuned fp32 BLAS when the CPU has a 1-instruction VNNI int8 multiply-accumulate (AvxVnni / Avx512 vpdpbusd). on AVX2-only CPUs the 3-instruction maddubs dot is no faster, so engaging it there would regress. gate the DenseLayer quantized path on AvxVnni/Avx512BW (net471 has no intrinsics -> false), so it auto-engages on the VNNI serving hardware and stays on fp32 otherwise. weights are still read + kept as Q8_0 regardless (RAM benefit); only the forward path is gated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * perf(serving): bulk-read paged KV under one lock + decode benchmark harness decode profiling (new DEVHOST_DECODE mode: real incremental paged generation, reports tok/s) showed ComputeTiledPagedAttention read the KV history one locked ReadKey/ReadValue per position -> O(seqLen) contended Monitor.Enter per layer per token. add PagedKVCache.ReadKeyValueRange (one lock, whole range) and read the layer's KV once into a pooled buffer; the attention loop then indexes it lock-free. removes the per-position locking (helps concurrent multi-sequence load where the cache lock is genuinely contended). 14/14 paged-attention + incremental-generation tests green (identical output). single-stream decode ~130ms/token is serial per-token compute-bound (POOL_THREADS 1/2/4 all equal), not lock-bound — the forward-thread lock samples were blocked-wait, not CPU. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * perf(decode): vectorize the per-token projection matvec (scalar -> AVX2) MatVecMul (the q/k/v/o + attention decode projections, run 7x per layer per token on the incremental path) was a pure scalar double loop -- ~150M scalar FMAs/token for a 135M model, the dominant fixed per-token decode cost. vectorize the inner dot with System.Numerics.Vector<float> (8-wide FMA + horizontal sum + scalar tail), net471 keeps the scalar path. decode 6.9 -> 11.3 tok/s (145 -> 88 ms/token, 1.6x) on smollm2-135m. 14/14 paged + incremental tests green (identical output). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

franklinic and others added 7 commits February 7, 2026 17:20

Copilot AI review requested due to automatic review settings February 7, 2026 23:44

vercel Bot deployed to Preview February 7, 2026 23:44 View deployment

Copilot started reviewing on behalf of ooples February 7, 2026 23:45 View session

vercel Bot deployed to Preview February 7, 2026 23:47 View deployment

Copilot AI reviewed Feb 7, 2026

View reviewed changes

Comment thread src/AiModelBuilder.cs Outdated

Comment thread src/Models/Options/FactorVAEOptions.cs Outdated

Comment thread src/Agents/AgentHyperparameterApplicator.cs Outdated

Comment thread src/Agents/AgentHyperparameterApplicator.cs

github-advanced-security AI found potential problems Feb 8, 2026

View reviewed changes

coderabbitai Bot requested changes Feb 8, 2026

View reviewed changes

vercel Bot deployed to Preview February 8, 2026 01:41 View deployment

coderabbitai Bot reviewed Feb 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 8, 2026 01:55

vercel Bot deployed to Preview February 8, 2026 01:56 View deployment

coderabbitai Bot requested changes Feb 8, 2026

View reviewed changes

Copilot AI reviewed Feb 8, 2026

franklinic and others added 3 commits February 7, 2026 21:16

vercel Bot deployed to Preview February 8, 2026 13:42 View deployment

coderabbitai Bot approved these changes Feb 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 8, 2026 23:09

vercel Bot deployed to Preview February 8, 2026 23:10 View deployment

coderabbitai Bot requested changes Feb 8, 2026

View reviewed changes

Comment thread src/Agents/AgentHyperparameterApplicator.cs

Copilot AI reviewed Feb 8, 2026

coderabbitai Bot approved these changes Feb 8, 2026

View reviewed changes

vercel Bot deployed to Preview February 8, 2026 23:40 View deployment

ooples merged commit fb14a3d into master Feb 9, 2026
38 of 39 checks passed

ooples deleted the feature/agent-hyperparameter-auto-apply branch February 9, 2026 04:29

This was referenced Feb 9, 2026

feat: expand meta-learning beyond MAML/Reptile/SEAL with 30+ algorithms #833

Merged

feat: KG embeddings, link prediction, GraphRAG & temporal support #853

Merged

This was referenced Feb 17, 2026

feat: add BEATs golden example model with audio facade wiring #867

Merged

feat: video SR, frame interpolation & optical flow models (issue #268) #874

Merged

This was referenced Feb 27, 2026

feat: add BiLSTM-CRF golden example NER model (issue #898) #899

Merged

fix: prevent ~280 models from crashing on default construction + code quality fixes (issue #915) #917

Merged

coderabbitai Bot mentioned this pull request Jun 11, 2026

feat(agentic): Phase 0 — IChatClient<T> model abstraction (epic #1544) #1545

Merged

ooples mentioned this pull request Jul 21, 2026

feat: device-agnostic execution + GGUF Q8_0 quantized serving + CPU decode/prefill perf #1917

Merged

@@ -1,4 +1,5 @@
             using AiDotNet.Enums;
+            using System.Linq;
             namespace AiDotNet.Agents;
@@ -43,25 +44,20 @@
                     // Check model-specific definitions first
                     if (_registry.TryGetValue(modelType, out var definitions))
                     {
-                        foreach (var def in definitions)
+                        var modelSpecificMatch = definitions
+                            .FirstOrDefault(def => def.NormalizedAliases.Contains(normalized));
+                        if (modelSpecificMatch != null)
                         {
-                            if (def.NormalizedAliases.Contains(normalized))
-                            {
-                                return def.PropertyName;
-                            }
+                            return modelSpecificMatch.PropertyName;
                         }
                     }
                     // Fall back to shared definitions
-                    foreach (var def in _sharedDefinitions)
-                    {
-                        if (def.NormalizedAliases.Contains(normalized))
-                        {
-                            return def.PropertyName;
-                        }
-                    }
+                    var sharedMatch = _sharedDefinitions
+                        .FirstOrDefault(def => def.NormalizedAliases.Contains(normalized));
-                    return null;
+                    return sharedMatch?.PropertyName;
                 }
                 /// <summary>
@@ -73,24 +63,17 @@
                     if (_registry.TryGetValue(modelType, out var definitions))
                     {
-                        foreach (var def in definitions)
-                        {
-                            if (def.NormalizedAliases.Contains(normalized))
-                            {
-                                return def;
-                            }
-                        }
-                    }
+                        var modelSpecificMatch = definitions
+                            .FirstOrDefault(def => def.NormalizedAliases.Contains(normalized));
-                    foreach (var def in _sharedDefinitions)
-                    {
-                        if (def.NormalizedAliases.Contains(normalized))
+                        if (modelSpecificMatch != null)
                         {
-                            return def;
+                            return modelSpecificMatch;
                         }
                     }
-                    return null;
+                    return _sharedDefinitions
+                        .FirstOrDefault(def => def.NormalizedAliases.Contains(normalized));
                 }
                 /// <summary>

@@ -147,7 +147,10 @@
                         result = Convert.ToDouble(value);
                         return true;
                     }
-                    catch
+                    catch (Exception ex) when (ex is FormatException
+                                               || ex is InvalidCastException
+                                               || ex is OverflowException
+                                               || ex is ArgumentNullException)
                     {
                         result = 0;
                         return false;

@@ -1,5 +1,6 @@
             using System.Text.RegularExpressions;
             using Newtonsoft.Json.Linq;
+            using Newtonsoft.Json;
             namespace AiDotNet.Agents;
@@ -101,10 +102,14 @@
                         }
                         return result.Count > 0;
                     }
-                    catch
+                    catch (JsonReaderException)
                     {
                         return false;
                     }
+                    catch (JsonException)
+                    {
+                        return false;
+                    }
                 }
                 /// <summary>

@@ -114,8 +114,17 @@
                         property.SetValue(options, convertedValue);
                         result.Applied[paramName] = paramValue;
                     }
-                    catch (Exception ex)
+                    catch (TargetInvocationException ex)
                     {
+                        var message = ex.InnerException?.Message ?? ex.Message;
+                        result.Failed[paramName] = $"Failed to set property: {message}";
+                    }
+                    catch (Exception ex) when (ex is ArgumentException
+                                               || ex is TargetException
+                                               || ex is TargetParameterCountException
+                                               || ex is MethodAccessException
+                                               || ex is InvalidOperationException)
+                    {
                         result.Failed[paramName] = $"Failed to set property: {ex.Message}";
                     }
                 }

@@ -201,9 +201,17 @@
                         // Try using Convert.ChangeType as a last resort
                         return Convert.ChangeType(value, targetType);
                     }
-                    catch
+                    catch (InvalidCastException)
                     {
                         return null;
                     }
+                    catch (FormatException)
+                    {
+                        return null;
+                    }
+                    catch (OverflowException)
+                    {
+                        return null;
+                    }
                 }
             }

Uh oh!

Conversation

ooples commented Feb 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Core Infrastructure

Options Hierarchy Standardization

Model Wiring

Pipeline Integration

Bug Fix

Test plan

Summary by CodeRabbit

Uh oh!

vercel Bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Blocking issues / attention items

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Feb 8, 2026

Quality Gate failed

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ooples commented Feb 7, 2026 •

edited by coderabbitai Bot

Loading

vercel Bot commented Feb 7, 2026 •

edited

Loading

coderabbitai Bot commented Feb 7, 2026 •

edited

Loading