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

feat(ci): Sprint 12 — HashMap lint, rng_seed, tracing, schedule debug #41

Closed
jpmschweitzer wants to merge 0 commits from ci into main
pull from: ci
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-02-19 14:16:16 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 12 CI tasks — all 4 tickets done, 564/564 tests pass, clippy clean.

#343 — Ban HashMap in simulation via clippy disallowed_types

  • New server/.clippy.toml with disallowed-types for std::collections::HashMap and HashSet
  • simulation/movement.rs: WalkabilityMap.chunks and occupied map → BTreeMap; added PartialOrd+Ord to ChunkCoord, TilePosition, TilePresence
  • simulation/monologue.rs: MonologueState.shown_ids → BTreeSet (was simulation state)
  • Perception pipeline files (shadowcast.rs, interpretation.rs, query.rs): #![allow(clippy::disallowed_types)] — per-frame scratch buffers where iteration order is irrelevant
  • CI fails on new violations after this PR

#527 — Add rng_seed to ObserverSnapshot (protocol v10)

  • bridge/types.rs: PROTOCOL_VERSION 9→10, rng_seed: Option<u64> with #[serde(default, skip_serializing_if = "Option::is_none")]
  • perception/observer/mod.rs: inject Res<SimRng>, populate rng_seed: Some(rng.seed()) each tick
  • All test constructors updated; msgpack fixtures and golden file regenerated
  • Completes WRONG button capture loop — replays now have the exact RNG seed

#344 — Tracing crate infrastructure

  • tracing-subscriber gains json feature
  • JSON format emitted when CI=true or RUST_LOG_FORMAT=json (structured log ingestion)
  • tracing::debug!(tick_ms, budget_ms, over_budget) on each game tick

#346 — Schedule dependency graph debug command

  • --dump-schedule CLI flag prints bevy_ecs registered schedules (7 schedules, Update has 23 systems) and exits — no TCP bridge required
  • make debug-schedule Makefile target for CI artifact generation and baseline diffing

Test plan

  • cargo clippy -- -D warnings clean
  • cargo fmt --check clean
  • cargo nextest run — 564/564 pass, 3 skipped
  • cargo run -- --dump-schedule — outputs schedule graph correctly
  • Fixtures and golden file regenerated for protocol v10

🤖 Generated with Claude Code

## Summary Sprint 12 CI tasks — all 4 tickets done, 564/564 tests pass, clippy clean. ### #343 — Ban HashMap in simulation via clippy disallowed_types - New `server/.clippy.toml` with `disallowed-types` for `std::collections::HashMap` and `HashSet` - `simulation/movement.rs`: `WalkabilityMap.chunks` and `occupied` map → `BTreeMap`; added `PartialOrd+Ord` to `ChunkCoord`, `TilePosition`, `TilePresence` - `simulation/monologue.rs`: `MonologueState.shown_ids` → `BTreeSet` (was simulation state) - Perception pipeline files (`shadowcast.rs`, `interpretation.rs`, `query.rs`): `#![allow(clippy::disallowed_types)]` — per-frame scratch buffers where iteration order is irrelevant - CI fails on new violations after this PR ### #527 — Add rng_seed to ObserverSnapshot (protocol v10) - `bridge/types.rs`: `PROTOCOL_VERSION` 9→10, `rng_seed: Option<u64>` with `#[serde(default, skip_serializing_if = "Option::is_none")]` - `perception/observer/mod.rs`: inject `Res<SimRng>`, populate `rng_seed: Some(rng.seed())` each tick - All test constructors updated; msgpack fixtures and golden file regenerated - Completes WRONG button capture loop — replays now have the exact RNG seed ### #344 — Tracing crate infrastructure - `tracing-subscriber` gains `json` feature - JSON format emitted when `CI=true` or `RUST_LOG_FORMAT=json` (structured log ingestion) - `tracing::debug!(tick_ms, budget_ms, over_budget)` on each game tick ### #346 — Schedule dependency graph debug command - `--dump-schedule` CLI flag prints bevy_ecs registered schedules (7 schedules, Update has 23 systems) and exits — no TCP bridge required - `make debug-schedule` Makefile target for CI artifact generation and baseline diffing ## Test plan - [x] `cargo clippy -- -D warnings` clean - [x] `cargo fmt --check` clean - [x] `cargo nextest run` — 564/564 pass, 3 skipped - [x] `cargo run -- --dump-schedule` — outputs schedule graph correctly - [x] Fixtures and golden file regenerated for protocol v10 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer commented 2026-02-19 15:06:29 +01:00
Author
Owner
Copy Link
Copy Source

