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

feat(simulation): Sprint 25 generator spike — NpcBlueprint + template assembly #87

Closed
jpmschweitzer wants to merge 0 commits from server into main
pull from: server
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-03-06 21:40:10 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 25 server deliverables: generator proof-of-life spike.

  • #611 — NpcBlueprint struct design: ZoneSpec, CultureProfile, NpcBlueprint structs with RON serde. Example RON files as schema contract for copy team. RON validator CLI (tooling/validate-ron) for copy team linting.
  • #612 — Template assembly generator (Phase 1): generator-spike binary producing deterministic NPC rosters from hardcoded zone/culture stubs. Supports rural (4 NPCs) and industrial (11 NPCs) zone types with Krenn culture. --from-files flag wired for Phase 2 integration with copy team's #609/#610.

Sprint proof (eyeball test)

  • Cross-type (rural vs industrial, seed 42): Different density, role distribution, and behaviors. Both feel Krenn.
  • Intra-type (rural seed 42 vs 43): Same zone shape, different people.
  • Zone taxonomy and culture profile both do visible work in output.

Phase 2 (not in this PR)

Blocked on copy team delivering #609 (zone identity spec RON) and #610 (Krenn culture profile RON). --from-files flag is wired and ready.

Test plan

  • 1152 tests pass, 0 failures
  • cargo run --bin generator_spike -- --zone rural --seed 42 produces 4 NPCs
  • cargo run --bin generator_spike -- --zone industrial --seed 42 produces 11 NPCs
  • cargo run --bin generator_spike -- --zone rural --seed 43 produces different NPCs (determinism + variance)
  • RON example files validate clean via tooling/validate-ron
