Uh oh!
There was an error while loading. Please reload this page.
perf(frontend): #111 runtime animations + lazy/next-image loading - #490
Conversation
- Landing hero: squared-distance link pass with axis rejects (no more Math.hypot over all ~27k node pairs/frame); framer-motion split out of the landing chunk via next/dynamic (HowItWorks) with a layout-stable placeholder. - AtmosphericBackdrop: orb gradients pre-baked to offscreen sprites (re-baked only on DPR change), RAF throttled to ~30fps with drift speed preserved, loop paused while the tab is hidden. - KnowledgeGraph2D: simulation ticks write node/edge positions directly to the DOM (zero React work per tick), tooltip setState only on open/reseed with direct style writes while shown, id->node Map replaces O(E*N) find(); testmode tests read the new transform position carrier. eslint-suppressions baseline pruned (13->12). - Images: loading="lazy" + decoding="async" on all raw <img> sites; intrinsic width/height added where the rendered box is fixed (TopNav/SideNav logos, Settings avatar/banner) and deliberately omitted for natural-aspect user uploads (Social attachments, Admin screenshots). - Study: framer-motion subtree moved to StudyMotion.tsx behind next/dynamic (ssr:false) with visually identical fallbacks; the #383 skipAnimations test seam moved with it. Verified: 55 files / 399 vitest tests, eslint 0 errors, tsc clean, production build green. Local E2E lane not runnable on this machine (unprovisioned: no backend/.env, supabase CLI/config.toml mismatch, bash 3.2 parser bug in scripts/lib/local-common.sh:99); e2e.yml covers the lane on push to main. Closes#111 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughChangesFrontend performance updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LandingPage
participant AtmosphericBackdrop
participant KnowledgeGraph2D
participant Study
LandingPage->>AtmosphericBackdrop: load optimized animated backdrop
LandingPage->>KnowledgeGraph2D: render graph with direct SVG updates
Study->>Study: load motion highlight and animate mode panel
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@frontend/src/components/AtmosphericBackdrop.tsx`:
- Around line 170-175: Update onReducedChange to restart the animation when
reduced motion changes to disabled: if the document is visible and
rafRef.current is null, reset lastFrameRef.current and schedule
requestAnimationFrame(tick). Preserve the existing reduced-motion behavior in
tick, including painting one still frame and clearing the RAF reference.
In `@frontend/src/components/screens/Study.tsx`:
- Around line 32-36: Update the StudyModePanel dynamic import fallback so it
renders the currently active Study pane content, not just an empty flex wrapper,
while the motion component is pending; preserve the wrapper layout and replace
it with the loaded StudyModePanel once available. Add a regression test covering
the pending-import state and verifying the active GuideMode or FlashcardsMode
remains visible.
In `@frontend/src/components/SideNav.tsx`:
- Around line 114-117: Remove the loading="lazy" attribute from both
above-the-fold navigation logos: the sidebar logo in
frontend/src/components/SideNav.tsx lines 114-117 and the top navigation logo in
frontend/src/components/TopNav.tsx lines 172-175. Leave their other image
attributes unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 934e3205-2249-4a4b-8124-5b86673a0b85
📒 Files selected for processing (12)
frontend/eslint-suppressions.jsonfrontend/src/app/(public)/page.tsxfrontend/src/components/AtmosphericBackdrop.tsxfrontend/src/components/KnowledgeGraph2D.testmode.test.tsxfrontend/src/components/KnowledgeGraph2D.tsxfrontend/src/components/SideNav.tsxfrontend/src/components/TopNav.tsxfrontend/src/components/screens/Admin.tsxfrontend/src/components/screens/Settings.tsxfrontend/src/components/screens/Social.tsxfrontend/src/components/screens/Study.tsxfrontend/src/components/screens/StudyMotion.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 5809465 | Commit Preview URL Branch Preview URL | Jul 31 2026, 06:39 AM |
#111) The throttle returned before the physics step, so orb positions advanced once per PAINTED frame rather than once per rAF — at 30fps that is half the original drift speed. A visible change, in a PR whose whole claim is that there isn't one. Each step is now scaled by how many 60fps-equivalent frames actually elapsed, so the apparent speed matches the 60fps original at any cadence. Clamped to 4 frames so a backgrounded tab (or the very first frame, when lastPaint is still 0) cannot teleport the field on resume. Caught by reading PR #490, which independently hit the same trap and handled it — my own review pass checked the throttle for stalls and for the reduced-motion still frame, but not for the drift rate. part of #111 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 31, 2026
Heads-up: I opened #492 against issue #111 before seeing this PR — my mistake, and entirely my own for not checking open PRs first. The two overlap on nine files, including the hero canvas, I've posted a side-by-side comparison on #492 rather than duplicating it here. Short version: this PR is the more thorough pass — direct DOM writes for the graph ticks (what issue #111's own proposal recommended, and deeper than my rAF coalescing), the Two things from #492 that may be worth porting here:
Also worth flagging, since you hit the same trap and handled it and I didn't: the 30fps cap halving the drift speed. Your elapsed-frame scaling is the right fix — I'd missed it until I read this PR. No action needed from you; @AndresL230 should decide which lands. |
…s, eager nav logos
- AtmosphericBackdrop: cap sprite backing resolution at 512px (was full
device resolution — ~100 MB of canvas memory at DPR 2 across 14 orbs);
the soft gradients upscale indistinguishably. Also repaint the static
frame on resize under reduced motion (canvas.width resets cleared it).
- Study: the mode transition no longer routes pane content through the
lazy motion chunk — next/dynamic's fallback can't carry children, which
gated the panes behind the chunk fetch, and un-gating them remounted
the pane mid-session, wiping state (caught by Study.test.tsx). Replaced
with a keyed CSS enter-fade (`study-mode-enter`) that needs no chunk;
AnimatePresence initial={false} semantics preserved via render-phase
state derivation. StudyMotion.tsx keeps only the toggle highlight and
the #383 test seam.
- TopNav/SideNav: drop loading="lazy" from the always-visible nav logos
(lazy only delays permanent chrome); keep decoding + dimensions.
- Landing: document the ssr:false SEO tradeoff on the HowItWorks split.
Verified: 399/399 vitest, eslint 0 errors, tsc clean, production build.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Jose-Gael-Cruz-Lopez
commented
Jul 31, 2026
Automated review pass (Claude Code) on No criticals. Two Important findings — both real, both fixed in
Minors also addressed: nav logos no longer Watch on the post-merge |
…Works copy - AtmosphericBackdrop: disabling reduced motion mid-session now restarts the parked RAF loop (tick() parks itself on a still frame; the old always-running loop resumed implicitly, so the backdrop stayed frozen). - Landing: drop ssr:false from the HowItWorks dynamic import (#492 review) — next/dynamic still splits the motion stack into its own chunk, but the section's marketing copy is back in the server-rendered HTML; verified "Upload Your Materials" present in the prerendered index.html. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Jose-Gael-Cruz-Lopez
commented
Jul 31, 2026
Re: the #492 overlap — ported the Deliberately did not port CodeRabbit's three findings are resolved: the reduced-motion revive was a real regression vs. the old always-running loop (fixed in 23b773f); the Study fallback and nav-logo findings were fixed in f92e0ce (Study went further — keyed CSS enter-fade, no chunk dependency at all, after the handoff approach proved to remount panes and wipe state). |
Brings in #341 (FullHeightScreen viewport-math), #290 (approval gate) and #288 (hero card). The globals.css conflict is resolved by keeping BOTH added blocks: this branch's .study-mode-enter and main's .pending-* beat. Ordering invariants re-verified after the merge — .card--hero still follows .card, and .pending-* still precedes the .anim-d* delay utilities (the animation shorthand resets animation-delay, so those have to come later to win). Also fixes the intermittent frontend-lane CI failure, which is unrelated to either PR but was red-flagging both. FlashcardImportModal.test.tsx wiped document.body in afterEach but never unmounted; with `globals: false` in vitest.config.ts, @testing-library/react does not auto-register its cleanup, so React kept the tree and its pending scheduler work alive past the end of the file and jsdom teardown landed it on a missing `window`: ReferenceError: window is not defined vitest counts those as unhandled errors and exits non-zero even with a fully green suite — which is exactly how #492 failed with 416/416 tests passing. Now calls cleanup() before the body sweep (both are needed; the sweep clears the Dialog/ToastProvider portal siblings cleanup() leaves behind). Verified 3/3 clean full-suite runs, exit 0, zero unhandled errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 31, 2026
Merged main in, ran the E2E lane this PR was missing, and fixed the CI flakePer @AndresL230's call: take this PR as the base, port anything worthwhile from #492, verify, merge. Pushed three things here. 1. Merged 2. The full local E2E cycle — the gap in this PR's own verification note. Run at 3. Fixed the intermittent frontend-lane CI failure. Unrelated to either PR but red-flagging both — #492 failed with 416/416 tests passing and On porting from #492 — one thing went in, one did notPorted: nothing, in the end — you'd already made the Dropped, and worth recording why: I was going to port #492's spatially-binned Identical pairs at every N. Map allocation and per-point sorting cost more than binning saves until roughly N=2000, and the node count here is fixed at 226. So the asymptotically-better algorithm is the slower one in practice, and your implementation stays. My recommendation to Andres to port it was wrong. ReviewRan the multi-agent review over the merged head, aimed at the riskiest area — React re-renders clobbering the direct DOM writes, or the reverse. No bugs found: both paths read and write the same mutable Nice work on this — the direct-DOM-write graph and the |
Summary
Implements the four acceptance items of #111 (June frontend-UI audit, Performance dimension). Much of the audit had already been fixed in the interim — this PR closes what was still real, verified finding-by-finding against current code:
(public)/page.tsx): reduced-motion gating, per-nodeshadowBlur, floating-cardquerySelectorAll-per-frame, and spotlight rect caching were already fixed. Still real and fixed here: the O(N²) link pass now hoists per-node bounds, rejects on |dx|/|dy| axes, and compares squared distances —Math.sqrtonly runs for the tiny fraction of pairs that draw a link (wasMath.hypoton ~27k pairs/frame). Pixel-identical output.HowItWorksis nownext/dynamic(ssr:false) with a height-preserving placeholder, mirroring the repo's MarkdownChat/react-force-graph split pattern.visibilitychange. Reduced-motion semantics unchanged.transformper node group, four attrs per edge). TooltipsetStatefires only on open/reseed; moves are direct style writes.nodeByIdMap replaces the per-edge.find().e2e/graph.spec.tsDOM contracts (testids, counts, labels, opacity) verified untouched; testmode tests updated to read the newtransformposition carrier. One-lineeslint-suppressions.jsonbaseline prune (13→12, same maintenance as 47950e5).<img>in the audit's scope now hasloading="lazy"+decoding="async"(nav logos excepted post-review — permanent chrome loads eagerly; raw imgs on the static public pages and in SignInModal/toasts were outside [P2] Optimize runtime animations + add lazy/next-image loading #111's enumerated scope); intrinsicwidth/heightadded where the rendered box is fixed (TopNav/SideNav logos, Settings avatar + banner preview). Deliberately no fixed dims on natural-aspect user uploads (Social attachments, Admin issue screenshots) — hardcoding both would distort content with no stored dimensions to derive from. Nonext/imageconversion (no loader configured; per issue).StudyMotion.tsxbehindnext/dynamic(ssr:false) with visually identical static fallbacks; the feat(frontend): NEXT_PUBLIC_TEST_MODE determinism flag #383skipAnimationstest seam moved with it so it still runs before any motion component renders.Verification
tsc --noEmit: clean insrc/next build): greenbackend/.env; installed Supabase CLI 2.90.0 rejectsconfig.toml'sauto_expose_new_tables/local_smtpkeys; macOS bash 3.2 hits its known quote-in-$()parser bug atscripts/lib/local-common.sh:99, somake e2e-updies at source time).e2e.ymlruns the full browser lane on the push to main. The graph journey's DOM assertions were manually cross-checked against the KnowledgeGraph2D changes.Perf-only PR: no visual/behavioral change for non-reduced-motion users.
Closes#111
🤖 Generated with Claude Code
Summary by CodeRabbit
Performance Improvements
User Experience
Bug Fixes