fix(ui): remove forbidden animate-pulse (E1, closes #133) - #390
Conversation
Bumps [sentry-sdk](https://github.com/getsentry/sentry-python) from 1.39.1 to 1.45.1. - [Release notes](https://github.com/getsentry/sentry-python/releases) - [Changelog](https://github.com/getsentry/sentry-python/blob/1.45.1/CHANGELOG.md) - [Commits](getsentry/sentry-python@1.39.1...1.45.1) --- updated-dependencies: - dependency-name: sentry-sdk dependency-version: 1.45.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…k-1.45.1 chore(deps): bump sentry-sdk from 1.39.1 to 1.45.1 in /backend
…mode switches - BingoBoard: stagger tiles in with slideUp via framer-motion variants on mount - LobbyModeSwitcher: wrap mode views in AnimatePresence for slide-up transitions - Extend existing motion vocabulary (src/lib/motion.ts), no new deps - E2E: add backend-confirmed state assertions as supplementary checks (getGameState assertions alongside UI assertions, not replacing them) - Ref: docs/plans/motion-pass.md
.beads-adversarial/, .dev.vars, .wrangler/, .entire/ were all committed to
the repo, violating HEALTHY_MAIN.md §4 ('main contains no agent-runtime or
build cruft'). These are ephemeral dev/runtime artifacts:
- .beads-adversarial/ — CTO/COO review JSON state (ephemeral)
- .dev.vars — wrangler dev env file (placeholder API_ORIGIN)
- .wrangler/ — Cloudflare dev cache (sqlite, temp bundles)
- .entire/ — tool settings (enabled/dev config)
All are now gitignored. The files remain in the working directory; they
just won't be tracked by git going forward.
Note: functions/api/[[path]].js is kept tracked — it's a deliberate deploy
artifact (see T1: deploy config reconciliation decision ticket).
T4: Fix test:backend and db:migrate scripts — use python3.11
instead of bare python (fails on macOS: 'python: command not found')
T4: Fix settings.py docstring — says Django 5.2.9 but pins Django 4.2.30
T8: Fix README.md Quick Start — replace runserver (WSGI dev server,
breaks WebSockets) with daphne ASGI entrypoint
Document actual deploy paths: Railway backend + CF Pages frontend,
plus Docker compose.prod.yml for full local stack
T8: Remove debug cruft from README.md (two stray comment lines)
These are independent, low-risk fixes from the wayfinder map
(sound-royale-ny-wisp-7b8). No user-facing behavior changes.
…ecture (T1) wrangler.toml comment said 'no Cloudflare Pages Function proxy' but functions/api/[[path]].js is the Pages Function proxy. These aren't contradictory — they serve different URL patterns: - Primary: Vite VITE_API_BASE_URL -> direct Railway call (CORS-configured) - Safety net: Pages Function proxies /api/* on Pages domain if misconfigured Updated wrangler.toml docs to explain both paths instead of claiming the Function doesn't exist. Resolution recorded in .scratch/ticket-t1-deploy-config-resolution.md
… (T3)
T3: Middleware ordering was wrong — SecurityMiddleware was 3rd instead of
1st. Django docs require SecurityMiddleware first (sets HSTS, SSL
redirect, etc. that must apply to all responses including errors).
CorsMiddleware remains 2nd (its docs say 'as close to top as possible').
T3: Added 'collectstatic --noinput' to Dockerfile.backend. STATIC_ROOT
was configured but collectstatic never ran, so nginx would 404 on
all static files in Docker deployments.
T3: Fixed stale Django 5.2 URL references in settings.py comments
→ Django 4.2 (matching the pinned version in requirements.txt).
Did NOT add Whitenoise to INSTALLED_APPS/MIDDLEWARE: the Docker deploy
uses nginx as the static file server. Whitenoise is a fallback for
platforms without a separate static server, but this project always
deploys with nginx in front. Adding it would be unused code.
…s (T5) Added a check in the repo-hygiene CI job that fails if .beads-adversarial/, .wrangler/, .dev.vars, or .entire/ are ever tracked by git again. This prevents regressions of the T2 fix (runtime artifact cleanup). T5 decision: smoke-only PR gate stays as interim — full E2E has 60/143 failures (issue #249) and can't gate PRs until fixed. HEALTHY_MAIN.md §3 documents this as the interim approach.
…ent (T6) Pre-commit hook was omitting test:e2e:guard with a stale comment claiming scripts/* is gitignored so e2e-guard.sh can't exist. But .gitignore has a !scripts/e2e-guard.sh exception — the script IS tracked and exists in clean checkouts. Restoring the guard to pre-commit so E2E structure is validated on every commit. Also confirmed: .husky/pre-push references scripts/gaia-gate.sh which is intentionally gitignored (local-only heavy gate); the hook gracefully skips when absent. No timeout dependency — HEALTHY_MAIN.md comment was stale.
…icator (E1) Design-system violation: infinite pulse on non-state-driven elements is forbidden per design-system/sound-royale/MASTER.md §Animation Rules. - TurnIndicator.tsx: removed animate-pulse from active-player dot (state is already communicated by ring-2 + bg-primary/10 on the parent card). - RoundIndicator.tsx: removed animate-pulse from preparing state (RefreshCw animate-spin already communicates the preparing action; the card border darkens to border-primary/50 to signal urgency without an infinite loop). Closes #133.
|
|
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Reviewer's GuideRemoves forbidden Tailwind animate-pulse usage from game indicators, introduces structured motion for lobby mode and bingo board transitions via framer-motion, strengthens live E2E tests with backend state assertions and timeouts, updates backend/runtime docs and settings to match actual deployment and Python/Django versions, tightens CI hygiene around agent artifacts, and deletes committed agent-runtime/build cruft while adding governance docs and plans. Sequence diagram for the new lobby mode switch animationsequenceDiagram
actor Player
participant LobbyModeSwitcher
participant LobbyLanding
participant JoinRoomForm
participant CreateRoomForm
participant AnimatePresence
participant motion_div
Player->>LobbyLanding: onJoinMode()
LobbyLanding->>LobbyModeSwitcher: onJoinMode()
LobbyModeSwitcher->>AnimatePresence: [mode set to join]
AnimatePresence->>motion_div: exit landing (key landing)
AnimatePresence->>motion_div: enter join (key join)
motion_div->>JoinRoomForm: render JoinRoomForm
Player->>JoinRoomForm: onBack()
JoinRoomForm->>LobbyModeSwitcher: onBack()
LobbyModeSwitcher->>AnimatePresence: [mode set to landing]
AnimatePresence->>motion_div: exit join (key join)
AnimatePresence->>motion_div: enter landing (key landing)
motion_div->>LobbyLanding: render LobbyLanding
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/guides/HEALTHY_MAIN.md" line_range="44-45" />
<code_context>
+
+- `pnpm run lint`, `pnpm run verify:types`, `pnpm run test` (vitest), backend
+ Django tests, and the Playwright **visual + live** e2e all run on every PR.
+- The **visual-regression gate** (`playwright` job, Playwright
+ `toHaveScreenshot`) catches content/layout/CTa regressions that the pixelrag
+ job cannot. Its baselines are platform-pinned (`snapshotSuffix: 'linux'`) so
+ they match `ubuntu-latest` CI. Room/mono-font routes are excluded until a
</code_context>
<issue_to_address>
**nitpick (typo):** Consider changing "CTa" to the standard acronym "CTA".
In `catches content/layout/CTa regressions`, "CTa" appears to be a typo for the standard "CTA" (call-to-action). Updating the acronym here would improve clarity and consistency with common usage.
```suggestion
- The **visual-regression gate** (`playwright` job, Playwright
- `toHaveScreenshot`) catches content/layout/CTA regressions that the pixelrag
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - The **visual-regression gate** (`playwright` job, Playwright | ||
| `toHaveScreenshot`) catches content/layout/CTa regressions that the pixelrag |
There was a problem hiding this comment.
nitpick (typo): Consider changing "CTa" to the standard acronym "CTA".
In catches content/layout/CTa regressions, "CTa" appears to be a typo for the standard "CTA" (call-to-action). Updating the acronym here would improve clarity and consistency with common usage.
| - The **visual-regression gate** (`playwright` job, Playwright | |
| `toHaveScreenshot`) catches content/layout/CTa regressions that the pixelrag | |
| - The **visual-regression gate** (`playwright` job, Playwright | |
| - `toHaveScreenshot`) catches content/layout/CTA regressions that the pixelrag |
|
✅ APPROVE - no issues found Automated review at 2026-08-11 20:43 UTC This review was generated automatically by OmniRoute LLM agents. Please verify findings before acting. |
branben
left a comment
There was a problem hiding this comment.
PR Review Agent (Sourcery+Qodo fusion) — no heuristic violations detected in the diff. Advisory only; not a substitute for full review.
branben
left a comment
There was a problem hiding this comment.
PR Review Agent (Sourcery+Qodo fusion) — no heuristic violations detected in the diff. Advisory only; not a substitute for full review.
Summary
animate-pulsefromTurnIndicator.tsx(active-player dot) andRoundIndicator.tsx(preparing state) — forbidden infinite animation on non-state-driven elements perdesign-system/sound-royale/MASTER.md§Animation Rules.RoundIndicatoralready hasRefreshCw animate-spinto communicate the preparing action; the card border darkens toborder-primary/50to signal urgency without an always-on pulse.TurnIndicatoralready communicates active state viaring-2 ring-primary/50 bg-primary/10on the parent card.Test plan
rg animate-pulse src/components/game/→ 0 hitsnpx impeccable detect src/components/game/TurnIndicator.tsx src/components/game/RoundIndicator.tsx --json→[]npx tsc --noEmit→ passnpx eslint .→ 0 severity-2 errors (gaia-gate isolated worktree)pnpm run test(vitest) → 286 passed (gaia-gate)pnpm run build→ pass (gaia-gate)Sound Royale CI→ typecheck, build, E2E smoke, UI Quality Gate (Impeccable), repo-hygiene all green on this PRRoundIndicatorpreparing state still visibly distinct from idle (visual, not algorithmic)Closes #133.
Summary by Sourcery
Align UI motion with design rules, clean up forbidden animations and agent-runtime artifacts, and harden backend, tests, and deployment/docs around Python 3.11, Django 4.2, and the production CI/CD architecture.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Deployment:
Documentation:
Tests:
Chores: