fix: preserve biome priority boundaries

[MichaelFisher1997]

Summary

• Keep multi-parameter ocean selection tied to low continentalness and normalized sea level so ocean biomes do not leak above sea level.
• Add explicit height limits to beach and shore biome definitions so coast selection remains narrow and slope/height aware.
• Add threshold tests for ocean/coast/inland-water and river/frozen-river boundaries.

Verification

• nix develop --command zig fmt modules/world-worldgen/src/biome_selector.zig modules/world-worldgen/src/biome_registry.zig modules/world-worldgen/src/biome_selector_tests.zig
• nix develop --command zig build test

Fixes #656

[github-actions]

📋 Summary

Linked Issue: Fixes #656 — Verified. The PR implements exactly what #656 requires:

1. Ocean biome selection is now gated by both low continentalness (< 0.35) and elevation at/below normalized sea level (<= 0.30), preventing ocean biomes from leaking above sea level.
2. Beach and shore biome definitions now have explicit max_height constraints (70 for beach/snowy_beach, 82 for stony_shore), keeping coast selection narrow and height-aware.
3. Three new threshold tests verify the boundary behavior for ocean/coast and river/frozen-river transitions.

Overall: Clean, focused bugfix PR with good test coverage. All tests pass and code is properly formatted.

📌 Review Metadata

• Reviewed Commit SHA: 1716b575d8b99886a895ca356e226d85073ab886
• Reviewed PR: fix: preserve biome priority boundaries #672
• Linked Issue: Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 — Fully addressed.

---

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

---

⚠️ High Priority Issues (Should Fix)

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

[MEDIUM] modules/world-worldgen/src/biome_selector.zig:15 - Hardcoded sea level constant duplicates normalization logic
Confidence: High
Description: NORMALIZED_SEA_LEVEL: f32 = 0.30 is hardcoded in biome_selector.zig but the same semantic value is computed in computeClimateParams (line 148-155). If the normalization formula in computeClimateParams changes, isOceanStructure will silently desync and ocean boundary checks will break.
Impact: Maintenance hazard; future changes to elevation normalization could reintroduce the ocean leak bug.
Suggested Fix: Either export the sea level constant from biome_registry.zig and reference it in both computeClimateParams and isOceanStructure, or add a doc comment on computeClimateParams explicitly stating that biome_selector.zig depends on the 0.30 sea level value.

[MEDIUM] modules/world-worldgen/src/biome_selector.zig:283-320 - LOD simplified selection lacks elevation-aware ocean gating
Confidence: Medium
Description: selectBiomeSimple still selects ocean biomes purely based on continental < 0.35 without any elevation check. For low-continentalness terrain above sea level (e.g., oceanic islands with raised terrain), LOD chunks may show ocean while the full-detail selectBiomeWithConstraints shows land, causing visual popping during LOD transitions.
Impact: Visual inconsistency at chunk LOD boundaries; ocean biomes may still appear above sea level at distance.
Suggested Fix: Add an elevation parameter to selectBiomeSimple or document this as an accepted LOD approximation. Alternatively, skip ocean selection in selectBiomeSimple when climate.elevation > NORMALIZED_SEA_LEVEL.

---

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/world-worldgen/src/biome_selector.zig:77,91,186,224 - Legacy selection functions lack ocean boundary protection
Confidence: Medium
Description: The pure climate functions (selectBiome, selectBiomeWithRiver, selectBiomeBlended, selectBiomeWithRiverBlended) still do not filter ocean biomes by elevation. While the production pipeline uses selectBiomeWithConstraintsAndRiver, these public functions remain exposed and could be called by other code paths or future features.
Impact: Potential for the same ocean leak bug to resurface if any caller uses the legacy functions.
Suggested Fix: Add the same isOceanBiome/isOceanStructure guard to selectBiomeMultiParam callers, or add doc comments warning that these functions do not enforce sea-level boundaries and should not be used for overworld generation.

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	7	Ocean boundary logic is cleanly isolated in isOceanStructure/isOceanBiome, though selectBiomeMultiParam now mixes selection scoring with structural filtering.
Open/Closed	6	The fix modifies selectBiomeMultiParam directly rather than injecting a filter; ocean constraints are hardcoded rather than configurable.
Liskov Substitution	8	No inheritance hierarchy; all selection functions operate on value types.
Interface Segregation	7	Many public functions, but they serve distinct use cases (Voronoi, blended, constrained, LOD).
Dependency Inversion	6	Hardcoded constants depend on implicit knowledge of computeClimateParams normalization.
Average	6.8

---

🎯 Final Assessment

