fix(simulation): PR #197 review round — real seaward chord for Mouth edges (Hoshe #1/#2, Tyre 1/2)
The batch's real bug: extract_river_network computed the seaward neighbor (nr,nc) to decide the MOUTH sentinel then discarded it, and build_edges placeholder-pointed mouth edges at themselves — zero chord, invent_course's degenerate 1-point return, resolve_mouth_ terminus dead code on real data, ALL real mouths resolving None, and (because Ruling 3g retired the D/Q clip on the promise of real termini) mouths vanishing at District/Quarter. The second instance of the threw-away-the-answer anti-pattern Ruling 2b fixed for interior pointers. Fix: river_seaward: Vec<(u16,u16)> on RiverNetwork (additive, serde-default, parallel array; meaningful only at MOUTH entries), captured in the same extraction pass; build_edges gives Mouth edges the real one-D8-step chord. Permanent acceptance: all_real_gj1c_mouths_resolve_to_mouth_terminus_not_none — 3/3, revert- verified failing at the golden's first mouth (38,47). Mouth golden coverage added (river_course_golden gains district_mouth/quarter_mouth samples + non-degeneracy test; the previously Interior-only filter gap closed). Tyre 1: the land-probe's dead dx/len*len arithmetic replaced — station_spacing_m threaded through the crop path, probe steps one real cell spacing, comment reconciled. Tyre 2: boxing comment reattributed to variant-size balancing (Layer1Output retention lives on TerrainAnalysisCache, not BodyWorldState). Hoshe #4: cascade_golden's doc now states the attractor cascade accurately (count-parity, not byte-identity — water_dist seeds from mouths). Full cargo test green; goldens re-pinned deliberately; bench +3.0%. Tickets: T-1170 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1237,6 +1237,11 @@ fn invent_courses_near_window(
|
||||
/// Crop the window's already-invented courses ([`invent_courses_near_window`])
|
||||
/// to the wire [`RiverCourse`] shape (Ruling 3h) — window rect + one station
|
||||
/// beyond each edge, terminus resolution (A3, Ruling 3e/3f).
|
||||
///
|
||||
/// `station_spacing_m` is the rung's own cell spacing (`granularity.spacing_m()`
|
||||
/// — District 2,048 m / Quarter 512 m) — threaded to [`resolve_mouth_terminus`]'s
|
||||
/// land-at-final-anchor probe, which extends "one cell length" (Ruling 3e's own
|
||||
/// words), not one Stage-B segment length (Tyre, PR #197 review issue 1).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn crop_courses_for_wire(
|
||||
invented: &[InventedCourse],
|
||||
@@ -1247,6 +1252,7 @@ fn crop_courses_for_wire(
|
||||
ta: &crate::atlas::features::TerrainAnalysis,
|
||||
climate: &crate::atlas::district_profile::ClimateConstants,
|
||||
min_wavelength_m: f64,
|
||||
station_spacing_m: f64,
|
||||
) -> Vec<RiverCourse> {
|
||||
invented
|
||||
.iter()
|
||||
@@ -1260,6 +1266,7 @@ fn crop_courses_for_wire(
|
||||
ta,
|
||||
climate,
|
||||
min_wavelength_m,
|
||||
station_spacing_m,
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
@@ -1280,6 +1287,7 @@ fn crop_course_to_window(
|
||||
ta: &crate::atlas::features::TerrainAnalysis,
|
||||
climate: &crate::atlas::district_profile::ClimateConstants,
|
||||
min_wavelength_m: f64,
|
||||
station_spacing_m: f64,
|
||||
) -> Option<RiverCourse> {
|
||||
let (x0, y0, x1, y1) = window_rect;
|
||||
let inside = |p: &(f64, f64)| p.0 >= x0 && p.0 <= x1 && p.1 >= y0 && p.1 <= y1;
|
||||
@@ -1328,6 +1336,7 @@ fn crop_course_to_window(
|
||||
ta,
|
||||
climate,
|
||||
min_wavelength_m,
|
||||
station_spacing_m,
|
||||
) {
|
||||
Some(mouth_point) => {
|
||||
// Replace the cropped course's tail with the resolved
|
||||
@@ -1373,6 +1382,12 @@ const MOUTH_BISECT_ITERATIONS: u32 = 6;
|
||||
/// probe past the final anchor) samples water, returns `None` — the
|
||||
/// degenerate "drawn coast receded past this edge" case (Ruling 3e), which
|
||||
/// the caller renders with no mouth flag.
|
||||
///
|
||||
/// `station_spacing_m` is the rung's own cell spacing — the probe extends
|
||||
/// exactly "one cell length" past the final anchor (Ruling 3e's own words),
|
||||
/// in the direction of the final Stage-B segment, but scaled to
|
||||
/// `station_spacing_m` rather than that segment's own (possibly much
|
||||
/// shorter, near-zero at a taper-to-zero anchor) length.
|
||||
fn resolve_mouth_terminus(
|
||||
course: &InventedCourse,
|
||||
seed: SeedChain,
|
||||
@@ -1381,6 +1396,7 @@ fn resolve_mouth_terminus(
|
||||
ta: &crate::atlas::features::TerrainAnalysis,
|
||||
climate: &crate::atlas::district_profile::ClimateConstants,
|
||||
min_wavelength_m: f64,
|
||||
station_spacing_m: f64,
|
||||
) -> Option<(f64, f64)> {
|
||||
let is_water = |p: (f64, f64)| -> bool {
|
||||
// `&[]`: the mouth-termination water-verdict probe has no use for
|
||||
@@ -1418,16 +1434,19 @@ fn resolve_mouth_terminus(
|
||||
}
|
||||
prev_land = p;
|
||||
}
|
||||
// Final anchor still land: extend one cell length along the segment's
|
||||
// own direction as a single probe (Ruling 3e: "extend along the D8
|
||||
// direction up to one cell length probing").
|
||||
// Final anchor still land: extend ONE CELL LENGTH (`station_spacing_m` —
|
||||
// Ruling 3e's own words, "up to one cell length probing", not one
|
||||
// Stage-B segment length, which can be much shorter near a
|
||||
// taper-to-zero anchor — Tyre, PR #197 review issue 1) along the final
|
||||
// segment's own direction, as a single probe.
|
||||
if pts.len() >= 2 {
|
||||
let a = pts[pts.len() - 2];
|
||||
let b = pts[pts.len() - 1];
|
||||
let (dx, dy) = (b.0 - a.0, b.1 - a.1);
|
||||
let len = (dx * dx + dy * dy).sqrt();
|
||||
if len > 1e-6 {
|
||||
let probe = (b.0 + dx / len * len, b.1 + dy / len * len); // one more segment-length step
|
||||
let (ux, uy) = (dx / len, dy / len); // unit direction of the final segment
|
||||
let probe = (b.0 + ux * station_spacing_m, b.1 + uy * station_spacing_m);
|
||||
if is_water(probe) {
|
||||
return Some(bisect_to_waterline(b, probe, is_water));
|
||||
}
|
||||
@@ -1589,6 +1608,7 @@ pub fn build_district_window_layer(
|
||||
ta,
|
||||
climate,
|
||||
min_wavelength_m,
|
||||
step_m,
|
||||
);
|
||||
|
||||
DistrictWindowLayer {
|
||||
@@ -1686,6 +1706,7 @@ fn build_district_window_layer_serial(
|
||||
ta,
|
||||
climate,
|
||||
min_wavelength_m,
|
||||
step_m,
|
||||
);
|
||||
DistrictWindowLayer {
|
||||
center,
|
||||
@@ -2946,6 +2967,144 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// **T-1170 PR #197 review, Hoshe #1 acceptance test (blocking, permanent
|
||||
/// — not a throwaway probe).** Every real Mouth edge on the GJ1c golden
|
||||
/// fixture (256×128 downsample, the SAME fixture `cascade_golden.rs`
|
||||
/// pins — 3 mouths: `[(38,47), (38,98), (124,239)]`) must resolve
|
||||
/// `CourseTerminus::Mouth`, not `CourseTerminus::None`.
|
||||
///
|
||||
/// **What this guards:** before the fix, `build_edges` set
|
||||
/// `downstream = upstream` for every Mouth edge (a same-cell
|
||||
/// placeholder — the SAME discard-then-need-it-later anti-pattern
|
||||
/// Ruling 2b's `river_downstream` field fixed for interior pointers,
|
||||
/// applied a second time to the seaward neighbor `extract_river_network`
|
||||
/// already computes and then threw away). That zeroed the chord
|
||||
/// (`chord_m < 1.0`), which tripped `invent_course`'s degenerate
|
||||
/// single-point return, which made `resolve_mouth_terminus`'s station
|
||||
/// walk a no-op (a 1-point course can't reach the `pts.len() >= 2`
|
||||
/// fallback probe either) — all 3 real GJ1c mouths silently resolved
|
||||
/// `CourseTerminus::None` instead of `Mouth`, and since Ruling 3g retired
|
||||
/// the District/Quarter draw-time clip on the promise of real termini,
|
||||
/// mouths would have disappeared entirely at those rungs. The fix:
|
||||
/// `RiverNetwork::river_seaward` (additive, captured in the same
|
||||
/// `extract_river_network` pass) carries the real seaward neighbor
|
||||
/// through to `build_edges`, giving Mouth edges a genuine ~one-cell
|
||||
/// chord to invent a course along.
|
||||
#[test]
|
||||
fn all_real_gj1c_mouths_resolve_to_mouth_terminus_not_none() {
|
||||
use crate::atlas::drainage;
|
||||
use crate::atlas::heightmap::load_heightmap_png;
|
||||
use crate::atlas::river_course;
|
||||
|
||||
let src = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
|
||||
.join("../wiki/star-systems/GJ-1/bodies/GJ1c/heightmap.png");
|
||||
let heightmap =
|
||||
load_heightmap_png(&src, "GJ1c", 0.3).expect("decode committed GJ1c heightmap");
|
||||
let small = heightmap.downsample(256, 128);
|
||||
let dr = drainage::analyze(&small.data, small.width, small.height, small.sea_level);
|
||||
let ta = crate::atlas::features::TerrainAnalysis::analyze(&small, &dr);
|
||||
let rn = &dr.river_network;
|
||||
|
||||
let params = crate::atlas::district_profile::BodyParams {
|
||||
hydrosphere: Some("ocean".into()),
|
||||
atmosphere: Some("breathable".into()),
|
||||
planet_class: Some("temperate".into()),
|
||||
body_radius_km: Some(6371.0),
|
||||
..Default::default()
|
||||
};
|
||||
let climate = crate::atlas::district_profile::ClimateConstants::default();
|
||||
let seed = SeedChain::root(0xC0FFEE_u64).derive(SeedDomain::Body, 1);
|
||||
let station_spacing_m = DISTRICT_M as f64;
|
||||
|
||||
let edges = river_course::build_edges(rn);
|
||||
let mouth_edges: Vec<_> = edges
|
||||
.iter()
|
||||
.filter(|e| e.terminus == river_course::EdgeTerminusKind::Mouth)
|
||||
.collect();
|
||||
assert_eq!(
|
||||
mouth_edges.len(),
|
||||
rn.mouths.len(),
|
||||
"build_edges must produce exactly one Mouth edge per RiverNetwork.mouths entry"
|
||||
);
|
||||
assert_eq!(
|
||||
mouth_edges.len(),
|
||||
3,
|
||||
"GJ1c at this downsample is expected to have 3 real mouths (matches the \
|
||||
committed cascade_golden fixture) — if this count changes, re-verify against \
|
||||
tests/golden/cascade_layer1.json before updating this assertion"
|
||||
);
|
||||
|
||||
let mut resolved_mouth_count = 0;
|
||||
for edge in &mouth_edges {
|
||||
// Sanity: the fix means Mouth edges get a real, non-degenerate
|
||||
// chord toward the seaward neighbor — never upstream==downstream.
|
||||
assert_ne!(
|
||||
edge.upstream, edge.downstream,
|
||||
"Mouth edge {:?} still has a same-cell placeholder downstream — \
|
||||
river_seaward threading regressed",
|
||||
edge.edge_id
|
||||
);
|
||||
|
||||
let course =
|
||||
river_course::invent_course(seed, edge, &ta, ¶ms, station_spacing_m, 0.0);
|
||||
assert!(
|
||||
course.points.len() >= 2,
|
||||
"Mouth edge {:?} invented a degenerate {}-point course — the chord-length \
|
||||
fix regressed",
|
||||
edge.edge_id,
|
||||
course.points.len()
|
||||
);
|
||||
|
||||
// Window rect generous enough to contain the whole short mouth
|
||||
// course (mouths are ~one cell chord, so a wide margin is cheap).
|
||||
let (min_x, max_x) = course
|
||||
.points
|
||||
.iter()
|
||||
.fold((f64::INFINITY, f64::NEG_INFINITY), |(lo, hi), p| {
|
||||
(lo.min(p.0), hi.max(p.0))
|
||||
});
|
||||
let (min_y, max_y) = course
|
||||
.points
|
||||
.iter()
|
||||
.fold((f64::INFINITY, f64::NEG_INFINITY), |(lo, hi), p| {
|
||||
(lo.min(p.1), hi.max(p.1))
|
||||
});
|
||||
let margin = 50_000.0;
|
||||
let window_rect = (min_x - margin, min_y - margin, max_x + margin, max_y + margin);
|
||||
|
||||
let wire = crop_course_to_window(
|
||||
&course,
|
||||
window_rect,
|
||||
seed,
|
||||
"GJ1c",
|
||||
¶ms,
|
||||
&ta,
|
||||
&climate,
|
||||
0.0,
|
||||
station_spacing_m,
|
||||
)
|
||||
.unwrap_or_else(|| panic!("Mouth edge {:?} cropped to nothing in its own window", edge.edge_id));
|
||||
|
||||
assert_eq!(
|
||||
wire.terminus,
|
||||
CourseTerminus::Mouth,
|
||||
"Mouth edge {:?} (upstream {:?}, downstream {:?}) resolved {:?} instead of \
|
||||
CourseTerminus::Mouth",
|
||||
edge.edge_id,
|
||||
edge.upstream,
|
||||
edge.downstream,
|
||||
wire.terminus
|
||||
);
|
||||
resolved_mouth_count += 1;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
resolved_mouth_count, 3,
|
||||
"acceptance criterion (Hoshe #1): all 3 real GJ1c mouths must resolve \
|
||||
CourseTerminus::Mouth"
|
||||
);
|
||||
}
|
||||
|
||||
/// Clamped-window edge: `n = 1` is the minimum valid window (a single
|
||||
/// district) — no panic, no empty output, exactly one cell per array.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user