Animate the board's columns from JS, so the entrance cannot play twice #32

Merged
Grandiras merged 3 commits from deterministic-board-entrance into main 2026-07-27 02:35:57 +00:00
Owner

Is there really no way to fix this deterministically? Like, just playing the animation once in the first place?

Yes, and that's what this does. Every previous attempt was a timer trying to out-guess DOM churn, which is why each one produced a new symptom instead of a fix.

Why the timers kept losing

The board is built twice on every load — the prerender paints columns, the circuit's first render replaces them with Board.razor's spinner, then the data builds them again (production: 82 ms / 311 ms / 317 ms). A CSS animation keyed to element creation fires on whichever sets happen to exist.

attempt outcome
latch on cascade end (v2.0.3) never armed — rebuild at 311 ms, cascade needs 562 ms
gate + hide, latch on start (v2.0.4) double gone, but the prerendered set flickers before being torn down
add a settle window still a guess

What these were racing is the framework's own render schedule. No amount of tuning wins that.

What this does instead

There is no CSS animation on the columns any more. Board.razor calls ponenteEntrance.playBoard(_columns.Count) from the render where its data has actually arrived — the same signal it already trusts to decide when its drag-and-drop interop can safely import — and JS animates exactly the elements present at that moment, once, via the Web Animations API.

Anything created afterwards cannot animate, because there is no rule left to fire on it. That's the deterministic part: correctness no longer depends on when the rebuild happens.

One subtlety cost me an intermittent "animates nothing at all" while building this, and is worth stating: Board.razor's _columns is not null is component state, not the DOM. OnInitializedAsync's continuation can land between the spinner render and its OnAfterRender, so the call sometimes arrives while the spinner is still on screen and querying for columns finds none. It now passes the count it rendered, and JS waits for that many columns — a wait on the thing being animated rather than on a clock.

Keyframe values live in app.css as custom properties (--entrance-duration, --entrance-lead, --entrance-stagger, --entrance-rise) and are read back off :root, so the stylesheet stays the source of truth instead of the timings being duplicated into JS.

Verification

The assertion is now the strongest available and identical at every timing: as many animations started as there are columns, and all of them finished. A cancelled animation is the flicker; an extra batch is the double.

circuit delay started finished
0 ms = columns = columns
250 ms (production gap) = columns = columns
700 ms = columns = columns
1500 ms = columns = columns
6500 ms (past the safety reveal) 0 — entrance forfeited, board visible

The safety reveal is a guard for a board that never renders at all (dead circuit, a throw in ReloadAsync), not a timing heuristic; past it the entrance is given up rather than played over content already on screen.

Three consecutive full runs of the file, no flakes. dotnet build -c Release clean, dotnet test 46/46, Playwright 77/77.

CLAUDE.md now says plainly that the columns must not be put back on the CSS gate, and records that delaying blazor.web.js to reproduce this gives a false negative — the trap that cost two releases.

