From eb26ca1e1fc3474d3eacd48d6b3ff9943a276dc2 Mon Sep 17 00:00:00 2001
From: jgrusewski <jgrusewski@localhost>
Date: Mon, 23 Feb 2026 19:30:00 +0100
Subject: [PATCH] fix(ml): address code review feedback on pipeline integration
 tests

- Add #![allow(unused_crate_dependencies)] for consistency with other ml tests
- Extract EXPECTED_FEATURE_DIM constant (replaces magic number 51)
- Add validated_folds counter to prevent vacuous pass in walk-forward test
- Replace unwrap_or_else(panic) with match pattern

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 ml/tests/real_data_pipeline_test.rs | 36 ++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/ml/tests/real_data_pipeline_test.rs b/ml/tests/real_data_pipeline_test.rs
index ca4df22a3..da93ed543 100644
--- a/ml/tests/real_data_pipeline_test.rs
+++ b/ml/tests/real_data_pipeline_test.rs
@@ -3,11 +3,16 @@
 //! Validates the full pipeline with synthetic data (no Databento API needed):
 //! generate bars -> extract features -> walk-forward split -> normalization -> verify dimensions.
 
+#![allow(unused_crate_dependencies)]
+
 use chrono::{Datelike, NaiveDate, NaiveTime, TimeZone, Utc, Weekday};
 use ml::features::extraction::extract_ml_features;
 use ml::types::OHLCVBar;
 use ml::walk_forward::{generate_walk_forward_windows, NormStats, WalkForwardConfig};
 
+/// Expected feature dimension from `extract_ml_features`.
+const EXPECTED_FEATURE_DIM: usize = 51;
+
 // ---------------------------------------------------------------------------
 // Synthetic bar generator
 // ---------------------------------------------------------------------------
@@ -103,8 +108,13 @@ fn test_pipeline_features_extract_from_synthetic() {
     );
 
     // Extract features
-    let features = extract_ml_features(&bars)
-        .unwrap_or_else(|e| panic!("Feature extraction failed: {e}"));
+    let features = match extract_ml_features(&bars) {
+        Ok(f) => f,
+        Err(e) => {
+            assert!(false, "Feature extraction failed: {e}");
+            return; // unreachable, satisfies type checker
+        }
+    };
 
     // Features should be non-empty (bars - warmup period of 50)
     assert!(
@@ -121,10 +131,11 @@ fn test_pipeline_features_extract_from_synthetic() {
     for (i, fv) in features.iter().enumerate() {
         assert_eq!(
             fv.len(),
-            51,
-            "Feature vector at index {} has {} dims, expected 51",
+            EXPECTED_FEATURE_DIM,
+            "Feature vector at index {} has {} dims, expected {}",
             i,
-            fv.len()
+            fv.len(),
+            EXPECTED_FEATURE_DIM
         );
 
         // No NaN or Inf in any feature
@@ -166,16 +177,18 @@ fn test_pipeline_walk_forward_with_features() {
         windows.len()
     );
 
+    let mut validated_folds = 0_usize;
+
     for window in &windows {
         // Extract features from training data
-        let train_features = if window.train.len() >= 51 {
+        let train_features = if window.train.len() >= EXPECTED_FEATURE_DIM {
             extract_ml_features(&window.train).ok()
         } else {
             None
         };
 
         // Extract features from validation data
-        let val_features = if window.val.len() >= 51 {
+        let val_features = if window.val.len() >= EXPECTED_FEATURE_DIM {
             extract_ml_features(&window.val).ok()
         } else {
             None
@@ -187,6 +200,8 @@ fn test_pipeline_walk_forward_with_features() {
             _ => continue, // Skip folds with insufficient data
         };
 
+        validated_folds += 1;
+
         // Compute NormStats from training data ONLY
         let stats = NormStats::from_features(train_feats);
 
@@ -197,7 +212,7 @@ fn test_pipeline_walk_forward_with_features() {
         if !normalized_train.is_empty() {
             let n = normalized_train.len() as f64;
             // Compute per-feature mean of normalized training data
-            let mut mean_per_feature = vec![0.0_f64; 51];
+            let mut mean_per_feature = vec![0.0_f64; EXPECTED_FEATURE_DIM];
             for fv in &normalized_train {
                 for (m, &v) in mean_per_feature.iter_mut().zip(fv.iter()) {
                     *m += v;
@@ -245,6 +260,11 @@ fn test_pipeline_walk_forward_with_features() {
             }
         }
     }
+
+    assert!(
+        validated_folds >= 1,
+        "No walk-forward folds were actually validated (all skipped due to insufficient data)"
+    );
 }
 
 // ---------------------------------------------------------------------------