fix: resolve 6 unresolved pr review comments from #424 (#486)

* fix: correct onnx attributeproto field numbers per spec

Changed field numbers to match ONNX protobuf specification:
- Field 20 for type (was field 3)
- Field 3 for int value (was field 4)
- Field 2 for float value (was field 5)
- Field 4 for string value (was field 6)
- Field 8 for repeated ints (unchanged, was correct)

This prevents corrupt ONNX attributes when exporting models.

Fixes critical code review issue #4 from PR #424.

Generated with Claude Code

Co-Authored-By: Claude <[email protected]>

* fix: preserve coreml-specific configuration during export

CoreMLExporter was converting CoreMLConfiguration to generic ExportConfiguration,
losing CoreML-specific settings like ComputeUnits, MinimumDeploymentTarget,
SpecVersion, InputFeatures, OutputFeatures, and FlexibleInputShapes.

This fix:
- Stores original CoreMLConfiguration in PlatformSpecificOptions during ExportToCoreML
- Retrieves preserved configuration in ConvertOnnxToCoreML
- Falls back to creating default config for backward compatibility

Addresses PR #424 review comment: exporter drops CoreML-specific configuration

* fix: add explicit null guard for directory creation

Added production-ready null handling for Path.GetDirectoryName edge cases:
- Explicit null check before directory operations
- Changed IsNullOrEmpty to IsNullOrWhiteSpace for better validation
- Added clarifying comments about edge cases (root paths, relative filenames)
- Documented fallback behavior when directory is null/empty

Addresses PR #424 review comment: null directory edge case handling

* fix: use constraint-free hash computation in modelcache

Replaced Marshal.SizeOf/Buffer.BlockCopy hashing with GetHashCode-based approach:
- Removed requirement for T : unmanaged constraint
- Uses unchecked hash combining with prime multipliers (17, 31)
- Samples large arrays (max 100 elements) for performance
- Includes array length and last element for better distribution
- Proper null handling for reference types

This allows ModelCache to work with any numeric type without cascading
constraint requirements through DeploymentRuntime, PredictionModelResult,
and dozens of other classes.

Addresses PR #424 review comment: ModelCache T constraint for hashing semantics

* fix: correct event ordering in telemetrycollector getevents

Fixed incorrect ordering logic where Take(limit) was applied before
OrderByDescending(timestamp), causing arbitrary events to be returned
instead of the most recent ones.

Changed:
- _events.Take(limit).OrderByDescending(e => e.Timestamp)
To:
- _events.OrderByDescending(e => e.Timestamp).Take(limit)

This ensures the method returns the MOST RECENT events as intended,
not random events from the ConcurrentBag.

Added clarifying documentation explaining the fix and return value semantics.

Addresses PR #424 review comment: GetEvents ordering issue

* fix: add comprehensive validation for tensorrt configuration

Added production-ready validation to prevent invalid TensorRT configurations:

1. ForInt8() method validation:
   - Throws ArgumentNullException if calibration data path is null/whitespace
   - Ensures INT8 configurations always have calibration data

2. New Validate() method checks:
   - INT8 enabled requires non-empty CalibrationDataPath
   - Calibration data file exists if path is provided
   - MaxBatchSize >= 1
   - MaxWorkspaceSize >= 0
   - BuilderOptimizationLevel in valid range [0-5]
   - NumStreams >= 1 when EnableMultiStream is true

This prevents runtime failures from misconfigured TensorRT engines,
especially the critical INT8 without calibration data scenario.

Addresses PR #424 review comment: TensorRTConfiguration calibration data validation

* fix: add bounds checking for inputsize/outputsize casts in coreml proto

Validate InputSize and OutputSize are non-negative before casting to ulong to prevent
negative values from wrapping to large unsigned values in CoreML protobuf serialization.

* fix: add production-ready onnx parsing with type validation and correct shape extraction

This commit fixes three critical issues in ONNX→CoreML conversion:

1. **Data type validation in ParseTensor**: Now reads and validates the data_type field
   (field 5), ensuring only FLOAT tensors are converted. Throws NotSupportedException
   for unsupported types (DOUBLE, INT8, etc.) instead of silently corrupting data.

