fix(simulation): one absolute-metre derive core (D-256, T-1174) #199

2026-07-25T01:18:21+02:00

jpmschweitzer commented

2026-07-25 01:18:21 +02:00

Summary

Implements T-1174 per D-256 (record ships in this PR): the batch and window derive paths sampled different world positions for the same DistrictPos — two coordinate systems sharing one bare (i32,i32) tuple type. derive_district_profile is now a thin wrapper over the shared derive_at_metres_with_riparian core at survey-cell-centre world metres, and the batch pseudo-grid is re-keyed by a real SurveyCellPos newtype so the compiler rejects cross-namespace passing.

Fixes surfaced along the way

Three mutually inconsistent latitude conventions in the batch path collapse into the core single inverse mapping.
Region-climate baseline no longer collapses the whole body onto region (0,0) — batch climate is now latitude/region graded (D-245 direction: every changed believability metric increased).
Basin map re-keyed as the survey-cell aggregate it is; a silent all-North lookup regression was caught in lead review and fixed, with the seam test restored to production-shaped keys.
Quarter-skeleton morphology now resolves at the settlement exact world position (work-item execution, where the terrain cache lives) instead of a ~625 km survey-cell centre.
Second pixel-vs-metre conflation fixed in the aliveness_probe anchor-walk math.

Verification

window_derivation_golden byte-unchanged (the window path must not move); derivation_harness golden untouched; believability golden regenerated.
Full gate green: fmt, clippy -D warnings, 1896 lib tests + every integration binary, cargo deny.
Paired-set screenshot eyeball (cold release server, main vs branch, GJ380c): all 9 pairs 0-diff including cold-start staged frames; the data change is proven at the wire level (elev_q array hash differs) — no player-reachable overlay renders the changed fields today, so no visual change is expected in the current Atlas.

Deferred per D-256(f)

LayerRegionOutput/region_grid rebuild rides T-1181 (rung-0); voxel carrier rides Phase 5. The believability-harness basin blindness found during review is recorded on T-964.

Unblocks T-1181 (step-canvas envelope — cell-for-cell derive agreement).

🤖 Generated with Claude Code

## Summary Implements T-1174 per D-256 (record ships in this PR): the batch and window derive paths sampled different world positions for the same DistrictPos — two coordinate systems sharing one bare (i32,i32) tuple type. derive_district_profile is now a thin wrapper over the shared derive_at_metres_with_riparian core at survey-cell-centre world metres, and the batch pseudo-grid is re-keyed by a real SurveyCellPos newtype so the compiler rejects cross-namespace passing. ## Fixes surfaced along the way - Three mutually inconsistent latitude conventions in the batch path collapse into the core single inverse mapping. - Region-climate baseline no longer collapses the whole body onto region (0,0) — batch climate is now latitude/region graded (D-245 direction: every changed believability metric increased). - Basin map re-keyed as the survey-cell aggregate it is; a silent all-North lookup regression was caught in lead review and fixed, with the seam test restored to production-shaped keys. - Quarter-skeleton morphology now resolves at the settlement exact world position (work-item execution, where the terrain cache lives) instead of a ~625 km survey-cell centre. - Second pixel-vs-metre conflation fixed in the aliveness_probe anchor-walk math. ## Verification - window_derivation_golden byte-unchanged (the window path must not move); derivation_harness golden untouched; believability golden regenerated. - Full gate green: fmt, clippy -D warnings, 1896 lib tests + every integration binary, cargo deny. - Paired-set screenshot eyeball (cold release server, main vs branch, GJ380c): all 9 pairs 0-diff including cold-start staged frames; the data change is proven at the wire level (elev_q array hash differs) — no player-reachable overlay renders the changed fields today, so no visual change is expected in the current Atlas. ## Deferred per D-256(f) LayerRegionOutput/region_grid rebuild rides T-1181 (rung-0); voxel carrier rides Phase 5. The believability-harness basin blindness found during review is recorded on T-964. Unblocks T-1181 (step-canvas envelope — cell-for-cell derive agreement). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jpmschweitzer added 4 commits 2026-07-25 01:18:21 +02:00

docs(meta): D-256 — canonical sampling convention ruling 35a4137304

