docs: Phase 4 production readiness design and implementation plan
16 tasks across 4 pillars: Safety & Crash Elimination, Trading Correctness, Compliance & Code Quality, ML Pipeline Quality. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
162
docs/plans/2026-02-21-production-readiness-phase4-design.md
Normal file
162
docs/plans/2026-02-21-production-readiness-phase4-design.md
Normal file
@@ -0,0 +1,162 @@
|
||||
# Production Readiness Phase 4 — Design
|
||||
|
||||
**Goal**: Eliminate remaining CRITICAL and HIGH-severity production gaps across safety, trading correctness, compliance, and ML pipeline quality — organized as 4 parallel work streams.
|
||||
|
||||
**Architecture**: 16 independent tasks grouped into 4 pillars. Each pillar targets a different concern and operates on non-overlapping files/crates, enabling parallel execution.
|
||||
|
||||
**Tech Stack**: Rust, Candle v0.9.1, tokio, serde_json, prometheus
|
||||
|
||||
---
|
||||
|
||||
## Pillar A: Safety & Crash Elimination (4 tasks)
|
||||
|
||||
Removes remaining panic! vectors in production code paths.
|
||||
|
||||
### Task 1: Replace GPU panic with Result fallback
|
||||
- **File**: `ml/src/lib.rs:1005-1040,1049-1064`
|
||||
- **Problem**: `get_training_device()` and `get_training_device_at()` panic with formatted error if CUDA unavailable
|
||||
- **Fix**: Change return type from `Device` to `Result<Device, MLError>`. Callers already handle errors. Keep diagnostic message as error context, not panic text.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p ml`
|
||||
|
||||
### Task 2: Replace Prometheus static panic with safe fallback
|
||||
- **File**: `trading_engine/src/types/metrics.rs:37-73`
|
||||
- **Problem**: 4 `Lazy<*Vec>` static initializers panic if both primary and "_noop" metric creation fail
|
||||
- **Fix**: Use `unwrap_or_else` that logs error and returns a pre-constructed default metric (using `register` instead of `new` to avoid double-registration). Since these are already no-op fallbacks, the panic path is extremely unlikely but should still be handled.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_engine`
|
||||
|
||||
### Task 3: Replace feature count panic with Result
|
||||
- **File**: `common/src/ml_strategy.rs:1445-1448`
|
||||
- **Problem**: `panic!("Unsupported feature count: {}")` in match arm of `SimpleDQNAdapter` weight initialization
|
||||
- **Fix**: Return `Err(CommonError::InvalidConfiguration(...))` instead of panic. Add the unsupported count to error message.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p common`
|
||||
|
||||
### Task 4: Replace semaphore panic with error propagation
|
||||
- **File**: `common/src/resilience/bounded_concurrency.rs:93-97`
|
||||
- **Problem**: `panic!("Semaphore closed unexpectedly")` in `execute()` async method
|
||||
- **Fix**: Convert `map_err(|_| panic!(...))` to return a new error variant. The `execute` method is generic over `E`, so wrap in a new concrete error type or use the existing `E` constraint. Since the function already returns `Result<T, E>`, add an `E: From<BoundedConcurrencyError>` bound or return the error through a different channel.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p common`
|
||||
|
||||
## Pillar B: Trading Correctness (4 tasks)
|
||||
|
||||
Wires real configurations, re-enables disabled functionality, and fixes risk calculations.
|
||||
|
||||
### Task 5: Wire broker config from BrokerConfig parameter
|
||||
- **File**: `services/trading_service/src/core/broker_routing.rs:295,298`
|
||||
- **Problem**: `ICMarketsConfig::default()` and `IBKRConfig::default()` ignore the passed `broker_config: BrokerConfig` parameter
|
||||
- **Fix**: Extract ICMarkets and IBKR sub-configs from `BrokerConfig`. Check what fields `BrokerConfig` provides (defined in `config` crate) and map to `ICMarketsConfig` and `IBKRConfig`.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_service`
|
||||
|
||||
### Task 6: Re-enable event publishing via interior mutability
|
||||
- **File**: `services/trading_service/src/services/trading.rs:1162`
|
||||
- **Problem**: `EventPublisher::publish()` requires `&mut self` but service holds `Arc<EventPublisher>`. Events are created then discarded.
|
||||
- **Fix**: Wrap `EventPublisher` internal state in `tokio::sync::Mutex` or `RwLock` so `publish()` can take `&self`. Then call `self.event_publisher.publish(event).await` instead of `let _ = event`.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_service`
|
||||
|
||||
### Task 7: Implement VaR recalculation on position changes
|
||||
- **File**: `services/trading_service/src/core/risk_manager.rs:1171-1179`
|
||||
- **Problem**: `recalculate_symbol_var()` is a no-op returning `Ok(())`. No VaR update when positions change.
|
||||
- **Fix**: Query positions via `self.position_tracker`, compute historical VaR from price history in `self.price_history`, update `self.exposure_tracker`. The risk crate already has VaR calculation utilities.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_service`
|
||||
|
||||
### Task 8: Re-enable hyperopt action counting
|
||||
- **File**: `ml/src/hyperopt/adapters/dqn.rs:3051` + `ml/src/trainers/dqn/trainer.rs`
|
||||
- **Problem**: HFT activity score (25% of objective) hardcoded to 0.0 because DQN trainer never populates `buy_count`/`sell_count`/`hold_count` in `additional_metrics`. Causes 62% Sharpe degradation (0.77 → 0.29).
|
||||
- **Fix**: In DQN trainer, count actions during training episodes and write to `additional_metrics` HashMap. Then re-enable `calculate_hft_activity_score_wave10()` call in the hyperopt adapter.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p ml`
|
||||
|
||||
## Pillar C: Compliance & Code Quality (4 tasks)
|
||||
|
||||
Addresses regulatory gaps and code safety standards.
|
||||
|
||||
### Task 9: Implement SOX audit trail async persistence
|
||||
- **File**: `trading_engine/src/compliance/sox_compliance.rs:2118-2132`
|
||||
- **Problem**: `log_event()` only pushes to in-memory `Vec`. No disk persistence. SOX 404 non-compliance.
|
||||
- **Fix**: Add async write to append-only file (JSONL format). Use `tokio::fs::OpenOptions` with append mode. Batch writes via channel — push events to an `mpsc::Sender`, spawn a background task that drains and flushes periodically.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_engine`
|
||||
|
||||
### Task 10: Add clippy deny to risk and data crates
|
||||
- **Files**: `risk/src/lib.rs:28-29`, `data/src/lib.rs:19`
|
||||
- **Problem**: Both crates use `#![allow(clippy::unwrap_used, clippy::expect_used)]` — anti-pattern for safety-critical financial code
|
||||
- **Fix**: Change `allow` to `deny`. Add `#[cfg_attr(test, allow(...))]` for test modules. Fix production violations (likely few since code was written carefully).
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo clippy -p risk -p data`
|
||||
|
||||
### Task 11: Implement TFT layer normalization
|
||||
- **File**: `ml/src/tft/quantized_grn.rs:222-229`
|
||||
- **Problem**: `apply_layer_norm()` returns input unchanged (`Ok(x.clone())`). TFT model quality degraded.
|
||||
- **Fix**: Implement standard layer norm: `(x - mean) / sqrt(var + eps) * weight + bias`. Add `ln_weight` and `ln_bias` tensors to struct, initialize in `from_varbuilder()`. Keep in F32 precision per the function comment.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo test -p ml --lib tft`
|
||||
|
||||
### Task 12: Improve market surveillance stub
|
||||
- **File**: `trading_engine/src/compliance/mod.rs:461-477`
|
||||
- **Problem**: `assess_mar_compliance()` always pushes a "not yet implemented" finding
|
||||
- **Fix**: Implement basic surveillance checks: unusual volume detection (compare to rolling average), price deviation check (compare to recent range), and wash trading detection (same account buy/sell within time window). Use data already in `ComplianceContext`.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p trading_engine`
|
||||
|
||||
## Pillar D: ML Pipeline Quality (4 tasks)
|
||||
|
||||
Wires real data through training and validation pipelines.
|
||||
|
||||
### Task 13: Wire validation pipeline to BacktestingService
|
||||
- **File**: `services/ml_training_service/src/validation_pipeline.rs:343-360`
|
||||
- **Problem**: `run_backtest()` calls `generate_mock_backtest_results()` instead of real BacktestingService gRPC
|
||||
- **Fix**: Use existing `BacktestingServiceClient` to call `RunBacktest` gRPC. Construct `BacktestRequest` from `training_job` config and `data_path`. Parse response into `ValidationMetrics`.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p ml_training_service`
|
||||
|
||||
### Task 14: Wire unified data loader feature extraction
|
||||
- **File**: `ml/src/training/unified_data_loader.rs:496-520`
|
||||
- **Problem**: `create_training_samples()` only extracts 2 features (close price, volume) instead of full feature set
|
||||
- **Fix**: Use `self.feature_extractor.extract(&container)` (the `UnifiedFeatureExtractor` is already stored in the struct). The trait method returns `MLResult<Vec<f64>>` with the complete feature vector.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p ml`
|
||||
|
||||
### Task 15: Implement batch tuning gRPC endpoints
|
||||
- **File**: `services/ml_training_service/src/service.rs:783-814`
|
||||
- **Problem**: 3 gRPC endpoints (`batch_start_tuning_jobs`, `get_batch_tuning_status`, `stop_batch_tuning_job`) return `Status::unimplemented`
|
||||
- **Fix**: Implement using existing `start_tuning_job` as building block. For batch: spawn parallel tasks per model, track with batch ID in `HashMap<String, Vec<JoinHandle>>`. For status: poll handles. For stop: abort handles.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p ml_training_service`
|
||||
|
||||
### Task 16: Complete OCSP revocation checking
|
||||
- **File**: `services/api_gateway/src/auth/mtls/revocation.rs:364`
|
||||
- **Problem**: OCSP infrastructure in place but actual check returns `Ok(false)` with warning log
|
||||
- **Fix**: Implement OCSP request construction using DER-encoded certificate, send HTTP POST to OCSP responder URL (extracted from certificate's Authority Information Access extension), parse OCSP response status.
|
||||
- **Verify**: `SQLX_OFFLINE=true cargo check -p api_gateway`
|
||||
|
||||
---
|
||||
|
||||
## Swarm Execution Strategy
|
||||
|
||||
All 4 pillars run in parallel. Within each pillar, tasks are independent (different files/crates):
|
||||
|
||||
```
|
||||
Pillar A (Safety): Tasks 1-4 [ml, trading_engine, common]
|
||||
Pillar B (Trading): Tasks 5-8 [trading_service, ml/hyperopt+trainers]
|
||||
Pillar C (Compliance): Tasks 9-12 [trading_engine/compliance, risk, data, ml/tft]
|
||||
Pillar D (ML Pipeline): Tasks 13-16 [ml_training_service, ml/training, api_gateway]
|
||||
```
|
||||
|
||||
**Conflict zones** (same crate touched by multiple tasks):
|
||||
- `ml` crate: Tasks 1, 8, 11, 14 (different submodules, parallelizable)
|
||||
- `common` crate: Tasks 3, 4 (different files, parallelizable)
|
||||
- `trading_engine`: Tasks 2, 9, 12 (different submodules, parallelizable)
|
||||
|
||||
**Maximum parallelism**: 16 agents (all tasks independent).
|
||||
|
||||
## Complexity Assessment
|
||||
|
||||
| Task | Effort | Risk |
|
||||
|------|--------|------|
|
||||
| 1 (GPU panic) | Low | Low — return type change |
|
||||
| 2 (Prometheus) | Low | Low — static initialization |
|
||||
| 3 (Feature panic) | Low | Low — simple match arm |
|
||||
| 4 (Semaphore) | Medium | Medium — generic error bounds |
|
||||
| 5 (Broker config) | Medium | Medium — need to map BrokerConfig fields |
|
||||
| 6 (Event publishing) | Medium | Medium — interior mutability refactor |
|
||||
| 7 (VaR recalc) | High | Medium — financial calculation |
|
||||
| 8 (Action counting) | Medium | Low — counter increment + re-enable |
|
||||
| 9 (SOX persistence) | Medium | Low — append-only file writes |
|
||||
| 10 (Clippy deny) | Medium | Medium — potential many violations |
|
||||
| 11 (TFT layer norm) | Medium | Low — standard ML operation |
|
||||
| 12 (MAR surveillance) | High | Medium — domain-specific checks |
|
||||
| 13 (Validation pipeline) | Medium | Medium — gRPC client integration |
|
||||
| 14 (Data loader features) | Low | Low — call existing trait method |
|
||||
| 15 (Batch tuning) | High | Medium — parallel task management |
|
||||
| 16 (OCSP) | High | High — ASN.1/DER encoding |
|
||||
1123
docs/plans/2026-02-21-production-readiness-phase4-implementation.md
Normal file
1123
docs/plans/2026-02-21-production-readiness-phase4-implementation.md
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user