> Is there really no way to fix this deterministically? Like, just playing the animation once in the first place? Yes, and that's what this does. Every previous attempt was a timer trying to out-guess DOM churn, which is why each one produced a new symptom instead of a fix. ## Why the timers kept losing The board is built twice on every load — the prerender paints columns, the circuit's first render replaces them with `Board.razor`'s spinner, then the data builds them again (production: 82 ms / 311 ms / 317 ms). A CSS animation keyed to element creation fires on whichever sets happen to exist. | attempt | outcome | |---|---| | latch on cascade **end** (v2.0.3) | never armed — rebuild at 311 ms, cascade needs 562 ms | | gate + hide, latch on start (v2.0.4) | double gone, but the prerendered set flickers before being torn down | | add a settle window | still a guess | What these were racing is the framework's own render schedule. No amount of tuning wins that. ## What this does instead **There is no CSS animation on the columns any more.** `Board.razor` calls `ponenteEntrance.playBoard(_columns.Count)` from the render where its data has actually arrived — the same signal it already trusts to decide when its drag-and-drop interop can safely import — and JS animates exactly the elements present at that moment, once, via the Web Animations API. Anything created afterwards **cannot** animate, because there is no rule left to fire on it. That's the deterministic part: correctness no longer depends on when the rebuild happens. One subtlety cost me an intermittent "animates nothing at all" while building this, and is worth stating: `Board.razor`'s `_columns is not null` is **component state, not the DOM**. `OnInitializedAsync`'s continuation can land between the spinner render and its `OnAfterRender`, so the call sometimes arrives while the spinner is still on screen and querying for columns finds none. It now passes the count it rendered, and JS waits for that many columns — a wait on the thing being animated rather than on a clock. Keyframe values live in `app.css` as custom properties (`--entrance-duration`, `--entrance-lead`, `--entrance-stagger`, `--entrance-rise`) and are read back off `:root`, so the stylesheet stays the source of truth instead of the timings being duplicated into JS. ## Verification The assertion is now the strongest available and identical at every timing: as many animations **started** as there are columns, and all of them **finished**. A cancelled animation is the flicker; an extra batch is the double. | circuit delay | started | finished | |---|---|---| | 0 ms | = columns | = columns | | 250 ms (**production gap**) | = columns | = columns | | 700 ms | = columns | = columns | | 1500 ms | = columns | = columns | | 6500 ms (past the safety reveal) | 0 — entrance forfeited, board visible | — | The safety reveal is a guard for a board that never renders at all (dead circuit, a throw in `ReloadAsync`), not a timing heuristic; past it the entrance is given up rather than played over content already on screen. Three consecutive full runs of the file, no flakes. `dotnet build -c Release` clean, `dotnet test` 46/46, Playwright 77/77. CLAUDE.md now says plainly that the columns must **not** be put back on the CSS gate, and records that delaying `blazor.web.js` to reproduce this gives a false negative — the trap that cost two releases.
Animate the board's columns from JS, so the entrance cannot play twice
All checks were successful
CI / build (pull_request) Successful in 33s
41bacc5f92
Asked whether this could be made deterministic rather than timed. It can, and
should have been from the start.

Every previous attempt gated a CSS animation keyed to element creation, and each
was a timer trying to out-guess DOM churn. The board is built twice on every
load — prerender, then Board.razor's spinner, then the data — so the animation
fires on whichever sets happen to exist. Latching on cascade-end never armed
(the rebuild lands at 311ms, the cascade needs 562ms). Latching on cascade-start
flickered: a sliver of motion on the prerendered set before it was torn down.
A settle window was still a guess. Three releases went out that way.

Timers cannot win here, because what they are racing is the framework's own
render schedule. So ask the framework instead. There is no CSS animation on the
columns any more. Board.razor calls playBoard() from the render where its data
has actually arrived — the same signal it already trusts to decide when its
drag-and-drop interop can safely import — and JS animates exactly the elements
present at that moment, once, with the Web Animations API. Anything built
afterwards cannot animate, because there is no rule left to fire on it.

One subtlety worth the extra argument: Board.razor's "_columns is not null" is
component state, not the DOM. OnInitializedAsync's continuation can land between
the spinner render and its OnAfterRender, so the call sometimes arrives while the
spinner is still on screen and querying for columns finds none — which is why
the first cut of this animated nothing at all, intermittently. It now passes the
count it rendered and JS waits for that many columns. That is a wait on the thing
being animated rather than on a clock, and it is what makes the result identical
at every timing.

Keyframe values live in app.css as custom properties and are read back off :root,
so the stylesheet stays the source of truth rather than the timings being
duplicated into JS.

Verified at circuit delays of 0, 250 (the production gap), 700 and 1500ms: every
one starts exactly as many animations as there are columns and finishes all of
them — no doubles, and nothing cancelled mid-flight, which is the flicker. Past
the safety reveal (a guard for a board that never renders at all, not a timing
heuristic) the entrance is forfeited rather than played over content already on
screen. Three consecutive full runs, no flakes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/115); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-115.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

💬 Verdict: comment — notes only, nothing blocking.

This is a well-reasoned fix and matches the failure mode described in the PR: gating the columns' entrance on a JS call driven by Board.razor's own render, rather than a CSS rule keyed to element creation, removes the race the previous three attempts kept losing. I traced the flicker concern the PR itself raises (reveal() synchronously removing the CSS-hide rule before animateColumns() applies the WAAPI fill: backwards state) and it holds up: whenRendered always waits at least one requestAnimationFrame before checking the DOM, so the animation's backward-filled opacity-0 state is applied in the same frame the hidden-rule's style invalidation would otherwise have painted, avoiding a visible flash. The played/pushState reset interaction between overlapping whenRendered polls on rapid navigation also checks out — a stale loop bails via if (!played) return rather than animating stale state. Tests were updated consistently with the new WAAPI-based mechanism and the new UNBLOCK_MS safety window.

