Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class replay recording/playback and session telemetry to the Fortress Rollback runtime, and strengthens docs↔wiki synchronization + validation to reduce drift and improve diagnostics.
Changes:
- Introduces replay domain model (
Replay,ReplayMetadata), P2P recording hooks, and a deterministicReplaySession(optionally validating checksums and emittingReplayDesync). - Adds a session telemetry API (
SessionTelemetry,TelemetryEvent,CollectingTelemetry) and integrates emission into P2P rollback/prediction/network-stats/frame-advance paths. - Hardens docs/wiki sync: sidebar coverage validation, deterministic output normalization, reciprocal SYNC header generation, improved hook diagnostics, and expanded tests.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wiki/_Sidebar.md | Adds wiki navigation entries for replay + telemetry pages. |
| wiki/Telemetry.md | Generated wiki mirror for telemetry guide. |
| wiki/Replay.md | Generated wiki mirror for replay guide. |
| wiki/Overview.md | Adjusts wiki spec overview table formatting. |
| src/telemetry.rs | Adds telemetry trait/events + collecting implementation and tests. |
| src/sync_layer/mod.rs | Adds helper for identifying players with incorrect predictions (telemetry support). |
| src/sessions/session_trait.rs | Updates session trait docs to include ReplaySession. |
| src/sessions/replay_session.rs | New replay playback session with optional checksum validation + tests. |
| src/sessions/p2p_session.rs | Integrates telemetry emission and replay recording into P2P session. |
| src/sessions/builder.rs | Adds builder options for recording + telemetry, and replay-session constructors + tests. |
| src/replay.rs | New replay model, deterministic serialization helpers, and internal recorder + tests. |
| src/prelude.rs | Re-exports replay/session telemetry types in the prelude. |
| src/lib.rs | Exposes replay module/types and adds FortressEvent::ReplayDesync + display tests. |
| scripts/tests/test_sync_wiki.py | Expands tests for sync-wiki SYNC headers, sidebar coverage, normalization, and fail-before-write behavior. |
| scripts/tests/test_check_sync_headers.py | Adds unit/integration tests for improved SYNC header diagnostics. |
| scripts/hooks/check-sync-headers.py | Improves diagnostics (case mismatch hints + remediation guidance). |
| scripts/docs/sync-wiki.py | Adds reciprocal SYNC header injection, deterministic EOF normalization, and sidebar coverage validation. |
| mkdocs.yml | Adds MkDocs nav entries for replay + telemetry docs. |
| docs/telemetry.md | New telemetry guide (docs source). |
| docs/replay.md | New replay guide (docs source). |
| CHANGELOG.md | Documents new replay/telemetry APIs and breaking ReplayDesync event. |
| .pre-commit-config.yaml | Adds sync-wiki hook before wiki consistency checks. |
| .llm/skills/ci-cd-tooling/wiki-sync.md | Updates internal wiki sync guidance to reflect new enforcement/behavior. |
| .gitignore | Ignores local pre-commit/pre-push log artifacts. |
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Fortress Rollback Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 3af8486 | Previous: f520cca | Ratio |
|---|---|---|---|
Message serialization/input_deserialize |
1244 ns/iter (± 1) |
681 ns/iter (± 2) |
1.83 |
Message serialization/input_encode_into_buffer |
1557 ns/iter (± 99) |
871 ns/iter (± 14) |
1.79 |
This comment was automatically generated by workflow using github-action-benchmark.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR delivers a full replay + telemetry feature set and hardens docs/wiki synchronization.
It solves two core gaps:
What changed
Replay<I>andReplayMetadataReplay::to_bytes(),Replay::from_bytes(),Replay::validate(),Replay::total_frames()ReplaySession<T>implementingSession<T>ReplaySession::new()andReplaySession::new_with_validation()SaveGameStatebeforeAdvanceFrameand compares checksums frame-by-frameSessionBuilder::with_recording(bool)P2PSession::is_recording(),P2PSession::into_replay(),P2PSession::take_replay()SessionTelemetrytraitTelemetryEventenum (Rollback,PredictionMiss,NetworkStatsUpdate,FrameAdvance)CollectingTelemetrySessionBuilder::with_telemetry(...)players_with_incorrect_predictions(...)lib.rsand preludesync-wikihook before wiki consistency checkssync-wiki.pynow:check-sync-headers.pynow reports case-mismatch hints and remediation guidance.gitignoreentries for local pre-commit/pre-push log artifactsBreaking change
FortressEvent::ReplayDesync { frame, expected_checksum, actual_checksum }.FortressEventis not#[non_exhaustive], downstream exhaustivematchstatements must add a branch forReplayDesync.Type of Change
Checklist
Required
unwrap()in production codeexpect()in production codepanic!()ortodo!()Resultcargo fmt && cargo clippy --all-targets --features tokio,jsonwith no warningscargo nextest runand all tests passIf Applicable
CHANGELOG.mdfor user-facing changesexamples/directoryTesting
Tests added/modified:
src/replay.rs(serialization roundtrip, validation invariants, metadata/display, recorder behavior)src/sessions/replay_session.rs(playback flow, completion behavior, validation mode, checksum mismatch event emission)ReplayDesyncinsrc/lib.rssrc/sessions/builder.rsandsrc/sessions/p2p_session.rsfor replay-session creation and recording API behaviorscripts/tests/test_check_sync_headers.pyscripts/tests/test_sync_wiki.pycoverage for SYNC header generation, sidebar coverage validation, idempotent output normalization, and fail-before-write behaviorManual testing performed:
cargo fmt,cargo clippy,cargo nextest) intentionally left unchecked in checklist for final author confirmationRelated Issues