From ff5e7ee9440a66e22b94126aba3ac0de6b3fa82f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 15 Jun 2026 17:30:46 +0200 Subject: [PATCH] fix(simulation): VoxelCache precondition doc + sec_phase decontamination + cache blend test (T-1042) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR #167 review (Hoshe H1/H2/H3, Tyre T2). H1 — the VoxelCache 'same (seed, body_id, tile_pos) -> same column' invariant is no longer unconditional after the T-1042 blend (the key omits blend_weight/ secondary). Replaced with an explicit precondition on the struct + get_or_derive: the caller must pass the ChunkContext of the chunk that contains the tile. H2/T2 — sec_phase claimed to be 'position-independent' but mixed in primary_meander_phase/2 (the primary's per-position seed), making the secondary phase proxy asymmetric and primary-contaminated. Dropped that term; sec_phase is now the secondary's own morphology scalars (slope_q + moisture_q), with a comment honestly naming it a bounded cosmetic approximation and where to thread the real secondary seed if needed. H3 — added cache_boundary_tile_blend_is_stable: derives a boundary ChunkContext (blend_weight=128, secondary=Some), calls get_or_derive twice, asserts a cache hit + identical blended elevation + that the Cow::Owned blend path actually ran. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/src/atlas/chunk_context.rs | 25 +++--- server/src/atlas/voxel.rs | 128 +++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 10 deletions(-) diff --git a/server/src/atlas/chunk_context.rs b/server/src/atlas/chunk_context.rs index 7ca0a0907..0262d358e 100644 --- a/server/src/atlas/chunk_context.rs +++ b/server/src/atlas/chunk_context.rs @@ -292,15 +292,22 @@ pub fn derive_chunk_context( let sec_wavelength = derive_meander_wavelength(sec); let sec_channel_width = derive_channel_width(sec); let sec_phase = { - // Secondary district's meander phase uses a secondary seed so it - // differs from the primary (different district-scale chunk id). - // We approximate: use the secondary's wavelength-derived phase - // as an integer representation of its channel geometry. For a - // well-defined phase, we re-derive from the secondary's slope and - // moisture as a deterministic position-independent proxy. - // Integer arithmetic (D-010). - (sec.slope_q.wrapping_add(sec.moisture_q) as u8) - .wrapping_add(primary_meander_phase / 2) + // The secondary district's true meander phase would require its + // district-scale seed, which in turn requires knowing the adjacent + // district's chunk coordinates — information not available at this + // call site. We use a bounded structural approximation: a hash of + // the secondary's morphology scalars (slope_q, moisture_q), which + // are the same inputs that drive `derive_meander_wavelength` and + // therefore capture the secondary district's channel character. + // + // This approximation is intentionally asymmetric in an acknowledged + // way: the blended phase is a cosmetic continuity aid at the 2 km + // seam (meander phase), not a structural gate decision. The seam + // is already invisible at the elevation level from elev_q blending; + // the phase contribution is second-order. If the real secondary seed + // is ever threaded through here, replace this with the proper + // district-scale derivation. Integer arithmetic (D-010). + sec.slope_q.wrapping_add(sec.moisture_q) as u8 }; let w = weight as i32; let w_sec = 255 - w; diff --git a/server/src/atlas/voxel.rs b/server/src/atlas/voxel.rs index 1f1e7f801..a60bed0d9 100644 --- a/server/src/atlas/voxel.rs +++ b/server/src/atlas/voxel.rs @@ -1837,9 +1837,31 @@ struct CacheEntry { /// /// - At most `capacity` entries are stored at any time. /// - Entries are evicted in LRU order (lowest `access_gen`). -/// - Same `(seed, body_id, tile_pos)` → same column, always. /// - Nothing is persisted to disk; eviction forces re-derivation. /// +/// ## Caller precondition — chunk context must cover the tile (T-1042) +/// +/// After T-1042, `derive_voxel_column` reads `chunk.blend_weight` and +/// `chunk.secondary` to apply cross-district terrain blending. The cache +/// key is `(seed, body_id_hash, post-warp voxel position)` — it does NOT +/// capture the blend parameters. The cache invariant therefore holds only +/// under the following precondition: +/// +/// **The `chunk` argument passed to `get_or_derive` must be the +/// `ChunkContext` derived for the chunk that actually contains the tile.** +/// Concretely: `chunk = derive_chunk_context(seed, body_id, district, +/// tile_pos.div_euclid(CHUNK_M), secondary)`. A boundary tile derived with +/// a blended context (blend_weight < 255) and later re-requested with a +/// pure-primary context (blend_weight = 255) would produce a different +/// column — the cache would return the first (blended) value, silently +/// giving the wrong result. The caller must ensure the same context +/// construction is used on every call for the same tile. +/// +/// **In practice this is satisfied by construction**: callers derive one +/// `ChunkContext` per chunk and call `get_or_derive` only for tiles within +/// that chunk. The precondition is made explicit here so Phase-5 render-loop +/// callers do not inadvertently mix contexts across calls. +/// /// ## D-010 compliance /// /// Uses `BTreeMap` (ordered) for the cache store. Iteration order is @@ -1871,6 +1893,15 @@ impl VoxelCache { /// /// The body-id hash is computed internally via the canonical /// [`crate::seed::fnv1a_64`] — callers pass the raw `body_id`. + /// + /// ## Precondition — context must cover the tile (T-1042) + /// + /// `chunk` must be the `ChunkContext` derived for the chunk that + /// contains `(tile_x, tile_y)`. The cache key does not capture + /// `chunk.blend_weight` or `chunk.secondary`; if the same tile is + /// requested with a different context (e.g. once blended, once + /// unblended) the cache will return the first-derived column for both + /// calls. See the struct-level doc for the full constraint. pub fn get_or_derive( &mut self, world_seed: u64, @@ -2263,6 +2294,101 @@ mod tests { ); } + #[test] + fn cache_boundary_tile_blend_is_stable() { + // T-1042 / H3: VoxelCache.get_or_derive must return identical blended + // output on two successive calls for the same boundary tile. This test + // exercises the `Cow::Owned` path (blend_weight=128, secondary=Some(...)) + // through the cache and guards against future cache-key drift where a + // second call with a different context returns a stale unblended entry. + // + // Precondition (VoxelCache struct doc, T-1042): both calls use the SAME + // ChunkContext — the one derived for the chunk containing the tile. This + // is the correct call pattern; the test intentionally validates it. + use crate::atlas::chunk_context::district_boundary_blend_weight; + + let district_primary = DistrictProfile { + morphology_zone: MorphologyZone::AlluvialPlain, + tectonic_class: crate::atlas::district_profile::TectonicClass::Stable, + glaciation_grade: crate::atlas::district_profile::GlaciationGrade::None, + precipitation_class: crate::atlas::district_profile::PrecipitationClass::Temperate, + slope_q: 5, + elev_q: 20, // low elevation + ocean_fraction_q: 0, // no channel — simpler tile layout for elevation check + river_threshold: 200, + temperature_c: Some(18.0), + moisture_q: 55, + vegetation_class: VegetationClass::Forest, + }; + let district_secondary = DistrictProfile { + morphology_zone: MorphologyZone::AlluvialPlain, + tectonic_class: crate::atlas::district_profile::TectonicClass::Stable, + glaciation_grade: crate::atlas::district_profile::GlaciationGrade::None, + precipitation_class: crate::atlas::district_profile::PrecipitationClass::Temperate, + slope_q: 5, + elev_q: 70, // high elevation — makes the blend measurable + ocean_fraction_q: 0, + river_threshold: 200, + temperature_c: Some(18.0), + moisture_q: 55, + vegetation_class: VegetationClass::Forest, + }; + + let (seed, body_id) = (42u64, "blend_cache_test"); + // Chunk (31, 0): the last chunk in district (0, 0) — at the district boundary. + let boundary_chunk_pos = (31i32, 0i32); + let (near, blend_w) = district_boundary_blend_weight(boundary_chunk_pos); + assert!(near, "chunk (31, 0) must be detected as a boundary chunk"); + assert_eq!(blend_w, 128); + + let chunk = derive_chunk_context( + seed, + body_id, + &district_primary, + boundary_chunk_pos, + Some((&district_secondary, blend_w)), + ); + + // A tile inside the boundary chunk — world position = chunk_x * 64 + offset. + let tile_x = boundary_chunk_pos.0 * 64 + 16; + let tile_y = boundary_chunk_pos.1 * 64 + 16; + + let mut cache = VoxelCache::new(64); + + // First call: cache miss → derives the blended column and inserts it. + let col1 = cache.get_or_derive(seed, body_id, &district_primary, &chunk, tile_x, tile_y); + assert_eq!(cache.len(), 1, "first call must produce one cache entry"); + + // Second call: cache hit → must return the same blended column. + let col2 = cache.get_or_derive(seed, body_id, &district_primary, &chunk, tile_x, tile_y); + assert_eq!( + cache.len(), + 1, + "second call must be a cache hit (len unchanged)" + ); + + assert_eq!( + col1.elevation_m, col2.elevation_m, + "cache hit must return identical blended elevation for boundary tile" + ); + assert_eq!( + col1.terrain, col2.terrain, + "cache hit must return identical terrain for boundary tile" + ); + + // Sanity: the blended elevation must sit between the two district values. + // primary elev_q=20 → base_elev_m=10; secondary elev_q=70 → base_elev_m=35. + // At 50-50 blend the blended elev_q = (20*128 + 70*127 + 127)/255 = 44 (rounded). + // base_elev_m = 44/2 = 22. With micro-relief ±4 m, range is [18, 25] m. + // This asserts the blend actually ran (not the pure-primary 10 m result). + assert!( + col1.elevation_m > 15, + "blended boundary tile elevation ({}) must be above pure-primary level (~10 m) \ + — blend did not apply", + col1.elevation_m + ); + } + // ----------------------------------------------------------------------- // Domain warp sanity // -----------------------------------------------------------------------