fix(simulation): PR #218 review round — real-body evidence, D-211, derived bound

Three findings from Hoshe (QA) and Tyre (architecture), plus a bug the first
of them uncovered.

HOSHE — the headline evidence lived only in a deleted scratch scan. All nine
tests used synthetic fixtures, so nothing committed held the "46 of 109
synthetic placements in water" claim on real data. Two tests now do. The
T-1206 verification scan itself is committed as an #[ignore]d full-corpus
test (267 bodies, run with --ignored), which also makes the recalibration
instruction on the search bound executable rather than aspirational; a fast
test pins the bodies it identifies.

That scan promptly caught a bug in its own first draft, and it is the reason
this commit is worth reading. `CascadeSnapshot::terrain_analysis` is
transient — the cascade nulls it the moment DistrictProfile and RoadGraph are
done (D-203/T-1048, ~2 MB a body) — so it is ALWAYS None on a returned
snapshot, whatever cascade_snapshot_for_body's doc-comment implies. Reading
the ocean mask off the snapshot and skipping when absent therefore skipped
every body while reporting success: 267 bodies "scanned", 0 findings, a green
assert over an empty set, in 697 seconds. Terrain is now re-derived through
the same run_layer1_with_moisture call the cascade used, reproducing the grid
the placements were computed against.

Two habits caught it, both prompted by Hoshe's finding: a vacuity guard that
refuses to pass when no synthetic placement was seen, and counters that stop
"none found" and "never got that far" from looking identical. Corrected
figures at seed 42: 267 bodies, all reaching Layer 3, 344 placements, 109
synthetic, 0 in water — the synthetic count matching the original scan, so
the claim is reproducible now rather than anecdotal.

TYRE 1 — MAX_LAND_SEARCH_RING was justified as grid_h/2 but written as a
literal 128, leaving the 512x256 coupling implicit. It is now derived from
the grid in scope, so the value cannot drift from its own rationale. On the
current working grid it evaluates to exactly 128: no behaviour change, and
the byte-identical-placement guarantee is untouched. Recalibration owner
recorded.

That derivation does change one test. nearest_land_cell_clamps_rows_no_wrap
uses a 16x16 fixture, so its bound drops 128 -> 8, which now sits BETWEEN the
clamped distance to the far pole (15) and the wrapped one (1). The assertion
moves from position to absence and gets sharper for it: previously both
implementations returned Some((15,0)) and only the position could be pinned;
now any Some at all proves rows wrapped.

TYRE 2 — D-211 carried no note though its behaviour changed. Dated amendment
added: step 4's outcome set is no longer total (synthetic overflow may now
resolve to a defined SKIP), and step 5's warning fires for a new legitimate
reason. No re-decision needed — position remains a pure function of seed and
terrain — and the dead-end cross-reference to D-210's closure is now a live
anchor.

Full cargo test green (30 binaries).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-06 17:55:23 +02:00
co-authored by Claude Opus 5
parent e52aa027b3
commit 4b75be5975
2 changed files with 287 additions and 33 deletions
+3 -2
View File
@@ -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<CityPlacement { city_id, position: [row, col], attractor: AttractorType, score: f32 }>` 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
+284 -31
View File
@@ -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 (3595%
/// 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<CityPlacement>, 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(&params),
);
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<String> {
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<String> = Vec::new();
let mut with_synthetic: Vec<String> = 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"
);
}
}