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

feat(simulation): Sprint 15 React — server deliverables #55

Closed
jpmschweitzer wants to merge 0 commits from server into main
pull from: server
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-02-21 14:41:31 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 15 server deliverables — 7 tickets, 774 tests passing (+92 new):

  • #340 SpatialIndex trait with naive Vec backend (Manhattan distance, Bevy resource)
  • #92 NPC generation pipeline — seeds all 10 D-024 axes via SimRng, constraint validation
  • #90 Personality & tell system — 5 tell categories derived from NPC axis values per tick
  • #105 Tolerance threshold triggers — breach events, mood FSM integration, per-NPC thresholds
  • #243 Routine deviation detection — wrong location/activity, absence detection, pathfinding-aware
  • #241 Follow mechanic — Follow verb, proximity/LOS tracking, suspicion accumulation, configurable thresholds
  • #119 Monologue event generation — 5 new triggers (observe_npc, hear_sound, observe_anomaly, witness_interaction, post_conversation)

Protocol bumped to v13: tell_state on VisibleEntity, follow_state on ObserverSnapshot, Follow verb on VerbKind.

Test plan

  • 774 tests passing (cargo nextest), 0 failures
  • SpatialIndex correctness: in_range, at, update, de-duplication, stress test (11 QA tests)
  • Tolerance threshold: breach/recovery/re-breach cycle, background exclusion, zero threshold (16 tests)
  • Routine deviation: wrong location, wrong activity, pathfinding suppression, multi-NPC (12 tests)
  • NPC generation: constraint validation, deterministic seeding, trait population (16 tests)
  • Tell state derivation: all 5 categories from axis values, priority ordering (21 tests)
  • Follow mechanic: proximity counting, LOS loss, suspicion, serde roundtrip (24 tests)
  • Monologue triggers: cooldown guard, sound filtering, content pool selection (49 tests)
  • Cross-module integration: RoutineDeviation → AnomalyMarker → CognitiveDelay → monologue (deferred)
