docs(simulation): PR #218 round 3 — the round-2 fix outran its own documentation
Three findings, all doc-accuracy, and all the same root cause: folding the spacing predicate into the ring walk changed what three comments describe, and two of those comments were written by this same PR one round earlier. TYRE 1 — road_graph.rs's T-1206 gap-closure comment cited `nearest_land_cell`, which round 2 made `#[cfg(test)]`. A reader chasing that name lands on a test-only function and reasonably wonders whether they are looking at dead code. Repointed to `nearest_cell_matching`, and the paragraph's closing claim that "T-1206 guarantees the placement pixel is land" is corrected: it has been land-AND-spacing-or-skip since round 2. TYRE 2 — `max_land_search_ring`'s doc named the same test-only wrapper as the thing that walks the bound. It now names the production consumer and both callers. TYRE 3 — the D-211 amendment was written in round 1, before round 2 existed, and still described a land-only correction. It now carries a dated refinement recording what the code actually does: the walk satisfies BOTH of step 4's promises in one search, and SKIP therefore also fires where land exists but none of it clears spacing within the bound. The no-re-decision conclusion is unaffected — position remains a deterministic, non-fabricated function of seed and terrain — and the refinement notes the spacing promise is step 4's alone, since Tier A/B/C placements sit on their matched attractor and were never subject to it. HOSHE's three findings were the same three hunks, observed uncommitted while the review ran: accurate content, but not in the branch tip, so the PR would have merged a governance record that misdescribes its own commit. That is this commit. Both reviewers independently confirmed what the round-2 fix claims. Tyre traced the ring geometry and tie-break order by hand against the spacing predicate; Hoshe re-ran the full 267-body corpus scan live (850s) and reproduced the figures exactly — 267 bodies, 267 reaching Layer 3, 344 placements, 109 synthetic, 0 in water, 0 spacing violations. The shared-ring-search-helper retraction is confirmed and settled, with NEW grounds rather than a restatement: round 2 strengthened the case for keeping them separate, since this walk is now parameterized by an arbitrary predicate over native u16 terrain coordinates while road_graph's is a RouteGrid method over downsampled routing cells with a fixed cost test and an unrelated bound. 22 module tests green; clippy and fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1118,7 +1118,9 @@ Technical foundation decisions that constrain implementation: engine, client-ser
|
||||
- **Two-tier mismatch flagging:** If a matched city-attractor pair has score < 0.35, log a `WARNING` (below expected quality). If score < 0.15, log an `ERROR` and flag for manual review. Generation proceeds in both cases; the flags are for content auditing, not hard blockers.
|
||||
- **Output:** `Vec<CityPlacement { city_id, position: [row, col], attractor: AttractorType, score: f32 }>` written to `atlas_city_positions` at build time.
|
||||
- **Rationale:** Greedy-first for large/locked cities ensures anchor cities (capitals, corp HQs, wiki-named cities) are placed at terrain features that match their lore role. Hungarian for medium cities finds the globally optimal assignment, not just locally optimal. Synthetic overflow prevents the algorithm from failing on bodies where city count exceeds natural attractor count (dense, flat worlds). The two-tier warning system enables content QA without blocking generation.
|
||||
- **Amendment (T-1206, 2026-08-06 — step 4 may now legitimately place nothing):** phase 4's synthetic position was derived by pure grid arithmetic with **no terrain input at all**, so it could and did land in open water — a scan of every heightmap body at world seed 42 found **46 of 109 synthetic placements sitting in ocean** (e.g. GJ903c at a genuine polar ocean cell). Synthetic overflow now takes the terrain analysis and land-corrects via a bounded nearest-land ring walk; land positions pass through untouched, so **no existing land placement moves** and this record's seed-derived-position promise is intact (position remains a pure function of seed + terrain — this fulfils the placement intent rather than deviating from it, so no re-decision is required). What **does** change is the record's outcome set: step 4's "cities that cannot be matched receive a synthetic `PlainCenter`" is no longer total — where no land exists within the search bound, the synthetic attractor is **skipped**, defined and deliberate, never a fabricated water position and never a panic. Step 5's name-fulfillment warning consequently fires for a new legitimate reason (a genuinely water-locked body), not only for a pipeline failure.
|
||||
- **Amendment (T-1206, 2026-08-06 — step 4 may now legitimately place nothing):** phase 4's synthetic position was derived by pure grid arithmetic with **no terrain input at all**, so it could and did land in open water — a scan of every heightmap body at world seed 42 found **46 of 109 synthetic placements sitting in ocean** (e.g. GJ903c at a genuine polar ocean cell). Synthetic overflow now takes the terrain analysis and corrects via a bounded ring walk; land positions pass through untouched, so **no existing land placement moves** and this record's seed-derived-position promise is intact (position remains a pure function of seed + terrain — this fulfils the placement intent rather than deviating from it, so no re-decision is required). What **does** change is the record's outcome set: step 4's "cities that cannot be matched receive a synthetic `PlainCenter`" is no longer total — where the search finds nothing acceptable within its bound, the synthetic attractor is **skipped**, defined and deliberate, never a fabricated water position and never a panic. Step 5's name-fulfillment warning consequently fires for a new legitimate reason, not only for a pipeline failure.
|
||||
|
||||
**Amendment refinement (PR #218 round 2, 2026-08-07 — the walk satisfies BOTH of step 4's promises).** The paragraph above was written when the correction searched for land alone, which quietly spent this step's *other* guarantee: step 4 promises a position that "respects minimum city spacing", and the spacing walk ran *before* the correction, so a candidate cleared against its neighbours could be nudged up to half a grid away onto one of them. The spacing test is now folded **into the same ring walk**, so a synthetic candidate must be both land **and** at least `MIN_SPACING` from every already-placed city, resolved in one search with the same deterministic tie-break. Consequently **SKIP also fires where land exists but none of it clears spacing within the bound** — not only on a genuinely water-locked body. The no-re-decision conclusion is unaffected: position remains a deterministic, non-fabricated function of seed and terrain. Note the spacing promise is step 4's alone — Tier A greedy and Tier B/C Hungarian placements sit on their matched terrain attractor and were never subject to it.
|
||||
- **Ticket:** T-919, T-925, T-1206 (ocean-mask guard amendment)
|
||||
- **Raised by:** Generation cascade workshop (T-897)
|
||||
- **Cross-reference:** D-195 (CompatibilityMatrix), D-207 (atlas_city_names), D-209 (GeographicAttractor — input), [D-210](#d-210) (terrain_modification_cost — input; its 2026-07-26 amendment records the *matched-attractor* half of this same placement-in-water gap, closed and validated under T-1116/T-1206)
|
||||
|
||||
Reference in New Issue
Block a user