One real edge case survives, noted below: an empty board (zero columns) leaves the JS in an unbounded polling loop.


⏱ 2m 39s · 10 turns · tokens: 48.0k in / 13.0k out (+546.8k cache-read)
📜 Full session transcript

💬 **Verdict: comment** — notes only, nothing blocking. This is a well-reasoned fix and matches the failure mode described in the PR: gating the columns' entrance on a JS call driven by `Board.razor`'s own render, rather than a CSS rule keyed to element creation, removes the race the previous three attempts kept losing. I traced the flicker concern the PR itself raises (`reveal()` synchronously removing the CSS-hide rule before `animateColumns()` applies the WAAPI `fill: backwards` state) and it holds up: `whenRendered` always waits at least one `requestAnimationFrame` before checking the DOM, so the animation's backward-filled opacity-0 state is applied in the same frame the hidden-rule's style invalidation would otherwise have painted, avoiding a visible flash. The `played`/pushState reset interaction between overlapping `whenRendered` polls on rapid navigation also checks out — a stale loop bails via `if (!played) return` rather than animating stale state. Tests were updated consistently with the new WAAPI-based mechanism and the new `UNBLOCK_MS` safety window. One real edge case survives, noted below: an empty board (zero columns) leaves the JS in an unbounded polling loop. --- ⏱ 2m 39s · 10 turns · tokens: 48.0k in / 13.0k out (+546.8k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-115.md) <!-- claude-verdict:comment --> <!-- claude-reviewed-head:41bacc5f92404d818bfb8afd9e992e4f13ac1c0f -->
@ -124,0 +172,4 @@
var look = function () {
if (!played) return;
var columns = document.querySelectorAll('.board > .column');
if (columns.length >= expected) { then(columns); return; }
Collaborator

whenRendered(expected || 1, animateColumns): when the board has zero columns — a real, reachable state (Board.razor has a dedicated _columns.Count == 0 empty-state branch, e.g. right after an admin deletes the last column) — Board.razor calls playBoard(0). 0 || 1 coerces expected to 1, and whenRendered's look loop then polls document.querySelectorAll('.board > .column') via requestAnimationFrame forever, since no .column element will ever appear while the empty state is showing. reveal() has already run so nothing stays visually hidden, but the tab is left spinning an unbounded per-frame busy loop for the rest of the page's lifetime (until a route change resets played). Worth guarding with something like if (!expected) return; before calling whenRendered, so a genuinely empty board doesn't leave JS polling forever.

`whenRendered(expected || 1, animateColumns)`: when the board has zero columns — a real, reachable state (`Board.razor` has a dedicated `_columns.Count == 0` empty-state branch, e.g. right after an admin deletes the last column) — `Board.razor` calls `playBoard(0)`. `0 || 1` coerces `expected` to `1`, and `whenRendered`'s `look` loop then polls `document.querySelectorAll('.board > .column')` via `requestAnimationFrame` forever, since no `.column` element will ever appear while the empty state is showing. `reveal()` has already run so nothing stays visually hidden, but the tab is left spinning an unbounded per-frame busy loop for the rest of the page's lifetime (until a route change resets `played`). Worth guarding with something like `if (!expected) return;` before calling `whenRendered`, so a genuinely empty board doesn't leave JS polling forever.
Author
Owner

Good catch — fixed in the latest push, and it was worse than "the tab spins": reveal() runs before the wait, so nothing would have been visibly wrong. It would only have shown up as battery drain.

playBoard(0) now returns immediately, and the wait is bounded for the general case of columns that never arrive (a throw in the render). Giving up is safe there for the same reason it's safe in the empty case: the board is already visible by then, so the cost is a missing animation rather than hidden content.

On the test: my first attempt reached the empty state by deleting the fixture's columns through the admin UI. That was wrong twice over — the selectors matched nothing, so it asserted on a board that still had three columns and passed vacuously; and had it worked, it would have left the shared test database with no columns for every later spec, since teardown restores rows it created, not ones a test removed. It now calls both guards directly on a page with no board.

Verified against the unguarded version: 43 frames scheduled where fewer than 10 are expected.

@claude /review

Good catch — fixed in the latest push, and it was worse than "the tab spins": `reveal()` runs before the wait, so nothing would have been visibly wrong. It would only have shown up as battery drain. `playBoard(0)` now returns immediately, and the wait is bounded for the general case of columns that never arrive (a throw in the render). Giving up is safe there for the same reason it's safe in the empty case: the board is already visible by then, so the cost is a missing animation rather than hidden content. On the test: my first attempt reached the empty state by deleting the fixture's columns through the admin UI. That was wrong twice over — the selectors matched nothing, so it asserted on a board that still had three columns and passed vacuously; and had it worked, it would have left the shared test database with no columns for every later spec, since teardown restores rows it created, not ones a test removed. It now calls both guards directly on a page with no board. Verified against the unguarded version: 43 frames scheduled where fewer than 10 are expected. @claude /review
Don't poll for columns that are never coming
All checks were successful
CI / build (pull_request) Successful in 33s
65b8811587
Review catch, and a good one. A board with no columns is a real state —
Board.razor has an empty-state branch, reached the moment an admin deletes the
last column — and it calls playBoard(0). `expected || 1` turned that into a wait
for one column that would never appear, leaving a requestAnimationFrame callback
rescheduling itself for the life of the tab. Nothing was hidden, since reveal()
had already run, so it would have been invisible except on a battery.

Zero now returns immediately, and the wait is bounded for the general case of
columns that never arrive at all. Giving up is safe there for the same reason:
the board is already visible by then, so the cost is a missing animation.

The test exercises both guards directly rather than deleting the fixture's
columns, which would have left the shared database empty for every later spec —
teardown restores rows it created, not ones a test removed. Verified against the
unguarded version: 43 frames scheduled where fewer than 10 are expected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/116); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-116.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

