docs: add cleanup implementation plan
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jgrusewski
320
docs/plans/2026-02-24-cleanup-deps-warnings-implementation.md
Normal file
320
docs/plans/2026-02-24-cleanup-deps-warnings-implementation.md
Normal file
@@ -0,0 +1,320 @@
|
||||
# Cleanup: Warnings, Legacy, Dependencies — Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Eliminate 12 ml warnings, remove broken vendor submodule, trim unused/heavy dependencies, and feature-gate MFA to speed up builds by ~30-40%.
|
||||
|
||||
**Architecture:** Fix warnings in ml/src/diffusion/ (unused imports, missing Debug, hidden lifetimes). Remove orphaned `vendor/candle-optimisers` gitlink. Replace `tokio = features=["full"]` in 8 test crates with workspace defaults. Remove unused `plotters`. Feature-gate MFA deps (`qrcode`, `image`, `totp-rs`, `base32`) in api_gateway behind `mfa` feature.
|
||||
|
||||
**Tech Stack:** Rust workspace, Cargo features, candle ML framework
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Fix ML Diffusion Warnings
|
||||
|
||||
**Files:**
|
||||
- Modify: `ml/src/diffusion/denoiser.rs:15,21,79,128,142`
|
||||
- Modify: `ml/src/diffusion/noise.rs:7,15`
|
||||
- Modify: `ml/src/diffusion/sampler.rs:7,17`
|
||||
- Modify: `ml/src/diffusion/trainable.rs:21`
|
||||
- Modify: `ml/src/liquid/candle_cfc.rs:7`
|
||||
- Modify: `ml/src/ensemble/coordinator.rs:13`
|
||||
|
||||
**Step 1: Remove unused imports**
|
||||
|
||||
In `ml/src/diffusion/noise.rs:7`, change:
|
||||
```rust
|
||||
use candle_core::{DType, Device, Tensor};
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
use candle_core::{Device, Tensor};
|
||||
```
|
||||
|
||||
In `ml/src/diffusion/sampler.rs:7`, change:
|
||||
```rust
|
||||
use candle_core::{DType, Device, Tensor};
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
use candle_core::{Device, Tensor};
|
||||
```
|
||||
|
||||
In `ml/src/liquid/candle_cfc.rs:7`, change:
|
||||
```rust
|
||||
use candle_core::{DType, Device, Tensor};
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
use candle_core::{DType, Tensor};
|
||||
```
|
||||
|
||||
In `ml/src/ensemble/coordinator.rs:13`, change:
|
||||
```rust
|
||||
use chrono::{DateTime, TimeZone, Timelike, Utc};
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
use chrono::{DateTime, Timelike, Utc};
|
||||
```
|
||||
|
||||
**Step 2: Add `#[derive(Debug)]` to diffusion structs**
|
||||
|
||||
Add `#[derive(Debug)]` before each of these struct definitions:
|
||||
- `ml/src/diffusion/denoiser.rs:15` — `pub struct TimeEmbedding`
|
||||
- `ml/src/diffusion/denoiser.rs:128` — `pub struct Denoiser`
|
||||
- `ml/src/diffusion/noise.rs:15` — `pub struct NoiseScheduler`
|
||||
- `ml/src/diffusion/sampler.rs:17` — `pub struct DDIMSampler`
|
||||
- `ml/src/diffusion/trainable.rs:21` — `pub struct DiffusionTrainableAdapter`
|
||||
|
||||
Note: Candle types (`Linear`, `VarMap`, `AdamW`, `Device`, `Tensor`) do NOT implement Debug. Use a manual impl with `#[derive(Debug)]` only for structs that have no Candle fields, and manual `impl Debug` for the rest.
|
||||
|
||||
Check which structs have Candle fields:
|
||||
- `TimeEmbedding` has `proj: Linear` → manual impl
|
||||
- `Denoiser` has `Linear`, `Vec<DenoiserBlock>`, `TimeEmbedding`, `Device` → manual impl
|
||||
- `DenoiserBlock` (line ~65) has `Linear` fields → manual impl (already warned via Denoiser)
|
||||
- `NoiseScheduler` has `device: Device` → manual impl
|
||||
- `DDIMSampler` has only `usize` and `f32` → `#[derive(Debug)]` works
|
||||
- `DiffusionTrainableAdapter` has `VarMap`, `Denoiser`, etc. → manual impl
|
||||
|
||||
For manual impls, use the pattern:
|
||||
```rust
|
||||
impl std::fmt::Debug for TypeName {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("TypeName").finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Step 3: Fix hidden lifetime parameters**
|
||||
|
||||
In `ml/src/diffusion/denoiser.rs`, three functions use `VarBuilder` without explicit lifetime:
|
||||
- Line 21: `pub fn new(embed_dim: usize, hidden_dim: usize, vb: VarBuilder)` → `vb: VarBuilder<'_>`
|
||||
- Line 79: `vb: VarBuilder,` → `vb: VarBuilder<'_>,`
|
||||
- Line 142: `vb: VarBuilder,` → `vb: VarBuilder<'_>,`
|
||||
|
||||
**Step 4: Verify zero warnings**
|
||||
|
||||
Run: `SQLX_OFFLINE=true cargo check -p ml --lib 2>&1 | grep "^warning"`
|
||||
Expected: empty output (0 warnings)
|
||||
|
||||
**Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add ml/src/diffusion/ ml/src/liquid/candle_cfc.rs ml/src/ensemble/coordinator.rs
|
||||
git commit -m "fix(ml): resolve all 12 warnings in diffusion, liquid, ensemble modules"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Remove Broken vendor/candle-optimisers
|
||||
|
||||
**Files:**
|
||||
- Remove: `vendor/candle-optimisers` (broken gitlink, no .gitmodules)
|
||||
- Remove: `vendor/` directory if empty
|
||||
|
||||
**Step 1: Remove the gitlink**
|
||||
|
||||
```bash
|
||||
git rm vendor/candle-optimisers
|
||||
```
|
||||
|
||||
If `vendor/` is now empty:
|
||||
```bash
|
||||
rmdir vendor/ 2>/dev/null || true
|
||||
```
|
||||
|
||||
**Step 2: Verify build still works**
|
||||
|
||||
Run: `SQLX_OFFLINE=true cargo check -p ml --lib 2>&1 | tail -3`
|
||||
Expected: `Finished` with no errors (ml uses upstream git dep, not vendor path)
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add -A vendor/
|
||||
git commit -m "chore: remove broken vendor/candle-optimisers gitlink
|
||||
|
||||
Orphaned submodule with no .gitmodules entry. ml crate uses upstream
|
||||
git dep (github.com/KGrewal1/optimisers) directly — vendor was unused."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Replace tokio "full" in Test Crates
|
||||
|
||||
**Files:**
|
||||
- Modify: `ml-data/Cargo.toml:12`
|
||||
- Modify: `foxhunt-deploy/Cargo.toml:20`
|
||||
- Modify: `tests/test_common/Cargo.toml:12`
|
||||
- Modify: `tests/harness/Cargo.toml:8`
|
||||
- Modify: `tests/e2e/Cargo.toml:8`
|
||||
- Modify: `tests/e2e/vault_integration/Cargo.toml:12`
|
||||
- Modify: `services/api_gateway/load_tests/Cargo.toml:12`
|
||||
- Modify: `services/load_tests/Cargo.toml:23`
|
||||
|
||||
**Step 1: Replace all tokio "full" declarations**
|
||||
|
||||
In each file, replace the tokio line with:
|
||||
```toml
|
||||
tokio = { workspace = true }
|
||||
```
|
||||
|
||||
For `services/load_tests/Cargo.toml` which also adds `test-util`:
|
||||
```toml
|
||||
tokio = { workspace = true }
|
||||
```
|
||||
(workspace default already includes `test-util`)
|
||||
|
||||
**Step 2: Verify workspace builds**
|
||||
|
||||
Run: `SQLX_OFFLINE=true cargo check --workspace 2>&1 | tail -5`
|
||||
Expected: successful compilation
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add ml-data/Cargo.toml foxhunt-deploy/Cargo.toml tests/test_common/Cargo.toml \
|
||||
tests/harness/Cargo.toml tests/e2e/Cargo.toml tests/e2e/vault_integration/Cargo.toml \
|
||||
services/api_gateway/load_tests/Cargo.toml services/load_tests/Cargo.toml
|
||||
git commit -m "chore: replace tokio features=[\"full\"] with workspace defaults in 8 crates
|
||||
|
||||
Workspace tokio already specifies the needed features (rt-multi-thread,
|
||||
macros, net, sync, time, fs, signal, io-util, test-util). \"full\" compiled
|
||||
30+ unused features across 8 test/tooling crates."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Remove Unused plotters from services/load_tests
|
||||
|
||||
**Files:**
|
||||
- Modify: `services/load_tests/Cargo.toml:40`
|
||||
|
||||
**Step 1: Remove plotters dependency**
|
||||
|
||||
Delete line 40 from `services/load_tests/Cargo.toml`:
|
||||
```toml
|
||||
plotters = { version = "0.3", features = ["svg_backend", "bitmap_backend"] }
|
||||
```
|
||||
|
||||
Note: `services/api_gateway/load_tests` DOES use plotters (in `reporting.rs`) — keep it there.
|
||||
|
||||
**Step 2: Verify build**
|
||||
|
||||
Run: `SQLX_OFFLINE=true cargo check -p load_tests 2>&1 | tail -3`
|
||||
Expected: successful compilation
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add services/load_tests/Cargo.toml
|
||||
git commit -m "chore: remove unused plotters dep from services/load_tests"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Feature-Gate MFA Dependencies in api_gateway
|
||||
|
||||
**Files:**
|
||||
- Modify: `services/api_gateway/Cargo.toml:62-69,114-117`
|
||||
- Modify: `services/api_gateway/src/auth/mod.rs:22`
|
||||
|
||||
**Step 1: Make MFA deps optional in Cargo.toml**
|
||||
|
||||
In `services/api_gateway/Cargo.toml`, change the MFA section (lines 62-69):
|
||||
```toml
|
||||
# MFA/TOTP dependencies
|
||||
totp-rs = { version = "5.6", optional = true }
|
||||
qrcode = { version = "0.14", optional = true }
|
||||
image = { version = "0.25", optional = true }
|
||||
base32 = { version = "0.5", optional = true }
|
||||
hmac = { version = "0.12", optional = true }
|
||||
sha1 = { version = "0.10", optional = true }
|
||||
urlencoding = { version = "2.1", optional = true }
|
||||
secrecy = { version = "0.8", features = ["serde"], optional = true }
|
||||
```
|
||||
|
||||
**Step 2: Add mfa feature to [features] section**
|
||||
|
||||
Change the `[features]` section (lines 114-117):
|
||||
```toml
|
||||
[features]
|
||||
default = ["minimal", "mfa"]
|
||||
minimal = []
|
||||
database = []
|
||||
mfa = ["dep:totp-rs", "dep:qrcode", "dep:image", "dep:base32", "dep:hmac", "dep:sha1", "dep:urlencoding", "dep:secrecy"]
|
||||
```
|
||||
|
||||
**Step 3: Gate the MFA module**
|
||||
|
||||
In `services/api_gateway/src/auth/mod.rs:22`, change:
|
||||
```rust
|
||||
pub mod mfa;
|
||||
```
|
||||
to:
|
||||
```rust
|
||||
#[cfg(feature = "mfa")]
|
||||
pub mod mfa;
|
||||
```
|
||||
|
||||
**Step 4: Gate MFA usage elsewhere**
|
||||
|
||||
Search for any `use` of `auth::mfa` or `crate::auth::mfa` outside the MFA module and wrap with `#[cfg(feature = "mfa")]`. Based on grep, only `auth/mod.rs` re-exports it.
|
||||
|
||||
Check MFA test files:
|
||||
- `tests/mfa_comprehensive.rs` — add `#![cfg(feature = "mfa")]` at top
|
||||
- `tests/mfa_enrollment_integration_test.rs` — add `#![cfg(feature = "mfa")]` at top
|
||||
- `tests/e2e_tests.rs` — check if MFA is used; if so, gate those sections
|
||||
|
||||
**Step 5: Verify build with and without MFA**
|
||||
|
||||
With MFA (default):
|
||||
```bash
|
||||
SQLX_OFFLINE=true cargo check -p api_gateway 2>&1 | tail -3
|
||||
```
|
||||
Expected: compiles
|
||||
|
||||
Without MFA:
|
||||
```bash
|
||||
SQLX_OFFLINE=true cargo check -p api_gateway --no-default-features --features minimal 2>&1 | tail -3
|
||||
```
|
||||
Expected: compiles (MFA module excluded, ~50 fewer transitive deps)
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add services/api_gateway/Cargo.toml services/api_gateway/src/auth/mod.rs \
|
||||
services/api_gateway/tests/mfa_comprehensive.rs \
|
||||
services/api_gateway/tests/mfa_enrollment_integration_test.rs
|
||||
git commit -m "feat(api_gateway): feature-gate MFA deps behind 'mfa' feature
|
||||
|
||||
MFA deps (qrcode, image, totp-rs, base32, hmac, sha1, urlencoding,
|
||||
secrecy) now behind optional 'mfa' feature, enabled by default. Builds
|
||||
without --features mfa skip ~50 transitive deps (rav1e, ravif, etc.)."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Final Verification
|
||||
|
||||
**Step 1: Full workspace check**
|
||||
|
||||
```bash
|
||||
SQLX_OFFLINE=true cargo check --workspace 2>&1 | tail -5
|
||||
```
|
||||
Expected: successful compilation
|
||||
|
||||
**Step 2: Verify ml zero warnings**
|
||||
|
||||
```bash
|
||||
SQLX_OFFLINE=true cargo check -p ml --lib 2>&1 | grep "^warning"
|
||||
```
|
||||
Expected: empty output
|
||||
|
||||
**Step 3: Count total warnings**
|
||||
|
||||
```bash
|
||||
SQLX_OFFLINE=true cargo check --workspace 2>&1 | grep "generated.*warning" | head -20
|
||||
```
|
||||
Record before/after for commit message.
|
||||
Reference in New Issue
Block a user