Uh oh!
There was an error while loading. Please reload this page.
fix(graph): gate 3D mode on WebGL2 and contain renderer crashes - #539
Merged
Conversation
A browser without WebGL landing on any graph surface with a persisted
"3d" mode collapsed the whole app into the root error fallback: the
wrapper honoured localStorage unconditionally, three r163+ throws from
the WebGLRenderer constructor in a mount-time layout effect, and the
only boundary above it was the root one.
The wrapper now probes canvas.getContext("webgl2") before honouring a
persisted "3d" (ignoring, not rewriting, the preference), disables the
mode toggle with "3D requires WebGL" when the probe fails, and wraps
the graph slot in a mode-keyed local ErrorBoundary whose fallback
degrades a crashed 3D renderer to the 2D graph and heals the persisted
mode to "2d".
Unit tests were written first and watched fail (the crash test
reproduced the exact production throw escaping uncaught); the promoted
journey simulates a no-WebGL browser via an init-script getContext
override and failed against a pre-fix stack build with the dashboard on
the root fallback, exactly as reported.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | d6cfaea | Commit Preview URL Branch Preview URL | Aug 13 2026, 01:07 AM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthrough
ChangesKnowledge graph WebGL fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score:⚪ Minimal · up to The change gates 3D rendering on WebGL2 and falls back to the 2D graph when renderer creation fails; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Browser
participant KnowledgeGraph
participant localStorage
participant KnowledgeGraph3D
participant KnowledgeGraph2D
Browser->>KnowledgeGraph: mount graph
KnowledgeGraph->>Browser: probe WebGL2
Browser-->>KnowledgeGraph: return capability result
KnowledgeGraph->>localStorage: read persisted mode
KnowledgeGraph->>KnowledgeGraph2D: render effective 2D mode when WebGL2 is unavailable
KnowledgeGraph-->>Browser: expose disabled 3D toggle explanation
KnowledgeGraph->>KnowledgeGraph3D: mount 3D renderer when supported
KnowledgeGraph3D-->>KnowledgeGraph: report renderer failure
KnowledgeGraph->>KnowledgeGraph2D: render local 2D fallback
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 1
🤖 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/graph/KnowledgeGraph.tsx`:
- Line 141: In frontend/src/components/graph/KnowledgeGraph.tsx:141-141, wrap
the fallback KnowledgeGraph2D rendered by GraphCrashFallback for crashedMode ===
"3d" in a nested ErrorBoundary using the inline status placeholder as its
fallback, preventing 2D failures from reaching the root fallback. In
frontend/src/components/graph/KnowledgeGraph.test.tsx:30-50, add the throw2d
mock control; in frontend/src/components/graph/KnowledgeGraph.test.tsx:148-176,
add a regression test covering simultaneous 3D and 2D failures and asserting the
nested placeholder is rendered.
🪄 Autofix
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: 5c77b3cc-2ebe-41aa-af76-de2ad1793ce2
📒 Files selected for processing (3)
frontend/e2e/graph-webgl-fallback.spec.tsfrontend/src/components/graph/KnowledgeGraph.test.tsxfrontend/src/components/graph/KnowledgeGraph.tsx
| React.useEffect(() => { | ||
| if (crashedMode === "3d") heal("2d"); | ||
| }, [crashedMode, heal]); | ||
| if (crashedMode === "3d") return <KnowledgeGraph2D {...graphProps} />; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -e
printf'%s\n''--- KnowledgeGraph.tsx outline ---'
ast-grep outline frontend/src/components/graph/KnowledgeGraph.tsx
printf'%s\n''--- Relevant implementation ---'
sed -n '1,230p' frontend/src/components/graph/KnowledgeGraph.tsx
printf'%s\n''--- Relevant test structure ---'
sed -n '1,230p' frontend/src/components/graph/KnowledgeGraph.test.tsxRepository: SaplingLearn/Sapling
Length of output: 14232
🏁 Script executed:
#!/bin/bashset -e
printf'%s\n''--- ErrorBoundary definition and usages ---'
fd -i 'ErrorBoundary' frontend/src
rg -n -C 12 'class ErrorBoundary|function ErrorBoundary|<ErrorBoundary' frontend/src
printf'%s\n''--- GraphCrashFallback references ---'
rg -n -C 8 'GraphCrashFallback|crashedMode === "3d"|The graph couldn' frontend/src/components/graphRepository: SaplingLearn/Sapling
Length of output: 10536
🏁 Script executed:
#!/bin/bashset -e
cat -n frontend/src/components/ErrorBoundary.tsx
printf'%s\n''--- package and test scripts ---'if [ -f frontend/package.json ];then
sed -n '1,220p' frontend/package.json
fiif [ -f package.json ];then
sed -n '1,180p' package.json
fiRepository: SaplingLearn/Sapling
Length of output: 7291
🏁 Script executed:
#!/bin/bashset -e
python3 - <<'PY'from pathlib import Pathimpl = Path("frontend/src/components/graph/KnowledgeGraph.tsx").read_text()boundary = Path("frontend/src/components/ErrorBoundary.tsx").read_text()test = Path("frontend/src/components/graph/KnowledgeGraph.test.tsx").read_text()checks = { "outer boundary invokes GraphCrashFallback as its fallback": ( '<GraphCrashFallback crashedMode={mode} graphProps={props} heal={setMode} />' in impl ), "3D fallback directly renders 2D": ( 'if (crashedMode === "3d") return <KnowledgeGraph2D {...graphProps} />;' in impl ), "ErrorBoundary calls fallback from its own render": ( "if (this.props.fallback) return this.props.fallback(this.state.error, this.reset);" in boundary ), "2D mock has no throw2d control": ( "throw2d" not in test ), "existing crash test only arms 3D": ( "mockState.throw3d = true;" in test and "mockState.throw2d" not in test ),}for name, result in checks.items(): print(f"{'PASS' if result else 'FAIL'}: {name}")# This models the relevant React error-boundary rule:# an error thrown while an ErrorBoundary renders its own fallback# cannot be handled by that same boundary.print("MODEL: 3D child throws -> outer boundary renders GraphCrashFallback")print("MODEL: GraphCrashFallback renders 2D directly -> a 2D throw occurs in outer fallback render")print("MODEL: without a nested boundary, that throw propagates to the next ancestor boundary")PYRepository: SaplingLearn/Sapling
Length of output: 637
Contain a 2D failure during the 3D fallback.
When KnowledgeGraph3D throws, GraphCrashFallback renders KnowledgeGraph2D from the outer ErrorBoundary fallback. A 2D error then propagates to the root fallback. Wrap the fallback 2D graph in a nested ErrorBoundary with the inline status placeholder as its fallback. Add a throw2d mock control and regression test for simultaneous 3D and 2D failures.
📍 Affects 2 files
frontend/src/components/graph/KnowledgeGraph.tsx#L141-L141(this comment)frontend/src/components/graph/KnowledgeGraph.test.tsx#L30-L50frontend/src/components/graph/KnowledgeGraph.test.tsx#L148-L176
🤖 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 `@frontend/src/components/graph/KnowledgeGraph.tsx` at line 141, In
frontend/src/components/graph/KnowledgeGraph.tsx:141-141, wrap the fallback
KnowledgeGraph2D rendered by GraphCrashFallback for crashedMode === "3d" in a
nested ErrorBoundary using the inline status placeholder as its fallback,
preventing 2D failures from reaching the root fallback. In
frontend/src/components/graph/KnowledgeGraph.test.tsx:30-50, add the throw2d
mock control; in frontend/src/components/graph/KnowledgeGraph.test.tsx:148-176,
add a regression test covering simultaneous 3D and 2D failures and asserting the
nested placeholder is rendered.
…led toggle, hardened tests Reworks the crash containment so the persisted mode is never rewritten: the wrapper derives an EFFECTIVE mode (one gate encoding) from the raw persisted wish + a module-memoized WebGL2 probe, and a 3D crash re-probes capability instead of healing localStorage — capability really gone flips the effective mode (boundary re-keys, 2D mounts fresh); transient crashes get a static placeholder with the boundary's reset wired to Try again. The fallback never renders another graph component (a throw during a boundary's own fallback render is uncatchable by that boundary), and crashes now log in production. The toggle swaps native disabled for aria-disabled + aria-describedby so keyboard/SR users can reach the "3D requires WebGL" reason; the action stays the accessible name. Test hardening from the review: crash sentinels throw from a layout effect (the real #538 crash phase); getContext is stubbed via vi.spyOn so restoreAllMocks actually restores; storage-key/sync-event constants are exported and imported by the unit tests (journey mirrors by comment); SYNC_EVENT gating has direct coverage; the journey anchors its negative assertions on the new error-fallback testid, re-asserts end state, and pins the positive path (toggle enabled under real WebGL); the four divergent next/dynamic passthrough mocks collapse into a shared hook-safe helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Aug 13, 2026
CollaboratorAuthor
Applied all 15 code-review findings in d6cfaea (design rework where the review asked for one, not point patches):
Verification: unit suite 627/627, eslint 0 errors (stale suppressions pruned), tsc clean, full e2e lane 44/44 (both #538 journeys) + oracles clean, run from a fresh worktree via one flock'd up→test→down cycle. 🤖 Generated with Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Fixes#538.
The bug
A browser without WebGL that lands on the dashboard (or Tree/Learn) with
localStorage["sapling.kg.mode"] = "3d"persisted collapses the entire app into the root error fallback ("We hit a snag"), permanently — "Try again" re-throws. Chain:KnowledgeGraphwrapper honoured the persisted"3d"unconditionally — no WebGL capability check existed anywhere in the frontend.react-force-graph-3dmounts its engine synchronously in auseLayoutEffect(react-kapsule), constructingnew three.WebGLRenderer(...).Error creating WebGL context.from the constructor whencanvas.getContext("webgl2")returns null.app/layout.tsx), so the whole tree unmounted.A profile that toggled 3D on a WebGL-capable browser was locked out of the app the moment WebGL went away (disabled, GPU blocklist, remote desktop, VM). Fresh profiles were unaffected (mode defaults to 2D) — which is why this read as "used to work".
The fix (both in the wrapper, so Dashboard/Tree/Learn are all covered)
webgl2Available()): probecanvas.getContext("webgl2")once per wrapper mount (probe context released viaWEBGL_lose_contextso probes never count toward the per-page context cap). Without WebGL2:"3d"is ignored, not rewritten — the preference survives for the profile's WebGL-capable browsers;"2d"; the heal flips the boundary key, clearing the error state into a normal 2D render. A (never-observed) 2D crash renders a minimal placeholder instead of remounting the thing that just threw.Tests
KnowledgeGraph.test.tsx, 6 tests, written first and watched fail): fresh-profile default pin; 3d-honoured-with-WebGL pin; forces-2D-without-WebGL2; toggle disabled with explanatory label; toggle enabled with WebGL; crash containment + mode heal (RED run reproduced the exact production throw escaping uncaught).e2e/graph-webgl-fallback.spec.ts): init script nulls the webgl/webgl2 branches ofgetContext(exactly the API three probes; 2D canvas stays real) and seeds the poisoned"3d"mode → dashboard must render the 2D SVG graph, no root fallback, toggle disabled, preference preserved.Verification
vitest run: 72 files / 624+ tests green (against a jsdom that is itself a no-WebGL environment, so every existing screen test now exercises the gated path).tsc --noEmitclean;eslintclean on changed files (no new suppressions).next buildclean."We hit a snag"count-0 assertion (exit 1) — the root fallback was live on the page.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests