jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(simulation): Phase-4 hardening + believability gate learns basin-direction variety (T-964) #210

Closed
jpmschweitzer wants to merge 0 commits from test-hardening into main
pull from: test-hardening
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-07-25 19:07:10 +02:00
Owner
Copy Link
Copy Source

Summary

The deferred #953/#963 review gaps, re-verified against current code before implementation (one stale item dropped: the test_sim_determinism wiring already landed via make test-tooling).

Coverage additions (commit 1): per-type attractor reachability fixtures — crafted heightmaps guaranteeing LakeShore (enclosed depression), PassEntrance (saddle), PlainCenter (flat), RiverCrossing (confluence) — plus thin_by_spacing behavior (collision, strict-< boundary, equirectangular column wrap); heightmap 8-bit decode / sea_level passthrough / downsample identity + zero-target early-return; drainage area_pct bit-for-bit determinism + the isolated-basin-fallback divergence comment (Tyre N1, citing the pre-#953 behavior it departs from); the layer1 mountain-branch pairing test (investigated first — the cascade test supplies a mountain pool but only ever asserted river counts: genuine gap); an importer idempotency test covering atlas_city_names AND atlas_feature_names plus the Sol exemption, wired into make test-tooling; oasis_water dilation scaled by GRID_W/512 (Tyre N2, hash-stable).

D-245 gate strengthening (commit 2) — the evidence-backed design call: the D-256/T-1174 finding proved the believability golden byte-identical under a total all-North basin_direction collapse. ContrastMetrics gains land_districts + basin_directions_distinct (over ALL districts, no new derive calls — chosen over the voxel-transect proxy, which washes out at production sample density; both candidates were investigated with scratch fixtures before the call). New criterion 'basin direction variety': pass = land_districts == 0 (the drained-body guard, mirroring the file's nothing-to-vary idiom — a waterworld can't false-fail) || distinct >= 2. A negative test proves the criterion catches the land-bearing all-North regression; golden regenerated and rerun-stable; both validation bodies pass at distinct=2.

Verification

Gate green on push: cargo (fmt/clippy -D warnings/full test incl. the regenerated believability golden), deny, ruff, make test-tooling with the new idempotency test executing. cargo test --lib atlas:: 752/752; believability_harness determinism + golden match confirmed. No client changes on this branch. Reviews next — server-only test/gate work, no eyeball owed (nothing rendered changes).

🤖 Generated with Claude Code

## Summary The deferred #953/#963 review gaps, re-verified against current code before implementation (one stale item dropped: the test_sim_determinism wiring already landed via make test-tooling). **Coverage additions (commit 1):** per-type attractor reachability fixtures — crafted heightmaps guaranteeing LakeShore (enclosed depression), PassEntrance (saddle), PlainCenter (flat), RiverCrossing (confluence) — plus thin_by_spacing behavior (collision, strict-< boundary, equirectangular column wrap); heightmap 8-bit decode / sea_level passthrough / downsample identity + zero-target early-return; drainage area_pct bit-for-bit determinism + the isolated-basin-fallback divergence comment (Tyre N1, citing the pre-#953 behavior it departs from); the layer1 mountain-branch pairing test (investigated first — the cascade test supplies a mountain pool but only ever asserted river counts: genuine gap); an importer idempotency test covering atlas_city_names AND atlas_feature_names plus the Sol exemption, wired into make test-tooling; oasis_water dilation scaled by GRID_W/512 (Tyre N2, hash-stable). **D-245 gate strengthening (commit 2) — the evidence-backed design call:** the D-256/T-1174 finding proved the believability golden byte-identical under a total all-North basin_direction collapse. ContrastMetrics gains land_districts + basin_directions_distinct (over ALL districts, no new derive calls — chosen over the voxel-transect proxy, which washes out at production sample density; both candidates were investigated with scratch fixtures before the call). New criterion 'basin direction variety': pass = land_districts == 0 (the drained-body guard, mirroring the file's nothing-to-vary idiom — a waterworld can't false-fail) || distinct >= 2. A negative test proves the criterion catches the land-bearing all-North regression; golden regenerated and rerun-stable; both validation bodies pass at distinct=2. ## Verification Gate green on push: cargo (fmt/clippy -D warnings/full test incl. the regenerated believability golden), deny, ruff, make test-tooling with the new idempotency test executing. cargo test --lib atlas:: 752/752; believability_harness determinism + golden match confirmed. No client changes on this branch. Reviews next — server-only test/gate work, no eyeball owed (nothing rendered changes). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 4 commits 2026-07-25 19:07:10 +02:00
test(simulation): Phase-4 hardening — deferred #953/#963 review gaps (T-964) f81622bbf0 
The verified-still-open coverage list: per-type attractor reachability
fixtures (LakeShore via enclosed depression, PassEntrance via crafted
saddle, PlainCenter via flat terrain, RiverCrossing via confluence) plus
thin_by_spacing behavior (collision, strict-< boundary, equirectangular
column wrap); heightmap 8-bit decode, sea_level passthrough, downsample
identity and zero-target early-return; drainage area_pct bit-for-bit
determinism plus the isolated-basin-fallback divergence comment (Tyre
N1, citing the pre-#953 behavior it deliberately departs from); the
layer1 mountain-branch pairing test (investigated first — the cascade
test supplies a mountain pool but only ever asserted river counts, a
genuine gap); an importer idempotency test covering atlas_city_names
AND atlas_feature_names plus the Sol exemption, wired into
make test-tooling; and the oasis_water dilation radius scaled by
GRID_W/512 (Tyre N2, hash-stable). One stale item dropped per the
refinement trim (test_sim_determinism wiring — already done).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat(simulation): believability gate learns basin-direction variety (T-964, D-245) e141a595b1 
A real D-245 gate-shape strengthening, not just a test: the D-256/T-1174
finding proved the believability golden byte-identical under a total
all-North basin_direction collapse — every scalar contrast field is
structurally blind to the one field that regressed. ContrastMetrics
gains land_districts and basin_directions_distinct (both over ALL
districts, no new derive calls — the evidence-backed pick over the
voxel-transect proxy, which washes out at production sample density),
and evaluate_criteria gains 'basin direction variety': pass when
land_districts == 0 (the drained-body guard — an all-ocean body has no
cells that can cast a D8 vote per the aggregator's own exclusion rule,
so a uniform default is legitimate, mirroring the file's existing
nothing-to-vary idiom) or distinct >= 2. Negative test proves the
criterion catches the land-bearing all-North regression; vacuous-pass
test proves the waterworld guard. Golden regenerated and rerun-stable;
both validation bodies (Arbour, Edict) pass at distinct=2.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
style(simulation): gate bounce — cargo fmt b929aa27b0 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
style(simulation): gate bounce — clippy unnecessary_cast in test fixture 272d3781d8 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jpmschweitzer added 4 commits 2026-07-25 19:53:32 +02:00
fix(simulation): PR #210 review round — guard boundary, live oasis pinning, unreachability proof (T-964) b165c8038d 
Guard becomes land_districts <= 1 (both reviewers converged — a lone
island definitionally cannot show two distinct directions; same
nothing-to-vary condition one value short), with a lone-island vacuous-
pass fixture; golden confirmed untouched. Oasis scaling adjudicated as
LIVE, not future — GRID_W is already 1024 on main, so ring iterations
change 2/4 -> 4/8 today: extracted a pure oasis_ring_iterations()
helper pinned by tests at both 512 and 1024, and traced exactly why the
determinism hash stayed green (it reads only elevation; the rings touch
only biome — a genuinely different array, not a coincidence). The
drainage merge-logic question answered byte-precisely: zero logic
changed vs main (comment-only diff) — and the deeper dig PROVED the
'isolated basin with another basin to escape to' branch is
mathematically unreachable for any connected grid (contracting vertex
groups of a connected graph cannot disconnect it), so the comment now
states that instead of narrating a divergence that never fires; two
direct merge-target tests added regardless. Wrap test renamed to what
it actually pins (non-wrap-awareness). D-010 docstring softened to
same-process purity, naming the cascade golden as the cross-run layer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge remote-tracking branch 'origin/main' into test-hardening e4b5c2bc9f
style(simulation): gate bounce — clippy identity_op in drainage fixtures 5d1935f8fc 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
style(simulation): gate bounce — cargo fmt in fix-round fixture 493cd716b6 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jpmschweitzer commented 2026-07-25 19:53:36 +02:00
Author
Owner
Copy Link
Copy Source

Review: test-hardening -> main (type: code)

Reviewers: Hoshe (test quality, at full standard for a test-hardening PR) + Tyre (gate-shape architecture). Both REQUEST_CHANGES; all findings addressed in b165c8038.

Converged finding (both reviewers)

Issue Resolution
The drained-body guard's land_districts == 0 misses the == 1 case — a lone island definitionally cannot show two distinct basin directions, so a legitimate single-land-district body would always false-fail Fixed — guard is <= 1 with docs updated and a lone-island vacuous-pass fixture; golden confirmed unaffected (both validation bodies sit at hundreds of land districts)

Hoshe's remaining findings

# Issue Resolution
1 Wrap test named as if wrap support were verified, when it pins the OPPOSITE (non-wrap-aware current behavior) Fixed — renamed thin_by_spacing_is_not_wrap_aware_pins_current_behavior, comment sharpened
2 Oasis scaling is LIVE, not future — GRID_W is already 1024 on main, so ring iterations change 2/4 → 4/8 on every planet generated today, shipped untested and framed as cosmetic Adjudicated + fixed — the change is the N2 nit's own intent (the display inconsistency at 1024 WAS the bug), but it needed honesty and coverage: extracted a pure oasis_ring_iterations() helper pinned by tests at both 512 and 1024, wired into make test-tooling. The hash-stability question was TRACED, not assumed: the determinism guard hashes only elevation; the rings touch only biome — a genuinely different output array. The earlier "cosmetic/no-op" framing is hereby corrected: this is a live behavior change to biome-display ring radii
3 The drainage merge diff read as a production logic change shipped under a "write a comment" item, with no test of the changed branch Clarified byte-precisely + strengthened — zero logic changed vs main (comment-only diff, verified against git show main:). The deeper dig then PROVED the "isolated basin with another basin to escape to" branch is mathematically unreachable for any connected grid (contracting vertex groups of a connected graph cannot disconnect it; verified empirically across every drainage test including real-body heightmaps) — so both the historical bug and the #953 fix described a path that never fires. The comment now states that precisely, and two direct merge-target tests were added regardless (deterministic surviving-id assertion + a bridge fixture proving the None-break path)
4 The bit-for-bit determinism test's docstring overclaims D-010 (same-process double-derive ≠ cross-process determinism) Fixed per lead ruling — docstring softened to same-process purity, explicitly naming tests/golden/cascade_layer1.json (which pins real area_pct values byte-for-byte) as the complementary cross-run layer; the overclaim pattern checked for and absent elsewhere in the PR

Tyre's confirmations on record

No D-245 amendment owed (the D-246 precedent added a criterion the same way — the record pins the mechanism, not a frozen criteria list); the distribution-stat-over-transect choice correctly evidenced (the addendum's own too-localized argument cuts against the transect); the golden regen purely additive; both his original #953 nits resolved as intended.

Verification (fix round)

cargo test --lib atlas:: 755/755; believability_harness determinism + golden match; make test-tooling PASS with the new oasis-pinning and idempotency tests executing. Full gate green on push. No eyeball owed — server/tooling test work, nothing rendered changes.

Verdict: CHANGES REQUESTED -> addressed in b165c8038; merging.

# Review: test-hardening -> main (type: code) Reviewers: Hoshe (test quality, at full standard for a test-hardening PR) + Tyre (gate-shape architecture). Both REQUEST_CHANGES; all findings addressed in `b165c8038`. ### Converged finding (both reviewers) | Issue | Resolution | |-------|------------| | The drained-body guard's `land_districts == 0` misses the `== 1` case — a lone island definitionally cannot show two distinct basin directions, so a legitimate single-land-district body would always false-fail | **Fixed** — guard is `<= 1` with docs updated and a lone-island vacuous-pass fixture; golden confirmed unaffected (both validation bodies sit at hundreds of land districts) | ### Hoshe's remaining findings | # | Issue | Resolution | |---|-------|------------| | 1 | Wrap test named as if wrap support were verified, when it pins the OPPOSITE (non-wrap-aware current behavior) | **Fixed** — renamed `thin_by_spacing_is_not_wrap_aware_pins_current_behavior`, comment sharpened | | 2 | **Oasis scaling is LIVE, not future** — GRID_W is already 1024 on main, so ring iterations change 2/4 → 4/8 on every planet generated today, shipped untested and framed as cosmetic | **Adjudicated + fixed** — the change is the N2 nit's own intent (the display inconsistency at 1024 WAS the bug), but it needed honesty and coverage: extracted a pure `oasis_ring_iterations()` helper pinned by tests at both 512 and 1024, wired into make test-tooling. The hash-stability question was TRACED, not assumed: the determinism guard hashes only `elevation`; the rings touch only `biome` — a genuinely different output array. The earlier "cosmetic/no-op" framing is hereby corrected: this is a live behavior change to biome-display ring radii | | 3 | The drainage merge diff read as a production logic change shipped under a "write a comment" item, with no test of the changed branch | **Clarified byte-precisely + strengthened** — zero logic changed vs main (comment-only diff, verified against `git show main:`). The deeper dig then PROVED the "isolated basin with another basin to escape to" branch is mathematically unreachable for any connected grid (contracting vertex groups of a connected graph cannot disconnect it; verified empirically across every drainage test including real-body heightmaps) — so both the historical bug and the #953 fix described a path that never fires. The comment now states that precisely, and two direct merge-target tests were added regardless (deterministic surviving-id assertion + a bridge fixture proving the None-break path) | | 4 | The bit-for-bit determinism test's docstring overclaims D-010 (same-process double-derive ≠ cross-process determinism) | **Fixed per lead ruling** — docstring softened to same-process purity, explicitly naming `tests/golden/cascade_layer1.json` (which pins real area_pct values byte-for-byte) as the complementary cross-run layer; the overclaim pattern checked for and absent elsewhere in the PR | ### Tyre's confirmations on record No D-245 amendment owed (the D-246 precedent added a criterion the same way — the record pins the mechanism, not a frozen criteria list); the distribution-stat-over-transect choice correctly evidenced (the addendum's own too-localized argument cuts against the transect); the golden regen purely additive; both his original #953 nits resolved as intended. ### Verification (fix round) cargo test --lib atlas:: 755/755; believability_harness determinism + golden match; make test-tooling PASS with the new oasis-pinning and idempotency tests executing. Full gate green on push. No eyeball owed — server/tooling test work, nothing rendered changes. ### Verdict: CHANGES REQUESTED -> addressed in `b165c8038`; merging.
jpmschweitzer closed this pull request 2026-07-25 19:56:12 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#210