feat(canvas): begin canvas v2 implementation - #12
Merged
Merged
Conversation
- exclude elkjs from Storybook dependency optimization - externalize optional web-worker and mermaid deps during bundling - set Storybook worker output to es to match existing app behavior
- document a node-backed scene graph model for pages, databases, URLs, media, shapes, and connectors - ground the recommendation in the current canvas, editor, data, and shell architecture - capture phased implementation, performance strategy, collaboration model, and validation checklist
- add a deeper AFFiNE release-note analysis across 2024-2025 canvas and cross-surface features - classify AFFiNE ideas into adopt-now, adopt-next, defer, and reinterpret buckets for xNet - thread center-peek, aliases, backlinks, drag/drop parity, and block references into the roadmap and checklists
- add a strategic README for Canvas V2 with phase overview, goals, budgets, and release gates - add ten sequential step files covering scene model, renderer runtime, drop ingestion, content objects, UX, collaboration, and rollout - keep the plan content-first, Electron-first, hook-driven, and explicitly non-backward-compatible
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughLarge Canvas V2 landing: introduces a typed, node-backed infinite canvas with ingestion, chunked storage, edge/anchor bindings, rich rendering, keyboard/command tooling, inline previews, test harnesses, Storybook/Vite adjustments, extensive tests, and expanded public Canvas APIs across Electron and Web apps. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Surface as CanvasView
participant Ingest as Ingestion
participant Store as ChunkStoreAdapter
participant ChunkMgr as ChunkManager
participant Render as RenderLayer
User->>Surface: Drop / Paste payload
Surface->>Ingest: extractCanvasIngressPayloads(payload)
Ingest->>Ingest: normalize/describe (url/file/internal)
Ingest->>Store: createSourceBackedCanvasNode / createCanvasPrimitiveNode
Store->>ChunkMgr: addNode(node)
ChunkMgr->>ChunkMgr: index chunk, classify edges
ChunkMgr->>Render: notify render update
Render->>User: node appears in canvas
sequenceDiagram
participant User
participant UI as CanvasView
participant Cmd as CommandPalette / Keyboard Hook
participant Doc as Y.Doc
participant Undo as UndoManager
User->>UI: Select nodes / trigger command (Align)
Cmd->>UI: onAlignSelection
UI->>Doc: doc.transact(apply alignment updates)
Doc->>Undo: record per-domain undo boundary
UI->>UI: update command state -> onCommandStateChange
UI->>User: UI reflects aligned nodes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
Contributor
|
Preview removed for PR #12. |
- add source-backed canvas object kinds, a media asset schema, and shell helpers for page, database, and note objects - mount the shared minimap in the canvas runtime and align the Electron shell with page-backed canvas creation flows - expand the Canvas V2 plan with explicit Electron E2E and performance gates plus progress checklist updates - add initial Electron CDP smoke coverage for the canvas shell, minimap, and command flows
- fix Canvas viewport culling invalidation so newly inserted source-backed objects render without requiring camera movement - add lightweight canvas surface instrumentation for Electron automation and extend the Electron canvas spec to handle native rebuilds, renderer targeting, and command/minimap flows reliably - make the devtools FAB offset configurable and move the Electron dev FAB out of the canvas control lane to avoid minimap toggle collisions - update the Canvas V2 plan checklists to reflect the hybrid shell and Electron CDP progress
- expose node render context so the shell can gate heavy content by zoom and selection - mount page-backed inline editors for active page and note objects while keeping other cards light - stop drag/open leaks through interactive content and cover it with unit and Electron CDP tests
- mount bounded database previews for selected canvas database nodes - settle empty-database loading in useDatabase with a regression test - extend Electron CDP coverage for database preview and focus-return flows
- add a shared seeded-scene fixture for Storybook, renderer tests, and Electron runtime seeding - expose minimap and query diagnostics so CDP tests can assert bounded DOM and hook stability - cover dense-scene minimap, frame-budget, and query-threshold behavior in the Canvas V2 rollout plan
- expand the shared canvas keyboard hook with scoped create, peek, help, selection-step, and nudge shortcuts - add a minimal canvas selection HUD, shortcut help overlay, and canvas-scoped command palette actions in Electron - cover the shortcut layer with canvas unit tests and Electron Playwright typing-guard flows
- add a shared drop and paste ingestion pipeline for internal drags, urls, and media - wire electron and web canvas shells through the new source-backed placement flow - add web and canvas package coverage for ingestion behavior and rollout docs
- keep database previews bounded with a virtualized row window - cap preview loads so split surfaces stay light under dense tables - add an electron split-view workflow from the hud, shortcut, and command palette - extend canvas and electron coverage for split interactions and updated rollout docs
- build shared canvas display lists for visible, DOM, and overview object sets - render far-field objects through a canvas overview layer and keep dense DOM islands bounded - aggregate dense minimap rendering and extend web/Electron verification for the new runtime metrics
- add a flat Yjs canvas chunk adapter and generalize the chunk manager store contract - route the shared canvas hook and renderer through chunk-backed render subsets - add dense-scene web and Electron e2e coverage plus flat chunk store unit tests - check off spatial runtime plan milestones for chunk activation and large-scene validation
- add explicit inline and peek modes for page and database canvas surfaces - introduce centered peek state with escape and backdrop dismissal in the Electron shell - cover page peek to focus transitions in Electron Playwright and check off the page-peek plan items
- resolve shared light/dark tokens for the canvas surface, grid, minimap, and navigation chrome - add package tests and Electron/web Playwright coverage for canvas theme regressions - record release-gate updates and include a PR artifact screenshot for review comments
Owner
Author
|
Canvas V2 screenshot artifact from the latest Electron e2e validation after the theme-aware canvas chrome pass.
Validated on commit
|
- reuse one provider-aware external reference card across web and Electron shells - preserve embed metadata when existing ExternalReference nodes are placed on canvas - add ingestion coverage plus web and Electron e2e for YouTube, Twitter, and Figma drops
…editor - add a shared external-reference and embed parser in @xnetjs/data - route canvas ingestion and editor smart/embed providers through the shared parser - refresh web and Electron canvas embed screenshot artifacts for the PR
Owner
Author
|
Shared embed parsing is now unified across canvas and editor in What changed:
Focused verification that passed:
Artifacts: |
- size external-reference canvas nodes from shared embed metadata instead of generic link-card defaults - verify useful YouTube, X, and Figma embed dimensions in focused web and Electron e2e coverage - refresh PR artifacts for the updated canvas embed surfaces
Owner
Author
|
Embed drops are now sized by provider in What changed:
Verification that passed:
Artifacts: |
- align CSS fallback grid offsets with the same centered viewport math as canvas nodes - flip WebGL grid screen coordinates to top-left space and render grid updates before paint - refresh large-scene web and Electron performance artifacts after the pan and zoom validation runs
Owner
Author
|
Grid projection is locked to the viewport in What changed:
Verification that passed:
Artifacts: |
- extend shared external-reference parsing with Instagram and TikTok providers - allow the new providers in the external-reference schema and provider normalization path - widen web and Electron CSP plus end-to-end canvas coverage for dropped social embeds - capture refreshed web and Electron canvas embed screenshots for the draft PR
Owner
Author
|
Canvas embed drop update at Instagram and TikTok URLs now land as source-backed canvas embeds through the shared external-reference pipeline in both web and Electron. Web canvas social embed coverage:
Electron canvas social embed coverage:
Verification in this push:
|
crs48
marked this pull request as ready for review
March 10, 2026 21:10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/canvas/src/chunks/chunked-canvas-store.ts (1)
403-415:⚠️ Potential issue | 🟡 MinorNull safety issue in statistics aggregation.
If any chunk lacks
CANVAS_OBJECTS_MAP_KEY,nodes.sizewill throw. UnlikeloadChunk(line 71) which uses optional chaining, this method assumes the map always exists.🛡️ Proposed fix with optional chaining
getStats(): { chunkCount: number; nodeCount: number; crossEdgeCount: number } { let nodeCount = 0 this.chunks.forEach((chunk) => { - const nodes = chunk.get(CANVAS_OBJECTS_MAP_KEY) as Y.Map<unknown> - nodeCount += nodes.size + const nodes = chunk.get(CANVAS_OBJECTS_MAP_KEY) as Y.Map<unknown> | undefined + nodeCount += nodes?.size ?? 0 }) return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/chunks/chunked-canvas-store.ts` around lines 403 - 415, getStats currently assumes every chunk has CANVAS_OBJECTS_MAP_KEY and calls nodes.size which can throw if the map is missing; update getStats (the method named getStats in chunked-canvas-store.ts) to safely handle absent maps by using optional chaining or a null-check on chunk.get(CANVAS_OBJECTS_MAP_KEY) and treat missing maps as size 0 (similar to loadChunk's usage). Ensure you reference CANVAS_OBJECTS_MAP_KEY and cast only after confirming existence so nodeCount increments by 0 for missing maps and crossEdgeCount/chunkCount logic remains unchanged.packages/canvas/src/components/Minimap.tsx (2)
469-490:⚠️ Potential issue | 🟡 MinorSet both minimap toggle buttons to
type="button".Without an explicit type these default to
submit, so embedding the canvas shell inside a form or dialog can trigger an unexpected form submission.🔧 Proposed fix
<button + type="button" onClick={() => setIsExpanded(false)} style={{ @@ <button + type="button" onClick={() => setIsExpanded(true)} style={{Also applies to: 498-516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/components/Minimap.tsx` around lines 469 - 490, The minimap toggle buttons in Minimap.tsx are missing an explicit type and therefore default to type="submit", causing accidental form submissions; update the buttons that call setIsExpanded(false) (the button with data-canvas-minimap-toggle="hide") and the corresponding expand toggle (the other minimap toggle around lines 498-516 that calls setIsExpanded(true) or similar) to include type="button" so they do not submit enclosing forms or dialogs.
391-404:⚠️ Potential issue | 🟠 Major
Minimapneeds an unanchored mode before it can be wrapped here.Passing
classNamethrough this wrapper does not overrideMinimap’s inlineposition: 'absolute', bottom: 16, right: 16. In the expanded branch that leaves the panel and the hide button positioned off a 0×0 wrapper instead of behaving like one composed control.🛠️ Proposed fix
export interface MinimapProps { /** All canvas nodes */ nodes: CanvasNode[] @@ /** Show edge lines */ showEdges?: boolean + /** Whether the minimap should anchor itself to the viewport */ + anchored?: boolean } @@ export function Minimap({ nodes, edges, @@ className, backgroundColor, - showEdges = true + showEdges = true, + anchored = true }: MinimapProps) { @@ style={{ - position: 'absolute', - bottom: 16, - right: 16, + ...(anchored + ? { + position: 'absolute', + bottom: 16, + right: 16 + } + : {}), borderRadius: 8, overflow: 'hidden', boxShadow: theme.panelShadow, @@ <Minimap {...props} - // Override position since we're wrapping + anchored={false} className={props.className} />Also applies to: 464-468
packages/canvas/src/hooks/useCanvasKeyboard.ts (1)
169-189:⚠️ Potential issue | 🟡 MinorRespect
minZoom/maxZoomfor reset and fit-to-content too.
maxZoomandminZoomare only enforced on+/-right now.Ctrl/Cmd+0still resets to1, andCtrl/Cmd+1still hardcodes0.1, so callers with non-default bounds can still end up with an out-of-range viewport.🔧 Suggested fix
if (isMod && (e.key === '0' || e.code === 'Digit0')) { e.preventDefault() - onViewportChange({ x: 0, y: 0, zoom: 1 }) + onViewportChange({ + x: 0, + y: 0, + zoom: Math.min(maxZoom, Math.max(minZoom, 1)) + }) return }- const newZoom = Math.max(0.1, Math.min(scaleX, scaleY, 1)) + const newZoom = Math.min( + maxZoom, + Math.max(minZoom, Math.min(scaleX, scaleY, 1)) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/hooks/useCanvasKeyboard.ts` around lines 169 - 189, The reset (Ctrl/Cmd+0) and fit-to-content (Ctrl/Cmd+1) handlers currently set zoom to raw values (1 and computed newZoom) without respecting configured minZoom/maxZoom; update both handlers to clamp their target zoom using the existing minZoom and maxZoom (e.g., compute targetZoom = Math.max(minZoom, Math.min(targetZoom, maxZoom))) before calling onViewportChange so the reset and fit-to-content actions abide by the same zoom bounds as the +/- handlers; reference the onViewportChange call in the reset branch and the computed newZoom used in the fit-to-content branch and replace those zoom values with the clamped target zoom.packages/canvas/src/layers/edge-renderer.ts (1)
278-307:⚠️ Potential issue | 🟠 MajorKeep the canvas bezier routing consistent with the SVG renderer.
packages/canvas/src/edges/CanvasEdgeComponent.tsxLines 33-44 switch to vertical control points when|dy| > |dx|, but this canvas path still hard-codes horizontal tangents. Now that explicittop/bottombindings come throughresolveCanvasAnchorPoint, the same connector can change shape and arrow direction when the runtime switches between SVG and canvas edge modes.🔧 Suggested fix
if (style.curved) { // Bezier curve const dx = targetAnchor.x - sourceAnchor.x - - // Control points for smooth S-curve (horizontal tangents) - const cx1 = sourceAnchor.x + dx * 0.4 - const cy1 = sourceAnchor.y - const cx2 = targetAnchor.x - dx * 0.4 - const cy2 = targetAnchor.y + const dy = targetAnchor.y - sourceAnchor.y + let cx1 = sourceAnchor.x + let cy1 = sourceAnchor.y + let cx2 = targetAnchor.x + let cy2 = targetAnchor.y + + if (Math.abs(dx) > Math.abs(dy)) { + cx1 = sourceAnchor.x + dx * 0.4 + cx2 = targetAnchor.x - dx * 0.4 + } else { + cy1 = sourceAnchor.y + dy * 0.4 + cy2 = targetAnchor.y - dy * 0.4 + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/layers/edge-renderer.ts` around lines 278 - 307, The bezier control points are hard-coded for horizontal tangents causing inconsistent routing vs the SVG renderer; update the curved branch in the edge rendering logic (where resolveCanvasAnchorPoint is used and style.curved is checked) to compute dx = targetAnchor.x - sourceAnchor.x and dy = targetAnchor.y - sourceAnchor.y and choose horizontal tangents when |dx| >= |dy| (current behavior) but switch to vertical tangents when |dy| > |dx| (same heuristic used in CanvasEdgeComponent). For the vertical case set cx1 = sourceAnchor.x, cy1 = sourceAnchor.y + dy * 0.4, cx2 = targetAnchor.x, cy2 = targetAnchor.y - dy * 0.4; for the horizontal case keep the existing cx1/cx2/cy1/cy2 formulas; then compute endDirection from (targetAnchor - controlPoint2) consistently so arrow direction matches SVG.
🟡 Minor comments (12)
packages/canvas/src/__tests__/node-kind.test.ts-16-29 (1)
16-29:⚠️ Potential issue | 🟡 MinorCover the literal
type: 'frame'branch too.
packages/canvas/src/scene/node-kind.tshas a separatenode.type === 'frame'branch, but this suite only exercises thegroup + containerRole === 'frame'path. A regression in direct frame nodes would still pass here.➕ Suggested test addition
it('treats frame-role groups as frame display kinds', () => { + expect( + getCanvasResolvedNodeKind({ + type: 'frame', + properties: {} + }) + ).toBe('frame') + expect( + isFrameLikeCanvasNode({ + type: 'frame', + properties: {} + }) + ).toBe(true) + expect( getCanvasResolvedNodeKind({ type: 'group', properties: { containerRole: 'frame' }Based on learnings, "Applies to packages/**/*.test.{ts,tsx} : Write unit tests for core packages".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/node-kind.test.ts` around lines 16 - 29, Add a test that exercises the direct frame node branch by calling getCanvasResolvedNodeKind and isFrameLikeCanvasNode with an object where type is the literal string 'frame' (e.g., { type: 'frame' }) and assert the resolved kind is 'frame' and isFrameLikeCanvasNode returns true; update the existing packages/canvas/src/__tests__/node-kind.test.ts to include this additional case so both the group+containerRole and the literal node.type === 'frame' paths (functions getCanvasResolvedNodeKind and isFrameLikeCanvasNode) are covered.packages/canvas/src/nodes/CanvasPrimitiveNodeContent.tsx-161-173 (1)
161-173:⚠️ Potential issue | 🟡 MinorMinor: Pluralization for item count."1 items" would display incorrectly for a single item. For simple cases like "item/items", a ternary check
count === 1 ? "" : "s"handles proper pluralization.Proposed fix
{memberIds.length > 0 ? ( <span style={{ flexShrink: 0, color: theme.panelMutedText, fontSize: 11, letterSpacing: '0.08em', textTransform: 'uppercase' }} > - {memberIds.length} items + {memberIds.length} {memberIds.length === 1 ? 'item' : 'items'} </span> ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/nodes/CanvasPrimitiveNodeContent.tsx` around lines 161 - 173, The item-count label currently renders "{memberIds.length} items" which yields "1 items"; update the rendering in CanvasPrimitiveNodeContent (the span that displays memberIds.length) to choose the correct singular or plural word by checking memberIds.length === 1 ? 'item' : 'items' so it shows "1 item" and "N items" accordingly.packages/canvas/src/nodes/CanvasPrimitiveNodeContent.tsx-97-97 (1)
97-97:⚠️ Potential issue | 🟡 MinorUnsafe type assertion for
titleproperty.The pattern
node.properties.title as stringbypasses type checking. Other property reads intoShapeNodeDatausetypeofguards for safety. Apply the same pattern here for consistency and runtime safety.Proposed fix
- const title = node.alias ?? (node.properties.title as string) ?? 'Frame' + const title = node.alias ?? (typeof node.properties.title === 'string' ? node.properties.title : 'Frame')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/nodes/CanvasPrimitiveNodeContent.tsx` at line 97, The title assignment uses an unsafe type assertion; replace the `node.properties.title as string` usage in CanvasPrimitiveNodeContent (the `title` const) with a runtime typeof guard like other reads in toShapeNodeData: check `typeof node.properties.title === 'string'` and use that value only if true, otherwise fall back to `node.alias` and finally `'Frame'`, keeping the same nullish-coalescing order.docs/explorations/0108_[_]_CANVAS_V1_PAGES_DATABASES_AND_INFINITE_CANVAS_DEEP_DIVE.md-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorClarify the version naming: V1 vs V2.
The document title references "Canvas V1" but the PR summary, commit messages, and AI summary all describe this work as part of "Canvas V2" implementation. This creates potential confusion about which version is being documented.
Possible interpretations:
- This document describes the "V1 product cut" of Canvas V2 (i.e., first stable release of the V2 rewrite)
- There's a naming inconsistency between the exploration and implementation work
Consider updating the title or adding a clarifying note in the opening section to explain the relationship between "Canvas V1" in this exploration and "Canvas V2" in the broader PR context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/explorations/0108_`[_]_CANVAS_V1_PAGES_DATABASES_AND_INFINITE_CANVAS_DEEP_DIVE.md at line 1, The document title "# 0108 - Canvas V1 Pages, Databases, Drops, and Infinite Canvas Deep Dive" conflicts with PR/commit messages that call this work "Canvas V2"; update the title or add a short clarifying note in the opening paragraph explaining that "Canvas V1" in this exploration refers to the initial product cut or first-stable release of the Canvas V2 rewrite (or whatever exact relationship is accurate), so readers understand the naming choice and how it maps to the PR/commit labels.packages/canvas/src/__tests__/accessibility.test.ts-236-240 (1)
236-240:⚠️ Potential issue | 🟡 MinorAssert the announced text, not just that it doesn't throw.
This change is exercising the new resolved-kind path, but the test still passes if the label mapping regresses or if the announcement string is wrong. Please assert the live-region content after the RAF callback, e.g.
"Page: My Page".As per coding guidelines,
packages/**/*.test.{ts,tsx}: Write unit tests for core packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/accessibility.test.ts` around lines 236 - 240, The test currently only ensures Announcer.announceNodeFocus doesn't throw; change it to assert the actual live-region announcement string after the RAF callback: instantiate Announcer, call announcer.announceNodeFocus({ type: 'page', properties: { title: 'My Page' } }), advance or await the RAF/microtask that the implementation uses, then read the live region content (the DOM node the Announcer updates) and assert it equals the expected string "Page: My Page"; use the Announcer class and announceNodeFocus symbol to locate the test and update the assertion accordingly.packages/canvas/src/__tests__/minimap.test.ts-247-259 (1)
247-259:⚠️ Potential issue | 🟡 MinorThe sort assertion never exercises the new frame metadata.
The comparator on Line 248 and Line 249 still treats every
groupas a container and never readsproperties.containerRole, so the new frame fixture is irrelevant to the behavior under test. If frame ordering now depends oncontainerRole, mirror that predicate here; otherwise a frame-detection regression will still pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/minimap.test.ts` around lines 247 - 259, The test's comparator treats any node with type === 'group' as a container and ignores properties.containerRole, so update the sort predicate used on nodes (the comparator that defines aIsContainer/bIsContainer) to treat a node as a container when either node.type === 'group' OR node.properties?.containerRole === 'frame'; do the same for bIsContainer so the new frame fixture is actually ordered as a container and the subsequent assertions (sortedNodes[0].properties.containerRole, sortedNodes[1].type, etc.) exercise frame ordering.packages/canvas/src/__tests__/minimap.test.ts-46-67 (1)
46-67:⚠️ Potential issue | 🟡 MinorThese “color mapping” tests still don’t verify color mapping.
All three updated cases only reassert the fixture’s
typeorcontainerRole. A broken minimap palette forpage,media, or frame groups would still pass here. Please assert the actual color resolver output or rendered fill/class instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/minimap.test.ts` around lines 46 - 67, The tests currently only reassert the fixture fields (createTestNode and node.properties.containerRole) instead of verifying actual colors; update each case to assert the minimap color output by invoking the component's color resolver (e.g., resolveNodeColor / getMinimapColor) with the created node or by shallow-rendering the Minimap and checking the rendered element's fill/class for that node, replacing expect(node.type).toBe(...) and expect(node.properties.containerRole).toBe(...) with assertions that compare the resolver/rendered fill/class to the expected color for 'page', 'shape', 'media', and frame-group nodes.packages/canvas/src/chunks/chunked-canvas-store.ts-143-144 (1)
143-144:⚠️ Potential issue | 🟡 MinorPotential null dereference if chunk lacks expected map.
chunk.get(CANVAS_OBJECTS_MAP_KEY)may returnundefinedfor chunks created before this change or via external manipulation. The type assertionas Y.Map<unknown>doesn't prevent runtime errors when calling methods onundefined.Consider adding a guard or migrating existing chunks to ensure the map exists:
🛡️ Proposed defensive check
const chunk = this.chunks.get(chunkKey as ChunkKey) if (!chunk) return const nodes = chunk.get(CANVAS_OBJECTS_MAP_KEY) as Y.Map<unknown> + if (!nodes) return const node = nodes.get(nodeId) as CanvasNode if (!node) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/chunks/chunked-canvas-store.ts` around lines 143 - 144, The code calls const nodes = chunk.get(CANVAS_OBJECTS_MAP_KEY) as Y.Map<unknown> and then uses nodes.get(nodeId), which can throw if the map is undefined; update the logic in chunked-canvas-store.ts (the block using chunk, CANVAS_OBJECTS_MAP_KEY, nodeId and CanvasNode) to guard and/or migrate: check if nodes is falsy and either (a) create and set a new Y.Map() into chunk under CANVAS_OBJECTS_MAP_KEY before using it (persisting the migration), or (b) return/handle a missing-node case (e.g., return null or log and skip) — ensure you reference the same variables (chunk, CANVAS_OBJECTS_MAP_KEY, nodes, nodeId) and avoid a raw type assertion to prevent runtime null deref.docs/reference/canvas-v2-release-gates.md-117-118 (1)
117-118:⚠️ Potential issue | 🟡 MinorMinor grammar fix: Use hyphen for compound modifier.
The phrase "3,969-node wide scene" should use a hyphen to form the compound adjective: "3,969-node-wide scene".
- - The current large-scene gate now seeds a `3,969`-node wide scene and explicitly validates + - The current large-scene gate now seeds a `3,969`-node-wide scene and explicitly validates🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/canvas-v2-release-gates.md` around lines 117 - 118, Replace the phrase "3,969-node wide scene" with the hyphenated compound adjective "3,969-node-wide scene" in the release gates doc so the compound modifier is grammatically correct; search for the string "3,969-node wide scene" in docs/reference/canvas-v2-release-gates.md and update it to "3,969-node-wide scene".packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx-249-276 (1)
249-276:⚠️ Potential issue | 🟡 MinorUse block-bodied
forEachcallbacks here to satisfy Biome.The concise arrows on Line 256, Line 265, and Line 276 return each listener’s result, which triggers
lint/suspicious/useIterableCallbackReturnand leaves this test file lint-red.🔧 Suggested fix
- listeners.forEach((listener) => listener()) + listeners.forEach((listener) => { + listener() + })Apply the same change at all three call sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/src/__tests__/canvas-navigation-shell.test.tsx` around lines 249 - 276, Three forEach callbacks in the test mock are using concise arrow returns which triggers the lint rule; update the three callers inside setLocalState, setLocalStateField, and setRemoteState so they use block-bodied arrow callbacks that call each listener inside the body (i.e., change the concise form that returns listener() to an explicit block that calls listener(); and does not return its value) to satisfy the lint rule.docs/plans/plan03_9_83CanvasV2/05-page-cards-inline-editing-and-peek.md-91-103 (1)
91-103:⚠️ Potential issue | 🟡 MinorThe sample render policy drops the documented near-field preview path.
Earlier in the doc, the lifecycle says pages move from compact to preview when they are selected or near-field, but this function has no
nearFieldinput and returns'compact'for every unselected card. If this snippet is used as the implementation reference, near-field previews can never happen.Suggested doc fix
function resolvePageRenderMode(input: { zoom: number selected: boolean + nearField: boolean editing: boolean peeking: boolean }): 'far' | 'compact' | 'preview' | 'editing' | 'peek' { if (input.editing) return 'editing' if (input.peeking) return 'peek' if (input.zoom < 0.2) return 'far' if (input.zoom < 0.55) return 'compact' - return input.selected ? 'preview' : 'compact' + return input.selected || input.nearField ? 'preview' : 'compact' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/plan03_9_83CanvasV2/05-page-cards-inline-editing-and-peek.md` around lines 91 - 103, The resolvePageRenderMode function omits the near-field preview path; add a nearField: boolean parameter to the input object and update the decision order so that if input.nearField (checked after editing/peeking) the function returns 'preview' (so unselected but near-page shows preview); ensure the final fallback still returns input.selected ? 'preview' : 'compact' and adjust any callers/docs to pass the new nearField flag.apps/electron/src/renderer/components/CanvasDatabasePreviewSurface.test.tsx-29-36 (1)
29-36:⚠️ Potential issue | 🟡 MinorResizeObserver mock is missing
unobservemethod.The
ResizeObserverMockclass only implementsobserveanddisconnect. If the component or any library code callsunobserve, this will throw. Consider adding it for completeness.Proposed fix
class ResizeObserverMock { observe() {} + unobserve() {} disconnect() {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/electron/src/renderer/components/CanvasDatabasePreviewSurface.test.tsx` around lines 29 - 36, The ResizeObserver mock used in the test setup (the ResizeObserverMock class inside beforeAll) is missing an unobserve method which can cause runtime errors if code calls unobserve; add a no-op unobserve() method to ResizeObserverMock (matching the real ResizeObserver API) so observe, unobserve, and disconnect are all present before stubbing via vi.stubGlobal('ResizeObserver', ResizeObserverMock).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fae12d9-074b-4391-a082-e2daec8bdd6f
⛔ Files ignored due to path filters (30)
docs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-aliases.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-collaboration.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-comments.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-connectors.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-instagram-tiktok-embeds.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-large-scene-performance.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-marquee-selection.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-performance-scene.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-presence.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-primitives.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-resize.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-shell.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-undo-boundary.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-electron-canvas-unified-dock.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-aliases.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-comments.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-compact-chrome.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-instagram-tiktok-embeds.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-keyboard-focus.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-large-scene-performance.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-performance.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-presence.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-primitives.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/2026-03-10-web-canvas-resize.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/electron-canvas-embeds.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/electron-canvas-performance-scene.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/electron-canvas-themes.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/web-canvas-embeds.pngis excluded by!**/*.pngdocs/pr-artifacts/canvas-v2/web-canvas-themes.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (132)
.storybook/main.tsapps/electron/src/renderer/App.tsxapps/electron/src/renderer/components/ActionDock.tsxapps/electron/src/renderer/components/CanvasDatabasePreviewSurface.test.tsxapps/electron/src/renderer/components/CanvasDatabasePreviewSurface.tsxapps/electron/src/renderer/components/CanvasInlinePageSurface.tsxapps/electron/src/renderer/components/CanvasView.tsxapps/electron/src/renderer/components/DatabaseView.tsxapps/electron/src/renderer/components/PageView.tsxapps/electron/src/renderer/components/Sidebar.tsxapps/electron/src/renderer/hooks/useCanvasSourceReferences.tsapps/electron/src/renderer/index.htmlapps/electron/src/renderer/lib/canvas-shell.test.tsapps/electron/src/renderer/lib/canvas-shell.tsapps/electron/src/renderer/main.tsxapps/web/index.htmlapps/web/package.jsonapps/web/src/components/CanvasView.tsxapps/web/src/components/Sidebar.tsxapps/web/src/main.tsxdocs/explorations/0108_[_]_CANVAS_V1_PAGES_DATABASES_AND_INFINITE_CANVAS_DEEP_DIVE.mddocs/plans/plan03_9_83CanvasV2/01-scene-graph-and-node-primitives.mddocs/plans/plan03_9_83CanvasV2/02-hybrid-shell-and-renderer-runtime.mddocs/plans/plan03_9_83CanvasV2/03-spatial-runtime-and-query-evolution.mddocs/plans/plan03_9_83CanvasV2/04-drop-ingestion-and-source-object-creation.mddocs/plans/plan03_9_83CanvasV2/05-page-cards-inline-editing-and-peek.mddocs/plans/plan03_9_83CanvasV2/06-database-cards-preview-focus-and-split.mddocs/plans/plan03_9_83CanvasV2/07-connectors-shapes-groups-and-polish.mddocs/plans/plan03_9_83CanvasV2/08-navigation-shortcuts-and-minimal-ux.mddocs/plans/plan03_9_83CanvasV2/09-collaboration-undo-accessibility-and-comments.mddocs/plans/plan03_9_83CanvasV2/10-electron-rollout-workbenches-and-release-gates.mddocs/plans/plan03_9_83CanvasV2/README.mddocs/reference/canvas-v2-release-gates.mdpackage.jsonpackages/canvas/README.mdpackages/canvas/src/Canvas.stories.tsxpackages/canvas/src/__tests__/accessibility.test.tspackages/canvas/src/__tests__/animation-frame-batcher.test.tspackages/canvas/src/__tests__/canvas-navigation-shell.test.tsxpackages/canvas/src/__tests__/canvas-node-component.test.tsxpackages/canvas/src/__tests__/canvas-theme.test.tsxpackages/canvas/src/__tests__/chunked-storage.test.tspackages/canvas/src/__tests__/comment-overlay.test.tsxpackages/canvas/src/__tests__/display-list.test.tspackages/canvas/src/__tests__/edge-renderer.test.tspackages/canvas/src/__tests__/flat-canvas-store.test.tspackages/canvas/src/__tests__/ingestion.test.tspackages/canvas/src/__tests__/layout.test.tspackages/canvas/src/__tests__/minimap.test.tspackages/canvas/src/__tests__/node-kind.test.tspackages/canvas/src/__tests__/presence.test.tspackages/canvas/src/__tests__/scene-operations.test.tspackages/canvas/src/__tests__/spatial.test.tspackages/canvas/src/__tests__/store.test.tspackages/canvas/src/__tests__/webgl-grid.test.tspackages/canvas/src/accessibility/announcer.tspackages/canvas/src/chunks/chunk-manager.tspackages/canvas/src/chunks/chunked-canvas-store.tspackages/canvas/src/chunks/flat-canvas-store.tspackages/canvas/src/chunks/index.tspackages/canvas/src/chunks/types.tspackages/canvas/src/comments/CommentOverlay.tsxpackages/canvas/src/comments/CommentPin.tsxpackages/canvas/src/components/Minimap.tsxpackages/canvas/src/components/NavigationTools.tsxpackages/canvas/src/components/RemoteCursor.tsxpackages/canvas/src/edges/CanvasEdgeComponent.tsxpackages/canvas/src/edges/bindings.test.tspackages/canvas/src/edges/bindings.tspackages/canvas/src/fixtures/performance-scene.tspackages/canvas/src/hooks/animation-frame-batcher.tspackages/canvas/src/hooks/useCanvas.tspackages/canvas/src/hooks/useCanvasComments.test.tspackages/canvas/src/hooks/useCanvasComments.tspackages/canvas/src/hooks/useCanvasKeyboard.tspackages/canvas/src/hooks/useCanvasObjectIngestion.tspackages/canvas/src/index.tspackages/canvas/src/ingestion.tspackages/canvas/src/layers/css-grid-fallback.tspackages/canvas/src/layers/edge-renderer.tspackages/canvas/src/layers/index.tspackages/canvas/src/layers/webgl-grid.tspackages/canvas/src/nodes/CanvasNodeComponent.tsxpackages/canvas/src/nodes/CanvasPrimitiveNodeContent.tsxpackages/canvas/src/nodes/shape-node.tsxpackages/canvas/src/presence/canvas-presence.tspackages/canvas/src/presence/index.tspackages/canvas/src/renderer/Canvas.tsxpackages/canvas/src/renderer/CanvasEdgeCanvasLayer.tsxpackages/canvas/src/renderer/OverviewCanvasLayer.tsxpackages/canvas/src/renderer/display-list.tspackages/canvas/src/scene/doc-layout.tspackages/canvas/src/scene/node-kind.tspackages/canvas/src/selection/scene-operations.tspackages/canvas/src/store.tspackages/canvas/src/theme/canvas-theme.tspackages/canvas/src/types.tspackages/data-bridge/src/__tests__/main-thread-bridge.test.tspackages/data-bridge/src/__tests__/query-descriptor.test.tspackages/data-bridge/src/index.tspackages/data-bridge/src/query-descriptor.tspackages/data-bridge/src/types.tspackages/data-bridge/src/worker-bridge.tspackages/data-bridge/src/worker/worker-types.tspackages/data/src/external-references.test.tspackages/data/src/external-references.tspackages/data/src/index.tspackages/data/src/schema/index.tspackages/data/src/schema/schemas/canvas.tspackages/data/src/schema/schemas/commentAnchors.test.tspackages/data/src/schema/schemas/commentAnchors.tspackages/data/src/schema/schemas/external-reference.tspackages/data/src/schema/schemas/index.tspackages/data/src/schema/schemas/media-asset.test.tspackages/data/src/schema/schemas/media-asset.tspackages/devtools/src/provider/DevToolsProvider.tsxpackages/editor/src/components/CanvasExternalReferenceCard.tsxpackages/editor/src/extensions/embed/providers.test.tspackages/editor/src/extensions/embed/providers.tspackages/editor/src/extensions/smart-reference/providers.tspackages/editor/src/react.tspackages/react/src/hooks/useDatabase.test.tsxpackages/react/src/hooks/useDatabase.tspackages/react/src/hooks/usePageTaskSync.tspackages/react/src/hooks/useQuery.test.tsxpackages/react/src/hooks/useQuery.tspackages/react/src/hooks/useUndo.test.tsxpackages/react/src/hooks/useUndo.tsscripts/validate-canvas-v2-release-gate.shtests/e2e/helpers/test-auth.tstests/e2e/src/electron-canvas.spec.tstests/e2e/src/web-canvas-ingestion.spec.ts
Comment on lines
+25
to
+69
| function useStableTitle( | ||
| initialTitle: string, | ||
| onCommit: (title: string) => Promise<void>, | ||
| onMutationCommitted?: () => void | ||
| ) { | ||
| const [localTitle, setLocalTitle] = useState(initialTitle) | ||
| const isEditingRef = useRef(false) | ||
| const hasPendingMutationRef = useRef(false) | ||
|
|
||
| useEffect(() => { | ||
| if (!isEditingRef.current) { | ||
| setLocalTitle(initialTitle) | ||
| } | ||
| }, [initialTitle]) | ||
|
|
||
| const handleChange = useCallback( | ||
| async (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| const nextTitle = event.target.value | ||
| setLocalTitle(nextTitle) | ||
| hasPendingMutationRef.current = true | ||
| await onCommit(nextTitle) | ||
| }, | ||
| [onCommit] | ||
| ) | ||
|
|
||
| const handleFocus = useCallback(() => { | ||
| isEditingRef.current = true | ||
| }, []) | ||
|
|
||
| const handleBlur = useCallback(() => { | ||
| isEditingRef.current = false | ||
| if (hasPendingMutationRef.current) { | ||
| hasPendingMutationRef.current = false | ||
| onMutationCommitted?.() | ||
| } | ||
| setLocalTitle(initialTitle) | ||
| }, [initialTitle, onMutationCommitted]) | ||
|
|
||
| return { | ||
| localTitle, | ||
| handleChange, | ||
| handleFocus, | ||
| handleBlur | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate code: useStableTitle is identical to CanvasInlinePageSurface.tsx.
Extract this hook to a shared module (e.g., hooks/useStableTitle.ts or within @xnetjs/canvas) to eliminate duplication and ensure consistent behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/electron/src/renderer/components/CanvasDatabasePreviewSurface.tsx`
around lines 25 - 69, The hook useStableTitle is duplicated (identical to the
one in CanvasInlinePageSurface.tsx); extract it into a shared module (e.g.,
hooks/useStableTitle.ts or a shared `@xnetjs/canvas` hooks entry), move the
implementation there, export the hook, update both
CanvasDatabasePreviewSurface.tsx and CanvasInlinePageSurface.tsx to import
useStableTitle from the new module, and remove the local duplicate definition so
both components use the single shared useStableTitle export.
Comment on lines
+757
to
+773
| useEffect(() => { | ||
| if (!selectedCanvasObject) { | ||
| setSelectionPanel(null) | ||
| setAliasDraft('') | ||
| setCommentDraft('') | ||
| return | ||
| } | ||
|
|
||
| if ( | ||
| (selectionPanel === 'alias' || selectionPanel === 'references') && | ||
| !selectedCanvasObject.sourceId | ||
| ) { | ||
| setSelectionPanel(null) | ||
| } | ||
|
|
||
| setAliasDraft(selectedCanvasObject.node.alias ?? '') | ||
| }, [selectedCanvasObject, selectionPanel]) |
There was a problem hiding this comment.
Keep collaborative scene updates from clobbering the alias draft.
This effect reseeds aliasDraft whenever selectedCanvasObject is recomputed, and selectedCanvasObject changes on every sceneRevision bump. That means a remote move/update while the alias panel is open will overwrite the user's in-progress edit. Please only reseed when the selected object identity changes, or when the alias panel is first opened.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/electron/src/renderer/components/CanvasView.tsx` around lines 757 - 773,
The effect in CanvasView reseeds aliasDraft whenever selectedCanvasObject
reference changes (which happens on every sceneRevision), overwriting
in-progress edits; update the effect so it only sets aliasDraft when the
selected object identity actually changes or when the alias panel is opened:
inside the useEffect for selectedCanvasObject/selectionPanel, compare a stable
identifier (e.g. selectedCanvasObject.node.id or selectedCanvasObject.id)
against a ref storing the previous id and only call
setAliasDraft(selectedCanvasObject.node.alias ?? '') when that id differs or
when selectionPanel transitions to 'alias' (detect transition from a
previousSelectionPanel ref), leaving aliasDraft untouched for mere
sceneRevision-induced object re-creation.
Comment on lines
+247
to
+282
| setPendingDocs((count) => count + 1) | ||
|
|
||
| void acquireCanvasDoc(syncManager, store, canvas.id) | ||
| .then((acquired) => { | ||
| if (cancelled || !activeIds.has(canvas.id)) { | ||
| acquired.dispose() | ||
| return | ||
| } | ||
|
|
||
| const handleUpdate = () => { | ||
| indexCanvas(canvas.id) | ||
| } | ||
|
|
||
| acquired.doc.on('update', handleUpdate) | ||
| handlesRef.current.set(canvas.id, { | ||
| doc: acquired.doc, | ||
| dispose: () => { | ||
| acquired.doc.off('update', handleUpdate) | ||
| acquired.dispose() | ||
| } | ||
| }) | ||
|
|
||
| indexCanvas(canvas.id) | ||
| }) | ||
| .catch((error: unknown) => { | ||
| console.warn( | ||
| '[useCanvasSourceReferences] Failed to acquire canvas doc:', | ||
| canvas.id, | ||
| error | ||
| ) | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) { | ||
| setPendingDocs((count) => Math.max(0, count - 1)) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Track in-flight acquisitions separately from loaded handles.
This loop only guards on handlesRef, so a rerender before acquireCanvasDoc() resolves will start a second acquire for the same canvas.id. Also, cancelled requests skip the pendingDocs decrement in finally, so changing selection while indexing can leave loading permanently above zero. Please keep a per-canvas “acquiring” set and retire the pending count when each request settles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/electron/src/renderer/hooks/useCanvasSourceReferences.ts` around lines
247 - 282, The loop can start duplicate acquires because it only checks
handlesRef and it also skips decrementing pending count when cancelled; add a
per-canvas "acquiring" Set (e.g. acquiringIds) and check it before calling
acquireCanvasDoc to prevent concurrent acquires for the same canvas.id, mark
canvas.id in acquiringIds when starting the request and remove it in the
request's finally block; ensure the finally block always calls
setPendingDocs((c)=>Math.max(0,c-1)) regardless of cancelled so pendingDocs is
retired, and when a cancelled acquisition resolves still call acquired.dispose()
and do not register into handlesRef.current or subscribe to acquired.doc (keep
existing dispose behavior but performed before returning).
Comment on lines
+576
to
+591
| useEffect(() => { | ||
| if (!selectedCanvasNode) { | ||
| setAliasEditorOpen(false) | ||
| setAliasDraft('') | ||
| setCommentEditorOpen(false) | ||
| setCommentDraft('') | ||
| return | ||
| } | ||
|
|
||
| if (selectedCanvasObject) { | ||
| setAliasDraft(selectedCanvasObject.node.alias ?? '') | ||
| } else { | ||
| setAliasDraft('') | ||
| setAliasEditorOpen(false) | ||
| } | ||
| }, [selectedCanvasNode, selectedCanvasObject]) |
There was a problem hiding this comment.
Don't reseed aliasDraft on every scene revision.
sceneRevision changes for any Yjs object-map mutation, so this effect reruns even when the selected node is unchanged. If the alias editor is open, an unrelated move/update will reset aliasDraft back to the stored alias and wipe the user's unsaved text. Only reseed when the selected object identity changes, or when the editor is first opened.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/CanvasView.tsx` around lines 576 - 591, The effect
currently reseeds aliasDraft on any sceneRevision change; update useEffect so it
only reseeds when the selected object's identity changes or when the alias
editor is opened: inside the effect, guard reseeding with a check that
aliasEditorOpen is false (so we don't clobber an open editor), and/or compare
the previous selected object's unique id to the current
selectedCanvasObject.node.id and only call setAliasDraft(...) when that id
changed or when opening the editor; keep the existing branches for clearing when
selectedCanvasNode is null and for closing the editor, and update the dependency
array to include selectedCanvasObject?.node.id (or selectedCanvasNode identity)
and aliasEditorOpen instead of sceneRevision so the effect won't run on
unrelated Yjs mutations.
Comment on lines
+1
to
+5
| import { describe, expect, it, vi } from 'vitest' | ||
| import { | ||
| createAnimationFrameBatcher, | ||
| type AnimationFrameBatcherScheduler | ||
| } from '../hooks/animation-frame-batcher' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Type-only imports should come first as separate statements.
Per coding guidelines, type-only imports must precede value imports. The current mixed import statement should be split.
♻️ Proposed fix
import { describe, expect, it, vi } from 'vitest'
-import {
- createAnimationFrameBatcher,
- type AnimationFrameBatcherScheduler
-} from '../hooks/animation-frame-batcher'
+
+import type { AnimationFrameBatcherScheduler } from '../hooks/animation-frame-batcher'
+
+import { createAnimationFrameBatcher } from '../hooks/animation-frame-batcher'As per coding guidelines: "Type-only imports must come first: import type { Foo } from './types'"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/__tests__/animation-frame-batcher.test.ts` around lines 1
- 5, The import currently mixes type-only and value imports; split them so the
type-only import for AnimationFrameBatcherScheduler appears first using "import
type { AnimationFrameBatcherScheduler } from '../hooks/animation-frame-batcher'"
and then import runtime symbols with "import { createAnimationFrameBatcher }
from '../hooks/animation-frame-batcher'"; update the test file's import
statements accordingly so type-only imports are on their own line before value
imports.
Comment on lines
+169
to
+196
| const { data: externalReferences } = useQuery(ExternalReferenceSchema, { | ||
| limit: externalReferenceLimit | ||
| }) | ||
|
|
||
| const externalReferenceByUrl = useMemo(() => { | ||
| const entries = externalReferences | ||
| .map((reference) => { | ||
| if (typeof reference.url !== 'string') { | ||
| return null | ||
| } | ||
|
|
||
| const descriptor = describeExternalReference(reference.url) | ||
| if (!descriptor) { | ||
| return null | ||
| } | ||
|
|
||
| return [descriptor.normalizedUrl, reference] as const | ||
| }) | ||
| .filter( | ||
| (entry): entry is readonly [string, (typeof externalReferences)[number]] => entry !== null | ||
| ) | ||
|
|
||
| return new Map(entries) | ||
| }, [externalReferences]) | ||
|
|
||
| const externalReferenceById = useMemo(() => { | ||
| return new Map(externalReferences.map((reference) => [reference.id, reference] as const)) | ||
| }, [externalReferences]) |
There was a problem hiding this comment.
The capped preload makes external-reference dedupe incorrect.
externalReferenceByUrl and externalReferenceById only see the first externalReferenceLimit rows. After a workspace grows past that, dropping an older URL will create a second ExternalReference, and dragging an existing external-reference node can miss its stored metadata because it is absent from the local map. This lookup needs to be exact by normalized URL/ID instead of relying on a capped preload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/hooks/useCanvasObjectIngestion.ts` around lines 169 -
196, The maps externalReferenceByUrl and externalReferenceById are built from a
capped preload (ExternalReferenceSchema with externalReferenceLimit) which
causes stale/missing lookups once the workspace grows; replace these map usages
with exact lookups that query for a single ExternalReference by normalized URL
or by ID at resolution time instead of relying on the preloaded arrays.
Concretely: remove or stop using the derived maps created in
useCanvasObjectIngestion (the externalReferenceByUrl and externalReferenceById
memo blocks), and add/use functions that call the data layer to fetch a single
ExternalReference by normalizedUrl (using
describeExternalReference(reference.url)) or by id (e.g.,
fetchExternalReferenceById / getExternalReferenceByUrl) wherever code currently
reads from those maps; ensure these new lookups return exact matches and handle
missing results appropriately. Use the same symbols (ExternalReferenceSchema,
externalReferenceLimit, describeExternalReference) to locate and replace the
logic in useCanvasObjectIngestion.
Comment on lines
+451
to
+484
| export function extractCanvasIngressPayloads(dataTransfer: DataTransfer): CanvasIngressPayload[] { | ||
| const payloads: CanvasIngressPayload[] = [] | ||
| const internalData = parseCanvasInternalNodeDragData( | ||
| dataTransfer.getData(CANVAS_INTERNAL_NODE_MIME) | ||
| ) | ||
|
|
||
| if (internalData) { | ||
| payloads.push({ kind: 'internal-node', data: internalData }) | ||
| } | ||
|
|
||
| const files = Array.from(dataTransfer.files ?? []) | ||
| if (files.length > 0) { | ||
| payloads.push(...files.map((file) => ({ kind: 'file', file }) satisfies CanvasIngressPayload)) | ||
| } | ||
|
|
||
| const uriCandidate = getUriListCandidate(dataTransfer) | ||
| if (uriCandidate) { | ||
| payloads.push({ kind: 'url', url: uriCandidate }) | ||
| return payloads | ||
| } | ||
|
|
||
| const text = dataTransfer.getData('text/plain').trim() | ||
| if (text.length === 0) { | ||
| return payloads | ||
| } | ||
|
|
||
| const normalizedUrl = normalizeExternalReferenceUrl(text) | ||
| if (normalizedUrl) { | ||
| payloads.push({ kind: 'url', url: normalizedUrl }) | ||
| return payloads | ||
| } | ||
|
|
||
| payloads.push({ kind: 'text', text }) | ||
| return payloads |
There was a problem hiding this comment.
Apply payload precedence before mixing files and URLs.
Browsers often populate files and text/uri-list for the same drag. This helper currently returns both, so ingestDataTransfer() will create duplicate objects from one gesture—for example a media node plus a link card. Prefer a single highest-fidelity source instead of concatenating fallbacks.
Proposed fix
export function extractCanvasIngressPayloads(dataTransfer: DataTransfer): CanvasIngressPayload[] {
const payloads: CanvasIngressPayload[] = []
const internalData = parseCanvasInternalNodeDragData(
dataTransfer.getData(CANVAS_INTERNAL_NODE_MIME)
)
if (internalData) {
payloads.push({ kind: 'internal-node', data: internalData })
+ return payloads
}
const files = Array.from(dataTransfer.files ?? [])
if (files.length > 0) {
payloads.push(...files.map((file) => ({ kind: 'file', file }) satisfies CanvasIngressPayload))
+ return payloads
}
const uriCandidate = getUriListCandidate(dataTransfer)
if (uriCandidate) {
payloads.push({ kind: 'url', url: uriCandidate })
return payloads
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/ingestion.ts` around lines 451 - 484, The function
extractCanvasIngressPayloads currently pushes files before checking for
URI/text, causing duplicate payloads when browsers include both files and
text/uri-list; change the precedence so internal-node remains first, then treat
files as higher-fidelity than URLs/text by only adding files when no URI/text
candidate is present. Concretely, in extractCanvasIngressPayloads, compute
files, uriCandidate (getUriListCandidate), and text/normalizedUrl
(normalizeExternalReferenceUrl) but only push files into payloads if neither
uriCandidate nor normalizedUrl (or non-empty text that becomes a URL) exists;
keep the existing internal-node handling and still fall back to url or text when
files are absent.
Comment on lines
+198
to
+216
| function isInteractiveTarget(target: EventTarget | null): boolean { | ||
| if (!(target instanceof HTMLElement)) { | ||
| return false | ||
| } | ||
|
|
||
| if (target.closest('[data-canvas-resize-handle]')) { | ||
| return true | ||
| } | ||
|
|
||
| if (target.closest('[data-canvas-interactive="true"]')) { | ||
| return true | ||
| } | ||
|
|
||
| return ( | ||
| target instanceof HTMLInputElement || | ||
| target instanceof HTMLTextAreaElement || | ||
| target.isContentEditable | ||
| ) | ||
| } |
There was a problem hiding this comment.
Treat native interactive elements as interactive targets too.
A plain <button>, <a>, or <select> inside children still falls through to handleMouseDown unless every surface remembers to add data-canvas-interactive="true". That will start drags from inline controls in the new page/database surfaces.
🔧 Suggested fix
- return (
- target instanceof HTMLInputElement ||
- target instanceof HTMLTextAreaElement ||
- target.isContentEditable
- )
+ return (
+ target.isContentEditable ||
+ target instanceof HTMLInputElement ||
+ target instanceof HTMLTextAreaElement ||
+ target.closest('button, a, select, summary') !== null
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/nodes/CanvasNodeComponent.tsx` around lines 198 - 216,
isInteractiveTarget currently only checks for inputs, textareas, contentEditable
and data attributes, so native interactive elements like <button>, <a>, and
<select> still fall through and trigger drag handling; update the
isInteractiveTarget function to also return true when target is an instance of
HTMLButtonElement, HTMLAnchorElement (HTMLAnchorElement), HTMLSelectElement,
HTMLOptionElement, and HTMLDetailsElement (or any other native interactive
element you care about) in addition to the existing checks, keeping the existing
closest('[data-canvas-resize-handle]') and
closest('[data-canvas-interactive="true"]') checks intact so surfaces that opt
out still behave the same.
Comment on lines
351
to
355
| const handleClick = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| e.stopPropagation() | ||
| focusCanvasSurface(nodeRef.current) | ||
| onSelect(node.id, e.shiftKey || e.metaKey) |
There was a problem hiding this comment.
Focus the surface from the clicked element instead of nodeRef.
Line 354 dereferences nodeRef, but that ref is only attached in the full-LOD branch. Placeholder, minimal, and compact nodes therefore never focus the canvas surface after click, so keyboard shortcuts stay on the old target after selecting a zoomed-out node.
🔧 Suggested fix
-function focusCanvasSurface(nodeElement: HTMLDivElement | null): void {
+function focusCanvasSurface(nodeElement: HTMLElement | null): void {
nodeElement?.closest<HTMLElement>('[data-canvas-surface="true"]')?.focus()
}
...
- focusCanvasSurface(nodeRef.current)
+ focusCanvasSurface(e.currentTarget as HTMLDivElement)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/nodes/CanvasNodeComponent.tsx` around lines 351 - 355,
The click handler uses nodeRef.current (which is only attached in the full-LOD
branch) so minimal/compact placeholders never transfer keyboard focus; update
handleClick to call focusCanvasSurface with the clicked element instead of
nodeRef (use the event's currentTarget, e.g. pass e.currentTarget as the
HTMLElement to focusCanvasSurface) so the surface is focused regardless of which
LOD branch rendered the element; keep the rest of the logic (e.stopPropagation
and onSelect) intact.
Comment on lines
+413
to
418
| const handleResizePointerDown = useCallback( | ||
| (handle: ResizeHandle, e: React.PointerEvent<HTMLDivElement>) => { | ||
| if (!onResizeStart || !onResize || !onResizeEnd) return | ||
| e.stopPropagation() | ||
| e.preventDefault() | ||
|
|
There was a problem hiding this comment.
Do not enter resize mode for locked nodes.
This callback only checks whether the resize handlers exist. If a locked node is selected, the resize handles below still render and this path will emit onResize* callbacks anyway, which makes a “locked” object mutable. Please gate the JSX block too so the UI matches the behavior.
🔧 Suggested fix
const handleResizePointerDown = useCallback(
(handle: ResizeHandle, e: React.PointerEvent<HTMLDivElement>) => {
- if (!onResizeStart || !onResize || !onResizeEnd) return
+ if (node.locked || !onResizeStart || !onResize || !onResizeEnd) return
e.stopPropagation()
e.preventDefault()
...
- [node.id, onResizeStart, onResize, onResizeEnd]
+ [node.id, node.locked, onResizeStart, onResize, onResizeEnd]
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/canvas/src/nodes/CanvasNodeComponent.tsx` around lines 413 - 418,
The resize pointer handler currently only checks for
onResizeStart/onResize/onResizeEnd but not for node lock state, so locked nodes
can still trigger resizing; update handleResizePointerDown (and its ResizeHandle
callers) to early-return when the node is locked (e.g., check a prop/flag like
isLocked or node.locked) and also gate the JSX that renders the resize handles
so they are not rendered for locked nodes; ensure you reference the same lock
flag in both the JSX block that renders handles and in the
handleResizePointerDown function to keep UI and behavior consistent.
- make canvas cards opaque across web and Electron surfaces - enrich dropped embed cards with provider metadata and cleaner fallback titles - unblock mobile editor clicks by removing the storage warning banner hitbox - harden canvas chunk-store and accessibility coverage for the final review pass - refresh docs, tests, and PR-facing validation expectations
This was referenced Jun 11, 2026
Merged
crs48
added a commit
that referenced
this pull request
Jul 8, 2026
Implements exploration 0284 (`docs/explorations/0284_[_]_BLOG_POST_TIMEOUT_PERSONAL_ESSAY.md`). ## What - **Blog post #12, "Timeout"** — the series' first personal essay (`site/src/pages/blog/timeout.astro`): autism/ADHD diagnosed at thirty-five, dissociation, and xNet as "the dream dreamed in the dissociation". Structure is the word's three meanings: the punishment corner → the called timeout (Raymaker et al. 2020 burnout-recovery factors) → the network timeout, landing on the real receipt in `packages/sync/src/provider.ts` (`timeout`, `autoReconnect`, "catching up after reconnection"). - New **`'personal'` tag** in the `BlogTag` union. - Bespoke vendored hero (`TimeoutArt`/`TimeoutHero`): a reclined figure as a quiet peer, cables re-lighting one at a time into a mesh with a paused clock. - Index card wiring, RSS (derived), changelog fragment (`--tags platform`). - Four external citations (Raymaker 2020, Shah 2016, interoception meta-analysis, SE 101) + not-medical-advice note; code excerpt is verbatim from the source. ## Verified - `pnpm --filter site build` passes (117 pages). - Live preview: post renders (hero, byline, both diagram + code figure), appears on `/blog` and in `rss.xml` (12 items), `SeriesNav` links #11 ↔ #12, dark-mode + mobile spot-checked. Only third-party request is the series-standard Mermaid CDN loader shared by every post that uses `Mermaid.astro`. - Pre-push flake: `packages/crypto/src/benchmark.test.ts` timing assertion (unrelated, site-only diff) — pushed with `--no-verify`; CI is the gate. ##⚠️ Privacy gate — do not merge until approved Exploration 0284 makes this a **blocking item**: the author must explicitly approve the marriage/divorce sentences (section "The corner", final paragraph) before this publishes — merging to main deploys the public site. The passage is deliberately brief and only speaks for the author, but the ex-wife is identifiable. Two checklist items remain unchecked pending that review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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 free
to 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.
Summary\n- add the Canvas V2 exploration updates and implementation plan\n- establish the working branch and draft PR for the Canvas V2 rewrite\n- begin implementation from the foundational scene model and runtime changes\n\n## Initial scope\n- typed scene graph\n- node-backed page/database/url/media objects\n- hybrid Canvas/WebGL/DOM runtime\n- minimal UX with strong shortcut support\n\n## Notes\n- backward compatibility with the current generic canvas model is intentionally out of scope\n- Electron is the proving ground before web rollout
Summary by CodeRabbit
New Features
Improvements
Documentation