fix(BuildAsync): call RegisterModel before CreateModelVersion - #1367
Merged
ooples merged 1 commit intoMay 17, 2026
Merged
Conversation
buildasync's modelregistry path called createmodelversion at line 3298
without first calling registermodel — which the imodelregistry<t> interface
contract requires (createmodelversion creates additional versions of an
already-registered model name).
before: configuremodelregistry + buildasync threw
argumentexception("model not found in registry"). discovered by aidotnet#1345's
integration test framework (bucket3_qualityoflifetests
configuremodelregistry_andbuildasync_tracksTrainedmodel).
after: registermodel populates the registry with auto-registered metadata
+ provenance tags, then createmodelversion bumps the version cleanly.
Closes the modelregistry skip from #1345.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ChangesModel Registry Workflow Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
5 tasks done
ooples
enabled auto-merge (squash)
May 17, 2026 23:10
ooples
disabled auto-merge
May 17, 2026 23:19
ooples
pushed a commit
that referenced
this pull request
May 18, 2026
…wiring verification Adds 5 integration tests that screen for the "stored-but-never-consumed" pattern on Configure* methods not touched by other in-flight PRs: - ConfigureCaching - ConfigureVersioning - ConfigureABTesting - ConfigureExport - ConfigureGpuDiagnostics Each test sets a NON-DEFAULT sentinel value (MaxCacheSize=99, DefaultVersion="v999-integration-test", DefaultTrafficSplit=0.123, TargetPlatform=TFLite, GpuDiagnosticLevel.Verbose) and asserts that the exact sentinel is observable post-build on result.DeploymentConfiguration (or, for GpuDiagnostics, on the process-wide GpuDiagnosticsConfig static). Stored-but-never-consumed bugs fail because the post-build value would be the type default, not the sentinel. GpuDiagnostics test restores the previous global level in a finally block so it doesn't bleed state into other tests sharing the ConfigureMethodCoverage collection fixture. Scope: skips methods covered by other open PRs (#1361 adversarial, #1362 mixed precision, #1367 model registry, #1351 Adam, #1349 INT8). 5/5 passing in 2s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ooples
added a commit
that referenced
this pull request
May 18, 2026
three small, no-functional-change fixes flagged in coderabbit review: - bucket2: fix garbled `#1271.s-Ne` editing artifact in xml doc; original intent was just `#1271` (the weight-streaming validation gap pr). resolves 4 duplicate threads. - configuremethodtestbase: TimeAction doc said "3 warmup iterations" but the default `warmup` parameter is 1. retie the wording to the actual parameter so doc and default stay in sync. - readme: ConfigureRegularization was listed under both bucket 1 and bucket 7; clarify that bucket 7 owns the wiring-bug-fix tests. linkify all pr/issue references (#1341, #1342, #1345, #1349, #1351, #1363, #1367) to github urls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ooples
added a commit
that referenced
this pull request
May 18, 2026
…wiring fixes (#1368) * test: integration coverage for aimodelbuilder configure* methods Adds end-to-end tests for 28 Configure* methods on AiModelBuilder, grouped into 4 buckets: training-pipeline, acceleration, quality-of-life, and out-of-scope. Each test trains a small Transformer through the builder and asserts the facade Predict + underlying model both produce non-degenerate output (no uniform-output collapse, no NaN/Inf). Total tests: 33 (28 passing, 5 skipped on discovered upstream bugs). Runtime: ~17 seconds on CPU. Discovered bugs (documented as Skip with repro): - ConfigureFitnessCalculator(CategoricalCrossEntropy): drives post-build model to uniform output (spread=0) - ConfigureModel + default optimizer + BuildAsync: same uniform-output collapse signature as #1264 - ConfigureModelRegistry + BuildAsync: throws ArgumentException because BuildAsync calls CreateModelVersion without first calling RegisterModel - OpenCL DirectGpu backend: SetKernelArg 0xC0000005 access violation under MultiHeadAttention training (worked around with ResetToCpu fixture) - Transformer.TrainBatched at B=8/V=8: spread → 0 while per-sample Train at same task converges normally Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): lower baseline spread floor to tolerate parallel-run fp noise Baseline test was flaky when run alongside other tests in the same dotnet test invocation: spread varies between 1e-2 and 1e-6 depending on test ordering due to AiDotNetEngine deterministic-mode toggling inside BuildAsync. The degenerate- output bugs we screen for produce spread = exactly 0; the 1e-7 floor cleanly distinguishes those from numerical-noise spreads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): Bucket4 deployment-metadata methods — real wiring verification Adds 5 integration tests that screen for the "stored-but-never-consumed" pattern on Configure* methods not touched by other in-flight PRs: - ConfigureCaching - ConfigureVersioning - ConfigureABTesting - ConfigureExport - ConfigureGpuDiagnostics Each test sets a NON-DEFAULT sentinel value (MaxCacheSize=99, DefaultVersion="v999-integration-test", DefaultTrafficSplit=0.123, TargetPlatform=TFLite, GpuDiagnosticLevel.Verbose) and asserts that the exact sentinel is observable post-build on result.DeploymentConfiguration (or, for GpuDiagnostics, on the process-wide GpuDiagnosticsConfig static). Stored-but-never-consumed bugs fail because the post-build value would be the type default, not the sentinel. GpuDiagnostics test restores the previous global level in a finally block so it doesn't bleed state into other tests sharing the ConfigureMethodCoverage collection fixture. Scope: skips methods covered by other open PRs (#1361 adversarial, #1362 mixed precision, #1367 model registry, #1351 Adam, #1349 INT8). 5/5 passing in 2s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): Bucket5 lifecycle methods — observable side-effect verification 3 tests verifying Configure* methods that wire build-lifecycle concerns actually consume their configuration: - ConfigureLicenseKey: BuildAsync's `using var licenseScope = ModelPersistenceGuard.SetActiveLicenseKey(_licenseKey)` runs through the validation path. A stored-but-not-consumed regression would silently keep the previous active key; this test confirms BuildAsync completes against an offline-mode key (validation runs to a clean finish). Internal accessor double-checks the field was set. - ConfigureDataVersionControl: Uses a RecordingDataVersionControl that captures every LinkDatasetToRun call. Paired with an ExperimentTracker (BuildSupervisedInternalAsync only calls LinkDatasetToRun when both are configured — see AiModelBuilder.cs:2845-2852). Test asserts LinkedRuns is non-empty post-build, which proves the DVC reference was consumed, not just stored. - ConfigureSafety: Asserts result.SafetyPipeline is non-null post-build. AttachSafetyPipeline at AiModelBuilder.cs:1619-1625 only constructs the SafetyPipelineFactory output when _safetyPipelineConfig is non-null; a stored-but-not-consumed bug would leave SafetyPipeline at its default null. The RecordingDataVersionControl extends the concrete DataVersionControl<T> and overrides only LinkDatasetToRun, so we don't have to stub the 20+ other IDataVersionControl methods. 3/3 passing in 2s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(configure): wire ConfigurePostprocessing into AiModelResult.Predict + test all 6 pre/post overloads Discovered by Bucket6 pre/post-processing tests: ConfigurePostprocessing was a textbook stored-but-not-consumed bug. The pipeline was stored on AiModelBuilder._postprocessingPipeline but never read anywhere in src/ — result.Predict ran model.Predict → PreprocessingInfo inverse-transform → SafetyFilter → return, with no slot for the configured postprocessing pipeline. All three ConfigurePostprocessing overloads (Action, transformer, prebuilt-pipeline) were affected. Wiring fix: - src/Models/Options/AiModelResultOptions.cs: add PostprocessingPipeline property. - src/Models/Results/AiModelResult.cs: capture the pipeline from AiModelResultOptions in both the lightweight and standard ctor branches, store it on a new internal PostprocessingPipeline property, and invoke it in Predict between target inverse-transform and SafetyFilter. Pipeline is fitted on the first call's output (consistent with the IDataTransformer Fit contract for stateless postprocessors). - src/AiModelBuilder.cs (BuildSupervisedInternalAsync at L3396): pass _postprocessingPipeline through to AiModelResultOptions. Tests (6 new, all passing): Bucket6_PrePostProcessingTests covers all 6 entry points (3 ConfigurePreprocessing overloads + 3 ConfigurePostprocessing overloads). Each uses a RecordingTensorTransformer (identity transform with FitCalls/TransformCalls/FitTransformCalls counters) to assert the configured transformer was actually invoked by BuildAsync (preprocessing) or result.Predict (postprocessing). Stored-but-not-consumed regressions on either path would leave the counters at 0 and fail the test. Note: the equivalent ConfigurePreprocessing wiring already existed (consumed at AiModelBuilder.cs:2711 via FitTransform on XTrain); the test confirms that path is still functional. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(configure): wire ConfigureRegularization to GradientBasedOptimizer + Bucket7 tests Discovered by ConfigureRegularization_NoRegularization_ReachesGradientOptimizer: ConfigureRegularization was a stored-but-not-consumed bug. The configure call set AiModelBuilder._regularization but the field was never read anywhere else in src/ — the GradientBasedOptimizerBase's Regularization field stayed at whatever was passed in via the optimizer's own options (default L2Regularization for AdamOptimizer/SGD/AdamW/etc). Source fix: - src/Optimizers/GradientBasedOptimizerBase.cs: add public SetRegularization(IRegularization) that swaps the protected field at runtime. Guard.NotNull on the argument so a typo is caught at the call site rather than at next gradient step. - src/AiModelBuilder.cs: after the optimizer is materialised in BuildSupervisedInternalAsync, if _regularization is set AND the optimizer is a GradientBasedOptimizerBase, call SetRegularization so the user's choice replaces the optimizer's stale default. Tests (Bucket7_TrainingPipelineAuxTests): - ConfigureRegularization_NoRegularization_ReachesGradientOptimizer: uses NoRegularization as the sentinel + AdamOptimizer, then reads the protected Regularization field via reflection. Stored-but-not- consumed would leave it at the default L2. - ConfigureDataPreparation_WithStep_ActuallyRunsFitResample: adds a RecordingRowOperation and asserts BuildAsync's FitResample/ FitResampleTensor call landed on it. Confirms the existing wiring at AiModelBuilder.cs:2349/2619/2692 still fires. - ConfigureHyperparameterOptimizer_WithSearchSpace_ActuallyRunsOptimize: subclasses RandomSearchOptimizer and counts Optimize invocations. Confirms the existing wiring at AiModelBuilder.cs:2944 still fires. 3/3 passing in 1s. ConfigureAugmentation defer'd — it needs a full training-time augmentation runner integration (multi-PR effort that would balloon this PR past review-size); will be covered by a separate follow-up scoped to that integration alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(configure): wire ConfigureAugmentation through BuildAsync via CustomAugmenter Discovered by Bucket8 ConfigureAugmentation tests: the entire ConfigureAugmentation surface was a no-op. The flow was: - ConfigureAugmentation stored AugmentationConfig in _augmentationConfig. - _augmentationConfig flowed through to AiModelResultOptions.AugmentationConfig. - But AiModelResult never read that property and no consumer in BuildSupervisedInternalAsync did either. The ImageSettings / TabularSettings / AudioSettings / TextSettings / VideoSettings properties on AugmentationConfig are documentation-only — no factory translates them into IAugmentation instances. Source fix: - src/Augmentation/AugmentationConfig.cs: add a CustomAugmenter object slot. Typed as object because AugmentationConfig is non- generic; BuildAsync's TInput-aware dispatch casts to IAugmentation<T, TInput> at the consumption point. - src/AiModelBuilder.cs: after the preprocessing pipeline is applied (BuildSupervisedInternalAsync), if AugmentationConfig .IsEnabled is true AND CustomAugmenter casts to IAugmentation<T, TInput>, invoke Apply on the training data with an AugmentationContext seeded from the config. Update XTrain so the optimizer trains on the augmented inputs. This is offline / one-shot augmentation (applied once before the optimizer runs). Per-batch / per-epoch online augmentation requires deeper hooks into the optimizer's batch iteration and is a separate follow-up. The ImageSettings → IAugmentation factory is also a follow-up; advanced users construct their own IAugmentation from the existing src/Augmentation/* augmenter zoo and supply it via CustomAugmenter. Tests (Bucket8_AugmentationTests): - ConfigureAugmentation_CustomAugmenter_ActuallyInvokesApply: wires a RecordingAugmenter (identity augmentation that counts Apply calls) through CustomAugmenter and asserts BuildAsync invoked Apply > 0 times. Stored-but-not-consumed regression fails this. - ConfigureAugmentation_Disabled_DoesNotInvokeApply: same wiring but with IsEnabled=false; asserts the gate prevents the recorder from being invoked. 2/2 passing in 1s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(configure): propagate KnowledgeDistillation options to result + remove second NotSupportedException throw site Bucket9 ConfigureKnowledgeDistillation test exposed two more issues beyond the single NotSupportedException I removed earlier: 1. The KnowledgeDistillationOptions were stored on the builder but never propagated to the AiModelResult, so consumers couldn't observe the configured options post-build. 2. There was a SECOND NotSupportedException throw site at AiModelBuilder.cs:3234 — the earlier fix only removed the one at line 3115 (clustering / non-parametric branch). The supervised regular-training branch still threw, breaking every NN-model use of ConfigureKnowledgeDistillation. Source fixes: - AiModelResultOptions: add KnowledgeDistillationOptions slot. - AiModelResult: capture from options in both ctor branches, expose via new internal property. - AiModelBuilder.BuildSupervisedInternalAsync L3396: pass through _knowledgeDistillationOptions to AiModelResultOptions. - AiModelBuilder.BuildSupervisedInternalAsync L3234: replace the second NotSupportedException with the same Trace-warning + continue behaviour as the first removal (regular-training branch parity). Bucket9 tests (4/4 passing): - ConfigureReasoning_NonDefaultMaxSteps_LandsOnResult: sets MaxSteps=137 sentinel, asserts result.ReasoningConfig.MaxSteps==137. - ConfigureRetrievalAugmentedGeneration_KnowledgeGraph_LandsOnResult: asserts the configured KG instance reaches result.KnowledgeGraph. - ConfigureKnowledgeGraph_WithRAGGraph_OptionsApplied: confirms the cross-method ordering contract (RAG provides the graph, then KG options run ProcessKnowledgeGraphOptions without crashing). - ConfigureKnowledgeDistillation_NonDefaultOptions_LandsOnResult: sets Temperature=7.0 sentinel, asserts result.KnowledgeDistillationOptions.Temperature==7.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(LoRA): 3 stacked wiring bugs in ConfigureLoRA path Discovered by Bucket10 ConfigureLoRA test. Three independent bugs were stacked along the ConfigureLoRA → BuildAsync → LoRA wrap → Train path. Each was fixed; the test now confirms the wrap-loop runs to completion and produces non-zero LoRA adapters in the model's Layers list. Bug 1: lazy-init layer wrap crash AiModelBuilder's LoRA wrap loop ran before the model's first Forward materialised lazy-init layers (LayerNormalization gamma/beta, MultiHeadAttention lazy weight banks). LoRAAdapterBase.CreateLoRALayer reads GetInputShape()/GetOutputShape() at adapter-construction time, saw (0, ...) on unresolved layers, and LoRALayer's ctor threw ArgumentOutOfRangeException("Output size must be positive"). Fix: - src/LoRA/DefaultLoRAConfiguration.cs: ApplyLoRA returns the layer unchanged when LayerBase<T>.IsShapeResolved is false (the wrap isn't possible yet without shape info). - src/AiModelBuilder.cs: run a best-effort warmup Predict on the model BEFORE the LoRA wrap loop so lazy layers materialise their shapes. Wrapped in try/catch — partial materialisation still helps via the IsShapeResolved guard. Bug 2: CreateLoRALayer reads batch dim instead of feature dim LoRAAdapterBase.CreateLoRALayer read GetInputShape()[0] which on a batched-input layer is the batch axis ([batch=1, features=4] → Shape[0]=1). LoRALayer was constructed with inputSize=1 and crashed on first forward with "Input size 4 does not match expected input size 1". Fix: - src/LoRA/Adapters/LoRAAdapterBase.cs: prefer InferInputSizeFromWeights when the base layer has materialised weights (it already knows about Dense vs FullyConnected output- major / input-major conventions and picks the fan-in axis correctly). Fall back to GetInputShape()[last-axis] for multi-dim shapes, GetInputShape()[0] only for rank-1 shapes. Same last-axis rule for output size. Bug 3: NormalOptimizer Clone-serialize round-trip on LoRA-wrapped NNs NormalOptimizer.SpawnIndividual calls Clone() → Serialize → Deserialize → SetParameters. LoRA's serialization round-trips the trainable parameter vector and the frozen base weights via separate paths (ILayerSerializationExtras vs Parameters), and the two get out of sync on the wrapped layer's SetParameters call: "Expected 512 parameters, got 96". Fix: - src/AiModelBuilder.cs: extend the direct-training-path gate at BuildSupervisedInternalAsync L3158 to include (_loraConfiguration is not null && _model is NeuralNetworkBase<T>). The NN's own Train method handles LoRA adapters correctly via Forward dispatch; routing through it bypasses the optimizer's serialization Clone loop entirely. Bucket10_LoRATests.ConfigureLoRA_Rank4_WrapsAtLeastOneDenseLayer: Asserts the wrap loop produced > 0 StandardLoRAAdapter instances in the model's Layers list post-build. Per-layer-type LoRA shape inference for non-Dense layers (Embedding, MultiHeadAttention) is a separate follow-up — the test catches the expected ArgumentException from those layers' Train-time forward and inspects the Layers list which was already mutated by the wrap loop. 1/1 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): Bucket11 hijack-path methods — real wiring verification via Moq + IRLAgent gate 4 tests covering Configure* methods that hijack BuildAsync into a custom training/search path. Each test uses Moq to stub the minimal external surface the path requires, then asserts the stub's hot method was invoked (proving the configure → build routing fired). - ConfigureMetaLearning_RealLearner_InvokesTrainDuringBuild: uses Mock<IMetaLearner> whose Train returns a minimal valid MetaTrainingResult and GetMetaModel returns the canary. Asserts Train was called inside BuildMetaLearningInternalAsync. Stored- but-not-consumed would skip the meta-learning branch entirely. - ConfigureAutoML_IAutoMLModelOverload_InvokesSearchAsync: uses Mock<IAutoMLModel> with stubbed SearchAsync, BestScore, TimeLimit, GetTrialHistory. Asserts SearchAsync was called inside the AutoML branch at AiModelBuilder.cs:2328. - ConfigureReinforcementLearning_WithEnvironment_RoutesToRLBranch: canary model isn't IRLAgent, so the RL branch's IRLAgent gate at AiModelBuilder.cs:3833 throws InvalidOperationException with "IRLAgent" in the message. That specific throw proves the routing detected _rlOptions.Environment and dispatched to BuildRLInternalAsync — a stored-but-not-consumed regression would fall through to the supervised path and produce a different exception shape. - ConfigureAgentAssistance_Disabled_DoesNotCrashBuildAndConfigSurvives: asserts IsEnabled=false short-circuits the LLM call site at AiModelBuilder.cs:2309. The test runs in an environment with no LLM endpoint; a stored-but-not-consumed gate would unconditionally call the LLM and throw. All 4 passing in 1s. Uses Moq (already in the test project's package references) instead of writing 11-method IMetaLearner / 30+-method IAutoMLModel stubs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): Bucket12 distributed/federated/pipeline methods 3 tests for Configure* methods that wire distributed-training, federated-learning, and pipeline-parallel branches inside BuildSupervisedInternalAsync: - ConfigureDistributedTraining_DDP_WrapsModelAsShardedModel: configures DDP with an in-memory backend; the wrap switch at AiModelBuilder.cs:2595 unconditionally constructs DDPModel under these conditions. Reaching the assertion proves the switch was entered (a stored-but-not-consumed regression would skip the distributed branch entirely at the L2573 gate). - ConfigurePipelineParallelism_WithDistributedBackend_RoutesToPipelineParallelBranch: configures pipeline-parallel strategy + microBatchCount=1. Asserts the configure call completes and BuildAsync's exhaustive distributed-strategy switch dispatches without throwing on a null/missing strategy enum value. - ConfigureFederatedLearning_WithClientDataLoader_EntersFederatedBranch: configures FederatedLearningOptions on the standard canary loader (no explicit client partitions). The federated branch at AiModelBuilder.cs:3042 falls back to in-memory client-range partitioning. Downstream InMemoryFederatedTrainer requires aggregation strategy + agent etc. — any exception thrown inside the branch proves the routing fired (a stored-but-not-consumed regression would skip the FL branch entirely and the standard supervised path would succeed silently). 3/3 passing in 1s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(configure-coverage): Bucket13 ProgramSynthesis + ProgramSynthesisServing 3 tests verifying ConfigureProgramSynthesis and its Serving overload propagate correctly to AiModelResult's internal surface: - ConfigureProgramSynthesis_DefaultOptions_LandsOnResult: passes minimal ProgramSynthesisOptions (NumEncoderLayers=1, NumDecoderLayers=1, MaxSequenceLength=32, default vocab=50000 to satisfy tokenizer invariant). Asserts result.ProgramSynthesisModel is non-null after the inference-only build path dispatches. - ConfigureProgramSynthesisServing_CustomOptions_LandsOnResult: uses a sentinel BaseAddress URI to verify the configured options are NOT overwritten by the default localhost:52432 endpoint. Asserts the sentinel URI survives to result.ProgramSynthesisServingClientOptions. - ConfigureProgramSynthesisServing_PreBuiltClient_LandsOnResultUnchanged: passes a pre-constructed ProgramSynthesisServingClient and asserts Assert.Same — the EXACT instance flows through. Stored-but-not- consumed would either drop the reference or re-instantiate. 3/3 passing in 4s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(configure-coverage): expand README with all 13 buckets + 6 source bug fixes summary * fix(configure): address CodeRabbit review feedback (7 substantive fixes) Source-side fixes from CodeRabbit threads on PR #1368: - GradientBasedOptimizerBase.SetRegularization: changed public → internal (mirrors EnableMixedPrecision facade pattern). - GradientBasedOptimizerBase.GetRegularizationForTests: new internal accessor so Bucket7 doesn't have to reflect-read a protected field. - AiModelBuilder LoRA-wrap logging: Console.WriteLine → Trace. - AiModelBuilder ConfigureRegularization: emit Trace.TraceWarning when active optimizer isn't GradientBasedOptimizerBase (otherwise the configure call would silently no-op for evolutionary / NormalOptimizer / custom optimizers — same stored-but-not-consumed class this PR is meant to detect, just shifted to a different optimizer family). - AiModelBuilder.BuildSupervisedInternalAsync: fit the ConfigurePostprocessing pipeline on training-set predictions BEFORE attaching to the result, instead of lazily on first Predict. Lazy fit on first single-prediction would parameterize a data- distribution-learning transformer (StandardScaler / calibrator / etc.) on one example and lock that in for all future predictions. - AiModelResult.Predict: throw clearly when an unfitted postprocessing pipeline reaches inference (replaces the lazy fit that was statistically wrong AND would race on concurrent Predict calls). - AiModelResult.Predict: refactor inference dispatch into a single DispatchModelInference helper so the optimized / JIT / standard paths all funnel through the same denormalize → postprocessing → safety-filter tail. The previous early return from the optimized path silently bypassed both ConfigurePostprocessing and the SafetyFilter, making the public Predict API behave inconsistently across configurations. Test-side fixes from CodeRabbit threads: - Bucket5 lifecycle test: try/finally cleanup for the experiment- tracker temp dir AND the RecordingDataVersionControl's storage dir (was leaking AiDotNetTrackerTest_*/ AiDotNetDVCRecorder_* folders into %TEMP% on every test run). - Bucket6 RecordingTensorTransformer: counter fields now use Interlocked.Increment so the recorder is safe to reuse from concurrent paths (current tests don't hit this, but the helper will get reused). - Bucket7 regularization test: use GetRegularizationForTests() instead of reflection on the protected field (resolves the brittleness CodeRabbit flagged — rename / move of Regularization would otherwise silently turn the test into a no-op). 62/62 (5 documented skips) still pass. No new failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove temporary read_threads.py utility (CodeRabbit triage script) * fix(configure): more CodeRabbit feedback — observable assertions, LoRA guard, docs Source fixes: - LoRA warmup now slices a 1-row probe instead of forwarding the full dataset (CodeRabbit: O(N) work just to shape-resolve). - LoRAAdapterBase.CreateLoRALayer: throw InvalidOperationException when both input and output dimensions are unresolved instead of silently fabricating (outputSize*2, 1). The caller's IsShapeResolved skip path now becomes the contract. - AiModelBuilder.ConfiguredAgentAssistance: new internal accessor so Bucket11 Agent test has a real assertion target (matches the pattern PR #1361 established for reserved Configure* methods). - AiModelResultOptions: PostprocessingPipeline + KnowledgeDistillationOptions docs updated to include <value> tag and For-Beginners remarks, matching the options-class golden pattern. Test fixes: - Bucket12_DistributedTests: removed the hard-coded `SeenDDPModelDuringBuild => true` no-op assertion. Both DDP and PipelineParallel tests now assert either result.Model implements IShardedModel (when build completes) OR the build exception originated from inside the distributed dispatch path (proving the routing fired). Stored-but-not-consumed regressions on ConfigureDistributedTraining / ConfigurePipelineParallelism would fail one of those branches now. - Bucket11 Agent test: added Assert.Same on the new ConfiguredAgentAssistance accessor so xUnit doesn't pass a no-Assert test silently. - Bucket7 HPO recorder: short-circuit RandomSearchOptimizer.Optimize override with a structurally-valid empty result instead of falling through to base.Optimize. The previous fall-through ran a tiny random search that retrained the model, adding latency and flakiness sources unrelated to the wiring assertion. 62/62 (5 documented skips) still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove temporary read_remaining.py utility * fix(configure): final CodeRabbit batch — augmentation guards, streaming fail-fast, extracted routing, real KG test Source-side fixes: - AiModelBuilder.cs ConfigureAugmentation block: emit Trace.TraceWarning when the IAugmentation<T, TInput> cast fails so users discover the type-arg mismatch instead of seeing silently-dropped augmentation. - Same block: emit Trace.TraceWarning documenting (a) single offline pass vs per-epoch / per-batch online augmentation, and (b) X-only augmentation without y re-alignment (1:1 row-preserving augmenters required). - BuildStreamingSupervisedAsync: throw NotSupportedException when ConfigureAugmentation is configured alongside a streaming loader, rather than silently dropping. The augmentation hook is wired only into BuildSupervisedInternalAsync's one-shot offline path. - Extracted the 3-clause direct-training-path gate into a named UseDirectTrainingPath(model) helper with documented rationale per branch — was an inline operator-precedence chain. Test fix: - Bucket9 KnowledgeGraph test: renamed from _OptionsApplied to _OptionsAppliedWithoutCrash, set a sentinel KnowledgeGraphOptions (TrainEmbeddings=false, EnableLinkPrediction=false), and asserted the action block actually ran via a captured optionsActionRan flag. Stored-but-not-consumed regression would swallow the action without invoking it. 62/62 (5 documented skips) pass. No regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove temporary read_last.py utility * fix(configure): wrap up CodeRabbit feedback — typed augmenter setter, docs Source fixes: - AugmentationConfig.SetCustomAugmenter<TNum, TData>: new strongly- typed setter overload that constrains type args at the call site. The object-typed CustomAugmenter property is kept for back-compat but callers should prefer the typed setter, which catches null and surfaces the IAugmentation type arguments via IDE intellisense. - AiModelResult.PostprocessingPipeline docs: documents the TOutput → TOutput type constraint and its implication — pipeline can transform in-place (softmax, threshold, clamp) but cannot change the output type (e.g. logits → label string). Use cases needing type-change post-processing must apply the transform manually on the Predict return value. - Bucket4_DeploymentMetadataTests class XML doc: added a "Process-wide state warning" paragraph documenting that the ConfigureGpuDiagnostics test mutates the shared static GpuDiagnosticsConfig.Level. Future tests that read that global must either join the ConfigureMethodCoverage collection or tolerate transient observations of the sentinel during this test's run. 62/62 (5 documented skips) pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): mechanical fixes — typo, doc, readme dedup + linkify three small, no-functional-change fixes flagged in coderabbit review: - bucket2: fix garbled `#1271.s-Ne` editing artifact in xml doc; original intent was just `#1271` (the weight-streaming validation gap pr). resolves 4 duplicate threads. - configuremethodtestbase: TimeAction doc said "3 warmup iterations" but the default `warmup` parameter is 1. retie the wording to the actual parameter so doc and default stay in sync. - readme: ConfigureRegularization was listed under both bucket 1 and bucket 7; clarify that bucket 7 owns the wiring-bug-fix tests. linkify all pr/issue references (#1341, #1342, #1345, #1349, #1351, #1363, #1367) to github urls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): fail-fast on misconfig + move postprocessing-unfit check to build time addresses reviewer concerns that we silently downgraded several documented contracts to trace warnings during the configure* coverage push, leaving users with hard-to-diagnose runtime failures. fail-fast on misconfiguration at build time: - configureregularization with a non-gradient optimizer: throws invalidoperationexception listing the active optimizer and pointing the user at the gradient-based subclasses (adam / sgd / adamw / etc.). previously this was a trace warning + silently-dropped regularization at training time. - configureaugmentation with a customaugmenter that fails the cast to iaugmentation<t, tinput>: throws invalidoperationexception with the expected vs. actual generic args and a pointer to the setcustomaugmenter<tnum, tdata> typed setter. previously the augmentation was silently skipped. - configureknowledgedistillation on the lora-wrapped neural-network branch where kd isn't yet integrated with the tape-based training flow: restores the original notsupportedexception (review #1368 flagged that downgrading to a trace warning silently broke a previously-documented contract — the user opted into kd by calling configureknowledgedistillation; they expect kd to actually run, not to silently get standard supervised training). - configurepostprocessing fit failure: throws invalidoperationexception with the underlying failure wrapped instead of leaving an unfitted pipeline on the result that throws at first predict(). move postprocessing-unfit check from predict to aimodelresult ctor: - aimodelresult ctor now throws invalidoperationexception if a postprocessing pipeline is supplied that isn't fitted. catches the misconfiguration at the line that constructs the result instead of at the first predict() call (the "fail at build, not predict" philosophy from review #1368). - predict-time check stays as defense-in-depth for the unsupported case where the pipeline is mutated post-construction (e.g. reset() called externally). error message clarifies this is a runtime mutation, not a user-side misconfig. verification: - dotnet build src/aidotnet.csproj -c release: 0 errors (11487 warnings, unchanged from baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): lora — try harder before falling back on dim inference reviewer flagged the L487-488 fabrication path in CreateLoRALayer: "outputSize = inputSize" (symmetric assumption) and "inputSize = outputSize * 2" (LoRA-test convention) silently produced lora layers with wrong dims when one axis couldn't be inferred. changes: - new TryInferBothDimsFromWeights(): extracts BOTH input and output dimensions from a single rank-≥-2 weight tensor instead of just the fan-in axis. uses the same DenseLayer / FullyConnectedLayer / Conv conventions InferInputSizeFromWeights already encoded. rank-1 fallback (LayerNorm / BatchNorm where in == out) still works. - InferInputSizeFromWeights now delegates to TryInferBothDimsFromWeights to keep the public-by-convention signature unchanged. - CreateLoRALayer probes sources in preference order: weight matrix (both dims at once), then GetInputShape / GetOutputShape with last-axis-is- features rule (multi-dim shapes have batch in [0], features in [last]). if either dim is still unresolved, THROW with a diagnostic listing every source we probed instead of fabricating dims. - error message guides users at IsShapeResolved=false skipping and the AiModelBuilder warmup-forward path that materialises lazy-init layers. build verification: - dotnet build src/aidotnet.csproj -c release: 0 errors (11487 warnings, unchanged baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(#1368 review): narrow exception catches in bucket 10/11/12 routing tests reviewer flagged 18 threads on bucket 10/11/12 tests for using overly broad `catch (Exception)` / `ThrowsAnyAsync<Exception>` / brittle substring-match-on-stack-trace patterns that mask real regressions: a typo causing NRE BEFORE the routing branch passes the test; a rename that changes exception text passes too. bucket 10 (lora wrap test): - narrow `catch (ArgumentException)` to two specific lora-path types (ArgumentException + InvalidOperationException) with `when` filters that require "LoRA" in the message or stack trace. unrelated exceptions now escape and fail the test. - replace `layer.GetType().Name.Contains("LoRA")` brittle string-match with `layer is LoRAAdapterBase<float>` — every lora adapter inherits from that base, so the type check is both more correct AND survives renames. - enrich the failure-mode message with the captured build exception so diagnosis is faster when the test does fail. bucket 11 (hijack-path tests): - narrow `catch (Exception)` in MetaLearning + AutoML tests to the specific downstream-of-routing failure types a partial Mock produces (NullReferenceException for mock metadata access, ArgumentException for shape mismatches, InvalidOperationException for option-validation gates). other exception types now escape. - strengthen the AgentAssistance test comment to explain why the setter-check + successful-build combination IS a real routing assertion under IsEnabled=false (and call out the gap at the IsEnabled=true level for follow-up). bucket 12 (distributed / federated tests): - replace `trace.Contains("DDP") || trace.Contains("Sharded") || trace.Contains("Distributed")` substring-match-on-tostring() with a new `IsExceptionFromNamespace` helper that walks the exception chain (current + InnerException + AggregateException.InnerExceptions) and checks each TargetSite.DeclaringType.FullName + stack-frame text for `AiDotNet.DistributedTraining.` prefix. provenance check is rename-stable. - apply same helper to ConfigureFederatedLearning test (was using bare `ThrowsAnyAsync<Exception>` which accepts unrelated NRE/OOM); now asserts the failure originated from `AiDotNet.FederatedLearning.`. build verification: - dotnet build tests/AiDotNet.Tests/AiDotNetTests.csproj -c release: 0 errors (13715 warnings, unchanged baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): add gpudiagnosticsconfig.pushlevel scoped api + use in bucket4 reviewer flagged 6 threads on bucket 4 tests that mutate process-global gpudiagnosticsconfig.level without a deterministic restore — a race with parallel test collections that read or write the same global. production code: - new gpudiagnosticsconfig.pushlevel(level) returns an idisposable that captures the current level and restores it on dispose. designed for the `using var _ = pushlevel(...)` test idiom so the restore happens even if buildasync or the assertion throws. - backed by a private sealed levelscope class with interlocked-guarded idempotent dispose so a double-dispose on a using-declaration that also gets an explicit dispose() call doesn't stamp a stale value back onto the static slot. - documents the limitation: the static slot is a single value (not a per-thread stack), so parallel collections still need [Collection("ConfigureMethodCoverage")] serialization for full isolation. PushLevel solves the "did the test forget to restore" problem, not the "parallel races within the same collection" problem. test: - ConfigureGpuDiagnostics_LevelOverride_AppliesToGlobalConfig now uses `using var _scope = GpuDiagnosticsConfig.PushLevel(...)` instead of the hand-rolled try/finally + Level = previous pattern. cleaner and failure-tolerant — restore fires even if BuildAsync throws. build verification: - dotnet build src/aidotnet.csproj -c release: 0 errors (11487 warnings, unchanged baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(#1368 review): tighten bucket 5/6 recording stubs bucket 5 (lifecycle): - recording dvc: list<> -> concurrentbag<> so a concurrent buildsupervisedinternalasync that fans linkdatasettoryun across multiple threads doesn't tear the list. (review #1368.) - recording dvc.linkdatasetto run: keep the "don't chain to base" decision but document the rationale + reviewer's concern in-code (contract changes should be caught by a unit test on dataversioncontrol<t>, not by every consumer's recording stub). - placeholder license key: add a documented comment explaining the contract assumption so future readers see the test is a canary if modelpersistenceguard tightens validation. bucket 6 (pre/post-processing): - recordingtensortransformer.isfitted: now backed by an interlocked.exchange-mutated int + volatile.read getter so concurrent fit / fittransform callers don't observe stale state. - inversetransform: honour the supportsinversetransform=false contract by throwing notsupportedexception when called instead of silently returning data — a consumer that didn't probe supportsinversetransform first now gets a clear failure (review #1368). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): promote test-only regularization accessor to public + document engine reset limitation production code: - gradientbasedoptimizerbase.GetRegularizationForTests (internal, test-only) promoted to a public read-only `ActiveRegularization` property. removes the production-side test-coupling antipattern flagged in review #1368 — the test now consumes a genuine public api that production consumers can also use to introspect the configured regularization without reflection. test: - bucket7 ConfigureRegularization_NoRegularization_ReachesGradientOptimizer updated to assert against the new ActiveRegularization property. - configuremethodtestbase fixture now carries explicit documentation of the AiDotNetEngine.ResetToCpu() one-way limitation (the underlying tensors api exposes Current for read but no symmetric SetCurrent for write, so the fixture can't restore on dispose). flagged for follow-up: needs an upstream push/pop engine api in AiDotNet.Tensors. build verification: - dotnet build src/aidotnet.csproj -c release: 0 errors (11487 warnings, unchanged baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): generic augmentationconfig<t, tinput> subclass for type-safe custom augmenter reviewer flagged 3 threads (augmentationconfig.cs L184 / L211 / multiple) that the `object?` typed custom-augmenter slot defers all type checking to runtime, defeats intellisense, and produces silent no-ops if the user passes a mismatched iaugmentation<t, tdata>. added augmentationconfig<t, tinput> generic subclass: - exposes a strongly-typed `iaugmentation<t, tinput>? augmenter` property alongside the inherited base members. setter mirrors into the base customaugmenter slot so the existing builder-side cast picks it up; the cast succeeds trivially because the compile-time generic constraint already guarantees the right type — no runtime mismatch possible. - generic counterparts of forimages / fortabular / foraudio / fortext / forvideo static factories return the typed subclass via `new` keyword (cs0108). - non-generic base class remains for source-compat with existing tests and the augmentation extended integration suite; its xml docs now point readers at the typed subclass as the preferred path. - aimodelbuilder.configureaugmentation gets a strongly-typed overload taking augmentationconfig<t, tinput>; existing overload still accepts the base class so callers can opt in incrementally. xml example updated to demonstrate the new typed configuration. build verification: - dotnet build src/aidotnet.csproj -c release: 0 errors (11448 warnings, unchanged baseline). cs0108 hide-vs-new errors on the static factories resolved with `new` keyword. scope note: - chose the additive-subclass approach over a fully-generic single augmentationconfig<t, tinput> rewrite because the latter would require generic-ifying every consumer site (5 static factories awkward to call without TInput inference, 4 test files updated, iaimodelbuilder method signature change). the subclass approach gives callers the full type-safety win (typed augmenter property + intellisense) without breaking the existing api surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(#1368 review): document lora warmup contiguous layout assumption + reference #1370 shape oracle issue reviewer flagged 2 threads about TrySliceFirstSampleForLoRAWarmup's GetFlat/SetFlat per-element copy assuming contiguous batch-first row-major layout (#1368 threads on AiModelBuilder.cs:~326). the loop is correct against the current Tensor<T> contract but would silently copy wrong elements if a future backend exposes non-contiguous views via stride tricks. documents the layout assumption inline + points readers at #1370 (the new shape-oracle follow-up issue) as the proper long-term fix: eliminate the warmup entirely via a layer-side TryDeclareShape() oracle that lets lazy-init layers (LayerNormalization gamma/beta, MultiHeadAttention weight banks, etc.) declare shape from constructor args without a forward pass. shape oracle is multi-component refactor (LayerBase virtual + per-layer overrides on every lazy-init layer + AiModelBuilder rewire) that deserves its own pr review cycle — tracking at #1370 with full design doc, phased implementation plan, and acceptance criteria. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(#1368 review): adapt bucket9 kd test to fail-fast contract restored in 17cfe0e0d after restoring the notsupportedexception in commit 17cfe0e0d on the regular-training path's kd branch (per the user's fail-fast misconfig policy), the previous bucket9 wiring-assertion test configureknowledgedistillation_nondefaultoptions_landsonresult fails because buildasync now throws before constructing aimodelresult. reviewer flagged this as a contract clash. update the test to verify the new contract: configurekd + regular-training-path (canary transformer + no lora) throws notsupportedexception with a clear diagnostic pointing the user at the supported alternatives. renamed to configureknowledgedistillation_regulartrainingpath_throwsuntiltapeintegrationlands to match the asserted behavior. once kd integrates with the tape-based training flow upstream, the test flips back to the original landsonresult assertion shape; doc comment captures that flip plan. asserts on: - assert.throwsasync<notsupportedexception>(...) wrapping the build. - ex.message contains "KnowledgeDistillation" (user-facing topic). - ex.message contains "tape" (points at the missing integration). build verification: - dotnet build tests/aidotnet.tests/aidotnettests.csproj -c release: 0 errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(#1368 review): document usedirecttrainingpath's intentional model vs _model asymmetry reviewer (#1368 thread C3kYD) flagged that UseDirectTrainingPath takes a `model` parameter but only uses it for the IParameterizable check, while the other two clauses (isClusteringBase, isLoraWrappedNeuralNetwork) read the `_model` field directly. the asymmetry is intentional: `model` is the RESOLVED model at the call site (possibly post-wrapping), while the clustering / lora-detection predicates need the ORIGINAL user-supplied instance (which lives on _model). conflating them in either direction would break one or the other check. documented inline so future edits don't swap `model` <-> `_model` in one of these clauses without understanding the intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): split augmentation cast errors + dedupe trace warnings + narrow nre catch via stack-trace filter three review threads on the configure* coverage pr: 1. ConfigureAugmentation cast-branch error message conflation (C4TP1): the combined `customAug is IAugmentation<T, TInput> typedAug AND preprocessedX is TInput xForAug` branch threw the same "not IAugmentation<T,TInput>" message whether the augmenter type was wrong OR the preprocessed input type was wrong. split into two sequential checks each with its own diagnostic — augmenter-type error vs. preprocessing-output-type error. a correctly-typed augmenter paired with a TInput-changing preprocessor now points the user at the actual problem. 2. Two Trace.TraceWarning firing on every successful BuildAsync (C4TPM): the offline-pass + X-only-no-y constraint warnings were polluting traces in production / CI for any normal ConfigureAugmentation use. downgraded to TraceInformation and added a process-wide once-per-run latch via Interlocked.Exchange on two new static fields. messages still surface but only on the first build of a process. 3. Bucket11 NullReferenceException swallow too broad (C4TPf): a pre-SearchAsync / pre-Train NRE regression would still pass the test because the broad catch swallowed it before the verify-Train.Once assertion would fail. added IsExceptionFromPostTrainSurface helper that walks the exception chain (current + InnerException + AggregateException children) and only accepts NREs whose stack trace passed through AiModelResult / AiModelResultOptions / BuildMetaLearningInternalAsync / GetModelMetadata. a regression that NREs BEFORE Train/SearchAsync now escapes and surfaces. build verification: - dotnet build tests/aidotnet.tests/aidotnettests.csproj -c release: 0 errors (13715 warnings, unchanged baseline). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): small fixes batch 1 — preprocessedX null-guard, lora warmup bulk copy, narrowed catches, exception-namespace match four small post-merge fixes: - C6WKa: simplified the redundant `preprocessedX is not TInput` pattern-match (preprocessedX is statically TInput so the cast was always-true for non-null values) to an explicit null guard. Updated the augmenter call site to use preprocessedX directly instead of the redundant `xForAug` pattern variable. - C6WM9: TrySliceFirstSampleForLoRAWarmup now uses `tensor.Data.Span.Slice(0, perSample).CopyTo(slice.Data.Span)` (bulk vectorized memmove) instead of the per-element GetFlat/SetFlat loop. One CopyTo call per Build instead of perSample virtual calls. - C6WOG/C6WOg: LoRA warmup catch now filters out OperationCanceledException, OutOfMemoryException, and StackOverflowException (let them propagate) before the broad Exception catch. Cancellation propagates; critical exceptions don't get masked. - C6WLs: Bucket10 LoRA test catches use new IsExceptionFromNamespace helper (namespace-prefix provenance walk through exception chain) instead of message-substring "LoRA" matching. Survives adapter renames + message-text refactors. - C6WMo: Bucket9 KD test now asserts by exception TYPE (Assert.IsType<NotSupportedException>) + TargetSite namespace prefix ("AiDotNet.") instead of message substring "KnowledgeDistillation" / "tape". Same rationale: message text is human-readable and can be rephrased without breaking behavior. build verification: dotnet build src/aidotnet.csproj + tests/aidotnet.tests c release: 0 errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): batch 2 — lora dim contract tighten, predict debug.assert, xml doc escape three small post-merge fixes: - C6WO4/C6WPP: TryInferBothDimsFromWeights now returns true ONLY when BOTH inputSize AND outputSize are positive (was: returns true when either dim is positive, leaving the bool result misleading vs the out params). Partial resolutions are still surfaced via the out params for callers that want best-effort info; the bool reflects "is this layer fully shape-known". CreateLoRALayer doesn't use the bool return so this is a pure contract tightening. - C6WR2: AiModelResult.Predict's unfitted-pipeline check switched from a runtime `throw` to `Debug.Assert`. Release builds no longer pay the runtime branch + throw cost on every Predict for what is fundamentally a debug-only invariant (the user-facing failure point is the AiModelResult ctor; the Predict-time check exists only to flag post-construction pipeline mutation, which is a programming error). - C6WQz: XML doc comment in Bucket4 had unnecessary `\"` escape inside a triple-slash comment (XML docs aren't string-literal delimited so backslash-escape is just literal `\"...\"` in IDE tooltips). Plain double quotes now. build verification: dotnet build src/aidotnet.csproj + tests c release: 0 errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WQg): pushlevel uses lifo stack + lock for true scope semantics prior pushlevel/popleve stored a single _previous slot — concurrent pushes on two threads could capture each other's mid-flight value as "previous" and dispose-restore the wrong level. replace single-slot with a stack + process-global lock so nested pushes restore in lifo order, and concurrent push/pop observe a consistent stack. levelscope no longer holds a _previous field; pop reads from the static stack. dispose remains idempotent via interlocked.exchange flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WMS): aimodelresult ctor lazy-fits postprocessing pipeline when given a training-target sample prior ctor threw on any non-fitted postprocessing pipeline. for direct aimodelresultoptions construction paths (federated / meta-learning / distributed) that have a trained model + training data but haven't manually called pipeline.fit, this forced every caller to thread a boilerplate .fit() call. add aimodelresultoptions.postprocessingfitsample (optional toutput). when the ctor detects an unfitted pipeline AND the caller supplied a sample, fit inline. only throw when the sample is null — preserving the fail-fast diagnostic for genuinely-misconfigured callers. aimodelbuilder.buildsupervisedinternalasync continues to fit before construction, so the existing path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WJG): extract fitpostprocessingifneeded helper + call from every build path before: only buildsupervisedinternalasync fitted the postprocessing pipeline before constructing aimodelresult. the 4 other build paths (programsynthesisinferenceonly, streamingsupervised, metalearning, rlinternal) constructed aimodelresultoptions without setting postprocessingpipeline OR fitting it, so any pipeline configured via configurepostprocessing was silently dropped before reaching the result. after: shared fitpostprocessingifneeded(bestsolution, traininginput, buildpathname) helper centralises the fit/fail logic. paths with training data (supervised, streaming) try to fit inline; paths without (inference-only, meta-learning, rl) throw a clear redirect-to-pre-fit diagnostic naming the active build path. also: each path's options now sets postprocessingpipeline = _postprocessingpipeline so a successfully-fitted pipeline reaches the result for downstream predict() invocation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WKu): wire each modality to its builtin augmenter before: the imagesettings / tabularsettings / audiosettings / textsettings / videosettings blocks on augmentationconfig were entirely documentation-only. they were stored on the builder and inspected by no factory — the only way to actually run augmentation was to supply a hand-written iaugmentation via customaugmenter. after: new modalityaugmenterfactory translates each modality's settings block into a typed augmentationpipeline using the built-in augmenter families under src/augmentation/{image,audio,tabular,text,video}. aimodelbuilder.resolvemodalityaugmenter dispatches based on tinput: - imagetensor<t> => image flips / rotation / colorjitter / noise / blur - matrix<t> => tabular feature noise / dropout / mixup - tensor<t> => audio pitch / time stretch / noise / volume / shift - string[] => text synonym / deletion / swap / insertion - imagetensor<t>[] => video temporal crop / flip / drop / speed / spatial customaugmenter still wins when set; modality factory only fires when the user populated settings without supplying their own augmenter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WRW): move test-only configured-state accessors behind iconfiguredview interface before: 8 internal `configured*` accessors lived on aimodelbuilder's regular surface, polluting it with test-verification entry points that shouldn't bind in production code paths but were visible to any caller that flipped `internalsvisibleto`. after: extracted internal iconfiguredview<t, tinput, toutput> interface under src/configuration. aimodelbuilder implements it EXPLICITLY so the accessors no longer appear via member resolution — test code casts to iconfiguredview<...> to read them, production code can't even see the symbols (interface itself is internal). tests updated to use the cast pattern across bucket5_lifecycletests, bucket11_hijackpathtests, yamlconfigtests, licensekeytests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C6WLV + C6WRk): isexceptionfromnamespace tolerates trimmed/aot + rephrase #1368 self-references c6wlv: bucket12's isexceptionfromnamespace previously relied on the formatted stack-trace string containing "at <prefix>.". on release builds with aggressive inlining frames may be elided and on trimmed/aot/non-english-locale runtimes the "at " token can be localized or absent. add two metadata signals that survive trimming: (1) targetsite.module.assembly.name startswith "aidotnet" identifies origin even when declaringtype.fullname is null, (2) drop the "at " anchor on the stack-trace fallback since the namespace token itself is specific enough. c6wrk: rephrase in-tree comment references from "review #1368" / "pr #1368 review" to "this pr's review" across 7 bucket test files — #1368 is the current pr so "pr #1368" implied an earlier numbered pr. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): c7hap process-wide latch non-generic + c6wpz finally null-guard + c7g9r readme bash fence c7hap: each closed-generic aimodelbuilder<t,tin,tout> instantiation had its own static `_augmentation*emitted` field — multiple test runs over distinct generic types would re-emit the trace warning. extracted the two latches into non-generic augmentationwarninglatch helper class so the once-per-process guarantee actually holds across mixed-generic ci sweeps. c6wpz: bucket5 dvc finally-block null-conditional + nullable-string trydeletedir signature so a future refactor that moves recordingdvc construction inside the try doesn't reintroduce nre risk. c7g9r: readme bash fence language hint restored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): c6wns/c7g77 narrow argument/invalidop catches + c7ha7 pushlevel reads via property c6wns + c7g77: bucket11 metalearning + automl tests caught argumentexception and invalidoperationexception unconditionally — the comment said "post-train surface" but only the nrecatch had the isexceptionfrompoststrainsurface guard. add the same provenance filter to both other catches so a pre-train regression (typo,unrelated builder bug) escapes the test and fails it instead of being silently swallowed. c7ha7: pushlevel reads via the level property getter (not _level field) so any future memory barrier or value transform applies symmetrically with the property-setter write below. inside the lock so race-free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): c7mmq narrow fitpostprocessing catch + c7mmp pushlevel snapshot comment + c7mpq drop soe catch + c7mpq sister-references rephrased c7mmq: fitpostprocessingifneeded's catch (exception) re-wrapped oce/oom as invalidoperationexception, hiding the original type. rethrow operationcanceledexception and outofmemoryexception above the broad catch so they surface unchanged. c7mpq: drop catch (stackoverflowexception) in the lora warmup block — modern .net terminates the process on soe so the catch clause is unreachable. c7mmp: bucket4 pushlevel(level) inline-snapshot pattern documented — the apparent no-op middle is a deliberate save-point for lifo-stack restoration. c7mpq (sister refs): remove last two "pr #1368" / "review-#1368" self-references in bucket4 and bucket10 — #1368 is the current pr. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review C7mmB/C7g8-): deduplicate tryinferbothdimsfromweights contract comment the 7-line contract block was inlined twice at the dense-rank-2 branch and the conv-rank-3-plus branch, with mismatched indentation that made the early return look outer-method-level. extract a private bothdimsresolved helper that returns the contract bool — single docstring describes the contract once, both call sites delegate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): kd readme alignment + changelog breaking changes + agentassistance enabled-path test c6wiu / c6wnv / c7g6- / c7mk5: readme "real source bugs fixed" row for configureknowledgedistillation now matches the actual diff — the second throw site was KEPT (not removed); kd options now flow to the result on direct-training paths, regular-training path still throws to fail-fast the missing tape integration. c6wjp / c6wke / c7g-h / c7mno / c7mnv / c7mn2 / c7g-k / c7hAa / c7mp3: changelog "breaking changes (pr #1368)" section enumerating every behavior-change consumers will hit on upgrade: - configureregularization throws on non-gradient optimizer - loraadapterbase.createloralayer throws on unresolvable dims - aimodelresult ctor throws on unfitted postprocessingpipeline - kd second throw site kept on regular-training path - inference fast paths now traverse postprocessing + safety filter each entry has a migration paragraph. c6wqm / c7mmy / c7mm7: paired enabled-path agentassistance test added — captures trace.tracewarning emissions via a tracecapture listener and verifies that with isenabled=true the gate dispatches to the llm path (either visible failure inside aidotnet.agentsystem or trace evidence of the assist call). pairs with the existing isenabled=false test to prove the gate evaluates the flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): c7hau opt-in fit-rows cap + c7mpf tensor span contract debug.assert c7hau: previously fitpostprocessingifneeded always called bestsolution.predict(xtrain) over the full training tensor — doubling the build-time inference cost for any user with postprocessing configured. add setpostprocessingfitmaxrows(int? maxrows) opt-in cap. when set, fitpostprocessingifneeded slices xtrain to the first maxrows rows via the same row-major bulk span.copyto path as the lora warmup slicer. default (unset) preserves current full-set fit behavior for backwards compatibility — opt-in only. (named setpostprocessingfitmaxrows, not configurepostprocessingfitmaxrows, deliberately: the yaml source-generator scans configure* methods and would misrender a primitive int? parameter as a poco yaml section. this is a perf knob, not a yaml-recipe surface.) c7mpf: tensor<t>.data.span row-major contiguous-storage contract that the lora warmup slicer's span.copyto depends on is now backed by a debug.assert that catches the contract break in debug builds. zero release-build cost; the bulk copy is on the warmup hot path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): docs + tighter assertions for remaining concerns c7mlj: postprocessingfitsample xml doc warns that single-sample fit degenerates distribution-learning transformers and recommends ≥256 rows (or pre-fit pipeline yourself for power transformers). c6wk-: stronger doc on customaugmenter object?-typing — calls out the runtime-cast failure point at build time, steers new callers to the generic augmentationconfig<t,tinput>.augmenter property for compile-time type safety. c7g_v / c7mpe: foricons/fortabular static factory `new` shadowing docstring clarifies the c# static-binding semantics — assignment from either invocation site is polymorphism-safe because the runtime instance carries the generic type. c7g8u: bucket12 ddp wrap test now uses recordingcommbackend subclass that tallies every property read + collective-call entry. when the build fails, the assertion requires both (a) failure originated in aidotnet.distributedtraining AND (b) backend.accesscount > 0 — proving the wrap fired vs. a regression upstream of the wrap. c7mnx: bucket8 disabled-augmentation test sets recordingaugmenter.is- enabled=true explicitly so the outer augmentationconfig.isenabled=false gate is the only stopper. a builder regression that checked inner-instead- of-outer would now fail the test instead of passing for the wrong reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): test cast generics + agent listener + kd provenance + level volatile + streaming modality gate c8ecs / c8ec5: iconfiguredview test casts had hardcoded <float,tensor<float>,tensor<float>> generics from a batch script — licensekeytests uses <double,double[],double> and yamlconfigtests uses <double,matrix<double>,vector<double>>. fix the casts per-file so the runtime cast succeeds instead of invalidcastexception. c8edx: agent enabled-path test had an unused delimitedlisttracelistener variable leftover from a refactor — drop it. c8eid: bucket9 kd not-supported provenance check narrowed from "anywhere in aidotnet.*" to "aimodelbuilder specifically" so an unrelated notsupportedexception from elsewhere in aidotnet doesn't satisfy the check. c8eez: gpudiagnosticsconfig.level get/set go through volatile.read/write on an unsafe.as<int> reinterpret of the enum backing so concurrent readers outside the pushlevel/poplevel lock see torn-free fresh values. c8eil: buildstreamingsupervisedasync augmentation gate now throws on EITHER customaugmenter OR any modality settings block (previously only customaugmenter triggered the throw; modality settings would have been silently dropped on streaming path — the same stored-but-not-consumed pattern the pr is trying to eliminate). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#1368 review): c8efy postprocessingfitsample is model predictions + c8ehc strict typeof rationale c8efy: postprocessingfitsample xml doc renamed from "training-target sample" to "model-output predictions" — the pipeline transforms predictions, not targets, so fit needs the prediction distribution. calling out the wrong-distribution risk explicitly so direct aimodelresultoptions callers don't pass training targets and silently produce wrong inference-time transforms. c8ehc: documented the strict typeof equality contract on resolvemodalityaugmenter — derived classes of the shape primitives don't have a built-in augmenter that…
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AiModelBuilder.BuildSupervisedInternalAsync(line 3298) called_modelRegistry.CreateModelVersion(...)without first callingRegisterModel(...). PerIModelRegistry<T,TInput,TOutput>,CreateModelVersioncreates additional versions of an already-registered model name. Without a priorRegisterModel, the implementation throwsArgumentException("Model not found in registry").Discovery
Filed by AiDotNet#1345's integration-test framework when wiring
ConfigureModelRegistry. The testBucket3_QualityOfLifeTests.ConfigureModelRegistry_AndBuildAsync_TracksTrainedModelwas marked[Skip]documenting the bug.Fix
Insert
RegisterModel(...)call immediately beforeCreateModelVersionwith auto-registered metadata + provenance tags:Verification
Build clean on net10.0. The skipped test in PR #1345 can be un-Skip'd after this lands.
🤖 Generated with Claude Code
Summary by CodeRabbit