The empty-board polling issue I flagged last pass is fixed (if (!expected) return; before whenRendered), and the new board-entrance.spec.js test explicitly pins both the empty-board and never-arrives cases via the frame-count guard. The WAAPI-driven column entrance itself is sound: playBoard is called from the render where _columns data has actually landed, animates exactly the elements present once, and there's no CSS rule left to fire on later churn — this genuinely removes the class of race the last three attempts kept losing.

However, this diff also rewrites the unrelated auth-card fallback in the same file's first IIFE, and that change looks like a real regression that isn't mentioned anywhere in the PR description. See inline comment.


⏱ 1m 5s · 3 turns · tokens: 35.6k in / 5.4k out (+151.7k cache-read)
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. The empty-board polling issue I flagged last pass is fixed (`if (!expected) return;` before `whenRendered`), and the new `board-entrance.spec.js` test explicitly pins both the empty-board and never-arrives cases via the frame-count guard. The WAAPI-driven column entrance itself is sound: `playBoard` is called from the render where `_columns` data has actually landed, animates exactly the elements present once, and there's no CSS rule left to fire on later churn — this genuinely removes the class of race the last three attempts kept losing. However, this diff also rewrites the *unrelated* auth-card fallback in the same file's first IIFE, and that change looks like a real regression that isn't mentioned anywhere in the PR description. See inline comment. --- ⏱ 1m 5s · 3 turns · tokens: 35.6k in / 5.4k out (+151.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-116.md) <!-- claude-verdict:request_changes --> <!-- claude-reviewed-head:65b8811587c5301f2766e2ed1164af9e50ee4977 -->
@ -105,2 +77,2 @@
// admin page. Routes.razor cannot help — those navigations reuse the
// circuit, so its first render happened long ago.
// Switching between Sign in and Create account is the one replay the design
// does want. Routes.razor cannot help here — that navigation reuses the
Collaborator

This changes the login-card fallback from reveal() to play() (and bumps CIRCUIT_FALLBACK_MS from 900ms to 3000ms), which is out of scope for a PR titled about the board columns and isn't mentioned in the description. It looks like a reintroduction of the exact bug class this PR is fixing for columns, just for the card:

If the circuit takes longer than 3000ms to report (slow, not dead), this timer now calls play() on the prerendered card — setting data-auth="playing", which the CSS rule html[data-auth="playing"] .login-card > * keys off element creation. When Blazor's circuit eventually connects and discards/recreates the prerendered card's children shortly after, those newly-created nodes match the still-playing selector and animate again — a second cascade on content that already finished (or was mid-) animating. That's the same "CSS animation restarts whenever its element is created" hazard CLAUDE.md documents as the original card bug, and the whole reason entrance.js previously called reveal() (never-animate) rather than play() on this fallback.