## Summary Sprint 15 server deliverables — 7 tickets, 774 tests passing (+92 new): - **#340** SpatialIndex trait with naive Vec backend (Manhattan distance, Bevy resource) - **#92** NPC generation pipeline — seeds all 10 D-024 axes via SimRng, constraint validation - **#90** Personality & tell system — 5 tell categories derived from NPC axis values per tick - **#105** Tolerance threshold triggers — breach events, mood FSM integration, per-NPC thresholds - **#243** Routine deviation detection — wrong location/activity, absence detection, pathfinding-aware - **#241** Follow mechanic — Follow verb, proximity/LOS tracking, suspicion accumulation, configurable thresholds - **#119** Monologue event generation — 5 new triggers (observe_npc, hear_sound, observe_anomaly, witness_interaction, post_conversation) Protocol bumped to v13: `tell_state` on `VisibleEntity`, `follow_state` on `ObserverSnapshot`, `Follow` verb on `VerbKind`. ## Test plan - [x] 774 tests passing (cargo nextest), 0 failures - [x] SpatialIndex correctness: in_range, at, update, de-duplication, stress test (11 QA tests) - [x] Tolerance threshold: breach/recovery/re-breach cycle, background exclusion, zero threshold (16 tests) - [x] Routine deviation: wrong location, wrong activity, pathfinding suppression, multi-NPC (12 tests) - [x] NPC generation: constraint validation, deterministic seeding, trait population (16 tests) - [x] Tell state derivation: all 5 categories from axis values, priority ordering (21 tests) - [x] Follow mechanic: proximity counting, LOS loss, suspicion, serde roundtrip (24 tests) - [x] Monologue triggers: cooldown guard, sound filtering, content pool selection (49 tests) - [ ] Cross-module integration: RoutineDeviation → AnomalyMarker → CognitiveDelay → monologue (deferred)
jpmschweitzer added 10 commits 2026-02-21 14:41:31 +01:00
feat(simulation): add SpatialIndex trait with naive Vec implementation (#340) 836c8e98cd 
Define SpatialIndex trait with entities_in_range, entities_at, and
update methods. NaiveSpatialIndex uses Vec backend with Manhattan
distance. sync_spatial_index system auto-updates from Changed<TilePosition>.
Registered as Bevy resource. Trait abstraction allows grid/quadtree
replacement later without touching callers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add tolerance threshold monitoring system (#105) 9a20e4dea9 
ToleranceBreachEvent emitted when NPC stress exceeds per-seed threshold.
ToleranceBreached marker prevents duplicate events per episode, cleared
on recovery. check_tolerance_threshold system runs after update_mood,
integrates with mood FSM to push toward Hostile/Anxious. Background-tier
NPCs excluded. Unblocks #250 (triangle escalation).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add NPC generation pipeline (#92) 78c43ab802 
RoleDefinition struct + generate_npc() seeds all 10 D-024 axes via
SimRng for deterministic generation. Constraint validation: no duplicate
relationship targets, stress < threshold, one routine entry per phase,
no contradictory personality trait pairs. Spawns fully-populated NPC
entity with 2-3 personality traits for tell system (#90).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add personality and tell system (#90) 029337a801 
TellCategory enum (Nervous, Angry, Friendly, Guarded, RoutineDeviation)
with DerivedTellState component. derive_tell_state system runs after
update_mood and detect_routine_deviation. Tell state derived from NPC
axis values per D-024: Secret+low Tolerance→Nervous, low Contentment+
Hostile→Angry, high Contentment+Friendly→Friendly, high Secret→Guarded.
v0.1 renderer is monologue text, not visual animation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add routine deviation detection (#243) f31be6f866 
RoutineDeviationEvent emitted when NPC breaks daily routine: wrong
location for day phase or wrong activity at correct location.
CurrentlyDeviating marker prevents duplicate events per episode.
Respects pathfinding-in-progress (no false positives). Feeds
observation event generator for observe_anomaly monologue triggers.
Primary detective mechanic per D-027 criterion 4.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add follow mechanic (#241) 0af86b1f0b 
Follow verb on interaction dispatcher. FollowTarget component tracks
target entity, proximity ticks, and LOS-lost ticks. update_follow_state
system: observation events fire at double frequency while following,
NPC suspicion increases via stress when player within 2 tiles for 60+
ticks (configurable). Follow ends on LOS lost timeout, suspicion
threshold crossed, or player issues different action. FollowStateWire
emitted in ObserverSnapshot for client HUD.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): extend monologue event generation with new triggers (#119) 517318c8a7 
Add observe_npc, hear_sound, observe_anomaly, witness_interaction, and
post_conversation triggers to monologue system. trigger_event_monologue
system fires after sound collection, NPC conversations, and walk-away.
Context tags (location, situation, character_state) populated for D-035
content pool matching. COOLDOWN_TICKS=300 anti-spam guard respected.
witness_interaction fires after overheard NPC-to-NPC conversation per
D-078. Voice sounds and ambient sounds correctly excluded from
hear_sound trigger.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): wire Sprint 15 systems + protocol v13 8538f916af 
Register all new systems in NpcPlugin and SimulationPlugin with correct
ordering constraints. Protocol bumped to v13: tell_state on VisibleEntity,
follow_state and PostConversationQueue on ObserverSnapshot, Follow verb
on VerbKind. Observer snapshot populates tell state from DerivedTellState
and follow state from FollowTarget. System ordering: tolerance after
mood, deviation after activity, tell after mood+deviation, follow after
visibility geometry, event monologue after conversations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
data(client): update msgpack fixtures and golden files for protocol v13 46cf744e6a 
Regenerate all msgpack test fixtures with tell_state and follow_state
fields. Update golden proof_room snapshot. Adjust serialization and
bridge tests for new ObserverSnapshot fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog c0fb316a0a 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-21 14:52:02 +01:00
Author
Owner
Copy Link
Copy Source

Review: server -> main (type: code) — PR #55

Hoshe (QA): REQUEST_CHANGES

Large, well-structured sprint — 6 new systems, comprehensive unit tests across all. Code quality is high. Three warnings found.

# File Severity Issue
1 bridge/types.rs:41 warning Stale doc comment: Protocol version for forward compatibility. Current: 12. — should say 13 (constant on line 18 is correct)
2 npc/generate.rs:151-152 vs npc/mod.rs:128 warning Want intensity generated in range 3..=9, but spec comment says 1-10 and test asserts 1..=10. Test gives false confidence — either widen generator to 1..=10 or narrow spec/test to 3..=9
3 simulation/monologue.rs:355-401 warning trigger_recognition_monologue contains verbatim copy of the two-pass content-pool selection loop that already exists in select_pool_line (line 441). ~50 lines of duplication — maintenance hazard, call the helper instead
4 npc/generate.rs:464-485 suggestion generate_npc spawns all 10 axis components but not TilePosition — undocumented caller invariant, add doc comment
5 npc/tell_state.rs:38-51 suggestion TellCategory derives Default with #[default] Nervous — a neutral NPC shouldn't default to highest-salience tell. Consider removing Default derive or moving it to None

Tyre (Architecture): REQUEST_CHANGES

Architecture is sound — trait abstractions, ordering chains, and integer/BTreeMap discipline are correct. Two warnings, one false alarm corrected.

# File Severity Issue
1 bridge/types.rs:41 warning Stale protocol version comment (same as Hoshe #1)
2 simulation/spatial.rs — sync_spatial_index warning Takes ResMut<NaiveSpatialIndex> directly, not trait object. bevy_ecs limitation (can't store dyn Trait as Resource), but means Grid/BVH swap requires changing call site — not zero-touch as trait abstraction implies. Document migration cost
3 npc/generate.rs suggestion Cannot verify from headers that all randomness routes through SimRng. Recommend a test that verifies same-seed generation produces identical NPC output
4 simulation/spatial.rs suggestion O(N) scan fine at current scale (50 NPCs). Track against D-026 tick budget — Grid swap should precede sustained 60+ Active NPCs

Tyre false alarms (verified by lead):

  • DialogueResponseEvent.speaker_color_index and speaker_name — both have #[serde(default)]. Wire compat is correct.
  • update_trust ordering — explicitly uses .after(crate::simulation::dialogue::process_confrontation_response). Cross-plugin constraint is properly specified.

Verdict: CHANGES REQUESTED

Actionable items for server team:

  1. Fix stale protocol version comment (12 → 13) — trivial
  2. Resolve want intensity range mismatch (generator 3..=9 vs spec 1-10)
  3. Extract duplicated pool selection in trigger_recognition_monologue to use select_pool_line
  4. Document NaiveSpatialIndex swap-path migration cost (Tyre W2)

All four are quick fixes. The architecture, system ordering, wire compat, and determinism guarantees are solid.

## Review: server -> main (type: code) — PR #55 ### Hoshe (QA): REQUEST_CHANGES Large, well-structured sprint — 6 new systems, comprehensive unit tests across all. Code quality is high. Three warnings found. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `bridge/types.rs:41` | warning | Stale doc comment: `Protocol version for forward compatibility. Current: 12.` — should say 13 (constant on line 18 is correct) | | 2 | `npc/generate.rs:151-152` vs `npc/mod.rs:128` | warning | Want intensity generated in range 3..=9, but spec comment says 1-10 and test asserts 1..=10. Test gives false confidence — either widen generator to 1..=10 or narrow spec/test to 3..=9 | | 3 | `simulation/monologue.rs:355-401` | warning | `trigger_recognition_monologue` contains verbatim copy of the two-pass content-pool selection loop that already exists in `select_pool_line` (line 441). ~50 lines of duplication — maintenance hazard, call the helper instead | | 4 | `npc/generate.rs:464-485` | suggestion | `generate_npc` spawns all 10 axis components but not `TilePosition` — undocumented caller invariant, add doc comment | | 5 | `npc/tell_state.rs:38-51` | suggestion | `TellCategory` derives Default with `#[default] Nervous` — a neutral NPC shouldn't default to highest-salience tell. Consider removing Default derive or moving it to `None` | ### Tyre (Architecture): REQUEST_CHANGES Architecture is sound — trait abstractions, ordering chains, and integer/BTreeMap discipline are correct. Two warnings, one false alarm corrected. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `bridge/types.rs:41` | warning | Stale protocol version comment (same as Hoshe #1) | | 2 | `simulation/spatial.rs` — `sync_spatial_index` | warning | Takes `ResMut<NaiveSpatialIndex>` directly, not trait object. bevy_ecs limitation (can't store `dyn Trait` as Resource), but means Grid/BVH swap requires changing call site — not zero-touch as trait abstraction implies. Document migration cost | | 3 | `npc/generate.rs` | suggestion | Cannot verify from headers that all randomness routes through SimRng. Recommend a test that verifies same-seed generation produces identical NPC output | | 4 | `simulation/spatial.rs` | suggestion | O(N) scan fine at current scale (50 NPCs). Track against D-026 tick budget — Grid swap should precede sustained 60+ Active NPCs | **Tyre false alarms (verified by lead):** - `DialogueResponseEvent.speaker_color_index` and `speaker_name` — both have `#[serde(default)]`. Wire compat is correct. - `update_trust` ordering — explicitly uses `.after(crate::simulation::dialogue::process_confrontation_response)`. Cross-plugin constraint is properly specified. ### Verdict: CHANGES REQUESTED **Actionable items for server team:** 1. Fix stale protocol version comment (12 → 13) — trivial 2. Resolve want intensity range mismatch (generator 3..=9 vs spec 1-10) 3. Extract duplicated pool selection in `trigger_recognition_monologue` to use `select_pool_line` 4. Document NaiveSpatialIndex swap-path migration cost (Tyre W2) All four are quick fixes. The architecture, system ordering, wire compat, and determinism guarantees are solid.
jpmschweitzer added 1 commit 2026-02-21 14:55:27 +01:00
fix(simulation): address PR #55 review — stale comment, range, duplication, docs 67cadddaf2 
- Fix protocol version comment (12 → 13) in ObserverSnapshot doc
- Widen Want intensity range from 3..=9 to 1..=10 to match spec and test
- Replace duplicated pool selection in trigger_recognition_monologue with
  call to select_pool_line helper (~50 lines removed)
- Document NaiveSpatialIndex migration cost for grid/quadtree swap
- Remove misleading Default derive from TellCategory (Nervous is not a
  sensible default for neutral NPCs)
- Add caller invariant doc on generate_npc (no TilePosition spawned)

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

Review Round 2: server -> main (type: code) — PR #55

Hoshe (QA): APPROVE

All 4 required fixes and both suggestions from round 1 verified. Full QA pass clean — 6 new systems with comprehensive unit tests across all.

# File Severity Issue
1 simulation/follow.rs:145-147 suggestion Two separate NPC queries (target_query + tolerance_query) are distinct components so no ECS conflict, but split is easy to misread — add comment explaining why
2 simulation/monologue.rs:676-712 suggestion trigger_monologue still has inline pool selection loop (not required by round 1 fix, but now select_pool_line exists) — follow-up consolidation ticket
3 npc/generate.rs:269 suggestion format!("Routine schedule") with no interpolation — clippy useless_format lint, use .to_string()

Tyre (Architecture): APPROVE

Fix commit correctly addresses all required changes. Full test suite passes (774 tests, 0 failures). Wire compatibility confirmed — TellCategory Default removal handled correctly by Option wrapper + #[serde(default)].

No new issues.

Verdict: APPROVED

## Review Round 2: server -> main (type: code) — PR #55 ### Hoshe (QA): APPROVE All 4 required fixes and both suggestions from round 1 verified. Full QA pass clean — 6 new systems with comprehensive unit tests across all. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `simulation/follow.rs:145-147` | suggestion | Two separate NPC queries (`target_query` + `tolerance_query`) are distinct components so no ECS conflict, but split is easy to misread — add comment explaining why | | 2 | `simulation/monologue.rs:676-712` | suggestion | `trigger_monologue` still has inline pool selection loop (not required by round 1 fix, but now `select_pool_line` exists) — follow-up consolidation ticket | | 3 | `npc/generate.rs:269` | suggestion | `format!("Routine schedule")` with no interpolation — clippy `useless_format` lint, use `.to_string()` | ### Tyre (Architecture): APPROVE Fix commit correctly addresses all required changes. Full test suite passes (774 tests, 0 failures). Wire compatibility confirmed — `TellCategory` Default removal handled correctly by `Option` wrapper + `#[serde(default)]`. No new issues. ### Verdict: APPROVED
jpmschweitzer closed this pull request 2026-02-21 15:13:37 +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#55