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

feat(server): Sprint 7 Talk — cognitive delay, line pool, YAML loader, eavesdrop, interaction memory #23

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

Summary

Sprint 7 server team deliverables — 5 tickets, all QA-reviewed and approved.

New systems

  • Cognitive delay (#423) — CognitiveDelay component buffers perception events (0.6s/0.3s), pending_recognitions in ObserverSnapshot v7, LOS exit cancellation
  • ListeningFocus (#426) — stationary_ticks eavesdrop positioning, stance-aware thresholds (30/20 ticks), Sprint blocks accumulation
  • YAML content loader (#326) — LinePool with BTreeMap indexing, 4-layer query filtering (D-028), hot-reload via timestamp polling (dev-only)

Architecture specs

  • Line pool format (#308) — docs/architecture/line-pool-format.md: YAML structure, tag enums, filtering pipeline, prerequisite-to-KG mapping
  • InteractionMemory schema (#442) — docs/architecture/interaction-memory-schema.md: FactMetadata extension, 5-state InteractionState, tolerance reconciliation

Stats

  • 389 tests, 0 failures (70 new tests)
  • Protocol v7 (pending_recognitions field)
  • Monologue schema fix: relationship prerequisite requires target+state

Follow-up tickets filed

  • #450: Wire RecognitionTrigger::Urgent to anomaly detection
  • #451: Monologue fires DURING cognitive delay window (D-060)

QA reviews (Hoshe)

  • #308: APPROVE (2 should-fix applied)
  • #423: APPROVE (cancellation fix applied, Urgent deferred to #450)
  • #326: APPROVE (clean, all enums verified against 3 sources)
  • #442: APPROVE (3 clarifications added)
  • #426: APPROVE (2 bugs fixed: return→continue, missing spawn)
## Summary Sprint 7 server team deliverables — 5 tickets, all QA-reviewed and approved. ### New systems - **Cognitive delay (#423)** — CognitiveDelay component buffers perception events (0.6s/0.3s), pending_recognitions in ObserverSnapshot v7, LOS exit cancellation - **ListeningFocus (#426)** — stationary_ticks eavesdrop positioning, stance-aware thresholds (30/20 ticks), Sprint blocks accumulation - **YAML content loader (#326)** — LinePool with BTreeMap indexing, 4-layer query filtering (D-028), hot-reload via timestamp polling (dev-only) ### Architecture specs - **Line pool format (#308)** — docs/architecture/line-pool-format.md: YAML structure, tag enums, filtering pipeline, prerequisite-to-KG mapping - **InteractionMemory schema (#442)** — docs/architecture/interaction-memory-schema.md: FactMetadata extension, 5-state InteractionState, tolerance reconciliation ### Stats - 389 tests, 0 failures (70 new tests) - Protocol v7 (pending_recognitions field) - Monologue schema fix: relationship prerequisite requires target+state ### Follow-up tickets filed - #450: Wire RecognitionTrigger::Urgent to anomaly detection - #451: Monologue fires DURING cognitive delay window (D-060) ### QA reviews (Hoshe) - #308: APPROVE (2 should-fix applied) - #423: APPROVE (cancellation fix applied, Urgent deferred to #450) - #326: APPROVE (clean, all enums verified against 3 sources) - #442: APPROVE (3 clarifications added) - #426: APPROVE (2 bugs fixed: return→continue, missing spawn)
jpmschweitzer added 7 commits 2026-02-16 00:42:41 +01:00
refactor(server): apply rustfmt and clippy suggestions d0663c4193 
Formatting pass across simulation, perception, knowledge, NPC, and
test modules. Includes two clippy fixes in monologue.rs (.values()
instead of for (_, v) pattern).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add ListeningFocus eavesdrop positioning (#426) 3a2cfc37e0 
New ListeningFocus component tracks stationary_ticks for eavesdrop
mechanic. Increments when position unchanged, resets on movement.
Sprint stance blocks accumulation, Careful reduces threshold from
30 to 20 ticks. Registered in SimulationPlugin after validate_movement.

17 tests covering all stances, thresholds, and edge cases.

Ref: D-053 (stance system), D-018 (sound model)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(perception): add cognitive delay system for fog recognition (#423) 4d8fe996fc 
New CognitiveDelay component buffers perception events before emitting
KnowledgeEvents. Normal delay: 6 ticks (0.6s), urgent: 3 ticks (0.3s).
Drain system runs after emit_observation_events, before process_knowledge_events.

Adds pending_recognitions to ObserverSnapshot (protocol v7) for client
fog entity visualization. Includes cancellation when entity leaves LOS.

Monologue fires during delay (not after) per D-060 — cross-system
wiring deferred to follow-up ticket #451.

Ref: D-060 (cognitive delay), D-031 (10 tps), D-059 (fog layers)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
docs(architecture): add line pool format specification (#308) ea0a3f0b35 
Formal spec at docs/architecture/line-pool-format.md defining YAML
structure for dialogue and monologue content files. Covers tag enums,
4-layer filtering pipeline, prerequisite-to-KG mapping, ID format,
validation rules, and Rust loader interface.

Fixes monologue schema: adds required constraint on relationship
prerequisite target/state fields.

Ref: D-028, D-032, D-035, D-041

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): add YAML content loader with hot-reload (#326) 91d6b1b1cb 
LinePool system parses dialogue.yaml and monologue.yaml into indexed
BTreeMap structures. Supports 4-layer query filtering (access >
situation > trust > topic+mood) per D-028. Hot-reload via timestamp
polling every 20 ticks (dev-only). Graceful failure preserves
previous content on reload error.

30+ tests covering enum parsing, index building, query filtering,
monologue fallback, and content watching.

Ref: D-028, D-032, D-035, D-041

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
docs(architecture): add interaction memory KG schema design (#442) 087ff67bfb 
Design doc at docs/architecture/interaction-memory-schema.md extending
FactKnowledge with interaction tracking. FactMetadata enum on existing
BTreeMap, 5-state InteractionState (Active/Completed/WalkedAway/
Confrontation/ConflictAbandoned), monologue prerequisite extension.

Includes entity slug resolution approach, NpcTolerance reconciliation,
and fact decay exemption documentation.

Ref: D-041, D-064, D-063

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog 8a334b47f3 
Sprint 7 server entries: cognitive delay, ListeningFocus, YAML loader,
line pool spec, interaction memory schema.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-16 00:55:20 +01:00
Author
Owner
Copy Link
Copy Source

Review: server → main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Well-structured PR with comprehensive test coverage (37 tests across new modules), zero unsafe code, and good error handling. However, 3 critical bugs must be fixed before merge.

# File Severity Issue
1 server/tests/serialization.rs critical Missing roundtrip test for PendingRecognitionWire. Protocol v7 adds pending_recognitions to ObserverSnapshot but no test verifies it survives MessagePack serialization. Wire format regression risk.
2 server/src/perception/observation.rs:139 critical delay.cancel(&sid) can return None but code logs success unconditionally. Misleading logs when entity leaves LOS but was never pending. Check the Option return.
3 server/src/simulation/input.rs:47 critical InputQueue has unbounded growth — no capacity limit on VecDeque::push_back(). Client flood or stalled ticks → OOM. Add capacity limit (~1000) with drop-oldest or reject.
4 server/src/content/hot_reload.rs:86 warning System spins forever if content root path is invalid. Guard against zero-file watcher insertion.
5 server/src/content/line_pool.rs warning No validation that location header matches parent directory name (spec Section 3.2 requires it). Silent wrong data on misplaced files.
6 server/src/perception/observation.rs:84 suggestion TODO comment without ticket reference — convert to tracked ticket.
7 server/src/content/line_pool.rs:356 suggestion parse_tags() should log the invalid value itself, not just the error message.
8 server/src/simulation/listening.rs:97 suggestion Eavesdrop silently cancels on Sprint stance change — add debug log.
9 server/src/content/hot_reload.rs:49 suggestion walk_yaml() recursive traversal has no depth limit. Add ~100 level cap.
10 server/src/perception/cognitive_delay.rs:104 suggestion is_pending() is O(n) — consider HashSet<StableId> for O(1) lookup at scale.
11 server/src/simulation/input.rs:241 suggestion handle_take()/handle_place() don't emit knowledge events. KG won't know about inventory changes.
12 server/src/content/line_pool.rs:689 suggestion Missing negative test for prerequisite filtering (line excluded when fact absent).
13 server/tests/observer suggestion No integration test for pending_recognitions field in compute_observer_snapshot.

Tyre (Architecture): APPROVE

Exceptionally solid architectural work. All systems respect D-010 determinism (BTreeMap everywhere), D-020 client-server separation (wire types properly isolated), and D-030 testability (37 tests, no test-only production paths). Documentation is specification-grade. System ordering is explicit and correct.

# File Severity Issue
1 server/src/bridge/types.rs:7 critical PROTOCOL_VERSION bumped to 7 — verify client branch has matching update before merge. Mismatch causes silent data corruption.
2 docs/architecture/interaction-memory-schema.md:78 warning ContentSlug component prerequisite doesn't exist yet — no ticket reference. Create ticket for Sprint 8.
3 server/src/simulation/listening.rs:22 warning ListeningFocus.last_position is 12-byte overhead per entity. Fine for singleton player, won't scale to NPCs. Flag for v0.2.
4 server/src/perception/cognitive_delay.rs:23 suggestion Add playtesting expectation comment on delay constants.
5 server/src/simulation/listening.rs:13 suggestion Add validation test: Careful threshold < default threshold.
6 server/src/content/hot_reload.rs:20 suggestion Expose CHECK_INTERVAL_TICKS via ContentConfig for authoring workflow tuning.
7 docs/architecture/line-pool-format.md:§5 suggestion Add regex validation rule for ID format.
8 server/src/content/loader.rs:71 suggestion Clarify comment — BTreeMap vs sort() ordering guarantee.
9 server/src/npc/relationships.rs suggestion TODO(v0.2) for multiplayer needs ticket reference.
10 server/src/content/line_pool.rs suggestion 1078-line file could split into submodules if tag parsing grows.
11 server/src/content/hot_reload.rs:99 suggestion Add consecutive failure counter — warn after 5+ reload failures.
12 docs/architecture/interaction-memory-schema.md:132 suggestion Verify Option A recommendation section is complete with trade-off analysis.
13 server/src/perception/cognitive_delay.rs:95 suggestion Edge case: pending recognition target despawned before delay expires — no cancellation.

Decision compliance: D-010 ✅ | D-020 ✅ | D-026 ✅ | D-030 ✅ | D-041 ✅ | D-060 ✅

Verdict: CHANGES REQUESTED

Must fix (4 critical):

  1. Missing PendingRecognitionWire serialization test
  2. Silent error in cognitive delay cancellation
  3. Unbounded InputQueue growth
  4. Verify client protocol v7 match before merge

Should fix (3 warnings):

  • Hot-reload spins on invalid content root
  • Line pool location header not validated against directory
  • ContentSlug prerequisite needs a ticket
## Review: server → main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Well-structured PR with comprehensive test coverage (37 tests across new modules), zero unsafe code, and good error handling. However, 3 critical bugs must be fixed before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | server/tests/serialization.rs | critical | Missing roundtrip test for `PendingRecognitionWire`. Protocol v7 adds `pending_recognitions` to `ObserverSnapshot` but no test verifies it survives MessagePack serialization. Wire format regression risk. | | 2 | server/src/perception/observation.rs:139 | critical | `delay.cancel(&sid)` can return `None` but code logs success unconditionally. Misleading logs when entity leaves LOS but was never pending. Check the `Option` return. | | 3 | server/src/simulation/input.rs:47 | critical | `InputQueue` has unbounded growth — no capacity limit on `VecDeque::push_back()`. Client flood or stalled ticks → OOM. Add capacity limit (~1000) with drop-oldest or reject. | | 4 | server/src/content/hot_reload.rs:86 | warning | System spins forever if content root path is invalid. Guard against zero-file watcher insertion. | | 5 | server/src/content/line_pool.rs | warning | No validation that `location` header matches parent directory name (spec Section 3.2 requires it). Silent wrong data on misplaced files. | | 6 | server/src/perception/observation.rs:84 | suggestion | TODO comment without ticket reference — convert to tracked ticket. | | 7 | server/src/content/line_pool.rs:356 | suggestion | `parse_tags()` should log the invalid value itself, not just the error message. | | 8 | server/src/simulation/listening.rs:97 | suggestion | Eavesdrop silently cancels on Sprint stance change — add debug log. | | 9 | server/src/content/hot_reload.rs:49 | suggestion | `walk_yaml()` recursive traversal has no depth limit. Add ~100 level cap. | | 10 | server/src/perception/cognitive_delay.rs:104 | suggestion | `is_pending()` is O(n) — consider `HashSet<StableId>` for O(1) lookup at scale. | | 11 | server/src/simulation/input.rs:241 | suggestion | `handle_take()`/`handle_place()` don't emit knowledge events. KG won't know about inventory changes. | | 12 | server/src/content/line_pool.rs:689 | suggestion | Missing negative test for prerequisite filtering (line excluded when fact absent). | | 13 | server/tests/observer | suggestion | No integration test for `pending_recognitions` field in `compute_observer_snapshot`. | ### Tyre (Architecture): APPROVE Exceptionally solid architectural work. All systems respect D-010 determinism (BTreeMap everywhere), D-020 client-server separation (wire types properly isolated), and D-030 testability (37 tests, no test-only production paths). Documentation is specification-grade. System ordering is explicit and correct. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | server/src/bridge/types.rs:7 | critical | `PROTOCOL_VERSION` bumped to 7 — verify client branch has matching update before merge. Mismatch causes silent data corruption. | | 2 | docs/architecture/interaction-memory-schema.md:78 | warning | `ContentSlug` component prerequisite doesn't exist yet — no ticket reference. Create ticket for Sprint 8. | | 3 | server/src/simulation/listening.rs:22 | warning | `ListeningFocus.last_position` is 12-byte overhead per entity. Fine for singleton player, won't scale to NPCs. Flag for v0.2. | | 4 | server/src/perception/cognitive_delay.rs:23 | suggestion | Add playtesting expectation comment on delay constants. | | 5 | server/src/simulation/listening.rs:13 | suggestion | Add validation test: Careful threshold < default threshold. | | 6 | server/src/content/hot_reload.rs:20 | suggestion | Expose `CHECK_INTERVAL_TICKS` via `ContentConfig` for authoring workflow tuning. | | 7 | docs/architecture/line-pool-format.md:§5 | suggestion | Add regex validation rule for ID format. | | 8 | server/src/content/loader.rs:71 | suggestion | Clarify comment — BTreeMap vs `sort()` ordering guarantee. | | 9 | server/src/npc/relationships.rs | suggestion | TODO(v0.2) for multiplayer needs ticket reference. | | 10 | server/src/content/line_pool.rs | suggestion | 1078-line file could split into submodules if tag parsing grows. | | 11 | server/src/content/hot_reload.rs:99 | suggestion | Add consecutive failure counter — warn after 5+ reload failures. | | 12 | docs/architecture/interaction-memory-schema.md:132 | suggestion | Verify Option A recommendation section is complete with trade-off analysis. | | 13 | server/src/perception/cognitive_delay.rs:95 | suggestion | Edge case: pending recognition target despawned before delay expires — no cancellation. | **Decision compliance:** D-010 ✅ | D-020 ✅ | D-026 ✅ | D-030 ✅ | D-041 ✅ | D-060 ✅ --- ### Verdict: CHANGES REQUESTED **Must fix (4 critical):** 1. Missing `PendingRecognitionWire` serialization test 2. Silent error in cognitive delay cancellation 3. Unbounded `InputQueue` growth 4. Verify client protocol v7 match before merge **Should fix (3 warnings):** - Hot-reload spins on invalid content root - Line pool location header not validated against directory - `ContentSlug` prerequisite needs a ticket
jpmschweitzer commented 2026-02-16 00:58:51 +01:00
Author
Owner
Copy Link
Copy Source

Protocol v7 verification: client branch has not been updated yet. Client PR should update PROTOCOL_VERSION to 7 and handle pending_recognitions field (with serde default for backward compatibility). Server uses #[serde(default)] on pending_recognitions so existing v6 clients won't break — they just won't see fog entity data until updated.

Protocol v7 verification: client branch has not been updated yet. Client PR should update PROTOCOL_VERSION to 7 and handle pending_recognitions field (with serde default for backward compatibility). Server uses #[serde(default)] on pending_recognitions so existing v6 clients won't break — they just won't see fog entity data until updated.
jpmschweitzer added 1 commit 2026-02-16 01:04:06 +01:00
fix(server): address PR #23 review — 4 critical bugs, 3 warnings, 11 suggestions d4fdbf426e 
Critical fixes:
- Add PendingRecognitionWire serialization roundtrip test
- Check Option return from delay.cancel() before logging
- InputQueue capacity limit (1000) with drop-oldest and warning
- TODO in observation.rs references ticket #450

Warning fixes:
- Hot-reload guards against invalid/empty content root
- Location header mismatch warning in line pool indexing
- walk_yaml() depth limit (100) against symlink loops
- Consecutive reload failure counter (warns after 5+)

Test additions:
- Negative prerequisite filtering test for monologue lines
- Integration test for pending_recognitions in observer snapshot
- Eavesdrop threshold ordering assertion (Careful < default)

Documentation:
- Playtesting expectation comments on delay constants
- is_pending() scalability note for future NPC cognitive delay
- ID format regex validation in line pool spec
- BTreeMap vs sort() ordering clarification in loader
- Multiplayer TODO in relationships.rs references D-010
- ContentSlug ticket #452 filed for entity slug resolution

394 tests, 0 failures.

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

Re-review: fix commit d4fdbf4

Criticals

# Issue Status
H1 Missing PendingRecognitionWire serialization test Fixed
H2 Silent error in delay.cancel() Fixed
H3 Unbounded InputQueue growth Fixed — cap 1000, drop-oldest
T1 Client protocol v7 mismatch NOT FIXED — client still at v6

Warnings

# Issue Status
H4 Hot-reload invalid root Fixed
H5 Location header vs directory validation Partial — empty check only
T2 ContentSlug ticket Fixed — #452

11 of 19 suggestions addressed

Remaining suggestions are low-priority and can be deferred.

Verdict: BLOCKED on client protocol version bump

Client branch has PROTOCOL_VERSION = 6, server has 7. Must sync before merge to avoid wire corruption.

## Re-review: fix commit d4fdbf4 ### Criticals | # | Issue | Status | |---|-------|--------| | H1 | Missing PendingRecognitionWire serialization test | **Fixed** | | H2 | Silent error in delay.cancel() | **Fixed** | | H3 | Unbounded InputQueue growth | **Fixed** — cap 1000, drop-oldest | | T1 | Client protocol v7 mismatch | **NOT FIXED** — client still at v6 | ### Warnings | # | Issue | Status | |---|-------|--------| | H4 | Hot-reload invalid root | **Fixed** | | H5 | Location header vs directory validation | **Partial** — empty check only | | T2 | ContentSlug ticket | **Fixed** — #452 | ### 11 of 19 suggestions addressed Remaining suggestions are low-priority and can be deferred. ### Verdict: **BLOCKED on client protocol version bump** Client branch has `PROTOCOL_VERSION = 6`, server has `7`. Must sync before merge to avoid wire corruption.
jpmschweitzer closed this pull request 2026-02-16 01:08:35 +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#23