feat(simulation): Sprint 26 server — voice pipeline, settings, behavior engine, v0.1 cleanup #89

2026-03-13T09:13:17+01:00

jpmschweitzer commented

2026-03-13 09:13:17 +01:00

Summary

Sprint 26 (Clean House) server deliverables:

#650 ContentType::Factual — Lines with numbers/denials/causal chains bypass LLM, served as base text
#652 Voice pipeline observer integration — Enrichment systems rewrite dialogue with voiced text before snapshot assembly; graceful degradation on cache miss
#651 Tell iteration — Stronger few-shot examples for Friendly/RoutineDeviation tells
#627 SQLite settings storage — Per-player persistent settings via rusqlite (bundled), protocol v20 with settings commands over IPC
#633 Composable behavior engine — Three-layer action+modifier+context primitives replace flat behavior strings; backward compatible fallback; D-139 filed, Q-057 resolved
#655 Remove v0.1 content loading — Deleted server/src/content/ (8200 lines), tooling converters, content-ron/, content/_meta/; runtime types relocated

Stats

77 files changed, +2174 / -8969 lines (net reduction)
Protocol v19 → v20
1233 lib tests passing, 49 voice tests passing
cargo check --tests clean

Test plan

cargo check --tests passes
Voice pipeline unit + integration tests pass
Settings CRUD roundtrip tests pass
Behavior assembly produces equivalent output for seed 42
Serialization tests pass with regenerated msgpack fixtures
PR review via /pr-review

## Summary Sprint 26 (Clean House) server deliverables: - **#650 ContentType::Factual** — Lines with numbers/denials/causal chains bypass LLM, served as base text - **#652 Voice pipeline observer integration** — Enrichment systems rewrite dialogue with voiced text before snapshot assembly; graceful degradation on cache miss - **#651 Tell iteration** — Stronger few-shot examples for Friendly/RoutineDeviation tells - **#627 SQLite settings storage** — Per-player persistent settings via rusqlite (bundled), protocol v20 with settings commands over IPC - **#633 Composable behavior engine** — Three-layer action+modifier+context primitives replace flat behavior strings; backward compatible fallback; D-139 filed, Q-057 resolved - **#655 Remove v0.1 content loading** — Deleted server/src/content/ (8200 lines), tooling converters, content-ron/, content/_meta/; runtime types relocated ## Stats - 77 files changed, +2174 / -8969 lines (net reduction) - Protocol v19 → v20 - 1233 lib tests passing, 49 voice tests passing - `cargo check --tests` clean ## Test plan - [x] `cargo check --tests` passes - [x] Voice pipeline unit + integration tests pass - [x] Settings CRUD roundtrip tests pass - [x] Behavior assembly produces equivalent output for seed 42 - [x] Serialization tests pass with regenerated msgpack fixtures - [ ] PR review via `/pr-review`

jpmschweitzer added 5 commits 2026-03-13 09:13:18 +01:00

