Files
foxhunt/docs/plans/2026-03-01-codebase-consolidation-implementation.md
2026-03-01 19:25:01 +01:00

337 lines
11 KiB
Markdown

# 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 <name>;` 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.