chore(repo): land cardshed blueprint and reconcile prp turnstate (#137)#138
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @w7-mgfcode, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR reconciles the CARD SHED PRP bundle's turn-state vocabulary and lands the comprehensive technical blueprint. PRP-02 and PRP-03 are updated to use the canonical TurnState type, and the blueprint document is committed to formalize the upstream decisions for stack, architecture, synchronization, and roadmap. ChangesTurnState Vocabulary Reconciliation
CARD SHED Technical Blueprint
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Align PRPs/02-deterministic-core.md and PRPs/03-experience-distribution.md
with the canonical TurnState vocabulary locked in PRPs/cardshed-01-blueprint.md:
TurnState = "Dealing" | "AwaitingAttack" | "AwaitingDefense"
| "Resolving" | "RoundEnded" | "MatchEnded"
Replaces the older TurnPhase shared-contract block in both PRPs and renames
pseudocode tokens (WaitingForAttack -> AwaitingAttack, RoundOver -> RoundEnded,
etc.). GameEvent and WebSocket message enums are intentionally untouched —
they share a namespace boundary with TurnState and are not part of this rename.
Blueprint already canonical, no edit required there.
Adds PRPs/cardshed-01-blueprint.md — the executed output of PRPs/01-strategy-blueprint.md. Locks the upstream decisions that the sibling PRPs (02-deterministic-core.md, 03-experience-distribution.md) consume: - Backend: Rust + Axum (over Go) — sum-type modelling of the rule space - Frontend: TS + React + Vite + Tailwind v4 + Radix (W7 ecosystem) - Sync model: hybrid (per-player redacted snapshot + ordered events, seq-keyed) - 9-module taxonomy + deployment-target matrix for local -> online jump - 20-section deliverable outline with [Self]/[PRP-2]/[PRP-3] ownership tags - 10 cross-PRP implications (seq numbers, seeded RNG, redaction, etc.) Canonical TurnState vocabulary in this file matches the #136 reconciliation.
w7-mgfcode
force-pushed
the
feat/reliquary-bootstrap
branch
from
cf86209 to
a47bf5a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PRPs/cardshed-01-blueprint.md`:
- Around line 129-130: The blueprint is ambiguous about whether clients send
server seq or clientSeq; standardize the contract so the server is the sole
emitter of a monotonic seq and clients never send server seq: update the
wire-shape for Action and Resume to require clients to send clientSeq (for
Action) and lastSeenSeq (for Resume) while removing any use of seq from
client-to-server messages, and document that ActionResult from server includes
the authoritative seq plus StateDelta and GameEvent[]; change references around
the Action, ActionResult, Resume, seq, clientSeq, and lastSeenSeq symbols to
reflect this single canonical rule to avoid PRP-2/PRP-3 drift.
- Around line 212-249: The markdown code fence containing the ASCII architecture
diagram is unlabeled which triggers markdownlint MD040; update the opening fence
from ``` to include a language hint such as ```text so the block is explicitly
labeled. Locate the unlabelled fence surrounding the diagram that includes
symbols like GameManager, Deck, TurnController, ActionValidator, RulesEngine,
StateReducer, EventBus, and UIHandler and change the fence to ```text (or
another appropriate tag) to silence the linter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a54fe650-7821-4bc2-b6df-6e9db41aa3ca
📒 Files selected for processing (3)
PRPs/02-deterministic-core.mdPRPs/03-experience-distribution.mdPRPs/cardshed-01-blueprint.md
Comment on lines
+129
to
+130
| | `Action` | public once applied; private while in-flight from a single client | wire envelope `{playerId, kind, payload, clientSeq}` | client → server | | ||
| | `ActionResult` | public | `Ok(StateDelta + GameEvent[])` \| `Err(RulesError)` | server → all | |
There was a problem hiding this comment.
Resolve seq vs clientSeq contract ambiguity before PRP handoff.
The blueprint currently conflicts on whether clients send seq (Action/Resume) or only clientSeq + lastSeenSeq. This will cause PRP-2/PRP-3 wire-shape drift. Define one canonical rule explicitly: server emits monotonic seq; clients send clientSeq (actions) and lastSeenSeq (resume), not server seq.
Suggested doc patch
-1. **Hybrid sync requires `seq: u64` on every message.** → [PRP-3] must include `seq` in every WebSocket envelope (`Snapshot`, `Event`, `Action`, `Resume`).
+1. **Hybrid sync requires server-issued `seq: u64` on all server→client envelopes.**
+ → [PRP-3] must include `seq` on `Snapshot` and `Event` (and any other server-originated envelope).
+ → Client-originated envelopes use `clientSeq` (`Action`) and `lastSeenSeq` (`Resume`) for idempotency/recovery.-- **Seq-number monotonicity** detects replay attacks; non-monotonic `seq` is rejected.
+- **Client idempotency monotonicity** detects replay/retry abuse; non-monotonic `clientSeq` is rejected per `(matchId, playerId)`.Also applies to: 195-200, 333-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PRPs/cardshed-01-blueprint.md` around lines 129 - 130, The blueprint is
ambiguous about whether clients send server seq or clientSeq; standardize the
contract so the server is the sole emitter of a monotonic seq and clients never
send server seq: update the wire-shape for Action and Resume to require clients
to send clientSeq (for Action) and lastSeenSeq (for Resume) while removing any
use of seq from client-to-server messages, and document that ActionResult from
server includes the authoritative seq plus StateDelta and GameEvent[]; change
references around the Action, ActionResult, Resume, seq, clientSeq, and
lastSeenSeq symbols to reflect this single canonical rule to avoid PRP-2/PRP-3
drift.
Comment on lines
+212
to
+249
| ``` | ||
| ┌──────────────────────────────┐ | ||
| │ GameManager │ | ||
| │ (lifecycle orchestrator) │ | ||
| └─────────────────┬────────────┘ | ||
| │ | ||
| ┌───────────────────────────────┼───────────────────────────────┐ | ||
| ▼ ▼ ▼ | ||
| ┌────────────────┐ ┌─────────────────────┐ ┌────────────────────┐ | ||
| │ Deck │ │ TurnController │ │ PersistenceAdapter │ | ||
| │ (cards+trump) │ │ (roles + rotation) │ │ (snapshot + events)│ | ||
| └────────┬───────┘ └──────────┬──────────┘ └─────────┬──────────┘ | ||
| │ │ │ | ||
| │ ┌───────────────▼──────────────┐ │ | ||
| │ │ ActionValidator │ │ | ||
| │ │ (schema-level gating) │ │ | ||
| │ └───────────────┬──────────────┘ │ | ||
| │ ▼ │ | ||
| │ ┌──────────────────────────────┐ │ | ||
| │ │ RulesEngine │ │ | ||
| │ │ (pure: state, action → res) │ │ | ||
| │ └───────────────┬──────────────┘ │ | ||
| │ ▼ │ | ||
| │ ┌──────────────────────────────┐ │ | ||
| └───────────►│ StateReducer │◄──────────────┘ | ||
| │ (state' = apply(state, Δ)) │ | ||
| └───────────────┬──────────────┘ | ||
| ▼ | ||
| ┌──────────────────────────────┐ | ||
| │ EventBus │ | ||
| │ (ordered GameEvent stream) │ | ||
| └───────────────┬──────────────┘ | ||
| ▼ | ||
| ┌──────────────────────────────┐ | ||
| │ UIHandler │ | ||
| │ (subscribes + dispatches) │ | ||
| └──────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language hint to the architecture code fence.
The fenced block is unlabeled; markdownlint MD040 will keep warning. Add a language tag (e.g., text) to stabilize docs lint.
Suggested doc patch
-```
+```text
...</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 212-212: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @PRPs/cardshed-01-blueprint.md around lines 212 - 249, The markdown code
fence containing the ASCII architecture diagram is unlabeled which triggers
markdownlint MD040; update the opening fence from to include a language hint such astext so the block is explicitly labeled. Locate the unlabelled fence
surrounding the diagram that includes symbols like GameManager, Deck,
TurnController, ActionValidator, RulesEngine, StateReducer, EventBus, and
UIHandler and change the fence to ```text (or another appropriate tag) to
silence the linter.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
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
1 participant
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.
Summary
After PR #135 squash-merged the reliquary bootstrap and the CARD SHED PRP bundle onto
master, this PR carries the two remaining net-new doc deliverables:a551a4d(chore(repo): reconcile cardshed prp turnstate vocabulary #136) — reconcilePRPs/02-deterministic-core.mdandPRPs/03-experience-distribution.mdonto the canonicalTurnStatevocabulary the executed blueprint locked. Replaces the olderTurnPhaseshared-contract block in both PRPs and renames pseudocode tokens (WaitingForAttack→AwaitingAttack,RoundOver→RoundEnded, etc.).GameEventand WebSocket message enums are intentionally untouched — they share a namespace boundary withTurnStateand are not part of this rename.a47bf5a(chore(repo): land cardshed strategy blueprint deliverable #137) — landPRPs/cardshed-01-blueprint.md, the executed output ofPRPs/01-strategy-blueprint.md. Locks the upstream decisions sibling PRPs consume: Rust+Axum backend, TS+React+Vite frontend, hybrid snapshot+event sync model, 9-module taxonomy, 20-section deliverable outline with PRP-2/PRP-3 ownership tags, and 10 cross-PRP implications.Closes #136.
Closes #137.
Zone(s) touched
@ops@dev@prod@lab.shared/.bin/ root platformrepo(CI, hygiene, tooling)Test plan
rg -n "TurnPhase|WaitingForAttack|WaitingForDefense|ResolvingDefense|RoundOver|MatchOver" PRPs→ zero matchesrg -n "TurnState" PRPs/02-deterministic-core.md PRPs/03-experience-distribution.md→ both contain the canonical blockTurnStateshared-contract blocks across PRP-2 and PRP-3 — they are byte-identicalPRPs/cardshed-01-blueprint.mdself-check: §10 Validation Self-Check ticks all three levels (Coherence, Round-trip with siblings, Authoritative-spec round-trip)w7 verifyresultn/a — documentation-only change; no stack touched.
Checklist
type(scope): description (#issue)privileged: trueoutside@lab/