Files
foxhunt/docs/plans/2026-02-22-codebase-deduplication-implementation.md
jgrusewski 3e1c415b8d 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

32 KiB

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:

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:

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:

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:

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:

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

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:

  • ConfigErrorGatewayConfigError
  • 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 ConfigErrorGatewayConfigError and ConfigResultGatewayConfigResult 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

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

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:

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

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:

#[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:

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

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:

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:

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:

common = { path = "../common" }

Remove the local ModelType enum (lines 22-37) and its as_str() impl (lines 39-52). Add:

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:

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:

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

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:

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

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

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

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:

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

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

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

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:

/// 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

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

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

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:

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:

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

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:

use common::resilience::circuit_breaker::{CircuitBreaker, CircuitBreakerConfig};

Where the inline version was constructed as CircuitBreaker::new(threshold, timeout), use:

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

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