diff --git a/docs/plans/2026-03-01-codebase-consolidation-implementation.md b/docs/plans/2026-03-01-codebase-consolidation-implementation.md new file mode 100644 index 000000000..937cd9d3b --- /dev/null +++ b/docs/plans/2026-03-01-codebase-consolidation-implementation.md @@ -0,0 +1,336 @@ +# Codebase Consolidation & DRY Cleanup — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Eliminate ~13,000 lines of duplicated code, god files, and unused compliance scaffolding. + +**Architecture:** Four independent cleanup areas executed in dependency order: DRY consolidation → types.rs split → compliance audit → enhanced_ml.rs cleanup. Single worktree, frequent commits. + +**Tech Stack:** Rust workspace, SQLX_OFFLINE=true for builds, cargo check/clippy/test + +--- + +### Task 1: Delete duplicate RetryStrategy from trading_engine + +**Files:** +- Modify: `crates/trading_engine/src/types/error.rs` — delete lines 14-85 (RetryStrategy enum + impl) +- Modify: `crates/trading_engine/src/types/retry.rs` — if it imports local RetryStrategy, switch to `common::error::RetryStrategy` + +**Step 1: Check if RetryStrategy is used anywhere in trading_engine** + +Run: `SQLX_OFFLINE=true grep -rn "RetryStrategy" crates/trading_engine/src/ --include="*.rs"` + +Note: Prior audit found ZERO callers outside the definition file. If confirmed, just delete. + +**Step 2: Delete the duplicate RetryStrategy enum and its impl block** + +In `crates/trading_engine/src/types/error.rs`, delete everything from line 14 (`/// Retry strategies`) through line 85 (`}` closing `max_attempts`). Also delete the comment on lines 10-12 about "moved to error-handling crate". + +**Step 3: Fix any compilation errors** + +Run: `SQLX_OFFLINE=true cargo check -p trading_engine` + +If `retry.rs` imports it, change to: `use common::error::RetryStrategy;` + +**Step 4: Commit** + +``` +git add -A && git commit -m "refactor: delete duplicate RetryStrategy from trading_engine" +``` + +--- + +### Task 2: Delete duplicate ErrorSeverity from data/ and database/ + +**Files:** +- Modify: `crates/data/src/error.rs` — delete ErrorSeverity enum (line 441+), update `severity()` method to use `common::error::ErrorSeverity` +- Modify: `crates/data/src/validation.rs` — delete ErrorSeverity enum (line 112+), update struct field and usages +- Modify: `crates/database/src/error.rs` — delete ErrorSeverity enum (line 119+) + +**Step 1: Verify common's ErrorSeverity has the same variants** + +Read `crates/common/src/error.rs` around line 132. Confirm it has: Critical, High, Medium, Low. + +**Step 2: In `crates/data/src/error.rs`:** +- Delete the `ErrorSeverity` enum definition (~line 441-460) and its `Display` impl +- Add `use common::error::ErrorSeverity;` to imports +- The `severity()` method on DataError (line 376) stays — it just returns the common type now + +**Step 3: In `crates/data/src/validation.rs`:** +- Delete the `ErrorSeverity` enum definition (~line 112-120) +- Add `use common::error::ErrorSeverity;` to imports +- The `severity` field in `ValidationError` struct stays + +**Step 4: In `crates/database/src/error.rs`:** +- Delete the `ErrorSeverity` enum definition (~line 119+) +- Add `use common::error::ErrorSeverity;` to imports if anything uses it + +**Step 5: Verify compilation** + +Run: `SQLX_OFFLINE=true cargo check -p data -p database` + +Note: If the Display impl differs between copies, keep common's version and fix callers. + +**Step 6: Run tests** + +Run: `SQLX_OFFLINE=true cargo test -p data --lib && SQLX_OFFLINE=true cargo test -p database --lib` + +**Step 7: Commit** + +``` +git add -A && git commit -m "refactor: consolidate ErrorSeverity to common crate" +``` + +--- + +### Task 3: Delete duplicate RetryConfig from storage/ + +**Files:** +- Modify: `crates/storage/src/model_helpers.rs` — delete RetryConfig struct (line 141+) and Default impl (line 152+) +- Modify: `crates/storage/src/object_store_backend.rs` — update import to use `common::resilience::RetryConfig` + +**Step 1: Verify common's RetryConfig has matching fields** + +Read `crates/common/src/resilience/retry.rs` around line 40. Confirm fields match: max_retries, base_delay, max_delay, circuit_breaker_threshold, circuit_breaker_timeout. + +**Step 2: Delete RetryConfig from model_helpers.rs** + +Delete the struct definition, Default impl, and update the field in the struct that uses it (`_retry_config: RetryConfig` at line 136). + +**Step 3: Update object_store_backend.rs import** + +Change: `use crate::model_helpers::{ConnectionPool, ProgressCallback, RetryConfig};` +To: `use crate::model_helpers::{ConnectionPool, ProgressCallback};` +Add: `use common::resilience::RetryConfig;` + +Note: If storage doesn't depend on common, add it to storage/Cargo.toml. + +**Step 4: Verify** + +Run: `SQLX_OFFLINE=true cargo check -p storage` + +**Step 5: Commit** + +``` +git add -A && git commit -m "refactor: consolidate RetryConfig to common::resilience" +``` + +--- + +### Task 4: Split common/src/types.rs into focused modules + +This is a large structural refactor. The 4,909-line file has clear section markers +that make splitting straightforward. + +**Files:** +- Create: `crates/common/src/types/` directory +- Create: `crates/common/src/types/mod.rs` — re-exports +- Create: `crates/common/src/types/aliases.rs` — type aliases (lines 26-77) +- Create: `crates/common/src/types/service.rs` — ServiceId, ServiceStatus, ConfigVersion, RequestId (lines 125-276) +- Create: `crates/common/src/types/market_data.rs` — MarketDataEvent, QuoteEvent, TradeEvent, etc. (lines 278-828) +- Create: `crates/common/src/types/type_error.rs` — CommonTypeError enum + impls (lines 828-1106) +- Create: `crates/common/src/types/trading_enums.rs` — OrderType, BrokerType, OrderStatus, OrderSide, Currency, TimeInForce (lines 1106-1393) +- Create: `crates/common/src/types/identifiers.rs` — EventId, FillId, AggregateId, AssetId, ClientId (lines 1393-1631) +- Create: `crates/common/src/types/domain.rs` — Order, Position, Execution, Price, Quantity, Money, Symbol, etc. (lines 1631-3970) +- Create: `crates/common/src/types/market.rs` — MarketRegime, TickType, Exchange, MarketTick, TradingSignal, OrderRef (lines 3970-4429) +- Delete: `crates/common/src/types.rs` (original file) + +**Step 1: Create the types/ directory** + +```bash +mkdir -p crates/common/src/types +``` + +**Step 2: Extract each section into its own file** + +For each file, copy the relevant lines from types.rs, adding necessary imports at the top +of each new file (use statements for chrono, serde, uuid, HashMap, etc.). + +The `mod.rs` re-exports everything: +```rust +mod aliases; +mod service; +mod market_data; +mod type_error; +mod trading_enums; +mod identifiers; +mod domain; +mod market; + +pub use aliases::*; +pub use service::*; +pub use market_data::*; +pub use type_error::*; +pub use trading_enums::*; +pub use identifiers::*; +pub use domain::*; +pub use market::*; +``` + +Cross-references within the module use `super::` or `crate::types::`. + +**Step 3: Move the test module** + +The `#[cfg(test)] mod tests` block (lines 4433+) should be split: +- Tests for domain types → `domain.rs` +- Tests for market data → `market_data.rs` +- Or keep as a single `tests.rs` that imports from the parent module + +**Step 4: Update lib.rs if needed** + +`crates/common/src/lib.rs` should already have `pub mod types;` — this stays. The re-exports +in `mod.rs` maintain backward compatibility. + +**Step 5: Verify** + +Run: `SQLX_OFFLINE=true cargo check --workspace` + +This must compile the entire workspace since many crates import from `common::types`. + +**Step 6: Run common tests** + +Run: `SQLX_OFFLINE=true cargo test -p common --lib` + +**Step 7: Commit** + +``` +git add -A && git commit -m "refactor: split common/types.rs (4909 LOC) into 8 focused modules" +``` + +--- + +### Task 5: Audit compliance module usage + +**Files:** +- Read: all files in `crates/trading_engine/src/compliance/` +- Read: `crates/trading_engine/src/lib.rs` (compliance module export) + +**Step 1: Find all external callers of each compliance submodule** + +For each of the 8 compliance files, grep the entire workspace for usage of their +public types/functions OUTSIDE the compliance directory: + +```bash +# For each file, check if its types are used externally +for type in ISO27001ComplianceManager SOXCompliance MiFIDCompliance BestExecutionAnalyzer \ + ComplianceReporting AuditTrailManager AutomatedReporting TransactionReport; do + echo "=== $type ===" + grep -rn "$type" crates/ services/ --include="*.rs" | grep -v "compliance/" | grep -v "/tests/" +done +``` + +**Step 2: Document findings** + +Create a table: +| Type/Module | External callers | Verdict | +|---|---|---| +| iso27001_compliance | ? | keep/delete | +| sox_compliance | ? | keep/delete | +| ... | | | + +**Step 3: Report findings before proceeding** + +Present the usage table and get confirmation before deleting anything. +This step is a checkpoint — do NOT delete without review. + +--- + +### Task 6: Delete unused compliance modules + +Based on Task 5 findings. Expected: iso27001, sox, automated_reporting, and +best_execution have zero external callers. + +**Step 1: Delete each unused file** + +For each confirmed-unused file: +- Delete the file +- Remove `pub mod ;` from `compliance/mod.rs` +- Remove any `pub use` re-exports from `compliance/mod.rs` + +**Step 2: Trim compliance/mod.rs** + +Remove re-exports for deleted modules. Remove struct definitions in mod.rs +that only served as wrappers for deleted submodules. + +**Step 3: Verify** + +Run: `SQLX_OFFLINE=true cargo check --workspace` + +Fix any compilation errors — if something was actually used, restore it. + +**Step 4: Run trading_engine tests** + +Run: `SQLX_OFFLINE=true cargo test -p trading_engine --lib` + +**Step 5: Commit** + +``` +git add -A && git commit -m "refactor: delete unused compliance scaffolding (-N lines)" +``` + +--- + +### Task 7: Audit and clean enhanced_ml.rs + +**Files:** +- Modify: `services/trading_service/src/services/enhanced_ml.rs` +- Reference: `crates/ml/src/ensemble/model_adapter.rs` (EnsembleModelAdapter) + +**Step 1: Read the full file and identify** + +1. `RuntimeModelInfo` — does it overlap with `ml::ModelMetadata`? +2. `FeaturePreprocessor` — does it duplicate `ProductionFeatureExtractorAdapter`? +3. `EnsembleConfig` — does it duplicate `ml::ensemble::ExtendedEnsembleConfig`? +4. Model loading code — can it use `build_production_strategy()`? +5. Dead `Status::unavailable` stubs + +**Step 2: Delete/consolidate identified duplicates** + +Replace internal types with imports from ml crate where possible. +Simplify model loading to delegate to `ml::ensemble::build_production_strategy()`. + +**Step 3: Verify** + +Run: `SQLX_OFFLINE=true cargo check -p trading_service` + +**Step 4: Run tests** + +Run: `SQLX_OFFLINE=true cargo test -p trading_service --lib` + +**Step 5: Commit** + +``` +git add -A && git commit -m "refactor: simplify enhanced_ml.rs, remove legacy patterns" +``` + +--- + +### Task 8: Full verification + +**Step 1: Workspace build** + +Run: `SQLX_OFFLINE=true cargo check --workspace` +Expected: zero errors + +**Step 2: Clippy on affected crates** + +Run: `SQLX_OFFLINE=true cargo clippy -p common -p trading_engine -p data -p database -p storage -p trading_service --lib -- -D warnings` + +**Step 3: Test affected crates** + +Run: +```bash +SQLX_OFFLINE=true cargo test -p common --lib +SQLX_OFFLINE=true cargo test -p trading_engine --lib +SQLX_OFFLINE=true cargo test -p data --lib +SQLX_OFFLINE=true cargo test -p database --lib +SQLX_OFFLINE=true cargo test -p storage --lib +``` + +**Step 4: Line count verification** + +Run: `git diff --stat main` — target: -13,000+ net lines + +**Step 5: Commit verification results** + +No new commit needed — just verify all tasks are green.