fix: address 5 CodeRabbit review comments on PR #1155

- NBEATSBlock ctor: extract CreateInputShape / CreateOutputShape static
  factories that validate lookbackWindow / forecastHorizon BEFORE the
  base(...) call. Invalid values now surface as ArgumentException with
  the right nameof(...) tag instead of a downstream LayerBase<T> shape
  error.
- InterfaceGuard: class visibility reduced from public to internal to
  match the AiDotNet facade pattern. InternalsVisibleTo on src/AiDotNet.csproj
  already grants access to AiDotNetTests / AiDotNetTestConsole / AiDotNet.Serving /
  AiDotNetBenchmarkTests, so the 58 existing test call sites still compile.
  Doc remark added explaining the visibility choice.
- PretrainedTeacherModel + TransformerTeacherModel: reworded "auto-compiles
  via Tensors' AutoTracer" remarks. The wrapper only invokes the delegate;
  whether auto-compile actually happens depends entirely on what's inside
  the delegate. Removed the unconditional guarantee and added a note that
  external paths (ONNX, REST, etc.) won't pick up engine optimizations.
- SelfTeacherModel.GetLogits: rewrote XML-doc so summary/returns/exception
  match the throw-only behavior (method has no underlying model to run and
  always throws InvalidOperationException). Previous summary said "Gets
  logits from the underlying model" which was misleading.

Verify: dotnet build net10.0 + net471 — 0 errors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: franklinic

src/Helpers/InterfaceGuard.cs

@@ -6,7 +6,15 @@ namespace AiDotNet.Helpers;
 /// callers must validate capabilities before use. These methods provide clear error
 /// messages when a model doesn't support the requested capability.
 /// </summary>
-public static class InterfaceGuard
+/// <remarks>
+/// <para>
+/// <b>Visibility:</b> <c>internal</c> to match the facade pattern — users interact with
+/// <c>AiModelBuilder</c> / <c>AiModelResult</c>, and the InternalsVisibleTo attribute on
+/// AiDotNet.csproj exposes this helper to the test/console/serving assemblies that need
+/// capability checks from outside the main assembly.
+/// </para>
+/// </remarks>
+internal static class InterfaceGuard
 {
     /// <summary>
     /// Returns the model as IParameterizable or throws with a clear message.

src/KnowledgeDistillation/Teachers/PretrainedTeacherModel.cs

@@ -13,9 +13,14 @@ namespace AiDotNet.KnowledgeDistillation.Teachers;
 /// </summary>
 /// <remarks>
 /// <para>
-/// Construction uses a <c>Func&lt;&gt;</c> forward-pass delegate. Inference goes
-/// through the standard model path, which auto-compiles via Tensors' AutoTracer
-/// once the input-shape pattern repeats.
+/// This wrapper takes a <c>Func&lt;Vector&lt;T&gt;, Vector&lt;T&gt;&gt;</c> forward-pass
+/// delegate and invokes it directly on every <see cref="GetLogits"/> call.
+/// The wrapper itself performs no caching or graph compilation — any
+/// optimizations (including Tensors' AutoTracer auto-compile) depend entirely
+/// on what happens inside the supplied delegate. A delegate that wraps a
+/// standard neural-network model's <c>Predict</c> path will pick up those
+/// engine-level optimizations; a delegate that invokes external code
+/// (pre-converted ONNX, a REST call, etc.) will not.
 /// </para>
 /// </remarks>
 [ModelDomain(ModelDomain.MachineLearning)]

src/KnowledgeDistillation/Teachers/SelfTeacherModel.cs

@@ -75,14 +75,19 @@ public void CachePredictions(Vector<T>[] predictions)
     }
 
     /// <summary>
-    /// Gets logits from the underlying model.
+    /// Not supported for <see cref="SelfTeacherModel{T}"/> — always throws.
     /// </summary>
-    /// <param name="input">Input to the model.</param>
-    /// <returns>The logits from the underlying model.</returns>
-    /// <exception cref="InvalidOperationException">Thrown when no underlying model is configured.</exception>
+    /// <param name="input">Ignored.</param>
+    /// <returns>This method does not return; it always throws.</returns>
+    /// <exception cref="InvalidOperationException">
+    /// Always thrown. <see cref="SelfTeacherModel{T}"/> serves pre-computed
+    /// predictions by index via <see cref="GetCachedPrediction"/> and cannot
+    /// evaluate a fresh input vector — it has no underlying model to run.
+    /// </exception>
     /// <remarks>
