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>
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 referenceConfigError
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→GatewayConfigErrorConfigResult<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
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.rsfor all usages ofErrorCategory
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 patternrisk/: 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 itrisk/src/circuit_breaker.rs— portfolio-based, fundamentally different from request-based CBtrading_engine/src/types/circuit_breaker.rs— richest implementation, keep as-is for now- Test files — dead code in tests is expected and harmless