Skip to content

Commit c60529d

Browse files
refactor: Phase 4.5 — final M8 splits bring every file under 300 prod LoC
Pre-publish file-split pass complete. The 7 files that survived Phase 4.4 all now have production code under the 300-line convention. Sub-module extractions (4 parallel-agent splits): scenario/spike.rs (331 → 217 prod): - `scenario/spike/phase.rs` (~127 lines) holds `drive_phase`, the per- phase worker loop (warmup → spike → cooldown). `spike.rs` keeps the `Spike` struct + `impl Scenario` + plan-summary helper. cli/cmd_cross.rs (322 → 189 prod): - `cmd_cross/render.rs` (~144 lines) holds `render_markdown` plus its helpers (`column_label`, `format_duration_ms`, `push_metric_row`) and the two rendering-specific tests. `cmd_cross.rs` keeps `CrossScenario`, `CrossArgs`, `ServerRow`, `run`, `run_one`, and the parse / split- server-command tests. protocol/transport/sse.rs (311 → 263 prod, no test mod): - `transport/sse/reader.rs` (~81 lines) holds `IdProbe`, `extract_id`, and a generic `spawn_reader<S, E>` wrapping the tokio::spawn loop. `sse.rs` keeps `SseTransport` + `impl Transport` + `connect` + handshake/post helpers + all module constants. scenario/fuzzer.rs (338 → 277 prod): - `scenario/fuzzer/classify.rs` (~69 lines) holds `classify_err` (mapping `SessionError` → `FuzzClass`) and `push_report_notes` (renders the aggregated `FuzzReport` into outcome notes). `fuzzer.rs` keeps the `Fuzzer` struct + `Scenario` impl + tests (which call the helpers via `super::*`). Inline extractions + doc-comment trim: metrics/mod.rs (328 → ~210 prod): - `metrics/per_tool.rs` (~33 lines) holds `PerToolState` (the per-tool histogram + counters wrapper). Module-doc trimmed from 33 lines to 13. Tests retained. scenario/soak.rs (312 → ~190 prod): - Module-doc trimmed from 36 lines to 16. No code moved (helper extraction was already done in Phase 4.2). serve/mod.rs (304 → ~280 prod): - Module-doc trimmed from 21 lines to 8. No code moved. CHANGELOG + POST_PUBLISH_ISSUES.md updated to reflect the M8 milestone as complete (was queued for v0.2). The post-publish issues file now records what shipped per wave so future contributors see the per-file "why was it split this way" trail. 264 tests pass, cargo fmt + clippy -D warnings clean. No public API change.
1 parent 01f2f3d commit c60529d

14 files changed

Lines changed: 533 additions & 508 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
125125
- `DEFAULT_LEAK_THRESHOLD_MB_PER_SEC` → use `DEFAULT_LATENCY_DRIFT_MS_PER_SEC`. The old constant remains as an alias for one release and will be removed in v0.2.0.
126126

127127
### Notes
128-
- 11 source files have production code (excluding `#[cfg(test)] mod tests`) over the 300-line convention. They are split into "split candidates" (genuine refactor opportunities) and "borderline" (just over). See `POST_PUBLISH_ISSUES.md` for the per-file plan; tracked under M8.
128+
- ✅ The M8 file-split pass completed in the pre-publish review. All source files have production code (excluding `#[cfg(test)] mod tests`) under the 300-line convention. See `POST_PUBLISH_ISSUES.md` for the per-wave summary of what split where.
129129
- `serve` and `tui` modules will move behind cargo feature flags in a future release to keep the default build slim.
130130
- HTTP / SSE transport host-allowlist for SSRF defense is deferred. Currently mitigated by `Policy::none()` on redirects; the allowlist is operator-facing config and will land alongside the broader transport-hardening pass.
131131

POST_PUBLISH_ISSUES.md

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -76,34 +76,19 @@ Each block below is in `gh issue create` shape — copy-paste-ready once the rep
7676

7777
Files where production code (excluding `#[cfg(test)] mod tests`) exceeds the 300-line convention. The bracketed numbers are production LoC / total LoC.
7878

79-
### `chore(refactor): split files > 300 production LoC (M8)`
79+
### ~~`chore(refactor): split files > 300 production LoC (M8)`~~ **COMPLETED in v0.1.0 pre-publish**
8080