2. **Correct TypeProto parsing**: Fixed ParseTypeProto to properly handle nested ONNX
   protobuf structure (TypeProto → tensor_type → shape → dim → dim_value) instead of
   incorrectly treating every varint as a dimension. This fixes tensor shape extraction
   for model inputs/outputs.

3. **Accurate InnerProduct layer sizing**: Changed from Math.Sqrt approximation (which
   assumed square matrices) to using actual tensor shape from ONNX dims. For MatMul/Gemm
   layers, correctly extracts [out_dim, in_dim] from weight tensor shape.

Technical changes:
- ParseTensor now returns OnnxTensor with Name, Data, and Shape fields
- Added OnnxTensor class to store tensor metadata alongside float data
- Updated OnnxGraphInfo.Initializers from Dictionary<string, float[]> to Dictionary<string, OnnxTensor>
- Added ParseTensorTypeProto, ParseTensorShapeProto, and ParseDimensionProto helper methods
- ConvertOperatorToLayer uses shape[0] and shape[1] for layer sizing with sqrt fallback

* fix: preserve all configuration properties across cloning and deserialization

This ensures deployment behavior, model adaptation capabilities, and training history
are maintained when copying or reloading models.

Updated three methods:
1. WithParameters: Now passes LoRAConfiguration, CrossValidationResult, AgentConfig,
   AgentRecommendation, and DeploymentConfiguration to constructor
2. DeepCopy: Same as WithParameters for consistency
3. Deserialize: Now assigns all RAG components (RagRetriever, RagReranker, RagGenerator,
   QueryProcessors) and configuration properties (LoRAConfiguration, CrossValidationResult,
   AgentConfig, AgentRecommendation, DeploymentConfiguration) from deserialized object

This fixes the issue where deployment/export/runtime settings, LoRA configurations, and
meta-learning properties were lost when calling WithParameters, DeepCopy, or Deserialize.

* fix: correct onnx field numbers and address pr review comments

CRITICAL: Fix ONNX TensorProto field number compliance:
- OnnxProto.cs: Change field 3 → 8 for tensor name per ONNX spec
- OnnxToCoreMLConverter.cs: Fix all TensorProto fields (1=dims, 2=data_type, 8=name, 9=raw_data)
- Previous incorrect field numbers would cause empty tensor names and broken shape inference

Additional fixes:
- CoreMLExporter.cs: Fix QuantizationBits mapping (Int8→8, Float16→16, default→32)
- TensorRTConfiguration.cs: Use ArgumentException instead of ArgumentNullException for whitespace validation
- ModelExporterBase.cs: Remove redundant null check (IsNullOrWhiteSpace handles null)

Addresses PR #486 review comments #1, #2, #4, #5, #6

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

Co-Authored-By: Claude <[email protected]>

* style: use ternary operator for coreml config assignment

Simplify CoreMLExporter.cs by using ternary conditional operator instead of if/else for CoreMLConfiguration assignment.

Addresses PR #486 review comment #5

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

Co-Authored-By: Claude <[email protected]>

* fix: replace gethashcode with sha256 for model cache correctness

CRITICAL: Model caching requires cryptographically secure hashing to prevent hash collisions that would cause incorrect predictions.

Previous GetHashCode() approach issues:
- Hash collision probability ~2^-32 (unacceptable for ML inference)
- Non-deterministic across .NET runtimes, machines, and process restarts
- Sampled only 100 elements from large arrays (incomplete hashing)
- Could return same cache entry for different inputs (silent data corruption)

SHA256-based approach:
- Collision probability ~2^-256 (cryptographically secure)
- Deterministic and stable across all platforms and runtimes
- Hashes ALL array elements for complete correctness
- Ensures cached results always match the correct input

Performance impact: SHA256 hashing adds microseconds, inference takes milliseconds/seconds - the overhead is negligible compared to model inference time.

This fix prioritizes correctness over premature optimization. For production ML systems, silent data corruption from hash collisions is unacceptable.

Addresses PR #486 review comment #3

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

Co-Authored-By: Claude <[email protected]>

