fix(simulation): address PR #168 review — fallback diagnostic, parity coverage, doc (T-1039)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user