81-
**Body:**
82-
> Splits to land in v0.2. Each line is a separate sub-task; some are easy extractions, some are genuine module restructures. Per-file plan:
83-
>
84-
> **Wave 1 — extract a helper (low risk):**
85-
> - [ ] `src/scenario/soak.rs` (358 prod LoC) — extract `detect_leak` + `LinReg` into `scenario/soak/leak_detect.rs`
86-
> - [ ] `src/report/html.rs` (336) — pull CSS constant + SVG chart renderer into `report/html/{css,chart}.rs`
87-
> - [ ] `src/scenario/spike.rs` (331) — barely over; trim or leave
88-
>
89-
> **Wave 2 — per-component split (medium):**
90-
> - [ ] `crates/mcp-loadtest-cli/src/cmd_compare.rs` (503) — split into `cmd_compare/{types,diff,render,classify}.rs`
91-
> - [ ] `src/scenario/fuzzer.rs` (488) — split into `fuzzer/{payloads,driver,classify}.rs`
92-
> - [ ] `src/serve/tools.rs` (509) — split into `serve/tools/{deadlock_probe,sustained_load,compare_runs}.rs` + `mod.rs`
93-
>
94-
> **Wave 3 — orchestrator / heavy types:**
95-
> - [ ] `src/run.rs` (447) — split into `run/{orchestrator,thresholds}.rs`
96-
> - [ ] `src/metrics/mod.rs` (426) — split into `metrics/{types,recorder,outcomes}.rs`
97-
> - [ ] `src/config.rs` (376) — split into `config/{schema,validate,example}.rs`
98-
> - [ ] `crates/mcp-loadtest-cli/src/main.rs` (434) — extract subcommand handlers into `main/cmd_*.rs`
99-
> - [ ] `src/serve/mod.rs` (304) — barely over; leave or trim
100-
>
101-
> Each split must:
102-
> - preserve the public API path via `pub use` re-export from `mod.rs`
103-
> - not break existing tests
104-
> - keep `cargo doc` warning-free
105-
106-
**Labels:** `tech-debt`, `refactor`, `v0.2`
81+
All 11 originally-flagged files now have production code under 300 lines. Splits landed across four commits in the pre-publish review:
82+
83+
- Wave 1: `scenario/soak.rs` (358→196 via `soak/leak_detect.rs`), `report/html.rs` (336→212 via `html/{css,chart}.rs`), `scenario/spike.rs` (331→217 via `spike/phase.rs`).
84+
- Wave 2: `cmd_compare.rs` (503→100 via `cmd_compare/{types,diff,render}.rs`), `scenario/fuzzer.rs` (488→277 via `fuzzer/{payloads,classify}.rs`), `serve/tools.rs` (509→130 via `serve/tools/{deadlock_probe,sustained_load,compare_runs}.rs`).
85+
- Wave 3: `run.rs` (447→293 via `run/thresholds.rs`), `metrics/mod.rs` (426→<300 via `metrics/{types,per_tool}.rs` + module-doc trim), `config.rs` (376→217 via `config/{validate,example}.rs`), `main.rs` (434→208 via `cmd_run.rs` + `cmd_deadlock.rs` + `emit.rs`).
86+
- Wave 4 (trim): `metrics/mod.rs`, `scenario/soak.rs`, `serve/mod.rs` — module-doc trim brought them under the 300 cap.
87+
- `sse.rs` (311→263 via `sse/reader.rs`), `cmd_cross.rs` (322→189 via `cmd_cross/render.rs`).
88+
89+
Public API paths preserved via `pub use` re-exports throughout. 264 tests pass, `cargo fmt --check` + `cargo clippy -D warnings` clean.
90+
91+
**Labels:** ~~`tech-debt`~~ — done.
10792

10893
---
10994

crates/mcp-loadtest-cli/src/cmd_cross.rs

Lines changed: 3 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use std::time::Duration;
1515

1616
use anyhow::{Context, Result, anyhow};
1717
use futures::StreamExt;
18-
use mcp_loadtest::analysis::grading::{GradingProfile, grade};
1918
use mcp_loadtest::config::{
2019
Config, OutputConfig, ScenarioConfig, ServerConfig, ThresholdsConfig, split_server_command,
2120
};
@@ -26,6 +25,9 @@ use mcp_loadtest::scenario::deadlock_probe::DeadlockProbe;
2625
use mcp_loadtest::scenario::sustained::Sustained;
2726
use serde_json::{Value, json};
2827

