jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(simulation): normalize window_center before cache and coalescing keys (T-1142 server half)

Browse Source 
The server clamped window_n but trusted window_center — the letterbox
bug's impossible (12276, 3021) was derived at clamped-garbage latitude
and CACHED under its raw key. normalize_window_center mirrors the
forward map exactly (column rem_euclid against the body's district
circumference, row clamped to +/-half-meridian; constants from
scale::DISTRICT_M, no new magic numbers; no-radius test bodies
identity). Applied in serve_district_window BEFORE the window-cache
key and the coalescing key, so an insane request and its sane twin
share one entry — proven end-to-end with the literal bug values on a
500km fixture body resolving to canonical (4, 383), byte-identical.

Echo contract: DistrictWindowLayer.center now carries the NORMALIZED
value. Reconciled by design with the client half in this same branch —
the client canonicalizes before sending (bit-identical math, posmod ==
rem_euclid verified), so request and echo agree from the first
round-trip; server normalization is defense-in-depth for buggy or
modified clients.

6 new tests incl. the twin-cache proof and the echo pin; atlas::
570/570, lib 1784/1784, timing-sensitive tests 3x stable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-07-21 14:52:50 +02:00
co-authored by Claude Fable 5
parent e7319cf322
commit 4a65bb210d
1 changed files with 419 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/atlas/layer_proxy.rs
+419 -34
View File
@@ -18,10 +18,11 @@ use crate::atlas::body_params_reader::BodyParamsReader;
use crate::atlas::body_world_state::{BodyWorldState, BodyWorldStateCache, SimTick};
use crate::atlas::cascade::CascadeLayer;
use crate::atlas::city_context_reader::CityContextReader;
use crate::atlas::district_profile::DistrictPos;
use crate::atlas::district_profile::{BodyParams, DistrictPos};
use crate::atlas::gen_queue::{GenPriority, GenWorkItem, GenerationQueue};
use crate::atlas::layer1::Layer1Output;
use crate::atlas::road_graph::RoadNodeKind;
use crate::atlas::scale::DISTRICT_M;
use crate::atlas::source_resolver::{BodySourceResolver, SourceResolveError};
use crate::bridge::ConnectionId;
use crate::seed::{SeedChain, SeedDomain};
@@ -745,6 +746,64 @@ pub fn build_settlement_layer(state: &BodyWorldState) -> Option<SettlementLayer>
Some(SettlementLayer { settlements })
}
/// Normalize a wire-supplied district-window centre against the body's
/// physical geometry (T-1142 — a letterbox-click bug sent `window_center`
/// wildly out of a body's valid range; the server accepted it, derived
/// clamped garbage inside `derive_district`, and CACHED that garbage under
/// the raw un-normalized key). Mirrors `district_profile::derive_district`'s
/// own forward mapping (`district_profile.rs:1426-1451`) EXACTLY, so a window
/// centre that survives this normalization derives identically to how
/// `derive_district` would have resolved it anyway — this function only
/// closes the gap between "derive_district silently clamps/wraps internally"
/// and "the SERVING path (cache key, coalescing key) saw the raw value".
///
/// **Column (longitude) wraps** — `rem_euclid` against the body's
/// circumference in districts, mirroring the forward map's
/// `(wx / circumference_m).rem_euclid(1.0)` (longitude is periodic; a click
/// at column 12276 on a body whose circumference is a few hundred districts
/// wide is the SAME point as some small in-range column, not garbage).
///
/// **Row (latitude) clamps** — to `±half_meridian_districts`, mirroring the
/// forward map's `(wy / meridian_m).clamp(-0.5, 0.5)` (latitude is NOT
/// periodic; it terminates at the poles, so out-of-range rows collapse to the
/// nearest pole rather than wrapping — same asymmetry `derive_district`
/// itself already encodes).
///
/// Both bounds are derived from `body_radius_km` via the SAME `DISTRICT_M`
/// (2 048 m, D-243) constant the forward map uses — no independent magic
/// numbers that could silently drift out of sync with `derive_district`.
///
/// **No radius (`body_radius_km` absent/non-positive — tiny test bodies
/// only, per `BodyParams`'s own doc: "e.g. tiny test bodies"; every real
/// `systems.db` body row carries a radius):** identity, no wrap/clamp. The
/// forward map's own no-radius branch has no periodicity concept either (it
/// clamps the FRACTIONAL PIXEL position directly against the heightmap's
/// working-grid dimensions, which aren't known at request-serving time — only
/// inside the Rayon work item once the heightmap is loaded); a real letterbox
/// click can never hit this branch, so it is out of this fix's scope.
fn normalize_window_center(params: &BodyParams, center: DistrictPos) -> DistrictPos {
let (dx, dy) = center;
match params.body_radius_km {
Some(r_km) if r_km > 0.0 => {
let circumference_m = std::f64::consts::TAU * r_km * 1000.0;
let meridian_m = std::f64::consts::PI * r_km * 1000.0;
// Whole districts per full circumference / per half-meridian —
// rounded (not truncated) so the bound matches the forward map's
// continuous fraction as closely as an integer district grid can.
let districts_per_circumference =
(circumference_m / DISTRICT_M as f64).round().max(1.0) as i32;
let half_meridian_districts = (meridian_m / DISTRICT_M as f64 / 2.0).round() as i32;
let wrapped_dx = dx.rem_euclid(districts_per_circumference);
let clamped_dy = dy.clamp(-half_meridian_districts, half_meridian_districts);
(wrapped_dx, clamped_dy)
}
// No radius: derive_district's own fallback has no wrap/clamp concept
// at the DistrictPos level (see the doc above) — identity.
_ => center,
}
}
/// Resolve `req`'s district-window query, if any (D-226 T-1124 amendment,
/// T-1137). Returns `None` immediately when `req.window_center` is absent (no
/// window requested — the common case, zero cost).
@@ -756,15 +815,28 @@ pub fn build_settlement_layer(state: &BodyWorldState) -> Option<SettlementLayer>
/// about to take its cache-hit or cache-miss branch, sharing neither's control
/// flow.
///
/// Cache hit (`(body_id, center, n)` already in `window_cache`) → `Some`
/// immediately, no queue submission (D-227: a previously-derived window for
/// this body+seed is valid forever, no staleness check needed). Cache miss →
/// submit a `DeriveWindow` work item (queue-based, per the amendment's binding
/// serving model — never inline here) and return `None`; the *next* request
/// for this `(body, center, n)` re-checks the cache and finds it populated
/// once `drain_generation_completions` has processed the completion (the
/// existing D-225 poll-and-recheck-cache pattern every other layer already
/// uses, not a push).
/// **`window_center` is normalized via [`normalize_window_center`] BEFORE the
/// `DistrictWindowCache` key AND the `submit_window` coalescing key are built**
/// (T-1142) — this is why `body_params` is read HERE, unconditionally,
/// rather than only inside the former miss-branch: normalization needs
/// `body_radius_km` to compute the wrap/clamp bounds, and it must happen
/// before either key exists, or an insane request and its sane normalized
/// twin would land in different cache entries / coalesce independently
/// (exactly the bug this fix closes — a garbage `window_center` was cached
/// standalone instead of collapsing onto its valid twin). The one-time cost
/// (a single indexed `bodies` row read) is paid on every window request now,
/// not just on a cache miss — a request whose normalized center hits the
/// cache still needed this read to know WHICH key to check.
///
/// Cache hit (`(body_id, normalized_center, n)` already in `window_cache`) →
/// `Some` immediately, no queue submission (D-227: a previously-derived
/// window for this body+seed is valid forever, no staleness check needed).
/// Cache miss → submit a `DeriveWindow` work item (queue-based, per the
/// amendment's binding serving model — never inline here) and return `None`;
/// the *next* request for this `(body, normalized_center, n)` re-checks the
/// cache and finds it populated once `drain_generation_completions` has
/// processed the completion (the existing D-225 poll-and-recheck-cache
/// pattern every other layer already uses, not a push).
///
/// `window_n` is clamped to `[1, DISTRICT_WINDOW_MAX_N]` here — the ONE place
/// that clamp is applied; nothing downstream re-checks the wire value.
@@ -778,29 +850,15 @@ fn serve_district_window(
world_seed: u64,
conn_id: ConnectionId,
) -> Option<DistrictWindowLayer> {
let center = req.window_center?;
let raw_center = req.window_center?;
let n = req.window_n.clamp(1, DISTRICT_WINDOW_MAX_N);
let key: DistrictWindowKey = (req.body_id.clone(), center, n);
if let Some(layer) = window_cache.get(&key) {
return Some(layer.clone());
}
// Miss — resolve heightmap + body params and submit a background derive.
// Read/resolve failures are non-fatal for the window (log + skip): the
// window simply stays None on this response, same as an unrun whole-body
// layer, rather than failing the entire AtlasLayerResponse.
let heightmap_path = match resolver.resolve(&req.body_id) {
Ok(p) => p,
Err(e) => {
tracing::warn!(
body_id = %req.body_id,
error = %e,
"district window request: heightmap resolve failed — window stays None"
);
return None;
}
};
// body_params is needed to normalize the centre BEFORE either cache key
// exists (T-1142) — read it first, unconditionally (not gated on a cache
// miss like the former structure). A read failure here can't distinguish
// "insane vs. sane center" for the key, so it's a hard skip for the whole
// window (window stays None on this response), same failure posture the
// former miss-only read already had.
let Some(reader) = body_params_reader else {
tracing::warn!(
body_id = %req.body_id,
@@ -820,6 +878,37 @@ fn serve_district_window(
}
};
let center = normalize_window_center(&body_params, raw_center);
if center != raw_center {
tracing::debug!(
body_id = %req.body_id,
raw = ?raw_center,
normalized = ?center,
"district window request: out-of-range window_center normalized (T-1142)"
);
}
let key: DistrictWindowKey = (req.body_id.clone(), center, n);
if let Some(layer) = window_cache.get(&key) {
return Some(layer.clone());
}
// Miss — resolve the heightmap and submit a background derive.
// Read/resolve failures are non-fatal for the window (log + skip): the
// window simply stays None on this response, same as an unrun whole-body
// layer, rather than failing the entire AtlasLayerResponse.
let heightmap_path = match resolver.resolve(&req.body_id) {
Ok(p) => p,
Err(e) => {
tracing::warn!(
body_id = %req.body_id,
error = %e,
"district window request: heightmap resolve failed — window stays None"
);
return None;
}
};
queue.submit_window(
GenWorkItem::DeriveWindow {
body_id: req.body_id.clone(),
@@ -1434,6 +1523,284 @@ mod tests {
);
}
// -------------------------------------------------------------------
// normalize_window_center (T-1142 — letterbox-click out-of-range bug)
// -------------------------------------------------------------------
/// A small-body fixture (500 km radius) whose bounds are hand-checkable:
/// `districts_per_circumference = round(TAU*500_000/2048) = 1534`,
/// `half_meridian_districts = round(PI*500_000/2048/2) = 383`. The
/// literal column/row this fixture uses (`12276`, `3021`) are the exact
/// values the reported T-1142 letterbox-click bug sent — at THIS radius
/// they genuinely overflow both bounds (at Earth radius, coincidentally,
/// they wouldn't — the bounds are tens of thousands of districts wide),
/// so this is a faithful small-body reproduction, not just an
/// arbitrarily-chosen out-of-range pair.
fn small_body_params() -> BodyParams {
BodyParams {
hydrosphere: Some("ocean".into()),
atmosphere: Some("breathable".into()),
planet_class: Some("temperate".into()),
body_radius_km: Some(500.0),
..Default::default()
}
}
/// Out-of-range COLUMN wraps (longitude is periodic) to its in-range
/// canonical twin via `rem_euclid` — mirroring `derive_district`'s own
/// `(wx / circumference_m).rem_euclid(1.0)` forward map.
#[test]
fn normalize_window_center_wraps_out_of_range_column() {
let params = small_body_params();
// districts_per_circumference = 1534 (hand-computed above).
// 12276 rem_euclid 1534 = 4 (12276 = 8*1534 + 4).
assert_eq!(
12276_i32.rem_euclid(1534),
4,
"sanity: the hand-computed wrap"
);
let (nx, ny) = normalize_window_center(&params, (12276, 0));
assert_eq!(
nx, 4,
"out-of-range column wraps to its canonical in-range twin"
);
assert_eq!(ny, 0, "an in-range row is untouched");
// The canonical twin normalizes to itself (idempotent).
let (nx2, _) = normalize_window_center(&params, (4, 0));
assert_eq!(nx2, 4);
// Negative columns wrap too (rem_euclid, not truncating rem) —
// longitude has no sign discontinuity.
let (nx3, _) = normalize_window_center(&params, (-1, 0));
assert_eq!(
nx3, 1533,
"negative column wraps to the top of the range, not a negative remainder"
);
}
/// Beyond-pole ROW clamps (latitude terminates, does not wrap) to
/// `±half_meridian_districts` — mirroring `derive_district`'s own
/// `(wy / meridian_m).clamp(-0.5, 0.5)` forward map. This is the
/// asymmetry the coordinator's fix explicitly calls out: columns wrap,
/// rows clamp — never the other way around.
#[test]
fn normalize_window_center_clamps_beyond_pole_row() {
let params = small_body_params();
// half_meridian_districts = 383 (hand-computed above).
let (_, ny) = normalize_window_center(&params, (0, 3021));
assert_eq!(
ny, 383,
"beyond-pole row clamps to the pole boundary, not wraps"
);
let (_, ny_neg) = normalize_window_center(&params, (0, -9000));
assert_eq!(ny_neg, -383, "clamping is symmetric at both poles");
// A row exactly at the boundary is untouched.
let (_, ny_boundary) = normalize_window_center(&params, (0, 383));
assert_eq!(ny_boundary, 383);
}
/// An in-range center (well inside both bounds) is returned UNCHANGED —
/// normalization must be a no-op for the overwhelming common case (every
/// legitimate click), not just a defensive clamp that happens to also
/// preserve valid input.
#[test]
fn normalize_window_center_leaves_in_range_center_unchanged() {
let params = small_body_params();
let center = (100, -50);
assert_eq!(normalize_window_center(&params, center), center);
// (0, 0) — the origin — is always in range regardless of body size.
assert_eq!(normalize_window_center(&params, (0, 0)), (0, 0));
}
/// No-radius bodies (tiny test-body fallback, `body_radius_km: None`) get
/// IDENTITY — `derive_district`'s own no-radius branch has no
/// wrap/clamp-in-district-space concept (see the function doc); an
/// extreme center here is out of this fix's scope by design, not an
/// oversight.
#[test]
fn normalize_window_center_no_radius_is_identity() {
let params = BodyParams::default(); // body_radius_km: None
let extreme = (999_999, -999_999);
assert_eq!(normalize_window_center(&params, extreme), extreme);
}
/// End-to-end (the coordinator's core ask): an out-of-range
/// `window_center` and its already-normalized twin, requested through the
/// REAL `handle_atlas_request` path, land in the SAME `DistrictWindowCache`
/// entry and produce a byte-identical `DistrictWindowLayer` — normalization
/// happens BEFORE the cache key is built, so an insane request and its
/// sane twin never diverge into separate cache entries (the bug this fix
/// closes: the insane request was cached STANDALONE).
#[test]
fn insane_and_sane_twin_requests_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("SmallMoon", 500.0);
let queue = GenerationQueue::with_threads(1);
// The insane request — the reported T-1142 letterbox-click values.
let insane_req = AtlasLayerRequest {
body_id: "SmallMoon".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((12276, 3021)),
window_n: 4,
};
let resp1 = handle_atlas_request(
&insane_req,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
1,
test_conn_id(),
);
assert!(
resp1.district_window.is_none(),
"first request — cache miss, DeriveWindow submitted"
);
// Wait for the background derive to complete and drain it into the cache
// (mirrors handle_atlas_request_clamps_oversized_window_n's pattern).
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 == "SmallMoon" {
window_cache.insert((body_id, layer.center, layer.n), *layer);
}
}
}
// Exactly ONE entry must exist in the window cache after the insane
// request's derive completes — normalization means it was keyed on
// the canonical (4, 383), not the raw (12276, 3021).
assert_eq!(
window_cache.len(),
1,
"the insane request's derive must be cached under its NORMALIZED key"
);
// The "sane twin" — the already-normalized canonical center — hits
// the SAME cache entry the insane request just populated.
let sane_twin_req = AtlasLayerRequest {
body_id: "SmallMoon".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((4, 383)), // the hand-computed canonical twin
window_n: 4,
};
let resp2 = handle_atlas_request(
&sane_twin_req,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
2,
test_conn_id(),
);
let twin_layer = resp2.district_window.expect(
"the sane twin must hit the cache the insane request populated — no new derive needed",
);
assert_eq!(
window_cache.len(),
1,
"the sane twin must NOT create a second cache entry"
);
// Re-request the ORIGINAL insane center too — it must ALSO now hit
// the same populated cache entry (both requests normalize to the
// same key).
let resp3 = handle_atlas_request(
&insane_req,
&mut cache,
&mut window_cache,
&queue,
&resolver,
None,
Some(&params_reader),
42,
3,
test_conn_id(),
);
let insane_layer_second_try = resp3
.district_window
.expect("the insane request, re-requested, must ALSO hit the shared cache entry");
assert_eq!(
twin_layer, insane_layer_second_try,
"the insane request and its sane twin must resolve to a BYTE-IDENTICAL layer"
);
assert_eq!(
window_cache.len(),
1,
"still exactly one entry — neither re-request created a second one"
);
}
/// The echoed `center` on `DistrictWindowLayer` is the NORMALIZED value,
/// not the raw wire value — the client's D-227 staleness guard (D-226
/// T-1124 amendment §2) must see what was ACTUALLY derived, so it can
/// correctly match this response against its own (now also normalized,
/// per the T-1142 fix note to the client team) cache key.
#[test]
fn echoed_center_is_the_normalized_value_not_the_raw_wire_value() {
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("SmallMoon2", 500.0);
let queue = GenerationQueue::with_threads(1);
let insane_req = AtlasLayerRequest {
body_id: "SmallMoon2".to_string(),
up_to: CascadeLayer::Topography,
window_center: Some((12276, 3021)), // raw, out-of-range
window_n: 4,
};
handle_atlas_request(
&insane_req,
&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();
let window_completion = completions.into_iter().find_map(|c| {
if let GenCompletion::WindowDerived { body_id, layer } = c {
if body_id == "SmallMoon2" {
return Some(layer);
}
}
None
});
let layer = window_completion.expect("DeriveWindow must complete");
assert_eq!(
layer.center,
(4, 383),
"the completed/echoed layer.center is the NORMALIZED value, not the raw (12276, 3021)"
);
assert_ne!(
layer.center,
(12276, 3021),
"the raw out-of-range wire value must never be echoed back"
);
}
/// `serve_district_window` returns `None` (no window requested) when the
/// request carries no `window_center` — the common case, and the ONLY
/// path every pre-T-1137 caller takes (wire back-compat: an old client's
@@ -2302,7 +2669,8 @@ mod tests {
}
/// Build a DB with the columns needed by both `BodySourceResolver` and
/// `BodyParamsReader` for the same body, plus a tiny heightmap root.
/// `BodyParamsReader` for the same body (Earth radius, 6371 km), plus a
/// tiny heightmap root.
///
/// Returns (db_path, resolver, body_params_reader, _root_kept_alive).
fn resolver_and_params_reader(
@@ -2312,6 +2680,23 @@ mod tests {
BodySourceResolver,
crate::atlas::body_params_reader::BodyParamsReader,
PathBuf, // root dir — must stay alive for the test duration
) {
resolver_and_params_reader_with_radius(body_id, 6371.0)
}
/// Same as [`resolver_and_params_reader`] with a caller-chosen
/// `body_radius_km` (T-1142: the window-centre normalization tests need a
/// SMALL body — at Earth radius the district-circumference/half-meridian
/// bounds are tens of thousands of districts wide, too large for a
/// hand-checkable out-of-range test value).
fn resolver_and_params_reader_with_radius(
body_id: &str,
r_km: f64,
) -> (
PathBuf,
BodySourceResolver,
crate::atlas::body_params_reader::BodyParamsReader,
PathBuf, // root dir — must stay alive for the test duration
) {
let n = SEQ.fetch_add(1, Ordering::Relaxed);
let db = std::env::temp_dir().join(format!("sr_proxybp_{}_{n}.db", std::process::id()));
@@ -2345,8 +2730,8 @@ mod tests {
.unwrap();
conn.execute(
"INSERT INTO bodies (body_id, system_id, terrain_reference, hydrosphere, atmosphere, planet_class, body_radius_km, orbital_period_days, axial_tilt_deg)
VALUES (?1, 'GJ-1', ?2, 'ocean', 'breathable', 'temperate', 6371.0, 365.25, 23.5)",
rusqlite::params![body_id, REL],
VALUES (?1, 'GJ-1', ?2, 'ocean', 'breathable', 'temperate', ?3, 365.25, 23.5)",
rusqlite::params![body_id, REL, r_km],
)
.unwrap();
drop(conn);