From 70cf6e31846bf13ec2eb53c253fe1ddf68c3c965 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 16:40:30 +0000 Subject: [PATCH] Log test-audit run from 2026-07-15 PR #758 fixed an RNG-reuse bug in game_loop_test.rs and trimmed an oversized loop in combat_submodules_test.rs; remaining findings (redundant test coverage across deep_tests/game_tick_tests/ achievement_tests, and two production-side unseeded-RNG call sites) are flagged for human review, not auto-fixed. --- .claude/skills/meta-audit/history/test-audit.jsonl | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/skills/meta-audit/history/test-audit.jsonl b/.claude/skills/meta-audit/history/test-audit.jsonl index 278d087a..ea2c191a 100644 --- a/.claude/skills/meta-audit/history/test-audit.jsonl +++ b/.claude/skills/meta-audit/history/test-audit.jsonl @@ -10,3 +10,4 @@ {"type":"run","date":"2026-07-13","commit_sha":"c507a1c368daffb11e1e909cec719eb7b745eb8b","pr_url":"https://github.com/stphung/quest/pull/735","agent_count":3,"scope":["tests/game_tick_tests/","tests/combat_tests/","tests/character_tests/","tests/zone_tests/","tests/fishing_tests/","tests/deep_tests/","tests/haven_tests/","tests/enhancement_tests/","tests/stormglass_tests/","tests/loom_tests/","tests/item_tests/","tests/achievement_tests/","tests/history_tests/","tests/misc_tests/"],"findings":[{"location":"tests/game_tick_tests/tick_integration_test.rs:1071-1074","claim":"Comment claimed spawn_enemy_if_needed and apply_tick_xp use thread_rng() internally and need refactoring for determinism","correct_value":"spawn_enemy_if_needed(state: &mut GameState) takes no RNG (no randomness); apply_tick_xp already threads the caller's seeded RNG","severity":"MEDIUM","category":"stale-comment","auto_fixed":true},{"location":"src/combat/enemy_generation.rs:10,39,274","claim":"generate_zone_enemy/generate_subzone_boss/generate_dungeon_elite/generate_dungeon_boss call rand::rng() directly with no seeded-RNG entry point, unlike items::drops which exposes generate_item_with_rng","correct_value":"Tests exercising these functions (tests/fishing_tests/fishing_core_coverage_test.rs, tests/item_tests/item_combat_coverage_test.rs) must pad loop bounds (e.g. 3500 vs 500-1200) or average over 200 samples to absorb non-determinism instead of using a fixed seed","severity":"HIGH","category":"unseeded-production-rng","auto_fixed":false},{"location":"src/dungeon/generation.rs:23","claim":"generate_dungeon() calls rand::rng() internally with no seeded variant","correct_value":"tests/misc_tests/dungeon_completion_test.rs loops 5-10x on structural assertions to compensate; same root cause/fix shape as enemy_generation.rs","severity":"MEDIUM","category":"unseeded-production-rng","auto_fixed":false},{"location":"tests/game_tick_tests/ (tick_integration_test.rs, tick_stage_coverage_test.rs, game_tick_behavior_test.rs, game_loop_test.rs)","claim":"create_strong_character()/test_rng() duplicated across 3+ files with no shared helpers.rs (unlike combat_tests/character_tests)","correct_value":"tick_stage_coverage_test.rs's copy additionally sets Constitution=30, a silent divergence from tick_integration_test.rs's copy","severity":"MEDIUM","category":"duplicated-test-scaffolding","auto_fixed":false},{"location":"tests/game_tick_tests/game_loop_orchestration_test.rs vs tick_stage_coverage_test.rs","claim":"Haven-discovery gating (prestige 10 requirement, dungeon/fishing blocking) tested near-identically in both files via different call paths","correct_value":"Consolidation candidate: keep unit-level tests in one file, trim the other to 1-2 integration smoke tests","severity":"LOW","category":"redundant-coverage","auto_fixed":false},{"location":"tests/fishing_tests/fishing_extended_coverage_test.rs, fishing_integration_test.rs, fishing_edge_cases_test.rs","claim":"roll_fish_rarity rank-scaling Monte Carlo property tested independently in 3 files with different seeds/trial counts","correct_value":"Consolidation candidate: one parametrized table-driven test over ranks [1,20,30,40]","severity":"MEDIUM","category":"redundant-coverage","auto_fixed":false},{"location":"tests/enhancement_tests/enhancement_test.rs, enhancement_coverage_test.rs, enhancement_edge_cases_test.rs","claim":"success_rate() and soulforge-discovery boundary/zero/exact-value checks duplicated across all 3 files under different naming conventions","correct_value":"Consolidation candidate: pick one file as canonical owner","severity":"MEDIUM","category":"redundant-coverage","auto_fixed":false},{"location":"tests/stormglass_tests/stormglass_test.rs, tests/enhancement_tests/enhancement_coverage_test.rs","claim":"Full game_tick harness (state/tick_counter/enhancement/haven/achievements/deep/loom/rng) inline-constructed per test with no shared helpers.rs, unlike fishing_tests/helpers.rs and loom_tests/helpers.rs","correct_value":"Add tests/stormglass_tests/helpers.rs and a shared make_tick_harness() helper for enhancement_coverage_test.rs","severity":"LOW","category":"duplicated-test-scaffolding","auto_fixed":false},{"location":"tests/achievement_tests/achievement_handlers_test.rs:854,866 vs achievement_coverage_test.rs:819,844","claim":"test_grand_champion_at_100_wins and its boundary companion are duplicated verbatim (same logic, different names) across two files","correct_value":"Delete the duplicate copy, keep the coverage_test.rs version with clearer assert messages","severity":"LOW","category":"duplicate-test","auto_fixed":false},{"location":"tests/misc_tests/prestige_cycle_test.rs:270-330,357-380","claim":"test_combat_to_prestige_full_loop grinds a full combat/XP simulation for up to 20,000 ticks (plus 2,000 post-prestige) to reach character level 10 before testing perform_prestige() reset, making it the slowest test in the reviewed scope","correct_value":"Level/XP could be set directly on GameState before perform_prestige(); post-prestige verification loop could shrink substantially; needs human judgment since it changes how (not just how fast) the test reaches its precondition","severity":"MEDIUM","category":"heavy-test-setup","auto_fixed":false},{"location":"tests/game_tick_tests/tick_stage_coverage_test.rs vs tick_stages_coverage_test.rs","claim":"Near-identical file names (57 tests targeting game_tick() vs 136 tests targeting core::tick_stages directly) invite future duplication/misplacement","correct_value":"Rename one file for clarity; requires updating the mod declaration in tests/game_tick_tests.rs too","severity":"LOW","category":"naming-clarity","auto_fixed":false},{"location":"brute-force RNG-search loops (multiple test files across all scopes)","claim":"~40+ tests loop over many seeds/iterations searching for a rare RNG event","correct_value":"Known, tracked debt recurring unresolved since 2026-07-02; requires seed research or production RNG-injection change, both outside this skill's auto-fix scope","severity":"LOW","category":"known-brute-force-rng-debt","auto_fixed":false}]} {"type":"run","date":"2026-07-14","commit_sha":"e5d4903640198422af4f6363e794d37a1dcee42d","pr_url":"https://github.com/stphung/quest/pull/749","agent_count":3,"scope":["tests/game_tick_tests/","tests/combat_tests/","tests/character_tests/","tests/zone_tests/","tests/fishing_tests/","tests/deep_tests/","tests/haven_tests/","tests/enhancement_tests/","tests/stormglass_tests/","tests/loom_tests/","tests/item_tests/","tests/achievement_tests/","tests/history_tests/","tests/misc_tests/"],"findings":[{"location":"tests/game_tick_tests/game_loop_test.rs:384","claim":"test_offline_progression_with_long_absence asserts elapsed_seconds >= ten_days_seconds - 2, comparing against a live re-read of Utc::now() with only a 2-second buffer; can flake under CI scheduling delays","correct_value":"widen tolerance to 30 seconds","severity":"HIGH","category":"timing-assertion","auto_fixed":true},{"location":"tests/game_tick_tests/game_loop_test.rs:421","claim":"test_short_offline_time_still_processes asserts elapsed_seconds >= 28 against a 30-second base offset (~7% margin), tight relative buffer","correct_value":"widen base offset to 300s and assert >= 270 (30s buffer)","severity":"MEDIUM","category":"timing-assertion","auto_fixed":true},{"location":"tests/game_tick_tests/game_loop_orchestration_test.rs:1350,1476","claim":"test_haven_discovery_via_game_tick_at_p10 and test_haven_discovery_debug_mode_suppresses_save each run an independent 100-seed x 50-tick (5,000 iteration) brute-force discovery search at P200, differing only in debug_mode flag and which result field is checked; redundant ~10,000 total seeded-RNG tick iterations","correct_value":"extract a shared helper that finds one discovery-triggering seed once and reuses it for both mode assertions","severity":"MEDIUM","category":"redundant-coverage","auto_fixed":false},{"location":"tests/fishing_tests/fishing_core_coverage_test.rs:1097","claim":"Loop bound of 3500 (vs ~500-1000 elsewhere) exists because src/combat/enemy_generation.rs (lines 10, 39, 274) calls rand::rng() (unseeded OS RNG) instead of a caller-supplied seeded RNG; test's own comment documents empirically observed variance (max first-trigger tick ~1361 across 15k sampled runs) rather than a proven bound","correct_value":"thread caller-supplied RNG through combat::enemy_generation (mirroring the fix already done for items::drops in #692); production-code change, out of auto-fix scope","severity":"HIGH","category":"unseeded-rng","auto_fixed":false}]} {"type":"eval_marker","date":"2026-07-14","pr_url":"https://github.com/stphung/quest/pull/751","runs_covered":5} +{"type":"run","date":"2026-07-15","commit_sha":"304d0159c7c4e5f4f89795152fbdb02ef6da233c","pr_url":"https://github.com/stphung/quest/pull/758","agent_count":3,"scope":["tests/game_tick_tests/","tests/combat_tests/","tests/character_tests/","tests/zone_tests/","tests/fishing_tests/","tests/deep_tests/","tests/haven_tests/","tests/enhancement_tests/","tests/stormglass_tests/","tests/loom_tests/","tests/item_tests/","tests/achievement_tests/","tests/history_tests/","tests/misc_tests/"],"findings":[{"location":"tests/game_tick_tests/game_loop_test.rs:24","claim":"simulate_tick()/simulate_ticks() reseeded a fresh ChaCha8Rng(42) on every call, so multi-tick loops replayed the identical roll every tick instead of a real sequence","correct_value":"Thread one ChaCha8Rng across each loop via simulate_tick_with_rng; removed the now-unused simulate_tick() wrapper","severity":"HIGH","category":"rng-reuse-bug","auto_fixed":true},{"location":"tests/combat_tests/combat_submodules_test.rs:1542","claim":"test_consecutive_deaths_triggers_retreat looped 0..100 for an event that resolves within ~20 ticks (one 2.0s enemy attack interval at 0.1s/tick)","correct_value":"Trimmed loop bound to 40 (2x margin); verified still passes","severity":"LOW","category":"structural-impossibility-loop","auto_fixed":true},{"location":"tests/game_tick_tests/game_tick_supplemental_test.rs:111","claim":"test_storm_leviathan_achievement_via_game_tick loops 0..1000 for a rare event with a tautological fallback assertion if it never fires","correct_value":"Part of the known ~40-test brute-force RNG-search category (unresolved since 2026-07-02); needs seed research or production RNG-injection, out of auto-fix scope","severity":"HIGH","category":"probabilistic-brute-force","auto_fixed":false},{"location":"tests/game_tick_tests/game_tick_supplemental_test.rs:762","claim":"test_subzone_boss_defeated_advances_zone (0..10_000) gates its real assertion behind an `if has(...)`, silently no-op-ing if the boss isn't defeated in time; duplicates coverage in tick_stage_coverage_test.rs:1032 and game_tick_behavior_test.rs:368","correct_value":"Needs a human decision on which of the 3 duplicate tests to keep and whether to remove the assertion guard (behavior change, not mechanical)","severity":"HIGH","category":"masked-assertion","auto_fixed":false},{"location":"tests/game_tick_tests/tick_stage_coverage_test.rs:1032","claim":"test_subzone_boss_defeated_advances_progression (0..2_000) has the same masked-assertion pattern, third file testing the same boss-defeat-advances-subzone behavior","correct_value":"Consolidation/removal decision needed alongside the other two duplicate tests","severity":"MEDIUM","category":"masked-assertion+redundant-coverage","auto_fixed":false},{"location":"tests/game_tick_tests/game_loop_orchestration_test.rs:535 (+16 more in same file)","claim":"~17 for-loop-with-early-break brute-force RNG-search tests, largest concentration of the known tracked category","correct_value":"Known, tracked debt since 2026-07-02; not re-derived, needs seed research or RNG-injection change","severity":"MEDIUM","category":"probabilistic-brute-force","auto_fixed":false},{"location":"tests/game_tick_tests/tick_stages_coverage_test.rs:1409 (+11 more in same file)","claim":"~12 more instances of the same known brute-force RNG-search category","correct_value":"Known, tracked debt since 2026-07-02; out of auto-fix scope","severity":"LOW","category":"probabilistic-brute-force","auto_fixed":false},{"location":"tests/game_tick_tests/tick_stage_coverage_test.rs:605","claim":"test_haven_discovery_sets_haven_changed_and_achievements_changed runs 8 seeds x 1000 ticks (8000 game_tick calls); well-justified with explicit probability math but expensive","correct_value":"Could boost the discovery-chance constant in test setup or reduce seed count, but changes statistical assumptions — left for human review","severity":"LOW","category":"performance-heavy-loop","auto_fixed":false},{"location":"tests/game_tick_tests/ (6 of 7 files)","claim":"Hand-rolled near-identical strong_state/create_strong_character/fresh_state/test_rng/seeded_rng helpers with drifted attribute values (e.g. CON=50 vs CON=30, one omits it) instead of a shared helpers.rs like combat_tests/ and character_tests/ already have","correct_value":"Extract to tests/game_tick_tests/helpers.rs after reconciling the drifted CON values — a behavior decision, not pure mechanics","severity":"LOW","category":"redundant-setup","auto_fixed":false},{"location":"tests/deep_tests/ (deep_types_coverage_test.rs:1142, deep_tutorial_test.rs:819, deep_mission_test.rs:638, plus ~20 other duplicate test-name pairs across the directory)","claim":"test_deep_state_serde_roundtrip triplicated; ~20 more test names duplicated verbatim across coverage-test files and their original unit-test files (e.g. 9 test_prestige_preserves_* tests in deep_prestige_persistence_test.rs vs deep_integration_test.rs)","correct_value":"Needs human judgment on which file is canonical per duplicate group before consolidating","severity":"MEDIUM","category":"redundant-coverage","auto_fixed":false},{"location":"tests/deep_tests/ (deep_mission_test.rs:48, deep_mission_lifecycle_test.rs:47, deep_missions_coverage_test.rs:46, deep_integration_test.rs:41, deep_pool_refresh_test.rs:39, deep_prestige_persistence_test.rs:40)","claim":"No shared deep_tests/helpers.rs; make_merc/force_discover/build_generation_1_state duplicated with drifted signatures — deep_missions_coverage_test.rs's make_merc hardcodes resilience/expertise instead of using archetype.base_stats(), a subtle inconsistency risk","correct_value":"Extract shared helpers.rs and reconcile the divergent make_merc variant — needs human review of which becomes canonical","severity":"LOW-MEDIUM","category":"redundant-setup","auto_fixed":false},{"location":"tests/fishing_tests/, tests/enhancement_tests/, tests/stormglass_tests/ (~23 instances, e.g. enhancement_test.rs:1055 `0..200_000`)","claim":"Loop-with-early-break rare-event search tests; all seeded/deterministic (not flaky) but a performance anti-pattern","correct_value":"Same known tracked category as game_tick_tests instances; out of auto-fix scope","severity":"LOW","category":"probabilistic-brute-force","auto_fixed":false},{"location":"src/combat/enemy_generation.rs, surfaced at tests/fishing_tests/fishing_core_coverage_test.rs:1097","claim":"generate_enemy_name/calc_zone_enemy_stats/generate_zone_enemy_name still call rand::rng() directly instead of an injected seeded RNG (unlike items::drops, fixed in #692); test needs a 3500-iteration bound instead of ~1000 to compensate","correct_value":"Genuine (if rare) non-determinism source in production code; needs a production-side RNG-injection change, not a test-only fix. Margins are wide enough that flake risk is negligible today","severity":"MEDIUM","category":"unseeded-rng-production","auto_fixed":false},{"location":"tests/fishing_tests/fishing_extended_coverage_test.rs:492-515, fishing_core_coverage_test.rs:902-921","claim":"Monte Carlo rate tests use far more trials (2000, 5000) than their generous tolerance bands require","correct_value":"Could reduce trial count but changes the false-failure rate — needs manual statistical verification, not auto-fixed","severity":"LOW","category":"performance-heavy-loop","auto_fixed":false},{"location":"tests/haven_tests/ (test_tier_cost_invalid_tiers_return_zero, duplicated)","claim":"Same test name/body appears twice","correct_value":"Low priority, harmless duplicate — flagged for cleanup, not fixed","severity":"LOW","category":"redundant-coverage","auto_fixed":false},{"location":"tests/achievement_tests/achievement_handlers_test.rs & achievements_expanded_test.rs (7 test names, e.g. test_unlock_records_timestamp, test_unlocked_count_starts_at_zero, test_take_newly_unlocked_returns_and_drains)","claim":"Near-identical/verbatim-duplicate tests across the two files (test_grand_champion_at_100_wins also duplicated a third time in achievement_coverage_test.rs)","correct_value":"Needs human judgment on which copy to keep before consolidating","severity":"MEDIUM","category":"redundant-coverage","auto_fixed":false},{"location":"tests/achievement_tests/achievement_handlers_test.rs:1478, achievements_expanded_test.rs:517","claim":"test_unlock_records_timestamp brackets a real chrono::Utc::now() call with only a +1s tolerance buffer","correct_value":"Reviewed: unlocked_at is recorded strictly between before/after so the assertion is safe regardless of buffer size; left as-is rather than risk-free-but-unnecessary churn","severity":"LOW","category":"timing-assertion","auto_fixed":false},{"location":"tests/item_tests/item_combat_coverage_test.rs:663,712,729,779","claim":"4 tests call production enemy-generation functions using rand::rng() internally rather than a seeded RNG — same production issue as the fishing_tests finding above, distinct call sites","correct_value":"Margins are wide enough that flake risk is negligible today; needs the same production-side RNG-injection fix, not test-side","severity":"LOW-MEDIUM","category":"unseeded-rng-production","auto_fixed":false},{"location":"tests/item_tests/item_pipeline_test.rs:236,280,964; item_generation_scoring_test.rs:392,471,519","claim":"Seeded/deterministic trials (2000x5 samples, 10,000 trials, 3000 samples) sample far more than needed to prove their stated generous statistical margins","correct_value":"Auto-fixable in principle (reduce trial count) but reducing sample size changes false-failure rate — left for human review per the skill's guidance against over-aggressive loop-count cuts","severity":"LOW-MEDIUM","category":"performance-heavy-loop","auto_fixed":false},{"location":"tests/achievement_tests/achievement_handlers_test.rs:44","claim":"test_slayer_i_at_exactly_100_kills loops 99 times to reach a kill threshold while sibling tests in the same file just set the counter field directly","correct_value":"99 iterations is trivially cheap and the loop form arguably exercises on_enemy_killed's increment path more realistically than direct field assignment; not worth changing","severity":"LOW","category":"style-inconsistency","auto_fixed":false}]}