Previously spent/reveal() made this safe by ensuring a late arrival could never animate content already on screen; that guard is now gone from the fallback path entirely. CLAUDE.md's description of this fallback ("if the circuit has not reported within ~900ms it reveals rather than plays, and marks the entrance spent so a late arrival cannot animate content already on screen") was also left unchanged, so the docs are now inconsistent with the code. auth-entrance.spec.js has no test past ~1500ms of circuit delay for the card, so this path isn't covered.

If this fallback change is intentional, it needs its own justification (why is animating the prerendered card now safe?) and a CLAUDE.md update; if it's accidental scope creep from refactoring the shared file, it should be reverted to fallback = setTimeout(reveal, CIRCUIT_FALLBACK_MS) with spent restored.

This changes the login-card fallback from `reveal()` to `play()` (and bumps `CIRCUIT_FALLBACK_MS` from 900ms to 3000ms), which is out of scope for a PR titled about the board columns and isn't mentioned in the description. It looks like a reintroduction of the exact bug class this PR is fixing for columns, just for the card: If the circuit takes longer than 3000ms to report (slow, not dead), this timer now calls `play()` on the **prerendered** card — setting `data-auth="playing"`, which the CSS rule `html[data-auth="playing"] .login-card > *` keys off *element creation*. When Blazor's circuit eventually connects and discards/recreates the prerendered card's children shortly after, those newly-created nodes match the still-`playing` selector and animate again — a second cascade on content that already finished (or was mid-) animating. That's the same "CSS animation restarts whenever its element is created" hazard CLAUDE.md documents as the original card bug, and the whole reason `entrance.js` previously called `reveal()` (never-animate) rather than `play()` on this fallback. Previously `spent`/`reveal()` made this safe by ensuring a late arrival could never animate content already on screen; that guard is now gone from the fallback path entirely. CLAUDE.md's description of this fallback ("if the circuit has not reported within ~900ms it *reveals* rather than plays, and marks the entrance spent so a late arrival cannot animate content already on screen") was also left unchanged, so the docs are now inconsistent with the code. `auth-entrance.spec.js` has no test past ~1500ms of circuit delay for the card, so this path isn't covered. If this fallback change is intentional, it needs its own justification (why is animating the prerendered card now safe?) and a CLAUDE.md update; if it's accidental scope creep from refactoring the shared file, it should be reverted to `fallback = setTimeout(reveal, CIRCUIT_FALLBACK_MS)` with `spent` restored.
Author
Owner

Correct on every point, and it was my error rather than a deliberate change. I started the board work by restoring the card's state machine from v2.0.2 as a clean base, which silently took the fallback back to v2.0.2 behaviour as well: play() at 3000 ms instead of reveal() at 900 ms, with spent gone.

So yes — that reintroduces, for the card, precisely the bug class this PR removes for the columns. A circuit that is slow rather than dead trips the timer, sets data-auth="playing" on the prerendered card, and then Blazor connects and recreates the card's children, which the CSS keys off element creation. Second cascade on content already on screen.

Telling detail: CLAUDE.md still described the v2.0.4 behaviour throughout. The docs were right and the code was wrong, which is the wrong way round for that to be caught by anything but review.

Fixed in the latest push: reveal() and spent are back, and a route change now clears spent — a new arrival says nothing about whether the previous entrance was spent. Also added the coverage that was missing: the card at a 2500 ms circuit delay must be visible early and must not animate when the circuit finally lands. auth-entrance.spec.js had nothing past ~1500 ms, which is exactly why this slipped through.

79 passing, build clean.

@claude /review

