fix(simulation): PR #201 review round — wire extent clamp + coalescing tests
Hoshe finding 1: StepCanvasRequest.extent is no longer wire-trusted — clamp_step_canvas_extent enforces the D-255(b) canvas budget at the request boundary (per-axis cap 3840 defeating u32::MAX before any multiplication, then an aspect-preserving total-cell ceiling at the measured 3840x2160 = 8,294,400-cell workshop budget), mirroring the legacy carrier's clamp_window_n_v2 discipline; the Global rung ignores the wire extent entirely. StepCanvasResponse gains the extent echo field so a client can detect the clamp (the DistrictWindowLayer.n precedent — a pre-existing gap closed in passing, recorded on the ticket for T-1182). Seven new tests including an end-to-end u32::MAX request proving actual allocation respects the cap. Hoshe finding 2: submit_step_canvas coalescing now has the same two regression tests its submit_window sibling always had (same-key collapses to one pending item, different-key does not), exercising step_canvas_supersede_key. Targeted suites green: 1928 lib, acceptance gate 5/5, bridge_tcp 22/22, window_derivation_golden 6/6 byte-green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Claude Fable 5
@@ -1843,6 +1843,203 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// DeriveStepCanvas / submit_step_canvas coalescing (T-1181, D-255(c)(d))
|
||||
//
|
||||
// PR #201 review, Hoshe finding 2 — zero coverage before these tests.
|
||||
// `submit_step_canvas` is a near-verbatim sibling of `submit_window`
|
||||
// (same coalesce-in-place-on-supersede shape, same defensive
|
||||
// `dispatch_next` call), keyed on `step_canvas_supersede_key()`
|
||||
// (`(conn_id, body_id, rung)`) instead of `window_supersede_key()`'s
|
||||
// `(conn_id, body_id, granularity)`. Mirrors
|
||||
// `submit_window_coalesces_same_connection_and_body` +
|
||||
// `submit_window_does_not_coalesce_different_keys` exactly, substituting
|
||||
// `DeriveStepCanvas`/`rung` for `DeriveWindow`/`granularity`.
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
/// Build a `DeriveStepCanvas` work item pointing at a tiny test
|
||||
/// heightmap, mirroring `derive_window_at`'s fixture shape.
|
||||
fn derive_step_canvas_at(
|
||||
body_id: &str,
|
||||
conn_id: ConnectionId,
|
||||
center: (i64, i64),
|
||||
rung: crate::atlas::step_canvas::StepCanvasRung,
|
||||
) -> GenWorkItem {
|
||||
GenWorkItem::DeriveStepCanvas {
|
||||
body_id: body_id.to_string(),
|
||||
conn_id,
|
||||
heightmap_path: test_heightmap_path(),
|
||||
sea_level: 0.3,
|
||||
body_seed: SeedChain::for_body(42, body_id),
|
||||
body_params: Box::new(BodyParams {
|
||||
hydrosphere: Some("ocean".into()),
|
||||
atmosphere: Some("breathable".into()),
|
||||
planet_class: Some("temperate".into()),
|
||||
body_radius_km: Some(6371.0),
|
||||
..Default::default()
|
||||
}),
|
||||
placements: Vec::new(),
|
||||
rung,
|
||||
center,
|
||||
extent: (4, 4),
|
||||
min_wl_m: 0,
|
||||
}
|
||||
}
|
||||
|
||||
/// `submit_step_canvas` coalescing (T-1181, mirroring `submit_window`'s
|
||||
/// D-226 T-1124 amendment §1 discipline): two `DeriveStepCanvas` items
|
||||
/// for the SAME `(connection, body, rung)` queued while the pool is
|
||||
/// saturated collapse to ONE pending entry — the second submission
|
||||
/// replaces the first rather than queuing alongside it.
|
||||
#[test]
|
||||
fn submit_step_canvas_coalesces_same_connection_body_and_rung() {
|
||||
// Single-thread pool: the first item occupies the only worker, so
|
||||
// subsequent DeriveStepCanvas submissions stay in `pending` long
|
||||
// enough to inspect (same saturation trick
|
||||
// `submit_window_coalesces_same_connection_and_body` uses, and for
|
||||
// the same reason: `analyze()` is real measurable-latency cascade
|
||||
// work, unlike `FillChunk`, which could complete before the next
|
||||
// `submit_step_canvas` call even runs).
|
||||
let q = GenerationQueue::with_threads(1);
|
||||
q.submit(analyze("StepOccupier"), GenPriority::Low);
|
||||
|
||||
let conn = ConnectionId(21);
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"Canvas",
|
||||
conn,
|
||||
(0, 0),
|
||||
crate::atlas::step_canvas::StepCanvasRung::Chunk,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
assert_eq!(
|
||||
q.pending_count(),
|
||||
1,
|
||||
"one DeriveStepCanvas queued behind the saturating item"
|
||||
);
|
||||
|
||||
// A second DeriveStepCanvas for the SAME (connection, body, rung)
|
||||
// supersedes the first — pending count stays at 1, not 2.
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"Canvas",
|
||||
conn,
|
||||
(5_000, 5_000),
|
||||
crate::atlas::step_canvas::StepCanvasRung::Chunk,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
assert_eq!(
|
||||
q.pending_count(),
|
||||
1,
|
||||
"same (connection, body, rung) DeriveStepCanvas must supersede, not queue alongside"
|
||||
);
|
||||
|
||||
// Drain everything and confirm exactly one StepCanvasDerived for
|
||||
// "Canvas", carrying the SECOND (superseding) center — not the
|
||||
// first.
|
||||
std::thread::sleep(Duration::from_millis(150));
|
||||
let mut completions = q.drain_completions();
|
||||
std::thread::sleep(Duration::from_millis(150));
|
||||
completions.extend(q.drain_completions());
|
||||
|
||||
let step_canvas_completions: Vec<_> = completions
|
||||
.iter()
|
||||
.filter_map(|c| {
|
||||
if let GenCompletion::StepCanvasDerived {
|
||||
body_id, center, ..
|
||||
} = c
|
||||
{
|
||||
if body_id == "Canvas" {
|
||||
return Some(*center);
|
||||
}
|
||||
}
|
||||
None
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
step_canvas_completions.len(),
|
||||
1,
|
||||
"exactly one StepCanvasDerived for the coalesced body, not two"
|
||||
);
|
||||
assert_eq!(
|
||||
step_canvas_completions[0],
|
||||
(5_000, 5_000),
|
||||
"the surviving item must be the SECOND (superseding) submission"
|
||||
);
|
||||
}
|
||||
|
||||
/// `submit_step_canvas` does NOT coalesce across different connections,
|
||||
/// different bodies, or different rungs — only an exact `(connection,
|
||||
/// body, rung)` match supersedes. Combines `submit_window_does_not_
|
||||
/// coalesce_different_keys` (connection axis) and `submit_window_does_
|
||||
/// not_coalesce_different_granularity` (rung axis) into one test, since
|
||||
/// `step_canvas_supersede_key` is a flat 3-tuple with no separate
|
||||
/// legacy-vs-v2 field split to test independently the way `WindowGranularity`
|
||||
/// needed.
|
||||
#[test]
|
||||
fn submit_step_canvas_does_not_coalesce_different_keys() {
|
||||
let q = GenerationQueue::with_threads(1);
|
||||
// See `submit_step_canvas_coalesces_same_connection_body_and_rung`'s
|
||||
// comment on why the occupier must be `analyze()`, not `FillChunk`.
|
||||
q.submit(analyze("StepOccupier2"), GenPriority::Low);
|
||||
|
||||
// Different connections, same body, same rung — must NOT coalesce.
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"Shared",
|
||||
ConnectionId(1),
|
||||
(0, 0),
|
||||
crate::atlas::step_canvas::StepCanvasRung::District,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"Shared",
|
||||
ConnectionId(2),
|
||||
(1, 1),
|
||||
crate::atlas::step_canvas::StepCanvasRung::District,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
assert_eq!(
|
||||
q.pending_count(),
|
||||
2,
|
||||
"different connections requesting the same body+rung must NOT coalesce"
|
||||
);
|
||||
|
||||
// Same connection, same body, but DIFFERENT rung — must NOT
|
||||
// coalesce (District vs. Chunk are separate in-flight slots).
|
||||
let conn = ConnectionId(23);
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"RungBody",
|
||||
conn,
|
||||
(0, 0),
|
||||
crate::atlas::step_canvas::StepCanvasRung::District,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
q.submit_step_canvas(
|
||||
derive_step_canvas_at(
|
||||
"RungBody",
|
||||
conn,
|
||||
(0, 0),
|
||||
crate::atlas::step_canvas::StepCanvasRung::Chunk,
|
||||
),
|
||||
GenPriority::Immediate,
|
||||
);
|
||||
assert_eq!(
|
||||
q.pending_count(),
|
||||
4,
|
||||
"same (connection, body) but DIFFERENT rung must NOT coalesce — \
|
||||
District and Chunk are separate in-flight slots (2 from the \
|
||||
connection-axis case above + 2 more here)"
|
||||
);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// TerrainAnalysisCache (T-1137, PR #187 review — Tyre C1)
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user