Records the T-1174 ruling: derive(seed, absolute world metres) via the
derive_at_metres family is the only derive core; the batch 64x32
pseudo-grid is re-scoped as a survey raster with a real SurveyCellPos
newtype (role name, not a D-243 rung); derive_district_profile becomes
a thin wrapper at survey-cell-centre positions with binding basin and
riparian preservation; point-feature terrain judgments move to exact
world positions; D-255 step canvases are fenced to the window family.
LayerRegionOutput rebuild deferred to T-1181 (tripwire verified: sole
reader is the region_grid overlay); voxel carrier deferred to Phase 5.

Consults: Tyre APPROVE-WITH-CHANGES, Dudley FEASIBLE-WITH-CHANGES —
both incorporated. Discharges the T-1174 blocker on D-255's T-1181
step-canvas envelope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(simulation): one absolute-metre derive core (D-256, T-1174) 933e1f4ee4

derive_district_profile is now a thin wrapper over the shared
derive_at_metres_with_riparian core at survey-cell-centre world metres
— one derive core, two position sets. The batch pseudo-grid and the
true D-243 district grid were two coordinate systems sharing one bare
(i32,i32) type; the new SurveyCellPos newtype re-keys every batch
product (BodyWorldState.districts, Layer1Output.survey_basin_dirs) so
the compiler rejects cross-namespace passing.

Fixes two latent same-position divergences the T-1174 investigation
surfaced: three inconsistent latitude conventions collapse into the
core's single inverse mapping, and the region-climate baseline now
floor-divides true world metres instead of collapsing the whole body
onto region (0,0)'s baseline — batch climate becomes latitude/region
graded (D-245 direction: every changed believability metric increased).

Binding preservations per D-256(c): basin_direction rides a post-call
override with the true L1 D8 survey-cell aggregate (layer1's map
re-keyed to SurveyCellPos, identity lookup — a floor-divide lookup
against the pseudo-keyed map would have silently defaulted every cell
North); the riparian verdict comes from near_perennial_water_at, never
the empty-slice default (which would have flipped riverside
vegetation_class).

Quarter-skeleton morphology_zone now resolves at the settlement's
exact world position via derive_at_metres at work-item execution
(where TerrainAnalysisCache lives), replacing the survey-cell-centre
map lookup (D-256(d)); settlement_district_pos fixed to true-district
floor-division in passing (same doc/impl mismatch class). Second
pixel-vs-metre conflation fixed in aliveness_probe's anchor-walk math.

Window path byte-unchanged (window_derivation_golden 6/6 byte-
identical); derivation_harness golden untouched; believability golden
regenerated. Full lib + integration suites green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

chore(meta): update changelog 630d735d13

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

style(simulation): cargo fmt on the T-1174 batch (gate bounce) 9068fcbc0b

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

jpmschweitzer commented

2026-07-25 01:25:36 +02:00

Review: t1174-derive-sampling -> main (type: code)

Tyre (Architecture): APPROVE

