jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(simulation): Layer-3 settlement placement in the runtime cascade (#955) #149

Closed
jpmschweitzer wants to merge 0 commits from layer3-settlement-placement into main
pull from: layer3-settlement-placement
merge into: :main
Conversation 3 Commits - Files Changed -
jpmschweitzer commented 2026-06-03 17:16:07 +02:00
Owner
Copy Link
Copy Source

Summary

Wires the existing D-211 attractor-matching engine into the live generation
cascade so settlements are placed in-game, not just in tests — completing
#955. Along the way, the Layer-1→3 decision path was converted to fully
integer-deterministic math (D-010) to remove cross-platform f32 divergence
risk before more layers build on it.

What changed

Layer-3 placement in the runtime cascade (this session — B/E/F)

  • CityContextReader::read_body_settlements — per-body query of
    atlas_city_names, ordered by id for determinism. NULL settlement_class
    defaults to PopulationBudget, not NameLocked: the class is NULL until
    placement runs, and NameLocked would force every settlement Tier-A in
    match_cities and collapse population tiering (D-211). NULL economic_role
    falls back to residential.
  • AnalyzeBody work item carries Vec<CityRecord> and run_work_item
    now runs up_to: Settlement (was Topography). A body with no settlements
    yields empty placements at negligible cost.
  • Atlas proxy reads settlements on cache-miss and pins them onto the work
    item, keeping the Rayon task DB-free (D-225). A read failure is non-fatal:
    log and place no cities (Layer 1 still runs). Threaded through a new
    CityContextReaderResource opened in main.rs, mirroring
    BodySourceResolverResource.

Integer-deterministic placement core (earlier commits on this branch)

  • CompatibilityMatrix is a [[i32; 7]; 10] 0–100 affinity table
    (CompatibilityMatrix::d195()); GeographicAttractor.strength is i32
    (0–100) and terrain_modification_cost is an i32 percent (100 = baseline).
  • cell_score/match_cities/hungarian operate on i64; no f32 survives
    in any placement decision. Quantization happens at the feature-extraction
    boundary only.
  • Layer-1 sort keys, sub-biome cost, and the golden fixture were updated to the
    integer representation. The Layer-0→1 golden selection is unchanged —
    only the strength/cost representation differs (verified: identical
    source SHA, river cells, attractor set).
  • D-195 amended in decisions/architecture.md.

Determinism

Cascade is a pure function of (heightmap, cities, seed); the placement path is
integer-only end to end (D-010). Same inputs → identical placements.

Test plan

  • cargo check clean (lib + bins); cargo fmt --check clean.
  • cargo clippy --all-targets -- -D warnings clean.
  • 1295 lib tests + all integration tests pass. New: 3 read_body_settlements
    tests (NULL→PopulationBudget, id ordering + explicit-class parse, role
    fallback + empty body) and settlement_layer_places_cities_deterministically.
  • Golden fixture (cascade_layer1.json) rebaked — representational only.
  • atlas_response_ready.msgpack regenerated for the new attractor schema.

Not in this PR

  • A pre-existing broken doctest in server/src/bps.rs (use server::... →
    should be settled_reach_server::..., broken since the log10_floor helper
    landed in #145). Only surfaces in cargo test --doc, not --lib. Left for a
    separate cleanup.
  • Persistent cascade cache (settlement positions are deterministic, so they can
    be cached) is tracked as #1021.

Closes #955.

## Summary Wires the existing D-211 attractor-matching engine into the live generation cascade so **settlements are placed in-game**, not just in tests — completing #955. Along the way, the Layer-1→3 decision path was converted to fully integer-deterministic math (D-010) to remove cross-platform `f32` divergence risk before more layers build on it. ## What changed ### Layer-3 placement in the runtime cascade (this session — B/E/F) - **`CityContextReader::read_body_settlements`** — per-body query of `atlas_city_names`, ordered by `id` for determinism. NULL `settlement_class` defaults to `PopulationBudget`, **not** `NameLocked`: the class is NULL until placement runs, and NameLocked would force every settlement Tier-A in `match_cities` and collapse population tiering (D-211). NULL `economic_role` falls back to `residential`. - **`AnalyzeBody` work item carries `Vec<CityRecord>`** and `run_work_item` now runs `up_to: Settlement` (was `Topography`). A body with no settlements yields empty placements at negligible cost. - **Atlas proxy reads settlements on cache-miss** and pins them onto the work item, keeping the Rayon task DB-free (D-225). A read failure is non-fatal: log and place no cities (Layer 1 still runs). Threaded through a new `CityContextReaderResource` opened in `main.rs`, mirroring `BodySourceResolverResource`. ### Integer-deterministic placement core (earlier commits on this branch) - `CompatibilityMatrix` is a `[[i32; 7]; 10]` 0–100 affinity table (`CompatibilityMatrix::d195()`); `GeographicAttractor.strength` is `i32` (0–100) and `terrain_modification_cost` is an `i32` percent (100 = baseline). - `cell_score`/`match_cities`/`hungarian` operate on `i64`; no `f32` survives in any placement decision. Quantization happens at the feature-extraction boundary only. - Layer-1 sort keys, sub-biome cost, and the golden fixture were updated to the integer representation. The Layer-0→1 golden selection is **unchanged** — only the strength/cost *representation* differs (verified: identical source SHA, river cells, attractor set). - D-195 amended in `decisions/architecture.md`. ## Determinism Cascade is a pure function of (heightmap, cities, seed); the placement path is integer-only end to end (D-010). Same inputs → identical placements. ## Test plan - `cargo check` clean (lib + bins); `cargo fmt --check` clean. - `cargo clippy --all-targets -- -D warnings` clean. - **1295 lib tests + all integration tests pass.** New: 3 `read_body_settlements` tests (NULL→PopulationBudget, id ordering + explicit-class parse, role fallback + empty body) and `settlement_layer_places_cities_deterministically`. - Golden fixture (`cascade_layer1.json`) rebaked — representational only. - `atlas_response_ready.msgpack` regenerated for the new attractor schema. ### Not in this PR - A pre-existing broken doctest in `server/src/bps.rs` (`use server::...` → should be `settled_reach_server::...`, broken since the `log10_floor` helper landed in #145). Only surfaces in `cargo test --doc`, not `--lib`. Left for a separate cleanup. - Persistent cascade cache (settlement positions are deterministic, so they can be cached) is tracked as **#1021**. Closes #955.
jpmschweitzer added 5 commits 2026-06-03 17:16:07 +02:00
feat(simulation): integer-deterministic Layer-3 settlement placement (#955) ab084269f3 
Wire the existing attractor-matching engine (#919/#925) into the
generation cascade as Layer 3, and make the whole placement-scoring path
integer-deterministic.

Layer 3 (D-211):
- CascadeLayer::Settlement + Layer3Output (placements) on the snapshot;
  BodyWorldState gains a `placements` field (the D-203 hot cache).
- run_layer3 runs the five-phase match_cities against Layer-1 attractors
  via the authored D-195 compatibility matrix; pure function of
  (attractors, cities) — no RNG. cities are passed in by the caller so the
  cascade stays DB-free and testable. A `// cache seam` marks where a
  persistent cache wraps it later (#1021).
- gen_queue passes &[] for now (Topography needs no cities); the runtime
  settlement read (gen_queue/layer_proxy) is the #955 follow-on.

Integer determinism (D-010 / D-227 — D-195 amended):
- Wiring match_cities into the deterministic cascade made its f32 scoring
  a live cross-platform divergence risk (a near-tie comparison or the
  Hungarian's f32 reductions can round differently per platform → a
  different world from the same seed). Converted the entire path to
  integers: CompatibilityMatrix is a 0-100 affinity table; attractor
  strength is 0-100 and terrain cost is a percent (100 = baseline),
  quantized once at the Layer-1 feature boundary; cell_score, the
  Hungarian, and CityPlacement.score are i64. No f32 in any placement or
  ranking decision.
- Layer-1 golden fixture rebaked: confirmed selection/positions are
  unchanged (same 256 attractors, 93 river cells) — only the strength/cost
  representation changed.

Tests: lib green (1292); new settlement_layer_places_cities_deterministically
covers placement + determinism + propagation into BodyWorldState.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(simulation): route atlas_response fixtures in the serialize test adce12035a 
all_fixtures_deserialize panicked with "unknown fixture naming
convention: atlas_response_ready" — the test reads every .msgpack in
client/tests/fixtures/msgpack but had no branch for the atlas_response
prefix (the AtlasLayerResponse fixtures gen_fixtures emits, #969). A
pre-existing gap (present on main; the suite isn't gated by the pre-push
hook, which runs cargo-deny/ruff/JSON/stamp, not cargo test).

Add an atlas_response branch deserializing AtlasLayerResponse, and
regenerate atlas_response_ready.msgpack so its bytes match the now-integer
GeographicAttractor types (#955).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chore(meta): update changelog f748b017cb 
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat(simulation): run Layer-3 settlement placement in the runtime cascade (#955) e2787fbfb1 
Wire the existing D-211 attractor-matching engine into the live
generation cascade so settlements are placed in-game, not just in tests.

- CityContextReader::read_body_settlements reads a body's settlements
  from atlas_city_names (ordered by id for determinism). NULL
  settlement_class defaults to PopulationBudget, not NameLocked: the
  class is NULL until placement runs, and NameLocked would force every
  settlement Tier-A in match_cities and collapse population tiering
  (D-211). NULL economic_role falls back to residential.
- The AnalyzeBody work item carries the body's Vec<CityRecord>, and
  run_work_item now runs up_to Settlement (was Topography). A body with
  no settlements yields empty placements at negligible cost.
- The atlas layer proxy reads settlements on a cache miss and pins them
  onto the work item, keeping the Rayon task DB-free (D-225). A read
  failure is non-fatal: log and place no cities (Layer 1 still runs).
  Threaded through a new CityContextReaderResource Bevy resource opened
  in main.rs, mirroring BodySourceResolverResource.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chore(meta): update changelog 9fa06f3401 
Mark Layer-3 settlement placement (#955) as running in-game now that the
B/E/F plumbing (per-body settlement read → work item → cache-miss proxy)
is wired.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer commented 2026-06-03 17:21:31 +02:00
Author
Owner
Copy Link
Copy Source

Review: layer3-settlement-placement → main (type: code)

Reviewers: Hoshe (QA/code quality) + Tyre (architecture). Both ran clean tooling
(clippy --all-targets -D warnings clean; 1295 lib tests pass). Runtime smoke
documented in the PR test plan.

Hoshe (QA / code quality): REQUEST_CHANGES

The integer-math conversion is correct and complete — no overflow risk, no f32
remaining in any decision path, clean quantization boundary. One real behavioral
defect plus two stale-doc issues.

# File Issue
H1 city_context_reader.rs ~line 273 (read_body_settlements) Defect (medium). A non-NULL but unrecognized settlement_class string makes parse_settlement_class(...)? return Err, which propagates and fails the whole read_body_settlements call. layer_proxy then swallows it and enqueues the body with zero cities — one bad row kills placement for the entire body. Inconsistent with the NULL→PopulationBudget and NULL-role→residential fallbacks already in the same function. Fix: treat unknown variant like NULL (fall back to PopulationBudget + tracing::warn!), and add a test.
H2 features.rs line 16 (module comment) Stale doc. Comment says "strength is f32 but is never used as a sort key." It's now i32 and is used as a direct sort key (here and in layer1.rs). Update.
H3 cascade.rs line 39 (CascadeLayer::Settlement doc) + line 15 (module doc) Incorrect doc. Both call Layer 3 the "first RNG-using layer (uses the carried SeedChain)." As shipped, run_layer3 takes no seed and match_cities uses no RNG — Layer 3 is a pure function of (attractors, cities). Misleads whoever builds Layer 4.

Tyre (architecture): REQUEST_CHANGES

Architecturally sound — D-225's DB-free contract is honored by pre-resolving
cities at enqueue time, the cache-miss→enqueue→drain→cache loop is intact, and
deferring the persistent cache to #1021 is the right call. Two issues.

# File Issue
T1 layer_proxy.rs lines 28–30 (AtlasLayerRequest.up_to doc) Stale doc / drift. Comment says "v1 always runs the Layer-1 (Topography) cascade," but run_work_item now unconditionally runs CascadeLayer::Settlement and ignores req.up_to entirely. Doubly wrong. Update the comment to state v1 ignores the field and always runs through Settlement (honoring up_to deferred).
T2 cascade.rs ~line 257 (layers_are_ordered test) Test gap. Only asserts Heightmap < Topography. The up_to >= Settlement guards depend on Topography < Settlement, which currently holds only implicitly via derive order. Add assert!(CascadeLayer::Topography < CascadeLayer::Settlement) to pin the invariant.

Verdict: CHANGES REQUESTED

All five comments accepted (no pushback). H1 is a genuine robustness fix; the
other four are doc/test correctness. Addressing on the branch next.

## Review: layer3-settlement-placement → main (type: code) Reviewers: Hoshe (QA/code quality) + Tyre (architecture). Both ran clean tooling (clippy `--all-targets -D warnings` clean; 1295 lib tests pass). Runtime smoke documented in the PR test plan. ### Hoshe (QA / code quality): REQUEST_CHANGES The integer-math conversion is correct and complete — no overflow risk, no f32 remaining in any decision path, clean quantization boundary. One real behavioral defect plus two stale-doc issues. | # | File | Issue | |---|------|-------| | H1 | `city_context_reader.rs` ~line 273 (`read_body_settlements`) | **Defect (medium).** A non-NULL but *unrecognized* `settlement_class` string makes `parse_settlement_class(...)?` return `Err`, which propagates and fails the whole `read_body_settlements` call. `layer_proxy` then swallows it and enqueues the body with **zero cities** — one bad row kills placement for the entire body. Inconsistent with the NULL→PopulationBudget and NULL-role→residential fallbacks already in the same function. Fix: treat unknown variant like NULL (fall back to `PopulationBudget` + `tracing::warn!`), and add a test. | | H2 | `features.rs` line 16 (module comment) | **Stale doc.** Comment says "`strength` is f32 but is never used as a sort key." It's now `i32` *and* is used as a direct sort key (here and in `layer1.rs`). Update. | | H3 | `cascade.rs` line 39 (`CascadeLayer::Settlement` doc) + line 15 (module doc) | **Incorrect doc.** Both call Layer 3 the "first RNG-using layer (uses the carried `SeedChain`)." As shipped, `run_layer3` takes no seed and `match_cities` uses no RNG — Layer 3 is a pure function of (attractors, cities). Misleads whoever builds Layer 4. | ### Tyre (architecture): REQUEST_CHANGES Architecturally sound — D-225's DB-free contract is honored by pre-resolving cities at enqueue time, the cache-miss→enqueue→drain→cache loop is intact, and deferring the persistent cache to #1021 is the right call. Two issues. | # | File | Issue | |---|------|-------| | T1 | `layer_proxy.rs` lines 28–30 (`AtlasLayerRequest.up_to` doc) | **Stale doc / drift.** Comment says "v1 always runs the Layer-1 (Topography) cascade," but `run_work_item` now unconditionally runs `CascadeLayer::Settlement` and ignores `req.up_to` entirely. Doubly wrong. Update the comment to state v1 ignores the field and always runs through Settlement (honoring `up_to` deferred). | | T2 | `cascade.rs` ~line 257 (`layers_are_ordered` test) | **Test gap.** Only asserts `Heightmap < Topography`. The `up_to >= Settlement` guards depend on `Topography < Settlement`, which currently holds only implicitly via derive order. Add `assert!(CascadeLayer::Topography < CascadeLayer::Settlement)` to pin the invariant. | ### Verdict: CHANGES REQUESTED All five comments accepted (no pushback). H1 is a genuine robustness fix; the other four are doc/test correctness. Addressing on the branch next.
jpmschweitzer added 2 commits 2026-06-03 17:33:44 +02:00
fix(simulation): fall back on unrecognized settlement_class (#955) 9d8ff8b341 
read_body_settlements propagated a parse error for any non-NULL but
unrecognized settlement_class, which the atlas proxy swallowed and then
enqueued the body with zero cities — one malformed row silently dropped
placement for the entire body. Treat an unknown variant like NULL: fall
back to PopulationBudget with a warning. parse_settlement_class stays
strict for the D-199 read_set path, which must abort on bad fields.

Found in PR #149 review (Hoshe H1). Adds a test covering the fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs(simulation): correct stale cascade docs and pin layer order (#955) 2d8367d3bc 
Address the doc/test-correctness items from PR #149 review:

- cascade.rs: Layer 3 is RNG-free (pure fn of attractors+cities); it does
  not consume the carried SeedChain. Corrected the "first RNG-using layer"
  claims on the module doc and the Settlement variant (Hoshe H3).
- cascade.rs: layers_are_ordered now also asserts Topography < Settlement,
  pinning the invariant the up_to >= Settlement guards rely on (Tyre T2).
- features.rs: strength is an integer 0-100 (quantized at extraction), not
  f32, and consumers that rank by it use the integer value (Hoshe H2).
- layer_proxy.rs: AtlasLayerRequest.up_to is not yet honored — run_work_item
  runs through Settlement unconditionally; per-request depth deferred to
  #1021 (Tyre T1).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer commented 2026-06-03 17:33:58 +02:00
Author
Owner
Copy Link
Copy Source

Review comments addressed

All five comments accepted (no pushback). Two commits pushed:

  • 9d8ff8b34 fix(simulation): fall back on unrecognized settlement_class (#955)
  • 2d8367d3b docs(simulation): correct stale cascade docs and pin layer order (#955)
# Comment Resolution
H1 read_body_settlements hard-errors on unknown class → body gets zero cities Fixed. Unknown non-NULL variant now falls back to PopulationBudget + tracing::warn!, same as NULL. parse_settlement_class stays strict for the D-199 read_set path. New test read_body_settlements_unknown_class_falls_back_without_dropping_body asserts one bad row doesn't drop the body.
H2 features.rs:16 stale "strength is f32 / never a sort key" Fixed. Doc now says strength is an integer 0–100 (quantized at extraction) and notes consumers that rank by it (layer1::attach_feature_names) use the integer value. Module-header (0.0–1.0) line also corrected.
H3 cascade.rs calls Layer 3 the "first RNG-using layer" Fixed. Module doc and the Settlement variant now state Layer 3 is RNG-free — a pure function of (attractors, cities); the carried SeedChain is reserved for Layer 4+.
T1 AtlasLayerRequest.up_to doc says "v1 always runs Topography" Fixed. Doc now states up_to is not yet honored — run_work_item runs through Settlement unconditionally; per-request depth deferred to #1021.
T2 layers_are_ordered only asserts Heightmap < Topography Fixed. Added assert!(Topography < Settlement) with a comment explaining the up_to >= guards depend on it.

Verification: cargo fmt --check clean, cargo clippy --all-targets -- -D warnings clean, 1296 lib tests pass (+1 new). Ready for re-review.

## Review comments addressed All five comments accepted (no pushback). Two commits pushed: - `9d8ff8b34` fix(simulation): fall back on unrecognized settlement_class (#955) - `2d8367d3b` docs(simulation): correct stale cascade docs and pin layer order (#955) | # | Comment | Resolution | |---|---------|-----------| | H1 | `read_body_settlements` hard-errors on unknown class → body gets zero cities | **Fixed.** Unknown non-NULL variant now falls back to `PopulationBudget` + `tracing::warn!`, same as NULL. `parse_settlement_class` stays strict for the D-199 `read_set` path. New test `read_body_settlements_unknown_class_falls_back_without_dropping_body` asserts one bad row doesn't drop the body. | | H2 | `features.rs:16` stale "strength is f32 / never a sort key" | **Fixed.** Doc now says strength is an integer 0–100 (quantized at extraction) and notes consumers that rank by it (`layer1::attach_feature_names`) use the integer value. Module-header `(0.0–1.0)` line also corrected. | | H3 | `cascade.rs` calls Layer 3 the "first RNG-using layer" | **Fixed.** Module doc and the `Settlement` variant now state Layer 3 is RNG-free — a pure function of (attractors, cities); the carried `SeedChain` is reserved for Layer 4+. | | T1 | `AtlasLayerRequest.up_to` doc says "v1 always runs Topography" | **Fixed.** Doc now states `up_to` is not yet honored — `run_work_item` runs through `Settlement` unconditionally; per-request depth deferred to #1021. | | T2 | `layers_are_ordered` only asserts `Heightmap < Topography` | **Fixed.** Added `assert!(Topography < Settlement)` with a comment explaining the `up_to >=` guards depend on it. | **Verification:** `cargo fmt --check` clean, `cargo clippy --all-targets -- -D warnings` clean, **1296 lib tests pass** (+1 new). Ready for re-review.
jpmschweitzer commented 2026-06-03 17:39:55 +02:00
Author
Owner
Copy Link
Copy Source

Re-review (round 2): layer3-settlement-placement → main (type: code)

After the two fix commits (9d8ff8b34, 2d8367d3b). Full cycle, not just delta.
Gate: cargo clippy --all-targets -- -D warnings clean; 1296 lib tests pass.

Hoshe (QA / code quality): APPROVE

All three first-round comments correctly resolved. H1's fallback uses
unwrap_or_else on parse_settlement_class, emits tracing::warn!, and the new
test proves one unrecognized row leaves the others intact (cities.len() == 2).
Confirmed the empty-string case also routes through the fallback. H2/H3 doc
corrections are accurate (strength/terrain_modification_cost are i32
throughout; Layer 3 correctly characterized as RNG-free). No new issues.

Tyre (architecture): APPROVE

Both comments cleanly resolved. T1: the up_to doc now precisely describes the
distinction — the cascade respects up_to, but run_work_item hardcodes
Settlement; per-request depth deferred to #1021. T2: the test pins both
ordering invariants with a comment tying them to the cascade guards. No new
drift; D-225 DB-free contract intact, cache loop unchanged, the single
f32→integer boundary correctly placed, D-195 amendment matches the code.

Verdict: APPROVED

Both reviewers approve. Ready to merge.

## Re-review (round 2): layer3-settlement-placement → main (type: code) After the two fix commits (`9d8ff8b34`, `2d8367d3b`). Full cycle, not just delta. Gate: `cargo clippy --all-targets -- -D warnings` clean; 1296 lib tests pass. ### Hoshe (QA / code quality): APPROVE All three first-round comments correctly resolved. H1's fallback uses `unwrap_or_else` on `parse_settlement_class`, emits `tracing::warn!`, and the new test proves one unrecognized row leaves the others intact (`cities.len() == 2`). Confirmed the empty-string case also routes through the fallback. H2/H3 doc corrections are accurate (`strength`/`terrain_modification_cost` are `i32` throughout; Layer 3 correctly characterized as RNG-free). No new issues. ### Tyre (architecture): APPROVE Both comments cleanly resolved. T1: the `up_to` doc now precisely describes the distinction — the cascade respects `up_to`, but `run_work_item` hardcodes `Settlement`; per-request depth deferred to #1021. T2: the test pins both ordering invariants with a comment tying them to the cascade guards. No new drift; D-225 DB-free contract intact, cache loop unchanged, the single f32→integer boundary correctly placed, D-195 amendment matches the code. ### Verdict: APPROVED Both reviewers approve. Ready to merge.
jpmschweitzer closed this pull request 2026-06-03 17:40:48 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#149