docs: add codebase de-duplication implementation plan

18 tasks across 4 phases: - Phase 1: OrderType, ConfigError, TLS types (zero risk) - Phase 2: ErrorCategory, ModelType, re-export cleanup (low risk) - Phase 3: Dead code audit, config warnings, Adam move (low risk) - Phase 4: CircuitBreaker trait + inline CB replacement (medium risk) Scoped out: ErrorSeverity (semantically different variants), RetryStrategy (complex merge), trading_engine/risk CBs (domain-specific) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-22 20:01:12 +01:00
parent f31c852a45
commit 3e1c415b8d
1 changed files with 947 additions and 0 deletions
--- a/docs/plans/2026-02-22-codebase-deduplication-implementation.md
+++ b/docs/plans/2026-02-22-codebase-deduplication-implementation.md
@@ -0,0 +1,947 @@
+# Codebase De-duplication Implementation Plan
+
+> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
+
+**Goal:** Eliminate duplicated types, dead code, and code smells across the 37-crate Rust workspace.
+
+**Architecture:** Bottom-up consolidation in 4 phases. Each phase compiles and tests independently. Canonical types live in `common/` or `ml/`; consumers re-export or import. Full `cargo check --workspace` and `cargo test` between phases.
+
+**Tech Stack:** Rust, Cargo workspace, SQLX_OFFLINE=true (no PostgreSQL needed)
+
+**Build/test commands:**
+- Compile: `SQLX_OFFLINE=true cargo check --workspace`
+- Test specific crate: `SQLX_OFFLINE=true cargo test -p <crate> --lib`
+- Test all: `SQLX_OFFLINE=true cargo test --workspace --lib`
+
+---
+
+## Phase 1: Zero-Risk Dedup
+
+### Task 1: Consolidate OrderType in ml/
+
+Four identical `OrderType` enums (Market/LimitMaker/IoC) exist in ml/. Consolidate into one shared definition.
+
+**Files:**
+- Create: `ml/src/common/action.rs`
+- Modify: `ml/src/common/mod.rs`
+- Modify: `ml/src/dqn/action_space.rs`
+- Modify: `ml/src/ppo/continuous_transaction_costs.rs`
+- Modify: `ml/src/ppo/factored_action.rs`
+- Modify: `ml/src/ppo/transaction_costs.rs`
+
+**Step 1: Create the unified OrderType module**
+
+Create `ml/src/common/action.rs` with the unified type. Include ALL methods from all 4 copies:
+
+```rust
+use crate::error::MLError;
+use serde::{Deserialize, Serialize};
+
+/// Order type for execution strategy.
+///
+/// Three execution modes with calibrated transaction costs (Wave 2.5).
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
+pub enum OrderType {
+    /// Immediate execution, taker fee (15 bps)
+    Market = 0,
+    /// Passive order, maker rebate (5 bps)
+    LimitMaker = 1,
+    /// Immediate-or-cancel (10 bps)
+    IoC = 2,
+}
+
+impl OrderType {
+    /// Transaction cost as decimal (e.g., 0.0015 for 15 bps).
+    pub fn transaction_cost(&self) -> f64 {
+        match self {
+            OrderType::Market => 0.0015,
+            OrderType::LimitMaker => 0.0005,
+            OrderType::IoC => 0.0010,
+        }
+    }
+
+    /// Transaction cost in basis points (e.g., 15.0 for Market).
+    pub fn cost_bps(&self) -> f32 {
+        match self {
+            OrderType::Market => 15.0,
+            OrderType::LimitMaker => 5.0,
+            OrderType::IoC => 10.0,
+        }
+    }
+
+    /// Cost as decimal from bps (convenience wrapper).
+    pub fn cost_decimal(&self) -> f64 {
+        (self.cost_bps() as f64) / 10000.0
+    }
+
+    /// Convert from index (0-2).
+    pub fn from_index(idx: usize) -> Result<Self, MLError> {
+        match idx {
+            0 => Ok(OrderType::Market),
+            1 => Ok(OrderType::LimitMaker),
+            2 => Ok(OrderType::IoC),
+            _ => Err(MLError::InvalidInput(format!(
+                "Invalid order type index: {}",
+                idx
+            ))),
+        }
+    }
+}
+
+impl std::fmt::Display for OrderType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            OrderType::Market => write!(f, "Market"),
+            OrderType::LimitMaker => write!(f, "LimitMaker"),
+            OrderType::IoC => write!(f, "IoC"),
+        }
+    }
+}
+```
+
+**Step 2: Export from ml/src/common/mod.rs**
+
+Add `pub mod action;` to `ml/src/common/mod.rs` (it currently has `circuit_breaker`, `config`, `metrics`, `performance`).
+
+**Step 3: Replace in dqn/action_space.rs**
+
+Remove the `OrderType` enum definition and its `impl` block (lines 44-73). Add at the top:
+```rust
+pub use crate::common::action::OrderType;
+```
+
+Keep all other code in the file (ExposureLevel, Urgency, FactoredAction structs use OrderType — they'll pick it up via the re-export).
+
+**Step 4: Replace in ppo/continuous_transaction_costs.rs**
+
+Remove the `OrderType` enum definition and its `impl` block (lines 81-108). Add at the top:
+```rust
+pub use crate::common::action::OrderType;
+```
+
+**Step 5: Replace in ppo/factored_action.rs**
+
+Remove the `OrderType` enum definition and its `impl` block (lines 56-85). Add at the top:
+```rust
+pub use crate::common::action::OrderType;
+```
+
+**Step 6: Replace in ppo/transaction_costs.rs**
+
+Remove the `OrderType` enum definition and its `impl` block (lines 17-40). Add at the top:
+```rust
+pub use crate::common::action::OrderType;
+```
+
+**Step 7: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p ml`
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Expected: PASS with 0 warnings about OrderType
+
+**Step 8: Commit**
+
+```bash
+git add ml/src/common/action.rs ml/src/common/mod.rs ml/src/dqn/action_space.rs ml/src/ppo/continuous_transaction_costs.rs ml/src/ppo/factored_action.rs ml/src/ppo/transaction_costs.rs
+git commit -m "refactor(ml): consolidate OrderType into common/action.rs"
+```
+
+---
+
+### Task 2: Consolidate ConfigError
+
+api_gateway defines its own `ConfigError` (8 variants) that overlaps with `config/src/error.rs` (5 variants). The api_gateway version has additional variants (Redis, Serialization, Unauthorized) — these are gateway-specific. Rename api_gateway's to `GatewayConfigError` to avoid name collision and confusion.
+
+**Files:**
+- Modify: `services/api_gateway/src/error.rs`
+- Check: all files in `services/api_gateway/src/` that reference `ConfigError`
+
+**Step 1: Audit usages in api_gateway**
+
+Search for `ConfigError` in all api_gateway source files to understand how it's used. The type and its `ConfigResult<T>` alias need updating everywhere they appear.
+
+**Step 2: Rename ConfigError to GatewayConfigError**
+
+In `services/api_gateway/src/error.rs`, rename:
+- `ConfigError` → `GatewayConfigError`
+- `ConfigResult<T>` → `GatewayConfigResult<T>`
+
+Keep all 8 variants — they are gateway-specific and belong here.
+
+**Step 3: Update all references in api_gateway**
+
+Find and replace `ConfigError` → `GatewayConfigError` and `ConfigResult` → `GatewayConfigResult` throughout `services/api_gateway/src/`.
+
+**Step 4: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p api_gateway`
+Run: `SQLX_OFFLINE=true cargo test -p api_gateway --lib`
+
+**Step 5: Commit**
+
+```bash
+git add services/api_gateway/
+git commit -m "refactor(api_gateway): rename ConfigError to GatewayConfigError to avoid collision with config crate"
+```
+
+---
+
+### Task 3: Consolidate TLS shared types
+
+The 4 service TLS configs share identical types: `TlsProtocolVersion` (enum), `UserRole` (enum with 6 RBAC roles), `ClientIdentity` (struct with 4 fields). Extract these shared types to `common/src/tls.rs`.
+
+**Important context:** The TLS *validation logic* differs between services (async vs sync, delegated vs monolithic), so we only extract the shared type definitions — NOT the full TlsConfig implementations. Each service keeps its own TlsConfig struct.
+
+**Files:**
+- Create: `common/src/tls.rs`
+- Modify: `common/src/lib.rs`
+- Modify: `services/trading_service/src/tls_config.rs`
+- Modify: `services/ml_training_service/src/tls_config.rs`
+- Modify: `services/backtesting_service/src/tls_config.rs`
+- Modify: `services/api_gateway/src/auth/mtls/tls_config.rs`
+
+**Step 1: Create common/src/tls.rs**
+
+```rust
+use serde::{Deserialize, Serialize};
+
+/// TLS protocol version selection.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+pub enum TlsProtocolVersion {
+    Tls12,
+    Tls13,
+}
+
+/// RBAC roles extracted from client certificates.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub enum UserRole {
+    Admin,
+    Trader,
+    Analyst,
+    Auditor,
+    System,
+    ReadOnly,
+}
+
+/// Client identity extracted from mTLS certificate.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ClientIdentity {
+    pub common_name: String,
+    pub organization: String,
+    pub role: UserRole,
+    pub certificate_serial: String,
+}
+```
+
+**Step 2: Add `pub mod tls;` to common/src/lib.rs**
+
+**Step 3: In each service TLS file, remove the local definitions and import from common**
+
+Replace local `TlsProtocolVersion`, `UserRole`, `ClientIdentity` definitions with:
+```rust
+pub use common::tls::{ClientIdentity, TlsProtocolVersion, UserRole};
+```
+
+Keep all other service-specific logic (TlsConfig struct, validation methods, TlsInterceptor).
+
+**Step 4: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Expected: PASS — services already depend on common
+
+**Step 5: Commit**
+
+```bash
+git add common/src/tls.rs common/src/lib.rs services/trading_service/src/tls_config.rs services/ml_training_service/src/tls_config.rs services/backtesting_service/src/tls_config.rs services/api_gateway/src/auth/mtls/tls_config.rs
+git commit -m "refactor: extract shared TLS types (TlsProtocolVersion, UserRole, ClientIdentity) to common/"
+```
+
+---
+
+### Task 4: Phase 1 verification
+
+**Step 1: Full workspace check**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Expected: PASS with 0 errors
+
+**Step 2: Run affected crate tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Run: `SQLX_OFFLINE=true cargo test -p api_gateway --lib`
+Run: `SQLX_OFFLINE=true cargo test -p common --lib`
+
+**Step 3: Clippy check**
+
+Run: `SQLX_OFFLINE=true cargo clippy -p ml -p api_gateway -p common -- -D warnings`
+
+---
+
+## Phase 2: Low-Risk Dedup
+
+### Task 5: Consolidate ErrorCategory
+
+`ml/src/lib.rs` has a stub `ErrorCategory` with only 1 variant (`System`). The canonical version in `common/src/error.rs` has 24 variants. Replace the ml stub with an import.
+
+**Files:**
+- Modify: `ml/src/lib.rs` (remove stub enum ~line 465-468)
+- Check: `ml/src/lib.rs` for all usages of `ErrorCategory`
+
+**Step 1: Remove the stub enum from ml/src/lib.rs**
+
+Find and remove:
+```rust
+#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+pub enum ErrorCategory {
+    System,
+}
+```
+
+**Step 2: Add import if needed**
+
+If `ErrorCategory` is used elsewhere in ml/, add:
+```rust
+pub use common::error::ErrorCategory;
+```
+
+If nothing in ml/ actually uses `ErrorCategory` (it may be dead since it was a stub), just delete it without re-exporting.
+
+**Step 3: Check data/src/providers/common.rs**
+
+Read the file to see if its `ErrorCategory` is used internally. If it maps cleanly to common's `ErrorCategory` variants, replace it. If it has genuinely different semantics (provider-specific categories like `DataFormat`), keep it but rename to `ProviderErrorCategory` to avoid confusion.
+
+**Step 4: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p ml -p data`
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+
+**Step 5: Commit**
+
+```bash
+git add ml/src/lib.rs
+git commit -m "refactor(ml): remove stub ErrorCategory, use common::error::ErrorCategory"
+```
+
+---
+
+### Task 6: Consolidate ModelType
+
+Four definitions with incompatible variant counts (15, 7, 4, 2). Strategy: keep `ml/src/lib.rs` as canonical (15 variants), make others import from ml.
+
+**Important:** `model_loader/Cargo.toml` does NOT depend on ml crate. Adding that dependency might cause issues. Instead: move ModelType to common/ where both ml and model_loader can import it.
+
+**Files:**
+- Create: `common/src/model_types.rs`
+- Modify: `common/src/lib.rs`
+- Modify: `common/Cargo.toml` (if needed)
+- Modify: `ml/src/lib.rs` (~line 2179-2211)
+- Modify: `model_loader/src/lib.rs` (~line 22-37)
+- Modify: `ml/src/hyperopt/campaign.rs` (~line 13-16)
+- Modify: `services/ml_training_service/src/job_spawner.rs` (~line 56-82)
+
+**Step 1: Create common/src/model_types.rs**
+
+Move the canonical ModelType enum (15 variants from ml/src/lib.rs) to common. Include the `as_str()` method from model_loader and the `to_db_string()` method from ml_training_service:
+
+```rust
+use serde::{Deserialize, Serialize};
+
+/// Canonical model type enum for the ML pipeline.
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)]
+pub enum ModelType {
+    CompactDQN,
+    DistilledMicroNet,
+    DQN,
+    RainbowDQN,
+    MAMBA,
+    TFT,
+    TGGN,
+    LNN,
+    TLOB,
+    PPO,
+    Transformer,
+    /// Alias for MAMBA
+    Mamba,
+    /// Alias for LNN
+    LiquidNet,
+    /// Alias for TGGN
+    TGNN,
+    Ensemble,
+}
+
+impl ModelType {
+    /// S3/storage prefix string.
+    pub const fn as_str(&self) -> &'static str {
+        match self {
+            ModelType::CompactDQN => "compact_dqn",
+            ModelType::DistilledMicroNet => "distilled_micro_net",
+            ModelType::DQN => "dqn",
+            ModelType::RainbowDQN => "rainbow_dqn",
+            ModelType::MAMBA | ModelType::Mamba => "mamba",
+            ModelType::TFT => "tft",
+            ModelType::TGGN | ModelType::TGNN => "tggn",
+            ModelType::LNN | ModelType::LiquidNet => "liquid",
+            ModelType::TLOB => "tlob",
+            ModelType::PPO => "ppo",
+            ModelType::Transformer => "transformer",
+            ModelType::Ensemble => "ensemble",
+        }
+    }
+
+    /// Database representation string.
+    pub fn to_db_string(&self) -> &'static str {
+        match self {
+            ModelType::DQN | ModelType::CompactDQN | ModelType::RainbowDQN | ModelType::DistilledMicroNet => "DQN",
+            ModelType::PPO => "PPO",
+            ModelType::MAMBA | ModelType::Mamba => "MAMBA-2",
+            ModelType::TFT => "TFT",
+            ModelType::TGGN | ModelType::TGNN => "TGGN",
+            ModelType::LNN | ModelType::LiquidNet => "LNN",
+            ModelType::TLOB => "TLOB",
+            ModelType::Transformer => "Transformer",
+            ModelType::Ensemble => "Ensemble",
+        }
+    }
+}
+
+impl std::fmt::Display for ModelType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_str())
+    }
+}
+```
+
+**Step 2: Add `pub mod model_types;` to common/src/lib.rs**
+
+Also add to the re-exports: `pub use model_types::ModelType;`
+
+**Step 3: In ml/src/lib.rs, replace ModelType with re-export**
+
+Remove the enum definition (~lines 2179-2211). Add:
+```rust
+pub use common::model_types::ModelType;
+```
+
+**Step 4: In model_loader/src/lib.rs, replace ModelType with import**
+
+Add `common` as dependency in `model_loader/Cargo.toml`:
+```toml
+common = { path = "../common" }
+```
+
+Remove the local ModelType enum (lines 22-37) and its `as_str()` impl (lines 39-52). Add:
+```rust
+pub use common::model_types::ModelType;
+```
+
+**Step 5: In ml/src/hyperopt/campaign.rs, replace 2-variant ModelType**
+
+Remove the local enum (lines 13-16) and Display impl (lines 18-25). Add:
+```rust
+use crate::ModelType;
+```
+
+The file only uses DQN and PPO variants, which exist in the canonical enum.
+
+**Step 6: In services/ml_training_service/src/job_spawner.rs, replace 4-variant ModelType**
+
+Remove the local enum (lines 56-82). Add:
+```rust
+use common::model_types::ModelType;
+```
+
+Update any match arms that used `MAMBA2` to use `ModelType::MAMBA` instead.
+
+**Step 7: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Run: `SQLX_OFFLINE=true cargo test -p model_loader --lib`
+Run: `SQLX_OFFLINE=true cargo test -p ml_training_service --lib`
+
+**Step 8: Commit**
+
+```bash
+git add common/src/model_types.rs common/src/lib.rs common/Cargo.toml ml/src/lib.rs model_loader/src/lib.rs model_loader/Cargo.toml ml/src/hyperopt/campaign.rs services/ml_training_service/src/job_spawner.rs
+git commit -m "refactor: unify ModelType into common/model_types.rs (was 4 separate definitions)"
+```
+
+---
+
+### Task 7: Clean up confusing re-exports in common/lib.rs
+
+Remove aliased re-exports like `BarEventFromMarketData`, `MarketDataEventFromMarketData` from `common/src/lib.rs`.
+
+**Files:**
+- Modify: `common/src/lib.rs` (lines ~64-69)
+- Check: all workspace crates for usage of the aliased names
+
+**Step 1: Search for usage of aliased names**
+
+Search the entire workspace for `BarEventFromMarketData` and `MarketDataEventFromMarketData`. If no code outside common/ uses these aliases, delete them. If code does use them, update those references to use the canonical names.
+
+**Step 2: Remove aliases from common/src/lib.rs**
+
+Remove lines like:
+```rust
+pub use market_data::{
+    BarEvent as BarEventFromMarketData,
+    MarketDataEvent as MarketDataEventFromMarketData,
+    ...
+};
+```
+
+Keep the canonical re-exports (without aliases) if the types need to be in the public API.
+
+**Step 3: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+
+**Step 4: Commit**
+
+```bash
+git add common/src/lib.rs
+git commit -m "refactor(common): remove confusing aliased re-exports"
+```
+
+---
+
+### Task 8: Phase 2 verification
+
+**Step 1: Full workspace check**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+
+**Step 2: Run all lib tests**
+
+Run: `SQLX_OFFLINE=true cargo test --workspace --lib 2>&1 | tail -30`
+
+**Step 3: Clippy**
+
+Run: `SQLX_OFFLINE=true cargo clippy --workspace -- -D warnings 2>&1 | head -50`
+
+---
+
+## Phase 3: Dead Code & Smell Removal
+
+### Task 9: Remove dead code in adaptive-strategy/src/risk/kelly_position_sizer.rs
+
+This file has ~24 `#[allow(dead_code)]` markers on structs that are defined but never used: ConcentrationMonitor, CorrelationMatrix, VolatilityOptimizer, VolatilityEstimate, VolatilityModelType, VolatilityModel, CalibrationRecord, DrawdownTracker, PerformanceTracker, DailyReturn, KellyPerformanceMetrics, AccuracyTracker, and functions calculate_base_kelly, calculate_variance, calculate_win_loss_stats.
+
+**Files:**
+- Modify: `adaptive-strategy/src/risk/kelly_position_sizer.rs`
+
+**Step 1: Verify these items are truly unused**
+
+Search the entire workspace for each struct/function name. If they're only referenced in the file where they're defined (and only with `#[allow(dead_code)]`), they're dead.
+
+**Step 2: Delete all confirmed dead code**
+
+Remove each dead struct/enum/function and its associated `#[allow(dead_code)]` marker. Be careful to preserve any structs whose fields ARE used even if the struct itself has `#[allow(dead_code)]` on some fields (check the exploration notes — `VolatilityEstimate.current` and `DrawdownTracker.recovery_factor` may be used).
+
+**Step 3: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p adaptive-strategy`
+Run: `SQLX_OFFLINE=true cargo test -p adaptive-strategy --lib`
+
+**Step 4: Commit**
+
+```bash
+git add adaptive-strategy/src/risk/kelly_position_sizer.rs
+git commit -m "refactor(adaptive-strategy): remove dead code from kelly_position_sizer"
+```
+
+---
+
+### Task 10: Remove dead code in adaptive-strategy execution, risk, and models
+
+Continue dead code cleanup in the remaining adaptive-strategy files.
+
+**Files:**
+- Modify: `adaptive-strategy/src/execution/mod.rs` (~12 dead items: SlippageTracker, routing_rules, venue_performance, ParticipationRateAlgorithm fields, VolumeProfile, VolumeTracker)
+- Modify: `adaptive-strategy/src/risk/mod.rs` (~14 dead items: PnLTracker fields, VaRCalculator fields, CorrelationMatrix, convert_regime, calculate_risk_parity_size, etc.)
+- Modify: `adaptive-strategy/src/models/traditional.rs` (~4 dead fields: config/ready on MovingAverageModel, MACDModel, BollingerBandsModel, RSIModel)
+- Modify: `adaptive-strategy/src/models/deep_learning.rs` (~6 dead fields: config/ready on LSTMModel, TransformerModel, CNNModel)
+
+**Step 1: For each file, verify which items are truly unused**
+
+Search the workspace for each flagged item. For struct fields marked dead: if the struct is constructed but the field is never read, the field is dead. If the struct itself is never constructed, the whole struct is dead.
+
+**Step 2: Delete confirmed dead items**
+
+For dead struct fields: remove the field and update all constructors. For dead functions: delete them. For dead structs: delete the entire struct and its impl blocks.
+
+**Important:** Some `#[allow(dead_code)]` fields (like `config` and `ready` on model structs) may be intentionally reserved for future use. If a model struct IS actively used and only some fields are dead, remove just the dead fields rather than the whole struct.
+
+**Step 3: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p adaptive-strategy`
+Run: `SQLX_OFFLINE=true cargo test -p adaptive-strategy --lib`
+
+**Step 4: Commit**
+
+```bash
+git add adaptive-strategy/src/execution/mod.rs adaptive-strategy/src/risk/mod.rs adaptive-strategy/src/models/traditional.rs adaptive-strategy/src/models/deep_learning.rs
+git commit -m "refactor(adaptive-strategy): remove dead code from execution, risk, and model modules"
+```
+
+---
+
+### Task 11: Remove hardcoded config warnings
+
+`adaptive-strategy/src/config.rs` has 8 `eprintln!("WARNING: Using hardcoded...")` calls in Default impls. These are noise — Default impls are legitimate Rust patterns.
+
+**Files:**
+- Modify: `adaptive-strategy/src/config.rs`
+
+**Step 1: Remove all eprintln! warnings from Default impls**
+
+Find and remove all lines matching:
+```rust
+eprintln!("WARNING: Using hardcoded ... - migrate to database configuration!");
+```
+
+These appear in Default impls for: GeneralConfig, AdaptiveStrategyConfig, EnsembleConfig, ModelConfig, RiskConfig, MicrostructureConfig, RegimeConfig, ExecutionConfig.
+
+Keep all the actual Default field values — just remove the warning prints.
+
+**Step 2: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p adaptive-strategy`
+Run: `SQLX_OFFLINE=true cargo test -p adaptive-strategy --lib`
+
+**Step 3: Commit**
+
+```bash
+git add adaptive-strategy/src/config.rs
+git commit -m "refactor(adaptive-strategy): remove noisy hardcoded config eprintln warnings"
+```
+
+---
+
+### Task 12: Move Adam optimizer out of ml/src/lib.rs
+
+The Adam optimizer struct and impl are defined inline in `ml/src/lib.rs` (approximately lines 80-180). This belongs in its own module.
+
+**Files:**
+- Create: `ml/src/optimizers/mod.rs`
+- Create: `ml/src/optimizers/adam.rs`
+- Modify: `ml/src/lib.rs`
+
+**Step 1: Create ml/src/optimizers/adam.rs**
+
+Move the `Adam` struct, its `impl Adam` block, and all related imports from `ml/src/lib.rs` into this new file. The struct uses `candle_optimisers::adam::Adam` internally, `candle_core::Var` and `Tensor`.
+
+**Step 2: Create ml/src/optimizers/mod.rs**
+
+```rust
+pub mod adam;
+pub use adam::Adam;
+```
+
+**Step 3: Update ml/src/lib.rs**
+
+- Add `pub mod optimizers;`
+- Add `pub use optimizers::Adam;` (preserves existing public API)
+- Remove the Adam struct and impl block from lib.rs
+
+**Step 4: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p ml`
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Run: `SQLX_OFFLINE=true cargo check --workspace` (other crates may import `ml::Adam`)
+
+**Step 5: Commit**
+
+```bash
+git add ml/src/optimizers/ ml/src/lib.rs
+git commit -m "refactor(ml): move Adam optimizer from lib.rs to optimizers/adam.rs"
+```
+
+---
+
+### Task 13: Phase 3 verification
+
+**Step 1: Full workspace check**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+
+**Step 2: Run affected tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Run: `SQLX_OFFLINE=true cargo test -p adaptive-strategy --lib`
+
+**Step 3: Clippy**
+
+Run: `SQLX_OFFLINE=true cargo clippy -p ml -p adaptive-strategy -- -D warnings 2>&1 | head -50`
+
+---
+
+## Phase 4: CircuitBreaker Consolidation
+
+### Task 14: Define CircuitBreaker trait in common/
+
+Create a `CircuitBreaker` trait in `common/src/resilience/circuit_breaker.rs` that captures the core state machine. Keep `SimpleCircuitBreaker` as the default implementation.
+
+**Important context:** The current `common/src/resilience/circuit_breaker.rs` has: `CircuitBreakerState` enum, `CircuitBreakerConfig` struct (3 fields), `CircuitBreakerMetrics` struct, and `CircuitBreaker` struct with async Mutex. We're adding a trait and renaming the existing struct.
+
+**Files:**
+- Modify: `common/src/resilience/circuit_breaker.rs`
+
+**Step 1: Read the current file fully**
+
+Read all 399 lines of `common/src/resilience/circuit_breaker.rs` to understand the existing API.
+
+**Step 2: Add the trait definition**
+
+Add above the existing struct:
+
+```rust
+/// Core circuit breaker trait for the state machine pattern.
+///
+/// Implementors manage the Closed → Open → HalfOpen state transitions.
+#[async_trait::async_trait]
+pub trait CircuitBreakerTrait: Send + Sync {
+    /// Check if a request is allowed through.
+    async fn can_execute(&self) -> bool;
+
+    /// Record a successful operation.
+    async fn record_success(&self);
+
+    /// Record a failed operation.
+    async fn record_failure(&self);
+
+    /// Get current state.
+    async fn state(&self) -> CircuitBreakerState;
+
+    /// Reset to closed state.
+    async fn reset(&self);
+}
+```
+
+**Step 3: Implement the trait for existing CircuitBreaker**
+
+Add `#[async_trait::async_trait]` and `impl CircuitBreakerTrait for CircuitBreaker` that delegates to the existing methods. The existing methods already have the right signatures.
+
+**Step 4: Ensure async-trait is in common/Cargo.toml**
+
+Check if `async-trait` is already a dependency. If not, add it.
+
+**Step 5: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p common`
+Run: `SQLX_OFFLINE=true cargo test -p common --lib`
+
+**Step 6: Commit**
+
+```bash
+git add common/src/resilience/circuit_breaker.rs common/Cargo.toml
+git commit -m "feat(common): add CircuitBreakerTrait for shared circuit breaker interface"
+```
+
+---
+
+### Task 15: Implement trait in ml/ CircuitBreaker
+
+Make `ml/src/common/circuit_breaker.rs` implement the new `CircuitBreakerTrait` from common.
+
+**Files:**
+- Modify: `ml/src/common/circuit_breaker.rs`
+
+**Step 1: Read the current ml CircuitBreaker**
+
+Read all 413 lines. It uses `parking_lot::RwLock` + `AtomicU32`/`AtomicU64` and has methods `allow_request()`, `record_success()`, `record_failure()`, `state()`, `reset()`.
+
+**Step 2: Add trait import and implementation**
+
+```rust
+use common::resilience::circuit_breaker::CircuitBreakerTrait;
+
+#[async_trait::async_trait]
+impl CircuitBreakerTrait for CircuitBreaker {
+    async fn can_execute(&self) -> bool {
+        self.allow_request()
+    }
+
+    async fn record_success(&self) {
+        CircuitBreaker::record_success(self);
+    }
+
+    async fn record_failure(&self) {
+        CircuitBreaker::record_failure(self);
+    }
+
+    async fn state(&self) -> common::resilience::circuit_breaker::CircuitBreakerState {
+        // Map ml's CircuitState to common's CircuitBreakerState
+        match self.state() {
+            CircuitState::Closed => common::resilience::circuit_breaker::CircuitBreakerState::Closed,
+            CircuitState::Open => common::resilience::circuit_breaker::CircuitBreakerState::Open,
+            CircuitState::HalfOpen => common::resilience::circuit_breaker::CircuitBreakerState::HalfOpen,
+        }
+    }
+
+    async fn reset(&self) {
+        CircuitBreaker::reset(self);
+    }
+}
+```
+
+**Note:** The ml CircuitBreaker's existing methods are synchronous (parking_lot::RwLock). The trait methods are async but the impl can call sync methods from async context without issue.
+
+**Step 3: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p ml`
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+
+**Step 4: Commit**
+
+```bash
+git add ml/src/common/circuit_breaker.rs
+git commit -m "refactor(ml): implement CircuitBreakerTrait for ML circuit breaker"
+```
+
+---
+
+### Task 16: Replace inline CircuitBreaker in broker_gateway_service
+
+Replace the ad-hoc inline CircuitBreaker in `broker_gateway_service/src/error_handler.rs` with `common::resilience::CircuitBreaker`.
+
+**Files:**
+- Modify: `services/broker_gateway_service/src/error_handler.rs`
+- Modify: `services/broker_gateway_service/Cargo.toml` (ensure common dependency)
+
+**Step 1: Read the inline implementation**
+
+The inline CircuitBreaker (~line 71) has: `CircuitBreakerState` enum, `CircuitBreaker` struct with `state: Arc<RwLock<>>`, `failure_count: Arc<RwLock<usize>>`, `opened_at: Arc<RwLock<Option<Instant>>>`. Methods: `new()`, `state()`, `record_success()`, `record_failure()`, `can_execute()` (previously named `allow_request` — check actual name).
+
+**Step 2: Replace with common's CircuitBreaker**
+
+Remove the inline CircuitBreakerState enum and CircuitBreaker struct/impl. Import from common:
+
+```rust
+use common::resilience::circuit_breaker::{CircuitBreaker, CircuitBreakerConfig, CircuitBreakerState};
+```
+
+Update the `ErrorHandler` struct to use `common::resilience::circuit_breaker::CircuitBreaker`. The config constants at the top of the file (`CIRCUIT_BREAKER_THRESHOLD: usize = 5`, `CIRCUIT_BREAKER_TIMEOUT: Duration = 60s`) should be used to construct a `CircuitBreakerConfig`:
+
+```rust
+let cb_config = CircuitBreakerConfig {
+    failure_threshold: CIRCUIT_BREAKER_THRESHOLD as u32,
+    timeout: CIRCUIT_BREAKER_TIMEOUT,
+    success_threshold: 1,
+};
+let circuit_breaker = CircuitBreaker::new(cb_config);
+```
+
+**Step 3: Update all usages**
+
+The ErrorHandler uses `circuit_breaker.can_execute()`, `circuit_breaker.record_success()`, `circuit_breaker.record_failure()`, `circuit_breaker.state()`. These should map directly to common's CircuitBreaker methods. Check method name differences (e.g., `allow_request` vs `can_execute`).
+
+**Step 4: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p broker_gateway_service`
+Run: `SQLX_OFFLINE=true cargo test -p broker_gateway_service --lib`
+
+**Step 5: Commit**
+
+```bash
+git add services/broker_gateway_service/src/error_handler.rs services/broker_gateway_service/Cargo.toml
+git commit -m "refactor(broker_gateway): replace inline CircuitBreaker with common::resilience::CircuitBreaker"
+```
+
+---
+
+### Task 17: Replace inline CircuitBreaker in data/benzinga
+
+Replace the ad-hoc inline CircuitBreaker in `data/src/providers/benzinga/production_streaming.rs` with `common::resilience::CircuitBreaker`.
+
+**Files:**
+- Modify: `data/src/providers/benzinga/production_streaming.rs`
+
+**Step 1: Read the inline implementation**
+
+The inline version (~line 203) has `CircuitBreakerState` enum, `CircuitBreaker` struct with configurable `threshold` and `timeout`. Methods: `new(threshold, timeout)`, `is_call_permitted()`, `record_success()`, `record_failure()`.
+
+**Step 2: Replace with common's CircuitBreaker**
+
+Remove the inline types. Import from common:
+```rust
+use common::resilience::circuit_breaker::{CircuitBreaker, CircuitBreakerConfig};
+```
+
+Where the inline version was constructed as `CircuitBreaker::new(threshold, timeout)`, use:
+```rust
+let config = CircuitBreakerConfig {
+    failure_threshold: threshold,
+    timeout,
+    success_threshold: 1,
+};
+CircuitBreaker::new(config)
+```
+
+Replace `is_call_permitted()` calls with `can_execute()`.
+
+**Step 3: Verify**
+
+Run: `SQLX_OFFLINE=true cargo check -p data`
+Run: `SQLX_OFFLINE=true cargo test -p data --lib`
+
+**Step 4: Commit**
+
+```bash
+git add data/src/providers/benzinga/production_streaming.rs
+git commit -m "refactor(data): replace inline CircuitBreaker with common::resilience::CircuitBreaker"
+```
+
+---
+
+### Task 18: Phase 4 verification and final check
+
+**Step 1: Full workspace check**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Expected: 0 errors
+
+**Step 2: Full lib test suite**
+
+Run: `SQLX_OFFLINE=true cargo test --workspace --lib 2>&1 | tail -30`
+
+**Step 3: Clippy on all modified crates**
+
+Run: `SQLX_OFFLINE=true cargo clippy -p common -p ml -p data -p broker_gateway_service -p adaptive-strategy -p api_gateway -p model_loader -- -D warnings 2>&1 | head -50`
+
+**Step 4: Verify warning count**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace 2>&1 | grep -c "warning\["`
+Expected: Same or fewer warnings than before
+
+---
+
+## Notes for Implementer
+
+### ErrorSeverity was scoped out
+
+The exploration revealed that `ErrorSeverity` is NOT identical across crates — it has different variants in different domains:
+- `common/`: Debug, Info, Warn, Error, Critical (logging levels)
+- `database/`, `data/`, `trading_engine/errors`: Low, Medium, High, Critical (impact levels)
+- `trading_engine/events`: Info, Warning, Error, Critical, Fatal (event levels)
+
+These are semantically different types that happen to share a name. Consolidating them requires a design decision about whether to create two distinct types or merge into a superset. This is deferred to a future cleanup.
+
+### RetryStrategy was scoped out
+
+The exploration revealed that `trading_engine/src/types/retry.rs` (622 lines) is significantly more complex than `common/src/resilience/retry.rs` — it has `RetryPolicy` (18 fields), `RetryContext`, `RetryExecutor`, domain-specific presets (hft_optimized, financial_conservative, etc.), and its own test suite. The trading_engine version also has a separate `RetryStrategy` enum in `types/error.rs`. Merging these requires careful analysis of which consumers depend on each. Deferred to future work.
+
+### CircuitBreaker in trading_engine and risk
+
+The `trading_engine/` (989 lines) and `risk/` (981 lines) CircuitBreakers are intentionally complex and domain-specific:
+- `trading_engine/`: Success rate thresholds, latency detection, error classification, HFT presets, registry pattern
+- `risk/`: Portfolio-percentage-based, per-account state, Redis coordination, PnL tracking
+
+These are NOT duplicates of common's simple CircuitBreaker — they're specialized implementations. The trait approach lets them implement `CircuitBreakerTrait` in the future while keeping their domain logic. Converting them fully is deferred.
+
+### What NOT to touch
+- `tli/` — being replaced by web-dashboard, don't waste time cleaning it
+- `risk/src/circuit_breaker.rs` — portfolio-based, fundamentally different from request-based CB
+- `trading_engine/src/types/circuit_breaker.rs` — richest implementation, keep as-is for now
+- Test files — dead code in tests is expected and harmless