refactor(simulation): remove v0.1 content loading system (#655 ) ce0df2f320

Delete the hand-authored YAML content pipeline (server/src/content/) superseded
by the v0.2 generator-first approach (D-122, D-128). Runtime ECS types that were
co-located with content loading have been extracted to dedicated simulation modules:

- simulation/triangle.rs: TriangleState, TriangleCrisisEventQueue, tick/resolve systems
- simulation/line_pool.rs: LinePoolIndex, AccessTier, TrustTier, Mood, LinePoolIndexResource
- simulation/knowledge_grant.rs: KnowledgeGrant, Prerequisites

Monologue systems (trigger_monologue, trigger_recognition_monologue,
trigger_event_monologue) now use hardcoded fallback lines only; the
ContentStoreResource branch and select_pool_line function are removed.

Deleted: content/{loader,types,line_pool,hot_reload,spawn,instantiation,entanglement,mod}.rs
Deleted: tests/{content_loading,content_runtime,content_scaling,template_instantiation,template_schema}.rs
Deleted: bin/line_preview.rs (v0.1 tool)

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

feat(simulation): voice pipeline integration — Factual bypass, observer wiring, tell iteration (#650 #652 #651 ) 1a4fd578cc

ContentType::Factual bypasses LLM for fact-bearing lines (numbers,
denials). Observer enrichment systems rewrite dialogue/conversation
text with voiced versions before snapshot assembly. Friendly and
RoutineDeviation tells get concrete surface-pattern examples.

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

feat(simulation): SQLite settings storage with IPC protocol v20 (#627 ) accbe579c9

Per-player settings via rusqlite (bundled, zero runtime dep). Server
owns the settings DB; client sends ChangeSettings commands over IPC.
Extensible key-value with typed columns (String/Int/Float/Bool).
Protocol bumped to v20 with settings_response in ObserverSnapshot.

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

feat(simulation): composable behavior engine — action+modifier+context primitives (#633 ) 50800cbf81

Replaces flat culture×zone×role behavior strings with three-layer
composition: BehaviorAction (role-generic), BehaviorModifier (culture
coloring), BehaviorContext (situation gating). Assembly at NpcBlueprint
instantiation. Backward compatible — falls back to legacy behaviors
when primitives are empty. D-139 filed, Q-057 resolved.

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

chore(meta): update changelog e317a66ac6

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

jpmschweitzer commented

2026-03-13 09:26:20 +01:00

Review: server → main (type: code + foundations)

Three review tracks: Hoshe (code quality), Tyre (architecture), Gestalt (systems design foundations).

Hoshe (Code Quality): REQUEST_CHANGES

#	File	Severity	Issue
1	`settings/mod.rs:173-185`	warning	`Delete` returns `success: true` with `error: Some(...)` when key not found — broken contract. Either return `success: false` when not found, or treat as silent no-op (no error set).
2	`npc/blueprint.rs:~167`	suggestion	Doc on `BehaviorModifier` claims "at most one modifier per category per NPC" but `assemble_behaviors` does no dedup. Drop the doc claim — occasional repetition is acceptable and avoids an uncanny "every NPC is unique" pattern. The true fix for variety is richer modifier pools in the content, not engine-level dedup.
3	`voice/integration.rs:173-175`	suggestion	Conversation content_index hash collision rationale is undocumented. The cache key includes entity_id so collisions require same culture + same speaker + same text — document why this is safe rather than leaving as a TODO.
4	`bridge/types.rs:87`	warning	Version comment says "Current: 19", should be 20. Third time this pattern has recurred — consider a compile-time assertion or doc-gen.
5	`settings/store.rs:~50`	suggestion	`open_in_memory()` skips `PRAGMA foreign_keys = ON` — tests may miss FK violations.
6	`settings/store.rs:161-188`	suggestion	`columns_to_value` / `columns_to_value_offset` differ only by index offset — consolidate into single helper.
7	`simulation/line_pool.rs:~358`	suggestion	Empty `active_situations` silently returns zero results — add tracing for content debugging.
8	`simulation/knowledge_grant.rs:~16`	suggestion	`#[serde(untagged)]` silently drops `entity_ref` if both fields present — consider validation at load.

Tyre (Architecture): REQUEST_CHANGES

#	File	Severity	Issue
1	`bridge/types.rs:87`	warning	`ObserverSnapshot.version` doc says "Current: 19" — should be 20. (Same as Hoshe #4.)
2	`simulation/monologue.rs`	suggestion	`trigger_monologue` is a no-op stub still registered in the schedule — remove from registration or annotate lifecycle.
3	`simulation/monologue.rs`	suggestion	`trigger_event_monologue` doc still references removed `COOLDOWN_TICKS` constant.

Gestalt (Foundations): APPROVE

All three new systems graded SOLID. Clean separation, graceful degradation at boundaries, determinism preserved (BTreeMap usage, seed-driven RNG). Ready to build on.

#	File	Severity	Issue
1	`npc/blueprint.rs`	suggestion	Silent fallback when `modifier_hint` doesn't match any category — log for content debugging.
2	`npc/blueprint.rs`	suggestion	`OffDuty` context has no test coverage in filter tests.
3	`settings/mod.rs`	suggestion	"Last response wins" batching — multi-change same-tick acks are incomplete. Worth a comment.
4	`settings/store.rs`	suggestion	Unknown `value_type` silently converts to String instead of logging.
5	`voice/integration.rs`	suggestion	Re-voicing heavily occluded text may produce incoherent output — accepted risk, worth a comment.
6	`voice/worker.rs`	suggestion	Dead child process causes repeated warnings without recovery — consider respawn or loop break.
7	`voice/prompt_builder.rs`	suggestion	Multiple INJECT blocks indistinguishable to model if multiple injections fire same prompt.

Verdict: CHANGES REQUESTED

Must-fix (2 items):

Protocol version comment — bridge/types.rs:87 says "Current: 19", should be 20.
Delete response contract — settings/mod.rs returns success: true with error: Some(...) when key not found. Pick one: success: false, or silent no-op.

Everything else is suggestion-level and can be addressed in follow-up tickets.

## Review: server → main (type: code + foundations) Three review tracks: Hoshe (code quality), Tyre (architecture), Gestalt (systems design foundations). --- ### Hoshe (Code Quality): REQUEST_CHANGES | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `settings/mod.rs:173-185` | **warning** | `Delete` returns `success: true` with `error: Some(...)` when key not found — broken contract. Either return `success: false` when not found, or treat as silent no-op (no error set). | | 2 | `npc/blueprint.rs:~167` | suggestion | Doc on `BehaviorModifier` claims "at most one modifier per category per NPC" but `assemble_behaviors` does no dedup. Drop the doc claim — occasional repetition is acceptable and avoids an uncanny "every NPC is unique" pattern. The true fix for variety is richer modifier pools in the content, not engine-level dedup. | | 3 | `voice/integration.rs:173-175` | suggestion | Conversation content_index hash collision rationale is undocumented. The cache key includes entity_id so collisions require same culture + same speaker + same text — document why this is safe rather than leaving as a TODO. | | 4 | `bridge/types.rs:87` | **warning** | Version comment says "Current: 19", should be 20. Third time this pattern has recurred — consider a compile-time assertion or doc-gen. | | 5 | `settings/store.rs:~50` | suggestion | `open_in_memory()` skips `PRAGMA foreign_keys = ON` — tests may miss FK violations. | | 6 | `settings/store.rs:161-188` | suggestion | `columns_to_value` / `columns_to_value_offset` differ only by index offset — consolidate into single helper. | | 7 | `simulation/line_pool.rs:~358` | suggestion | Empty `active_situations` silently returns zero results — add tracing for content debugging. | | 8 | `simulation/knowledge_grant.rs:~16` | suggestion | `#[serde(untagged)]` silently drops `entity_ref` if both fields present — consider validation at load. | ### Tyre (Architecture): REQUEST_CHANGES | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `bridge/types.rs:87` | **warning** | `ObserverSnapshot.version` doc says "Current: 19" — should be 20. (Same as Hoshe #4.) | | 2 | `simulation/monologue.rs` | suggestion | `trigger_monologue` is a no-op stub still registered in the schedule — remove from registration or annotate lifecycle. | | 3 | `simulation/monologue.rs` | suggestion | `trigger_event_monologue` doc still references removed `COOLDOWN_TICKS` constant. | ### Gestalt (Foundations): APPROVE All three new systems graded **SOLID**. Clean separation, graceful degradation at boundaries, determinism preserved (BTreeMap usage, seed-driven RNG). Ready to build on. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `npc/blueprint.rs` | suggestion | Silent fallback when `modifier_hint` doesn't match any category — log for content debugging. | | 2 | `npc/blueprint.rs` | suggestion | `OffDuty` context has no test coverage in filter tests. | | 3 | `settings/mod.rs` | suggestion | "Last response wins" batching — multi-change same-tick acks are incomplete. Worth a comment. | | 4 | `settings/store.rs` | suggestion | Unknown `value_type` silently converts to String instead of logging. | | 5 | `voice/integration.rs` | suggestion | Re-voicing heavily occluded text may produce incoherent output — accepted risk, worth a comment. | | 6 | `voice/worker.rs` | suggestion | Dead child process causes repeated warnings without recovery — consider respawn or loop break. | | 7 | `voice/prompt_builder.rs` | suggestion | Multiple INJECT blocks indistinguishable to model if multiple injections fire same prompt. | --- ### Verdict: **CHANGES REQUESTED** **Must-fix (2 items):** 1. **Protocol version comment** — `bridge/types.rs:87` says "Current: 19", should be 20. 2. **Delete response contract** — `settings/mod.rs` returns `success: true` with `error: Some(...)` when key not found. Pick one: `success: false`, or silent no-op. Everything else is suggestion-level and can be addressed in follow-up tickets.

jpmschweitzer referenced this issue from a commit

2026-03-13 09:34:40 +01:00

fix(simulation): address PR #89 review — version comment, settings contract, docs, dead process recovery

jpmschweitzer added 2 commits 2026-03-13 09:34:40 +01:00

fix(simulation): address PR review items for #655 content removal b37583ddfc

- line_pool.rs: add trace! when active_situations is empty (Layer 2
  will silently filter all lines — important for content debugging)
- knowledge_grant.rs: document serde(untagged) ambiguity hazard; add
  KnowledgeGrant::validate() for load-time field validation
- monologue.rs: annotate trigger_monologue as schedule ordering anchor;
  fix stale "periodic trigger cooldown tracking" doc reference in
  trigger_event_monologue (COOLDOWN_TICKS removed in #655)

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

fix(simulation): address PR #89 review — version comment, settings contract, docs, dead process recovery 51414b0b63

Must-fix: protocol version comment now references PROTOCOL_VERSION
(no hardcoded number), settings delete is idempotent no-op.

Suggestions addressed: BehaviorModifier dedup claim dropped, hash
collision safety documented, FK pragma in test store, columns_to_value
consolidated, modifier_hint mismatch logging, OffDuty test coverage,
batching tradeoff documented, unknown value_type warning, occluded
text risk documented, dead child respawn in voice worker, INJECT
block ambiguity documented.

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

jpmschweitzer commented

2026-03-13 09:39:10 +01:00

Re-review: server → main (fix commits `b37583d`, `51414b0`)

Hoshe (Code Quality): APPROVE

All 16 items from the initial review verified resolved. One low-severity note: KnowledgeGrant::validate() doc implies it catches the serde(untagged) ambiguity, but it only validates empty strings. Not blocking — just a doc tightening opportunity.

Tyre (Architecture): APPROVE

All 3 items resolved. Version comment now references PROTOCOL_VERSION via intra-doc link — eliminates the recurring doc-drift class entirely. Monologue stub properly documented as schedule ordering anchor with clear removal criteria. Voice worker respawn logic is architecturally sound with correct one-shot semantics.

Verdict: APPROVED

Ready to merge.

## Re-review: server → main (fix commits `b37583d`, `51414b0`) ### Hoshe (Code Quality): APPROVE All 16 items from the initial review verified resolved. One low-severity note: `KnowledgeGrant::validate()` doc implies it catches the serde(untagged) ambiguity, but it only validates empty strings. Not blocking — just a doc tightening opportunity. ### Tyre (Architecture): APPROVE All 3 items resolved. Version comment now references `PROTOCOL_VERSION` via intra-doc link — eliminates the recurring doc-drift class entirely. Monologue stub properly documented as schedule ordering anchor with clear removal criteria. Voice worker respawn logic is architecturally sound with correct one-shot semantics. ### Verdict: **APPROVED** Ready to merge.

jpmschweitzer closed this pull request

2026-03-13 09:40:58 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#89

feat(simulation): Sprint 26 server — voice pipeline, settings, behavior engine, v0.1 cleanup #89

Summary

Stats

Test plan

Review: server → main (type: code + foundations)

Hoshe (Code Quality): REQUEST_CHANGES

Tyre (Architecture): REQUEST_CHANGES

Gestalt (Foundations): APPROVE

Verdict: CHANGES REQUESTED

Re-review: server → main (fix commits b37583d, 51414b0)

Hoshe (Code Quality): APPROVE

Tyre (Architecture): APPROVE

Verdict: APPROVED

Pull request closed

Re-review: server → main (fix commits `b37583d`, `51414b0`)