cleanup: declarative rewrites for ml-tests and trading-engine TODOs

- ml/tests/dqn_training_pipeline_test.rs: the inline TODO speculated
  about a future `load_checkpoint` hook; loader round-trip coverage
  already lives in dqn_checkpoint_tests. Reword to point there.
- ml/tests/ppo_lstm_training_loop_tests.rs: the assertion on
  `hidden_state_manager.is_some()` is the public-surface proxy for
  "LSTM path active"; deeper introspection isn't exposed. Say so.
- ml/tests/ppo_recurrent_integration_tests.rs: the test is already
  `#[ignore]`d; rewrite the inline TODO as a description of the
  missing `from_varbuilder` constructors on LSTMPolicyNetwork /
  LSTMValueNetwork.
- risk/risk_engine.rs: VarEngine receives a default asset-class
  config because the schema-to-config conversion is not wired.
  Reword the TODO to describe that plainly.
- trading_engine/types/errors.rs: `common::ConversionError` does
  not exist; keep ConversionError local and drop the aspirational
  re-export comment.
- trading_engine/tests/audit_persistence_tests.rs: describe why
  the query assertion only checks the Ok shape (row-to-event
  mapping not wired) rather than pointing at a nonexistent line
  number.

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

crates/ml/tests/dqn_training_pipeline_test.rs

@@ -398,8 +398,9 @@ async fn test_dqn_checkpoint_save_load() -> Result<()> {
 
     assert!(checkpoint_size > 1024, "Checkpoint should be >1KB");
 
-    // TODO: Once we have a load_checkpoint method, test loading here
-    // For now, just verify the file is valid SafeTensors format
+    // Verify the checkpoint bytes round-trip back from disk. A dedicated
+    // `load_checkpoint` entry point is not part of this end-to-end test;
+    // roundtrip coverage for the loader lives in `dqn_checkpoint_tests`.
     let checkpoint_data = std::fs::read(&saved_checkpoint_path)?;
     assert!(
         checkpoint_data.len() == checkpoint_size as usize,

crates/ml/tests/ppo_lstm_training_loop_tests.rs

@@ -192,8 +192,9 @@ fn test_ppo_training_with_lstm_enabled() {
         "Hidden state manager should be initialized when use_lstm=true"
     );
 
-    // TODO: Add verification that LSTM networks are actually being used
-    // This requires checking network types or tracking LSTM state updates
+    // The `hidden_state_manager.is_some()` check above is the observable
+    // proxy for "LSTM path is active"; deeper introspection of the
+    // underlying network types is not surfaced through the public API.
 
     // Create dummy trajectory batch
     let mut batch = create_dummy_trajectory_batch(10, config.state_dim);

crates/ml/tests/ppo_recurrent_integration_tests.rs

@@ -391,9 +391,10 @@ fn test_recurrent_vs_feedforward_ppo() {
 #[test]
 #[ignore = "LSTM checkpoint loading not yet implemented (requires from_varbuilder methods in lstm_networks.rs)"]
 fn test_recurrent_ppo_checkpointing() {
-    // Test that checkpoints can be saved and loaded with LSTM configuration
-    // TODO: Implement from_varbuilder methods in LSTMPolicyNetwork and LSTMValueNetwork
-    // to enable loading LSTM checkpoints from safetensors files
+    // Test that checkpoints can be saved and loaded with LSTM configuration.
+    // `LSTMPolicyNetwork` / `LSTMValueNetwork` currently lack `from_varbuilder`
+    // constructors, so safetensors-based checkpoint loading is not wired; the
+    // test is left under #[ignore] until those constructors exist.
     let config = PPOConfig {
         state_dim: 16,
         num_actions: 63,

crates/risk/src/risk_engine.rs

@@ -890,9 +890,11 @@ impl RiskEngine {
         // Initialize metrics collector (fix: provide max_samples parameter)
         let metrics = Arc::new(RiskMetricsCollector::new(10000));
 
-        // Initialize VarEngine with the var_config and asset classification
-        // Convert AssetClassificationSchema to AssetClassificationConfig
-        let asset_config = AssetClassificationConfig::default(); // TODO: proper conversion
+        // Initialize VarEngine with the var_config. The VaR engine currently
+        // uses `AssetClassificationConfig::default()` — a schema-to-config
+        // conversion from `config.asset_classification` is not wired, so the
+        // VaR side sees default asset buckets regardless of user schema.
+        let asset_config = AssetClassificationConfig::default();
         let var_engine = Arc::new(VarEngine::new(config.var_config.clone(), asset_config));
 
         // Initialize circuit breaker if enabled (safe configuration)

crates/trading_engine/src/types/errors.rs

@@ -14,10 +14,10 @@ use serde::{Deserialize, Serialize};
 use std::fmt;
 use thiserror::Error;
 
-// Re-export common error types for convenience
-// TODO: Import these from common crate once they exist there
-// pub use common::ConversionError;
-// Note: ProtocolError and SymbolError are defined locally in this module
+// `ConversionError` is defined locally in this module. The `common`
+// crate does not currently export a parallel error type, so nothing
+// is re-exported here. `ProtocolError` and `SymbolError` also live
+// in this module.
 
 /// Conversion error types used by trading engine
 #[derive(Error, Debug, Clone, PartialEq, Serialize, Deserialize)]

crates/trading_engine/tests/audit_persistence_tests.rs

@@ -716,9 +716,12 @@ mod query_engine_tests {
         };
 
         let result = engine.query(query).await;
-        // Note: Query will succeed but return empty results until row-to-event mapping is implemented (TODO line 1109)
+        // The audit-persistence layer accepts the query and returns an
+        // empty result set because row-to-event mapping is not wired in
+        // the engine's query path yet. Assert only that the RPC
+        // succeeds — shape assertions come online with the mapping.
         assert!(result.is_ok(), "Query failed: {:?}", result.err());
-        println!("✅ Query by time range successful (implementation pending row mapping)");
+        println!("✅ Query by time range returned Ok (row mapping not yet wired)");
     }
 
     #[tokio::test]