First foundation slice of the Atlas-to-tile derivation model (epic T-974, decision D-239) — builds the carrier layer ahead of its T-1027+ consumers. Selected via /whats-next; refined with Si + lead before activation.
T-1026 — Anti-squaring domain warp (D-239 §4)
Stateless pure domain_warp(seed, body_id, pos) -> (f64, f64), ±8m displacement, f64 to the final voxel then as i32 truncation for IEEE-754 cross-target determinism (no platform guards).
New server/src/atlas/domain_warp.rs; SeedDomain::DomainWarp appended (discriminant-pinned). Golden-vector + cross-thread determinism tests.
Position math only — D-010 integer discipline preserved on all downstream gates.
Per-region derived river_threshold replacing the global 200 for tile consumers (drainage itself unchanged).
regions: BTreeMap<RegionPos, RegionProfile> on BodyWorldState, filled by the cascade's new RegionProfile layer (runs when body_params is Some, else falls back to Settlement). BTree ordering for determinism.
Morphology vocabulary reconciliation (12→17 zones) intentionally deferred to T-1027 — uses the existing MorphologyZone.
T-1024 — District climate primitives (D-239 §2)
Nullable temperature_c + moisture on RegionProfile; mean-annual scalar (no clock dependency — the dynamic seasonal branch stays deferred to Q-105).
Hybrid inputs: new bodies.axial_tilt_deg column imported from the planet-gen body-defs (populate_axial_tilt_deg, 2611 bodies); luminosity (spectral class) and orbital distance (Kepler) derived at runtime.
Greenhouse offsets + diurnal-swing amplitudes live in source-canonical server/data/climate_constants.toml (tunable without recompiling).
D-239 record annotated with the implementation note (carrier rung + hybrid-input strategy).
Known limitation (follow-up)
Every production AnalyzeBody enqueue currently passes body_params: None, so the region/climate layer runs in tests but is not yet dispatched in production (the DB-read that builds BodyParams at enqueue time is deferred — see the layer_proxy.rs TODO). This mirrors how T-1022's geometry was dormant before its dispatch landed. Wiring the production data path is a natural next ticket under T-974.
Verification
cargo test: 1498 passed, 0 failed
cargo clippy --all-targets -- -D warnings: clean
cargo fmt --check: clean
make check-systems-db: stamp fresh (axial_tilt_deg present on 2611 bodies)
## Summary
First foundation slice of the **Atlas-to-tile derivation model** (epic **T-974**, decision **D-239**) — builds the carrier layer ahead of its T-1027+ consumers. Selected via `/whats-next`; refined with Si + lead before activation.
### T-1026 — Anti-squaring domain warp (D-239 §4)
- Stateless pure `domain_warp(seed, body_id, pos) -> (f64, f64)`, ±8m displacement, f64 to the final voxel then `as i32` truncation for IEEE-754 cross-target determinism (no platform guards).
- New `server/src/atlas/domain_warp.rs`; `SeedDomain::DomainWarp` appended (discriminant-pinned). Golden-vector + cross-thread determinism tests.
- Position math only — **D-010** integer discipline preserved on all downstream gates.
### T-1023 — RegionProfile carrier (D-239 §1, §10)
- New `RegionProfile` + `TectonicClass` / `GlaciationGrade` / `PrecipitationClass` enums + `BodyParams`.
- Per-region derived `river_threshold` replacing the global `200` for tile consumers (drainage itself unchanged).
- `regions: BTreeMap<RegionPos, RegionProfile>` on `BodyWorldState`, filled by the cascade's new `RegionProfile` layer (runs when `body_params` is `Some`, else falls back to `Settlement`). BTree ordering for determinism.
- Morphology vocabulary reconciliation (12→17 zones) intentionally **deferred to T-1027** — uses the existing `MorphologyZone`.
### T-1024 — District climate primitives (D-239 §2)
- Nullable `temperature_c` + `moisture` on `RegionProfile`; **mean-annual scalar** (no clock dependency — the dynamic seasonal branch stays deferred to Q-105).
- **Hybrid inputs:** new `bodies.axial_tilt_deg` column imported from the planet-gen body-defs (`populate_axial_tilt_deg`, 2611 bodies); luminosity (spectral class) and orbital distance (Kepler) derived at runtime.
- Greenhouse offsets + diurnal-swing amplitudes live in source-canonical `server/data/climate_constants.toml` (tunable without recompiling).
- D-239 record annotated with the implementation note (carrier rung + hybrid-input strategy).
## Known limitation (follow-up)
Every production `AnalyzeBody` enqueue currently passes `body_params: None`, so the region/climate layer runs in tests but is **not yet dispatched in production** (the DB-read that builds `BodyParams` at enqueue time is deferred — see the `layer_proxy.rs` TODO). This mirrors how T-1022's geometry was dormant before its dispatch landed. Wiring the production data path is a natural next ticket under T-974.
## Verification
- `cargo test`: **1498 passed, 0 failed**
- `cargo clippy --all-targets -- -D warnings`: clean
- `cargo fmt --check`: clean
- `make check-systems-db`: stamp fresh (axial_tilt_deg present on 2611 bodies)
Tickets: T-1023, T-1024, T-1026 (epic T-974, Phase 4 / T-750).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
First foundation slice of the Atlas-to-tile derivation model (epic T-974),
building the carrier layer ahead of its T-1027+ consumers.
T-1026 — Anti-squaring domain warp (D-239 §4): stateless pure
fn(seed,body_id,pos)->(f64,f64), ±8m, f64 to the final voxel then as-i32
truncation for IEEE-754 cross-target determinism. New domain_warp.rs,
SeedDomain::DomainWarp; golden-vector + cross-thread tests. Position math
only — D-010 integer discipline preserved downstream. Marked dead_code
until the T-1028 VoxelColumn pipeline consumes it.
T-1023 — RegionProfile carrier (D-239 §1,§10): new RegionProfile +
TectonicClass/GlaciationGrade/PrecipitationClass enums + BodyParams; derived
per-region river_threshold replacing the global 200 for tile consumers.
regions: BTreeMap on BodyWorldState, populated via the cascade's new
RegionProfile layer (runs when body_params is Some, else falls back to
Settlement). D-010 integer discipline, BTree ordering.
T-1024 — District climate primitives (D-239 §2): nullable temperature_c +
moisture on RegionProfile, mean-annual scalar (no clock dep; dynamic branch
deferred to Q-105). Hybrid inputs — new bodies.axial_tilt_deg column imported
from planet-gen body-defs (populate_axial_tilt_deg, 2611 bodies), luminosity
and orbital distance derived at runtime; greenhouse + diurnal-swing tables in
source-canonical climate_constants.toml. D-239 implementation note added.
cargo test: 1498 passed, 0 failed. clippy clean (pre-existing
large_enum_variant only). make check-systems-db: stamp fresh.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review: tile-derivation-carriers → main (type: code)
Reviewers: Hoshe (code quality) + Tyre (architecture). Source read from the branch worktree.
Process: no merge-path surface (pure generation/server code). Verification green and now gated by pre-push (cargo test 1498 pass, clippy -D warnings, fmt, systems.db stamp).
Hoshe (Code Quality): REQUEST_CHANGES
Solid, well-structured foundation — determinism discipline throughout, physically coherent climate math, respectable suite. Three issues before merge.
Tyre (Architecture): REQUEST_CHANGES
Architecturally sound. D-010 integer discipline honored — float correctly confined to domain_warp position math and the physics-only temperature path. Clean carrier/cache design and Option<Box<BodyParams>>. Four issues, two are correctness defects.
Combined findings
#
Severity
File:loc
Issue
Raised by
1
blocker
domain_warp.rs:134-163
golden_vector and pinned_golden_vector are not pinned — let _ = (dx, dy) discards the values; the only assertion is dx != dy. No literal regression anchor, so any change to the hash chain / displacement map / seed structure passes silently. Fill in actual f64 literals (the infra is there).
Hoshe + Tyre
2
blocker
region_profile.rs:595-600, 652-655
Per-region elevation is never applied: derive_temperature_c gets the body-level elevation_km unchanged. derive_all_regions sets region_latitude_deg per region but not elevation_km, so an alpine region and a sea-level region on the same body get identical temperature — defeats the per-district temperature primitive (D-239 §2). The in-code comment describes deriving elevation_km from elev_q but the code doesn't. Fix: build per-region elevation_km = (elev_q/100)*MAX_ELEV_KM, or delete the comment and explicitly defer per-region elevation to ChunkContext.
Tyre
3
blocker
region_profile.rs:589
_region_seed = seed.derive(SeedDomain::DomainWarp, …) reuses the warp domain tag (seed-collision risk with tile warp) and is then discarded unused. Delete the line; add a dedicated SeedDomain::RegionProfile (append-only) only when a consumer needs per-region stochastic derivation.
Hoshe
4
comment
region_profile.rs:466-467
tilt_factor comment is inverted: says 0.0 = no tilt but the formula yields 1.0 at 0° tilt. Physics is correct; comment misleads.
Hoshe
5
doc/feature
region_profile.rs:326, 633
Doc says "use ClimateConstants::load to read the TOML" but there is no load method — every path uses ClimateConstants::default() (embedded values). climate_constants.toml matches the defaults so behavior is correct, but "tunable without recompile" is not actually true. Implement load, or correct the doc to say embedded defaults are authoritative.
Hoshe + Tyre
6
perf (flag)
cascade.rs:225-231
drainage::analyze re-runs to build TerrainAnalysis for the RegionProfile layer (Layer 1 already ran drainage but doesn't store it). Pure/deterministic, but at ~6k regions/body the "cheap" comment understates it. Cache TerrainAnalysis on Layer1Output, or add an explicit budget note and validate against the D-239 §10 ~45ms budget in T-1028/T-1032. Not a hard blocker.
Tyre
Non-blocking notes:populate_axial_tilt_deg silently leaves axial_tilt_deg NULL when absent from frontmatter (Rust defaults to 23.4°) — fine, worth a docstring note. The cascade_golden.rs fixture is correctly unchanged (passes body_params: None).
Verdict: CHANGES REQUESTED
Blockers 1–3 must be fixed (two are correctness; the test gap means the warp has no real regression coverage). 4–5 are quick. 6 is a tracking/validation flag.
## Review: tile-derivation-carriers → main (type: code)
Reviewers: **Hoshe** (code quality) + **Tyre** (architecture). Source read from the branch worktree.
Process: no merge-path surface (pure generation/server code). Verification green and now gated by pre-push (cargo test 1498 pass, clippy `-D warnings`, fmt, systems.db stamp).
### Hoshe (Code Quality): REQUEST_CHANGES
Solid, well-structured foundation — determinism discipline throughout, physically coherent climate math, respectable suite. Three issues before merge.
### Tyre (Architecture): REQUEST_CHANGES
Architecturally sound. D-010 integer discipline honored — float correctly confined to domain_warp position math and the physics-only temperature path. Clean carrier/cache design and `Option<Box<BodyParams>>`. Four issues, two are correctness defects.
### Combined findings
| # | Severity | File:loc | Issue | Raised by |
|---|----------|----------|-------|-----------|
| 1 | **blocker** | `domain_warp.rs:134-163` | `golden_vector` and `pinned_golden_vector` are **not pinned** — `let _ = (dx, dy)` discards the values; the only assertion is `dx != dy`. No literal regression anchor, so any change to the hash chain / displacement map / seed structure passes silently. Fill in actual f64 literals (the infra is there). | Hoshe + Tyre |
| 2 | **blocker** | `region_profile.rs:595-600, 652-655` | Per-region **elevation is never applied**: `derive_temperature_c` gets the body-level `elevation_km` unchanged. `derive_all_regions` sets `region_latitude_deg` per region but not `elevation_km`, so an alpine region and a sea-level region on the same body get identical temperature — defeats the per-district temperature primitive (D-239 §2). The in-code comment describes deriving `elevation_km` from `elev_q` but the code doesn't. Fix: build per-region `elevation_km = (elev_q/100)*MAX_ELEV_KM`, or delete the comment and explicitly defer per-region elevation to ChunkContext. | Tyre |
| 3 | **blocker** | `region_profile.rs:589` | `_region_seed = seed.derive(SeedDomain::DomainWarp, …)` reuses the **warp** domain tag (seed-collision risk with tile warp) and is then discarded unused. Delete the line; add a dedicated `SeedDomain::RegionProfile` (append-only) only when a consumer needs per-region stochastic derivation. | Hoshe |
| 4 | comment | `region_profile.rs:466-467` | `tilt_factor` comment is inverted: says `0.0 = no tilt` but the formula yields `1.0` at 0° tilt. Physics is correct; comment misleads. | Hoshe |
| 5 | doc/feature | `region_profile.rs:326, 633` | Doc says "use `ClimateConstants::load` to read the TOML" but there is **no `load` method** — every path uses `ClimateConstants::default()` (embedded values). `climate_constants.toml` matches the defaults so behavior is correct, but "tunable without recompile" is not actually true. Implement `load`, or correct the doc to say embedded defaults are authoritative. | Hoshe + Tyre |
| 6 | perf (flag) | `cascade.rs:225-231` | `drainage::analyze` re-runs to build `TerrainAnalysis` for the RegionProfile layer (Layer 1 already ran drainage but doesn't store it). Pure/deterministic, but at ~6k regions/body the "cheap" comment understates it. Cache `TerrainAnalysis` on `Layer1Output`, or add an explicit budget note and validate against the D-239 §10 ~45ms budget in T-1028/T-1032. Not a hard blocker. | Tyre |
**Non-blocking notes:** `populate_axial_tilt_deg` silently leaves `axial_tilt_deg` NULL when absent from frontmatter (Rust defaults to 23.4°) — fine, worth a docstring note. The `cascade_golden.rs` fixture is correctly unchanged (passes `body_params: None`).
### Verdict: CHANGES REQUESTED
Blockers 1–3 must be fixed (two are correctness; the test gap means the warp has no real regression coverage). 4–5 are quick. 6 is a tracking/validation flag.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Hoshe + Tyre review (CHANGES REQUESTED):
- domain_warp golden tests were hollow (discarded values, only asserted
dx!=dy). Consolidate into one real golden_vector test with pinned f64
literals as the regression anchor. (Hoshe #1, Tyre #2)
- Per-region temperature used body-level elevation_km for every region, so
alpine and sea-level regions on a body got identical lapse — defeats the
D-239 §2 per-district temperature. Derive elevation_km per region from the
region's own elev_q (× MAX_REGION_ELEVATION_KM). (Tyre #1)
- Region seed was derived under SeedDomain::DomainWarp (collision risk with
the tile warp) and discarded unused. Remove it + the orphaned region_pos_id
and SeedDomain import; keep a reserved _seed param for T-1027/T-1028. (Hoshe #2)
- Fix inverted tilt_factor comment. (Hoshe #3)
- ClimateConstants doc referenced a load() that doesn't exist; correct it —
embedded default() is authoritative today, climate_constants.toml is the
canonical mirror, runtime load lands with T-1032. (Hoshe + Tyre)
- cascade.rs: drainage re-run was mislabeled 'cheap'/'no drainage needed';
document the real cost + PERF/TODO(T-1028/T-1032) against the D-239 §10
budget. (Tyre #3)
cargo test 1497 pass, clippy -D warnings clean, fmt clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All six findings fixed. cargo test 1497 pass, clippy --all-targets -D warnings clean, fmt clean (the new pre-push cargo test gate also ran on push).
#
Finding
Resolution
1
Hollow golden-vector tests (Hoshe + Tyre)
Consolidated the two empty tests into one golden_vector with pinned f64 literals (dx=-1.8472385880244921, dy=5.033644613199796) computed from the implementation — a real regression anchor that fails if the warp output drifts.
2
Per-region elevation never applied → identical lapse (Tyre)
derive_region_profile now builds a region-local BodyParams with elevation_km = (elev_q/100) × MAX_REGION_ELEVATION_KM for the temperature call, so each region's temperature reflects its own elevation. Latitude was already per-region.
3
Region seed under DomainWarp domain + discarded (Hoshe)
Removed the wrong-domain discarded derive, plus the now-orphaned region_pos_id fn and SeedDomain import. Kept a reserved _seed param documented for the T-1027/T-1028 consumer (to be derived under a dedicated SeedDomain::RegionProfile).
ClimateConstants::load doc'd but unimplemented (Hoshe + Tyre)
Corrected the doc: embedded default() is the authoritative runtime source today; climate_constants.toml is the source-canonical mirror; runtime TOML loading lands with the production dispatch wiring (T-1032). No phantom load().
Replaced the misleading comment with an honest cost note + PERF/TODO(T-1028/T-1032): cache TerrainAnalysis on Layer1Output to drop the redundant pass, validate against the D-239 §10 ~45 ms budget in the T-1031 harness. Latent for now (only runs when body_params is Some, deferred to T-1032).
Note on #5/#6: both defer real work to T-1032 (the production-dispatch follow-up) rather than building runtime-load / caching ahead of the consumer — flagging in case you'd prefer either done now.
## Review addressed — `95f26ae3`
All six findings fixed. `cargo test` 1497 pass, `clippy --all-targets -D warnings` clean, `fmt` clean (the new pre-push `cargo test` gate also ran on push).
| # | Finding | Resolution |
|---|---------|------------|
| 1 | Hollow golden-vector tests *(Hoshe + Tyre)* | Consolidated the two empty tests into one `golden_vector` with **pinned f64 literals** (`dx=-1.8472385880244921`, `dy=5.033644613199796`) computed from the implementation — a real regression anchor that fails if the warp output drifts. |
| 2 | Per-region elevation never applied → identical lapse *(Tyre)* | `derive_region_profile` now builds a region-local `BodyParams` with `elevation_km = (elev_q/100) × MAX_REGION_ELEVATION_KM` for the temperature call, so each region's temperature reflects its own elevation. Latitude was already per-region. |
| 3 | Region seed under `DomainWarp` domain + discarded *(Hoshe)* | Removed the wrong-domain discarded derive, plus the now-orphaned `region_pos_id` fn and `SeedDomain` import. Kept a reserved `_seed` param documented for the T-1027/T-1028 consumer (to be derived under a dedicated `SeedDomain::RegionProfile`). |
| 4 | Inverted `tilt_factor` comment *(Hoshe)* | Comment corrected: `1.0 = no tilt (full gradient), 0.5 = 90° tilt (reduced gradient)`. |
| 5 | `ClimateConstants::load` doc'd but unimplemented *(Hoshe + Tyre)* | Corrected the doc: embedded `default()` is the authoritative runtime source today; `climate_constants.toml` is the source-canonical mirror; runtime TOML loading lands with the production dispatch wiring (**T-1032**). No phantom `load()`. |
| 6 | Drainage re-run mislabeled "cheap"/"no drainage needed" *(Tyre)* | Replaced the misleading comment with an honest cost note + `PERF/TODO(T-1028/T-1032)`: cache `TerrainAnalysis` on `Layer1Output` to drop the redundant pass, validate against the D-239 §10 ~45 ms budget in the T-1031 harness. Latent for now (only runs when `body_params` is `Some`, deferred to T-1032). |
Note on #5/#6: both defer real work to **T-1032** (the production-dispatch follow-up) rather than building runtime-load / caching ahead of the consumer — flagging in case you'd prefer either done now.
Ready for re-review.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Re-review (Hoshe + Tyre, both APPROVE) caught one stale comment: after the
per-region elevation fix, derive_region_profile owns elevation_km (from elev_q),
not the caller. Update the comment so maintainers don't read it as caller-supplied.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-review: tile-derivation-carriers → main (type: code) — APPROVED
Both reviewers re-ran against the fix commit 95f26ae3 and confirm all prior findings resolved.
Hoshe (Code Quality): APPROVE
All four prior findings resolved: golden tests now have real pinned f64 literals; the wrong-domain region seed + orphaned region_pos_id/import are gone (reserved _seed param only); tilt_factor comment corrected; per-region elevation now derived from each region's own elev_q (×8 km), ocean/None paths unaffected.
Tyre (Architecture): APPROVE
Per-region elevation fix faithfully restores D-239 §2 per-district temperature variation. Golden anchor sound per the §4 single-platform-pin rationale. Drainage re-run now honestly documented with the §10 budget + T-1028/T-1031/T-1032 deferral chain. ClimateConstants doc no longer claims a phantom load(). T-1032 deferrals (runtime TOML load, TerrainAnalysis caching) are architecturally acceptable since that path only runs when body_params is Some (production-deferred).
Shared minor finding — resolved
Both flagged a stale comment in derive_all_regions (it still described elevation as caller-supplied after the fix moved that into derive_region_profile). Fixed in 7084a4101.
Verdict: APPROVED
cargo test 1497 pass, clippy -D warnings clean, fmt clean (pre-push gate, incl. the new cargo test step). Merging.
## Re-review: tile-derivation-carriers → main (type: code) — APPROVED
Both reviewers re-ran against the fix commit `95f26ae3` and confirm all prior findings resolved.
### Hoshe (Code Quality): APPROVE
All four prior findings resolved: golden tests now have real pinned f64 literals; the wrong-domain region seed + orphaned `region_pos_id`/import are gone (reserved `_seed` param only); `tilt_factor` comment corrected; per-region elevation now derived from each region's own `elev_q` (×8 km), ocean/None paths unaffected.
### Tyre (Architecture): APPROVE
Per-region elevation fix faithfully restores D-239 §2 per-district temperature variation. Golden anchor sound per the §4 single-platform-pin rationale. Drainage re-run now honestly documented with the §10 budget + T-1028/T-1031/T-1032 deferral chain. `ClimateConstants` doc no longer claims a phantom `load()`. T-1032 deferrals (runtime TOML load, TerrainAnalysis caching) are architecturally acceptable since that path only runs when `body_params` is `Some` (production-deferred).
### Shared minor finding — resolved
Both flagged a stale comment in `derive_all_regions` (it still described elevation as caller-supplied after the fix moved that into `derive_region_profile`). Fixed in `7084a4101`.
### Verdict: APPROVED
`cargo test` 1497 pass, clippy `-D warnings` clean, fmt clean (pre-push gate, incl. the new `cargo test` step). Merging.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
First foundation slice of the Atlas-to-tile derivation model (epic T-974, decision D-239) — builds the carrier layer ahead of its T-1027+ consumers. Selected via
/whats-next; refined with Si + lead before activation.T-1026 — Anti-squaring domain warp (D-239 §4)
domain_warp(seed, body_id, pos) -> (f64, f64), ±8m displacement, f64 to the final voxel thenas i32truncation for IEEE-754 cross-target determinism (no platform guards).server/src/atlas/domain_warp.rs;SeedDomain::DomainWarpappended (discriminant-pinned). Golden-vector + cross-thread determinism tests.T-1023 — RegionProfile carrier (D-239 §1, §10)
RegionProfile+TectonicClass/GlaciationGrade/PrecipitationClassenums +BodyParams.river_thresholdreplacing the global200for tile consumers (drainage itself unchanged).regions: BTreeMap<RegionPos, RegionProfile>onBodyWorldState, filled by the cascade's newRegionProfilelayer (runs whenbody_paramsisSome, else falls back toSettlement). BTree ordering for determinism.MorphologyZone.T-1024 — District climate primitives (D-239 §2)
temperature_c+moistureonRegionProfile; mean-annual scalar (no clock dependency — the dynamic seasonal branch stays deferred to Q-105).bodies.axial_tilt_degcolumn imported from the planet-gen body-defs (populate_axial_tilt_deg, 2611 bodies); luminosity (spectral class) and orbital distance (Kepler) derived at runtime.server/data/climate_constants.toml(tunable without recompiling).Known limitation (follow-up)
Every production
AnalyzeBodyenqueue currently passesbody_params: None, so the region/climate layer runs in tests but is not yet dispatched in production (the DB-read that buildsBodyParamsat enqueue time is deferred — see thelayer_proxy.rsTODO). This mirrors how T-1022's geometry was dormant before its dispatch landed. Wiring the production data path is a natural next ticket under T-974.Verification
cargo test: 1498 passed, 0 failedcargo clippy --all-targets -- -D warnings: cleancargo fmt --check: cleanmake check-systems-db: stamp fresh (axial_tilt_deg present on 2611 bodies)Tickets: T-1023, T-1024, T-1026 (epic T-974, Phase 4 / T-750).
🤖 Generated with Claude Code
Review: tile-derivation-carriers → main (type: code)
Reviewers: Hoshe (code quality) + Tyre (architecture). Source read from the branch worktree.
Process: no merge-path surface (pure generation/server code). Verification green and now gated by pre-push (cargo test 1498 pass, clippy
-D warnings, fmt, systems.db stamp).Hoshe (Code Quality): REQUEST_CHANGES
Solid, well-structured foundation — determinism discipline throughout, physically coherent climate math, respectable suite. Three issues before merge.
Tyre (Architecture): REQUEST_CHANGES
Architecturally sound. D-010 integer discipline honored — float correctly confined to domain_warp position math and the physics-only temperature path. Clean carrier/cache design and
Option<Box<BodyParams>>. Four issues, two are correctness defects.Combined findings
domain_warp.rs:134-163golden_vectorandpinned_golden_vectorare not pinned —let _ = (dx, dy)discards the values; the only assertion isdx != dy. No literal regression anchor, so any change to the hash chain / displacement map / seed structure passes silently. Fill in actual f64 literals (the infra is there).region_profile.rs:595-600, 652-655derive_temperature_cgets the body-levelelevation_kmunchanged.derive_all_regionssetsregion_latitude_degper region but notelevation_km, so an alpine region and a sea-level region on the same body get identical temperature — defeats the per-district temperature primitive (D-239 §2). The in-code comment describes derivingelevation_kmfromelev_qbut the code doesn't. Fix: build per-regionelevation_km = (elev_q/100)*MAX_ELEV_KM, or delete the comment and explicitly defer per-region elevation to ChunkContext.region_profile.rs:589_region_seed = seed.derive(SeedDomain::DomainWarp, …)reuses the warp domain tag (seed-collision risk with tile warp) and is then discarded unused. Delete the line; add a dedicatedSeedDomain::RegionProfile(append-only) only when a consumer needs per-region stochastic derivation.region_profile.rs:466-467tilt_factorcomment is inverted: says0.0 = no tiltbut the formula yields1.0at 0° tilt. Physics is correct; comment misleads.region_profile.rs:326, 633ClimateConstants::loadto read the TOML" but there is noloadmethod — every path usesClimateConstants::default()(embedded values).climate_constants.tomlmatches the defaults so behavior is correct, but "tunable without recompile" is not actually true. Implementload, or correct the doc to say embedded defaults are authoritative.cascade.rs:225-231drainage::analyzere-runs to buildTerrainAnalysisfor the RegionProfile layer (Layer 1 already ran drainage but doesn't store it). Pure/deterministic, but at ~6k regions/body the "cheap" comment understates it. CacheTerrainAnalysisonLayer1Output, or add an explicit budget note and validate against the D-239 §10 ~45ms budget in T-1028/T-1032. Not a hard blocker.Non-blocking notes:
populate_axial_tilt_degsilently leavesaxial_tilt_degNULL when absent from frontmatter (Rust defaults to 23.4°) — fine, worth a docstring note. Thecascade_golden.rsfixture is correctly unchanged (passesbody_params: None).Verdict: CHANGES REQUESTED
Blockers 1–3 must be fixed (two are correctness; the test gap means the warp has no real regression coverage). 4–5 are quick. 6 is a tracking/validation flag.
🤖 Generated with Claude Code
Review addressed —
95f26ae3All six findings fixed.
cargo test1497 pass,clippy --all-targets -D warningsclean,fmtclean (the new pre-pushcargo testgate also ran on push).golden_vectorwith pinned f64 literals (dx=-1.8472385880244921,dy=5.033644613199796) computed from the implementation — a real regression anchor that fails if the warp output drifts.derive_region_profilenow builds a region-localBodyParamswithelevation_km = (elev_q/100) × MAX_REGION_ELEVATION_KMfor the temperature call, so each region's temperature reflects its own elevation. Latitude was already per-region.DomainWarpdomain + discarded (Hoshe)region_pos_idfn andSeedDomainimport. Kept a reserved_seedparam documented for the T-1027/T-1028 consumer (to be derived under a dedicatedSeedDomain::RegionProfile).tilt_factorcomment (Hoshe)1.0 = no tilt (full gradient), 0.5 = 90° tilt (reduced gradient).ClimateConstants::loaddoc'd but unimplemented (Hoshe + Tyre)default()is the authoritative runtime source today;climate_constants.tomlis the source-canonical mirror; runtime TOML loading lands with the production dispatch wiring (T-1032). No phantomload().PERF/TODO(T-1028/T-1032): cacheTerrainAnalysisonLayer1Outputto drop the redundant pass, validate against the D-239 §10 ~45 ms budget in the T-1031 harness. Latent for now (only runs whenbody_paramsisSome, deferred to T-1032).Note on #5/#6: both defer real work to T-1032 (the production-dispatch follow-up) rather than building runtime-load / caching ahead of the consumer — flagging in case you'd prefer either done now.
Ready for re-review.
🤖 Generated with Claude Code
Re-review: tile-derivation-carriers → main (type: code) — APPROVED
Both reviewers re-ran against the fix commit
95f26ae3and confirm all prior findings resolved.Hoshe (Code Quality): APPROVE
All four prior findings resolved: golden tests now have real pinned f64 literals; the wrong-domain region seed + orphaned
region_pos_id/import are gone (reserved_seedparam only);tilt_factorcomment corrected; per-region elevation now derived from each region's ownelev_q(×8 km), ocean/None paths unaffected.Tyre (Architecture): APPROVE
Per-region elevation fix faithfully restores D-239 §2 per-district temperature variation. Golden anchor sound per the §4 single-platform-pin rationale. Drainage re-run now honestly documented with the §10 budget + T-1028/T-1031/T-1032 deferral chain.
ClimateConstantsdoc no longer claims a phantomload(). T-1032 deferrals (runtime TOML load, TerrainAnalysis caching) are architecturally acceptable since that path only runs whenbody_paramsisSome(production-deferred).Shared minor finding — resolved
Both flagged a stale comment in
derive_all_regions(it still described elevation as caller-supplied after the fix moved that intoderive_region_profile). Fixed in7084a4101.Verdict: APPROVED
cargo test1497 pass, clippy-D warningsclean, fmt clean (pre-push gate, incl. the newcargo teststep). Merging.🤖 Generated with Claude Code
Pull request closed