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

feat(simulation): Sprint 22 — NPC pool, triangles, contamination #79

Closed
jpmschweitzer wants to merge 0 commits from server into main
pull from: server
merge into: :main
Conversation 4 Commits - Files Changed -
jpmschweitzer commented 2026-02-28 16:35:45 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 22 server-side delivery: wires authored Sova content into production and activates the contamination pacing layer.

  • #176 NPC pool generation — 23 authored Sova NPCs spawn with EntanglementTag (Flat/Intrigue) based on triangle membership (D-029)
  • #188 Triangle instantiation — 5 authored triangles (3 ActiveFork, 2 PassiveTension per D-087) with deterministic FNV-1a IDs
  • #254 Contamination activation — timer-based storyteller fires at tick 1800 (30 game-min), pressures active triangles, emits ContaminationEvent
  • #567 Modifications stub — Vec data model on chunks, round-trips through save/load (D-112, DLC prep)
  • #512 Zone Gate room — two-zone gauntlet room with ZoneCrossEventQueue detection system
  • #509 Fuzzy map tests — 50-seed randomized testing of procedural maps against 4 structural invariants
  • #565 z-level fix — closed as no-op (codebase already uses i32)

Test Results

  • 1,111 tests passed, 0 failed
  • Clippy clean (2 pre-existing cosmetic warnings)
  • All integration tests green: content_loading (10/10), contamination (1/1), fuzzy_map (5/5), golden_suite, zone_gate

Test Plan

  • cargo test — full suite green
  • cargo clippy — no new warnings
  • Acceptance tests for #176, #188, #254 verified by QA (Hoshe)
  • PR review