Correct on every point, and it was my error rather than a deliberate change. I started the board work by restoring the card's state machine from `v2.0.2` as a clean base, which silently took the *fallback* back to v2.0.2 behaviour as well: `play()` at 3000 ms instead of `reveal()` at 900 ms, with `spent` gone. So yes — that reintroduces, for the card, precisely the bug class this PR removes for the columns. A circuit that is slow rather than dead trips the timer, sets `data-auth="playing"` on the prerendered card, and then Blazor connects and recreates the card's children, which the CSS keys off element creation. Second cascade on content already on screen. Telling detail: CLAUDE.md still described the v2.0.4 behaviour throughout. The docs were right and the code was wrong, which is the wrong way round for that to be caught by anything but review. Fixed in the latest push: `reveal()` and `spent` are back, and a route change now clears `spent` — a new arrival says nothing about whether the previous entrance was spent. Also added the coverage that was missing: the card at a 2500 ms circuit delay must be visible early and must **not** animate when the circuit finally lands. `auth-entrance.spec.js` had nothing past ~1500 ms, which is exactly why this slipped through. 79 passing, build clean. @claude /review
Put back the card's reveal-on-fallback, which I reverted by accident
All checks were successful
CI / build (pull_request) Successful in 35s
c2f9ecdca9
Review catch, and it was a real regression of my own making. Restoring the
card's state machine from v2.0.2 as a starting point silently took the fallback
back to v2.0.2 behaviour too: play() at 3000ms instead of reveal() at 900ms,
with the `spent` guard gone.

That reintroduces, for the card, exactly the bug class this PR removes for the
columns. A circuit that is slow rather than dead would trip the timer, set
data-auth="playing" on the prerendered card, and then Blazor would connect and
recreate the card's children — which the CSS keys off element creation, so they
animate. A second cascade on content already on screen.

CLAUDE.md still described the v2.0.4 behaviour throughout, so the docs were
right and the code was wrong. reveal() and `spent` are back, and a route change
clears `spent` since a new arrival says nothing about the previous entrance.

Covered now: the card at a 2500ms circuit delay must be visible early and must
not animate when the circuit finally lands. auth-entrance.spec.js had nothing
past ~1500ms, which is why this slipped through.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/117); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-117.md) — updating every ~5 minutes while the task runs.
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/118); the reply will link the full session transcript. 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-118.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

Both blocking issues from my previous passes are resolved. The empty-board infinite-poll bug is fixed with if (!expected) return; before whenRendered, and now has a dedicated test (waiting for columns cannot spin forever). The out-of-scope regression I flagged last round — the auth-card fallback switching from reveal() to play() with CIRCUIT_FALLBACK_MS bumped to 3000ms, which would have replayed the card entrance on newly-created nodes from a late-connecting circuit — has been reverted back to reveal() at 900ms, matching the documented behavior.

The core mechanism is sound: Board.razor calls ponenteEntrance.playBoard(_columns.Count) exactly once per circuit (guarded by _entrancePlayed, which is never reset by the mutation-triggered ReloadAsync calls, only by client-side route changes in JS), animates only the columns present via WAAPI, and there's no CSS rule left to fire on later churn — this genuinely removes the race the prior three attempts kept losing. The reveal()-before-animate ordering avoids the flicker concern I traced in an earlier pass (the WAAPI fill: backwards state is applied within the same rendering frame as the CSS-hide removal), the played/shownAlready interplay correctly skips animating content that the UNBLOCK_MS safety guard already revealed, and the bounded RENDER_WAIT_MS poll can't spin forever. Tests updated consistently (started/finished animation counts replace the old animationstart-count assertions) and cover 0/250/700/1500ms circuit delays plus the >UNBLOCK_MS case.