Review: ci -> main (type: code)

Hoshe (Code Quality): APPROVE

Well-structured PR delivering four tickets cleanly. HashMap-to-BTreeMap migration is correct with principled clippy enforcement and documented exceptions. Tracing infrastructure is clean with JSON toggle via CI env var. rng_seed field completes the WRONG button replay loop with correct serde annotations. 566/566 tests pass.

# File Severity Issue
1 server/src/main.rs:187 warning Duplicate doc comment on dump_schedule_graph Fixed in e047218
2 server/tests/serialization.rs warning Missing rng_seed round-trip test and v9-to-v10 backward compat test Fixed in e047218
3 server/src/main.rs:109 suggestion Bare .unwrap() on local_addr() — pre-existing, not introduced by this PR
4 server/src/perception/query.rs suggestion sector_lookup HashMap exemption could use an inline field comment

Tyre (Architecture): APPROVE

Architecturally sound and aligned with D-010, D-041, D-030. HashMap ban is correct for deterministic simulation. The #![allow] exemptions in perception/ are defensible — per-frame scratch buffers where iteration order is irrelevant. rng_seed addition is clean protocol design. Tracing is solid: JSON on CI, human-readable locally, stderr always clean for LISTENING handshake.

# File Severity Issue
1 server/src/main.rs:187 suggestion Duplicate doc comment Fixed in e047218
2 server/src/main.rs suggestion Arg parsing skip_next list could use a maintenance comment
3 server/.clippy.toml + Makefile warning No committed schedule baseline file — doc comment oversells CI integration. Consider follow-on ticket
4 server/src/simulation/rng.rs suggestion seed() returns initial seed not stream position — correct for replay, noted for future optimization

Verdict: APPROVED

Both reviewers approve. Warning-level issues from initial review have been addressed in commit e047218 (duplicate doc comment removed, serialization tests added). Remaining items are suggestions only.