---------

Co-authored-by: Claude <[email protected]>

Author: ooples

src/Deployment/Export/Onnx/OnnxProto.cs

@@ -353,8 +353,8 @@ private static byte[] CreateTensorProto<T>(string name, Vector<T> data)
         writer.WriteTag(2, WireFormat.WireType.Varint);
         writer.WriteInt32(GetDataTypeValue(typeof(T)));
 
-        // Field 3: name
-        writer.WriteTag(3, WireFormat.WireType.LengthDelimited);
+        // Field 8: name (per ONNX TensorProto specification)
+        writer.WriteTag(8, WireFormat.WireType.LengthDelimited);
         writer.WriteString(name);
 
         // Field 9: raw_data

src/Deployment/Mobile/CoreML/CoreMLProto.cs

@@ -263,6 +263,12 @@ private static byte[] CreateInnerProductLayer(CoreMLLayer layer)
         using var stream = new MemoryStream();
         using var writer = new CodedOutputStream(stream);
 
+        // Validate layer sizes before casting to prevent negative values from wrapping to large unsigned values
+        if (layer.InputSize < 0 || layer.OutputSize < 0)
+            throw new ArgumentException(
+                $"Layer '{layer.Name}' has invalid size: InputSize={layer.InputSize}, OutputSize={layer.OutputSize}. " +
+                "Both must be non-negative for CoreML protobuf serialization.");
+
         // Field 1: inputChannels
         writer.WriteTag(1, WireFormat.WireType.Varint);
         writer.WriteUInt64((ulong)layer.InputSize);

src/Deployment/Mobile/CoreML/OnnxToCoreMLConverter.cs

@@ -75,8 +75,8 @@ private static void ParseGraph(byte[] graphBytes, OnnxGraphInfo graphInfo)
                     break;
                 case 5: // initializer (weights)
                     var initBytes = reader.ReadBytes();
-                    var (name, weights) = ParseTensor(initBytes.ToByteArray());
-                    graphInfo.Initializers[name] = weights;
+                    var tensor = ParseTensor(initBytes.ToByteArray());
+                    graphInfo.Initializers[tensor.Name] = tensor;
                     break;
                 case 11: // input
                     var inputBytes = reader.ReadBytes();
@@ -128,13 +128,15 @@ private static OnnxNode ParseNode(byte[] nodeBytes)
         return node;
     }
 
-    private static (string name, float[] weights) ParseTensor(byte[] tensorBytes)
+    private static OnnxTensor ParseTensor(byte[] tensorBytes)
     {
         using var stream = new MemoryStream(tensorBytes);
         using var reader = new CodedInputStream(stream);
 
         string name = string.Empty;
         float[] weights = Array.Empty<float>();
+        int dataType = -1; // ONNX TensorProto.DataType: 1 = FLOAT, 11 = DOUBLE, etc.
+        var dims = new List<long>();
 
         while (!reader.IsAtEnd)
         {
@@ -143,20 +145,47 @@ private static (string name, float[] weights) ParseTensor(byte[] tensorBytes)
 
             switch (fieldNumber)
             {
-                case 3: // name
+                case 1: // dims (repeated) - ONNX TensorProto field 1
+                    dims.Add(reader.ReadInt64());
+                    break;
+                case 2: // data_type - ONNX TensorProto field 2
+                    dataType = reader.ReadInt32();
+                    break;
+                case 8: // name - ONNX TensorProto field 8
                     name = reader.ReadString();
                     break;
                 case 9: // raw_data
                     var rawBytes = reader.ReadBytes().ToByteArray();
-                    weights = BytesToFloatArray(rawBytes);
+                    // Validate data type before conversion
+                    if (dataType == 1) // FLOAT (32-bit)
+                    {
+                        weights = BytesToFloatArray(rawBytes);
+                    }
+                    else if (dataType == -1)
+                    {
+                        // data_type field not yet encountered, assume float for backward compatibility
+                        weights = BytesToFloatArray(rawBytes);
+                    }
+                    else
+                    {
+                        throw new NotSupportedException(
+                            $"Tensor '{name}' has unsupported data type {dataType}. " +
+                            $"Only FLOAT (type 1) tensors are supported for ONNX→CoreML conversion. " +
+                            $"Common types: 1=FLOAT, 11=DOUBLE, 2=UINT8, 3=INT8, 6=INT32, 7=INT64.");
+                    }
                     break;
                 default:
                     reader.SkipLastField();
                     break;
             }
         }
 
-        return (name, weights);
+        return new OnnxTensor
+        {
+            Name = name,
+            Data = weights,
+            Shape = dims.Select(d => (int)d).ToArray()
+        };
     }
 
     private static OnnxValueInfo ParseValueInfo(byte[] valueInfoBytes)
