[SOLID] [M17] Stop re-exporting concrete generators; collapse generator_interface shims (covers M18)

[MichaelFisher1997]

Part of #776 — SOLID architecture debt umbrella.
Phase: P3 · Finding: M17

Problem

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. Separately (M18), three near-identical 17-line generator_interface.zig re-export shims exist in world-worldgen, worldgen-overworld, and worldgen-api — pure indirection that invites drift.

Evidence

(Full evidence and file:line citations are in the umbrella #776.)

Fix

1. Remove the concrete re-exports from world-worldgen/root.zig:53-61. Move any direct construction behind test-only imports.
2. Delete the two non-api generator_interface.zig shims; have those modules @import("worldgen-api") directly.

Verification

• nix develop --command zig build test (includes shader validation)
• If rendering-touching: capture headless screenshot baseline before/after (-Dskip-present)
• If behavior-preserving refactor: golden-output test must stay green

Constraints

• Small reviewable PR targeting dev; conventional commit (refactor: / feat:)
• Preserve current behavior unless this issue explicitly changes it
• Keep worker-thread RHI isolation intact (no RHI calls off main thread)

Notes

Resolves M17 + M18. The registry (createGenerator) remains the only sanctioned construction path.

Tracking: #776

[github-actions]

Classification

Refactor (behavior-preserving SOLID cleanup). Combined M17 + M18 fix.

Validation of findings

Confirmed against dev:

• M17 concrete re-exports live in modules/world-worldgen/src/root.zig:53 (FlatWorldGenerator), :59 (OverworldGenerator), :60 (OverworldV2Generator), :61 (ShadowTestWorldGenerator).
• M17 reach-through consumer is src/worldgen_tests.zig:11,19 plus ~16 OverworldGenerator.init(...) call sites in the same file (e.g. :34, :36, :70, :153-156, :189, :231, :256, :276, :307).
• M18 shim files are byte-identical 17-line re-export tables:
  • modules/world-worldgen/src/generator_interface.zig
  • modules/worldgen-overworld/src/generator_interface.zig
  • The third ("worldgen-api" one in the issue body) is actually the canonical definition at modules/worldgen-api/ itself — that one stays.

No open PR appears to be working on this.

Implementation notes (sharpening the issue body)

M18 is mechanical. Delete the two shim files, then update the internal importers:

• modules/world-worldgen/src/registry.zig:3-4 (currently @import("generator_interface.zig") for Generator/GeneratorInfo) → @import("worldgen-api") directly. build.zig:284 already exposes worldgen-api as an import to world_worldgen, so no build-file change is needed.
• modules/worldgen-overworld/src/root.zig and any other internal @import("generator_interface.zig") inside worldgen-overworld/... need the same swap.

M17 is the subtle part. A naïve "route tests through createGeneratorById" doesn't work:

• The registry returns an opaque Generator interface (modules/worldgen-api/src/root.zig-side), but tests need concrete-only members — notably OverworldGenerator.generateFeatures(chunk) at src/worldgen_tests.zig:324 and the init(seed, allocator, decoration_provider) 3-arg signature. Those aren't on the registry's Generator vtable.

So the fix for the test side should be: have src/worldgen_tests.zig directly @import("worldgen-overworld") / @import("worldgen-overworld-v2") (build.zig already routes them) instead of going through world-worldgen.OverworldGenerator. That's exactly what the issue's own Step 1 ("Move any direct construction behind test-only imports") calls for — just confirming the import paths to use.

Risk check

• No rendering-thread impact: this is purely a module-surface refactor, no Generator.generate call sites move; worker-thread RHI isolation is unaffected.
• No public-API breakage for production code: searched all *.zig under modules/ and src/ — concrete Generator types are only referenced outside world-worldgen/... and its sub-modules by src/worldgen_tests.zig. Production consumers (world-runtime, game-core) only use interface types (Generator, ColumnInfo, FeatureFocus), which keep working through the surviving API module re-exports or via direct worldgen-api import.
• Golden-output tests (e.g. "WorldGen stable chunk fingerprints for known seed" at src/worldgen_tests.zig:228) keep passing — the underlying OverworldGenerator.init + .generate call is unchanged, only the import path moves.

Suggested split (or keep combined?)

The two findings are independent enough to land in one PR but distinct in reviewers: a cleaner commit history would be two commits in one PR — refactor(world-worldgen): collapse generator_interface shims (M18) then refactor(world-worldgen): drop concrete generator re-exports; rewire tests (M17). I'd recommend keeping it as a single small PR rather than splitting into two — total diff is well under 200 lines and they're tightly related.

Label note: automated-audit is present, treating as a trusted machine finding per issue context — this comment validates and sharpens the plan rather than re-debating it.

[github-actions]

Posted analysis as issue #800 comment: comment

Summary of findings:

• Classification: Refactor (behavior-preserving SOLID cleanup combining M17 + M18).
• Validation: Confirmed all four concrete re-export sites (root.zig:53,59,60,61) and the two byte-identical 17-line shim files. Tests at src/worldgen_tests.zig:11,19 are the only consumer reaching past the registry; production code uses only interface types.
• Key clarification: The registry's Generator interface lacks generateFeatures() and the 3-arg init(seed, allocator, decoration_provider) signature used in tests, so tests can't simply migrate to createGeneratorById. They should directly @import("worldgen-overworld") / @import("worldgen-overworld-v2") instead (already routed in build.zig).
• M18 is mechanical: just internal importers need to switch — production code (registry, world-runtime, game-core) already uses interface types, so no breaking change.
• Suggestion: Single PR with two conventional commits (refactor:) is well-scoped; no need to split.
• No duplicate PRs found working on this.

opencode session  |  github run