[Audit][Umbrella] SOLID architecture debt: RHI seam leakage, god-objects, and extension-safety gaps

[MichaelFisher1997]

Goal

Track and drive down SOLID-principle debt across the engine. This is the umbrella issue — an in-depth, evidence-cited audit of ~78,726 LOC across 29 modules. Work lands in 4 priority phases (P0–P3), each carved into sub-issues containing small reviewable PRs targeting dev.

The audit is read-only; this issue is the canonical reference. Every finding below cites a file:line so sub-issues can be scoped precisely.

Scope & method

• 4 parallel deep-dives: RHI/graphics, worldgen, world-runtime/meshing/LOD, cross-cutting (duplication, wiring, dependency direction, concurrency).
• Each principle graded per subsystem. Verdict scale: Strong · Adequate · Weak · Violated.
• 32 findings (5 Critical, 21 Major, 6 Minor) + 9 architectural strengths to preserve.

Scorecard

Principle	Overall	Worst area
SRP — Single Responsibility	Weak	lod_mesh.zig (2,277 LOC, ~14 responsibilities)
OCP — Open/Closed	Weak	RHI native-handles seam; hardcoded generator registry
LSP — Liskov Substitution	Adequate	bindShader silent no-op; INativeHandlesContext returns 0 from Mock
ISP — Interface Segregation	Adequate	Dead segregated worldgen interfaces; fat IRenderOptionsContext
DIP — Dependency Inversion	Weak	gpu_mesher.zig downcasts rhi.ptr → *VulkanContext

---

Critical findings

C1 — 16,043 lines of dead code: stale Vulkan fork