-    /// <para>Not supported for <see cref="SelfTeacherModel{T}"/> — callers must use
-    /// <see cref="GetCachedPrediction"/>, which returns a pre-computed prediction by index.</para>
+    /// <para>Callers must use <see cref="GetCachedPrediction"/> instead, which
+    /// returns a prediction from the cache populated via
+    /// <see cref="CachePredictions"/>.</para>
     /// </remarks>
     public override Vector<T> GetLogits(Vector<T> input)
     {

src/KnowledgeDistillation/Teachers/TransformerTeacherModel.cs

@@ -14,9 +14,11 @@ namespace AiDotNet.KnowledgeDistillation.Teachers;
 /// <typeparam name="T">The numeric type for calculations (e.g., double, float).</typeparam>
 /// <remarks>
 /// <para>
-/// Construction uses a <c>Func&lt;&gt;</c> forward-pass delegate. Inference goes
-/// through the standard model path, which auto-compiles via Tensors' AutoTracer
-/// once the input-shape pattern repeats.
+/// This wrapper takes a <c>Func&lt;Vector&lt;T&gt;, Vector&lt;T&gt;&gt;</c> forward-pass
+/// delegate and invokes it directly on every <see cref="GetLogits"/> call.
+/// The wrapper performs no caching or graph compilation itself — any
+/// optimizations (including Tensors' AutoTracer auto-compile) depend on what
+/// the supplied delegate does internally.
 /// </para>
 /// <para>For attention-based distillation strategies that need attention weights, implement
 /// a custom IDistillationStrategy that can extract attention from the underlying model.</para>

src/TimeSeries/NBEATSBlock.cs

@@ -119,6 +119,38 @@ public override int ParameterCount
     /// - useInterpretableBasis: Whether to use human-understandable basis functions
     /// </para>
     /// </remarks>
+    /// <summary>
+    /// Validates <paramref name="lookbackWindow"/> and returns the corresponding
+    /// LayerBase input shape. Runs BEFORE the base ctor so invalid values surface
+    /// as <see cref="ArgumentException"/> with the argument name instead of a
+    /// downstream shape error.
+    /// </summary>
+    private static int[] CreateInputShape(int lookbackWindow)
+    {
+        if (lookbackWindow <= 0)
+        {
+            throw new ArgumentException("Lookback window must be positive.", nameof(lookbackWindow));
+        }
+        return new[] { lookbackWindow };
+    }
+
+    /// <summary>
+    /// Validates <paramref name="forecastHorizon"/> (and re-checks lookback for
+    /// consistency) and returns the corresponding LayerBase output shape.
+    /// </summary>
+    private static int[] CreateOutputShape(int lookbackWindow, int forecastHorizon)
+    {
+        if (lookbackWindow <= 0)
+        {
+            throw new ArgumentException("Lookback window must be positive.", nameof(lookbackWindow));
+        }
+        if (forecastHorizon <= 0)
+        {
+            throw new ArgumentException("Forecast horizon must be positive.", nameof(forecastHorizon));
+        }
+        return new[] { lookbackWindow + forecastHorizon };
+    }
+
     public NBEATSBlock(
         int lookbackWindow,
         int forecastHorizon,
@@ -128,16 +160,16 @@ public NBEATSBlock(
         int thetaSizeForecast,
         bool useInterpretableBasis,
         int polynomialDegree = 3)
-        : base(new[] { lookbackWindow }, new[] { lookbackWindow + forecastHorizon })
+        : base(
+            CreateInputShape(lookbackWindow),
+            CreateOutputShape(lookbackWindow, forecastHorizon))
     {
-        if (lookbackWindow <= 0)
-        {
-            throw new ArgumentException("Lookback window must be positive.", nameof(lookbackWindow));
-        }
-        if (forecastHorizon <= 0)
-        {
-            throw new ArgumentException("Forecast horizon must be positive.", nameof(forecastHorizon));
-        }
+        // Primary-argument validation happens inside the static shape factories
+        // above so `lookbackWindow` / `forecastHorizon` are rejected BEFORE
+        // LayerBase<T> consumes them — users see the nameof(...)-tagged
+        // ArgumentException instead of a downstream shape error from the base.
+        // (The two blocks that previously validated those here are now in
+        // CreateInputShape / CreateOutputShape below.)
         if (hiddenLayerSize <= 0)
         {
             throw new ArgumentException("Hidden layer size must be positive.", nameof(hiddenLayerSize));