The implementation faithfully honors every binding clause of D-256: SurveyCellPos is a real newtype with exactly one sanctioned bridge function, derive_district_profile is a genuine thin wrapper over the shared derive_at_metres_with_riparian core, both binding preservation requirements (inert basin_direction override, extracted riparian core) are implemented as specified, the D-256(d) exact-position skeleton judgment resolves server-side at execution time, and the D-256(f) deferral is honestly fenced. The two collateral fixes (believability chunk-sampling math, settlement_district_pos) are root-cause cleanup, not scope creep. Determinism discipline intact (BTreeMap throughout, integer-gated classification); wrapper≡core bit-identity tests directly protect the D-255(f) cache-hit==cache-miss property. Independently re-ran the atlas lib suite (682 passed) + believability/derivation/gen_fixtures harnesses — window goldens byte-unchanged, only the batch believability golden regenerated (the ruling's accepted cost).

No comments.

Hoshe (Code Quality): REQUEST_CHANGES

Well-designed, thoroughly-documented, well-tested unification — the newtype closes the namespace-collision root cause and the extraction preserves derive_at_metres byte behavior (confirmed by the two new identity/agreement tests). Two issues:

#	File	Issue
1	server/src/atlas/district_profile.rs (~2078-2093, doc ~380-401, wrapper doc ~1343)	Performance regression, unacknowledged: `derive_all_districts` now passes a permanently-empty `region_cache`, so all ~2048 survey cells miss and pay 4x `derive_region_baseline_c` each (~8192 baseline computations/body) instead of reusing a per-body pre-built cache. The old cache was keyed wrong (survey pseudo-coords), but the fix discards caching instead of pre-building a correctly-keyed cache via `survey_cell_centre_world_m` -> true region + ±1 ring. `derive_district_profile`'s docstring still instructs "Build with `derive_regions_for_body` before calling this in a loop" — contradicted by its own sole production caller.
2	server/src/atlas/cascade.rs (~305-310)	Stale comment: "derive_all_districts builds the region cache internally" — no longer true post-diff; the one miss in an otherwise careful comment sweep, and it misleads at exactly the call site that needs to know.

Verdict: CHANGES REQUESTED

Author to address both findings (single focused fix: pre-build the true-keyed covering region cache inside derive_all_districts, restoring both the docstring's and the cascade comment's truth). Region baselines are pure functions of (seed, region), so cache-hit vs on-the-fly is byte-identical — the existing full-profile agreement test guards value drift; the believability golden must not move.

Process context: full pre-push gate green (fmt/clippy -D warnings/1896 lib + all integration tests/deny); cold-launch paired-set screenshot check main-vs-branch all 0-diff (change is wire-level-proven; no player-reachable overlay renders the changed fields today).

## Review: t1174-derive-sampling -> main (type: code) ### Tyre (Architecture): APPROVE The implementation faithfully honors every binding clause of D-256: `SurveyCellPos` is a real newtype with exactly one sanctioned bridge function, `derive_district_profile` is a genuine thin wrapper over the shared `derive_at_metres_with_riparian` core, both binding preservation requirements (inert `basin_direction` override, extracted riparian core) are implemented as specified, the D-256(d) exact-position skeleton judgment resolves server-side at execution time, and the D-256(f) deferral is honestly fenced. The two collateral fixes (believability chunk-sampling math, `settlement_district_pos`) are root-cause cleanup, not scope creep. Determinism discipline intact (BTreeMap throughout, integer-gated classification); wrapper≡core bit-identity tests directly protect the D-255(f) cache-hit==cache-miss property. Independently re-ran the atlas lib suite (682 passed) + believability/derivation/gen_fixtures harnesses — window goldens byte-unchanged, only the batch believability golden regenerated (the ruling's accepted cost). No comments. ### Hoshe (Code Quality): REQUEST_CHANGES Well-designed, thoroughly-documented, well-tested unification — the newtype closes the namespace-collision root cause and the extraction preserves `derive_at_metres` byte behavior (confirmed by the two new identity/agreement tests). Two issues: | # | File | Issue | |---|------|-------| | 1 | server/src/atlas/district_profile.rs (~2078-2093, doc ~380-401, wrapper doc ~1343) | Performance regression, unacknowledged: `derive_all_districts` now passes a permanently-empty `region_cache`, so all ~2048 survey cells miss and pay 4x `derive_region_baseline_c` each (~8192 baseline computations/body) instead of reusing a per-body pre-built cache. The old cache was keyed wrong (survey pseudo-coords), but the fix discards caching instead of pre-building a correctly-keyed cache via `survey_cell_centre_world_m` -> true region + ±1 ring. `derive_district_profile`'s docstring still instructs "Build with `derive_regions_for_body` before calling this in a loop" — contradicted by its own sole production caller. | | 2 | server/src/atlas/cascade.rs (~305-310) | Stale comment: "derive_all_districts builds the region cache internally" — no longer true post-diff; the one miss in an otherwise careful comment sweep, and it misleads at exactly the call site that needs to know. | ### Verdict: CHANGES REQUESTED Author to address both findings (single focused fix: pre-build the true-keyed covering region cache inside `derive_all_districts`, restoring both the docstring's and the cascade comment's truth). Region baselines are pure functions of (seed, region), so cache-hit vs on-the-fly is byte-identical — the existing full-profile agreement test guards value drift; the believability golden must not move. *Process context: full pre-push gate green (fmt/clippy -D warnings/1896 lib + all integration tests/deny); cold-launch paired-set screenshot check main-vs-branch all 0-diff (change is wire-level-proven; no player-reachable overlay renders the changed fields today).*

jpmschweitzer referenced this issue from a commit

2026-07-25 01:31:54 +02:00

fix(simulation): pre-build the true-keyed region cache (PR #199 review)

jpmschweitzer added 1 commit 2026-07-25 01:31:54 +02:00

fix(simulation): pre-build the true-keyed region cache (PR #199 review) 236725eb3d

Hoshe finding 1: derive_all_districts passed a permanently-empty
region_cache, so every survey cell paid four on-the-fly baseline
derivations — and derive_district_profile's docstring (build the cache
before calling in a loop) was contradicted by its sole production
caller. The cache is now pre-built on the TRUE region keys the shared
core looks up (survey-cell centre metres -> containing district ->
region ±1 ring, BTreeSet-deduped), restoring the per-body build-once
cost model. Cache-hit == cache-miss byte-identity holds by D-227 purity
(build_region_profile and the miss branch share the same expressions) —
verified by the believability harness passing against the UNCHANGED
golden. Finding 2: the cascade.rs call-site comment and the stale
module header now state the pre-build truthfully.

Found in passing, filed as T-1186 (pre-existing, out of scope here):
region_centre_latitude_deg is pole-anchored while the derive core keys
equator-anchored signed regions — northern-hemisphere baselines clamp
to +90.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

jpmschweitzer commented

2026-07-25 01:32:33 +02:00

Author response — review round 1

Both findings fixed in 236725eb3 (fix(simulation): pre-build the true-keyed region cache (PR #199 review)):

Finding 1 (region-cache regression) — FIXED. derive_all_districts now pre-builds the region-baseline cache on the TRUE region keys the shared core looks up: each survey cell's centre world metres → containing district (the same floor-divide derive_at_metres_with_riparian applies) → region ±1 neighbour ring (covers every key the edge-fuzz blend can read), BTreeSet-deduped, derived once per body via derive_regions_for_body. derive_district_profile's "build the cache before calling in a loop" docstring is truthful again. Byte-identity of cache-hit vs cache-miss verified the strong way: the believability harness passes against the unchanged golden (believability.json not modified by the fix commit).

Finding 2 (stale cascade.rs comment) — FIXED. The call-site comment now states the true-key pre-build explicitly; the stale pre-D-243 module header in district_profile.rs ("~1 km tier, 80×75") was the same class of leftover and got the same sweep.

Found in passing, filed as T-1186 (pre-existing, deliberately out of scope): region_centre_latitude_deg is pole-anchored (region row 0 = +90°, negatives clamp) while the derive core keys regions by equator-anchored signed rows — northern-hemisphere region baselines clamp to +90° on the window path today, and have since the region climate stack landed (predates this PR). Fix needs a one-convention ruling at the region tier (likely with the T-1181 rung-0 rebuild) plus golden regen, so it is ticketed rather than smuggled into this diff.

Gate re-ran green on the fix round (fmt/clippy/1896 lib + all integration/deny). Requesting re-review or merge.

## Author response — review round 1 Both findings fixed in `236725eb3` (`fix(simulation): pre-build the true-keyed region cache (PR #199 review)`): **Finding 1 (region-cache regression) — FIXED.** `derive_all_districts` now pre-builds the region-baseline cache on the TRUE region keys the shared core looks up: each survey cell's centre world metres → containing district (the same floor-divide `derive_at_metres_with_riparian` applies) → region ±1 neighbour ring (covers every key the edge-fuzz blend can read), `BTreeSet`-deduped, derived once per body via `derive_regions_for_body`. `derive_district_profile`'s "build the cache before calling in a loop" docstring is truthful again. Byte-identity of cache-hit vs cache-miss verified the strong way: the believability harness passes against the **unchanged** golden (`believability.json` not modified by the fix commit). **Finding 2 (stale cascade.rs comment) — FIXED.** The call-site comment now states the true-key pre-build explicitly; the stale pre-D-243 module header in `district_profile.rs` ("~1 km tier, 80×75") was the same class of leftover and got the same sweep. **Found in passing, filed as T-1186 (pre-existing, deliberately out of scope):** `region_centre_latitude_deg` is pole-anchored (region row 0 = +90°, negatives clamp) while the derive core keys regions by equator-anchored signed rows — northern-hemisphere region baselines clamp to +90° on the window path today, and have since the region climate stack landed (predates this PR). Fix needs a one-convention ruling at the region tier (likely with the T-1181 rung-0 rebuild) plus golden regen, so it is ticketed rather than smuggled into this diff. Gate re-ran green on the fix round (fmt/clippy/1896 lib + all integration/deny). Requesting re-review or merge.

jpmschweitzer closed this pull request

2026-07-25 01:35:06 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#199