docs: add codebase consolidation & DRY cleanup design
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jgrusewski
144
docs/plans/2026-03-01-codebase-consolidation-design.md
Normal file
144
docs/plans/2026-03-01-codebase-consolidation-design.md
Normal file
@@ -0,0 +1,144 @@
|
||||
# Codebase Consolidation & DRY Cleanup — Design
|
||||
|
||||
**Date**: 2026-03-01
|
||||
**Branch**: `feature/codebase-consolidation`
|
||||
**Estimated savings**: ~13,100 lines
|
||||
|
||||
## Context
|
||||
|
||||
Following the ML inference cleanup (-8,281 lines), a comprehensive codebase audit
|
||||
revealed 4 major cleanup areas across 543K LOC (crates+services). The `ml` crate
|
||||
alone is 247K LOC but is well-structured; the biggest opportunities are in
|
||||
shared infrastructure, compliance boilerplate, and service-level glue code.
|
||||
|
||||
## Area 1: DRY Violations (~600 lines saved)
|
||||
|
||||
### Problem
|
||||
|
||||
Three type definitions exist as exact duplicates in multiple crates:
|
||||
|
||||
| Type | Canonical Location | Duplicate Location |
|
||||
|---|---|---|
|
||||
| `RetryStrategy` enum | `common/src/error.rs:159` | `trading_engine/src/types/error.rs:18` |
|
||||
| `ErrorSeverity` enum | `common/src/error.rs:132` | `data/src/error.rs:441`, `database/src/error.rs:119` |
|
||||
| `RetryConfig` struct | `common/src/resilience/retry.rs:40` | `storage/src/model_helpers.rs:141` |
|
||||
|
||||
### Design
|
||||
|
||||
1. Keep canonical definitions in `common/`
|
||||
2. Delete duplicates from `trading_engine`, `data`, `database`, `storage`
|
||||
3. Re-export from `common` crate root if not already
|
||||
4. Update all `use` statements in consuming code
|
||||
5. Verify with `cargo check --workspace`
|
||||
|
||||
### Risk
|
||||
|
||||
Low. These are exact duplicates with identical APIs. The only risk is missed
|
||||
import sites, which the compiler will catch.
|
||||
|
||||
## Area 2: `common/src/types.rs` Split (structural, ~4,909 LOC)
|
||||
|
||||
### Problem
|
||||
|
||||
`common/src/types.rs` is a 4,909-line god file containing 6+ unrelated domains:
|
||||
type aliases, service types, market data types, order types, connection types,
|
||||
error types, and currency types. This makes the file hard to navigate and
|
||||
violates the project's 500-line-per-file convention.
|
||||
|
||||
### Design
|
||||
|
||||
Split into focused modules under `common/src/types/`:
|
||||
|
||||
| Module | Content | Est. LOC |
|
||||
|---|---|---|
|
||||
| `aliases.rs` | `AsyncResult`, `SharedHashMap`, `PositionMap`, etc. | ~80 |
|
||||
| `service.rs` | `ServiceId`, `ServiceStatus`, `ConfigVersion` | ~120 |
|
||||
| `market_data.rs` | `MarketDataEvent`, `QuoteEvent`, `TradeEvent`, `BarEvent`, orderbook types | ~1,500 |
|
||||
| `order.rs` | `OrderType`, `OrderSide`, `OrderStatus`, `OrderEvent`, `BrokerType` | ~400 |
|
||||
| `connection.rs` | `ConnectionInfo`, `ConnectionEvent`, `ConnectionStatus`, `ResourceLimits` | ~200 |
|
||||
| `error.rs` | `CommonTypeError` + trait impls | ~200 |
|
||||
| `currency.rs` | `Currency` enum + Display/Default | ~100 |
|
||||
| `mod.rs` | `pub use` re-exports for backward compatibility | ~30 |
|
||||
|
||||
### Backward Compatibility
|
||||
|
||||
`types/mod.rs` will `pub use` everything, so existing `use common::types::*`
|
||||
and `use common::types::QuoteEvent` imports continue working unchanged.
|
||||
|
||||
### Risk
|
||||
|
||||
Low. Pure structural refactor with re-exports. No API changes.
|
||||
|
||||
## Area 3: `trading_engine/compliance/` Audit (~11,000 lines saved)
|
||||
|
||||
### Problem
|
||||
|
||||
The compliance directory contains 16,068 LOC across 9 files:
|
||||
|
||||
- `iso27001_compliance.rs` (3,217) — ISMS scope, organization info, facilities
|
||||
- `compliance_reporting.rs` (2,406) — Report generation
|
||||
- `sox_compliance.rs` (2,262) — Sarbanes-Oxley controls
|
||||
- `audit_trails.rs` (1,847) — Audit trail storage
|
||||
- `transaction_reporting.rs` (1,714) — MiFID/Dodd-Frank TCA
|
||||
- `automated_reporting.rs` (1,628) — Automated report scheduling
|
||||
- `best_execution.rs` (1,101) — 29 struct/enums for best execution analysis
|
||||
- `mod.rs` (1,107) — Module orchestration
|
||||
- `regulatory_api.rs` (786) — API endpoints
|
||||
|
||||
Much of this appears to be enterprise scaffolding (ISO27001 `OrganizationInfo`,
|
||||
`Location`, `FacilityType`, `ContactInfo`, `ISMSScope`) that is unlikely to be
|
||||
called from production code paths in an HFT futures trading system.
|
||||
|
||||
### Design
|
||||
|
||||
1. **Audit usage**: grep for each public type/function to find callers outside tests
|
||||
2. **Categorize**: active (called in production) vs dormant (tests-only or zero callers)
|
||||
3. **Delete dormant code**: Remove types and functions with zero production callers
|
||||
4. **Consolidate active code**: Merge remaining functions into fewer, smaller files
|
||||
5. **Keep essentials**: audit trail logging, basic compliance checks that risk/ references
|
||||
|
||||
### Risk
|
||||
|
||||
Medium. Need to verify each type's usage carefully. Some compliance types may be
|
||||
referenced transitively through trait bounds. The `#[allow(unused_variables)]`
|
||||
at the top of `best_execution.rs` is a red flag suggesting it may be entirely unused.
|
||||
|
||||
## Area 4: `trading_service/enhanced_ml.rs` Cleanup (~1,500 lines saved)
|
||||
|
||||
### Problem
|
||||
|
||||
At 3,027 LOC, this file implements the ML gRPC service with:
|
||||
- Runtime model tracking (`RuntimeModelInfo`) duplicating `ml::ModelMetadata`
|
||||
- Model loading code that should delegate to `EnsembleModelAdapter`
|
||||
- Known `Status::unavailable` stubs for retrain and feature_importance
|
||||
- PPO/TFT inference code with hardcoded fallbacks
|
||||
|
||||
### Design
|
||||
|
||||
1. Audit for legacy patterns matching deleted `MLFeatureExtractor`/`SimpleDQNAdapter`
|
||||
2. Consolidate `RuntimeModelInfo` with existing `ml::ModelMetadata` types
|
||||
3. Simplify model loading to use `ml::ensemble::build_production_strategy()`
|
||||
4. Remove dead code paths that duplicate what `EnsembleModelAdapter` now handles
|
||||
5. Keep the gRPC service interface — just slim the implementation
|
||||
|
||||
### Risk
|
||||
|
||||
Medium. This file implements active gRPC endpoints. Changes must preserve the
|
||||
proto contract. Focus on internal simplification, not API changes.
|
||||
|
||||
## Execution Order
|
||||
|
||||
1. **DRY violations** (creates canonical shared types)
|
||||
2. **types.rs split** (structural, no deps on other areas)
|
||||
3. **compliance/ audit** (largest deletion target)
|
||||
4. **enhanced_ml.rs cleanup** (builds on Area 1 patterns)
|
||||
|
||||
## Verification
|
||||
|
||||
After each area:
|
||||
- `SQLX_OFFLINE=true cargo check --workspace` — zero errors
|
||||
- `SQLX_OFFLINE=true cargo clippy -p <affected_crates> --lib -- -D warnings`
|
||||
- `SQLX_OFFLINE=true cargo test -p <affected_crates> --lib`
|
||||
- `git diff --stat main` — track cumulative line savings
|
||||
|
||||
Final target: **-13,000+ net lines deleted** while preserving all production functionality.
|
||||
Reference in New Issue
Block a user