refactor: remove distant terrain LOD

[MichaelFisher1997]

Summary

• Remove the distant-terrain LOD subsystem, worldgen sampling/cache APIs, Vulkan/RHI compact pipelines and shaders, horizon settings, and dedicated Phase 5 diagnostics/gates.
• Retain ordinary full-detail streaming, progressive startup, queue reprioritization, saves, menus, water, shadows, F4 timing controls, and standard benchmark scenarios.
• Preserve legacy graphics/input settings migration and enable focused regression test roots.
• Preserve the complete experiment remotely on feature/backup-distant-lod-20260907 at 14df75b9 for possible future work.

Verification

• Debug and ReleaseSafe zig build test: 36/36 steps passed in each mode.
• ReleaseFast build passed; integration and robustness passed.
• Offscreen playable-world and main-menu captures completed.
• ReleaseSafe world capture with Khronos validation completed without validation errors after retaining the independent FXAA dependency fix.
• Normal-world startup at render distance 22 reached 1,517 renderable chunks with generation, mesh, and upload queues empty.
• Five-second Low traversal benchmark completed and passed artifact validation.

Scope

This intentionally removes the earlier LOD implementation already on dev, not merely the latest experiment or an enable flag. Saved worlds and local capture artifacts are not deleted. Old performance baselines containing removed LOD measurements are removed rather than compared against the new renderer. Local unrelated devenv.lock and dist-ignore changes are excluded.

[github-actions]

kcov coverage

Line coverage ran for this PR and uploaded a non-blocking report artifact named kcov-report. Codecov upload is configured as non-blocking while the project captures a stable baseline.

[blacksmith-sh]

Found 1 test failure on Blacksmith runners:

Failure

Test	View Logs
(TestExpectedEqual)/(TestExpectedEqual)	View Logs

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[github-actions]

All verification complete. Compiling the final review.

---

📋 Summary