Overall Confidence Score: 85%

• Code Quality: 85% (Clean, well-formatted, idiomatic Zig with good use of continue for filtering)
• Completeness: 90% (Fully addresses Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 for the primary generation path; LOD path unchanged by design)
• Risk Level: 20% (Low risk — pure logic change, no GPU/resource changes, no threading)
• Test Coverage: 85% (Three new boundary tests cover the exact bug; legacy paths untested)

Merge Readiness:

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

Verdict:

MERGE WITH FIXES

The PR is safe to merge as-is, but consider documenting or deduplicating the NORMALIZED_SEA_LEVEL constant relationship with computeClimateParams to prevent future maintenance bugs.

{
  "reviewed_sha": "1716b575d8b99886a895ca356e226d85073ab886",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 2,
  "overall_confidence_score": 85,
  "recommendation": "MERGE"
}

opencode session  |  github run

[github-actions]

📋 Summary

Linked Issue: Fixes #656 — Verified. The PR fully addresses #656 by:

1. Gating ocean biome selection in selectBiomeMultiParam with both low continentalness (< 0.35) and elevation at/below normalized sea level (<= 0.30), preventing ocean biomes from leaking above sea level.
2. Adding explicit max_height constraints to beach (70) and shore (82) biome definitions, keeping coast selection narrow and height-aware.
3. Adding four new threshold tests covering ocean/coast boundaries, inland-water boundaries, river/frozen-river thresholds, and LOD simple selection.

The PR is a clean, focused bugfix with good test coverage. All tests pass and code is properly formatted.

📌 Review Metadata

• Reviewed Commit SHA: 9026c9d63e4ea1d4f52e3254f364ca9aa72dd01a
• Reviewed PR: fix: preserve biome priority boundaries #672
• Linked Issue: Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 — Fully addressed.

---

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

---

⚠️ High Priority Issues (Should Fix)

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

None identified.

✅ [FIXED] Previous medium issue: Hardcoded sea level constant duplicates normalization logic

• The NORMALIZED_SEA_LEVEL constant is now exported from biome_registry.zig and referenced consistently in both computeClimateParams and isOceanStructure, eliminating the maintenance hazard.

✅ [FIXED] Previous medium issue: LOD simplified selection lacks elevation-aware ocean gating

• selectBiomeSimple now correctly gates ocean selection with climate.elevation <= NORMALIZED_SEA_LEVEL, preventing visual popping at LOD boundaries.

---

ℹ️ Low Priority Suggestions (Optional)

[LOW] modules/world-worldgen/src/biome_selector.zig:77,129,186,224 - Legacy selection functions lack ocean boundary protection
Confidence: Medium
Description: The pure climate functions (selectBiome, selectBiomeWithRiver, selectBiomeBlended, selectBiomeWithRiverBlended) still do not filter ocean biomes by elevation. While the production pipeline uses selectBiomeWithConstraintsAndRiver (which is now protected), these public functions remain exposed and could be called by other code paths or future features.
Impact: Potential for the same ocean leak bug to resurface if any caller uses the legacy functions.
Suggested Fix: Add doc comments warning that these functions do not enforce sea-level boundaries and should not be used for overworld generation.

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	Ocean boundary logic is cleanly isolated in isOceanStructure/isOceanBiome, and selectBiomeMultiParam delegates filtering to these helpers.
Open/Closed	7	Ocean constraints are now centralized via exported constants rather than hardcoded in multiple places.
Liskov Substitution	8	No inheritance hierarchy; all selection functions operate on value types.
Interface Segregation	7	Many public functions serve distinct use cases (Voronoi, blended, constrained, LOD).
Dependency Inversion	7	NORMALIZED_SEA_LEVEL is now explicitly exported from the registry, removing implicit coupling.
Average	7.4

---

🎯 Final Assessment

Overall Confidence Score: 90%

• Code Quality: 90% (Clean, well-formatted, idiomatic Zig with good helper function decomposition)
• Completeness: 95% (Fully addresses Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 for primary and LOD paths; legacy paths intentionally unchanged)
• Risk Level: 10% (Low risk — pure logic change, no GPU/resource changes, no threading)
• Test Coverage: 90% (Four new boundary tests cover the exact bug; all tests pass)

Merge Readiness:

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

Verdict:

MERGE

The PR is safe to merge. Both previously reported medium-priority issues have been resolved: the NORMALIZED_SEA_LEVEL constant is now properly exported and deduplicated, and selectBiomeSimple now correctly applies elevation-aware ocean gating. The remaining low-priority item about legacy functions is optional and does not block merge.

