fix(simulation): VoxelCache precondition doc + sec_phase decontamination + cache blend test (T-1042)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
|
||||
+127
-1
@@ -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
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user