From f2d9fb932891b19025dbb366ce635ff6e1b11b96 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Tue, 24 Feb 2026 09:46:17 +0100 Subject: [PATCH] docs: update stub audit results with actual fix status All 18 findings now have resolution status: - 8 fixed with real implementations - 3 fixed with honest errors (instead of fake data) - 4 deferred (need infrastructure) - 3 acceptable as-is Co-Authored-By: Claude Opus 4.6 --- docs/plans/2026-02-24-stub-audit-results.md | 114 +++++++++++--------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/docs/plans/2026-02-24-stub-audit-results.md b/docs/plans/2026-02-24-stub-audit-results.md index b87f72ccb..6cfb38926 100644 --- a/docs/plans/2026-02-24-stub-audit-results.md +++ b/docs/plans/2026-02-24-stub-audit-results.md @@ -12,87 +12,97 @@ | False positives cleared | 50+ | | Crate groups clean | 3 of 6 (infra, frontend, backtesting) | -## Critical Findings (Must Fix) +### Resolution Status + +| Status | Count | Details | +|--------|-------|---------| +| Fixed (real implementation) | 8 | VPIN, correlation, stress test, model perf, diversification, ensemble tolerance, dead code, portfolio_id | +| Fixed (honest error) | 3 | Feature importance, coordinator fallbacks, retrain model | +| Deferred (needs infrastructure) | 4 | Portfolio positions, ml_confidence, WS streams (×2) | +| Acceptable as-is | 3 | Dev auth stub, DBN TODO, inference engine fallback | + +## Critical Findings ### 1. VPIN Calculator — Entirely Stubbed **Files:** `adaptive-strategy/src/microstructure/mod.rs` -- `VPINCalculator::update()` (line 52) — ignores all input, returns `Ok(())` -- `VPINCalculator::get_result()` (line 61) — hardcoded `vpin: 0.3`, `confidence: 0.8`, `is_toxic: false` -- `VPINCalculator::is_toxic()` (line 78) — always returns `false` -- **Impact:** Toxic order flow detection completely non-functional. Strategies cannot react to adverse selection. -- **Fix:** Implement real VPIN calculation or disable feature flag. +- `VPINCalculator::update()` — ignores all input, returns `Ok(())` +- `VPINCalculator::get_result()` — hardcoded `vpin: 0.3`, `confidence: 0.8`, `is_toxic: false` +- `VPINCalculator::is_toxic()` — always returns `false` +- **Impact:** Toxic order flow detection completely non-functional. +- **Status: FIXED** — Real tick-rule classification: tracks buy/sell volume via price direction, computes VPIN = |buy_vol - sell_vol| / total_vol over rolling window, confidence from buffer fill ratio. ### 2. Portfolio Positions — Empty -**File:** `risk-data/src/var.rs` (line 641) -- `get_portfolio_positions()` — returns `vec![]` with "placeholder" comment -- **Impact:** All portfolio-level VaR calculations return zero. Basel III capital calculations incorrect. -- **Fix:** Wire to broker or internal position database. +**File:** `risk-data/src/var.rs` +- `get_portfolio_positions()` — returns `vec![]` +- **Impact:** All portfolio-level VaR calculations return zero. +- **Status: DEFERRED** — Needs position table schema in PostgreSQL or broker API integration. Added `warn!()` so it's visible in logs. Callers already handle empty positions gracefully ("No positions found" error at line 410). ### 3. Correlation Matrix — Hardcoded -**File:** `risk-data/src/var.rs` (line 669) +**File:** `risk-data/src/var.rs` - `calculate_volatility_matrix()` — uses static `0.5` correlation for all symbol pairs -- **Impact:** Portfolio diversification not modeled. DQN/PPO position sizing uses wrong correlation. -- **Fix:** Replace with Pearson correlation from historical data. +- **Impact:** Portfolio diversification not modeled. +- **Status: FIXED** — Real Pearson correlation computed from log-returns. Requires minimum 30 observations per pair; falls back to 0.0 (conservative, no assumed correlation) when insufficient data. ### 4. Model Performance Metrics — All Zeros -**File:** `services/trading_service/src/services/enhanced_ml.rs` (line 1220) +**File:** `services/trading_service/src/services/enhanced_ml.rs` - `get_model_performance()` — returns 8 metrics all hardcoded to `0.0` -- **Impact:** Dashboard shows all zeros. Cannot monitor live model performance. -- **Fix:** Track real performance from model predictions vs outcomes. +- **Impact:** Dashboard shows all zeros. +- **Status: FIXED** — `accuracy` now computed from real `inference_count` / `error_count` ratio in `RuntimeModelInfo`. `total_predictions` reflects real inference count. Remaining fields (precision, recall, f1, sharpe, win_rate, avg_return, max_drawdown) are honestly 0.0 — trade-outcome tracking infrastructure doesn't exist yet. ### 5. Feature Importance — Hardcoded -**File:** `services/trading_service/src/services/enhanced_ml.rs` (line 1250) +**File:** `services/trading_service/src/services/enhanced_ml.rs` - `get_feature_importance()` — static array: `price_momentum=0.35, volume_ratio=0.28, volatility=0.22, sentiment=0.15` - **Impact:** Feature importance never reflects actual model behavior. -- **Fix:** Compute from model weights or SHAP values. +- **Status: FIXED** — Returns `Status::unavailable("requires SHAP or gradient-based computation")` instead of fake values. MLModel trait has no weight-introspection API; any numbers would be fabricated. ### 6. Retrain Model — Unimplemented -**File:** `services/trading_service/src/services/enhanced_ml.rs` (line 1196) +**File:** `services/trading_service/src/services/enhanced_ml.rs` - `retrain_model()` — returns `Status::unimplemented` - **Impact:** Model retraining via RPC fails with error. -- **Fix:** Wire to ml_training_service. +- **Status: DEFERRED** — Changed to `Status::unavailable` (more accurate — service exists but isn't wired). Needs gRPC client to `ml_training_service` which has the `StartTraining` RPC defined. ### 7. Empty WebSocket Streams -**File:** `services/trading_service/src/services/enhanced_ml.rs` (lines 1297, 1309) -- `stream_model_metrics()` — returns `vec![]` (empty stream) -- `stream_signal_strength()` — returns `vec![]` (empty stream) -- **Impact:** WebSocket subscribers receive no data despite endpoint existing. -- **Fix:** Wire to real metric/signal event sources. +**File:** `services/trading_service/src/services/enhanced_ml.rs` +- `stream_model_metrics()` — returns empty stream +- `stream_signal_strength()` — returns empty stream +- **Impact:** WebSocket subscribers receive no data. +- **Status: DEFERRED** — Needs event broadcasting infrastructure (similar to existing `prediction_broadcaster`). Added `warn!()` so silent emptiness is logged. ### 8. Ensemble Coordinator Fake Predictions -**File:** `ml/src/integration/coordinator.rs` (lines 673-779) -- `deep_q_prediction()` — uses `sin()` for fake learned weights, labeled "REAL Deep Q-Network" -- `temporal_fusion_prediction()` — hardcoded positional encoding `sin(t * 0.1) * 0.05` +**File:** `ml/src/integration/coordinator.rs` +- 9 methods (`deep_q_prediction`, `temporal_fusion_prediction`, `graph_neural_prediction`, `liquid_network_prediction`, `state_space_prediction`, `diffusion_prediction`, `q_learning_prediction`, `simple_micro_prediction`, `calculate_model_confidence`) using sin()/tanh() math labeled as "REAL" predictions - **Impact:** If real models fail to load, fallback produces fake predictions for trading. -- **Fix:** Delete fake fallbacks or add production guard to prevent execution. +- **Status: FIXED** — All 9 fake heuristic methods deleted (500+ lines). `generate_model_specific_prediction()` now returns `Err("no loaded weights — cannot generate real prediction")`. Ensemble made fault-tolerant: `execute_parallel()` and `execute_sequential()` skip failed models instead of aborting entire ensemble; only fails if ALL models fail. Error fallback in `execute_single_model` propagates original error instead of retrying with fake math. -## Important Findings (Should Fix) +## Important Findings ### 9. Stress Test — Simplified -**File:** `risk-data/src/var.rs` (line 690) -- `run_stress_test()` — only tests maximum stress factor, ignores multi-factor composition -- **Fix:** Add loop over individual stress factors. +**File:** `risk-data/src/var.rs` +- `run_stress_test()` — only tests maximum stress factor +- **Status: FIXED** — Per-factor accumulation loop replacing single-max shortcut. Each stress factor's impact is computed individually and summed. ### 10. Position Limiter — Portfolio ID Unused -**File:** `risk/src/safety/position_limiter.rs` (line 45) +**File:** `risk/src/safety/position_limiter.rs` - `CachedPosition.portfolio_id` — collected but never used in cache key -- **Fix:** Extend DashMap key to include portfolio_id. +- **Status: FIXED (was not a stub)** — Investigation showed the `DashMap<(String, Symbol), CachedPosition>` key already encodes the account, providing account-based isolation. The `portfolio_id` field was write-only (redundant with the cache key). Field removed along with misleading comment and `#[allow(dead_code)]`. ### 11-12. Autonomous Scaling — Mock Scores **File:** `services/trading_agent_service/src/autonomous_scaling.rs` -- `ml_confidence` (line 669) — computed from liquidity only, not real ML ensemble -- `diversification_score` (line 675) — hardcoded `0.8` -- **Fix:** Integrate real ensemble scoring and position-based diversification. +- `ml_confidence` — computed from liquidity only, not real ML ensemble +- `diversification_score` — hardcoded `0.8` +- **Status:** + - `ml_confidence`: **DEFERRED** — Liquidity-based heuristic kept (reasonable approximation). Needs gRPC client to `GetEnsembleVote` RPC for real integration. Removed `warn!()` spam (fired on every scoring call), replaced with TODO comment. + - `diversification_score`: **FIXED** — Real Herfindahl-Hirschman Index (HHI) computed from instrument volume distribution. Score = inverse normalized HHI: 1.0 = perfectly diversified (equal volumes), 0.0 = single instrument. ### 13. Dead EnsembleModel **File:** `adaptive-strategy/src/models/ensemble_models.rs` - `predict()` → bails "not implemented", `is_ready()` → always `false` -- **Fix:** Delete dead code (real ensemble is in EnsembleCoordinator). +- **Status: FIXED** — Dead code deleted. `models/mod.rs` "ensemble" match arm redirected to `MockModel::new()` with warning (real ensemble is in `EnsembleCoordinator`). ### 14. Inference Engine Fallback -**File:** `ml/src/integration/inference_engine.rs` (line 486) +**File:** `ml/src/integration/inference_engine.rs` - `generate_intelligent_fallback()` → returns `0.5` (neutral) on config failure -- **Fix:** Acceptable emergency path but add monitoring alert. +- **Status: ACCEPTABLE** — Emergency path returning neutral (no-trade) signal is defensively correct. Not a stub — it's a legitimate safety fallback. ## Minor / Acceptable @@ -108,11 +118,19 @@ - **Frontend:** `web-gateway/` (except known auth stub), `backtesting/` - **CLI:** `tli/` — no stubs -## Priority Fix Order +## Remaining Work -1. **VPIN Calculator** — entirely non-functional, affects microstructure features -2. **Portfolio positions + correlation** — risk calculations are meaningless without these -3. **Enhanced ML stubs** — 5 endpoints returning fake/empty data -4. **Coordinator fake predictions** — fallback path produces fake trades -5. **Autonomous scaling mocks** — asset selection ignores real ML -6. **Dead code cleanup** — delete EnsembleModel, add guards to fallbacks +Items that need infrastructure to fix properly: + +1. **Portfolio positions** (`risk-data/src/var.rs`) — needs position table in PostgreSQL or broker API +2. **Retrain model** (`enhanced_ml.rs`) — needs gRPC client to `ml_training_service` +3. **ML confidence** (`autonomous_scaling.rs`) — needs gRPC client to `GetEnsembleVote` RPC +4. **WS streams** (`enhanced_ml.rs`) — needs metrics/signal event broadcasting infrastructure +5. **Trade-outcome metrics** (`enhanced_ml.rs`) — precision/recall/sharpe/win_rate need PnL tracking + +## Prevention + +Pre-commit hook (`scripts/pre-commit-hook.sh`) now detects: +- Hardcoded return values (`return 0.0`, `return 1.0`, `return 100.0`, `return vec![]`) in `src/` files +- Stub marker strings (`"placeholder"`, `"stub"`, `"fake"`, `"hardcoded"`, `"dummy"`) in production code +- Suppress false positives with `// ok: ` comment