{
  "reviewed_sha": "9026c9d63e4ea1d4f52e3254f364ca9aa72dd01a",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 90,
  "recommendation": "MERGE"
}

opencode session  |  github run

[github-actions]

📋 Summary

Linked Issue: Fixes #656 — Verified. The PR fully implements the requirements:

1. Ocean biome selection in selectBiomeMultiParam is gated by both low continentalness (< 0.35) and elevation at/below normalized sea level (<= 0.30), preventing ocean biomes from appearing above sea level.
2. Beach and shore biome definitions now have explicit max_height constraints (70 for beach/snowy_beach, 82 for stony_shore), keeping coast selection narrow and height-aware.
3. Four new threshold tests verify boundary behavior for ocean/coast, inland-water, river/frozen-river, and LOD simple selection.

The PR is a clean, focused bugfix with good test coverage.

📌 Review Metadata

• Reviewed Commit SHA: c4ee93b7e9f491542e8b7044710158215788a903
• Reviewed PR: fix: preserve biome priority boundaries #672
• Linked Issue: Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 — Fully addressed.

---

🔴 Critical Issues (Must Fix - Blocks Merge)

None identified.

---

⚠️ High Priority Issues (Should Fix)

None identified.

---

💡 Medium Priority Issues (Nice to Fix)

None identified.

✅ [FIXED] Previous medium issue: Hardcoded sea level constant duplicates normalization logic

• The NORMALIZED_SEA_LEVEL constant is now exported from biome_registry.zig (line 15) and referenced consistently in both computeClimateParams and isOceanStructure, eliminating the maintenance hazard.

✅ [FIXED] Previous medium issue: LOD simplified selection lacks elevation-aware ocean gating

• selectBiomeSimple (line 319-320) now delegates to selectBiome which correctly applies isOceanClimate gating, preventing visual popping at LOD boundaries.

---

ℹ️ Low Priority Suggestions (Optional)

None identified.

✅ [FIXED] Previous low issue: Legacy selection functions lack ocean boundary protection

• All climate-based selection paths are now protected: selectBiome, selectBiomeBlended, selectBiomeWithRiver (via selectBiome), selectBiomeWithRiverBlended (via selectBiomeBlended), selectBiomeMultiParam, selectBiomeWithConstraints (via selectBiomeMultiParam), and selectBiomeWithConstraintsAndRiver (via selectBiomeWithConstraints).

---

📊 SOLID Principles Score

Principle	Score	Notes
Single Responsibility	8	Ocean boundary logic cleanly isolated in isOceanStructure/isOceanClimate/isOceanBiome helpers; selectBiomeMultiParam delegates filtering without mixing concerns.
Open/Closed	7	Ocean constraints centralized via exported NORMALIZED_SEA_LEVEL and OCEAN_CONTINENTALNESS_MAX constants rather than hardcoded in multiple places.
Liskov Substitution	8	No inheritance hierarchy; all selection functions operate on value types.
Interface Segregation	7	Many public functions serve distinct use cases (Voronoi, blended, constrained, LOD), each with appropriate parameter sets.
Dependency Inversion	7	NORMALIZED_SEA_LEVEL is explicitly exported from the registry, removing implicit coupling with computeClimateParams.
Average	7.4

---

🎯 Final Assessment

Overall Confidence Score: 92%

• Code Quality: 90% (Clean, well-formatted, idiomatic Zig with good helper decomposition and continue filtering pattern)
• Completeness: 95% (Fully addresses Worldgen Phase 2: preserve ocean, coast, and river priority rules #656 for all primary, LOD, and blended selection paths)
• Risk Level: 5% (Very low risk — pure logic change in biome selection, no GPU/resource changes, no threading)
• Test Coverage: 90% (Four new boundary tests cover the exact bug; existing baseline tests remain intact)

Merge Readiness:

• All critical issues resolved
• SOLID average score >= 6.0
• Overall confidence >= 60%
• No security concerns
• Tests present and passing (verified by prior CI runs)

Verdict:

MERGE

The PR is safe to merge. All previously reported issues have been resolved: the NORMALIZED_SEA_LEVEL constant is properly exported and deduplicated, selectBiomeSimple correctly applies elevation-aware ocean gating, and all climate-based selection functions are now protected. The implementation fully satisfies issue #656.

{
  "reviewed_sha": "c4ee93b7e9f491542e8b7044710158215788a903",
  "critical_issues": 0,
  "high_priority_issues": 0,
  "medium_priority_issues": 0,
  "overall_confidence_score": 92,
  "recommendation": "MERGE"
}

opencode session  |  github run