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

feat(simulation): foundation shared types + struct plumbing (#1006) #143

Closed
jpmschweitzer wants to merge 0 commits from foundation-types into main
pull from: foundation-types
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-05-30 19:16:58 +02:00
Owner
Copy Link
Copy Source

#1006 — Foundation: shared Rust types + struct plumbing

The type-definition layer every Phase-4 fill-seam ticket waits on (the most-repeated refinement gap). Compiles with stubs/defaults; behavior logic lands in #982–985/#998.

Types (D-229/D-230/D-231/D-232/D-233)

BuildingPropertyTag, FloorExtent + FloorHeightProfile (floor_at_voxel_z/voxel_range_for_floor, resolves Q-104), BuildingEntryClass, ConstructionEra, ZoneTypeId, MorphologyZone, BulkClass(5), ProductionUbiquity, DoorSpec, InteriorDescriptor, DistrictWorldState.

Plumbing

  • Rename spatial AccessTier → ZoneAccessTier (frees the name for per-building BuildingEntryClass; serde variant names unchanged → no snapshot break).
  • CityGenerationContext: +morphology_zone, +trait_selection, +dominant_bulk_class, +dominant_production_ubiquity.
  • BodyWorldState: +districts (DistrictWorldState w/ block_tags).
  • GenCompletion::SkeletonGenerated carries body_id + DistrictWorldState; plugin handler inserts into BodyWorldState.districts.
  • smallvec added as a direct dep (DoorSpec list stays Vec for now, TODO).

Review

Architecture (Tyre) + QA (Hoshe): both APPROVE. Follow-up commit cleared the clippy --all-targets warning and fixed floor_at_voxel_z doc direction + building-relative coordinate contract + a TODO(#957) on the city_id-as-DistrictId stub key. cargo fmt/clippy --all-targets clean; 1259 lib tests pass.

Deferred follow-ups (tracked for #957/#982–985): add PartialEq to new structs; unit tests for the Q-104 floor math; district_id keying when real skeleton gen lands.

🤖 Generated with Claude Code

## #1006 — Foundation: shared Rust types + struct plumbing The type-definition layer every Phase-4 fill-seam ticket waits on (the most-repeated refinement gap). Compiles with stubs/defaults; behavior logic lands in #982–985/#998. ### Types (D-229/D-230/D-231/D-232/D-233) `BuildingPropertyTag`, `FloorExtent` + `FloorHeightProfile` (`floor_at_voxel_z`/`voxel_range_for_floor`, **resolves Q-104**), `BuildingEntryClass`, `ConstructionEra`, `ZoneTypeId`, `MorphologyZone`, `BulkClass`(5), `ProductionUbiquity`, `DoorSpec`, `InteriorDescriptor`, `DistrictWorldState`. ### Plumbing - Rename spatial `AccessTier` → `ZoneAccessTier` (frees the name for per-building `BuildingEntryClass`; serde variant names unchanged → no snapshot break). - `CityGenerationContext`: +`morphology_zone`, +`trait_selection`, +`dominant_bulk_class`, +`dominant_production_ubiquity`. - `BodyWorldState`: +`districts` (`DistrictWorldState` w/ `block_tags`). - `GenCompletion::SkeletonGenerated` carries `body_id` + `DistrictWorldState`; plugin handler inserts into `BodyWorldState.districts`. - `smallvec` added as a direct dep (DoorSpec list stays `Vec` for now, TODO). ### Review Architecture (Tyre) + QA (Hoshe): **both APPROVE**. Follow-up commit cleared the clippy `--all-targets` warning and fixed `floor_at_voxel_z` doc direction + building-relative coordinate contract + a `TODO(#957)` on the city_id-as-DistrictId stub key. `cargo fmt`/`clippy --all-targets` clean; 1259 lib tests pass. Deferred follow-ups (tracked for #957/#982–985): add `PartialEq` to new structs; unit tests for the Q-104 floor math; `district_id` keying when real skeleton gen lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 2 commits 2026-05-30 19:16:58 +02:00
feat(simulation): foundation shared types + struct plumbing (#1006) 46b8688fa3 
Define the type-definition layer the Phase-4 fill-seam tickets depend on
(D-229/D-230/D-231/D-232/D-233), compiling with stubs/defaults; behavior
logic lands in #982-985/#998.

- New types in generator.rs: BuildingPropertyTag, FloorExtent +
  FloorHeightProfile (floor_at_voxel_z/voxel_range_for_floor, resolves
  Q-104), BuildingEntryClass, ConstructionEra, ZoneTypeId, MorphologyZone,
  BulkClass(5), ProductionUbiquity, DoorSpec, InteriorDescriptor,
  DistrictWorldState.
- Rename spatial AccessTier -> ZoneAccessTier to free the name for the new
  per-building BuildingEntryClass.
- CityGenerationContext: +morphology_zone, +trait_selection,
  +dominant_bulk_class, +dominant_production_ubiquity.
- BodyWorldState: +districts (DistrictWorldState w/ block_tags).
- GenCompletion::SkeletonGenerated carries body_id + DistrictWorldState;
  plugin handler inserts into BodyWorldState.districts.
- Add smallvec as a direct dep (DoorSpec list stays Vec for now, TODO).

cargo check --all-targets / clippy clean; 1259 lib tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(simulation): review follow-ups on foundation types (#1006) 899f447eed 
- Derive MorphologyZone Default (#[default] AlluvialPlain), drop manual
  impl — clears the clippy --all-targets warning (pre-push gate).
- Fix floor_at_voxel_z doc (described the inverse function).
- Document floor_at_voxel_z / voxel_range_for_floor voxel-z as
  building-relative (base_floor = z:0) for #982-985 callers.
- TODO(#957): districts keyed by city_id is a stub; use district_id.

clippy --all-targets clean; 1259 lib tests pass.

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

Review: foundation-types → main (code) — CHANGES REQUESTED

Reviewers: Hoshe (code quality), Tyre (architecture). Both REQUEST_CHANGES.

Build is healthy — cargo check --all-targets clean, cargo clippy --all-targets -- -D warnings clean, 1259/1259 lib tests pass, Q-104 boundary math verified correct by hand (basement + top-floor cases). The all_fixtures_deserialize failure is pre-existing on main, not this branch. Types are faithful to D-229/230/231/232/233; AccessTier → ZoneAccessTier rename is complete and serde-safe; determinism (D-010) holds.

Five items to address before merge:

# File Issue By
1 server/src/simulation/generator.rs:~901 DistrictWorldState missing the skeleton: DistrictSkeleton field that D-230 specifies. Without it, #982–985 have no cache path to the district seed and will have to add it in a follow-up that breaks the established schema. Add now — pub skeleton: DistrictSkeleton with a Default stub (mirrors the block_tags: BTreeMap::new() stub pattern). Tyre
2 server/src/simulation/generator.rs:538/565/594 FloorExtent / FloorHeightProfile missing PartialEq. These own the Q-104 logic this PR ships; #982–985 will write round-trip/boundary assert_eq! tests against them and won't compile. Add PartialEq here (and to DistrictWorldState / CityGenerationContext in the same pass). Hoshe
3 server/src/simulation/generator.rs:565–607 No unit tests for floor_at_voxel_z / voxel_range_for_floor — the declared Q-104 deliverable. Add: Uniform at ground level (round-trip), basement (base_floor=-1), Variable([5,3,3]) ranges, and boundary (one voxel above top floor → None). Tyre
4 server/Cargo.toml:+37 smallvec added but entirely unused — BuildingPropertyTag.doors stays Vec<DoorSpec> with a TODO. Remove the dep until #957 actually switches the type (or wire SmallVec<[DoorSpec;4]> now per D-231). An unused dep is a misleading signal. Hoshe + Tyre
5 server/src/atlas/plugin.rs:93 Once the skeleton field (#1) lands, retarget the TODO(#957) comment to key districts off state.skeleton.district_id rather than city_id (a city has many districts per D-194). Tyre

No issues with: the rename, trait_selection naming (D-232), BulkClass 5-variant set (D-233), BTreeMap determinism, FloorExtent.above_ground() logic.

🤖 review via /pr-review

## Review: foundation-types → main (code) — **CHANGES REQUESTED** Reviewers: **Hoshe** (code quality), **Tyre** (architecture). Both REQUEST_CHANGES. Build is healthy — `cargo check --all-targets` clean, `cargo clippy --all-targets -- -D warnings` clean, 1259/1259 lib tests pass, Q-104 boundary math verified correct by hand (basement + top-floor cases). The `all_fixtures_deserialize` failure is pre-existing on main, not this branch. Types are faithful to D-229/230/231/232/233; `AccessTier → ZoneAccessTier` rename is complete and serde-safe; determinism (D-010) holds. Five items to address before merge: | # | File | Issue | By | |---|------|-------|----| | 1 | `server/src/simulation/generator.rs:~901` | **`DistrictWorldState` missing the `skeleton: DistrictSkeleton` field** that D-230 specifies. Without it, #982–985 have no cache path to the district seed and will have to add it in a follow-up that breaks the established schema. Add now — `pub skeleton: DistrictSkeleton` with a `Default` stub (mirrors the `block_tags: BTreeMap::new()` stub pattern). | Tyre | | 2 | `server/src/simulation/generator.rs:538/565/594` | **`FloorExtent` / `FloorHeightProfile` missing `PartialEq`.** These own the Q-104 logic this PR ships; #982–985 will write round-trip/boundary `assert_eq!` tests against them and won't compile. Add `PartialEq` here (and to `DistrictWorldState` / `CityGenerationContext` in the same pass). | Hoshe | | 3 | `server/src/simulation/generator.rs:565–607` | **No unit tests for `floor_at_voxel_z` / `voxel_range_for_floor`** — the declared Q-104 deliverable. Add: Uniform at ground level (round-trip), basement (`base_floor=-1`), `Variable([5,3,3])` ranges, and boundary (one voxel above top floor → `None`). | Tyre | | 4 | `server/Cargo.toml:+37` | **`smallvec` added but entirely unused** — `BuildingPropertyTag.doors` stays `Vec<DoorSpec>` with a TODO. Remove the dep until #957 actually switches the type (or wire `SmallVec<[DoorSpec;4]>` now per D-231). An unused dep is a misleading signal. | Hoshe + Tyre | | 5 | `server/src/atlas/plugin.rs:93` | Once the `skeleton` field (#1) lands, retarget the `TODO(#957)` comment to key districts off `state.skeleton.district_id` rather than `city_id` (a city has many districts per D-194). | Tyre | No issues with: the rename, `trait_selection` naming (D-232), `BulkClass` 5-variant set (D-233), `BTreeMap` determinism, `FloorExtent.above_ground()` logic. 🤖 review via /pr-review
jpmschweitzer added 1 commit 2026-05-30 22:07:12 +02:00
fix(simulation): address PR #143 review (#1006) 99da796cde 
- Add D-230 skeleton: DistrictSkeleton field to DistrictWorldState;
  cascade Default to DistrictSkeleton + contained enums/structs. Box the
  GenCompletion::SkeletonGenerated state to avoid large_enum_variant.
- Derive PartialEq on FloorExtent/FloorHeightProfile/DistrictWorldState/
  CityGenerationContext (+ minimal cascade) for downstream assert_eq tests.
- Add 4 unit tests for floor_at_voxel_z / voxel_range_for_floor (uniform,
  basement, variable heights, boundary) — the Q-104 deliverable.
- Drop unused smallvec direct dep (stays transitive via bevy_ecs).
- Key districts insert by skeleton.district_id, sharpen TODO(#957).

clippy --all-targets -D warnings clean; 1263 lib tests pass; fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer commented 2026-05-30 22:49:09 +02:00
Author
Owner
Copy Link
Copy Source

Re-review (commit 99da796cd) — APPROVED

Both reviewers re-verified their findings against the follow-up commit.

Hoshe (code quality): APPROVE — smallvec removed (only the TODO comment remains); PartialEq derived on FloorExtent/FloorHeightProfile (+ DistrictWorldState/CityGenerationContext/DistrictSkeleton/BlockSkeleton/SettingType/DistrictLayoutMode); 4 new FloorExtent tests present and correct (the variable-heights case exercises the non-linear accumulation path); cargo clippy --all-targets -- -D warnings clean; 1263/1263 lib tests pass.

Tyre (architecture): APPROVE — DistrictWorldState now carries skeleton: DistrictSkeleton per D-230 (full Default chain resolves); district insert keyed by state.skeleton.district_id (canonical D-194/D-230 key), so #957 lands with no schema break; Box<DistrictWorldState> on the SkeletonGenerated variant is the idiomatic large_enum_variant fix (negligible off-hot-path alloc, no determinism concern).

No remaining items. Clean to merge.

🤖 re-review via /pr-review

## Re-review (commit `99da796cd`) — **APPROVED** Both reviewers re-verified their findings against the follow-up commit. **Hoshe (code quality): APPROVE** — `smallvec` removed (only the TODO comment remains); `PartialEq` derived on `FloorExtent`/`FloorHeightProfile` (+ `DistrictWorldState`/`CityGenerationContext`/`DistrictSkeleton`/`BlockSkeleton`/`SettingType`/`DistrictLayoutMode`); 4 new `FloorExtent` tests present and correct (the variable-heights case exercises the non-linear accumulation path); `cargo clippy --all-targets -- -D warnings` clean; 1263/1263 lib tests pass. **Tyre (architecture): APPROVE** — `DistrictWorldState` now carries `skeleton: DistrictSkeleton` per D-230 (full `Default` chain resolves); district insert keyed by `state.skeleton.district_id` (canonical D-194/D-230 key), so #957 lands with no schema break; `Box<DistrictWorldState>` on the `SkeletonGenerated` variant is the idiomatic `large_enum_variant` fix (negligible off-hot-path alloc, no determinism concern). No remaining items. Clean to merge. 🤖 re-review via /pr-review
jpmschweitzer closed this pull request 2026-05-30 22:50:16 +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#143