One minor, non-blocking note below: this diff also silently drops the popstate listener that previously let browser back/forward navigation replay the card and board entrances — worth a mention in the PR or a quick follow-up, but not a functional break (worst case is a missing animation, and it isn't tested either way).


⏱ 2m 33s · 12 turns · tokens: 47.7k in / 11.6k out (+790.7k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Both blocking issues from my previous passes are resolved. The empty-board infinite-poll bug is fixed with `if (!expected) return;` before `whenRendered`, and now has a dedicated test (`waiting for columns cannot spin forever`). The out-of-scope regression I flagged last round — the auth-card fallback switching from `reveal()` to `play()` with `CIRCUIT_FALLBACK_MS` bumped to 3000ms, which would have replayed the card entrance on newly-created nodes from a late-connecting circuit — has been reverted back to `reveal()` at 900ms, matching the documented behavior. The core mechanism is sound: `Board.razor` calls `ponenteEntrance.playBoard(_columns.Count)` exactly once per circuit (guarded by `_entrancePlayed`, which is never reset by the mutation-triggered `ReloadAsync` calls, only by client-side route changes in JS), animates only the columns present via WAAPI, and there's no CSS rule left to fire on later churn — this genuinely removes the race the prior three attempts kept losing. The `reveal()`-before-animate ordering avoids the flicker concern I traced in an earlier pass (the WAAPI `fill: backwards` state is applied within the same rendering frame as the CSS-hide removal), the `played`/`shownAlready` interplay correctly skips animating content that the `UNBLOCK_MS` safety guard already revealed, and the bounded `RENDER_WAIT_MS` poll can't spin forever. Tests updated consistently (started/finished animation counts replace the old animationstart-count assertions) and cover 0/250/700/1500ms circuit delays plus the >UNBLOCK_MS case. One minor, non-blocking note below: this diff also silently drops the `popstate` listener that previously let browser back/forward navigation replay the card and board entrances — worth a mention in the PR or a quick follow-up, but not a functional break (worst case is a missing animation, and it isn't tested either way). --- ⏱ 2m 33s · 12 turns · tokens: 47.7k in / 11.6k out (+790.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-117.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:c2f9ecdca96a459cf90afde4337d14c09a0aa718 -->
@ -124,0 +184,4 @@
// Board.razor tells us how many columns it just rendered. That matters
// because its own guard ("_columns is not null") is component state, not the
// DOM: OnInitializedAsync's continuation can land between the spinner render
Collaborator

The old file's shared routeChanged was wired to both the patched history.pushState/replaceState and window.addEventListener('popstate', routeChanged), so browser back/forward navigation (e.g. admin page → back to the board, or between login tabs) replayed the relevant entrance. This split rewrite drops popstate entirely: the card's replay is now gated on a click on .auth-tab, and the board's replay is gated only on the patched pushState/replaceState. Blazor's router does not re-invoke pushState/replaceState for a native back/forward navigation — the browser already owns the URL and fires popstate directly — so returning to the board or a login tab via the browser back/forward buttons no longer resets played/spent, and the entrance silently doesn't replay.

Not mentioned in the PR description and not covered by a test (board-entrance.spec.js's "returning to the board animates it again" uses page.goto, a full reload, not history.back()). The worst case is a missing animation rather than a functional bug, so this doesn't block merging, but it's the same kind of silent scope-adjacent change to this shared file that the last review round had to catch — worth confirming it's intentional.

The old file's shared `routeChanged` was wired to both the patched `history.pushState`/`replaceState` **and** `window.addEventListener('popstate', routeChanged)`, so browser back/forward navigation (e.g. admin page → back to the board, or between login tabs) replayed the relevant entrance. This split rewrite drops `popstate` entirely: the card's replay is now gated on a click on `.auth-tab`, and the board's replay is gated only on the patched `pushState`/`replaceState`. Blazor's router does not re-invoke `pushState`/`replaceState` for a native back/forward navigation — the browser already owns the URL and fires `popstate` directly — so returning to the board or a login tab via the browser back/forward buttons no longer resets `played`/`spent`, and the entrance silently doesn't replay. Not mentioned in the PR description and not covered by a test (`board-entrance.spec.js`'s "returning to the board animates it again" uses `page.goto`, a full reload, not `history.back()`). The worst case is a missing animation rather than a functional bug, so this doesn't block merging, but it's the same kind of silent scope-adjacent change to this shared file that the last review round had to catch — worth confirming it's intentional.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

The blocking issue from my last pass — the auth-card fallback silently switching from reveal() to play() with no spent guard, which would have re-animated Blazor-recreated card children on a slow-but-live circuit — is fixed. It's back to reveal() on the CIRCUIT_FALLBACK_MS timer, spent is restored, and CLAUDE.md's description of that fallback now matches the code.

The empty-board polling issue from my first pass was already fixed in the previous revision and remains fixed here (if (!expected) return; before whenRendered).

The core mechanism is sound: Board.razor calls ponenteEntrance.playBoard(_columns.Count) from the render where its data has actually landed (the same render that gates the drag-and-drop interop import), JS animates exactly the elements present via WAAPI once, and there is no CSS rule left keyed to element creation for anything created afterwards to replay. reveal() removing the CSS-hide happens synchronously inside playBoard, but whenRendered always waits at least one requestAnimationFrame before reading the DOM, so the WAAPI fill: 'backwards' state is applied within the same frame the un-hide would otherwise paint — no flash. played/shownAlready correctly gate against the empty-state, the never-arrives case (bounded by RENDER_WAIT_MS), the safety UNBLOCK_MS reveal, and stale polls after a route-change reset (if (!played) return inside look). Tests were updated consistently with the WAAPI-based mechanism (started/finished counts instead of animationstart), and the new board-entrance.spec.js cases cover the safety-reveal path and the empty/never-arrives polling bounds directly rather than by deleting fixture data.

One minor, non-blocking observation below: this revision also drops the shared popstate listener entirely from auth-enter.js (both the card's and — newly — the board's re-entrance are now driven only by patched pushState/replaceState plus, for the card, an explicit .auth-tab click listener). That's a small behavioral change not mentioned in the PR description, but it degrades gracefully (a missed replay, not broken content) and is unlikely to matter in practice since the primary path is clicking the tab control, not the browser back button.


⏱ 2m 39s · 13 turns · tokens: 21.3k in / 12.3k out (+927.3k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The blocking issue from my last pass — the auth-card fallback silently switching from `reveal()` to `play()` with no `spent` guard, which would have re-animated Blazor-recreated card children on a slow-but-live circuit — is fixed. It's back to `reveal()` on the `CIRCUIT_FALLBACK_MS` timer, `spent` is restored, and CLAUDE.md's description of that fallback now matches the code. The empty-board polling issue from my first pass was already fixed in the previous revision and remains fixed here (`if (!expected) return;` before `whenRendered`). The core mechanism is sound: `Board.razor` calls `ponenteEntrance.playBoard(_columns.Count)` from the render where its data has actually landed (the same render that gates the drag-and-drop interop import), JS animates exactly the elements present via WAAPI once, and there is no CSS rule left keyed to element creation for anything created afterwards to replay. `reveal()` removing the CSS-hide happens synchronously inside `playBoard`, but `whenRendered` always waits at least one `requestAnimationFrame` before reading the DOM, so the WAAPI `fill: 'backwards'` state is applied within the same frame the un-hide would otherwise paint — no flash. `played`/`shownAlready` correctly gate against the empty-state, the never-arrives case (bounded by `RENDER_WAIT_MS`), the safety `UNBLOCK_MS` reveal, and stale polls after a route-change reset (`if (!played) return` inside `look`). Tests were updated consistently with the WAAPI-based mechanism (`started`/`finished` counts instead of `animationstart`), and the new `board-entrance.spec.js` cases cover the safety-reveal path and the empty/never-arrives polling bounds directly rather than by deleting fixture data. One minor, non-blocking observation below: this revision also drops the shared `popstate` listener entirely from `auth-enter.js` (both the card's and — newly — the board's re-entrance are now driven only by patched `pushState`/`replaceState` plus, for the card, an explicit `.auth-tab` click listener). That's a small behavioral change not mentioned in the PR description, but it degrades gracefully (a missed replay, not broken content) and is unlikely to matter in practice since the primary path is clicking the tab control, not the browser back button. --- ⏱ 2m 39s · 13 turns · tokens: 21.3k in / 12.3k out (+927.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-32/run-118.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:c2f9ecdca96a459cf90afde4337d14c09a0aa718 -->
@ -130,4 +283,3 @@
return result;
};
});
window.addEventListener('popstate', routeChanged);
Collaborator

This PR removes the shared window.addEventListener('popstate', routeChanged) entirely (visible as a deletion in the diff, with no replacement in either the card's or the new board IIFE). Browser back/forward navigation between the sign-in/create-account tabs (or, in principle, back to the board) no longer replays the entrance — only history.pushState/replaceState and, for the card, clicks on .auth-tab are watched now. Not called out in the PR description, and worth a one-line mention if intentional, but low severity: the fallback is a missing animation rather than any visible breakage, and the primary interaction path (clicking the tab) is unaffected.

This PR removes the shared `window.addEventListener('popstate', routeChanged)` entirely (visible as a deletion in the diff, with no replacement in either the card's or the new board IIFE). Browser back/forward navigation between the sign-in/create-account tabs (or, in principle, back to the board) no longer replays the entrance — only `history.pushState`/`replaceState` and, for the card, clicks on `.auth-tab` are watched now. Not called out in the PR description, and worth a one-line mention if intentional, but low severity: the fallback is a missing animation rather than any visible breakage, and the primary interaction path (clicking the tab) is unaffected.
Grandiras deleted branch deterministic-board-entrance 2026-07-27 02:35:57 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!32
No description provided.