Skip to content

fix(canvas): restore metrics on page reload and IPFS view#224

Merged
tnkshuuhei merged 3 commits intodevfrom
fix/loading-canvas
Mar 26, 2026
Merged

fix(canvas): restore metrics on page reload and IPFS view#224
tnkshuuhei merged 3 commits intodevfrom
fix/loading-canvas

Conversation

@tnkshuuhei
Copy link
Member

@tnkshuuhei tnkshuuhei commented Mar 26, 2026

Summary

  • Fix bug where metric badges disappeared after page reload or when viewing IPFS-saved canvases
  • cardsToNodes() did not include metrics in node.data.metrics, so CardNode could not render badges after state restoration
  • Merge cardMetrics into node data during both initialization and localStorage hydration

Related Issues

Test plan

  • Verify metric badges display after agent generates a logic model
  • Verify metric badges persist after page reload
  • Verify metric badges display when viewing IPFS-saved canvas via URL
  • Verify bun run build passes without errors

…ge read

Move loadCanvasState() from useMemo (runs during SSR/hydration) to useEffect
(runs after mount) so server and client initial renders produce identical HTML.
cardsToNodes() did not include metrics in node data, causing metric
badges to disappear after reload or when viewing IPFS-saved canvases.
Merge cardMetrics into node data during initialization and hydration.
@vercel
Copy link

vercel bot commented Mar 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
muse Ready Ready Preview, Comment Mar 26, 2026 7:14am

@claude
Copy link

claude bot commented Mar 26, 2026

PR Review: fix(canvas): restore metrics on page reload and IPFS view

Good fix overall — the root cause is correctly identified and the solution is sound. A few items worth addressing:


Bug: Potential runtime error with legacy localStorage data

components/canvas/context/CanvasContext.tsx:155

metrics: savedState.cardMetrics[node.id],

If a user has existing localStorage data saved before cardMetrics was part of the schema, savedState.cardMetrics could be undefined, causing a runtime crash that wipes the canvas. Defensive access is cheap here:

metrics: savedState.cardMetrics?.[node.id],

Same applies to setCardMetrics(savedState.cardMetrics) on line 160 — worth guarding with ?? {}:

setCardMetrics(savedState.cardMetrics ?? {});

Design concern: Dual source of truth for metrics

This PR now stores metric data in two places simultaneously:

  • node.data.metrics (used by CardNode to render badges and by editingNodeData)
  • cardMetrics state (used by auto-save, IPFS upload, and export)

They're initialized together correctly here, but any downstream operation that calls setCardMetrics without a corresponding setNodes update (or vice versa) will cause silent divergence. It's worth auditing createCanvasOperations to confirm all metric mutation paths update both, or adding a comment here explicitly documenting the invariant that these two must remain in sync.


Minor: Brief flash of unsaved state on reload

The deferred useEffect hydration is the right call for SSR correctness, but there's now a brief moment on reload where the canvas renders with initialCards (empty) before hydrating from localStorage. On fast devices this is imperceptible, but on slower connections it could produce a visible flicker. A simple isHydrated state flag and a skeleton/loading overlay on the canvas would address this if it becomes a user-visible issue.


Nit: hasHydrated guard vs. dependency array

The hasHydrated.current guard is effective, but the effect's dependency array [disableLocalStorage, setNodes, setEdges] implies the effect might legitimately re-run when disableLocalStorage changes — but it won't due to the guard. If disableLocalStorage ever flips from truefalse after mount, the canvas won't hydrate. This edge case is probably fine for current usage, but a comment explaining the intentional single-run behavior would help future readers.


Positives

  • Moving loadCanvasState() from useMemo to useEffect is exactly the right SSR-safe pattern — this correctly resolves the hydration mismatch.
  • Metrics are now consistently merged into node data at both the initialization path (IPFS view) and the hydration path (localStorage reload).
  • The hasHydrated ref correctly prevents double-hydration.
  • Clear commit messages and test plan.

The legacy data guard (first issue) is the one worth fixing before merge; the rest are lower priority.

@tnkshuuhei tnkshuuhei merged commit 9254092 into dev Mar 26, 2026
3 checks passed
@tnkshuuhei tnkshuuhei deleted the fix/loading-canvas branch March 26, 2026 07:17
tnkshuuhei added a commit that referenced this pull request Mar 26, 2026
git log --pretty=format: does not append a trailing newline.
When the output is a single line, `while read` sees EOF without
a newline and skips the loop body entirely, silently dropping
the entry (e.g. PR #224 was missing from the release summary).

Switching to tformat: adds a terminating newline after each entry.
@github-actions github-actions bot mentioned this pull request Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: metric badges disappear after page reload and on IPFS-saved canvas view

1 participant