## Summary Sprint 25 server deliverables: generator proof-of-life spike. - **#611 — NpcBlueprint struct design**: `ZoneSpec`, `CultureProfile`, `NpcBlueprint` structs with RON serde. Example RON files as schema contract for copy team. RON validator CLI (`tooling/validate-ron`) for copy team linting. - **#612 — Template assembly generator (Phase 1)**: `generator-spike` binary producing deterministic NPC rosters from hardcoded zone/culture stubs. Supports rural (4 NPCs) and industrial (11 NPCs) zone types with Krenn culture. `--from-files` flag wired for Phase 2 integration with copy team's #609/#610. ### Sprint proof (eyeball test) - **Cross-type** (rural vs industrial, seed 42): Different density, role distribution, and behaviors. Both feel Krenn. - **Intra-type** (rural seed 42 vs 43): Same zone shape, different people. - Zone taxonomy and culture profile both do visible work in output. ### Phase 2 (not in this PR) Blocked on copy team delivering #609 (zone identity spec RON) and #610 (Krenn culture profile RON). `--from-files` flag is wired and ready. ## Test plan - [x] 1152 tests pass, 0 failures - [x] `cargo run --bin generator_spike -- --zone rural --seed 42` produces 4 NPCs - [x] `cargo run --bin generator_spike -- --zone industrial --seed 42` produces 11 NPCs - [x] `cargo run --bin generator_spike -- --zone rural --seed 43` produces different NPCs (determinism + variance) - [x] RON example files validate clean via `tooling/validate-ron`
jpmschweitzer added 2 commits 2026-03-06 21:40:11 +01:00
feat(simulation): NpcBlueprint struct design, RON schema, and validator CLI (#611) fb3ebf4313 
Define ZoneSpec, CultureProfile, and NpcBlueprint structs with serde/RON
deserialization. Ship example RON files as schema contract for the copy
team (#609, #610). Add validate-ron CLI for copy team to lint their files
without compiling the server.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): generator spike binary — template assembly Phase 1 (#612) 0f83c64e8f 
Add generator-spike binary producing NPC rosters from hardcoded zone and
culture stubs. Deterministic via SimRng, supports rural and industrial
zone types with Krenn culture. Phase 2 (RON file loading) wired via
--from-files flag, awaiting copy team deliverables (#609, #610).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-03-06 21:44:13 +01:00
Author
Owner
Copy Link
Copy Source

Review: server → main (PR #87, type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Good structure overall, but two correctness bugs that will show in the proof output.

# File Severity Issue
1 generator_spike.rs:473 critical Name collision breaks relationships. 20 given names, up to 12 NPCs — ~45% chance of duplicates. Duplicate-named NPCs silently lose relationships, and target_name becomes ambiguous for any downstream consumer. Fix: generate names without replacement (shuffle + pop).
2 generator_spike.rs:650-653 warning No validation that population_density >= 1. Density 0 in malformed RON → subtly wrong output. Formula works by accident.
3 generator_spike.rs:361 warning gen_name panics on empty given_names list. Validator doesn't catch this.
4 generator_spike.rs:378 warning pick_role panics on empty roles list. Same gap.
5 generator_spike.rs:404-409 suggestion Cultural behavior gate checks given_names.is_empty() — unrelated to what it's guarding.
6 generator_spike.rs:466-510 suggestion One-directional relationships look like bugs in the output. Add a note.
7 blueprint.rs:81-83 suggestion min_npcs > max_npcs not validated — future panic.
8 validate_ron.rs suggestion Validator should warn on empty name/role pools (catches #3 and #4).
9 validate-ron script suggestion realpath fails before Rust binary can give a helpful error.

Tyre (Architecture): APPROVE (with warnings)

Clean spike. Properly isolated from DistrictSkeleton and ECS. RON is the right format. DayPhase collision avoided. D-010 compliant (no HashMap, all SimRng).

# File Severity Issue
1 generator_spike.rs:360-371 warning Name duplication — same issue Hoshe flagged. Birthday problem at industrial density.
2 generator_spike.rs:433 warning Filler word cap at 2 is correct but reads confusingly. Extract to named variable.
3 generator_spike.rs:298-313 suggestion traits_contradict duplicates logic from generate.rs — expected for spike, share later.
4 generator_spike.rs:544-558 suggestion trait_label reimplements Display — derive on the enum when integrating.
5 generator_spike.rs:491 suggestion Relationship type strings match existing enum variants — clean for now, convert later.

Verdict: CHANGES REQUESTED

Both reviewers flag the name collision bug — it will visibly corrupt the proof-of-life output (duplicate names, missing relationships). That needs fixing before this is meaningful as a spike test.

The validator gaps (empty pools → panic) are worth fixing too since the copy team will use it immediately.

## Review: server → main (PR #87, type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Good structure overall, but two correctness bugs that will show in the proof output. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `generator_spike.rs:473` | **critical** | Name collision breaks relationships. 20 given names, up to 12 NPCs — ~45% chance of duplicates. Duplicate-named NPCs silently lose relationships, and `target_name` becomes ambiguous for any downstream consumer. Fix: generate names without replacement (shuffle + pop). | | 2 | `generator_spike.rs:650-653` | **warning** | No validation that `population_density >= 1`. Density 0 in malformed RON → subtly wrong output. Formula works by accident. | | 3 | `generator_spike.rs:361` | **warning** | `gen_name` panics on empty `given_names` list. Validator doesn't catch this. | | 4 | `generator_spike.rs:378` | **warning** | `pick_role` panics on empty `roles` list. Same gap. | | 5 | `generator_spike.rs:404-409` | suggestion | Cultural behavior gate checks `given_names.is_empty()` — unrelated to what it's guarding. | | 6 | `generator_spike.rs:466-510` | suggestion | One-directional relationships look like bugs in the output. Add a note. | | 7 | `blueprint.rs:81-83` | suggestion | `min_npcs > max_npcs` not validated — future panic. | | 8 | `validate_ron.rs` | suggestion | Validator should warn on empty name/role pools (catches #3 and #4). | | 9 | `validate-ron` script | suggestion | `realpath` fails before Rust binary can give a helpful error. | ### Tyre (Architecture): APPROVE (with warnings) Clean spike. Properly isolated from DistrictSkeleton and ECS. RON is the right format. DayPhase collision avoided. D-010 compliant (no HashMap, all SimRng). | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `generator_spike.rs:360-371` | **warning** | Name duplication — same issue Hoshe flagged. Birthday problem at industrial density. | | 2 | `generator_spike.rs:433` | **warning** | Filler word cap at 2 is correct but reads confusingly. Extract to named variable. | | 3 | `generator_spike.rs:298-313` | suggestion | `traits_contradict` duplicates logic from `generate.rs` — expected for spike, share later. | | 4 | `generator_spike.rs:544-558` | suggestion | `trait_label` reimplements Display — derive on the enum when integrating. | | 5 | `generator_spike.rs:491` | suggestion | Relationship type strings match existing enum variants — clean for now, convert later. | ### Verdict: CHANGES REQUESTED Both reviewers flag the **name collision bug** — it will visibly corrupt the proof-of-life output (duplicate names, missing relationships). That needs fixing before this is meaningful as a spike test. The validator gaps (empty pools → panic) are worth fixing too since the copy team will use it immediately.
jpmschweitzer added 1 commit 2026-03-06 21:50:37 +01:00
fix(simulation): address PR #87 review — name collision, validation, and polish a54b9d24e0 
- Fix critical name collision: shuffle+pop for unique NPC names (#3)
- Validate population_density >= 1 in generator and validator (#4)
- Guard against empty given_names/roles with validator warnings (#5)
- Extract filler word cap to MAX_FILLER_WORDS constant (#6)
- Fix cultural behavior gate checking wrong field (#7)
- Document intentional one-directional relationships (#8)
- Validate min_npcs <= max_npcs in validator (#9)
- Fix validate-ron script realpath error handling (#10)
- Add TODO comments for spike-specific code duplication (#11, #12, #13)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer closed this pull request 2026-03-07 08:47:43 +01: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#87