## Summary Sprint 22 server-side delivery: wires authored Sova content into production and activates the contamination pacing layer. - **#176 NPC pool generation** — 23 authored Sova NPCs spawn with EntanglementTag (Flat/Intrigue) based on triangle membership (D-029) - **#188 Triangle instantiation** — 5 authored triangles (3 ActiveFork, 2 PassiveTension per D-087) with deterministic FNV-1a IDs - **#254 Contamination activation** — timer-based storyteller fires at tick 1800 (30 game-min), pressures active triangles, emits ContaminationEvent - **#567 Modifications stub** — Vec<Modification> data model on chunks, round-trips through save/load (D-112, DLC prep) - **#512 Zone Gate room** — two-zone gauntlet room with ZoneCrossEventQueue detection system - **#509 Fuzzy map tests** — 50-seed randomized testing of procedural maps against 4 structural invariants - **#565 z-level fix** — closed as no-op (codebase already uses i32) ## Test Results - 1,111 tests passed, 0 failed - Clippy clean (2 pre-existing cosmetic warnings) - All integration tests green: content_loading (10/10), contamination (1/1), fuzzy_map (5/5), golden_suite, zone_gate ## Test Plan - [x] `cargo test` — full suite green - [x] `cargo clippy` — no new warnings - [x] Acceptance tests for #176, #188, #254 verified by QA (Hoshe) - [ ] PR review
jpmschweitzer added 6 commits 2026-02-28 16:35:45 +01:00
feat(simulation): add modifications data model stub (#567, D-112) 61eb40fcab 
DLC entry point for future player construction system. Adds
Modification struct, ModificationType enum, and Modifications
component. Wired into SaveStateV1 with #[serde(default)] for
forward-compatible save format.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add Zone Gate gauntlet room and zone crossing detection (#512) daae3dd6ab 
New test_world room with two zones (Terminal/Corridor) separated by
a door. Adds ZoneCrossEventQueue resource and detect_zone_crossings
system to fire events when the player crosses zone boundaries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): wire NPC pool generation and authored triangle instantiation (#176, #188) 42febf4059 
Production startup now spawns 23 Sova NPCs with EntanglementTag
(Flat/Intrigue) based on triangle_membership. Three-phase spawn:
entity creation, cross-reference resolution, and authored triangle
instantiation. Five triangles (3 ActiveFork, 2 PassiveTension per
D-087) with deterministic IDs via FNV-1a hashing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add contamination activation mechanic (#254) b126e1830d 
Timer-based storyteller system fires after 1800 ticks (30 game-min).
Sets ContaminationActive resource, applies tension delta to all
ActiveFork triangles, and emits ContaminationEvent for downstream
monologue/behavioral hooks. Q-017 fallback constants in place.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test(simulation): add fuzzy tests for procedural map generation (#509) 3cd3a998c0 
50-seed randomized testing against 4 structural invariants:
walkable connectivity (BFS), entity bounds, door adjacency,
and minimum tile count floor. Includes generator module for
test-scoped procedural map creation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog d867cf1e3b 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-28 20:08:18 +01:00
Author
Owner
Copy Link
Copy Source

Review: server → main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Well-structured sprint with solid test coverage. Three warnings.

# File Severity Issue
1 modification.rs:1,3 warning Doc cites D-111/D-112 but neither describes the modification data model — misleading spec reference
2 fuzzy_map.rs:32 warning MIN_WALKABLE_TILES = 200 not structurally guaranteed — a seed outside 0..50 could fail CI
3 storyteller/mod.rs:44-46 + save_state.rs warning ContaminationActive not persisted — save at tick 2000, load → contamination fires again (double tension delta)
4 contamination.rs:47 suggestion Test name says "after 1801 ticks" but activation is at tick 1800
5 generator.rs:497 suggestion BlockSkeleton has no Default impl — future test fixtures will be verbose
6 fuzzy_map.rs:277-313 suggestion Individual invariant tests duplicate combined test — 5x eval per seed
7 zone.rs suggestion No test for zone crossing through unzoned tile (Some(id) → None)

Tyre (Architecture): REQUEST_CHANGES

Correct D-110/D-112 compliance, clean BTreeMap discipline. Two critical issues.

# File Severity Issue
1 main.rs:139-149 critical StorytellerPlugin defined but never registered — contamination never fires in production. D-027 success criterion fails.
2 save_state.rs:70 critical ContaminationActive not in SaveStateV1 — save/load resets timer, double-fires contamination, corrupts triangle tension
3 simulation/mod.rs:129 warning detect_zone_crossings missing .before(compute_observer_snapshot) — one-tick lag in audio crossfade at zone boundaries
4 generator.rs:347 warning z_bands_connected: Vec<u8> — needs doc comment clarifying band indices vs z-level coordinates (D-110 compliance)
5 contamination.rs:47 suggestion Test name mismatches behavior (1801 vs 1800)
6 fuzzy_map.rs:32 suggestion Comment arithmetic wrong ("5-10 tiles" but means per side)
7 storyteller/mod.rs:45 suggestion ContaminationActive missing Serialize, Deserialize derives

Verdict: CHANGES REQUESTED

Critical (must fix):

  1. Register StorytellerPlugin in main.rs — without it, contamination never fires in production
  2. Persist ContaminationActive in SaveStateV1 — add field with #[serde(default)] to prevent double-fire on load

Warnings:
3. Add .before(compute_observer_snapshot) to detect_zone_crossings system ordering
4. Doc comment on z_bands_connected clarifying band indices vs z-level coordinates
5. MIN_WALKABLE_TILES floor not structurally guaranteed
6. Misleading D-111/D-112 citation on modification module

## Review: server → main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Well-structured sprint with solid test coverage. Three warnings. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `modification.rs:1,3` | warning | Doc cites D-111/D-112 but neither describes the modification data model — misleading spec reference | | 2 | `fuzzy_map.rs:32` | warning | `MIN_WALKABLE_TILES = 200` not structurally guaranteed — a seed outside 0..50 could fail CI | | 3 | `storyteller/mod.rs:44-46` + `save_state.rs` | warning | `ContaminationActive` not persisted — save at tick 2000, load → contamination fires again (double tension delta) | | 4 | `contamination.rs:47` | suggestion | Test name says "after 1801 ticks" but activation is at tick 1800 | | 5 | `generator.rs:497` | suggestion | `BlockSkeleton` has no `Default` impl — future test fixtures will be verbose | | 6 | `fuzzy_map.rs:277-313` | suggestion | Individual invariant tests duplicate combined test — 5x eval per seed | | 7 | `zone.rs` | suggestion | No test for zone crossing through unzoned tile (`Some(id) → None`) | ### Tyre (Architecture): REQUEST_CHANGES Correct D-110/D-112 compliance, clean BTreeMap discipline. Two critical issues. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `main.rs:139-149` | **critical** | `StorytellerPlugin` defined but never registered — contamination never fires in production. D-027 success criterion fails. | | 2 | `save_state.rs:70` | **critical** | `ContaminationActive` not in `SaveStateV1` — save/load resets timer, double-fires contamination, corrupts triangle tension | | 3 | `simulation/mod.rs:129` | warning | `detect_zone_crossings` missing `.before(compute_observer_snapshot)` — one-tick lag in audio crossfade at zone boundaries | | 4 | `generator.rs:347` | warning | `z_bands_connected: Vec<u8>` — needs doc comment clarifying band indices vs z-level coordinates (D-110 compliance) | | 5 | `contamination.rs:47` | suggestion | Test name mismatches behavior (1801 vs 1800) | | 6 | `fuzzy_map.rs:32` | suggestion | Comment arithmetic wrong ("5-10 tiles" but means per side) | | 7 | `storyteller/mod.rs:45` | suggestion | `ContaminationActive` missing `Serialize, Deserialize` derives | ### Verdict: CHANGES REQUESTED **Critical (must fix):** 1. **Register `StorytellerPlugin` in `main.rs`** — without it, contamination never fires in production 2. **Persist `ContaminationActive` in `SaveStateV1`** — add field with `#[serde(default)]` to prevent double-fire on load **Warnings:** 3. Add `.before(compute_observer_snapshot)` to `detect_zone_crossings` system ordering 4. Doc comment on `z_bands_connected` clarifying band indices vs z-level coordinates 5. `MIN_WALKABLE_TILES` floor not structurally guaranteed 6. Misleading D-111/D-112 citation on modification module
jpmschweitzer commented 2026-02-28 23:06:01 +01:00
Author
Owner
Copy Link
Copy Source

Re-review: server → main (round 2)

No new commits since previous review. Original verdict stands: CHANGES REQUESTED.

Unresolved critical issues:

  1. StorytellerPlugin not registered in main.rs — contamination never fires in production. D-027 success criterion fails.
  2. ContaminationActive not in SaveStateV1 — save/load resets timer, double-fires contamination, corrupts triangle tension.

Unresolved warnings:

  1. detect_zone_crossings missing .before(compute_observer_snapshot) — one-tick lag at zone boundaries
  2. z_bands_connected: Vec<u8> needs doc comment clarifying band indices vs z-level coordinates (D-110)

Server team: please push fix commits addressing at minimum the two critical issues before requesting re-review.

## Re-review: server → main (round 2) No new commits since previous review. **Original verdict stands: CHANGES REQUESTED.** ### Unresolved critical issues: 1. **`StorytellerPlugin` not registered in `main.rs`** — contamination never fires in production. D-027 success criterion fails. 2. **`ContaminationActive` not in `SaveStateV1`** — save/load resets timer, double-fires contamination, corrupts triangle tension. ### Unresolved warnings: 3. `detect_zone_crossings` missing `.before(compute_observer_snapshot)` — one-tick lag at zone boundaries 4. `z_bands_connected: Vec<u8>` needs doc comment clarifying band indices vs z-level coordinates (D-110) Server team: please push fix commits addressing at minimum the two critical issues before requesting re-review.
jpmschweitzer added 3 commits 2026-02-28 23:25:14 +01:00
fix(simulation): correct CONTAMINATION_DELAY_TICKS from 1800 to 300 70de959ac3 
The constant was supposed to represent 30 game-minutes but the formula
was wrong (30 × 10 tps × 60s = 1800). Correct derivation: 30 minutes ×
TICKS_PER_GAME_MINUTE (10) = 300. Now uses the canonical constant
directly. Also fixes stale assertion message in integration test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(simulation): fix triangle state save/load and persist contamination 339f112c8f 
Three save/load bugs fixed:
- ContaminationActive not persisted in SaveStateV1 — caused double-fire
  of contamination pressure on reload after tick 300.
- Loaded triangle entities missing ActiveSim marker — made them
  invisible to escalation and contamination systems after any load.
- Existing triangle entities not despawned before load — created
  duplicates, doubling tension escalation per tick.

Also: HashSet → BTreeSet for D-010 compliance, defensive event queue
reset on load, and three regression tests for triangle roundtrip.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(simulation): replace f32 fields in generator with integer types d3217c5ae3 
BlockPlacement.street_width_factor (f32) → street_width_bps (u16,
basis points where 10000 = 1.0×). BlockSkeleton.density (f32) →
density_pct (u8, 0–100 percentage). Eliminates latent f32
non-determinism per D-010 principle 1.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-28 23:32:36 +01:00
Author
Owner
Copy Link
Copy Source

Review: server → main (type: code, round 3)

Hoshe (Code Quality): REQUEST_CHANGES

ContaminationActive save/load fixed, CONTAMINATION_DELAY_TICKS corrected. But the #1 critical from round 2 remains unresolved.

# File Severity Issue
1 main.rs:140-149 critical StorytellerPlugin STILL not registered in app builder. Contamination never fires in production. Same issue as round 2 — unfixed.
2 save_io.rs:148 warning modifications hardcoded to vec\![] in save_to_file — no TODO comment for when DLC populates it.
3 generator.rs:349 warning z_bands_connected: Vec<u8> still has no doc comment clarifying band indices vs absolute z-levels.

Tyre (Architecture): REQUEST_CHANGES

Same blocking issue — StorytellerPlugin absent from production app.

# File Severity Issue
1 main.rs:140-149,321-325 critical StorytellerPlugin not in app builder or dump_schedule_graph(). Tests pass because they wire systems manually — production never runs them.
2 save_io.rs:397-404 warning minimal_world() test helper doesn't init ContaminationActive — tests rely on get_resource fallback.
3 generator.rs:349 suggestion z_bands_connected doc comment clarifying band index vs z-level.
4 fuzzy_map.rs suggestion Uses ChaCha20Rng directly instead of SimRng — inconsistent with D-010.

Round 2 issue status:

Issue Status
CRITICAL: StorytellerPlugin not registered NOT FIXED (flagged in round 2 AND round 3)
CRITICAL: ContaminationActive not in SaveStateV1 Fixed
WARNING: detect_zone_crossings ordering Fixed
WARNING: z_bands_connected doc comment Not addressed

Verdict: CHANGES REQUESTED

The StorytellerPlugin registration has been flagged as critical in two consecutive rounds. One-line fix: app.add_plugins(StorytellerPlugin) in main().

## Review: server → main (type: code, round 3) ### Hoshe (Code Quality): REQUEST_CHANGES ContaminationActive save/load fixed, CONTAMINATION_DELAY_TICKS corrected. But the #1 critical from round 2 remains unresolved. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | main.rs:140-149 | **critical** | `StorytellerPlugin` STILL not registered in app builder. Contamination never fires in production. Same issue as round 2 — unfixed. | | 2 | save_io.rs:148 | warning | `modifications` hardcoded to `vec\![]` in `save_to_file` — no TODO comment for when DLC populates it. | | 3 | generator.rs:349 | warning | `z_bands_connected: Vec<u8>` still has no doc comment clarifying band indices vs absolute z-levels. | ### Tyre (Architecture): REQUEST_CHANGES Same blocking issue — StorytellerPlugin absent from production app. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | main.rs:140-149,321-325 | **critical** | `StorytellerPlugin` not in app builder or `dump_schedule_graph()`. Tests pass because they wire systems manually — production never runs them. | | 2 | save_io.rs:397-404 | warning | `minimal_world()` test helper doesn't init `ContaminationActive` — tests rely on `get_resource` fallback. | | 3 | generator.rs:349 | suggestion | `z_bands_connected` doc comment clarifying band index vs z-level. | | 4 | fuzzy_map.rs | suggestion | Uses `ChaCha20Rng` directly instead of `SimRng` — inconsistent with D-010. | ### Round 2 issue status: | Issue | Status | |-------|--------| | CRITICAL: `StorytellerPlugin` not registered | **NOT FIXED** (flagged in round 2 AND round 3) | | CRITICAL: `ContaminationActive` not in SaveStateV1 | Fixed | | WARNING: `detect_zone_crossings` ordering | Fixed | | WARNING: `z_bands_connected` doc comment | Not addressed | ### Verdict: CHANGES REQUESTED The `StorytellerPlugin` registration has been flagged as critical in **two consecutive rounds**. One-line fix: `app.add_plugins(StorytellerPlugin)` in `main()`.
jpmschweitzer added 1 commit 2026-02-28 23:48:09 +01:00
fix(simulation): address all round 3 PR review issues 282dad8d50 
- Register StorytellerPlugin in main() and dump_schedule_graph() so
  contamination system runs in production (critical, rounds 2+3)
- Add doc comment to z_bands_connected clarifying band indices vs
  absolute z-levels (D-110)
- Add TODO on hardcoded modifications: vec![] in save_to_file
- Init ContaminationActive in minimal_world() test helper
- Replace ChaCha20Rng with SimRng in fuzzy_map tests (D-010)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-28 23:51:37 +01:00
Author
Owner
Copy Link
Copy Source

Review: server → main (type: code, round 4)

Hoshe (Code Quality): APPROVE

All four round 3 issues confirmed fixed:

  • StorytellerPlugin registered in main() (L150) and dump_schedule_graph() (L327)
  • modifications: vec![] has TODO comment
  • z_bands_connected has doc comment distinguishing band indices from z-levels
  • minimal_world() inits ContaminationActive

Tyre (Architecture): APPROVE

Round 3 critical resolved. D-010 determinism, D-087 triangle classification, D-089 no cascade — all respected. CONTAMINATION_DELAY_TICKS = 300 correctly maps to 30 game-minutes per D-027.

Verdict: APPROVED

## Review: server → main (type: code, round 4) ### Hoshe (Code Quality): APPROVE All four round 3 issues confirmed fixed: - `StorytellerPlugin` registered in `main()` (L150) and `dump_schedule_graph()` (L327) - `modifications: vec![]` has TODO comment - `z_bands_connected` has doc comment distinguishing band indices from z-levels - `minimal_world()` inits `ContaminationActive` ### Tyre (Architecture): APPROVE Round 3 critical resolved. D-010 determinism, D-087 triangle classification, D-089 no cascade — all respected. `CONTAMINATION_DELAY_TICKS = 300` correctly maps to 30 game-minutes per D-027. ### Verdict: APPROVED
jpmschweitzer closed this pull request 2026-02-28 23:55:46 +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#79