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:
Claude Fable 5
@@ -89,6 +89,36 @@ pub struct RiverNetwork {
|
||||
/// array (pre-T-1170 payload/fixture) decodes to empty, never an error.
|
||||
#[serde(default)]
|
||||
pub river_downstream: Vec<u8>,
|
||||
/// Per-`river_cells`-entry seaward neighbor position (T-1170 PR #197
|
||||
/// review, Hoshe #1) — same index/length shape as `river_class`/
|
||||
/// `river_downstream`. **Only meaningful where `river_downstream[i] ==
|
||||
/// RIVER_DOWNSTREAM_MOUTH`**; every other entry (interior direction or
|
||||
/// `EDGE_DRAIN`) carries the placeholder `(0, 0)` and must not be read.
|
||||
///
|
||||
/// **Why this exists — the second instance of the Ruling-2b anti-pattern.**
|
||||
/// `extract_river_network` already computes the real sub-sea-level
|
||||
/// neighbor `(nr, nc)` to decide the `MOUTH` sentinel (the elevation
|
||||
/// check that flips `river_downstream[i]` to
|
||||
/// [`RIVER_DOWNSTREAM_MOUTH`]) — the exact same "true answer already in
|
||||
/// scope, then discarded" shape Ruling 2b fixed for interior D8 pointers
|
||||
/// via `river_downstream` itself. Discarding `(nr, nc)` a second time
|
||||
/// left `river_course::build_edges` with nothing to build a real chord
|
||||
/// from for Mouth edges: it filled `downstream = upstream` as a
|
||||
/// placeholder, which zeroes the chord (`chord_m < 1.0`), which trips
|
||||
/// `invent_course`'s single-point degenerate return, which makes
|
||||
/// `resolve_mouth_terminus`'s station walk a no-op (a 1-point course
|
||||
/// never enters the `pts.len() >= 2` probe branch either) — every Mouth
|
||||
/// edge silently resolved `CourseTerminus::None` instead of `Mouth`.
|
||||
/// Capturing `(nr, nc)` here (one more push per mouth cell — mouths are
|
||||
/// a small fraction of river cells, not a new grid pass) is what makes
|
||||
/// `build_edges` give Mouth edges a real ~one-cell chord toward the raw
|
||||
/// sea, so the termination walk + bisect + one-segment D8-probe fallback
|
||||
/// (Ruling 3e) are actually reachable.
|
||||
///
|
||||
/// `#[serde(default)]` — same additive pattern as `river_class`/
|
||||
/// `river_downstream`.
|
||||
#[serde(default)]
|
||||
pub river_seaward: Vec<(u16, u16)>,
|
||||
}
|
||||
|
||||
/// [`RiverNetwork::river_downstream`] sentinel: this river cell's D8 flow
|
||||
|
||||
@@ -351,33 +351,51 @@ fn extract_river_network(
|
||||
// `RIVER_THRESHOLD`, but handled the same as `EDGE_DRAIN` (no downstream
|
||||
// neighbor to point at, not a sea mouth) rather than crashing the
|
||||
// pointer's "always points somewhere real" contract.
|
||||
//
|
||||
// river_seaward (T-1170 PR #197 review, Hoshe #1): captured in the SAME
|
||||
// pass, parallel to river_downstream — `(nr, nc)` is already computed
|
||||
// here to decide the MOUTH sentinel; discarding it after the elevation
|
||||
// check (the former code) was the second instance of the exact
|
||||
// discard-then-need-it-later anti-pattern Ruling 2b fixed for
|
||||
// river_downstream itself. Non-mouth entries get the `(0, 0)` placeholder
|
||||
// (documented on the field as unreadable outside the MOUTH case).
|
||||
let mut mouths: Vec<(u16, u16)> = Vec::new();
|
||||
let river_downstream: Vec<u8> = (0..n)
|
||||
.filter(|&i| is_river[i])
|
||||
.map(|i| {
|
||||
let r = i / w;
|
||||
let c = i % w;
|
||||
let k = fdir[i];
|
||||
if k < 0 {
|
||||
// No outflow at all — edge/flat-peak. Not a mouth (Ruling 3f).
|
||||
return RIVER_DOWNSTREAM_EDGE_DRAIN;
|
||||
}
|
||||
let (dr, dc) = D8[k as usize];
|
||||
let nr = r as i32 + dr;
|
||||
let nc = (c as i32 + dc).rem_euclid(w as i32) as usize;
|
||||
if nr < 0 || nr >= h as i32 {
|
||||
// Flow direction points off the polar edge — a grid artifact,
|
||||
// not a mouth (Ruling 3f, the pole-edge-drain fix).
|
||||
return RIVER_DOWNSTREAM_EDGE_DRAIN;
|
||||
}
|
||||
if elevation[nr as usize * w + nc] < sea_level {
|
||||
// Flows into a sub-sea-level cell — a real mouth.
|
||||
mouths.push((r as u16, c as u16));
|
||||
return RIVER_DOWNSTREAM_MOUTH;
|
||||
}
|
||||
k as u8
|
||||
})
|
||||
.collect();
|
||||
let mut river_downstream: Vec<u8> = Vec::new();
|
||||
let mut river_seaward: Vec<(u16, u16)> = Vec::new();
|
||||
for i in 0..n {
|
||||
if !is_river[i] {
|
||||
continue;
|
||||
}
|
||||
let r = i / w;
|
||||
let c = i % w;
|
||||
let k = fdir[i];
|
||||
if k < 0 {
|
||||
// No outflow at all — edge/flat-peak. Not a mouth (Ruling 3f).
|
||||
river_downstream.push(RIVER_DOWNSTREAM_EDGE_DRAIN);
|
||||
river_seaward.push((0, 0));
|
||||
continue;
|
||||
}
|
||||
let (dr, dc) = D8[k as usize];
|
||||
let nr = r as i32 + dr;
|
||||
let nc = (c as i32 + dc).rem_euclid(w as i32) as usize;
|
||||
if nr < 0 || nr >= h as i32 {
|
||||
// Flow direction points off the polar edge — a grid artifact,
|
||||
// not a mouth (Ruling 3f, the pole-edge-drain fix).
|
||||
river_downstream.push(RIVER_DOWNSTREAM_EDGE_DRAIN);
|
||||
river_seaward.push((0, 0));
|
||||
continue;
|
||||
}
|
||||
if elevation[nr as usize * w + nc] < sea_level {
|
||||
// Flows into a sub-sea-level cell — a real mouth. Capture the
|
||||
// seaward neighbor position (Hoshe #1) alongside the sentinel.
|
||||
mouths.push((r as u16, c as u16));
|
||||
river_downstream.push(RIVER_DOWNSTREAM_MOUTH);
|
||||
river_seaward.push((nr as u16, nc as u16));
|
||||
continue;
|
||||
}
|
||||
river_downstream.push(k as u8);
|
||||
river_seaward.push((0, 0));
|
||||
}
|
||||
|
||||
RiverNetwork {
|
||||
river_cells,
|
||||
@@ -385,6 +403,7 @@ fn extract_river_network(
|
||||
mouths,
|
||||
river_class,
|
||||
river_downstream,
|
||||
river_seaward,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -272,9 +272,23 @@ pub enum GenCompletion {
|
||||
BodyAnalyzed {
|
||||
body_id: String,
|
||||
/// The computed world state, ready for `BodyWorldStateCache::insert`.
|
||||
/// Boxed: `BodyWorldState` grew past clippy's large-enum-variant
|
||||
/// threshold when it began retaining `Layer1Output` (T-1170 Ruling 4b)
|
||||
/// — the box keeps the enum small while the state moves by pointer.
|
||||
/// Boxed to keep `GenCompletion` variant sizes balanced — the same
|
||||
/// discipline as `SkeletonGenerated`/`ChunkFilled`/`WindowDerived`
|
||||
/// below, all boxed for the same reason (`GenCompletion`'s overall
|
||||
/// size is bounded by its largest unboxed variant; a `BodyWorldState`
|
||||
/// field carried by value here would force every other variant to
|
||||
/// pay for its full stack size on every match/move).
|
||||
///
|
||||
/// **Not this variant's own recent growth** (T-1170 PR #197 review,
|
||||
/// Tyre issue 2 — the prior comment here overattributed the boxing
|
||||
/// rationale): `BodyWorldState` itself only grew by
|
||||
/// `RiverNetwork`'s two new `Vec` fields (`river_downstream`,
|
||||
/// `river_seaward` — a few bytes/river-cell, Ruling 2a / Hoshe #1).
|
||||
/// The much larger `Layer1Output` retention (T-1170 Ruling 4b) lives
|
||||
/// on `TerrainAnalysisCache` — a queue-scoped struct further down
|
||||
/// this file, never part of `BodyWorldState`/`BodyWorldStateCache` at
|
||||
/// all. The box here predates T-1170 and stays for the pre-existing
|
||||
/// variant-size-balancing reason, unrelated to this ticket's growth.
|
||||
state: Box<BodyWorldState>,
|
||||
},
|
||||
SkeletonGenerated {
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -176,18 +176,25 @@ pub fn pack_cell_id(cell: RiverCell) -> u32 {
|
||||
/// always be exactly parallel to `river_cells`, but a mismatched-length
|
||||
/// legacy payload must degrade to "no edges" rather than panic).
|
||||
///
|
||||
/// For `Mouth`/`EdgeDrain` termini there is no real downstream river cell to
|
||||
/// point at — the caller's termination logic (Ruling 3e/3f) walks or
|
||||
/// truncates from the upstream anchor using the edge's own direction, not a
|
||||
/// second real cell. This function still records SOME `downstream` position
|
||||
/// for those edges — one D8 step from the upstream cell in its own flow
|
||||
/// direction, when that direction is known (i.e. `river_downstream < 8`, the
|
||||
/// `Mouth` sentinel case where the OLD code already had a real direction
|
||||
/// before classifying the neighbor as sub-sea-level) — falling back to the
|
||||
/// upstream cell itself (a zero-length probe anchor) for `EdgeDrain`, where
|
||||
/// no direction survived at all. Both are legitimate: Stage A/B degrade
|
||||
/// gracefully to a near-zero-length or short edge, and A3's termination logic
|
||||
/// (layer_proxy) is what actually resolves the real geometry from here.
|
||||
/// **Mouth edges get a REAL seaward chord (T-1170 PR #197 review, Hoshe #1),
|
||||
/// not a same-cell placeholder.** The former code set `downstream = upstream`
|
||||
/// for `Mouth` edges — a zero-length chord (`chord_m < 1.0` in
|
||||
/// [`invent_course`]) that silently tripped the degenerate single-point
|
||||
/// return, which made [`crate::atlas::layer_proxy::resolve_mouth_terminus`]'s
|
||||
/// station walk a no-op (a 1-point course can't even reach that function's
|
||||
/// `pts.len() >= 2` fallback probe) — every Mouth edge resolved
|
||||
/// `CourseTerminus::None` instead of `Mouth`, DESPITE `extract_river_network`
|
||||
/// having already computed the real seaward neighbor to decide the sentinel
|
||||
/// in the first place. `RiverNetwork::river_seaward` (additive, captured in
|
||||
/// the SAME extraction pass) now carries that neighbor through; this
|
||||
/// function reads it for `Mouth` edges, giving them a genuine ~one-cell chord
|
||||
/// toward the raw sea so Stage A/B actually have something to invent and the
|
||||
/// termination walk is reachable.
|
||||
///
|
||||
/// `EdgeDrain` termini still use the upstream cell itself as a zero-length
|
||||
/// placeholder — there is no seaward neighbor for a grid-artifact exit, and
|
||||
/// none is needed: A3's `EdgeDrain` handling (Ruling 3f) never probes for
|
||||
/// water, it just ends the course at the last in-grid station.
|
||||
pub fn build_edges(rn: &RiverNetwork) -> Vec<RiverEdge> {
|
||||
let mut edges = Vec::with_capacity(rn.river_cells.len());
|
||||
for (i, &upstream) in rn.river_cells.iter().enumerate() {
|
||||
@@ -203,14 +210,17 @@ pub fn build_edges(rn: &RiverNetwork) -> Vec<RiverEdge> {
|
||||
let downstream = step_cell(upstream, dr, dc);
|
||||
(downstream, EdgeTerminusKind::Interior)
|
||||
} else if sentinel == RIVER_DOWNSTREAM_MOUTH {
|
||||
// The mouth sentinel is recorded once the D8 walk already found a
|
||||
// real neighbor cell (see `extract_river_network`); we don't have
|
||||
// that direction anymore at this layer, so use the upstream cell
|
||||
// itself as a zero-length placeholder — A3's mouth-walk logic
|
||||
// (layer_proxy, Ruling 3e) resolves the real terminus by sampling
|
||||
// the morphology water verdict outward from `upstream`, not by
|
||||
// trusting this placeholder position.
|
||||
(upstream, EdgeTerminusKind::Mouth)
|
||||
// Real seaward neighbor (Hoshe #1) — falls back to the upstream
|
||||
// cell (the old placeholder) ONLY on a legacy/pre-fix payload
|
||||
// where `river_seaward` is absent or the specific entry is the
|
||||
// unset `(0, 0)` fill AND that happens to differ from a genuine
|
||||
// seaward cell at (0,0) (an acceptable, vanishingly rare
|
||||
// degradation at the map's literal origin — never hit on any
|
||||
// real body, since (0,0) is a pole/edge pixel, never sub-sea
|
||||
// adjacent to an actual river mouth in practice).
|
||||
let seaward = rn.river_seaward.get(i).copied().unwrap_or((0, 0));
|
||||
let downstream = if seaward == (0, 0) { upstream } else { seaward };
|
||||
(downstream, EdgeTerminusKind::Mouth)
|
||||
} else {
|
||||
debug_assert_eq!(sentinel, RIVER_DOWNSTREAM_EDGE_DRAIN);
|
||||
(upstream, EdgeTerminusKind::EdgeDrain)
|
||||
|
||||
@@ -30,6 +30,32 @@
|
||||
//! sea-adjacent mouths (`RIVER_DOWNSTREAM_MOUTH`). `river_cells`/`attractors`
|
||||
//! counts are unchanged (93/256) — this is a pure re-classification + one new
|
||||
//! additive field, not a drainage-algorithm change.
|
||||
//!
|
||||
//! **Attractor cascade correction (T-1170 PR #197 review, Hoshe #4):** the A1
|
||||
//! commit's "pure reclassification" framing overclaimed — `attractors` is
|
||||
//! count-PARITY (256/256), not byte-identical. Mouths dropping 19→3 shrinks
|
||||
//! `TerrainAnalysis::water_dist`'s seed set (`compute_water_dist` seeds from
|
||||
//! `river_network.mouths`), which shifts the water-distance field, which
|
||||
//! feeds `RawAttractor::strength` scoring in `features::extract_attractors`.
|
||||
//! This is a principled, in-scope cascade (the pole-edge cells genuinely
|
||||
//! aren't water-distance sources anymore) — not a bug — but it is a REAL
|
||||
//! field-level change, not a no-op re-tagging. Restated accurately here so
|
||||
//! the record doesn't imply byte-identical attractor output.
|
||||
//!
|
||||
//! **Second deliberate re-pin (T-1170 PR #197 review, Hoshe #1):**
|
||||
//! `RiverNetwork` gained a second additive field, `river_seaward:
|
||||
//! Vec<(u16,u16)>` — the real seaward neighbor position for MOUTH-sentinel
|
||||
//! river cells, captured in the SAME `extract_river_network` pass (the
|
||||
//! elevation check that already computes `(nr, nc)` to decide the MOUTH
|
||||
//! sentinel). Fixes a second instance of the discard-then-need-it-later
|
||||
//! anti-pattern Ruling 2b's `river_downstream` field fixed for interior D8
|
||||
//! pointers: `river_course::build_edges` needs a real seaward cell to give
|
||||
//! Mouth edges a non-degenerate chord (see `river_course.rs`'s
|
||||
//! `build_edges` doc and `layer_proxy::tests::
|
||||
//! all_real_gj1c_mouths_resolve_to_mouth_terminus_not_none` for the full
|
||||
//! bug/fix story). `river_cells`/`mouths`/`attractors` counts unchanged by
|
||||
//! this second re-pin (93/3/256) — purely the new parallel array, 3 non-
|
||||
//! placeholder entries (one per real mouth).
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
|
||||
@@ -559,6 +559,7 @@ fn generate_atlas_layer_response_fixtures() {
|
||||
mouths: vec![(12, 58)],
|
||||
river_class: vec![1, 2],
|
||||
river_downstream: vec![2, 8], // 2=E direction; 8=MOUTH sentinel
|
||||
river_seaward: vec![(0, 0), (12, 60)], // meaningful only for the MOUTH entry
|
||||
},
|
||||
drainage_basins: vec![DrainageBasin {
|
||||
basin_id: 1,
|
||||
|
||||
@@ -8342,6 +8342,380 @@
|
||||
9,
|
||||
2,
|
||||
8
|
||||
],
|
||||
"river_seaward": [
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
39,
|
||||
47
|
||||
],
|
||||
[
|
||||
38,
|
||||
99
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
0,
|
||||
0
|
||||
],
|
||||
[
|
||||
125,
|
||||
240
|
||||
]
|
||||
]
|
||||
}
|
||||
},
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -444,6 +444,20 @@ fn golden_cutoffs_match_the_scale_ladder() {
|
||||
// geometry for an edge from that chain, at both District and Quarter station
|
||||
// spacing, so courses themselves — not just the district-profile fields the
|
||||
// main golden above covers — are regression-pinned.
|
||||
//
|
||||
// **Mouth coverage (T-1170 PR #197 review, Hoshe #2):** the sample set below
|
||||
// also includes a real Mouth-terminus edge from `sample_hm()` (empirically
|
||||
// probed: 4 mouth edges exist in this fixture) — `invent_course`'s raw output
|
||||
// for that edge is pinned here (edge_id/class/terminus/points), closing the
|
||||
// "zero Mouth coverage in any golden" gap at the invention layer. The FULL
|
||||
// end-to-end path (invent → crop → `resolve_mouth_terminus` →
|
||||
// `CourseTerminus::Mouth`) is covered separately and permanently by
|
||||
// `layer_proxy::tests::all_real_gj1c_mouths_resolve_to_mouth_terminus_not_none`
|
||||
// (in-crate, since `crop_course_to_window`/`resolve_mouth_terminus` are
|
||||
// private to `layer_proxy.rs` and unreachable from this integration test) —
|
||||
// that test is the actual Hoshe #1 acceptance criterion (3/3 real mouths);
|
||||
// this golden's job is regression-pinning the raw invented geometry, not
|
||||
// re-proving the crop/resolve path.
|
||||
|
||||
const RIVER_COURSE_GOLDEN_FILE: &str = "tests/golden/river_course_golden.json";
|
||||
|
||||
@@ -481,6 +495,15 @@ fn river_course_golden_samples() -> Vec<GoldenCourseSample> {
|
||||
})
|
||||
.expect("sample_hm() fixture must have at least one interior river edge near (10, 116)");
|
||||
|
||||
// Hoshe #2: a real Mouth-terminus edge, deterministically selected as the
|
||||
// lowest edge_id among the fixture's mouth edges (fixed tie-break, no
|
||||
// hardcoded index).
|
||||
let mouth_target = edges
|
||||
.iter()
|
||||
.filter(|e| e.terminus == river_course::EdgeTerminusKind::Mouth)
|
||||
.min_by_key(|e| e.edge_id)
|
||||
.expect("sample_hm() fixture must have at least one Mouth edge (empirically verified: 4)");
|
||||
|
||||
let mut out = Vec::new();
|
||||
for (rung, spacing_m, min_wl_m) in [
|
||||
("district", DISTRICT_MIN_WL_M, DISTRICT_MIN_WL_M),
|
||||
@@ -498,6 +521,20 @@ fn river_course_golden_samples() -> Vec<GoldenCourseSample> {
|
||||
.map(|p| (p.0.round() as i64, p.1.round() as i64))
|
||||
.collect(),
|
||||
});
|
||||
|
||||
let mouth_course =
|
||||
river_course::invent_course(seed, mouth_target, &ta, ¶ms, spacing_m, min_wl_m);
|
||||
out.push(GoldenCourseSample {
|
||||
rung: format!("{rung}_mouth"),
|
||||
edge_id: mouth_course.edge_id,
|
||||
class: mouth_course.class,
|
||||
terminus: format!("{:?}", mouth_course.terminus),
|
||||
points: mouth_course
|
||||
.points
|
||||
.iter()
|
||||
.map(|p| (p.0.round() as i64, p.1.round() as i64))
|
||||
.collect(),
|
||||
});
|
||||
}
|
||||
out
|
||||
}
|
||||
@@ -567,3 +604,29 @@ fn river_course_rungs_have_different_station_counts() {
|
||||
quarter.points.len()
|
||||
);
|
||||
}
|
||||
|
||||
/// **T-1170 PR #197 review, Hoshe #2 (golden coverage):** the pinned Mouth
|
||||
/// edge samples must be genuine, non-degenerate courses (`terminus == "Mouth"`,
|
||||
/// `points.len() >= 2`) — the direct golden-level check that the Hoshe #1 fix
|
||||
/// (real seaward chord via `RiverNetwork::river_seaward`) reaches this fixture
|
||||
/// too, not just the dedicated GJ1c acceptance test.
|
||||
#[test]
|
||||
fn river_course_mouth_samples_are_non_degenerate() {
|
||||
let samples = river_course_golden_samples();
|
||||
for rung in ["district_mouth", "quarter_mouth"] {
|
||||
let sample = samples
|
||||
.iter()
|
||||
.find(|s| s.rung == rung)
|
||||
.unwrap_or_else(|| panic!("missing golden sample for rung {rung}"));
|
||||
assert_eq!(
|
||||
sample.terminus, "Mouth",
|
||||
"{rung}: build_edges must produce a Mouth-terminus RiverEdge for the pinned target"
|
||||
);
|
||||
assert!(
|
||||
sample.points.len() >= 2,
|
||||
"{rung}: Mouth edge invented a degenerate {}-point course — the chord-length fix \
|
||||
(Hoshe #1) regressed for this fixture",
|
||||
sample.points.len()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user