28+
mod render;
29+
use render::render_markdown;
30+
2931
/// Cap on how many servers we spawn in parallel from `cross`. Each spawn
3032
/// invokes `tokio::process::Command::spawn`; on Windows hitting the
3133
/// JobObject limit when N is large causes spawns to fail. 8 chosen so a
@@ -185,140 +187,6 @@ async fn run_one(server: &str, args: &CrossArgs, args_value: &Value) -> Result<R
185187
Ok(report)
186188
}
187189

188-
// ---- rendering ----------------------------------------------------------
189-
190-
/// Render the cross-comparison as a Markdown report.
191-
fn render_markdown(rows: &[ServerRow], args: &CrossArgs) -> String {
192-
let scenario_label = match args.scenario {
193-
CrossScenario::Sustained => "sustained",
194-
CrossScenario::DeadlockProbe => "deadlock_probe",
195-
};
196-
let mut out = String::new();
197-
out.push_str("# Cross-server comparison\n\n");
198-
out.push_str(&format!(
199-
"- Scenario: `{}`\n- Tool: `{}`\n- Duration: {}s per server\n- Servers: {}\n\n",
200-
scenario_label,
201-
args.tool,
202-
args.duration.as_secs_f64(),
203-
rows.len(),
204-
));
205-
206-
// Servers list — separate from the metrics table so failed servers still
207-
// show up clearly above and below.
208-
out.push_str("## Servers\n\n");
209-
for (idx, row) in rows.iter().enumerate() {
210-
let label = column_label(idx);
211-
let status = match &row.result {
212-
Ok(_) => "ok",
213-
Err(_) => "FAILED",
214-
};
215-
out.push_str(&format!("- **{label}**: `{}` — {status}\n", row.command));
216-
}
217-
out.push('\n');
218-
219-
// Metrics table — one column per server, one row per metric.
220-
out.push_str("## Metrics\n\n");
221-
222-
// Header row.
223-
out.push_str("| Metric |");
224-
for (idx, _) in rows.iter().enumerate() {
225-
out.push_str(&format!(" {} |", column_label(idx)));
226-
}
227-
out.push('\n');
228-
out.push_str("|---|");
229-
for _ in rows {
230-
out.push_str("---:|");
231-
}
232-
out.push('\n');
233-
234-
let profile = GradingProfile::default_general();
235-
236-
// Per-row formatters keep the table easy to scan; each function plucks
237-
// a different field out of a Report (or returns "n/a" on a failed run).
238-
push_metric_row(&mut out, "p50 latency", rows, |r| {
239-
format_duration_ms(r.metrics.latency.p50)
240-
});
241-
push_metric_row(&mut out, "p95 latency", rows, |r| {
242-
format_duration_ms(r.metrics.latency.p95)
243-
});
244-
push_metric_row(&mut out, "p99 latency", rows, |r| {
245-
format_duration_ms(r.metrics.latency.p99)
246-
});
247-
push_metric_row(&mut out, "max latency", rows, |r| {
248-
format_duration_ms(r.metrics.latency.max)
249-
});
250-
push_metric_row(&mut out, "RPS", rows, |r| {
251-
format!("{:.2}", r.metrics.throughput.requests_per_sec)
252-
});
253-
push_metric_row(&mut out, "error rate", rows, |r| {
254-
let total = r.metrics.throughput.total_requests;
255-
let success = r.metrics.throughput.successful_requests;
256-
if total == 0 {
257-
"0.00%".to_string()
258-
} else {
259-
let errors = total.saturating_sub(success);
260-
format!("{:.2}%", errors as f64 / total as f64 * 100.0)
261-
}
262-
});
263-
push_metric_row(&mut out, "deadlocks", rows, |r| {
264-
r.scenario_outcome.deadlock_count.to_string()
265-
});
266-
push_metric_row(&mut out, "Grade", rows, |r| {
267-
let g = grade(r, &profile);
268-
g.overall.name().to_string()
269-
});
270-
271-
// Errors section — list any per-server failures with their full message
272-
// so the user can debug without spelunking through stderr.
273-
let failures: Vec<&ServerRow> = rows.iter().filter(|r| r.result.is_err()).collect();
274-
if !failures.is_empty() {
275-
out.push_str("\n## Errors\n\n");
276-
for row in failures {
277-
if let Err(e) = &row.result {
278-
out.push_str(&format!("- `{}`: {e:#}\n", row.command));
279-
}
280-
}
281-
}
282-
283-
out
284-
}
285-
286-
/// Push one row of the metrics table. `extract` runs only on successful
287-
/// reports; failed servers get `"n/a"`.
288-
fn push_metric_row<F>(out: &mut String, label: &str, rows: &[ServerRow], extract: F)
289-
where
290-
F: Fn(&Report) -> String,
291-
{
292-
out.push_str(&format!("| {label} |"));
293-
for row in rows {
294-
let cell = match &row.result {
295-
Ok(report) => extract(report),
296-
Err(_) => "n/a".to_string(),
297-
};
298-
out.push_str(&format!(" {cell} |"));
299-
}
300-
out.push('\n');
301-
}
302-
303-
/// Letter label for a column: `A`, `B`, ..., then `S1`, `S2`, ... once we
304-
/// run out of single letters. Cross-comparing more than 26 servers in one
305-
/// table is unlikely but the fallback keeps headers unambiguous.
306-
fn column_label(idx: usize) -> String {
307-
if idx < 26 {
308-
let c = (b'A' + idx as u8) as char;
309-
c.to_string()
310-
} else {
311-
format!("S{}", idx + 1)
312-
}
313-
}
314-
315-
/// Format a `Duration` as millisecond-precision text. Mirrors the helper in
316-
/// `run.rs` — small enough to inline here rather than expose pub from the lib.
317-
fn format_duration_ms(d: Duration) -> String {
318-
let total_ms = d.as_secs_f64() * 1000.0;
319-
format!("{total_ms:.2}ms")
320-
}
321-
322190
#[cfg(test)]
323191
mod tests {
324192
use super::*;
@@ -363,18 +231,4 @@ mod tests {
363231
fn split_server_command_empty_errors() {
364232
assert!(split_server_command("").is_err());
365233
}
366-
367-
#[test]
368-
fn column_label_first_letters() {
369-
assert_eq!(column_label(0), "A");
370-
assert_eq!(column_label(1), "B");
371-
assert_eq!(column_label(25), "Z");
372-
assert_eq!(column_label(26), "S27");
373-
}
374-
375-
#[test]
376-
fn format_duration_ms_two_decimals() {
377-
assert_eq!(format_duration_ms(Duration::from_millis(1)), "1.00ms");
378-
assert_eq!(format_duration_ms(Duration::from_micros(1234)), "1.23ms");
379-
}
380234
}
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
//! Markdown rendering for the `cross` subcommand.
2+
//!
3+
//! Split out of `cmd_cross.rs` to keep that file under the 300-line
4+
//! production-code convention. Pure formatting — no I/O, no async.
5+
6+
use std::time::Duration;
7+
8+
use mcp_loadtest::analysis::grading::{GradingProfile, grade};
9+
use mcp_loadtest::report::Report;
10+
11+
use super::{CrossArgs, CrossScenario, ServerRow};
12+
13+
/// Render the cross-comparison as a Markdown report.
14+
pub(super) fn render_markdown(rows: &[ServerRow], args: &CrossArgs) -> String {
15+
let scenario_label = match args.scenario {
16+
CrossScenario::Sustained => "sustained",
17+
CrossScenario::DeadlockProbe => "deadlock_probe",
18+
};
19+
let mut out = String::new();
20+
out.push_str("# Cross-server comparison\n\n");
21+
out.push_str(&format!(
22+
"- Scenario: `{}`\n- Tool: `{}`\n- Duration: {}s per server\n- Servers: {}\n\n",
23+
scenario_label,
24+
args.tool,
25+
args.duration.as_secs_f64(),
26+
rows.len(),
27+
));
28+
29+
// Servers list — separate from the metrics table so failed servers still
30+
// show up clearly above and below.
31+
out.push_str("## Servers\n\n");
32+
for (idx, row) in rows.iter().enumerate() {
33+
let label = column_label(idx);
34+
let status = match &row.result {
35+
Ok(_) => "ok",
36+
Err(_) => "FAILED",
37+
};
38+
out.push_str(&format!("- **{label}**: `{}` — {status}\n", row.command));
39+
}
40+
out.push('\n');
41+
42+
// Metrics table — one column per server, one row per metric.
43+
out.push_str("## Metrics\n\n");
44+
45+
// Header row.
46+
out.push_str("| Metric |");
47+
for (idx, _) in rows.iter().enumerate() {
48+
out.push_str(&format!(" {} |", column_label(idx)));
49+
}
50+
out.push('\n');
51+
out.push_str("|---|");
52+
for _ in rows {
53+
out.push_str("---:|");
54+
}
55+
out.push('\n');
56+
57+
let profile = GradingProfile::default_general();
58+
59+
// Per-row formatters keep the table easy to scan; each function plucks
60+
// a different field out of a Report (or returns "n/a" on a failed run).
61+
push_metric_row(&mut out, "p50 latency", rows, |r| {
62+
format_duration_ms(r.metrics.latency.p50)
63+
});
64+
push_metric_row(&mut out, "p95 latency", rows, |r| {
65+
format_duration_ms(r.metrics.latency.p95)
66+
});
67+
push_metric_row(&mut out, "p99 latency", rows, |r| {
68+
format_duration_ms(r.metrics.latency.p99)
69+
});
70+
push_metric_row(&mut out, "max latency", rows, |r| {
71+
format_duration_ms(r.metrics.latency.max)
72+
});
73+
push_metric_row(&mut out, "RPS", rows, |r| {
74+
format!("{:.2}", r.metrics.throughput.requests_per_sec)
75+
});
76+
push_metric_row(&mut out, "error rate", rows, |r| {
77+
let total = r.metrics.throughput.total_requests;
78+
let success = r.metrics.throughput.successful_requests;
79+
if total == 0 {
80+
"0.00%".to_string()
81+
} else {
82+
let errors = total.saturating_sub(success);
83+
format!("{:.2}%", errors as f64 / total as f64 * 100.0)
84+
}
85+
});
86+
push_metric_row(&mut out, "deadlocks", rows, |r| {
87+
r.scenario_outcome.deadlock_count.to_string()
88+
});
89+
push_metric_row(&mut out, "Grade", rows, |r| {
90+
let g = grade(r, &profile);
91+
g.overall.name().to_string()
92+
});
93+
94+
// Errors section — list any per-server failures with their full message
95+
// so the user can debug without spelunking through stderr.
96+
let failures: Vec<&ServerRow> = rows.iter().filter(|r| r.result.is_err()).collect();
97+
if !failures.is_empty() {
98+
out.push_str("\n## Errors\n\n");
99+
for row in failures {
100+
if let Err(e) = &row.result {
101+
out.push_str(&format!("- `{}`: {e:#}\n", row.command));
102+
}
103+
}
104+
}
105+
106+
out
107+
}
108+
109+
/// Push one row of the metrics table. `extract` runs only on successful
110+
/// reports; failed servers get `"n/a"`.
111+
fn push_metric_row<F>(out: &mut String, label: &str, rows: &[ServerRow], extract: F)
112+
where
113+
F: Fn(&Report) -> String,
114+
{
115+
out.push_str(&format!("| {label} |"));
116+
for row in rows {
117+
let cell = match &row.result {
118+
Ok(report) => extract(report),
119+
Err(_) => "n/a".to_string(),
120+
};
121+
out.push_str(&format!(" {cell} |"));
122+
}
123+
out.push('\n');
124+
}
125+
126+
/// Letter label for a column: `A`, `B`, ..., then `S1`, `S2`, ... once we
127+
/// run out of single letters. Cross-comparing more than 26 servers in one
128+
/// table is unlikely but the fallback keeps headers unambiguous.
129+
pub(super) fn column_label(idx: usize) -> String {
130+
if idx < 26 {
131+
let c = (b'A' + idx as u8) as char;
132+
c.to_string()
133+
} else {
134+
format!("S{}", idx + 1)
135+
}
136+
}
137+
138+
/// Format a `Duration` as millisecond-precision text. Mirrors the helper in
139+
/// `run.rs` — small enough to inline here rather than expose pub from the lib.
140+
fn format_duration_ms(d: Duration) -> String {
141+
let total_ms = d.as_secs_f64() * 1000.0;
142+
format!("{total_ms:.2}ms")
143+
}
144+
145+
#[cfg(test)]
146+
mod tests {
147+
use super::*;
148+
149+
#[test]
150+
fn column_label_first_letters() {
151+
assert_eq!(column_label(0), "A");
152+
assert_eq!(column_label(1), "B");
153+
assert_eq!(column_label(25), "Z");
154+
assert_eq!(column_label(26), "S27");
155+
}
156+
157+
#[test]
158+
fn format_duration_ms_two_decimals() {
159+
assert_eq!(format_duration_ms(Duration::from_millis(1)), "1.00ms");
160+
assert_eq!(format_duration_ms(Duration::from_micros(1234)), "1.23ms");
161+
}
162+
}

0 commit comments

Comments
 (0)