## Review: ci -> main (type: code) ### Hoshe (Code Quality): APPROVE Well-structured PR delivering four tickets cleanly. HashMap-to-BTreeMap migration is correct with principled clippy enforcement and documented exceptions. Tracing infrastructure is clean with JSON toggle via CI env var. rng_seed field completes the WRONG button replay loop with correct serde annotations. 566/566 tests pass. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | server/src/main.rs:187 | warning | ~~Duplicate doc comment on dump_schedule_graph~~ **Fixed in e047218** | | 2 | server/tests/serialization.rs | warning | ~~Missing rng_seed round-trip test and v9-to-v10 backward compat test~~ **Fixed in e047218** | | 3 | server/src/main.rs:109 | suggestion | Bare .unwrap() on local_addr() — pre-existing, not introduced by this PR | | 4 | server/src/perception/query.rs | suggestion | sector_lookup HashMap exemption could use an inline field comment | ### Tyre (Architecture): APPROVE Architecturally sound and aligned with D-010, D-041, D-030. HashMap ban is correct for deterministic simulation. The `#![allow]` exemptions in perception/ are defensible — per-frame scratch buffers where iteration order is irrelevant. rng_seed addition is clean protocol design. Tracing is solid: JSON on CI, human-readable locally, stderr always clean for LISTENING handshake. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | server/src/main.rs:187 | suggestion | ~~Duplicate doc comment~~ **Fixed in e047218** | | 2 | server/src/main.rs | suggestion | Arg parsing skip_next list could use a maintenance comment | | 3 | server/.clippy.toml + Makefile | warning | No committed schedule baseline file — doc comment oversells CI integration. Consider follow-on ticket | | 4 | server/src/simulation/rng.rs | suggestion | seed() returns initial seed not stream position — correct for replay, noted for future optimization | ### Verdict: APPROVED Both reviewers approve. Warning-level issues from initial review have been addressed in commit e047218 (duplicate doc comment removed, serialization tests added). Remaining items are suggestions only.
jpmschweitzer added 7 commits 2026-02-19 15:07:17 +01:00
feat(ci): ban HashMap in simulation crate via clippy (#343) 0f5a990f82 
Add clippy::disallowed_types for std::collections::HashMap scoped to
the simulation crate. Replace HashMap with BTreeMap in movement.rs for
deterministic iteration order. Allow exception in perception/query.rs
where iteration order is irrelevant (per-frame scratch buffer).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(engine): add rng_seed to ObserverSnapshot for deterministic replay (#527) 9ea9fd2a74 
Add rng_seed: Option<u64> to ObserverSnapshot (protocol v10). Populated
from SimRng state each tick. Completes the WRONG button capture loop —
seed.txt now writes a valid u64 instead of "unavailable", enabling
deterministic replay from bug reports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(simulation): ban HashMap via clippy disallowed_types, fix violations (#343) 3248603838 
Adds server/.clippy.toml with disallowed-types for std::collections::HashMap
and std::collections::HashSet. HashMap iteration order is non-deterministic
and breaks deterministic simulation replay (D-030).

Changes:
- server/.clippy.toml: disallow HashMap and HashSet crate-wide
- simulation/movement.rs: WalkabilityMap.chunks and occupied map → BTreeMap;
  add PartialOrd+Ord to ChunkCoord, TilePosition, TilePresence
- simulation/monologue.rs: MonologueState.shown_ids → BTreeSet (simulation state)
- perception/shadowcast.rs: #![allow] — per-frame FOV scratch, iteration irrelevant
- perception/interpretation.rs: #![allow] — per-frame lookup table, key-only access
- perception/query.rs: #![allow] — sector_lookup is a per-frame read-only cache

Also applies cargo fmt to pre-existing format drift in contraband.rs,
dialogue.rs, test_world/mod.rs, and several integration tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(bridge): add rng_seed to ObserverSnapshot for deterministic replay (#527) 6a7dc915de 
Adds rng_seed: Option<u64> to ObserverSnapshot. The WRONG button (#507) captures
inputs.jsonl and seed.txt for replay, but seed.txt was writing "unavailable"
because the server did not include the RNG seed in ObserverSnapshot.

Changes:
- bridge/types.rs: PROTOCOL_VERSION 9→10, rng_seed field with serde(default,
  skip_serializing_if = "Option::is_none") for backward compatibility
- perception/observer/mod.rs: inject Res<SimRng> into compute_observer_snapshot,
  populate rng_seed: Some(rng.seed()) each tick
- All test files: add rng_seed: None to ObserverSnapshot constructors
- tests/serialization.rs: bump protocol_version_constant assertion 9→10
- Regenerate msgpack fixtures and golden file for protocol v10

Completes the WRONG button capture loop: replays can now fully reproduce
observed bugs with the exact RNG seed from the capture.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(ci): tracing JSON format in CI, tick duration logging, schedule dump (#344, #346) a71218f6bc 
Tracing (#344):
- Add 'json' feature to tracing-subscriber dependency
- Emit JSON log format when CI=true or RUST_LOG_FORMAT=json is set
  (structured log ingestion in CI pipelines)
- Add tracing::debug! with tick_ms/budget_ms/over_budget fields on each
  tick for performance profiling and tier system debugging prerequisite

Schedule dump (#346):
- Add --dump-schedule CLI flag that prints bevy_ecs schedule graph and exits
  without requiring TCP bridge or world setup
- Add make debug-schedule target for CI artifact generation and diff-based
  regression detection of unintended system reordering

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(ci): add tracing::instrument to heavy per-tick systems, fix stale protocol comment (#344, #527) 3d306b8a2b 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(ci): remove duplicate doc comment, add rng_seed serialization tests (#344, #527) e047218ad2 
Review fixes: deduplicate dump_schedule_graph doc comment,
add rng_seed round-trip test and v9→v10 backward compat test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer closed this pull request 2026-02-19 15:37:36 +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#41