feat(simulation): sprint 12 server — tier system, sound events, KG access, line previewer #42

Closed

jpmschweitzer wants to merge 0 commits from server into main

pull from: server

merge into: :main

:main

Conversation 2 Commits - Files Changed -

+2476 -56

jpmschweitzer commented 2026-02-19 14:50:54 +01:00

Owner

Copy Link

Copy Source

Summary

Sprint 12 server deliverables — simulation tier system, sound event pipeline, knowledge graph access control, and line previewer CLI.

• Tier system (#93, #94, #99): ActiveSim/BackgroundSim/StateSaved marker components with promote/demote transitions based on player proximity. With<ActiveSim> filters on path following and NPC routines. 28+ tests.
• Sound events (#124): SoundEventEmitter, SoundEventQueue, 5 event kinds (Footstep/Voice/Machinery/Alert/Ambient), D-018 three-range model, bridge integration via ObserverSnapshot.sound_events. 17 tests.
• Knowledge access control (#138, #139): ObserverAccess enum (Public/OwnerOnly/FactionOnly/RelationshipGated/KnowledgeGated), filter_by_access() wired into observer snapshot builder. 27 tests.
• Line previewer CLI (#193): 641-line line_preview binary — YAML content loading, monologue/dialogue filtering, --explain mode, sequence preview. Unblocks copy team content packs (#190, #191, #192).

Tickets

• #93 Tier marker components (done)
• #94 Active tier simulation (review)
• #99 Tier transition logic (review)
• #124 Sound event system — server (done)
• #138 Information tag schema (review)
• #139 Component-level access control (review)
• #193 Line previewer CLI (review)

Test plan

• cargo build — clean
• cargo test — 548 tests pass, 0 failures
• Verify tier transitions in debug overlay (promote <5ms, demote <5ms)
• Verify sound events appear in ObserverSnapshot for NPCs in range
• Verify filter_by_access blocks OwnerOnly components for non-owners
• Run cargo run --bin line_preview -- --character smuggler --knows smuggling_operation against content YAML

## Summary Sprint 12 server deliverables — simulation tier system, sound event pipeline, knowledge graph access control, and line previewer CLI. - **Tier system** (#93, #94, #99): `ActiveSim`/`BackgroundSim`/`StateSaved` marker components with promote/demote transitions based on player proximity. `With<ActiveSim>` filters on path following and NPC routines. 28+ tests. - **Sound events** (#124): `SoundEventEmitter`, `SoundEventQueue`, 5 event kinds (Footstep/Voice/Machinery/Alert/Ambient), D-018 three-range model, bridge integration via `ObserverSnapshot.sound_events`. 17 tests. - **Knowledge access control** (#138, #139): `ObserverAccess` enum (Public/OwnerOnly/FactionOnly/RelationshipGated/KnowledgeGated), `filter_by_access()` wired into observer snapshot builder. 27 tests. - **Line previewer CLI** (#193): 641-line `line_preview` binary — YAML content loading, monologue/dialogue filtering, `--explain` mode, sequence preview. Unblocks copy team content packs (#190, #191, #192). ## Tickets - #93 Tier marker components (done) - #94 Active tier simulation (review) - #99 Tier transition logic (review) - #124 Sound event system — server (done) - #138 Information tag schema (review) - #139 Component-level access control (review) - #193 Line previewer CLI (review) ## Test plan - [x] `cargo build` — clean - [x] `cargo test` — 548 tests pass, 0 failures - [ ] Verify tier transitions in debug overlay (promote <5ms, demote <5ms) - [ ] Verify sound events appear in `ObserverSnapshot` for NPCs in range - [ ] Verify `filter_by_access` blocks `OwnerOnly` components for non-owners - [ ] Run `cargo run --bin line_preview -- --character smuggler --knows smuggling_operation` against content YAML

jpmschweitzer added 2 commits 2026-02-19 14:50:54 +01:00

feat(simulation): sprint 12 server — tier system, sound events, KG access, line previewer ... 066f8031fd

Implements 4 completed tickets + partial progress on 2 more:

- #93 Tier marker components (ActiveSim, BackgroundSim, StateSaved + TierPlugin)
- #138 Information tag schema (ObserverAccess enum in knowledge/types.rs)
- #124 Sound event system (SoundEventEmitter, SoundEventQueue, bridge wiring)
- #193 Line previewer CLI (line_preview binary with filter/explain/sequence modes)
- #94 Active tier simulation (in progress — With<ActiveSim> filters)
- #139 Component-level access control (in progress — filter_by_access)

Updates snapshot fixtures and test golden files for new sound_events field.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

feat(simulation): #94 #99 active tier filtering and transition system ... 8e00db2b2d

#94 — Active tier simulation (complete):
- Add ActiveSim marker to all 9 test world room NPC spawns
- Fix test entities in routine.rs and path_follow.rs to include ActiveSim
  so With<ActiveSim> queries match correctly in unit tests

#99 — Tier transition logic (complete):
- Implement update_tier_markers system in tier.rs
- Promotes/demotes tier markers by manhattan distance from PlayerCharacter:
  ≤40 tiles → ActiveSim, ≤120 → BackgroundSim, beyond → StateSaved
- Handles cross-z-level as u32::MAX (effectively unreachable)
- No-op when no PlayerCharacter entity present (headless tests safe)
- 11 new unit tests covering all distance bands and boundary cases
- TierPlugin now registers the system after movement::validate_movement

Also picks up extended test coverage added by hoshe:
- observer/tests.rs — 230 lines of perception observer tests
- sound.rs — additional sound event integration tests

All 548 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jpmschweitzer commented 2026-02-19 15:04:04 +01:00

Author

Owner

Copy Link

Copy Source

Review: server -> main (type: code)

Verdict: CHANGES REQUESTED

---

Hoshe (Code Quality): REQUEST_CHANGES

The PR delivers working tier system, sound pipeline, KG access control, and line previewer with solid unit tests. No incomplete implementations or dead code from the crash found. However, the sound pipeline has a critical integration test gap and there are naming/doc issues.

#	File	Severity	Issue
1	observer/tests.rs:10-18	warning	setup_world never inserts SoundEventQueue — all observer tests take the None branch. Zero integration coverage of emitter -> queue -> snapshot -> wire.
2	observer/mod.rs:91	warning	_observer_pos uses underscore-prefix but IS used at line 191. Triggers clippy::used_underscore_binding. Rename to observer_pos.
3	knowledge/graph.rs:~263	warning	FactionOnly has only a negative test. No positive test for matching faction_id.
4	bridge/types.rs:34	suggestion	Doc comment says "Current: 9" but PROTOCOL_VERSION is 10.
5	simulation/tier.rs:133-160	suggestion	SimulationTier, LastInteraction, ScopeTag, ScopeKind unused outside own tests. Forward stubs or leftover scaffolding.
6	simulation/sound.rs:86	suggestion	audible_at has no obstruction check per D-018 Medium range. Acceptable now, needs ticket ref for occlusion work.

Tyre (Architecture): REQUEST_CHANGES

Tier system, KG access control, and line previewer are architecturally solid and D-026/D-010 compliant. Three issues block merge.

#	File	Severity	Issue
1	sound.rs + movement.rs, dialogue.rs, monologue.rs	critical	SoundEventEmitter is never inserted by any system. Pipeline wired end-to-end but produces zero events at runtime. Strongest signal of the session crash. Needs at minimum a Footstep emitter or explicit "empty in v0.1" comment with tracking ticket.
2	npc/routine.rs tests (~lines 122, 145, 178)	warning	3 of 5 routine tests spawn NPCs without ActiveSim. System query requires it, so assert!(is_none()) passes trivially. Three behavior branches untested.
3	bridge/types.rs:34	warning	Doc comment reads "Current: 9" but constant is 10.
4	simulation/tier.rs:135-164	suggestion	SimulationTier enum unused outside tests. Ungenerated has no marker. Looks like abandoned design.
5	knowledge/graph.rs — FactionOnly	suggestion	.parse::<u64>() silently returns false on non-numeric strings. A tracing::warn on parse failure would help.
6	observer/mod.rs:91	suggestion	_observer_pos misleadingly prefixed as unused but is used at line 191.

Deduplicated action items

1. CRITICAL — Sound producers missing (crash artifact). Wire at least Footstep emitter, or add explicit "empty" comment + tracking ticket.
2. WARNING — 3 routine tests missing ActiveSim, producing false positives.
3. WARNING — _observer_pos -> observer_pos (misleading unused prefix).
4. WARNING — FactionOnly needs a positive test case.
5. WARNING — Stale version comment: "Current: 9" should be 10.
6. Suggestion — Orphaned SimulationTier/LastInteraction/ScopeTag types need comment or removal.
7. Suggestion — FactionOnly parse failure should log a warning.
8. Suggestion — Track occlusion gap for Medium-range audibility.

## Review: server -> main (type: code) **Verdict: CHANGES REQUESTED** --- ### Hoshe (Code Quality): REQUEST_CHANGES The PR delivers working tier system, sound pipeline, KG access control, and line previewer with solid unit tests. No incomplete implementations or dead code from the crash found. However, the sound pipeline has a critical integration test gap and there are naming/doc issues. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `observer/tests.rs:10-18` | warning | `setup_world` never inserts `SoundEventQueue` — all observer tests take the `None` branch. Zero integration coverage of emitter -> queue -> snapshot -> wire. | | 2 | `observer/mod.rs:91` | warning | `_observer_pos` uses underscore-prefix but IS used at line 191. Triggers `clippy::used_underscore_binding`. Rename to `observer_pos`. | | 3 | `knowledge/graph.rs:~263` | warning | `FactionOnly` has only a negative test. No positive test for matching faction_id. | | 4 | `bridge/types.rs:34` | suggestion | Doc comment says "Current: 9" but `PROTOCOL_VERSION` is 10. | | 5 | `simulation/tier.rs:133-160` | suggestion | `SimulationTier`, `LastInteraction`, `ScopeTag`, `ScopeKind` unused outside own tests. Forward stubs or leftover scaffolding. | | 6 | `simulation/sound.rs:86` | suggestion | `audible_at` has no obstruction check per D-018 Medium range. Acceptable now, needs ticket ref for occlusion work. | ### Tyre (Architecture): REQUEST_CHANGES Tier system, KG access control, and line previewer are architecturally solid and D-026/D-010 compliant. Three issues block merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `sound.rs` + `movement.rs`, `dialogue.rs`, `monologue.rs` | **critical** | `SoundEventEmitter` is never inserted by any system. Pipeline wired end-to-end but produces zero events at runtime. Strongest signal of the session crash. Needs at minimum a Footstep emitter or explicit "empty in v0.1" comment with tracking ticket. | | 2 | `npc/routine.rs` tests (~lines 122, 145, 178) | warning | 3 of 5 routine tests spawn NPCs without `ActiveSim`. System query requires it, so `assert!(is_none())` passes trivially. Three behavior branches untested. | | 3 | `bridge/types.rs:34` | warning | Doc comment reads "Current: 9" but constant is 10. | | 4 | `simulation/tier.rs:135-164` | suggestion | `SimulationTier` enum unused outside tests. `Ungenerated` has no marker. Looks like abandoned design. | | 5 | `knowledge/graph.rs` — `FactionOnly` | suggestion | `.parse::<u64>()` silently returns false on non-numeric strings. A `tracing::warn` on parse failure would help. | | 6 | `observer/mod.rs:91` | suggestion | `_observer_pos` misleadingly prefixed as unused but is used at line 191. | ### Deduplicated action items 1. **CRITICAL** — Sound producers missing (crash artifact). Wire at least Footstep emitter, or add explicit "empty" comment + tracking ticket. 2. **WARNING** — 3 routine tests missing `ActiveSim`, producing false positives. 3. **WARNING** — `_observer_pos` -> `observer_pos` (misleading unused prefix). 4. **WARNING** — `FactionOnly` needs a positive test case. 5. **WARNING** — Stale version comment: "Current: 9" should be 10. 6. Suggestion — Orphaned `SimulationTier`/`LastInteraction`/`ScopeTag` types need comment or removal. 7. Suggestion — `FactionOnly` parse failure should log a warning. 8. Suggestion — Track occlusion gap for Medium-range audibility.

jpmschweitzer referenced this issue from a commit 2026-02-19 15:08:58 +01:00

fix(simulation): address PR #42 review — 8 items from Hoshe and Tyre

jpmschweitzer added 2 commits 2026-02-19 15:08:58 +01:00

fix(simulation): address PR #42 review — 8 items from Hoshe and Tyre ... 03c44bebaf

1. Sound producers: document empty v0.1 pipeline explicitly (critical)
2. Routine tests: add ActiveSim to 3 tests that passed trivially
3. Rename _observer_pos → observer_pos (used at line 191)
4. Add FactionOnly positive test case (matching faction_id)
5. Fix stale doc comment "Current: 9" → 10 in ObserverSnapshot
6. Remove orphaned SimulationTier/LastInteraction/ScopeTag types
7. Add tracing::warn on FactionOnly non-numeric parse failure
8. Document Medium-range occlusion gap as TODO in audible_at
9. Insert SoundEventQueue in observer test setup_world

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

chore(meta): update changelog ... f533235f58

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jpmschweitzer referenced this issue from a commit 2026-02-19 15:14:58 +01:00

feat(simulation): wire footstep sound events into movement system

jpmschweitzer added 1 commit 2026-02-19 15:14:58 +01:00

feat(simulation): wire footstep sound events into movement system ... 87a90b27bd

validate_movement now inserts SoundEventEmitter with Footstep events on
every successful move. Intensity scales by stance: Sprint 0.8, Walk 0.5,
Careful 0.3, Crouch 0.15. Range is Close (3 tiles) for all stances.

This completes the sound event pipeline end-to-end: movement produces
events → collect_sound_events drains to queue → observer snapshot
includes audible events → client bridge receives them.

Addresses Tyre critical review item #1 on PR #42.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jpmschweitzer commented 2026-02-19 15:28:50 +01:00

Author

Owner

Copy Link

Copy Source

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

Verdict: APPROVED

All 8 items from the previous review are verified fixed. No new critical or warning issues introduced.

---

Hoshe (Code Quality): APPROVE

All 8 items properly addressed. The footstep emission in validate_movement is clean — intensity scales by stance, range is Close, emitter only inserted on successful moves. Four new tests verify the key contracts.

#	File	Severity	Issue
1	movement.rs:341	suggestion	source_entity_id: None for footstep events — footsteps have a known source entity. Passing the stable ID would enable entity attribution. Understandable deferral for v0.1 but a TODO comment would make the gap explicit.
2	movement.rs tests	suggestion	No dedicated test for Careful stance intensity (0.3) or explicit Walk (0.5). Sprint/Crouch bracket the range and impl is simple, but careful_stance_produces_moderate_footstep would complete the matrix.
3	movement.rs tests	suggestion	successful_move_emits_footstep_sound does not assert source_entity_id. An explicit assert_eq!(None) would document the behavior as deliberate.

Tyre (Architecture): APPROVE

Pipeline is end-to-end connected and D-010/D-012/D-018/D-026 compliant. System ordering is explicit and correct: validate_movement -> collect_sound_events -> compute_observer_snapshot. No ordering hazards.

Footstep wiring is architecturally sound: emits at target position (where entity lands), transient component (zero accumulation risk), intensity matches D-053 stance semantics, Option<u64> source_entity_id field ready for future attribution.

No issues found.

Previous review items — all verified

1. CRITICAL — Sound producers: FIXED. validate_movement inserts SoundEventEmitter with Footstep events. 4 new tests.
2. WARNING — Routine tests ActiveSim: FIXED. All 5 tests have ActiveSim.
3. WARNING — _observer_pos naming: FIXED. Renamed to observer_pos.
4. WARNING — FactionOnly positive test: FIXED. Positive + negative coverage.
5. WARNING — Stale version comment: FIXED. "Current: 10".
6. Suggestion — Orphaned types: FIXED. SimulationTier/LastInteraction/ScopeTag/ScopeKind removed.
7. Suggestion — FactionOnly parse warning: FIXED. tracing::warn! with structured fields.
8. Suggestion — Occlusion tracking: FIXED. TODO comment in audible_at().

## Re-Review: server -> main (type: code) — PR #42 **Verdict: APPROVED** All 8 items from the previous review are verified fixed. No new critical or warning issues introduced. --- ### Hoshe (Code Quality): APPROVE All 8 items properly addressed. The footstep emission in `validate_movement` is clean — intensity scales by stance, range is Close, emitter only inserted on successful moves. Four new tests verify the key contracts. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `movement.rs:341` | suggestion | `source_entity_id: None` for footstep events — footsteps have a known source entity. Passing the stable ID would enable entity attribution. Understandable deferral for v0.1 but a TODO comment would make the gap explicit. | | 2 | `movement.rs` tests | suggestion | No dedicated test for `Careful` stance intensity (0.3) or explicit `Walk` (0.5). Sprint/Crouch bracket the range and impl is simple, but `careful_stance_produces_moderate_footstep` would complete the matrix. | | 3 | `movement.rs` tests | suggestion | `successful_move_emits_footstep_sound` does not assert `source_entity_id`. An explicit `assert_eq!(None)` would document the behavior as deliberate. | ### Tyre (Architecture): APPROVE Pipeline is end-to-end connected and D-010/D-012/D-018/D-026 compliant. System ordering is explicit and correct: `validate_movement` -> `collect_sound_events` -> `compute_observer_snapshot`. No ordering hazards. Footstep wiring is architecturally sound: emits at target position (where entity lands), transient component (zero accumulation risk), intensity matches D-053 stance semantics, `Option<u64>` source_entity_id field ready for future attribution. No issues found. ### Previous review items — all verified 1. **CRITICAL — Sound producers**: FIXED. `validate_movement` inserts `SoundEventEmitter` with Footstep events. 4 new tests. 2. **WARNING — Routine tests ActiveSim**: FIXED. All 5 tests have `ActiveSim`. 3. **WARNING — `_observer_pos` naming**: FIXED. Renamed to `observer_pos`. 4. **WARNING — FactionOnly positive test**: FIXED. Positive + negative coverage. 5. **WARNING — Stale version comment**: FIXED. "Current: 10". 6. **Suggestion — Orphaned types**: FIXED. `SimulationTier`/`LastInteraction`/`ScopeTag`/`ScopeKind` removed. 7. **Suggestion — FactionOnly parse warning**: FIXED. `tracing::warn!` with structured fields. 8. **Suggestion — Occlusion tracking**: FIXED. TODO comment in `audible_at()`.

jpmschweitzer closed this pull request 2026-02-19 15:29:21 +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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#42