diff --git a/governance/decisions/architecture.md b/governance/decisions/architecture.md index 67d4942f1..5d9de3f8c 100644 --- a/governance/decisions/architecture.md +++ b/governance/decisions/architecture.md @@ -1118,9 +1118,10 @@ Technical foundation decisions that constrain implementation: engine, client-ser - **Two-tier mismatch flagging:** If a matched city-attractor pair has score < 0.35, log a `WARNING` (below expected quality). If score < 0.15, log an `ERROR` and flag for manual review. Generation proceeds in both cases; the flags are for content auditing, not hard blockers. - **Output:** `Vec` written to `atlas_city_positions` at build time. - **Rationale:** Greedy-first for large/locked cities ensures anchor cities (capitals, corp HQs, wiki-named cities) are placed at terrain features that match their lore role. Hungarian for medium cities finds the globally optimal assignment, not just locally optimal. Synthetic overflow prevents the algorithm from failing on bodies where city count exceeds natural attractor count (dense, flat worlds). The two-tier warning system enables content QA without blocking generation. -- **Ticket:** T-919, T-925 +- **Amendment (T-1206, 2026-08-06 — step 4 may now legitimately place nothing):** phase 4's synthetic position was derived by pure grid arithmetic with **no terrain input at all**, so it could and did land in open water — a scan of every heightmap body at world seed 42 found **46 of 109 synthetic placements sitting in ocean** (e.g. GJ903c at a genuine polar ocean cell). Synthetic overflow now takes the terrain analysis and land-corrects via a bounded nearest-land ring walk; land positions pass through untouched, so **no existing land placement moves** and this record's seed-derived-position promise is intact (position remains a pure function of seed + terrain — this fulfils the placement intent rather than deviating from it, so no re-decision is required). What **does** change is the record's outcome set: step 4's "cities that cannot be matched receive a synthetic `PlainCenter`" is no longer total — where no land exists within the search bound, the synthetic attractor is **skipped**, defined and deliberate, never a fabricated water position and never a panic. Step 5's name-fulfillment warning consequently fires for a new legitimate reason (a genuinely water-locked body), not only for a pipeline failure. +- **Ticket:** T-919, T-925, T-1206 (ocean-mask guard amendment) - **Raised by:** Generation cascade workshop (T-897) -- **Cross-reference:** D-195 (CompatibilityMatrix), D-207 (atlas_city_names), D-209 (GeographicAttractor — input), D-210 (terrain_modification_cost — input) +- **Cross-reference:** D-195 (CompatibilityMatrix), D-207 (atlas_city_names), D-209 (GeographicAttractor — input), [D-210](#d-210) (terrain_modification_cost — input; its 2026-07-26 amendment records the *matched-attractor* half of this same placement-in-water gap, closed and validated under T-1116/T-1206) ### D-212: TerritorialStatus Priority-Ordered Derivation Algorithm - **Date:** 2026-05-01 diff --git a/server/src/atlas/attractor_matching.rs b/server/src/atlas/attractor_matching.rs index 571d13fc4..e5508389a 100644 --- a/server/src/atlas/attractor_matching.rs +++ b/server/src/atlas/attractor_matching.rs @@ -279,15 +279,32 @@ const MIN_SPACING: u16 = 15; /// landing in a genuine HIGH-LATITUDE OCEAN BAND near the grid's pole row — /// not a small coastal-cell artifact. Real, substantially-land bodies (35–95% /// land overall) had their nearest land cell to `(0,0)` as far as 125 cells -/// away (`grid_h` = 256, so `grid_h / 2` = 128 is the natural ceiling — a -/// point beyond that is more than half the grid's height from the pole and a -/// wider search stops paying for itself). 128 comfortably covers every -/// observed real-body case while still being a bounded, cheap search (worst -/// case ~128² ≈ 16k candidate cells, negligible next to the cascade's other -/// per-body costs) — genuinely water-locked bodies (no land within half the +/// away. Half the grid's height is the natural ceiling: a point beyond that +/// is more than half the grid from the pole, and a wider search stops paying +/// for itself. It is also a bounded, cheap search — worst case ~(h/2)² ≈ 16k +/// candidate cells on the current grid, negligible beside the cascade's other +/// per-body costs. Genuinely water-locked bodies (no land within half the /// grid height of the arithmetic position) still correctly degrade to the -/// skip path below. -const MAX_LAND_SEARCH_RING: u16 = 128; +/// skip path in the caller. +/// +/// **Derived, not hardcoded** (PR #218 review, Tyre). The bound is `h / 2` +/// and it is computed from the grid actually in scope, because the whole +/// justification above is expressed in grid heights: a literal `128` silently +/// carried the 512×256 working-grid assumption, so a change in grid +/// resolution would have left the constant behind while its own doc still +/// claimed to be half the height. On the current 512×256 working grid this +/// evaluates to exactly the 128 it replaces — no behaviour change, and the +/// PR's byte-identical-placement guarantee is untouched. +/// +/// **Recalibration owner:** whoever changes the body corpus or the working- +/// grid resolution. Re-run the T-1206 verification scan (every heightmap +/// body at world seed 42, recording each synthetic placement's distance to +/// nearest land) and confirm the observed maximum still sits comfortably +/// under `h / 2`. If it does not, the ceiling — not just the literal — is +/// what needs revisiting. +fn max_land_search_ring(ta: &TerrainAnalysis) -> u16 { + (ta.h / 2) as u16 +} /// Ring-expansion search from `(row, col)` to the nearest cell with /// `!ocean_mask` (T-1206, the T-1116 `nearest_passable_cell` pattern at @@ -301,14 +318,14 @@ const MAX_LAND_SEARCH_RING: u16 = 128; /// seed-derived-position promise intact). Columns wrap (equirectangular /// globe, matching every other grid walk in this cascade); rows clamp /// (poles). Returns `None` if no land cell exists within -/// [`MAX_LAND_SEARCH_RING`] — the caller's defined degradation is to skip +/// [`max_land_search_ring`] — the caller's defined degradation is to skip /// the synthetic attractor entirely (T-1206), never to fabricate a position. fn nearest_land_cell(ta: &TerrainAnalysis, row: u16, col: u16) -> Option<(u16, u16)> { let (w, h) = (ta.w as i32, ta.h as i32); if !ta.is_ocean(row as usize, col as usize) { return Some((row, col)); } - for ring in 1..=MAX_LAND_SEARCH_RING { + for ring in 1..=max_land_search_ring(ta) { let ring_i = ring as i32; let (r0, c0) = (row as i32, col as i32); let mut candidates: Vec<(i32, i32)> = Vec::new(); @@ -359,7 +376,7 @@ fn nearest_land_cell(ta: &TerrainAnalysis, row: u16, col: u16) -> Option<(u16, u /// no currently-land synthetic-overflow placement moves on any existing body /// (D-211's seed-derived-position promise). Only a water-arithmetic position /// is nudged, via [`nearest_land_cell`]'s deterministic ring-walk. If no land -/// cell exists within [`MAX_LAND_SEARCH_RING`] (an all-ocean region of the +/// cell exists within [`max_land_search_ring`] (an all-ocean region of the /// grid), returns `None` — the caller's defined degradation is to skip this /// synthetic attractor entirely (the city goes unplaced and is reported by /// Phase 5's existing name-fulfillment warning) rather than fabricate a @@ -612,7 +629,7 @@ pub fn match_cities( if placed_ids.contains(&city.city_id) { continue; } - // T-1206: `None` here (all-ocean region beyond MAX_LAND_SEARCH_RING, + // T-1206: `None` here (all-ocean region beyond max_land_search_ring, // or degenerate MIN_SPACING exhaustion) means the defined degradation // is to skip this city's synthetic placement entirely — never // fabricate a water position. Phase 5's name-fulfillment check below @@ -1337,7 +1354,7 @@ mod tests { let ta = ta_with_land(grid_w as usize, grid_h as usize, &[(2, 0)]); let guarded = synthetic_attractor(&placed, grid_w, grid_h, Some(&ta)) - .expect("land exists within MAX_LAND_SEARCH_RING"); + .expect("land exists within max_land_search_ring"); assert_eq!( guarded.position, (2, 0), @@ -1396,29 +1413,34 @@ mod tests { #[test] fn nearest_land_cell_clamps_rows_no_wrap() { let (w, h) = (16usize, 16usize); - // Land only at the FAR pole (row h-1) — from row 0, a row-wrapping - // implementation would find it at ring (h-1); a row-clamping one - // must exhaust MAX_LAND_SEARCH_RING first if h-1 > that bound, or - // find it only via the correct non-wrapped ring distance. Here - // h=16 keeps h-1=15 comfortably inside MAX_LAND_SEARCH_RING (128), - // so the assertion is on the POSITION found, not on absence: a - // wrapping bug would still find (15, c) — same as clamping would, - // since row 15 IS within the grid — so instead assert a cell just - // below row 0 wrapping to the top is never sourced from "negative - // row mod h" by using an asymmetric single-land-cell placement at a - // row that would be reached MUCH sooner via wraparound than via the - // real clamped ring distance. + // Land ONLY at the far pole (row h-1), searching from row 0. + // + // The two implementations disagree by an order of magnitude here. + // Clamped, the only route is straight down the column: ring 15. + // Wrapping, row 0 steps "up" to row 15 immediately: ring 1. + // + // Since the search bound became h/2 (PR #218 review — see + // `max_land_search_ring`), this grid bounds the walk at ring 8, which + // sits BETWEEN those two distances. That turns a position assertion + // into an absence assertion, and a sharper one: ring 15 is now out of + // reach by construction, so a correct clamping walk must give up and + // return None, while ANY `Some` result proves rows wrapped — there is + // no other way to reach row 15 within 8 rings. + // + // (Before the bound was derived it was a flat 128, far beyond this + // 16-row fixture, so both implementations returned Some((15, 0)) and + // the test could only pin the position. It now discriminates.) let ta = ta_with_land(w, h, &[(15, 0)]); let found = nearest_land_cell(&ta, 0, 0); - // Real (clamped) distance is ring 15 (straight down the column). - // A wrapping implementation could equally reach it at ring 1 (one - // step "up" from row 0 wrapping to row 15) — assert the ring-15 - // (non-wrapped) result to pin clamping behavior. - assert_eq!(found, Some((15, 0))); + assert_eq!( + found, None, + "rows must clamp: the far pole is 15 rings away and the bound is \ + h/2 = 8, so any Some(..) here means the walk wrapped rows" + ); } /// **Degradation (T-1206, requirement 2): no land within - /// `MAX_LAND_SEARCH_RING` → skip, never fabricate or panic.** A tiny + /// `max_land_search_ring` → skip, never fabricate or panic.** A tiny /// all-ocean grid guarantees no land cell exists anywhere, so the guard /// must return `None` — `match_cities`'s Phase 4 then skips this city /// entirely (verified below via `match_cities` directly, matching Phase @@ -1553,4 +1575,235 @@ mod tests { assert_eq!(placements.len(), 1); assert_eq!(placements[0].position, (0, 0)); } + + /// Run the real cascade for `body` and hand back its placements together + /// with the `TerrainAnalysis` they must be checked against. + /// + /// The re-derivation is not incidental — it is the whole reason this + /// helper exists. `CascadeSnapshot::terrain_analysis` is transient: the + /// cascade sets it to `None` the moment DistrictProfile and RoadGraph are + /// done with it (D-203/T-1048, ~2 MB a body), so by the time a completed + /// snapshot is handed back it is ALWAYS `None`, whatever + /// `cascade_snapshot_for_body`'s doc-comment implies. A caller that reads + /// it off the snapshot and skips when absent skips every body, silently, + /// and reports a clean run — which is exactly what the first draft of the + /// scan below did across all 267 bodies. + /// + /// So re-derive it from the surviving heightmap through the SAME call the + /// cascade itself used (`run_layer1_with_moisture` with the body's own + /// moisture ceiling), which reproduces the identical grid the placements + /// were computed against. + #[cfg(test)] + fn cascade_placements_and_terrain( + body: &str, + seed: u64, + ) -> Option<(Vec, TerrainAnalysis)> { + let (snapshot, params) = + crate::atlas::believability::cascade_snapshot_for_body(seed, body).ok()?; + let placements = snapshot.layer3.as_ref()?.placements.clone(); + let (_l1, ta) = crate::atlas::layer1::run_layer1_with_moisture( + &snapshot.heightmap, + crate::atlas::district_profile::derive_moisture_ceiling_q(¶ms), + ); + Some((placements, ta)) + } + + /// Directory of bodies carrying a committed heightmap, read off the wiki + /// tree rather than the DB (the tree IS the heightmap corpus, and this + /// avoids a second source of truth). Tries repo-root and `server/`-relative + /// paths, matching `believability::find_heightmap`'s own CWD tolerance. + #[cfg(test)] + fn bodies_with_heightmaps() -> Vec { + let root = ["wiki/star-systems", "../wiki/star-systems"] + .iter() + .map(std::path::PathBuf::from) + .find(|p| p.is_dir()); + let Some(root) = root else { + return Vec::new(); + }; + let mut out = Vec::new(); + let Ok(systems) = std::fs::read_dir(&root) else { + return out; + }; + for system in systems.flatten() { + let bodies_dir = system.path().join("bodies"); + let Ok(bodies) = std::fs::read_dir(&bodies_dir) else { + continue; + }; + for body in bodies.flatten() { + if body.path().join("heightmap.png").is_file() { + if let Some(name) = body.file_name().to_str() { + out.push(name.to_string()); + } + } + } + } + out.sort(); + out + } + + /// **The T-1206 verification scan, committed (PR #218 review, Hoshe).** + /// + /// The scan that motivated this fix — every heightmap body at world seed + /// 42, counting synthetic placements that land in water — was run once and + /// thrown away, which is precisely why its headline number (46 of 109 in + /// water) had nothing holding it. This is that scan, re-runnable: + /// + /// ```text + /// cargo test --lib t1206_verification_scan -- --ignored --nocapture + /// ``` + /// + /// `#[ignore]` because it derives the full cascade for ~267 bodies — far + /// too slow for the default suite. The fast invariant lives in + /// `real_body_synthetic_placements_never_land_in_ocean` below, which pins + /// the bodies this scan identifies. + /// + /// Re-run it when the body corpus or the working-grid resolution changes: + /// it reports both the ocean-guard invariant AND the distance-to-land + /// distribution that sizes `max_land_search_ring`. + #[test] + #[ignore = "full-corpus scan (~267 bodies) — run explicitly"] + fn t1206_verification_scan() { + let bodies = bodies_with_heightmaps(); + assert!(!bodies.is_empty(), "no heightmap bodies found"); + + let (mut loaded, mut synthetic, mut in_water) = (0usize, 0usize, 0usize); + // Counted separately so a zero cannot be ambiguous: "no synthetic + // placements" and "Layer 3 never ran" would otherwise look identical, + // and the difference is the whole meaning of the result. + let (mut with_layer3, mut total_placements) = (0usize, 0usize); + let mut offenders: Vec = Vec::new(); + let mut with_synthetic: Vec = Vec::new(); + + for body in &bodies { + let Some((placements, ta)) = cascade_placements_and_terrain(body, 42) else { + continue; + }; + loaded += 1; + with_layer3 += 1; + total_placements += placements.len(); + let mut body_synthetic = 0usize; + for p in placements.iter().filter(|p| p.synthetic) { + synthetic += 1; + body_synthetic += 1; + let (row, col) = p.position; + if ta.is_ocean(row as usize, col as usize) { + in_water += 1; + offenders.push(format!("{body} city {} @ {:?}", p.city_id, p.position)); + } + } + if body_synthetic > 0 { + with_synthetic.push(format!("{body}({body_synthetic})")); + } + } + + println!( + "T-1206 scan @ seed 42: {loaded} bodies loaded, {with_layer3} reached Layer 3, \ + {total_placements} placements total, {synthetic} synthetic, {in_water} in water" + ); + println!( + "bodies with synthetic placements: {}", + with_synthetic.join(" ") + ); + for o in &offenders { + println!(" IN WATER: {o}"); + } + assert_eq!( + in_water, 0, + "the ocean-mask guard has regressed on real data: {offenders:?}" + ); + } + + /// **The claim this PR was built on, pinned to real data (PR #218 review, + /// Hoshe).** + /// + /// Every other test above proves the guard on synthetic fixtures. But the + /// evidence that motivated T-1206 was a scan of production data — 46 of + /// 109 synthetic placements across the 267 real bodies sat in open water + /// at world seed 42 — and that scan was a throwaway. Nothing committed + /// held the line on the bodies that actually exhibited the bug, so a + /// regression would have been caught only by fixtures, never by the data + /// that revealed it. + /// + /// This walks the real cascade over real bodies (the T-1116 precedent in + /// `road_graph.rs::real_body_coastal_settlements_produce_routable_edges` + /// — committed `systems.db` + `wiki/star-systems` heightmaps, entered + /// through the same loader production uses) and asserts the invariant + /// directly: NO synthetic placement may sit on an ocean cell. + /// + /// `cascade_snapshot_for_body`, not `cascade_for_body`, because the + /// `TerrainAnalysis` carrying `ocean_mask` is transient and is dropped by + /// `into_body_world_state()` (D-203/T-1048 size budget) — the mask has to + /// be read off the snapshot before conversion. + /// + /// Guarded against passing vacuously: the run must produce at least one + /// synthetic placement somewhere in the corpus, otherwise the assertion + /// below is over an empty set and proves nothing. If the body list ever + /// stops exercising overflow, that guard fails loudly rather than going + /// quietly green. It has already earned that — it caught both a body list + /// that exercised nothing and, worse, a terrain-source bug that made the + /// whole-corpus scan report a clean 267 bodies while checking none of them + /// (see `cascade_placements_and_terrain`). + /// + /// Full-corpus figures at seed 42, from `t1206_verification_scan`: + /// **267 bodies, all reaching Layer 3, 344 placements, 109 of them + /// synthetic, 0 in water.** The synthetic count matches the number the + /// original T-1206 scan reported, so the fix's headline evidence is now + /// reproducible rather than anecdotal. + #[test] + fn real_body_synthetic_placements_never_land_in_ocean() { + // Every one of these DOES reach Phase-4 synthetic overflow at seed 42 + // — taken from `t1206_verification_scan`'s own output, not guessed. + // That matters: the first draft of this list paired GJ903c with four + // headline bodies (GJ251c/GJ380c/GJ820Bc/GJ338Bd) that turn out to + // have no synthetic placements at all, so the assertion rested on a + // single body. GJ903c is the case the T-1206 scan named (a genuine + // polar ocean cell); GJ3737e-m2 is the only body carrying two. + let bodies = [ + "GJ903c", + "GJ3737e-m2", + "GJ1245Bb", + "GJ667Ce", + "GJ581c", + "GJ892f", + ]; + let mut synthetic_seen = 0usize; + let mut bodies_loaded = 0usize; + let mut total_placements = 0usize; + + for body in bodies { + // A body may legitimately be absent from this checkout's data; + // skip rather than fail, matching the believability harness's own + // contract for the `*_for_body` loaders. + let Some((placements, ta)) = cascade_placements_and_terrain(body, 42) else { + continue; + }; + bodies_loaded += 1; + total_placements += placements.len(); + + for p in placements.iter().filter(|p| p.synthetic) { + synthetic_seen += 1; + let (row, col) = p.position; + assert!( + !ta.is_ocean(row as usize, col as usize), + "{body}: synthetic placement for city {} sits in open water at \ + {:?} — the T-1206 ocean-mask guard has regressed", + p.city_id, + p.position + ); + } + } + + assert!( + bodies_loaded > 0, + "no test body could be loaded — systems.db or the heightmaps are \ + missing, so this test verified nothing" + ); + assert!( + synthetic_seen > 0, + "{bodies_loaded} bodies loaded, {total_placements} placements among them, \ + but not one synthetic — this test would pass vacuously; pick bodies that \ + still reach Phase-4 overflow" + ); + } }