Where: src/engine/graphics/vulkan/*.zig (57 files)
Evidence: build.zig:126 is the only graphics root and points at modules/engine-graphics/src/root.zig. Nothing imports src/engine/. The only diffs vs the live copy are import-path rewrites (../rhi.zig → engine-rhi). Compiles nominally, never reached, never tested, silently rots.
Fix: Delete src/engine/graphics/vulkan/ entirely. Zero behavior change.

C2 — INativeHandlesContext leaks Vulkan through the abstraction

Where: modules/engine-rhi/src/rhi.zig:763-830; callers modules/engine-graphics/src/atmosphere_system.zig:39-42, render_graph.zig:645-655
Evidence: 15 methods like getSkyPipeline() u64 return Vulkan handles disguised as u64. Callers immediately cast back to VkPipeline/VkCommandBuffer and issue raw vkCmdBindPipeline/vkCmdBindDescriptorSets. A non-Vulkan backend cannot return a VkPipeline — OCP is violated for sky, water, debug-shadow.
Fix: Move sky/water/shadow draws behind the RHI as first-class methods (drawSky(params), drawWater(...)). Eliminate INativeHandlesContext.

C3 — gpu_mesher.zig bypasses the RHI and downcasts to VulkanContext

Where: modules/world-runtime/src/gpu_mesher.zig:9-11,89; same pattern in modules/engine-graphics/src/lpv_backend.zig:13-15
Evidence:

const graphics = @import("engine-graphics");
const VulkanContext = graphics.VulkanContext;
const vk_ctx: *VulkanContext = @ptrCast(@alignCast(rhi.ptr));  // line 89
// ...then 27 raw c.vk* calls (vkCmdDispatch, vkCmdPipelineBarrier, vkWaitForFences...)

The highest-tier module (world-runtime) imports the concrete Vulkan backend and reaches into private frames/resources. lpv_backend.zig:6-7 candidly documents this as intentional.
Fix: Add a compute facet to RHI.VTable (createComputePipeline, dispatch, pipelineBarrier, pushConstants, fillBuffer). Rewrite gpu_mesher + LPV against it. Remove both @ptrCast downcasts.

C4 — lod_mesh.zig is a 2,277-line god-file (~14 responsibilities)

Where: modules/world-lod/src/lod_mesh.zig
Evidence: One file mixes: RHI buffer vtable (43-126), 4 mesh-build algorithms (193-544), skirt geometry (569-728), column-span math (731-900), UV mapping (901-922), material heuristics (932-1040), vegetation impostors (1041-1407), color sampling (1122-1172), vertex packing (1198-1234), quad emission (1236-1334), height stitching (1077-1120), LOD orchestration (1410-1588), and 686 lines of embedded tests (18 test blocks, 1608-2277).
Fix: Split into lod_mesh_resources.zig, lod_mesh.zig, lod_geometry.zig, lod_materials.zig, lod_vegetation.zig; move tests to lod_mesh_tests.zig.

C5 — Worldgen segregated interfaces are dead code

Where: modules/worldgen-api/src/root.zig:139-177 (IChunkGenerator, ILODHeightmapGenerator, IGeneratorInfoProvider, ICacheRecenterable)
Evidence: No consumer uses them. The registry demands the full fat 7-method Generator, forcing FlatWorldGenerator and ShadowTestWorldGenerator to publish no-op shims (worldgen-flat/src/root.zig:68-90, worldgen-test/src/root.zig:166-187) for methods semantically meaningless to them.
Fix: Either consume the segregated interfaces at registry/runtime call-sites (split registration into "chunk-only" vs "chunk+LOD"), or delete them.

---

Major findings

Single Responsibility (SRP)

• M1 — modules/worldgen-overworld-v2/src/root.zig (1,479 LOC) inlines 9 concerns: noise math, climate, terrain shape, biome select, caves, trees, vegetation, LOD sampling, block/color. Regression vs the well-decomposed v1. Fix: split along v1's collaborator shape.
• M2 — modules/worldgen-overworld/src/overworld_generator.zig (1,107 LOC) inlines LOD tinting (662-725), tree-hint computation (492-621), material layering (884-931). Fix: extract lod_coloring.zig, tree_hints.zig.
• M3 — modules/engine-rhi/src/rhi.zig (1,384 LOC) mixes vtables + 4 wrappers + legacy composite + ~50 deprecated passthroughs (self-admitted at line 1012). VulkanContext (rhi_context_types.zig:192-231) is a 24+-field god-struct spanning every subsystem. Fix: split into interfaces.zig + wrappers.zig; delete passthroughs; group VulkanContext fields.
• M4 — modules/world-runtime/src/world_renderer.zig:487-653 interleaves ~180 lines of diagnostic block-type logging inside the CPU cull loop. Fix: extract world_diagnostics.zig.
• M5 — modules/world-runtime/src/world_mutation.zig:53-92, 137-422 embeds a full BFS lighting engine (~285 lines) inside the block-edit coordinator. Fix: extract LightingEngine.
• M6 — modules/world-runtime/src/world.zig:717-831 embeds the LPV light-grid builder (~115 lines) in the facade. Fix: extract LpvGridBuilder.

Open/Closed (OCP)

• M7 — modules/world-meshing/src/chunk_mesh.zig:178-182 hardcodes the mesher sequence (cross/flat-quad/tall-cross/wall/custom). No strategy table; adding a render_shape forces editing this file. Fix: function-pointer table keyed on render_shape.
• M8 — modules/world-worldgen/src/registry.zig:24-49 hardcodes all 4 generators in GENERATORS. Adding worldgen-foo requires edits to 6 files (build.zig ×2, registry.zig ×2, root.zig, app.zig). Fix: comptime array of *const GeneratorDescriptor.
• M9 — modules/world-worldgen/src/registry.zig:51-65 redefines a factory signature different from the GeneratorDescriptor.create the API already provides, forcing 4 trivially-delegating wrappers. Fix: consume GeneratorDescriptor directly.
• M10 — modules/engine-graphics/src/render_system.zig:127-128 hardcodes rhi_vulkan.createRHI; no backend dispatcher. render_device.zig:14 doc-comment advertises a backend_type param the real signature lacks. Fix: BackendChoice enum or registered factory.

Interface Segregation (ISP)

• M11 — modules/engine-rhi/src/rhi.zig:907-1010 IRenderOptionsContext is a 23-method kitchen sink (quality toggles + device recovery + culling-system factory + frame capture). Fix: split into IRenderQualityOptions / IDeviceRecovery / ICullingSystemFactory / IScreenshotContext.
• M12 — modules/engine-rhi/src/rhi.zig:1028-1046 composite RHI.VTable re-bundles all 14 sub-vtables (comment: (temp)). A backend must implement all 14 even for resource creation only. rhi_vulkan.zig:959-1067 is one 100+-function literal. Fix: allow backends to implement individual sub-interfaces.
• M13 — IWorldSimulation/IWorldRenderView/IWorldTelemetry exist and render_graph.zig uses them, but game-core/session.zig:228,320,355, player.zig:277,386, game-ui/screens/world.zig:135,243 reach past them into the concrete 845-line World. Fix: migrate callers onto the role interfaces.

Liskov Substitution (LSP)

• M14 — modules/engine-rhi/src/rhi_vulkan.zig:764-769 bindShader is a silent no-op in Vulkan but in the public IGraphicsCommandEncoder contract. Undefined behavioral contract.
• M15 — modules/engine-rhi/src/rhi_tests.zig:84-88 Mock returns 0 from getNativeSkyPipeline, forcing callers to defensively special-case 0 (atmosphere_system.zig:31-37) — textbook LSP smell. Resolved implicitly by C2.
• M16 — modules/engine-rhi/src/render_device.zig is split-brain: backend_data always null; real IResourceFactory.createBuffer ignores it. Two unrelated types sharing a name.

Dependency Inversion (DIP)

• M17 — modules/world-worldgen/src/root.zig:53-61 re-exports concrete OverworldGenerator/FlatWorldGenerator/etc.; consumers (src/worldgen_tests.zig:11,19) reach past the registry. DIP boundary unenforced.
• M18 — Triplicated generator_interface.zig re-export shim in world-worldgen, worldgen-overworld, worldgen-api. Fix: @import("worldgen-api") directly.
• M19 — modules/world-runtime/src/world.zig:44-45 imports interface types IWorldRenderView/IShadowScene/ILPVWorld from the concrete engine-graphics. Fix: move contracts into engine-rhi.
• M20 — modules/worldgen-overworld/src/world_map.zig:7-21 a world generator instantiates a GPU Texture via engine-rhi. Worldgen should produce pixels; an upper layer owns the GPU resource.
• M21 — Mutable globals: modules/engine-ui/src/font.zig:8 var active_atlas (hidden, no sync, read every drawText); modules/game-core/src/settings/json_presets.zig:51 pub var graphics_presets (process-wide list, no mutex). Fix: inject the font atlas; mutex-guard graphics_presets.

---

Minor / hygiene

• m1 — Stale build deps: build.zig:265 (world-meshing → engine-graphics) and build.zig:281 (world-worldgen → engine-rhi) have zero @import hits. Remove.
• m2 — ~30 shim re-export files under src/engine/ kept alive only by legacy relative-path imports in src/game/app.zig:6-33. Switch to package imports and delete the tree.
• m3 — modules/engine-core/src/job_system.zig:393 var cleanup_count is test-only at file scope. Move into a test block.
• m4 — modules/worldgen-common/src/lighting_computer.zig:12,53 uses a u8 as a fake anyopaque pointer. Document or restructure.
• m5 — modules/engine-rhi/src/render_device.zig:14 doc-comment lies about a backend_type parameter. Fix doc or implement.
• m6 — modules/engine-graphics/src/rhi_vulkan.zig:742 createShader skips the mutex its siblings hold. Document/align the threading contract.

---

Strengths to preserve (do not regress)

These demonstrate the team executes cleanly — the issues elsewhere are correctable, not structural. Any refactor in the phases below must not break them:

• P1 — Composition root (src/game/app.zig:119-261) is textbook manual constructor injection with errdefer cleanup. No service locator, no IoC container.
• P2 — Worker-thread RHI isolation is rigorously respected: zero RHI/Vulkan calls on workers; GPU upload on main thread; chunk pin()/unpin() discipline (chunk_queue_coordinator.zig:326-353).
• P3 — No dependency cycles; clean layered DAG. Engine never imports world/game. engine-rhi imports only engine-math + engine-core. engine-physics has zero project imports.
• P4 — Mutexes narrowly scoped per-subsystem (SaveManager has 3 separate locks; per-mesh locks; per-device lock). No global lock, no cross-module lock ordering.
• P5 — Worldgen LSP is strong: all 4 generators fully substitutable, no @panic("unsupported")/unreachable in vtable methods, runtime treats them polymorphically.
• P6 — V1 worldgen (overworld_generator.zig) is well-decomposed into named collaborators (NoiseSampler, BiomeSource, CaveSystem, etc.).
• P7 — LOD bridge interfaces (lod_upload_queue.zig LODGPUBridge/LODRenderInterface; lod_mesh.zig:43-126 LODMeshResources) are clean inversion seams — LOD core doesn't import engine-rhi.
• P8 — Meshing leaf modules (cross_mesher, custom_mesh_mesher, flat_quad_mesher, etc.) are tight, single-shape, SRP-respecting.
• P9 — Vulkan managers (render_pass_manager.zig, pipeline_manager.zig, descriptor_manager.zig) carry an explicit "Extracted from rhi_vulkan.zig to eliminate the god object anti-pattern" header — SRP is actively being pursued.

---

Remediation phases

Each phase becomes one or more sub-issues. Phases are ordered by impact and risk; P0 unblocks a second graphics backend, P3 is hygiene that shrinks the surface for the bigger refactors.

P0 — Unblock multi-backend & decruft (Critical)

• [SOLID] [C1] Delete stale Vulkan fork (src/engine/graphics/vulkan/) #782 — delete stale Vulkan fork
• [SOLID] [C3] Add compute facet to RHI.VTable; rewrite gpu_mesher + lpv_backend against it #783 — add compute facet to RHI.VTable; rewrite gpu_mesher + lpv_backend
• [SOLID] [C2] Eliminate INativeHandlesContext; move sky/water/shadow draws behind the RHI #784 — eliminate INativeHandlesContext; move sky/water/shadow draws behind the RHI

P1 — Split the god-objects (Major SRP)

• [SOLID] [C4] Split lod_mesh.zig (2,277 LOC) into focused modules + dedicated test file #785 — split lod_mesh.zig into 5 focused modules + test file
• [SOLID] [M1] Decompose worldgen-overworld-v2 monolith along the v1 collaborator shape #786 — decompose worldgen-overworld-v2/root.zig along v1 shape
• [SOLID] [M3] Split rhi.zig into interfaces/wrappers; decompose VulkanContext god-struct #787 — split rhi.zig into interfaces.zig + wrappers.zig; delete deprecated passthroughs; group VulkanContext fields
• [SOLID] [M5] Extract LightingEngine from world_mutation.zig #788 — extract LightingEngine from world_mutation.zig
• [SOLID] [M6] Extract LpvGridBuilder from world.zig facade #789 — extract LpvGridBuilder from world.zig
• [SOLID] [M4] Extract diagnostics from world_renderer cull loop #790 — extract diagnostics from world_renderer cull loop
• [SOLID] [M2] Extract LOD-coloring and tree-hints from overworld_generator.zig #791 — extract LOD-coloring + tree-hints from overworld_generator.zig

P2 — Close OCP/ISP holes (Major OCP/ISP)

• [SOLID] [M8] Replace hardcoded generator registry with comptime GeneratorDescriptor array (covers M9) #792 — replace generator registry with comptime [*]const *const GeneratorDescriptor; delete wrapper boilerplate
• [SOLID] [M11] Split IRenderOptionsContext into 4 role interfaces #793 — split IRenderOptionsContext into 4 role interfaces
• [SOLID] [C5] Consume or delete dead segregated worldgen interfaces #794 — use or delete dead segregated worldgen interfaces
• [SOLID] [M7] Replace hardcoded mesher dispatch with strategy table #795 — replace hardcoded mesher dispatch with strategy table
• [SOLID] [M13] Migrate game-core/game-ui onto existing role interfaces (stop using concrete World) #796 — migrate game-core/game-ui onto existing role interfaces
• [SOLID] [M12] Allow backends to implement individual RHI sub-interfaces (drop composite bundling) #797 — allow backends to implement individual RHI sub-interfaces; drop composite bundling
• [SOLID] [M10] Add backend dispatcher at RenderSystem.init #798 — add backend dispatcher at RenderSystem.init

P3 — Hygiene (Minor + remaining Major DIP)

• [SOLID] [M19] Move IWorldRenderView/IShadowScene/ILPVWorld contracts into engine-rhi #799 — move IWorldRenderView/IShadowScene/ILPVWorld into engine-rhi
• [SOLID] [M17] Stop re-exporting concrete generators; collapse generator_interface shims (covers M18) #800 — stop re-exporting concrete generators; collapse the 3 generator_interface shims
• [SOLID] [m1] Remove stale addImport lines and delete src/engine/ tree (covers m2) #801 — remove stale addImport lines; switch app.zig to package imports; delete src/engine/ tree
• [SOLID] [M16] Fix or delete split-brain RenderDevice; honor bindShader contract (covers M14, m5) #802 — fix or delete split-brain RenderDevice; fix bindShader/doc lies
• [SOLID] [M21] Inject font atlas; mutex-guard graphics_presets #803 — inject font atlas; mutex-guard graphics_presets
• [SOLID] [M20] Worldgen produces pixels; upper layer owns the GPU texture #804 — worldgen produces pixels, upper layer owns GPU texture
• [SOLID] [m3] Misc cleanup: test-only globals, fake anyopaque, createShader mutex (covers m4, m6) #805 — misc cleanup

---

Constraints

• Use existing architecture (RHI vtables, job tokens, pin/unpin, LODGPUBridge, role interfaces) — no parallel systems.
• Small reviewable PRs targeting dev; conventional commits (refactor:, feat:); never push to dev directly.
• Preserve current gameplay/rendering behavior except where a sub-issue explicitly changes it.
• All build/test commands wrapped in nix develop --command.
• Verification baseline for every PR: nix develop --command zig build test (includes shader validation); rendering-touching PRs also run headless crash/screenshot/benchmark skill checks with -Dskip-present.
• P0 work that touches the RHI seam must keep the existing Vulkan path green end-to-end before any second-backend work is considered.

Key risks

• RHI seam refactor (C2/C3) is the highest-risk, highest-value work. Sky/water/LPV/GPU-mesher all bypass the abstraction today; moving them behind the RHI without regressing visuals requires headless screenshot baselines before and after.
• lod_mesh.zig split (C4) touches the largest file in the repo with 686 lines of embedded tests — tests must move with their code, not be dropped.
• worldgen-overworld-v2 decomposition (M1) must preserve terrain output bit-for-bit; pin a golden-output test before splitting.
• Threading: any refactor near the job system must keep worker-thread RHI isolation (P2) intact — no new ad-hoc locks, no RHI calls off the main thread.
• Phasing dependency: P1 god-object splits should land before P2 interface work where possible, so the new interfaces are designed against already-decomposed collaborators.

[github-actions]

Audit validated — recommend carving sub-issues in priority order

Classification: Refactor / architecture-debt umbrella. Not a bug. The body is correctly self-scoped: every finding cites file:line and a fix, and all implementation is explicitly deferred to sub-issues. This issue should remain a tracking/coordination artifact; no code work belongs on it.

---

Spot-check verification of the top findings

I verified a sample of the findings against the current tree. All check out:

Finding	Status	Evidence
C1 stale Vulkan fork	✅ confirmed	src/engine/graphics/vulkan/ has 57 .zig files; build.zig only wires modules/engine-graphics; zero @import("src/engine/...)frommodules/`. The tree is dead.
C2 INativeHandlesContext	✅ confirmed	modules/engine-rhi/src/rhi.zig:763-830 returns u64; line 237 puts native: INativeHandlesContext in the RHI struct; callers @ptrCast back to VkPipeline / VkCommandBuffer.
C3 gpu_mesher.zig downcast	✅ confirmed	modules/world-runtime/src/gpu_mesher.zig:10 imports graphics.VulkanContext; line 89 does const vk_ctx: *VulkanContext = @ptrCast(@alignCast(rhi.ptr));. Same pattern in lpv_backend.zig:13-15.
C4 lod_mesh.zig god-file	✅ confirmed	exactly 2,277 LOC via wc -l.
m1 stale build deps	✅ confirmed	build.zig:265 (world-meshing → engine-graphics) and build.zig:281 (world-worldgen → engine-rhi) have zero @import hits in their respective module trees. Pure dead addImport lines.

The remaining M1–M21 + m2–m6 citations are precise enough to scope PRs from this issue alone.

---

Recommended first sub-issue: P0-C1 (delete stale Vulkan fork)

Lowest-risk, highest-leverage entry point — pure deletion, zero behavior change, but removes ~16k LOC of confusable code and shrinks zig fmt / zig build test surface immediately. Suggested PR:

1. git rm -r src/engine/graphics/vulkan/ (and src/engine/ if m2 sub-issue goes too)
2. nix develop --command zig fmt src/
3. nix develop --command zig build test (should be green — nothing imports the tree)
4. nix develop --command zig build -Doptimize=ReleaseFast for the smoke check
5. Conventional commit: refactor: delete stale src/engine/graphics/vulkan/ fork (#N)

This PR unblocks the whole P0 phase and gives reviewers a quick win to validate the sub-issue workflow before the riskier C2/C3 work begins.

---

Phasing notes (from the body)

• P0 ordering: C1 → C3 → C2. C3 establishes the compute facet that C2's drawSky/drawWater will share; doing C2 first would force re-designs.
• C4 (lod_mesh.zig split): 686 lines of embedded tests must move with their code (git mv, not copy-paste). nix develop --command zig build test -- --test-filter 'lod mesh' after the move.
• M1 (worldgen-overworld-v2/root.zig split): pin a golden-output test before splitting — the body flags this explicitly. Without a baseline the refactor is unverifiable.
• P2 M13 (game-core/game-ui migration onto role interfaces): cited files (session.zig:228,320,355, player.zig:277,386, world.zig:135,243) are concrete touchpoints — one focused PR per caller is feasible.
• Threading: any refactor near the job system must keep worker-thread RHI isolation (P2 of the strengths list) intact — no new ad-hoc locks, no RHI calls off the main thread. Worth calling out in the C2/C3 sub-issue descriptions explicitly.

Scope verdict

The umbrella is correctly scoped — already a tracker with checkbox sub-issues. No further split is needed at this level. Each checkbox should become its own sub-issue with a Parent: #776 reference, and this issue's checklist updated as sub-issues land.

Overlap check

No duplicates of the 32 findings among the existing per-bug audit issues (#705–#747). Those are correctness bugs; this umbrella is structural debt. Adjacent but distinct.

---

Suggested first batch of sub-issues (1 PR each, target dev):

1. #N+1 P0-C1 — delete stale Vulkan fork (no-risk decruft)
2. #N+2 P3-m1 — remove the two stale addImport lines in build.zig:265,281 (1-line PR)
3. #N+3 P0-C3 — add compute facet to RHI.VTable + rewrite gpu_mesher against it (needs headless screenshot baseline first per the "Key risks" section)
4. #N+4 P1-C4 — split lod_mesh.zig into 5 modules + dedicated test file (move tests with git mv)
5. #N+5 P2-M8+M9 — replace GENERATORS hardcoded list with comptime [*]const *const GeneratorDescriptor; delete the 4 trivially-delegating wrappers

Items 1 and 2 are trivial decruft that can land today; items 3–5 require the headless screenshot baselines the body recommends before touching rendering code.

[github-actions]

Comment posted on issue #776: #776 (comment)

Verified C1, C2, C3, C4, and m1 against the tree — all citations accurate. Recommended P0-C1 (delete stale Vulkan fork) as the safest first sub-issue, plus a 5-item first batch of sub-issues ordered by risk.

opencode session  |  github run