Linked issues: The PR body contains no issue-closing keywords (Fixes, Closes, Resolves). No linked issues to verify. (It does remove ADR docs/decisions/917-compact-gpu-lod-tiles.md and the LOD steering doc for #752, consistent with removing the feature those documents describe.)

Previous reviews: No previous automated reviews found — nothing to re-verify or acknowledge.

This PR removes the entire distant-terrain LOD subsystem (~70k lines): the world-lod module, RHI/Vulkan LOD API surface, compact pipelines/shaders, LOD descriptor snapshots, horizon settings, Phase 5 gates, and LOD benchmark telemetry, while carefully preserving user-facing migrations (legacy input bindings, settings JSON) and rewriting the benchmark harness to a simpler schema-v4 form with SLO enforcement. This is a high-quality removal PR: I verified zig fmt --check passes, the full zig build test suite passes (585 tests incl. shader/SPIR-V/shadow-ABI checks), ReleaseFast builds, the Python benchmark validator self-test passes, and a headless Weston/Lavapipe smoke run initializes Vulkan and renders offscreen cleanly.

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified. Verified: no dangling references to any removed symbol (setLOD*, CompactLOD*, ILODCullingSystem, horizon_distance, toggle_lod_render, LOD_TILE_ID, prepareLODCulling); GPU ABI changes are consistent — shader InstanceData shrunk to 64 bytes matches CPU extern struct (modules/engine-rhi/src/rhi_types.zig:244), push-constant mask_radius removal is mirrored in every ModelUniforms writer, and removed varyings (locations 14/16) are matched in all consumer shaders; SPIR-V baseline parity is test-enforced and passing.

⚠️ High Priority Issues (Should Fix)

None identified. Notably, the input-settings migration is a genuine bug fix: on dev, v2 positional array migration copied bindings 1:1 into the smaller enum, shifting every action after toggle_lod_render (old index 40 — verified against dev's GameAction order). actionForLegacyBindingIndex (modules/game-core/src/input_settings.zig:256-265) correctly skips slot 40 and shifts subsequent indices, with tests covering both array and object formats. The FXAA subpass-dependency srcAccessMask addition (modules/engine-graphics/src/vulkan/fxaa_system.zig:89) is a real synchronization correctness fix.

💡 Medium Priority Issues (Nice to Fix)

None identified.

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/game-core/src/benchmark.zig:216 + src/game/app.zig:597 - Zero-delta frame now aborts the entire benchmark run
Confidence: High
Description: recordFrame returns error.InvalidBenchmarkSample when dt <= 0 or non-finite, and app.zig propagates it with try. The previous code tolerated dt == 0 (if (dt > 0.000001) 1.0/dt else fps). A single pathological frame (e.g., two SDL_GetPerformanceCounter reads in the same tick) fails the whole run instead of skipping one sample.
Impact: Rare, but a spurious benchmark failure with no artifact.
Suggested Fix: catch |err| if (err != error.InvalidBenchmarkSample) return err at the call site, or skip recording the sample and log a warning.

[LOW] modules/game-core/src/benchmark.zig:358-364 - Unknown preset name now falls back to the most lenient (extreme) SLO thresholds
Confidence: Medium
Description: thresholdsForPreset previously returned high-preset-like defaults for unrecognized names; it now returns the extreme values (fps_p1_min = 3, draw_calls_max = 5500). A typo'd -Dbenchmark-preset silently gets the loosest gates in the Zig-side check. (Mitigated in CI: benchmark_baseline.py rejects presets outside low/medium/high.)
Impact: A misconfigured local benchmark could pass SLOs it shouldn't.
Suggested Fix: Return an error or the strictest preset thresholds for unrecognized names.

[LOW] modules/game-core/src/benchmark.zig:249-257 - Failed validation now leaves no JSON artifact
Confidence: High
Description: writeResults calls validateResults before creating the output file; dev wrote the artifact first, then enforced SLOs. On an SLO breach the detailed metrics now exist only in the log line.
Impact: Slightly harder post-mortem of CI benchmark failures.
Suggested Fix: Write the artifact (e.g., with a completion-adjacent breach marker) before returning the error, or keep as-is if fail-closed artifacts are intentionally avoided.

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	9	Removal tightens responsibilities: RHI sheds LOD concerns; benchmark.zig returns to pure frame sampling + SLO policy
Open/Closed	7	Thresholds/validator remain extension points; wholesale vtable edits are inherent to a removal PR
Liskov Substitution	8	IWorldRenderView/IWorld/vtable implementers (Vulkan backend, mocks, ECS) all updated in lockstep; tests pass
Interface Segregation	8	Deleting LOD-specific vtable entries (drawCompactLOD, setLOD*, supportsCompactLODGpuCulling) actively improves segregation
Dependency Inversion	7	world-runtime no longer depends on world-lod; modules still depend on RHI abstractions, not Vulkan directly
Average	7.8

🎯 Final Assessment

Overall Confidence Score: 88%

Confidence Breakdown:

• Code Quality: 90% (clean removals, careful migrations, no dangling refs; minor strictness regressions in benchmark error paths)
• Completeness: 90% (all stated retention goals verified: full-detail streaming, F4 timing, settings migration, saves, standard scenarios, backup branch referenced)
• Risk Level: 88% (large surface but verified: Debug tests 585 pass, ReleaseFast build, headless Vulkan smoke run clean; graphics regression risk limited to removed LOD paths and theInstanceData/push-constant ABI, which is test-enforced)
• Test Coverage: 85% (new migration/SLO tests added and — importantly — build.zig now registers them as dedicated test roots so they actually execute; integration/robustness documented as run by author)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing (if applicable)

Verdict:

MERGE

A meticulously executed removal PR — verified locally across format, full test suite, ReleaseFast, and a headless Vulkan smoke run — whose only findings are low-severity benchmark-harness ergonomics.

{
  "reviewed_sha": "40e1b9fb179f2c689d663d6824dfe6c4aa4d4c48",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 88,
  "recommendation": "MERGE"
}

opencode session  |  github run