docs(71): design spec for per-playout reshuffle + shuffle-state surfacing
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -0,0 +1,159 @@
|
||||
# Design — Per-playout Reshuffle + shuffle-state surfacing (#71)
|
||||
|
||||
Date: 2026-07-16
|
||||
Issue: [ersatztv#71](http://192.168.1.95:3000/timothy/ersatztv/issues/71) — "Persistent shuffle state + explicit 'Reroll' control"
|
||||
|
||||
## Premise correction
|
||||
|
||||
The issue's stated premise is largely **false against the current code** (verified by a full
|
||||
shuffle/seed source map):
|
||||
|
||||
- **"Shuffle is re-randomized every rebuild" — FALSE.** Only `PlayoutBuildMode.Reset` reseeds
|
||||
(`ErsatzTV.Core/Scheduling/PlayoutBuilder.cs:301`, `playout.Seed = new Random().Next()`).
|
||||
`Continue` and `Refresh` reuse the persisted seed + position.
|
||||
- **"No persistent shuffle memory today" — FALSE.** `Playout.Seed` (int column, migration
|
||||
`20240124204743_Add_PlayoutSeed`) plus owned `CollectionEnumeratorState` (Seed + Index) rows on
|
||||
`PlayoutProgramScheduleAnchor`/`PlayoutAnchor` already persist the exact shuffled order **and** the
|
||||
current position across rebuilds and restarts. The Fisher-Yates shuffle in
|
||||
`ShuffledMediaCollectionEnumerator` is fully deterministic given the seed.
|
||||
- **"Anti-repeat exists but is implicit" — TRUE.** Each item plays once per permutation before any
|
||||
reshuffle; the in-cycle reshuffle even rejects a seed whose new head equals the previous tail.
|
||||
|
||||
So the persistence #71 asks us to build **already exists**. The genuine, missing gaps are:
|
||||
|
||||
1. **No user-triggered, per-playout reshuffle.** The only reseed path is `POST /api/v1/playouts/reset-all`,
|
||||
which for **Classic** playouts enqueues `Refresh` (preserves the seed), not `Reset`. A Classic
|
||||
channel's shuffle seed is therefore effectively frozen after first build — there is no way to roll a
|
||||
new order for one channel.
|
||||
2. **No visibility** into the shuffle state.
|
||||
|
||||
## Scope (approved)
|
||||
|
||||
"Reshuffle + state surfacing." Out of scope: a separate lighter "reseed-only" mode that preserves rerun
|
||||
history (rejected — YAGNI for the common Classic+Shuffle case, where reseeding inherently clears
|
||||
per-collection anchors); a bulk "reshuffle all" (the existing `reset-all` covers bulk rebuild).
|
||||
|
||||
## A. Backend — Reshuffle action
|
||||
|
||||
- **Command:** `ErsatzTV.Application/Playouts/Commands/ReshufflePlayout.cs` —
|
||||
`public record ReshufflePlayout(int PlayoutId) : IRequest;` (fire-and-forget, no result body).
|
||||
- **Handler:** `ReshufflePlayoutHandler(ChannelWriter<IBackgroundServiceRequest> channel,
|
||||
IDbContextFactory<TvContext> dbContextFactory)`. Follows the `EraseItems` shape (controller owns the
|
||||
404/409/422 prechecks; the handler is a pure fire-and-forget enqueue): load the playout with a
|
||||
resettable-`ScheduleKind` filter, `foreach (playout in maybePlayout)` (silent no-op if absent),
|
||||
enqueue `BuildPlayout(PlayoutId, PlayoutBuildMode.Reset)`. The `Reset` build mode already performs
|
||||
reseed → clear anchors/history → rebuild; the handler does **not** manually reseed and does **not**
|
||||
re-check the lock (the controller 409-guards and the build worker acquires its own lock).
|
||||
- **Controller action** in `ErsatzTV/Controllers/Api/PlayoutController.cs`, mirroring `EraseItems`:
|
||||
```
|
||||
[HttpPost("/api/v1/playouts/{id:int}/reshuffle", Name = "ReshufflePlayout")]
|
||||
[Tags("Playouts")] [EndpointGroupName("general")]
|
||||
[EndpointSummary("Reshuffle a playout — roll a new random play order and rebuild")]
|
||||
[ProducesResponseType(StatusCodes.Status202Accepted)]
|
||||
[ProducesResponseType(typeof(ProblemDetails), 404)] // playout not found
|
||||
[ProducesResponseType(typeof(ProblemDetails), 409)] // build in progress
|
||||
[ProducesResponseType(typeof(ProblemDetails), 422)] // unsupported schedule kind
|
||||
```
|
||||
Body order (mirrors erase-items): `IsPlayoutLocked(id)` → `PlayoutLockedProblem()` (409);
|
||||
`GetPlayoutById(id)` `IsNone` → `ApiResults.NotFoundProblem()` (404); `ScheduleKind` not in
|
||||
{Classic, Block, Sequential, Scripted} → `BaseError.New(...).ToErrorResult()` (422); else
|
||||
`await mediator.Send(new ReshufflePlayout(id), ct)` → `Accepted()` (202, no body — single playout).
|
||||
|
||||
### Semantics (decisions.md entry)
|
||||
|
||||
- Named `/reshuffle`, **not** `/reset`, to (a) match the user intent, (b) avoid the confusing
|
||||
"reset-one reseeds Classic while reset-all refreshes Classic" naming clash.
|
||||
- It **is** a full per-playout `Reset`: reseed + clear per-collection anchors + **clear rerun history** +
|
||||
rebuild. For Classic+Shuffle this yields a genuinely new Fisher-Yates order; for Chronological it is a
|
||||
harmless rebuild; for Block it re-randomizes content selection and resets block rotation. The SPA
|
||||
confirm dialog states the rerun-history side effect explicitly.
|
||||
- Per-playout reshuffle deliberately reseeds Classic even though `reset-all` refreshes Classic:
|
||||
a targeted single-channel action is an explicit "give me a new order" request; the bulk action stays
|
||||
non-disruptive by design.
|
||||
|
||||
## B. Backend — Seed surfacing
|
||||
|
||||
Add `int Seed` to:
|
||||
|
||||
- `ErsatzTV.Application/Playouts/PlayoutNameViewModel.cs`
|
||||
- `ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs` (list) and `PlayoutResponseModel.cs` (detail)
|
||||
|
||||
Wire the projections that build the VM:
|
||||
|
||||
- `ErsatzTV.Application/Playouts/Mapper.cs` `ProjectToViewModel(Playout playout)` (used by
|
||||
`GetPagedPlayoutsHandler`)
|
||||
- `ErsatzTV.Application/Playouts/Queries/GetPlayoutByIdHandler.cs`
|
||||
|
||||
and the controller mappers `ToResponse` / `ToListItemResponse` in `PlayoutController.cs`.
|
||||
|
||||
Purpose: the seed changes after a reshuffle, giving the user **visible confirmation** the action worked.
|
||||
|
||||
## C. SPA
|
||||
|
||||
Files: `web/src/api/playouts.ts`, `web/src/screens/PlayoutsScreen.tsx`, regenerated
|
||||
`web/src/api/generated/v1.d.ts`.
|
||||
|
||||
- `reshufflePlayout(playoutId: number)` client fn → `POST /playouts/{id}/reshuffle` via the shared
|
||||
`request()` helper (CSRF added centrally).
|
||||
- New per-playout **"Reshuffle"** button in the action block (the `erase-*`/`delete` group), lucide
|
||||
`Dices` (or `Shuffle`) icon, `disabled={mutating || selectedLocked}`,
|
||||
`title={selectedLocked ? 'A build is in progress for this playout' : undefined}`, kind-gated to
|
||||
{Classic, Block, Sequential, Scripted}. Wired via the existing `runMutation(confirmMessage, action)`
|
||||
helper (guards re-entrancy, `window.confirm`, `query.refresh()` on success, and on `ApiError` 409 also
|
||||
refreshes so the row picks up `IsLocked`).
|
||||
- Confirm message: *"Reshuffle this playout? This rolls a new random play order and rebuilds the
|
||||
schedule from scratch, clearing rerun history."*
|
||||
- Selected-playout card: show the **seed** (monospace, e.g. `Play-order seed: 1847362`) plus one line of
|
||||
help text: *"The play order is saved and stays consistent across rebuilds. Reshuffle to roll a new
|
||||
order."* Seed comes from the already-loaded list row.
|
||||
|
||||
No new SPA convention is introduced (reuses `runMutation` + `window.confirm` + the action-button
|
||||
pattern), so `spa-conventions.md` needs no change.
|
||||
|
||||
## D. Docs (same PR)
|
||||
|
||||
- Regenerate OpenAPI + client: `dotnet build ErsatzTV.sln` → `./scripts/update-openapi.sh`
|
||||
(updates `ErsatzTV/wwwroot/openapi/v1.json` + `docs/endpoint-index.md`) →
|
||||
`cd web && npm run generate:api` (updates `web/src/api/generated/v1.d.ts`). Commit all three.
|
||||
- `docs/blazor-route-parity.md` — Playouts row: note the new reshuffle action.
|
||||
- `docs/domain-model.md` — playout rows: note `Playout.Seed` is now surfaced and the reshuffle action.
|
||||
- `docs/decisions.md` — append the semantics entry above.
|
||||
- `docs/README.md` — no change (no doc added/removed/retitled).
|
||||
|
||||
## E. Tests
|
||||
|
||||
- **Handler** (Application test harness): `ReshufflePlayoutHandler` writes exactly one
|
||||
`BuildPlayout(id, PlayoutBuildMode.Reset)` to the channel for a resettable kind; no write for a locked
|
||||
playout / unsupported kind (whichever the handler is responsible for — controller owns 409/422, so the
|
||||
handler test asserts the enqueue for the happy path). Capture channel writes via a test
|
||||
`Channel<IBackgroundServiceRequest>`.
|
||||
- **API/controller** (`ErsatzTV.Tests` harness, mirroring the erase-items / ChannelController tests):
|
||||
202 on a valid resettable playout; 404 for a missing id; 409 when the build lock is held; 422 for an
|
||||
ExternalJson/None playout.
|
||||
- **DTO**: the playout list/detail responses include `Seed`.
|
||||
- **SPA** (`PlayoutsScreen.test.tsx`): the Reshuffle button calls `reshufflePlayout`, respects the
|
||||
confirm dialog, refreshes on success; the seed renders in the selected-playout card.
|
||||
|
||||
## F. Verification gate
|
||||
|
||||
- Backend: `dotnet build ErsatzTV.sln`, `dotnet test` (touched projects), `dotnet format` on touched
|
||||
files (de-BOM per the #311 formatting gate).
|
||||
- OpenAPI: `npm run check:api` clean; the three generated artifacts committed.
|
||||
- SPA: `npm test`, `npm run lint`, `npm run build`.
|
||||
- **Live-E2E (required — write-path handler that enqueues a build):** via `scripts/e2e-local.sh` —
|
||||
seed a Classic playout with a Shuffle-order schedule, record the seed + first items, reshuffle, and
|
||||
assert the seed changed and the built order changed; verify a 409 while a build lock is held. Never
|
||||
exercise download endpoints via a browser tab.
|
||||
- **Independent review (required — write-path + enqueues a build; not skippable):** cold-context
|
||||
adversarial review scoped "review only" over the PR diff, then re-review the fix commit. Cross-model
|
||||
(Codex) if quota allows, else a cold Claude agent. Enumerate every producer/consumer of the
|
||||
`BuildPlayout` message and the reset path when judging soundness.
|
||||
|
||||
## Risks / edge cases
|
||||
|
||||
- **Rerun-history loss** is intentional but user-visible; the confirm dialog must call it out.
|
||||
- **Concurrent build**: the controller 409-guards, and the build worker acquires its own lock, so an
|
||||
enqueue that races a starting build is benign (the worker serializes). No new lock is introduced.
|
||||
- **Chronological / non-random playbacks**: reshuffle is a harmless rebuild (no visible order change);
|
||||
the button is still offered per-kind, not per-playback-order, since a playout can mix items.
|
||||
- **Seed = 0** (never-reset playout) still shuffles deterministically; surfacing it is fine.
|
||||
Reference in New Issue
Block a user