fix(simulation): PR #191 review round — n-clamp mirror, min_wl band quantization, coalescing coverage, fixture consumer

All seven Hoshe/Tyre findings addressed, none retracted:
- n-clamp/echo/staleness triangle (Tyre C1): client _clamp_window_n_mirror
  (bit-for-bit twin of the server clamp, canonicalize_district_center
  precedent) applied before _n is stored/sent; server test pins the
  quarter n=32 -> echo 16 contract.
- min_wl band quantization (Hoshe 1/Tyre C3): quantize_min_wl_m snaps to
  MIN_WL_BANDS_M {0, 32768, 16384, 8192, 4096} before cache key and echo
  (design doc §5's unbounded-key fix), reusing the one true
  OCTAVE_WAVELENGTHS_M array; docstrings now state the server-quantizes/
  client-sends-raw split; same-band cache-sharing test.
- coalescing granularity axis (Hoshe 2): two tests pin different-
  granularity requests as separate in-flight slots and same-granularity
  coalescing unchanged.
- orphaned fixture (Hoshe 3): test_protocol.gd consumer decodes
  atlas_response_ready_with_window.msgpack through the real IPC path and
  asserts the new fields.
- atlas_window_request coverage (Hoshe 4): new test file — stale-drop on
  granularity mismatch, old-server-shape defaults accepted, clamp mirror
  formula + wiring. First draft's quarter-via-request_now test would have
  passed for the wrong reason (request_now resets granularity by design
  until T-1153) — split into formula pin + reachable-path wiring proof.
- granularity type seam (Tyre C2): field + resolver docstrings state
  finer-only integer multiples with resolve_window_granularity as the
  single widening point; matching contract note added to the D-226
  T-1143-rulings amendment.

cargo --lib 1807/1807; goldens bit-identical; gdlint clean.
This commit is contained in:
2026-07-22 00:47:53 +02:00
parent 3e87fd5b4f
commit 0159a63cc2
8 changed files with 670 additions and 14 deletions
+9 -1
View File
@@ -22,7 +22,15 @@ use crate::seed::splitmix64;
/// Mid-scale octave wavelengths in metres — the 240 km band. Coarsest first.
/// Below the finest (~4 km) the district→voxel layers own the detail; above the
/// coarsest (~33 km) the heightmap itself carries the shape.
const OCTAVE_WAVELENGTHS_M: [f64; 4] = [32_768.0, 16_384.0, 8_192.0, 4_096.0];
///
/// `pub(crate)`: also the quantization band set for `layer_proxy`'s
/// `window_min_wl_m` (T-1150, zoom ladder design doc §5 — "quantize
/// `min_wl_m` to a small fixed set of bands per rung, **matching the rung's
/// own octave bands**"). District/quarter windows both derive via
/// `terrain_detail`, so this IS "the rung's own octave bands" for both rungs
/// today — one array, no duplicated magic numbers that could drift out of
/// sync with the actual cutoff behavior.
pub(crate) const OCTAVE_WAVELENGTHS_M: [f64; 4] = [32_768.0, 16_384.0, 8_192.0, 4_096.0];
/// Voxel-tier octave wavelengths in metres — the ≈0.131 km **sub-district** band
/// (all finer than the 2 km district planning unit) that the district-tier
+96 -2
View File
@@ -1144,8 +1144,26 @@ mod tests {
// -------------------------------------------------------------------
/// Build a `DeriveWindow` work item pointing at a tiny test heightmap,
/// mirroring `analyze()`'s fixture shape.
/// mirroring `analyze()`'s fixture shape. `granularity` defaults to
/// district (matching every pre-T-1150 call site) via
/// `derive_window_at()` below — extended (PR #191 review, Hoshe 2) so
/// coalescing tests can exercise the granularity axis of
/// `window_supersede_key()` without a second near-duplicate helper.
fn derive_window(body_id: &str, conn_id: ConnectionId, center: DistrictPos) -> GenWorkItem {
derive_window_at(
body_id,
conn_id,
center,
crate::atlas::layer_proxy::WINDOW_GRANULARITY_DISTRICT,
)
}
fn derive_window_at(
body_id: &str,
conn_id: ConnectionId,
center: DistrictPos,
granularity: u32,
) -> GenWorkItem {
GenWorkItem::DeriveWindow {
body_id: body_id.to_string(),
conn_id,
@@ -1161,7 +1179,7 @@ mod tests {
}),
center,
n: 4,
granularity: crate::atlas::layer_proxy::WINDOW_GRANULARITY_DISTRICT,
granularity,
min_wl_m: 0,
}
}
@@ -1282,6 +1300,82 @@ mod tests {
);
}
/// **PR #191 review, Hoshe 2 — zero coverage before this test.**
/// `window_supersede_key()`'s doc claims district and quarter requests
/// for the SAME `(connection, body)` are separate in-flight slots (the
/// key is `(conn_id, body_id, granularity)`, not `(conn_id, body_id)`).
/// Two submissions for the same connection+body but DIFFERENT
/// granularity must NOT coalesce — both survive as independent pending
/// items.
#[test]
fn submit_window_does_not_coalesce_different_granularity() {
let q = GenerationQueue::with_threads(1);
// See `submit_window_coalesces_same_connection_and_body`'s comment on
// why the occupier must be `analyze()`, not `FillChunk`.
q.submit(analyze("Occupier3"), GenPriority::Low);
let conn = ConnectionId(9);
q.submit_window(
derive_window_at(
"GranBody",
conn,
(0, 0),
crate::atlas::layer_proxy::WINDOW_GRANULARITY_DISTRICT,
),
GenPriority::Immediate,
);
q.submit_window(
derive_window_at(
"GranBody",
conn,
(0, 0),
crate::atlas::layer_proxy::WINDOW_GRANULARITY_QUARTER,
),
GenPriority::Immediate,
);
assert_eq!(
q.pending_count(),
2,
"same (connection, body) but DIFFERENT granularity must NOT coalesce — \
district and quarter are separate in-flight slots"
);
}
/// The coalescing-DOES-happen counterpart to the test above: two
/// submissions for the SAME `(connection, body, granularity)` still
/// collapse to one pending item — confirms the granularity axis didn't
/// accidentally loosen the existing same-key coalescing behavior.
#[test]
fn submit_window_coalesces_same_connection_body_and_granularity() {
let q = GenerationQueue::with_threads(1);
q.submit(analyze("Occupier4"), GenPriority::Low);
let conn = ConnectionId(11);
q.submit_window(
derive_window_at(
"SameGranBody",
conn,
(0, 0),
crate::atlas::layer_proxy::WINDOW_GRANULARITY_QUARTER,
),
GenPriority::Immediate,
);
q.submit_window(
derive_window_at(
"SameGranBody",
conn,
(5, 5),
crate::atlas::layer_proxy::WINDOW_GRANULARITY_QUARTER,
),
GenPriority::Immediate,
);
assert_eq!(
q.pending_count(),
1,
"same (connection, body, granularity) must still coalesce to one pending item"
);
}
// -------------------------------------------------------------------
// TerrainAnalysisCache (T-1137, PR #187 review — Tyre C1)
// -------------------------------------------------------------------
+311 -6
View File
@@ -66,6 +66,16 @@ pub const WIRE_CAP_CELLS: u32 = 4_096;
/// Resolve a wire-supplied `window_granularity` value to one of the two legal
/// granularities, clamping anything else down to district spacing — **never
/// trust the wire** (same posture as `window_n`/`normalize_window_center`).
///
/// **This is THE single widening point (Tyre C2, PR #191 review).** The
/// field only ever expresses finer-than-district integer multiples (see
/// [`AtlasLayerRequest::window_granularity`]'s doc for the full type-seam
/// contract); adding a future finer rung means adding its legal value here
/// and nowhere else. Do NOT add a value < 1 or attempt to encode
/// coarser-than-district rungs (region/orbital) through this function —
/// design doc §5/§9 R5 requires a signed/log-scale or enum redesign for that
/// direction, which this `u32` cannot express regardless of what this
/// function returns.
fn resolve_window_granularity(raw: u32) -> u32 {
if raw == WINDOW_GRANULARITY_QUARTER {
WINDOW_GRANULARITY_QUARTER
@@ -79,6 +89,16 @@ fn resolve_window_granularity(raw: u32) -> u32 {
/// ([`WIRE_CAP_CELLS`]) — `window_n² × granularity² ≤ WIRE_CAP_CELLS` (T-1150
/// design doc §3). Applied AFTER the per-axis clamp so a request that already
/// satisfies `DISTRICT_WINDOW_MAX_N` still shrinks further at granularity 4.
///
/// **This clamp is echoed, not silently applied** — `serve_district_window`
/// puts the CLAMPED `n` into `DistrictWindowLayer.n`, so a client that
/// requests an oversized `n` gets back a smaller one. Any client-side
/// staleness guard comparing its own requested `n` against the echo MUST
/// mirror this exact function first (PR #191 review, Tyre C1) — see
/// `atlas_window_request.gd`'s `_clamp_window_n_mirror()`, which matches this
/// function bit-for-bit, the same load-bearing-mirror pattern
/// `canonicalize_district_center()` (`atlas_descend_geometry.gd`) already
/// uses for `normalize_window_center`.
fn clamp_window_n(raw_n: u32, granularity: u32) -> u32 {
let n = raw_n.clamp(1, DISTRICT_WINDOW_MAX_N);
let g = granularity.max(1);
@@ -86,6 +106,49 @@ fn clamp_window_n(raw_n: u32, granularity: u32) -> u32 {
n.min(cap_n.floor().max(1.0) as u32)
}
/// Quantized `window_min_wl_m` bands (T-1150, zoom ladder design doc §5):
/// `0` (no cutoff) plus every entry of
/// [`crate::atlas::detail_scatter::OCTAVE_WAVELENGTHS_M`] — the SAME array
/// `terrain_detail`'s octave sum truncates against (both district and
/// quarter rungs derive via `terrain_detail`, so this is genuinely "the
/// rung's own octave bands", not a second independently-chosen scale).
/// Descending order except the leading `0.0` sentinel, matched by
/// `quantize_min_wl_m`'s scan below.
const MIN_WL_BANDS_M: [f64; 5] = [
0.0,
crate::atlas::detail_scatter::OCTAVE_WAVELENGTHS_M[0],
crate::atlas::detail_scatter::OCTAVE_WAVELENGTHS_M[1],
crate::atlas::detail_scatter::OCTAVE_WAVELENGTHS_M[2],
crate::atlas::detail_scatter::OCTAVE_WAVELENGTHS_M[3],
];
/// Snap a wire-supplied `window_min_wl_m` to the nearest fixed band in
/// [`MIN_WL_BANDS_M`] (T-1150, design doc §5's gap-fix): "as specified,
/// `window_min_wl_m` is viewport-continuous while the cache key/echo tuple is
/// `(body, center, n, granularity)` — same key, different `min_wl`, would
/// silently collide. Fix: quantize `min_wl_m` to a small fixed set of bands
/// ... and add the quantized band to both the echo and the cache key." This
/// is that quantization, applied unconditionally to every request before it
/// touches either the cache key or the `DeriveWindow` work item — **never
/// the raw wire value past this point**, same discipline as `window_n`'s
/// clamp and `window_center`'s normalization. Nearest-band snap (ties round
/// to the coarser/lower band, i.e. `<=` on the running best distance) keeps
/// the mapping total and deterministic for any `u32` input, including values
/// far outside the octave range (e.g. `u32::MAX` snaps to the coarsest band).
fn quantize_min_wl_m(raw: u32) -> u32 {
let raw_f = raw as f64;
let mut best = MIN_WL_BANDS_M[0];
let mut best_dist = (raw_f - best).abs();
for &band in &MIN_WL_BANDS_M[1..] {
let dist = (raw_f - band).abs();
if dist < best_dist {
best = band;
best_dist = dist;
}
}
best as u32
}
/// A client request for a body's generation layers (D-225), extended with an
/// optional district-resolution window query (D-226 T-1124 amendment §1, T-1137).
///
@@ -115,15 +178,38 @@ pub struct AtlasLayerRequest {
/// [`WINDOW_GRANULARITY_QUARTER`]. Resolved via
/// [`resolve_window_granularity`] — **never trusted from the wire**,
/// unrecognized values fall back to district.
///
/// **Type seam (Tyre C2, PR #191 review):** this field expresses ONLY
/// finer-than-district integer multiples of the district spacing — `1`
/// and `4` are legal today, and each new finer rung (e.g. a future
/// block/tile value) is a deliberate widening of
/// [`resolve_window_granularity`]'s whitelist, the single point where
/// that widening happens. It CANNOT express coarser-than-district rungs
/// (region/orbital, granularity < 1) — reusing this field for those is
/// explicitly out of scope; design doc §5/§9 R5 requires a
/// signed/log-scale value or an explicit rung enum instead. Do not smuggle
/// a "granularity 0 means region" convention into this `u32` — that is
/// the redesign R5 already flags, not a value to add here.
#[serde(default)]
pub window_granularity: u32,
/// Octave cutoff for the invented-terrain scatter (T-1149's
/// `min_wavelength_m`), in whole metres. `0` (absent) = no cutoff = the
/// pre-T-1150 behavior. Threaded straight to `derive_at_metres` as
/// `min_wl as f64` — no client-side quantization band is enforced here
/// (the design doc's §5 quantized-band gap-fix is a client-request-shaping
/// concern; the server takes whatever whole-metre value it's given and
/// keys the cache on it verbatim, same posture as `window_n`).
/// pre-T-1150 behavior.
///
/// **Quantization contract (Hoshe 1 / Tyre C3, PR #191 review; design doc
/// §5):** the wire value here is an UNQUANTIZED, unclamped raw passthrough
/// — client codecs may send any `u32`. The SERVER is the one place
/// quantization happens: `serve_district_window` snaps every request's
/// value to the nearest fixed band in
/// [`MIN_WL_BANDS_M`] via [`quantize_min_wl_m`] BEFORE it ever touches
/// the cache key or the `DeriveWindow` work item, and the QUANTIZED value
/// (not this raw field) is what gets echoed back on
/// `DistrictWindowLayer.min_wl_m` and used as the cache key component.
/// This closes the §5 gap: without quantization, two requests differing
/// only in a continuous-valued `min_wl_m` would silently miss each
/// other's cache entries (the unbounded-key-space problem §5 exists to
/// close) — the client is free to send a viewport-continuous estimate;
/// the server's quantization is what makes the key space bounded again.
#[serde(default)]
pub window_min_wl_m: u32,
}
@@ -1182,7 +1268,11 @@ fn serve_district_window(
let raw_center = req.window_center?;
let granularity = resolve_window_granularity(req.window_granularity);
let n = clamp_window_n(req.window_n, granularity);
let min_wl_m = req.window_min_wl_m;
// T-1150 design doc §5: quantize BEFORE either the cache key or the
// DeriveWindow work item sees it — the raw wire value never reaches
// either (same discipline as window_n's clamp above and
// normalize_window_center's wrap/clamp below).
let min_wl_m = quantize_min_wl_m(req.window_min_wl_m);
// body_params is needed to normalize the centre BEFORE either cache key
// exists (T-1142) — read it first, unconditionally (not gated on a cache
@@ -1971,6 +2061,67 @@ mod tests {
);
}
/// **Contract-pinning test (PR #191 review, Tyre C1):** a quarter
/// (granularity=4) request for `n=32` echoes the WIRE-CAP-CLAMPED `n=16`,
/// not the requested 32 — `32² × 4² = 16,384` cells, 4x over
/// `WIRE_CAP_CELLS`. This is the exact scenario the review flagged as
/// silently breaking the client the moment T-1153 requests quarter at
/// n=32: the server echoes a DIFFERENT `n` than what was asked for, and
/// any client staleness guard comparing raw `_n` against the echo must
/// already know this will happen (see
/// `atlas_window_request.gd::_clamp_window_n_mirror()`, the client-side
/// fix landed alongside this test).
#[test]
fn quarter_n32_request_echoes_clamped_n16() {
let mut cache = BodyWorldStateCache::new(CACHE_CAPACITY);
let mut window_cache = DistrictWindowCache::new(DISTRICT_WINDOW_CACHE_CAPACITY);
let (_db, resolver, params_reader, _root) = resolver_and_params_reader("GJ1c");
let queue = GenerationQueue::with_threads(1);
let quarter_n32_req = AtlasLayerRequest {
body_id: "GJ1c".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((0, 0)),
window_n: 32,
window_granularity: WINDOW_GRANULARITY_QUARTER,
window_min_wl_m: 0,
};
let resp = handle_atlas_request(
&quarter_n32_req,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
1,
test_conn_id(),
);
assert!(resp.district_window.is_none(), "first request — cache miss");
std::thread::sleep(Duration::from_millis(300));
let completions = queue.drain_completions();
let window_completion = completions.into_iter().find_map(|c| {
if let GenCompletion::WindowDerived { body_id, layer } = c {
if body_id == "GJ1c" {
return Some(layer);
}
}
None
});
let layer = window_completion.expect("DeriveWindow must complete for GJ1c");
assert_eq!(
layer.granularity, WINDOW_GRANULARITY_QUARTER,
"granularity must echo back as requested (4 is within budget on its own)"
);
assert_eq!(
layer.n, 16,
"a quarter n=32 request must echo the wire-cap-clamped n=16, not the requested 32"
);
}
// -------------------------------------------------------------------
// resolve_window_granularity / clamp_window_n (T-1150)
// -------------------------------------------------------------------
@@ -2043,6 +2194,160 @@ mod tests {
assert_eq!(clamp_window_n(8, WINDOW_GRANULARITY_QUARTER), 8);
}
// -------------------------------------------------------------------
// quantize_min_wl_m (T-1150, PR #191 review — Hoshe 1 / Tyre C3, design doc §5)
// -------------------------------------------------------------------
#[test]
fn quantize_min_wl_m_exact_band_values_are_stable() {
for &band in &MIN_WL_BANDS_M {
assert_eq!(quantize_min_wl_m(band as u32), band as u32);
}
}
#[test]
fn quantize_min_wl_m_zero_stays_zero() {
assert_eq!(quantize_min_wl_m(0), 0);
}
/// A value nearer to 0 than to the finest real octave band (4,096) snaps
/// to 0 (no cutoff) — the band set includes 0 as a real, selectable band,
/// not just a special-cased default.
#[test]
fn quantize_min_wl_m_small_value_snaps_to_zero_band() {
assert_eq!(quantize_min_wl_m(500), 0);
}
/// A value between two real octave bands snaps to the NEAREST one, not
/// always up or always down.
#[test]
fn quantize_min_wl_m_mid_value_snaps_to_nearest_band() {
// Between 4,096 and 8,192: 5,000 is nearer 4,096 (dist 904 vs 3,192).
assert_eq!(quantize_min_wl_m(5_000), 4_096);
// 7,500 is nearer 8,192 (dist 692 vs 3,404).
assert_eq!(quantize_min_wl_m(7_500), 8_192);
}
/// A value far above the coarsest band snaps to the coarsest band, never
/// panics or overflows — quantization must be a TOTAL function over all
/// u32 input (never trust the wire).
#[test]
fn quantize_min_wl_m_huge_value_snaps_to_coarsest_band() {
assert_eq!(quantize_min_wl_m(u32::MAX), 32_768);
assert_eq!(quantize_min_wl_m(1_000_000), 32_768);
}
/// The mandatory §5 aliasing-closing test: two requests differing only in
/// an UNQUANTIZED `min_wl_m` that both fall in the SAME band must share
/// ONE cache entry, not two — this is the exact gap §5 flags ("same key,
/// different min_wl, would silently collide" becomes "same key, same
/// quantized min_wl, correctly coalesce").
#[test]
fn two_requests_in_same_min_wl_band_share_one_cache_entry() {
let mut cache = BodyWorldStateCache::new(CACHE_CAPACITY);
let mut window_cache = DistrictWindowCache::new(DISTRICT_WINDOW_CACHE_CAPACITY);
let (_db, resolver, params_reader, _root) =
resolver_and_params_reader_with_radius("BandBody", 6371.0);
let queue = GenerationQueue::with_threads(2);
// Both values are nearer 4,096 than any other band (4,000 and 4,300
// both round to 4,096 — see the mid-value test above for the
// distance math), so they must land in the SAME quantized band.
let req_a = AtlasLayerRequest {
body_id: "BandBody".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((10, -5)),
window_n: 4,
window_granularity: WINDOW_GRANULARITY_DISTRICT,
window_min_wl_m: 4_000,
};
let req_b = AtlasLayerRequest {
body_id: "BandBody".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((10, -5)),
window_n: 4,
window_granularity: WINDOW_GRANULARITY_DISTRICT,
window_min_wl_m: 4_300,
};
handle_atlas_request(
&req_a,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
1,
test_conn_id(),
);
handle_atlas_request(
&req_b,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
1,
test_conn_id(),
);
std::thread::sleep(Duration::from_millis(300));
let completions = queue.drain_completions();
for c in completions {
if let GenCompletion::WindowDerived { body_id, layer } = c {
if body_id == "BandBody" {
window_cache.insert(
(body_id, layer.center, layer.n, layer.granularity, layer.min_wl_m),
*layer,
);
}
}
}
assert_eq!(
window_cache.len(),
1,
"two requests in the SAME quantized min_wl_m band at identical \
(body, center, n, granularity) must share ONE cache entry, not two"
);
// Re-request both — each must hit the SAME cached entry and echo the
// QUANTIZED band (4,096), not either raw wire value.
let resp_a = handle_atlas_request(
&req_a,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
2,
test_conn_id(),
);
let resp_b = handle_atlas_request(
&req_b,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
2,
test_conn_id(),
);
let layer_a = resp_a.district_window.expect("req_a must hit the cache");
let layer_b = resp_b.district_window.expect("req_b must hit the cache");
assert_eq!(layer_a.min_wl_m, 4_096, "echo must be the QUANTIZED band");
assert_eq!(layer_b.min_wl_m, 4_096, "echo must be the QUANTIZED band");
assert_eq!(layer_a, layer_b, "both requests must resolve to the identical cached layer");
}
// -------------------------------------------------------------------
// normalize_window_center (T-1142 — letterbox-click out-of-range bug)
// -------------------------------------------------------------------