From 7776d99c1449d5c56e52cc85ab3858ceafff3c6f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 9 Jun 2026 08:18:58 +0200 Subject: [PATCH] =?UTF-8?q?test(simulation):=20isolate=20voice::lookup=20c?= =?UTF-8?q?ache=20dirs=20=E2=80=94=20fix=20flaky=20cache=5Fmiss=20(T-1035)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four voice::lookup tests shared one on-disk cache directory (base_dir temp/sr-voice-lookup-test + world_seed 99). VoiceCacheStore persists on Drop (save_all) and reloads on first zone access (load_zone), and cache_hit stores 'Voiced line.' under the identical (zone_id=100, CacheKey) that cache_miss looks up — so under parallel execution the hit test's Drop/save could leak into the miss test's lookup, returning 'Voiced line.' instead of 'Base line.'. Give each test a directory keyed by test label + process id, so neither parallel tests nor concurrent cargo test runs collide. Verified clean across repeated runs. Closes T-1035. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 2 ++ server/src/voice/lookup.rs | 24 +++++++++++++++++------ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index ad7bade3a..337f3f0e9 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -261,3 +261,4 @@ test, the +356C climate bug) — keep reviewing + verifying agent output, don''t INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1031', 'status', 'in_progress', 'done', NULL, '2026-06-08 20:38:59', '2026-06-08 20:38:59', '2026-06-08 20:38:59', NULL, 'db4d162c8bddb6ead74d6d6458c1a41c', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1034', 'status', 'backlog', 'done', NULL, '2026-06-08 21:01:34', '2026-06-08 21:01:34', '2026-06-08 21:01:34', NULL, '03c07199221bba3a0305d5e0e2d81ec5', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-974', 'status', 'backlog', 'done', NULL, '2026-06-08 21:01:34', '2026-06-08 21:01:34', '2026-06-08 21:01:34', NULL, 'eb367d867a3258091fc50d8ad6743748', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1035', 'status', 'backlog', 'done', NULL, '2026-06-09 06:18:57', '2026-06-09 06:18:57', '2026-06-09 06:18:57', NULL, '507ddc172f2e7337b9b27792359791b2', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 45ff82428..6b2c1c870 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -730,3 +730,5 @@ bugs on EVERY PR this session (hollow tests, a cache-coherence bug hidden by a t test, the +356C climate bug) — keep reviewing + verifying agent output, don''t trust "all green".', 'done', 'high', NULL, 'server', 'D-239', '2026-06-07 10:34:11', '2026-06-08 20:38:59', NULL, '1120adccd354f2864b061fa8dba28b1b', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1034', 'bug', 'T-974', 'derive_moisture_q hydrosphere vocab mismatch — liquid_water etc. fall to default', 'derive_moisture_q (region_profile.rs) matches hydrosphere on ''ocean''/''rivers''/''ice''/''subsurface''/''none'', but the actual bodies.hydrosphere vocab in systems.db is: liquid_water (175 bodies), ice (962), rivers (31), moderate (21), ocean (only 19), minimal (10), trace (8), subsurface_liquid (7), rivers-lakes (4), extensive (4), subsurface_ice (3), ocean-coastal (1), plus NULL (1649). So ''liquid_water'' and most surface-water worlds fall to the ''_ => 30'' default instead of high moisture — propagating wrong precipitation_class / glaciation_grade / vegetation_class for ~175+ water worlds. Same root cause as D-240 (code keying on a vocab the data doesn''t use). Fix: align derive_moisture_q (and audit any other hydrosphere consumer) to the real vocab — surface liquid (liquid_water/ocean/ocean-coastal/extensive) -> high; rivers/rivers-lakes/moderate -> mid; ice/subsurface* -> low; minimal/trace/none/NULL -> arid. T-1033''s maritime_factor table already maps the real vocab and can be the reference. Found during T-1033 verification (2026-06-08).', 'done', 'high', NULL, 'server', 'D-240', '2026-06-08 17:00:07', '2026-06-08 21:01:34', NULL, '99d8cef76a78b012c9838f1dd033fa62', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-974', 'epic', 'T-750', 'Atlas-to-tile derivation model', 'Foundational world-derivation model from the atlas-derivation workshop. D-227 (deterministic-rebuild world model: pure function of seed+atlas, derived on demand + cached, only tile mutators persisted, fully volumetric voxel/subvoxel, vertical extent physical not floor-count, variable-height floors) + D-228 (composite tile schema: orthogonal axes TerrainMaterial/FloorMaterial/Vegetation/Water/elevation, derived shape/tactical-form, region-level morphology zones, cohesion matrix, dynamic clock-bound region seasonal state). Most implementation lands as the cascade reaches the tile layers (Phase 4+). Open design tracked in Q-100 (biome authority), Q-101 (refinement contract + body-class river density + ocean mask), Q-102 (cohesion-matrix algorithm), Q-103 (mutator op schema), Q-104 (floor-to-voxel-z mapping), Q-105 (region seasonal/clock state: water/snow/weather/crops).', 'done', 'high', NULL, 'server', 'D-228', '2026-05-25 10:51:49', '2026-06-08 21:01:34', NULL, '1c48b836fb7b63e287ab13872c6f0aca', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1035', 'bug', NULL, 'voice::lookup tests share an on-disk cache dir — flaky under parallelism', 'voice::lookup::cache_miss_returns_base_text intermittently fails the full-suite push gate under cargo''s default parallel execution (passes in isolation). Root cause is test-isolation, not production nondeterminism: the test helper test_cache() builds every store with the SAME base_dir (temp_dir/sr-voice-lookup-test) and world_seed (99), so all four module tests share the on-disk zone path .../99/100.msgpack. VoiceCacheStore persists to disk on Drop (save_all) and reads it back on first zone access (load_zone). cache_hit_returns_voiced_text stores ''Voiced line.'' under the IDENTICAL (zone_id=100, CacheKey) that cache_miss_returns_base_text looks up; model/injector versions match so the invalidation guard doesn''t help. When the hit test''s Drop/save interleaves with the miss test (after its remove_dir_all, before its lookup), the miss test loads the leaked entry and returns ''Voiced line.'' instead of ''Base line.'' -> assertion fails. Fix: isolate each test''s cache directory (per-test label + process id) so neither parallel tests nor concurrent cargo test runs collide. Found during T-1031/T-1034 verification (2026-06-08).', 'backlog', 'medium', NULL, 'server', NULL, '2026-06-08 21:24:06', '2026-06-08 21:24:06', NULL, '7e34d25bd4c2846c02c7e6627a997502', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-1035', 'bug', NULL, 'voice::lookup tests share an on-disk cache dir — flaky under parallelism', 'voice::lookup::cache_miss_returns_base_text intermittently fails the full-suite push gate under cargo''s default parallel execution (passes in isolation). Root cause is test-isolation, not production nondeterminism: the test helper test_cache() builds every store with the SAME base_dir (temp_dir/sr-voice-lookup-test) and world_seed (99), so all four module tests share the on-disk zone path .../99/100.msgpack. VoiceCacheStore persists to disk on Drop (save_all) and reads it back on first zone access (load_zone). cache_hit_returns_voiced_text stores ''Voiced line.'' under the IDENTICAL (zone_id=100, CacheKey) that cache_miss_returns_base_text looks up; model/injector versions match so the invalidation guard doesn''t help. When the hit test''s Drop/save interleaves with the miss test (after its remove_dir_all, before its lookup), the miss test loads the leaked entry and returns ''Voiced line.'' instead of ''Base line.'' -> assertion fails. Fix: isolate each test''s cache directory (per-test label + process id) so neither parallel tests nor concurrent cargo test runs collide. Found during T-1031/T-1034 verification (2026-06-08).', 'done', 'medium', NULL, 'server', NULL, '2026-06-08 21:24:06', '2026-06-09 06:18:57', NULL, 'f57f8b214f7143f17a37bea5314ac764', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/server/src/voice/lookup.rs b/server/src/voice/lookup.rs index ccac3241c..78d7d9559 100644 --- a/server/src/voice/lookup.rs +++ b/server/src/voice/lookup.rs @@ -72,8 +72,20 @@ pub fn voiced_behavior( mod tests { use super::*; - fn test_cache() -> Arc> { - let dir = std::env::temp_dir().join("sr-voice-lookup-test"); + /// Build a store with a directory unique to this test (T-1035). + /// + /// `VoiceCacheStore` persists to disk on `Drop` (`save_all`) and reads it back + /// on first zone access (`load_zone`). A shared directory therefore lets one + /// test's stored entry leak into another's lookup under parallel execution — + /// the `cache_hit`/`cache_miss` pair use an identical `(zone_id, CacheKey)`, + /// which is exactly how this module flaked. Isolating by test label + process + /// id keeps both parallel tests and concurrent `cargo test` runs from colliding. + fn test_cache(label: &str) -> Arc> { + let dir = std::env::temp_dir().join(format!( + "sr-voice-lookup-test/{}-{}", + std::process::id(), + label + )); let _ = std::fs::remove_dir_all(&dir); Arc::new(Mutex::new(VoiceCacheStore::new( dir, @@ -85,7 +97,7 @@ mod tests { #[test] fn cache_hit_returns_voiced_text() { - let cache = test_cache(); + let cache = test_cache("cache-hit"); let key = CacheKey { culture_id: "van-maanens-star".into(), npc_stable_id: 42, @@ -111,7 +123,7 @@ mod tests { #[test] fn cache_miss_returns_base_text() { - let cache = test_cache(); + let cache = test_cache("cache-miss"); let result = voiced_behavior( &cache, 100, @@ -128,7 +140,7 @@ mod tests { #[test] fn factual_content_always_passthrough() { - let cache = test_cache(); + let cache = test_cache("factual"); // Store a voiced version under the Factual key — it must never be returned. let key = CacheKey { culture_id: "van-maanens-star".into(), @@ -158,7 +170,7 @@ mod tests { #[test] fn tell_behavior_always_passthrough() { - let cache = test_cache(); + let cache = test_cache("tell"); // Even if cache has a voiced version, tell behaviors return base text let key = CacheKey { culture_id: "van-maanens-star".into(),