From 43aadc449113a81f2c035295aded565158f15752 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 16 Jun 2026 12:51:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20address=20PR=20#168=20review?= =?UTF-8?q?=20=E2=80=94=20fallback=20diagnostic,=20parity=20coverage,=20do?= =?UTF-8?q?c=20(T-1039)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoshe H1: the morphology_zone AlluvialPlain fallback fired silently. Now logs tracing::debug! when the district grid is empty (expected: params-missing body / early cascade) and tracing::warn! when a populated grid misses the key (indicates a pixel->district convention bug) — no more silently-wrong topology. Hoshe H2: the arrangement_pattern parity tripwire only covered transit_hub with Commission + Corporate. Added Pioneer/Industrial/Military/Academic rows so an accidental archetype-conditionalization of the cross-archetype override is caught. Tyre (non-blocking): clarified the build_skeleton_work_item doc — the arrangement_pattern re-derivation call lives in generate_quarter_skeleton (the consumer), not in this function. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/src/atlas/plugin.rs | 54 ++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/server/src/atlas/plugin.rs b/server/src/atlas/plugin.rs index cc4bbce00..4e86df600 100644 --- a/server/src/atlas/plugin.rs +++ b/server/src/atlas/plugin.rs @@ -201,7 +201,9 @@ fn drain_generation_completions( /// `arrangement_pattern` is **re-derived** at L4 from `(political_archetype, /// economic_role)` via the same pure function used at L3 (T-1039 OPTION (b) — /// locked, no `CityGenerationContext` field added). Re-derivation is provably -/// identical to the L3 value (pure total function, no RNG). +/// identical to the L3 value (pure total function, no RNG). The re-derivation call +/// itself lives in the consumer (`generate_quarter_skeleton`), not in this function +/// — `build_skeleton_work_item` only threads the inputs it needs. /// /// `quarter_id` is the canonical D-194/D-230 derivation from `(world_seed, body, /// city)` — not the `city_id * 10` placeholder. @@ -235,10 +237,29 @@ fn build_skeleton_work_item( // Convert the placement's working-grid pixel position to a DistrictPos using // the canonical scale constant — no hardcoded magic numbers here. let district_pos = scale::heightmap_pixel_to_district(placement.position); - context.morphology_zone = districts - .get(&district_pos) - .map(|d| d.morphology_zone) - .unwrap_or(MorphologyZone::AlluvialPlain); + context.morphology_zone = match districts.get(&district_pos) { + Some(d) => d.morphology_zone, + None => { + // An empty grid is the expected params-missing / early-cascade case + // (debug); a miss against a *populated* grid means the pixel→DistrictPos + // conversion is off — a real bug worth a warning, not a silent wrong + // topology. + if districts.is_empty() { + tracing::debug!( + city_id = placement.city_id, + ?district_pos, + "morphology_zone fallback to AlluvialPlain: district grid not built for this body" + ); + } else { + tracing::warn!( + city_id = placement.city_id, + ?district_pos, + "morphology_zone fallback to AlluvialPlain: pos not in populated district grid — check pixel→district convention" + ); + } + MorphologyZone::AlluvialPlain + } + }; // ── T-1043: road_entry_directions from road_graph ─────────────────────────── // Find this city's settlement node index in the road graph (O(n) scan on a @@ -769,6 +790,29 @@ mod tests { "transit_hub", ArrangementPattern::HubAndSpoke, ), + // transit_hub must override EVERY archetype (the guard fires before the + // archetype match) — cover the rest so an accidental + // archetype-conditionalization of the override can't slip through. + ( + PoliticalArchetype::Pioneer, + "transit_hub", + ArrangementPattern::HubAndSpoke, + ), + ( + PoliticalArchetype::Industrial, + "transit_hub", + ArrangementPattern::HubAndSpoke, + ), + ( + PoliticalArchetype::Military, + "transit_hub", + ArrangementPattern::HubAndSpoke, + ), + ( + PoliticalArchetype::Academic, + "transit_hub", + ArrangementPattern::HubAndSpoke, + ), ]; for (archetype, role, expected_pattern) in cases {