From 1fee6fd9dae1d0f87122f915b7b011027ac7427a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 16 Jul 2026 17:03:30 +0200 Subject: [PATCH] =?UTF-8?q?fix(simulation):=20PR=20#178=20review=20round?= =?UTF-8?q?=20=E2=80=94=20H1-H4,=20T1,=20T2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H1: split_edge_at now splits length_cells geometric-distance proportionally (fixed-order f64 polyline sums, halves sum exactly to parent) — subsumes the 2-point-parent all-or-nothing bug. H2: chained-snap test (minor onto minor spur) + determinism assertion actually exercising attach_minors + direct 2-point split unit test. H3: cascade-level RailHeadFacing end-to-end assertion on a plus-shaped landmass fixture (2 deterministic degree-3 settlement junctions). H4+T2: STANDALONE_HQ_JOIN_SQL shared constant, both readers compose it; believability lockstep unit test. T1: HUB_SPACING_DIAG_PX doc cross-refs D-243 elastic seam. Co-Authored-By: Claude Fable 5 --- server/src/atlas/believability.rs | 106 ++++++++++++-- server/src/atlas/cascade.rs | 105 ++++++++++++++ server/src/atlas/city_context_reader.rs | 55 +++++--- server/src/atlas/road_graph.rs | 176 +++++++++++++++++++++++- 4 files changed, 404 insertions(+), 38 deletions(-) diff --git a/server/src/atlas/believability.rs b/server/src/atlas/believability.rs index fb6a30cca..866192318 100644 --- a/server/src/atlas/believability.rs +++ b/server/src/atlas/believability.rs @@ -538,21 +538,23 @@ pub fn cascade_for_body(world_seed: u64, body_id: &str) -> Result Result, String> { + use crate::atlas::city_context_reader::STANDALONE_HQ_JOIN_SQL; + let conn = rusqlite::Connection::open(db).map_err(|e| format!("open db: {e}"))?; + let sql = format!( + "SELECT acn.id, acn.name, COALESCE(acn.economic_role,'service_mixed'), + COALESCE(acn.population,0), COALESCE(acn.kind,'city'), + acn.settlement_class, c.corp_id IS NOT NULL + FROM atlas_city_names AS acn + {STANDALONE_HQ_JOIN_SQL} + WHERE acn.body_id = ?1 ORDER BY acn.id" + ); let mut stmt = conn - .prepare( - "SELECT acn.id, acn.name, COALESCE(acn.economic_role,'service_mixed'), - COALESCE(acn.population,0), COALESCE(acn.kind,'city'), - acn.settlement_class, c.corp_id IS NOT NULL - FROM atlas_city_names AS acn - LEFT JOIN corporations AS c - ON c.hq_placement = 'Standalone' - AND c.headquarters_body = acn.body_id - AND c.proper_name = acn.name - WHERE acn.body_id = ?1 ORDER BY acn.id", - ) + .prepare(&sql) .map_err(|e| format!("prepare city query: {e}"))?; let rows = stmt .query_map([body_id], |r| { @@ -759,4 +761,84 @@ mod tests { let b = analyze(7, "GJ1c", &districts); assert_eq!(a, b); } + + /// PR #178 H4 — the believability settlement reader honours baked + /// settlement_class (D-242/T-1075) and derives is_standalone_hq from the + /// shared corporations join, in lockstep with + /// `CityContextReader::read_body_settlements` (T2). Mirrors that module's + /// synthetic-DB fixture pattern. + #[test] + fn read_cities_honours_baked_class_and_standalone_hq_flag() { + use crate::simulation::generator::SettlementClass; + + let path = + std::env::temp_dir().join(format!("sr_believability_cities_{}.db", std::process::id())); + let _ = std::fs::remove_file(&path); + let conn = rusqlite::Connection::open(&path).expect("create db"); + conn.execute_batch( + "CREATE TABLE atlas_city_names ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + body_id TEXT NOT NULL, + name TEXT NOT NULL, + kind TEXT NOT NULL DEFAULT 'city', + economic_role TEXT, + population INTEGER NOT NULL, + settlement_class TEXT + ); + CREATE TABLE corporations ( + corp_id TEXT PRIMARY KEY, + proper_name TEXT NOT NULL, + hq_placement TEXT, + headquarters_body TEXT + );", + ) + .expect("create tables"); + for (name, pop, sclass) in [ + ("Hero City", 2_000_000i64, Some("NameLocked")), + ("Plain Town", 300_000, Some("PopulationBudget")), + ("Weird Row", 50_000, Some("TotallyBogus")), // unknown → default + ("Gate Corporation", 900_000, Some("PopulationBudget")), + ] { + conn.execute( + "INSERT INTO atlas_city_names + (body_id, name, economic_role, population, settlement_class) + VALUES ('PlanetX', ?1, 'manufacturing', ?2, ?3)", + rusqlite::params![name, pop, sclass], + ) + .expect("insert city"); + } + conn.execute( + "INSERT INTO corporations (corp_id, proper_name, hq_placement, headquarters_body) + VALUES ('gate-corporation', 'Gate Corporation', 'Standalone', 'PlanetX')", + [], + ) + .expect("insert corp"); + drop(conn); + + let cities = read_cities(&path, "PlanetX").expect("read"); + assert_eq!(cities.len(), 4); + let by_name = |n: &str| cities.iter().find(|c| c.name == n).unwrap(); + assert_eq!( + by_name("Hero City").settlement_class, + SettlementClass::NameLocked, + "baked NameLocked must be honoured, not flattened to the default" + ); + assert_eq!( + by_name("Plain Town").settlement_class, + SettlementClass::PopulationBudget + ); + assert_eq!( + by_name("Weird Row").settlement_class, + SettlementClass::PopulationBudget, + "unknown class text falls back to the default" + ); + assert!( + by_name("Gate Corporation").is_standalone_hq, + "registered Standalone corp row is flagged via the shared join" + ); + assert!( + !by_name("Plain Town").is_standalone_hq, + "unregistered rows stay unflagged" + ); + } } diff --git a/server/src/atlas/cascade.rs b/server/src/atlas/cascade.rs index 70c9691c8..33aedccca 100644 --- a/server/src/atlas/cascade.rs +++ b/server/src/atlas/cascade.rs @@ -689,4 +689,109 @@ mod tests { edge_count ); } + + /// PR #178 H3 — the RailHeadFacing wiring end-to-end (T-1076 §4): after the + /// RoadGraph layer runs, the cascade must have mutated + /// `layer3.placements[..].founding_orientation` to `RailHeadFacing` for + /// exactly the settlements that are high-connectivity junctions + /// (degree ≥ 3), and for no others. A regression that drops the + /// `assign_railhead_orientations` call (or runs it before the graph + /// exists) fails here mechanically. + #[test] + fn cascade_assigns_railhead_orientation_at_junctions() { + use crate::atlas::attractor_matching::CityRecord; + use crate::atlas::road_graph::{RoadNodeKind, JUNCTION_DEGREE}; + use crate::simulation::generator::{FoundingOrientation, SettlementClass}; + + // A plus-shaped landmass (arms meeting at the centre, ocean elsewhere): + // settlements string along the arms, so the trunk MST must branch where + // the arms meet — diagnosed to yield exactly 2 degree-3 settlement + // junctions with 8 cities. Deterministic: the junction requirement + // below is a stable fixture property, not flakiness. (The default + // slope fixture never branches — its coastal attractors form a chain, + // and snapped minors raise Junction-node degrees, not settlement + // degrees.) + let (width, height) = (64u32, 32u32); + let mut data = vec![0.05f32; (width * height) as usize]; // ocean + for r in 0..height { + for c in 0..width { + let in_v_arm = (24..40).contains(&c); // vertical arm + let in_h_arm = (12..20).contains(&r); // horizontal arm + if in_v_arm || in_h_arm { + data[(r * width + c) as usize] = 0.6; + } + } + } + let cross_hm = crate::atlas::heightmap::BodyHeightmap { + body_id: "test_body".into(), + width, + height, + data, + sea_level: 0.3, + }; + + let cities: Vec = (1..=8u64) + .map(|id| CityRecord { + city_id: id, + name: format!("City{id}"), + settlement_class: SettlementClass::PopulationBudget, + population: 1_000_000 - (id as i64) * 1_000, // hubs = lowest 6 ids + economic_role: "manufacturing".into(), + is_capital: false, + is_standalone_hq: false, + }) + .collect(); + + let snap = run_cascade_from_heightmap( + body_seed(), + cross_hm, + &cities, + Some("independent"), + None, + CascadeLayer::RoadGraph, + ); + let graph = snap.road_graph.as_ref().expect("RoadGraph layer ran"); + let junction_city_ids: Vec = graph + .high_connectivity_junctions() + .iter() + .filter_map(|&i| graph.nodes[i].city_id) + .collect(); + assert!( + !junction_city_ids.is_empty(), + "fixture must produce at least one degree ≥ {JUNCTION_DEGREE} settlement \ + junction — if this fires the fixture changed, not the wiring" + ); + + // The wiring assertion, both directions: junction settlements carry + // RailHeadFacing; every other settlement does not. + let l3 = snap.layer3.as_ref().expect("Layer 3 ran"); + for p in &l3.placements { + let is_junction = junction_city_ids.contains(&p.city_id); + let is_rail = matches!( + p.founding_orientation, + FoundingOrientation::RailHeadFacing { .. } + ); + assert_eq!( + is_junction, is_rail, + "city {} junction={} but rail_facing={} — cascade wiring broken", + p.city_id, is_junction, is_rail + ); + } + // And the mutated placements are what BodyWorldState carries forward. + let junction_count = junction_city_ids.len(); + let state = snap.into_body_world_state(); + let rail_count = state + .placements + .iter() + .filter(|p| { + matches!( + p.founding_orientation, + FoundingOrientation::RailHeadFacing { .. } + ) + }) + .count(); + assert_eq!(rail_count, junction_count); + // Silence unused-import warning when the filter above changes. + let _ = RoadNodeKind::Settlement; + } } diff --git a/server/src/atlas/city_context_reader.rs b/server/src/atlas/city_context_reader.rs index 0d8d703fb..ad11903c4 100644 --- a/server/src/atlas/city_context_reader.rs +++ b/server/src/atlas/city_context_reader.rs @@ -62,6 +62,25 @@ use crate::simulation::generator::{ ProductionUbiquity, SettingType, SettlementClass, WorldTier, }; +// --------------------------------------------------------------------------- +// Shared SQL fragments +// --------------------------------------------------------------------------- + +/// The D-242 standalone-corp-HQ LEFT JOIN (T-1076 §1; single source of truth, +/// PR #178 T2). Marks an `atlas_city_names` row (aliased `acn`) as a Standalone +/// corp-HQ company town when a `corporations` row (aliased `c`) matches on +/// `hq_placement = 'Standalone'`, `headquarters_body = body_id`, and +/// `proper_name = name` — the exact shape +/// `populate_standalone_hq_settlements` (economy_import/corporations.py) emits +/// settlement rows with. Composed into a query whose FROM clause aliases +/// `atlas_city_names AS acn`; the flag is read as `c.corp_id IS NOT NULL`. +/// Used by [`CityContextReader::read_body_settlements`] and the believability +/// harness's `read_cities` — extend both if the join shape ever changes. +pub(crate) const STANDALONE_HQ_JOIN_SQL: &str = "LEFT JOIN corporations AS c + ON c.hq_placement = 'Standalone' + AND c.headquarters_body = acn.body_id + AND c.proper_name = acn.name"; + // --------------------------------------------------------------------------- // Error type // --------------------------------------------------------------------------- @@ -266,14 +285,12 @@ impl CityContextReader { /// any malformed field), this method is best-effort: it never fails on row /// content, only on a DB/connection error. /// - /// `is_standalone_hq` (T-1076 §1): a LEFT JOIN against `corporations` - /// marks the rows that are D-242 Standalone corp-HQ company towns - /// (`hq_placement = 'Standalone'`, `headquarters_body = body_id`, - /// `proper_name = name` — the exact shape `populate_standalone_hq_settlements` - /// emits them with). The road-graph hub rule demotes these to minor nodes - /// regardless of population. The believability harness's own settlement - /// read (`believability::read_cities`) performs the identical join — keep - /// the two in lockstep. + /// `is_standalone_hq` (T-1076 §1): the [`STANDALONE_HQ_JOIN_SQL`] LEFT JOIN + /// marks the rows that are D-242 Standalone corp-HQ company towns. The + /// road-graph hub rule demotes these to minor nodes regardless of + /// population. The believability harness's own settlement read + /// (`believability::read_cities`) composes the same shared constant — + /// single source of truth for the join (PR #178 T2). pub fn read_body_settlements( &self, body_id: &str, @@ -282,19 +299,17 @@ impl CityContextReader { .conn .lock() .map_err(|e| CityContextReadError::Db(format!("mutex poisoned: {e}")))?; + let sql = format!( + "SELECT acn.id, acn.name, acn.economic_role, acn.population, + acn.settlement_class, COALESCE(acn.kind, 'city'), + c.corp_id IS NOT NULL + FROM atlas_city_names AS acn + {STANDALONE_HQ_JOIN_SQL} + WHERE acn.body_id = ?1 + ORDER BY acn.id" + ); let mut stmt = conn - .prepare( - "SELECT acn.id, acn.name, acn.economic_role, acn.population, - acn.settlement_class, COALESCE(acn.kind, 'city'), - c.corp_id IS NOT NULL - FROM atlas_city_names AS acn - LEFT JOIN corporations AS c - ON c.hq_placement = 'Standalone' - AND c.headquarters_body = acn.body_id - AND c.proper_name = acn.name - WHERE acn.body_id = ?1 - ORDER BY acn.id", - ) + .prepare(&sql) .map_err(|e| CityContextReadError::Db(e.to_string()))?; let rows = stmt .query_map([body_id], |row| { diff --git a/server/src/atlas/road_graph.rs b/server/src/atlas/road_graph.rs index c90d8c3f2..99c7bdc6f 100644 --- a/server/src/atlas/road_graph.rs +++ b/server/src/atlas/road_graph.rs @@ -110,6 +110,10 @@ pub const JUNCTION_DEGREE: u16 = 3; /// diagonal ([`RouteGrid::body_scale`]) — the same measure every other /// workshop tunable in this file keys off (`SECONDARY_*_FRAC`, /// `LONG_HAUL_FRAC`); physical radius is not available to this layer. +/// Fixed-grid premise: revisit when [D-243]'s elastic planetary seam gives +/// bodies varying grid sizes (`round(2πR / 204.8 km)` regions) — the diagonal +/// then genuinely scales with `body_radius_km` and this constant starts doing +/// real per-body work (PR #178 T1). const HUB_SPACING_DIAG_PX: f64 = 64.0; /// Floor for the hub cap: small grids keep at least this many trunk hubs so @@ -590,13 +594,34 @@ fn project_onto_segment(p: (u16, u16), a: (u16, u16), b: (u16, u16)) -> (f64, (u (d2, (jr.round() as u16, jc.round() as u16)) } +/// Geometric length of a polyline in grid px — Euclidean segment lengths +/// summed in path order (fixed order + correctly-rounded f64 ops, so the sum +/// is bit-reproducible per the module's determinism doctrine). +fn polyline_len(path: &[(u16, u16)]) -> f64 { + let mut total = 0.0; + for w in path.windows(2) { + total += euclid( + (w[0].0 as f64, w[0].1 as f64), + (w[1].0 as f64, w[1].1 as f64), + ); + } + total +} + /// Split `edges[ei]` at `jpos` on segment `si`: a new [`RoadNodeKind::Junction`] /// node replaces the single edge with two halves meeting at the junction. /// Both halves inherit the parent's maintenance/named-route identity; -/// `length_cells` splits proportionally by polyline vertex count (a routed- -/// length proxy — the halves' true routed lengths are not re-measured). -/// Returns the junction's node index. Caller guarantees `jpos` is not an -/// endpoint of the edge's path (guarded at the call site). +/// `length_cells` splits **geometric-distance proportionally** — each half +/// gets the parent's routed length scaled by its polyline's share of the +/// total geometry (PR #178 H1: the earlier vertex-count proxy went +/// all-or-nothing on 2-point parents — `l1 = full, l2 = 0` wherever the +/// junction fell — and 2-point parents are routine: every snap spur is one, +/// so chained snaps always hit it). The halves always sum exactly to the +/// parent's `length_cells` (l2 is the remainder), and an interior junction +/// on a parent with `length_cells > 0` gives both halves a non-zero share +/// whenever its geometry does. Returns the junction's node index. Caller +/// guarantees `jpos` is not an endpoint of the edge's path (guarded at the +/// call site). fn split_edge_at( nodes: &mut Vec, edges: &mut Vec, @@ -626,8 +651,13 @@ fn split_edge_at( path2.extend_from_slice(&old.path[si + 1..]); } - let total_segs = (old.path.len() - 1).max(1) as u32; - let l1 = old.length_cells * (path1.len().saturating_sub(1) as u32) / total_segs; + let (len1, len2) = (polyline_len(&path1), polyline_len(&path2)); + let total = len1 + len2; + let l1 = if total > 0.0 { + ((old.length_cells as f64 * len1 / total).round() as u32).min(old.length_cells) + } else { + 0 // fully degenerate geometry (all points coincide) — nothing to apportion + }; let l2 = old.length_cells - l1; edges[ei] = norm_edge(RoadEdge { @@ -1784,4 +1814,138 @@ mod tests { )); } } + + // ─── PR #178 H1/H2 — edge-split length arithmetic ──────────────────────── + + /// Direct unit test of the H1 bug shape: a 2-POINT parent path with real + /// `length_cells`. The old vertex-count proxy computed + /// `l1 = length × (path1.len()-1) / total_segs` = all-or-nothing whenever + /// `total_segs == 1`; the geometric split apportions by where the junction + /// actually falls. + #[test] + fn split_edge_at_two_point_parent_splits_geometrically() { + let mk_node = |pos: (u16, u16)| RoadNode { + city_id: Some(99), + position: pos, + kind: RoadNodeKind::Settlement, + degree: 0, + parent_edge: None, + is_hub: true, + }; + // Midpoint split: 10 cells → 5 + 5. + let mut nodes = vec![mk_node((10, 10)), mk_node((10, 30))]; + let mut edges = vec![RoadEdge { + from: 0, + to: 1, + path: vec![(10, 10), (10, 30)], + length_cells: 10, + maintenance: MaintenanceAuthority::Communal, + named_route_id: None, + is_rail: false, + }]; + let jn = split_edge_at(&mut nodes, &mut edges, 0, 0, (10, 20)); + assert_eq!(nodes[jn].kind, RoadNodeKind::Junction); + assert_eq!(edges.len(), 2); + assert_eq!(edges[0].length_cells, 5, "midpoint → equal halves"); + assert_eq!(edges[1].length_cells, 5); + + // Quarter-point split: 10 cells at t=0.25 → 2/3 + remainder split, but + // NEVER all-or-nothing: both halves non-zero, summing to the parent. + let mut nodes = vec![mk_node((10, 10)), mk_node((10, 30))]; + let mut edges = vec![RoadEdge { + from: 0, + to: 1, + path: vec![(10, 10), (10, 30)], + length_cells: 10, + maintenance: MaintenanceAuthority::Communal, + named_route_id: None, + is_rail: false, + }]; + split_edge_at(&mut nodes, &mut edges, 0, 0, (10, 15)); + let (l1, l2) = (edges[0].length_cells, edges[1].length_cells); + assert_eq!(l1 + l2, 10, "halves always sum to the parent"); + assert!(l1 > 0 && l2 > 0, "interior split is never all-or-nothing"); + assert!(l1 < l2, "the shorter geometric half gets the smaller share"); + } + + /// H2(a) — chained snap: minor B snaps onto minor A's already-created spur + /// (the accreting behavior), splitting a 2-point spur path. Also the H2(b) + /// determinism pin on a scenario that actually exercises attach_minors + /// (the pre-existing determinism test's 5 placements all become hubs under + /// HUB_CAP_MIN = 6, so it never reaches the attach path). + #[test] + fn chained_snap_onto_earlier_spur_and_attach_determinism() { + let hm = flat_hm(64, 32); + let ta = ta_for(&hm); + let mut placements: Vec = vec![ + placement(1, (16, 4), PoliticalArchetype::Pioneer), + placement(2, (16, 60), PoliticalArchetype::Pioneer), + placement(3, (4, 4), PoliticalArchetype::Pioneer), + placement(4, (4, 60), PoliticalArchetype::Pioneer), + placement(5, (28, 4), PoliticalArchetype::Pioneer), + placement(6, (28, 60), PoliticalArchetype::Pioneer), + ]; + for p in placements.iter_mut() { + p.population = 1_000_000; // the six trunk hubs + } + // Minor A: 8 px above the row-16 trunk edge → boundary snap; its spur + // runs down col 32 from the trunk to (8,32). + let mut minor_a = placement(7, (8, 32), PoliticalArchetype::Pioneer); + minor_a.population = 10_000; + // Minor B: 3 px from A's spur segment, 4 px from the trunk → B's + // nearest edge is the spur an earlier attachment created. + let mut minor_b = placement(8, (12, 35), PoliticalArchetype::Pioneer); + minor_b.population = 5_000; + placements.push(minor_a); + placements.push(minor_b); + + let run = || { + build_road_graph( + &placements, + &ta, + &[], + 64, + 32, + &TerritorialStatus::FrontierUnclaimed, + &[], + ) + }; + let g = run(); + + let junctions: Vec = g + .nodes + .iter() + .enumerate() + .filter(|(_, n)| n.kind == RoadNodeKind::Junction) + .map(|(i, _)| i) + .collect(); + assert_eq!( + junctions.len(), + 2, + "A splits the trunk, B splits A's spur — two junctions" + ); + // B's junction sits ON A's spur (col 32, strictly between the trunk + // row and A's row) — proving the chain hit a 2-point spur parent. + let chained = junctions + .iter() + .map(|&ji| g.nodes[ji].position) + .find(|p| p.1 == 32 && p.0 > 8 && p.0 < 16) + .expect("a junction must sit interior to A's spur on col 32"); + assert_eq!(chained.1, 32); + // Both minors are connected; every edge honours the invariants + // (including the split halves of the 2-point spur). + for mi in [6usize, 7usize] { + assert!( + g.edges.iter().any(|e| e.from == mi || e.to == mi), + "minor node {mi} must be attached" + ); + } + for e in &g.edges { + assert!(e.from < e.to); + assert_eq!(g.nodes[e.from].position, *e.path.first().unwrap()); + assert_eq!(g.nodes[e.to].position, *e.path.last().unwrap()); + } + // H2(b): the whole accreting attach sequence is deterministic. + assert_eq!(run(), run(), "attach_minors must be deterministic"); + } }