@@ -191,7 +220,7 @@ private static OnnxValueInfo ParseValueInfo(byte[] valueInfoBytes)
 
     private static int[] ParseTypeProto(byte[] typeBytes)
     {
-        // Simplified: extract shape dimensions from tensor type
+        // Parse ONNX TypeProto structure: TypeProto → tensor_type → shape → repeated dim → dim_value
         var shape = new List<int>();
 
         using var stream = new MemoryStream(typeBytes);
@@ -200,10 +229,12 @@ private static int[] ParseTypeProto(byte[] typeBytes)
         while (!reader.IsAtEnd)
         {
             var tag = reader.ReadTag();
-            if (WireFormat.GetTagWireType(tag) == WireFormat.WireType.Varint)
+            var fieldNumber = WireFormat.GetTagFieldNumber(tag);
+
+            if (fieldNumber == 1) // tensor_type (LengthDelimited)
             {
-                var dim = (int)reader.ReadInt64();
-                if (dim > 0) shape.Add(dim);
+                var tensorTypeBytes = reader.ReadBytes().ToByteArray();
+                shape = ParseTensorTypeProto(tensorTypeBytes);
             }
             else
             {
@@ -214,6 +245,88 @@ private static int[] ParseTypeProto(byte[] typeBytes)
         return shape.ToArray();
     }
 
+    private static List<int> ParseTensorTypeProto(byte[] tensorTypeBytes)
+    {
+        // Parse TensorTypeProto: field 1 = elem_type (skip), field 2 = shape
+        var shape = new List<int>();
+
+        using var stream = new MemoryStream(tensorTypeBytes);
+        using var reader = new CodedInputStream(stream);
+
+        while (!reader.IsAtEnd)
+        {
+            var tag = reader.ReadTag();
+            var fieldNumber = WireFormat.GetTagFieldNumber(tag);
+
+            if (fieldNumber == 2) // shape (LengthDelimited)
+            {
+                var shapeBytes = reader.ReadBytes().ToByteArray();
+                shape = ParseTensorShapeProto(shapeBytes);
+            }
+            else
+            {
+                reader.SkipLastField(); // Skip elem_type and unknown fields
+            }
+        }
+
+        return shape;
+    }
+
+    private static List<int> ParseTensorShapeProto(byte[] shapeBytes)
+    {
+        // Parse TensorShapeProto: repeated field 1 = dim
+        var dims = new List<int>();
+
+        using var stream = new MemoryStream(shapeBytes);
+        using var reader = new CodedInputStream(stream);
+
+        while (!reader.IsAtEnd)
+        {
+            var tag = reader.ReadTag();
+            var fieldNumber = WireFormat.GetTagFieldNumber(tag);
+
+            if (fieldNumber == 1) // dim (LengthDelimited, repeated)
+            {
+                var dimBytes = reader.ReadBytes().ToByteArray();
+                var dimValue = ParseDimensionProto(dimBytes);
+                if (dimValue > 0)
+                {
+                    dims.Add(dimValue);
+                }
+            }
+            else
+            {
+                reader.SkipLastField();
+            }
+        }
+
+        return dims;
+    }
+
+    private static int ParseDimensionProto(byte[] dimBytes)
+    {
+        // Parse DimensionProto: field 1 = dim_value (Varint)
+        using var stream = new MemoryStream(dimBytes);
+        using var reader = new CodedInputStream(stream);
+
+        while (!reader.IsAtEnd)
+        {
+            var tag = reader.ReadTag();
+            var fieldNumber = WireFormat.GetTagFieldNumber(tag);
+
+            if (fieldNumber == 1) // dim_value
+            {
+                return (int)reader.ReadInt64();
+            }
+            else
+            {
+                reader.SkipLastField(); // Skip dim_param and unknown fields
+            }
+        }
+
+        return 0;
+    }
+
     private static float[] BytesToFloatArray(byte[] bytes)
     {
         var floats = new float[bytes.Length / 4];
@@ -285,7 +398,7 @@ private static CoreMLNeuralNetwork ConvertNeuralNetwork(OnnxGraphInfo onnxGraph,
         return network;
     }
 
-    private static CoreMLLayer? ConvertOperatorToLayer(OnnxNode op, Dictionary<string, float[]> initializers, int layerIndex)
+    private static CoreMLLayer? ConvertOperatorToLayer(OnnxNode op, Dictionary<string, OnnxTensor> initializers, int layerIndex)
     {
         var layer = new CoreMLLayer
         {
@@ -303,18 +416,31 @@ private static CoreMLNeuralNetwork ConvertNeuralNetwork(OnnxGraphInfo onnxGraph,
 
                 // Extract weights from initializers
                 var weightsKey = op.Inputs.Count > 1 ? op.Inputs[1] : null;
-                if (weightsKey != null && initializers.TryGetValue(weightsKey, out var weights))
+                if (weightsKey != null && initializers.TryGetValue(weightsKey, out var weightsTensor))
                 {
-                    layer.Weights = weights;
-                    layer.InputSize = weights.Length / (weights.Length > 0 ? (int)Math.Sqrt(weights.Length) : 1);
-                    layer.OutputSize = (int)Math.Sqrt(weights.Length);
+                    layer.Weights = weightsTensor.Data;
+
+                    // Use actual tensor shape instead of sqrt approximation
+                    // ONNX weight matrices for MatMul/Gemm are typically [out_dim, in_dim]
+                    if (weightsTensor.Shape != null && weightsTensor.Shape.Length == 2)
+                    {
+                        layer.OutputSize = weightsTensor.Shape[0];
+                        layer.InputSize = weightsTensor.Shape[1];
+                    }
+                    else if (weightsTensor.Data.Length > 0)
+                    {
+                        // Fallback for 1D or missing shape: infer square matrix (legacy behavior)
+                        var sqrtLen = (int)Math.Sqrt(weightsTensor.Data.Length);
+                        layer.InputSize = sqrtLen;
+                        layer.OutputSize = sqrtLen;
+                    }
                 }
 
                 // Extract bias if present
                 var biasKey = op.Inputs.Count > 2 ? op.Inputs[2] : null;
-                if (biasKey != null && initializers.TryGetValue(biasKey, out var bias))
+                if (biasKey != null && initializers.TryGetValue(biasKey, out var biasTensor))
                 {
-                    layer.Bias = bias;
+                    layer.Bias = biasTensor.Data;
                     layer.HasBias = true;
                 }
                 break;
@@ -349,7 +475,7 @@ internal class OnnxGraphInfo
 {
     public string Name { get; set; } = string.Empty;
     public List<OnnxNode> Operations { get; set; } = new();
-    public Dictionary<string, float[]> Initializers { get; set; } = new();
+    public Dictionary<string, OnnxTensor> Initializers { get; set; } = new();
     public List<OnnxValueInfo> Inputs { get; set; } = new();
     public List<OnnxValueInfo> Outputs { get; set; } = new();
 }
@@ -373,3 +499,13 @@ internal class OnnxValueInfo
     public string Name { get; set; } = string.Empty;
     public int[] Shape { get; set; } = Array.Empty<int>();
 }
+
+/// <summary>
+/// ONNX tensor with data and shape information.
+/// </summary>
+internal class OnnxTensor
+{
+    public string Name { get; set; } = string.Empty;
+    public float[] Data { get; set; } = Array.Empty<float>();
+    public int[] Shape { get; set; } = Array.Empty<int>();
+}

src/Deployment/TensorRT/TensorRTConfiguration.cs

@@ -157,12 +157,11 @@ public static TensorRTConfiguration ForHighThroughput(int batchSize = 64, string
     /// Creates a configuration with INT8 quantization.
     /// </summary>
     /// <param name="calibrationDataPath">Path to calibration data file (required for INT8 quantization)</param>
-    /// <exception cref="ArgumentNullException">Thrown when calibrationDataPath is null or whitespace</exception>
+    /// <exception cref="ArgumentException">Thrown when calibrationDataPath is null or whitespace</exception>
     public static TensorRTConfiguration ForInt8(string calibrationDataPath)
     {
         if (string.IsNullOrWhiteSpace(calibrationDataPath))
-            throw new ArgumentNullException(nameof(calibrationDataPath),
-                "Calibration data path is required for INT8 quantization");
+            throw new ArgumentException("Calibration data path cannot be null or whitespace", nameof(calibrationDataPath));
 
         return new TensorRTConfiguration
         {

src/Models/Results/PredictionModelResult.cs

@@ -995,7 +995,8 @@ public IFullModel<T, TInput, TOutput> WithParameters(Vector<T> parameters)
         updatedOptimizationResult.BestSolution = newModel;
 
         // Create new result with updated optimization result
-        // Use constructor that preserves BiasDetector, FairnessEvaluator, and RAG components
+        // Preserve all configuration properties to ensure deployment behavior, model adaptation,
+        // and training history are maintained across parameter updates
         return new PredictionModelResult<T, TInput, TOutput>(
             updatedOptimizationResult,
             NormalizationInfo,
@@ -1004,7 +1005,12 @@ public IFullModel<T, TInput, TOutput> WithParameters(Vector<T> parameters)
             RagRetriever,
             RagReranker,
             RagGenerator,
-            QueryProcessors);
+            QueryProcessors,
+            loraConfiguration: LoRAConfiguration,
+            crossValidationResult: CrossValidationResult,
+            agentConfig: AgentConfig,
+            agentRecommendation: AgentRecommendation,
+            deploymentConfiguration: DeploymentConfiguration);
     }
 
     /// <summary>
@@ -1090,7 +1096,8 @@ public IFullModel<T, TInput, TOutput> DeepCopy()
 
         var clonedNormalizationInfo = NormalizationInfo.DeepCopy();
 
-        // Use constructor that preserves BiasDetector, FairnessEvaluator, and RAG components
+        // Preserve all configuration properties to ensure deployment behavior, model adaptation,
+        // and training history are maintained across deep copy
         return new PredictionModelResult<T, TInput, TOutput>(
             clonedOptimizationResult,
             clonedNormalizationInfo,
@@ -1099,7 +1106,12 @@ public IFullModel<T, TInput, TOutput> DeepCopy()
             RagRetriever,
             RagReranker,
             RagGenerator,
-            QueryProcessors);
+            QueryProcessors,
+            loraConfiguration: LoRAConfiguration,
+            crossValidationResult: CrossValidationResult,
+            agentConfig: AgentConfig,
+            agentRecommendation: AgentRecommendation,
+            deploymentConfiguration: DeploymentConfiguration);
     }
 
     /// <summary>
@@ -1218,6 +1230,17 @@ public void Deserialize(byte[] data)
                 ModelMetaData = deserializedObject.ModelMetaData;
                 BiasDetector = deserializedObject.BiasDetector;
                 FairnessEvaluator = deserializedObject.FairnessEvaluator;
+
+                // Preserve RAG components and all configuration properties
+                RagRetriever = deserializedObject.RagRetriever;
+                RagReranker = deserializedObject.RagReranker;
+                RagGenerator = deserializedObject.RagGenerator;
+                QueryProcessors = deserializedObject.QueryProcessors;
+                LoRAConfiguration = deserializedObject.LoRAConfiguration;
+                CrossValidationResult = deserializedObject.CrossValidationResult;
+                AgentConfig = deserializedObject.AgentConfig;
+                AgentRecommendation = deserializedObject.AgentRecommendation;
+                DeploymentConfiguration = deserializedObject.DeploymentConfiguration;
             }
             else
             {