Grida Canvas - Image Persistency#495
Conversation
…ading, persisting, and rewriting image references in documents
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
WalkthroughThis PR introduces comprehensive image asset handling across the editor stack: OPFS-based persistence for images in the playground, new editor APIs for archiving documents with embedded image data, hash-based image reference serialization in the format layer, and expanded OPFS interface methods for image operations. Additionally, color picker UI utilities are added to the editor surface, and serialization tests are updated to reflect container-based image paint representation. Changes
Sequence Diagram(s)sequenceDiagram
participant Playground as Playground
participant OPFS as OPFS Storage
participant Editor as Editor
participant WASM as WASM Surface
participant Archive as Archive/ZIP
Note over Playground,Archive: Image Load Flow
Playground->>OPFS: readImage(hash)
OPFS-->>Playground: Uint8Array
Playground->>Editor: loadImagesToWasmSurface(images)
Editor->>WASM: Register image assets
WASM-->>Editor: Pending assets buffered if not mounted
Editor->>WASM: Apply pending assets on mount
Note over Playground,Archive: Image Save Flow
Playground->>Editor: archivedir()
Editor-->>Playground: {document with images, image bytes}
Playground->>Archive: Create ZIP with document + images/
Playground->>OPFS: writeImage(hash, bytes)
OPFS-->>Playground: Write complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8a42eac5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const usedSrcs = new dq.DocumentStateQuery(snapshot).image_srcs(); | ||
| if (this.backend !== "canvas" || !this._m_wasm_canvas_scene) { | ||
| throw new Error( | ||
| "`archivedir()` requires the canvas/WASM backend to be mounted." | ||
| ); |
There was a problem hiding this comment.
archivedir() now throws unless the canvas/WASM backend is mounted, but the playground defaults to backend="dom" and both Cmd/Ctrl+S and export call archivedir() (see playground save/export flow). This means saving or exporting from the default DOM backend will always hit the exception and show "Failed to save" even though the comment says DOM should persist without embedding. Consider allowing DOM to bypass WASM asset collection (returning the snapshot as‑is) so non‑canvas backends can still save/export.
Useful? React with 👍 / 👎.
| if (!/^[0-9a-f]{16}$/.test(src)) { | ||
| throw new Error( | ||
| `ImagePaint.src must be hex16 (or res://images/<hex16>, mem://<hex16>) for persistence. Got: "${paint.src}"` | ||
| ); |
There was a problem hiding this comment.
The encoder now throws unless ImagePaint.src is a hex hash or res:///mem:// form. In the DOM backend, image paints are created from external URLs (e.g., drag‑drop uses imageRef.url), and existing documents may persist URL sources. Those documents will now fail io.GRID.encode/archive save with a thrown error, breaking persistence for DOM workflows. Consider accepting URL/RID values for non‑WASM backends or preserving them when encoding.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/grida-canvas-io/format.ts (1)
2269-2326: Decode ImagePaint losslessly (quarter_turns + transform/tile scale), otherwise persistence is incomplete.
encode.paintTypes.image()persistsquarterTurnsand fit union payloads, butdecode.paintTypes.image()drops them. Any docs using quarter rotations / transform-fit / tile-fit won’t roundtrip correctly after reload.Proposed fix (decode quarter_turns, transform, tile scale)
export function image(paintValue: unknown): cg.ImagePaint { // NOTE: hash-only decode for image resource reference. const imagePaint = paintValue as fbs.ImagePaint; // Decode src from ResourceRef let src = ""; const imageType = imagePaint.imageType(); if (imageType === fbs.ResourceRef.ResourceRefHASH) { const resourceRef = imagePaint.image( new fbs.ResourceRefHASH() ) as fbs.ResourceRefHASH | null; if (resourceRef) { // NOTE: WASM renderer currently uses `res://images/<hex16>` as the cross-boundary image id. // Persisted form remains plain hex16; decode returns the runtime identifier. // // TODO(resources): once `res://<dir>/<id>` is fully generalized, avoid hardcoding `images/`. const hash = resourceRef.hash() ?? ""; src = hash ? `res://images/${hash}` : ""; } } // Decode fit from ImagePaintFit union let fit: cg.BoxFit | "transform" | "tile" = "cover"; + let transform: cg.AffineTransform | undefined = undefined; + let scale: number | undefined = undefined; const fitType = imagePaint.fitType(); if (fitType === fbs.ImagePaintFit.ImagePaintFitFit) { const fitFit = imagePaint.fit( new fbs.ImagePaintFitFit() ) as fbs.ImagePaintFitFit | null; if (fitFit) { const boxFit = fitFit.boxFit(); fit = enums.BOX_FIT_DECODE.get(boxFit) ?? "cover"; } } else if (fitType === fbs.ImagePaintFit.ImagePaintFitTransform) { fit = "transform"; - // TODO: decode transform if needed + const fitTransform = imagePaint.fit( + new fbs.ImagePaintFitTransform() + ) as fbs.ImagePaintFitTransform | null; + transform = decodeGradientTransform(fitTransform?.transform() ?? null); } else if (fitType === fbs.ImagePaintFit.ImagePaintFitTile) { fit = "tile"; - // TODO: decode tile scale if needed + const fitTile = imagePaint.fit( + new fbs.ImagePaintFitTile() + ) as fbs.ImagePaintFitTile | null; + scale = fitTile?.tile()?.scale() ?? 1.0; } + const qt = imagePaint.quarterTurns() ?? 0; return { type: "image", src, fit, + ...(fit === "transform" ? { transform } : {}), + ...(fit === "tile" ? { scale } : {}), + ...(qt ? { quarter_turns: qt % 4 } : {}), blend_mode: styling.decode.blendMode(imagePaint.blendMode()), opacity: imagePaint.opacity(), active: imagePaint.active(), filters: { exposure: imagePaint.filters()?.exposure() ?? 0.0, contrast: imagePaint.filters()?.contrast() ?? 0.0, saturation: imagePaint.filters()?.saturation() ?? 0.0, temperature: imagePaint.filters()?.temperature() ?? 0.0, tint: imagePaint.filters()?.tint() ?? 0.0, highlights: imagePaint.filters()?.highlights() ?? 0.0, shadows: imagePaint.filters()?.shadows() ?? 0.0, }, } satisfies cg.ImagePaint; }
🤖 Fix all issues with AI agents
In `@editor/grida-canvas/editor.ts`:
- Around line 3380-3391: The reducer unconditionally mutates
state.document.images which can throw if images is undefined and can mark the
document dirty even when metadata already exists; inside the this.doc.reduce
callback (used by loadImagesToWasmSurface) ensure state.document.images is
initialized if missing (create an empty object) and only assign
state.document.images[hash] when that key is absent (i.e., skip writing if
metadata already exists) so you avoid errors and unnecessary mutations that flip
the dirty flag.
In `@editor/grida-canvas/query/index.ts`:
- Around line 616-668: image_srcs() currently collects srcs only from image
paints via image_paints(), missing node-level images/videos; update image_srcs()
to also iterate this.nodeids and collect src from nodes that are image or video
types (e.g., check node.type === "image" || node.type === "video" or use
cg.isImageNode / cg.isVideoNode if available) and include their node.src values
(filtering falsy) into the Set so archivedir()/persistence covers both
paint-based and node-based media sources.
- Around line 1-4: query/index.ts creates a circular runtime import with
editor.i.ts because it imports editor at module top-level while editor.i.ts
imports dq from this module; fix by removing the top-level import of editor and
performing a local (lazy) require/import for the editor symbol only inside the
functions that call editor.resolvePaints (the call sites around the current
lines labeled 551, 638, 671), e.g., inside the functions that need resolvePaints
do const { editor } = await import("..") or require("..") and then call
editor.resolvePaints; alternatively, move any query code that depends on
editor.resolvePaints into editor.i.ts or a new module that does not import dq to
eliminate the bidirectional dependency.
🧹 Nitpick comments (10)
editor/grida-canvas/editor.ts (2)
2835-2869:__rewriteImagePaintSrcDeepshould skipArrayBuffer/TypedArrays to avoid huge traversalsRight now
__isRecord()will treatUint8Arrayetc. as a record and recurse over indices if any binary data ever ends up on the document object (e.g. future thumbnail bytes).Proposed fix
@@ private static __rewriteImagePaintSrcDeep( value: unknown, mapSrc: (src: string) => string | undefined ): number { if (Array.isArray(value)) { @@ } + // Avoid walking binary blobs (ArrayBuffer, Uint8Array, etc.) + if (value instanceof ArrayBuffer) return 0; + if (typeof ArrayBuffer !== "undefined" && ArrayBuffer.isView(value)) return 0; if (!Editor.__isRecord(value)) return 0;
3013-3024: Pending image assets: avoid overwriting on multiple calls before mount
loadImagesToWasmSurface()currently overwrites_pending_image_assets(Line 3319). If multiple loads happen before mount (e.g. document load + additional asset fetch), earlier assets are dropped. Merging is safer, and it makes the Line 3016 flush more robust.Proposed fix
@@ public loadImagesToWasmSurface(images: Record<string, Uint8Array>) { if (!images || Object.keys(images).length === 0) return; // If we're on canvas backend but not mounted yet, defer. if (this.backend === "canvas" && !this._m_wasm_canvas_scene) { - this._pending_image_assets = images; + this._pending_image_assets = { + ...(this._pending_image_assets ?? {}), + ...images, + }; return; }Also applies to: 3265-3336
editor/grida-canvas-hosted/playground/playground.tsx (2)
466-525: OPFS load: consider loading only images referenced by the loaded document
listImages()+ reading every file works, but OPFS will likely accumulate stale images over time. Since you already haveloadedDocumentbefore resetting, you can compute the required hashes (via the new query helpers) and only read those.
647-671: Save path:archivedir()dependency on mounted WASM should be gated (orarchivedir()must support DOM)Line 654 calls
instance.archivedir()unconditionally (insideif (opfs)), which will now fail for DOM backend unlessarchivedir()has a DOM fallback. Also consider pruning images that are no longer referenced to avoid unbounded OPFS growth.Possible guard (if you want save-to-OPFS to work on DOM backend without embedding)
@@ async () => { if (opfs) { try { - const snapshot = instance.getSnapshot(); - const dir = instance.archivedir(); + const snapshot = instance.getSnapshot(); + const dir = + backend === "canvas" + ? instance.archivedir() + : { document: snapshot.document, images: {} as Record<string, Uint8Array> }; @@ // Write images into OPFS (images/<hash>.<ext>) for (const [filename, bytes] of Object.entries(dir.images)) { await opfs.writeImage(filename, bytes); }packages/grida-canvas-io/__tests__/format-roundtrip.test.ts (1)
848-912: Test updated to reflect container-based image paint representation — LGTM.The changes correctly align with the new hash-based image persistence model and expand the test matrix to include the
"none"fit option.Consider adding an assertion to verify the
srctransformation after roundtrip. According to the format changes, encoding normalizessrcto hex16 and decoding returnsres://images/<hash>, so the roundtrippedpaint.srcshould be"res://images/0123456789abcdef".💡 Optional: Add src verification
expect(paint.fit).toBe(expected); + expect(paint.src).toBe("res://images/0123456789abcdef"); });packages/grida-canvas-io/index.ts (4)
775-799: Normalize + validate ZIP image keys to canonical hex16 (lowercase), and guard against collisions.Right now
hashHexparsing is “best effort” (split(".")[0]) and doesn’t enforce the same canonical format asformat.tsdoes; tightening this makes cross-layer lookups more reliable.Proposed hardening for hash parsing
for (const [key, imageData] of Object.entries(_x_images)) { - const base = key.split("/").pop() ?? key; - const hashHex = base.includes(".") ? base.split(".")[0]! : base; + const base = (key.split("/").pop() ?? key).trim(); + const dot = base.lastIndexOf("."); + const hashHex = (dot > 0 ? base.slice(0, dot) : base).toLowerCase(); + if (!/^[0-9a-f]{16}$/.test(hashHex)) { + throw new Error(`Invalid image filename (expected <hex16>[.<ext>]): ${key}`); + } + if (hashHex in assets) { + throw new Error(`Duplicate image hash in archive: ${hashHex}`); + } const dimensions = imageSize(new Uint8Array(imageData)); if (!dimensions || !dimensions.width || !dimensions.height) { throw new Error(`Failed to get dimensions for image: ${key}`); } const { width, height, type } = dimensions; const mimeType = IMAGE_TYPE_TO_MIME_TYPE[type || "png"] || "image/png"; assets[hashHex] = imageData; imagesRepo[hashHex] = { url: hashHex, width, height, bytes: imageData.byteLength, type: mimeType, }; }
894-918: Manifestimagesindex looks fine, but clarify “best-effort” vs “guaranteed”.The type includes
mime/width/height, but pack currently doesn’t populate them; callers may assume they exist. Either fill these (or explicitly document they’re optional/best-effort, as you started).
971-1004: Hardenmanifest.imageskeying and ZIP entry naming (avoidsplit(".")[0]+ pathy keys).
name.split(".")[0]will mis-handle filenames with multiple dots, andfiles[\images/${key}`]trustskeyverbatim (surprising if it contains//..). Even if today keys are always.`, it’s worth enforcing that contract at the boundary.Proposed normalization + validation
if (images && Object.keys(images).length > 0) { const index: NonNullable<Manifest["images"]> = {}; for (const [key, imageData] of Object.entries(images)) { - const name = key.split("/").pop() ?? key; - const ext = name.includes(".") ? name.split(".").pop() : undefined; - index[name.split(".")[0] ?? name] = { - ext, - bytes: imageData.byteLength, - }; + if (key.includes("/") || key.includes("\\") || key.includes("..")) { + throw new Error(`Invalid image key: ${key}`); + } + const name = key.trim(); + const dot = name.lastIndexOf("."); + const id = (dot > 0 ? name.slice(0, dot) : name).toLowerCase(); + const ext = dot > 0 ? name.slice(dot + 1).toLowerCase() : undefined; + if (!/^[0-9a-f]{16}$/.test(id)) { + throw new Error(`Invalid image id (expected hex16): ${id}`); + } + index[id] = { ext, bytes: imageData.byteLength }; } manifest.images = index; } // Add images if (images && Object.keys(images).length > 0) { // ... for (const [key, imageData] of Object.entries(images)) { - files[`images/${key}`] = imageData; + if (key.includes("/") || key.includes("\\") || key.includes("..")) { + throw new Error(`Invalid image key: ${key}`); + } + files[`images/${key.trim()}`] = imageData; } }
1187-1191: Add filename validation and consistent error handling to OPFS image helpers.The
writeImage()andreadImage()methods accept arbitrary filenames and pass them directly togetFileHandle()without validation or error wrapping. Per the File System Access API spec,getFileHandle(name)only accepts single path segments and forbids "/" and NUL characters—invalid names trigger a TypeError that goes unhandled. Additionally, other methods in this class wrap OPFS errors consistently (distinguishingNotFoundErrorfrom other IO failures), but these image helpers do not, making it impossible for callers to handle "not found" vs "IO error" cleanly.Add a simple filename validator (single path segment, non-empty) and wrap errors to match the existing pattern in the class (see the
FileKeygetter handlers for the error structure).Proposed validator and error wrapping
export class Handle { private _dirHandle: FileSystemDirectoryHandle | null = null; private _dirHandlePromise: Promise<FileSystemDirectoryHandle> | null = null; private _fileHandles = new Map<FileKey, FileHandle>(); + private normalizeImageFilename(filename: string): string { + const name = filename.trim(); + if ( + !name || + name === "." || + name === ".." || + name.includes("/") || + name.includes("\\") || + name.includes("\0") + ) { + throw new Error(`Invalid image filename: ${filename}`); + } + return name; + } // ... async writeImage(filename: string, bytes: Uint8Array): Promise<void> { + const name = this.normalizeImageFilename(filename); // ... const dir = await this.getDirectoryHandle(); const imagesDir = await dir.getDirectoryHandle("images", { create: true, }); - const fileHandle = await imagesDir.getFileHandle(filename, { - create: true, - }); - const writable = await fileHandle.createWritable(); - await writable.write(bytes as unknown as FileSystemWriteChunkType); - await writable.close(); + try { + const fileHandle = await imagesDir.getFileHandle(name, { create: true }); + const writable = await fileHandle.createWritable(); + await writable.write(bytes as unknown as FileSystemWriteChunkType); + await writable.close(); + } catch (error) { + throw new Error( + `Failed to write OPFS images/${name}: ${error instanceof Error ? error.message : String(error)}` + ); + } } async readImage(filename: string): Promise<Uint8Array> { + const name = this.normalizeImageFilename(filename); const dir = await this.getDirectoryHandle(); - const imagesDir = await dir.getDirectoryHandle("images"); - const fileHandle = await imagesDir.getFileHandle(filename); - const file = await fileHandle.getFile(); - return new Uint8Array(await file.arrayBuffer()); + try { + const imagesDir = await dir.getDirectoryHandle("images"); + const fileHandle = await imagesDir.getFileHandle(name); + const file = await fileHandle.getFile(); + return new Uint8Array(await file.arrayBuffer()); + } catch (error) { + if (error instanceof DOMException && error.name === "NotFoundError") { + throw new Error(`images/${name} not found in OPFS`); + } + throw new Error( + `Failed to read OPFS images/${name}: ${error instanceof Error ? error.message : String(error)}` + ); + } } }Also applies to: 1384-1436
packages/grida-canvas-io/format.ts (1)
1899-1932: Extract a sharedImagePaint.srcnormalization helper to prevent duplication and divergence.The hash-only validation exists in two places—
editor.ts(__try_parse_hex16_from_image_src) andformat.ts(encode logic, lines 1914–1925)—with identical implementations. Both stripres://images/andmem://prefixes and validate hex16. Since both files carry identical TODO comments about restricting tores://only, extracting a shared helper is essential to prevent the rules from drifting when one is updated.Note: The archive code correctly stores plain hex16 keys without extensions (
files[images/${key}], notimages/${key}.ext), so the current extension concern is a future TODO, not a present drift risk. However, centralizing the normalization will protect against accidental divergence as the resource system evolves.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
editor/grida-canvas-hosted/playground/playground.tsxeditor/grida-canvas/editor.tseditor/grida-canvas/query/index.tspackages/grida-canvas-io/__tests__/format-roundtrip.test.tspackages/grida-canvas-io/format.tspackages/grida-canvas-io/index.ts
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.220Z
Learning: Applies to crates/grida-canvas/**/*.rs : Use `skia-safe` crate for painting operations in the rendering engine
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.220Z
Learning: Applies to crates/grida-canvas/**/*.rs : Maintain bidirectional mapping between NodeId and UserNodeId at the application layer for API boundary management
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use Canvas 2D API with path commands for rendering geometric shapes (circle, square, triangle, etc.)
Applied to files:
packages/grida-canvas-io/__tests__/format-roundtrip.test.tseditor/grida-canvas/query/index.tspackages/grida-canvas-io/format.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : For SVG export, convert circles to <circle> elements, rectangles to <rect> elements, and polygons to <polygon> elements with calculated points
Applied to files:
packages/grida-canvas-io/__tests__/format-roundtrip.test.tseditor/grida-canvas/query/index.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Cache ImageData and dimensions in refs (imageDataRef, sizeRef) for efficient exports
Applied to files:
packages/grida-canvas-io/__tests__/format-roundtrip.test.tseditor/grida-canvas-hosted/playground/playground.tsxeditor/grida-canvas/query/index.tspackages/grida-canvas-io/index.tspackages/grida-canvas-io/format.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/main.rs : Update `grida-canvas-wasm.d.ts` TypeScript definitions file when new APIs are introduced via `main.rs`
Applied to files:
packages/grida-canvas-io/__tests__/format-roundtrip.test.tseditor/grida-canvas-hosted/playground/playground.tsxeditor/grida-canvas/query/index.tspackages/grida-canvas-io/index.tspackages/grida-canvas-io/format.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Custom images used as halftone shapes should be loaded as HTMLImageElement for efficient canvas rendering and preserve original image colors in as-is mode
Applied to files:
packages/grida-canvas-io/__tests__/format-roundtrip.test.tseditor/grida-canvas-hosted/playground/playground.tsxeditor/grida-canvas/query/index.tspackages/grida-canvas-io/format.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/*.{js,ts,jsx,tsx} : Use Web Workers for heavy graphics operations to improve performance and responsiveness
Applied to files:
editor/grida-canvas-hosted/playground/playground.tsx
📚 Learning: 2025-12-01T00:22:19.083Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas-wasm/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:19.083Z
Learning: Applies to crates/grida-canvas-wasm/**/+(grida-canvas-wasm.js|grida-canvas-wasm.wasm) : Include WASM artifacts (`grida-canvas-wasm.js` and `grida-canvas-wasm.wasm`) in git for faster CI builds
Applied to files:
editor/grida-canvas-hosted/playground/playground.tsxpackages/grida-canvas-io/index.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : When adding new shape types, update the Shape type union, add cases in drawShape() function, add cases in shapeToSVG() function, and add SelectItem in UI
Applied to files:
editor/grida-canvas/query/index.tseditor/grida-canvas/editor.ts
📚 Learning: 2025-12-01T00:22:56.899Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/app/(tools)/tools/halftone/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:56.899Z
Learning: Applies to editor/app/(tools)/tools/halftone/app/(tools)/tools/halftone/_page.tsx : Use React hooks for state management (imageSrc, shape, grid, maxRadius, gamma, jitter, opacity, color, customShapeImage, imageDataRef, sizeRef)
Applied to files:
editor/grida-canvas/query/index.ts
📚 Learning: 2026-01-10T04:48:31.415Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:48:31.415Z
Learning: Use /editor/grida-* directories to isolate domain-specific modules that can be promoted to <root>/packages when reasonably well-defined
Applied to files:
editor/grida-canvas/query/index.ts
📚 Learning: 2025-12-20T08:11:16.220Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.220Z
Learning: Applies to crates/grida-canvas/**/*.rs : Handle NodeId to UserNodeId conversion via `IdConverter` during .grida file loading
Applied to files:
editor/grida-canvas/query/index.ts
📚 Learning: 2025-12-20T08:11:16.220Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: crates/grida-canvas/AGENTS.md:0-0
Timestamp: 2025-12-20T08:11:16.220Z
Learning: Applies to crates/grida-canvas/**/*.grida : Validate .grida files using the `tool_io_grida` CLI tool to check file structure, parse all nodes, and detect parsing errors
Applied to files:
editor/grida-canvas/query/index.ts
📚 Learning: 2026-01-10T04:48:31.415Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T04:48:31.415Z
Learning: Applies to editor/app/(library)/** : Use /editor/app/(library) for Grida Library (open assets) specific pages
Applied to files:
editor/grida-canvas/query/index.ts
📚 Learning: 2025-12-01T00:22:41.045Z
Learnt from: CR
Repo: gridaco/grida PR: 0
File: editor/AGENTS.md:0-0
Timestamp: 2025-12-01T00:22:41.045Z
Learning: Applies to editor/**/next-env.d.ts : Include `next-env.d.ts` file in git for the project to avoid requiring dev/build runs for typechecking in CI environments
Applied to files:
editor/grida-canvas/query/index.ts
🧬 Code graph analysis (5)
packages/grida-canvas-io/__tests__/format-roundtrip.test.ts (2)
packages/grida-canvas-io/format.ts (3)
node(1058-1676)paint(2040-2066)paint(2345-2360)packages/grida-canvas-io-svg/lib.ts (1)
paint(35-123)
editor/grida-canvas-hosted/playground/playground.tsx (1)
packages/grida-canvas-io-figma/lib.ts (1)
document(673-974)
editor/grida-canvas/query/index.ts (1)
packages/grida-canvas-cg/lib.ts (1)
ImagePaint(835-876)
packages/grida-canvas-io/index.ts (1)
packages/grida-canvas-schema/grida.ts (1)
ImageRef(709-715)
packages/grida-canvas-io/format.ts (3)
packages/grida-canvas-io-svg/lib.ts (1)
paint(35-123)crates/csscascade/src/tree/mod.rs (1)
builder(185-187)packages/grida-canvas-cg/lib.ts (1)
ImagePaint(835-876)
🔇 Additional comments (6)
editor/grida-canvas/editor.ts (2)
713-713: No review comment needed (small refactor)
1821-1875: Nice: fill/stroke mutations now read current paints viaeditor.resolvePaints()This should be more robust across legacy/new paint representations than reading raw node properties.
packages/grida-canvas-io/__tests__/format-roundtrip.test.ts (2)
2056-2107: ImagePaint roundtrip test uses correct hex16 format — LGTM.The test data now uses a deterministic hex16 string which aligns with the hash-based image reference format.
2379-2428: ImagePaint stroke_paints test updated consistently — LGTM.Uses a distinct hex16 value (
"fedcba9876543210") for stroke_paints, maintaining good test isolation from the fill_paints test.packages/grida-canvas-io/index.ts (2)
599-610:LoadedDocument.assetsaddition is a clean way to expose raw image bytes.Good separation:
document.images(metadata) vsassets.images(bytes keyed by hash).
935-964: Packing logic is consistent: stripimages/bitmapsfrom FlatBuffers and carry assets in the container.This keeps the FlatBuffers payload minimal and makes ZIP the canonical asset carrier.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Prepares a persisted “archive directory” view of the current document state. | ||
| * | ||
| * This method is shared by both: | ||
| * - ZIP archive export (`.grida` via `io.archive.pack`) | ||
| * - OPFS persistence (playground `Cmd/Ctrl+S`) | ||
| * | ||
| * What it does: | ||
| * - Collects **used** image paint `src` references in the document. | ||
| * - For the WASM backend, resolves those runtime refs (`mem://...`) to bytes and a | ||
| * WASM-provided content hash (hex16), producing `images/<hash>.<ext>` entries. | ||
| * - Rewrites image paint `src` in the persisted document from runtime URL → hash id. | ||
| * | ||
| * Notes / policy: | ||
| * - We intentionally **do not** compute SeaHash in JS; persisted image IDs come from WASM. | ||
| * - For DOM backend (hosted HTML flow), we keep external `src` values as-is and do not embed. | ||
| * | ||
| * TODO: support `thumbnail.png` generation/persistence alongside the document. | ||
| */ | ||
| public archivedir(): { | ||
| document: grida.program.document.Document; | ||
| /** | ||
| * Image files to persist under `images/`. | ||
| * | ||
| * Keys are filenames (e.g. "<hash>.png"), values are bytes. | ||
| */ | ||
| images: Record<string, Uint8Array>; | ||
| } { | ||
| const snapshot = this.getSnapshot() | ||
| .document as grida.program.document.Document; | ||
| const images: Record<string, Uint8Array> = {}; | ||
|
|
||
| const usedSrcs = new dq.DocumentStateQuery(snapshot).image_srcs(); | ||
| if (this.backend !== "canvas" || !this._m_wasm_canvas_scene) { | ||
| throw new Error( | ||
| "`archivedir()` requires the canvas/WASM backend to be mounted." | ||
| ); | ||
| } | ||
|
|
||
| // Collect bytes keyed by hash, by reading them back from WASM. | ||
| for (const src of usedSrcs) { | ||
| const hashHex = Editor.__try_parse_hex16_from_image_src(src); | ||
| if (!hashHex) { | ||
| throw new Error( | ||
| `Cannot persist image paint src (expected res://images/<hex16>, mem://<hex16>, or <hex16>): "${src}"` | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| // Rust/WASM normalizes non-RID ids into `res://images/<hex16>`. | ||
| const bytes = this.__get_image_bytes_for_wasm(hashHex); | ||
| if (!bytes) { | ||
| throw new Error( | ||
| `Cannot persist image bytes (WASM missing resource for "${hashHex}")` | ||
| ); | ||
| } | ||
|
|
||
| const mime = snapshot.images?.[hashHex]?.type; | ||
| const ext = (mime && mime.split("/")[1]) || "bin"; | ||
| images[`${hashHex}.${ext}`] = bytes; | ||
| } | ||
|
|
||
| // Rewrite persisted image paint refs (runtime URL -> hash id) before encoding. | ||
| const persisted = structuredClone(snapshot); | ||
| Editor.__rewriteImagePaintSrcDeep(persisted, (src) => { | ||
| const hex = Editor.__try_parse_hex16_from_image_src(src); | ||
| return hex ?? undefined; | ||
| }); | ||
|
|
||
| return { document: persisted, images }; | ||
| } | ||
|
|
||
| public archive(): Blob { | ||
| const { document, images } = this.archivedir(); | ||
| const blob = new Blob([io.archive.pack(document, images) as BlobPart], { | ||
| type: "application/zip", | ||
| }); | ||
|
|
||
| return blob; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the __get_image_bytes_for_wasm implementation
echo "=== Finding __get_image_bytes_for_wasm implementation ==="
rg -n "__get_image_bytes_for_wasm" -A 5 --max-count=5
echo "=== Finding archivedir() call sites ==="
rg -n "archivedir\(\)" --max-count=10
echo "=== Locating WASM TypeScript definitions ==="
fd -a 'grida-canvas-wasm.d.ts' . || echo "Not found with fd, trying rg..."
rg -l "grida-canvas-wasm.d.ts" . || echo "Not found"
echo "=== Checking WASM-related image functions ==="
rg -n "getImageBytes|getImageSize|addImage" -S --max-count=20Repository: gridaco/grida
Length of output: 3380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking playground.tsx archivedir() call and backend gating ==="
rg -n "archivedir\(\)" -B 5 -A 5 editor/grida-canvas-hosted/playground/playground.tsx
echo "=== Finding __try_parse_hex16_from_image_src implementation ==="
rg -n "__try_parse_hex16_from_image_src" -A 10
echo "=== Checking WASM getImageBytes implementation ==="
head -200 crates/grida-canvas-wasm/lib/modules/canvas.ts | tail -100
echo "=== Checking what format is expected by WASM ==="
rg -n "getImageBytes" -B 2 -A 8 crates/grida-canvas-wasm/lib/modules/canvas.tsRepository: gridaco/grida
Length of output: 7120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full __try_parse_hex16_from_image_src implementation ==="
rg -n "__try_parse_hex16_from_image_src" -A 30 editor/grida-canvas/editor.ts | head -60
echo "=== Checking if DOM backend is even supposed to support archivedir ==="
rg -n "this.backend" -B 2 -A 2 | grep -E "(backend|dom|canvas)" | head -30
echo "=== Check if there's any backend === 'dom' or similar in codebase ==="
rg -n 'backend.*==.*["\x27](dom|canvas)["\x27]' --max-count=15Repository: gridaco/grida
Length of output: 4943
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full __try_parse_hex16_from_image_src implementation ==="
sed -n '3278,3320p' editor/grida-canvas/editor.ts
echo "=== Verify archive() call context ==="
sed -n '2951,2960p' editor/grida-canvas/editor.ts
echo "=== Check if there's a check before archive() in other files ==="
rg -n "\.archive\(\)" -B 5 --max-count=10 | head -50Repository: gridaco/grida
Length of output: 3423
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if DOM backend has any image handling for persistence ==="
rg -n "this.backend.*dom" -A 5 -B 5 | head -80
echo "=== Verify loadImagesToWasmSurface gating pattern for reference ==="
sed -n '3302,3320p' editor/grida-canvas/editor.ts
echo "=== Check WASM TypeScript definitions are in sync ==="
cat crates/grida-canvas-wasm/lib/api.d.ts | grep -A 5 "getImageBytes\|addImage"Repository: gridaco/grida
Length of output: 2295
archivedir() hard-requires mounted WASM but lacks backend checks in multiple call sites, contradicting its docstring
The method throws on non-canvas backend (line 2913), yet archive() (line 2951), playground.tsx line 654, playground.tsx line 689, and uxhost-menu.tsx line 115 all call it without gating by backend. The docstring explicitly states a DOM policy (keep external src as-is, no embed), but callers can invoke these methods on the DOM backend and hit the error. This is a UX regression.
Suggested fix: Return early for DOM backend with empty images and unmodified snapshot (matching the documented behavior), and only require WASM for the canvas backend:
Proposed fix
public archivedir(): {
document: grida.program.document.Document;
images: Record<string, Uint8Array>;
} {
const snapshot = this.getSnapshot()
.document as grida.program.document.Document;
const images: Record<string, Uint8Array> = {};
const usedSrcs = new dq.DocumentStateQuery(snapshot).image_srcs();
+ // DOM backend: do not embed bytes; keep src values as-is.
+ if (this.backend !== "canvas") {
+ return { document: structuredClone(snapshot), images };
+ }
if (!this._m_wasm_canvas_scene) {
throw new Error("`archivedir()` requires the WASM surface to be mounted.");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Prepares a persisted “archive directory” view of the current document state. | |
| * | |
| * This method is shared by both: | |
| * - ZIP archive export (`.grida` via `io.archive.pack`) | |
| * - OPFS persistence (playground `Cmd/Ctrl+S`) | |
| * | |
| * What it does: | |
| * - Collects **used** image paint `src` references in the document. | |
| * - For the WASM backend, resolves those runtime refs (`mem://...`) to bytes and a | |
| * WASM-provided content hash (hex16), producing `images/<hash>.<ext>` entries. | |
| * - Rewrites image paint `src` in the persisted document from runtime URL → hash id. | |
| * | |
| * Notes / policy: | |
| * - We intentionally **do not** compute SeaHash in JS; persisted image IDs come from WASM. | |
| * - For DOM backend (hosted HTML flow), we keep external `src` values as-is and do not embed. | |
| * | |
| * TODO: support `thumbnail.png` generation/persistence alongside the document. | |
| */ | |
| public archivedir(): { | |
| document: grida.program.document.Document; | |
| /** | |
| * Image files to persist under `images/`. | |
| * | |
| * Keys are filenames (e.g. "<hash>.png"), values are bytes. | |
| */ | |
| images: Record<string, Uint8Array>; | |
| } { | |
| const snapshot = this.getSnapshot() | |
| .document as grida.program.document.Document; | |
| const images: Record<string, Uint8Array> = {}; | |
| const usedSrcs = new dq.DocumentStateQuery(snapshot).image_srcs(); | |
| if (this.backend !== "canvas" || !this._m_wasm_canvas_scene) { | |
| throw new Error( | |
| "`archivedir()` requires the canvas/WASM backend to be mounted." | |
| ); | |
| } | |
| // Collect bytes keyed by hash, by reading them back from WASM. | |
| for (const src of usedSrcs) { | |
| const hashHex = Editor.__try_parse_hex16_from_image_src(src); | |
| if (!hashHex) { | |
| throw new Error( | |
| `Cannot persist image paint src (expected res://images/<hex16>, mem://<hex16>, or <hex16>): "${src}"` | |
| ); | |
| } | |
| ); | |
| // Rust/WASM normalizes non-RID ids into `res://images/<hex16>`. | |
| const bytes = this.__get_image_bytes_for_wasm(hashHex); | |
| if (!bytes) { | |
| throw new Error( | |
| `Cannot persist image bytes (WASM missing resource for "${hashHex}")` | |
| ); | |
| } | |
| const mime = snapshot.images?.[hashHex]?.type; | |
| const ext = (mime && mime.split("/")[1]) || "bin"; | |
| images[`${hashHex}.${ext}`] = bytes; | |
| } | |
| // Rewrite persisted image paint refs (runtime URL -> hash id) before encoding. | |
| const persisted = structuredClone(snapshot); | |
| Editor.__rewriteImagePaintSrcDeep(persisted, (src) => { | |
| const hex = Editor.__try_parse_hex16_from_image_src(src); | |
| return hex ?? undefined; | |
| }); | |
| return { document: persisted, images }; | |
| } | |
| public archive(): Blob { | |
| const { document, images } = this.archivedir(); | |
| const blob = new Blob([io.archive.pack(document, images) as BlobPart], { | |
| type: "application/zip", | |
| }); | |
| return blob; | |
| } | |
| /** | |
| * Prepares a persisted "archive directory" view of the current document state. | |
| * | |
| * This method is shared by both: | |
| * - ZIP archive export (`.grida` via `io.archive.pack`) | |
| * - OPFS persistence (playground `Cmd/Ctrl+S`) | |
| * | |
| * What it does: | |
| * - Collects **used** image paint `src` references in the document. | |
| * - For the WASM backend, resolves those runtime refs (`mem://...`) to bytes and a | |
| * WASM-provided content hash (hex16), producing `images/<hash>.<ext>` entries. | |
| * - Rewrites image paint `src` in the persisted document from runtime URL → hash id. | |
| * | |
| * Notes / policy: | |
| * - We intentionally **do not** compute SeaHash in JS; persisted image IDs come from WASM. | |
| * - For DOM backend (hosted HTML flow), we keep external `src` values as-is and do not embed. | |
| * | |
| * TODO: support `thumbnail.png` generation/persistence alongside the document. | |
| */ | |
| public archivedir(): { | |
| document: grida.program.document.Document; | |
| /** | |
| * Image files to persist under `images/`. | |
| * | |
| * Keys are filenames (e.g. "<hash>.png"), values are bytes. | |
| */ | |
| images: Record<string, Uint8Array>; | |
| } { | |
| const snapshot = this.getSnapshot() | |
| .document as grida.program.document.Document; | |
| const images: Record<string, Uint8Array> = {}; | |
| const usedSrcs = new dq.DocumentStateQuery(snapshot).image_srcs(); | |
| // DOM backend: do not embed bytes; keep src values as-is. | |
| if (this.backend !== "canvas") { | |
| return { document: structuredClone(snapshot), images }; | |
| } | |
| if (!this._m_wasm_canvas_scene) { | |
| throw new Error( | |
| "`archivedir()` requires the WASM surface to be mounted." | |
| ); | |
| } | |
| // Collect bytes keyed by hash, by reading them back from WASM. | |
| for (const src of usedSrcs) { | |
| const hashHex = Editor.__try_parse_hex16_from_image_src(src); | |
| if (!hashHex) { | |
| throw new Error( | |
| `Cannot persist image paint src (expected res://images/<hex16>, mem://<hex16>, or <hex16>): "${src}"` | |
| ); | |
| } | |
| // Rust/WASM normalizes non-RID ids into `res://images/<hex16>`. | |
| const bytes = this.__get_image_bytes_for_wasm(hashHex); | |
| if (!bytes) { | |
| throw new Error( | |
| `Cannot persist image bytes (WASM missing resource for "${hashHex}")` | |
| ); | |
| } | |
| const mime = snapshot.images?.[hashHex]?.type; | |
| const ext = (mime && mime.split("/")[1]) || "bin"; | |
| images[`${hashHex}.${ext}`] = bytes; | |
| } | |
| // Rewrite persisted image paint refs (runtime URL -> hash id) before encoding. | |
| const persisted = structuredClone(snapshot); | |
| Editor.__rewriteImagePaintSrcDeep(persisted, (src) => { | |
| const hex = Editor.__try_parse_hex16_from_image_src(src); | |
| return hex ?? undefined; | |
| }); | |
| return { document: persisted, images }; | |
| } | |
| public archive(): Blob { | |
| const { document, images } = this.archivedir(); | |
| const blob = new Blob([io.archive.pack(document, images) as BlobPart], { | |
| type: "application/zip", | |
| }); | |
| return blob; | |
| } |
| this.doc.reduce((state) => { | ||
| // Persistable image metadata is keyed by canonical content hash (hex16). | ||
| // Runtime refs use `res://images/<hex16>` (returned by WASM) and are stored in `ImagePaint.src` after registration. | ||
| state.document.images[hash] = { | ||
| url: hash, | ||
| width, | ||
| height, | ||
| bytes: data.byteLength, | ||
| type: type as grida.program.document.ImageType, | ||
| }; | ||
| return state; | ||
| }); |
There was a problem hiding this comment.
Avoid unconditional state.document.images[hash] = ... (can mark document dirty on asset-load, and can crash if missing map)
Line 3383 writes into state.document.images unconditionally:
- If
document.imagescan be absent on some documents, this can throw. - Even when present, setting the same metadata during archive/OPFS load can flip the “dirty” flag right after a reset (because it mutates the document), contradicting the
loadImagesToWasmSurfacedocstring.
Proposed fix (init + only set when missing)
@@
this.images.set(url, ref);
this.doc.reduce((state) => {
// Persistable image metadata is keyed by canonical content hash (hex16).
// Runtime refs use `res://images/<hex16>` (returned by WASM) and are stored in `ImagePaint.src` after registration.
- state.document.images[hash] = {
- url: hash,
- width,
- height,
- bytes: data.byteLength,
- type: type as grida.program.document.ImageType,
- };
+ state.document.images = state.document.images ?? ({} as any);
+ if (!state.document.images[hash]) {
+ state.document.images[hash] = {
+ url: hash,
+ width,
+ height,
+ bytes: data.byteLength,
+ type: type as grida.program.document.ImageType,
+ };
+ }
return state;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.doc.reduce((state) => { | |
| // Persistable image metadata is keyed by canonical content hash (hex16). | |
| // Runtime refs use `res://images/<hex16>` (returned by WASM) and are stored in `ImagePaint.src` after registration. | |
| state.document.images[hash] = { | |
| url: hash, | |
| width, | |
| height, | |
| bytes: data.byteLength, | |
| type: type as grida.program.document.ImageType, | |
| }; | |
| return state; | |
| }); | |
| this.doc.reduce((state) => { | |
| // Persistable image metadata is keyed by canonical content hash (hex16). | |
| // Runtime refs use `res://images/<hex16>` (returned by WASM) and are stored in `ImagePaint.src` after registration. | |
| state.document.images = state.document.images ?? ({} as any); | |
| if (!state.document.images[hash]) { | |
| state.document.images[hash] = { | |
| url: hash, | |
| width, | |
| height, | |
| bytes: data.byteLength, | |
| type: type as grida.program.document.ImageType, | |
| }; | |
| } | |
| return state; | |
| }); |
🤖 Prompt for AI Agents
In `@editor/grida-canvas/editor.ts` around lines 3380 - 3391, The reducer
unconditionally mutates state.document.images which can throw if images is
undefined and can mark the document dirty even when metadata already exists;
inside the this.doc.reduce callback (used by loadImagesToWasmSurface) ensure
state.document.images is initialized if missing (create an empty object) and
only assign state.document.images[hash] when that key is absent (i.e., skip
writing if metadata already exists) so you avoid errors and unnecessary
mutations that flip the dirty flag.
| import { editor } from ".."; | ||
| import type grida from "@grida/schema"; | ||
| import assert from "assert"; | ||
| import cg from "@grida/cg"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Where does query/.. point? =="
python - <<'PY'
import os
print(os.path.normpath("editor/grida-canvas/query/.."))
PY
echo "== Find editor/grida-canvas index entrypoints =="
fd -a '^index\.(ts|tsx|js|mjs|cjs)$' editor/grida-canvas || true
echo "== Check whether editor/grida-canvas/index.* pulls in ./editor or ./query (cycle risk) =="
fd -a '^index\.(ts|tsx|js|mjs|cjs)$' editor/grida-canvas -0 | xargs -0 -I{} rg -n "from\s+['\"]\./(editor|query)\b" -S {} || true
echo "== Confirm editor.ts imports dq =="
rg -n "from\s+['\"]@/grida-canvas/query['\"]|from\s+['\"]\./query" -S editor/grida-canvas/editor.ts || true
echo "== Locate resolvePaints export location (candidate to import directly to avoid cycle) =="
rg -n "resolvePaints" editor/grida-canvas -SRepository: gridaco/grida
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Step 1: Verify path resolution =="
python3 - <<'PY'
import os
print("query/.. resolves to:", os.path.normpath("editor/grida-canvas/query/.."))
PY
echo "== Step 2: List index files in editor/grida-canvas =="
find editor/grida-canvas -maxdepth 1 -type f -name "index.*" 2>/dev/null | head -5
echo "== Step 3: Check if main index exists and its imports =="
if [ -f "editor/grida-canvas/index.ts" ]; then
echo "--- editor/grida-canvas/index.ts ---"
head -30 editor/grida-canvas/index.ts | grep -E "from|import" || true
fi
echo "== Step 4: Check editor.ts imports (targeted search) =="
if [ -f "editor/grida-canvas/editor.ts" ]; then
echo "--- Checking editor.ts for query imports ---"
rg "query" editor/grida-canvas/editor.ts -n -C 1 --max-count=10 || echo "No matches"
fi
echo "== Step 5: Check query/index.ts import of editor (line 1) =="
if [ -f "editor/grida-canvas/query/index.ts" ]; then
echo "--- First 10 lines of query/index.ts ---"
head -10 editor/grida-canvas/query/index.ts
fiRepository: gridaco/grida
Length of output: 2083
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if editor.i file exists =="
ls -la editor/grida-canvas/ | grep -E "editor\.(ts|i|js)" || echo "Checking with find..."
find editor/grida-canvas -maxdepth 1 -name "editor.*" -type f
echo "== Check the actual export chain =="
echo "--- index.ts exports from: ---"
rg "export.*editor" editor/grida-canvas/index.ts -A 2 -B 2
echo "== Check editor.ts structure (first 50 lines) =="
head -50 editor/grida-canvas/editor.ts | grep -E "^(import|export|class|function)" | head -20
echo "== Verify if query is imported at top-level or lazy-loaded =="
rg "import.*dq|from.*query" editor/grida-canvas/editor.ts -B 2 -A 2 | head -20Repository: gridaco/grida
Length of output: 1590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check editor.i.ts imports (first 50 lines) =="
head -50 editor/grida-canvas/editor.i.ts | grep -E "^(import|export)" | head -30
echo "== Check if editor.i imports from editor or query =="
rg "import.*from.*['\"].*editor['\"]|import.*from.*['\"].*query" editor/grida-canvas/editor.i.ts -n | head -20
echo "== Determine what query/index.ts actually uses from editor =="
rg "editor\." editor/grida-canvas/query/index.ts -n | head -10Repository: gridaco/grida
Length of output: 1160
Circular dependency confirmed: query/index.ts and editor.i.ts have bidirectional runtime imports
Line 1 imports editor at runtime. The exported editor module (editor.i.ts) imports dq from this module at line 11, creating a circular dependency. Both imports are at the top level, meaning initialization order determines whether editor.resolvePaints() (used at lines 638, 671) is defined when query/index.ts executes. Depending on bundler behavior, this can cause editor.resolvePaints to be undefined at runtime.
Resolve by either:
- Converting the
editorimport to a local/lazy import inside the functions that use it (lines 551, 638, 671) - Moving query logic that depends on
editor.resolvePaintstoeditor.i.tsor a separate module that doesn't import fromquery
🤖 Prompt for AI Agents
In `@editor/grida-canvas/query/index.ts` around lines 1 - 4, query/index.ts
creates a circular runtime import with editor.i.ts because it imports editor at
module top-level while editor.i.ts imports dq from this module; fix by removing
the top-level import of editor and performing a local (lazy) require/import for
the editor symbol only inside the functions that call editor.resolvePaints (the
call sites around the current lines labeled 551, 638, 671), e.g., inside the
functions that need resolvePaints do const { editor } = await import("..") or
require("..") and then call editor.resolvePaints; alternatively, move any query
code that depends on editor.resolvePaints into editor.i.ts or a new module that
does not import dq to eliminate the bidirectional dependency.
|
|
||
| /** | ||
| * Returns all image paints used across the document (fills + strokes). | ||
| * | ||
| * This is the right primitive to build "persist resources" steps on top of. | ||
| */ | ||
| private image_paints(): Array<{ | ||
| node_id: string; | ||
| target: "fill" | "stroke"; | ||
| paint: cg.ImagePaint; | ||
| }> { | ||
| const out: Array<{ | ||
| node_id: string; | ||
| target: "fill" | "stroke"; | ||
| paint: cg.ImagePaint; | ||
| }> = []; | ||
|
|
||
| for (const node_id of this.nodeids) { | ||
| const node = this.nodes[node_id]; | ||
| if (!node) continue; | ||
|
|
||
| for (const target of ["fill", "stroke"] as const) { | ||
| const { paints } = editor.resolvePaints( | ||
| node as grida.program.nodes.UnknownNode, | ||
| target, | ||
| 0 | ||
| ); | ||
| for (const paint of paints) { | ||
| if (cg.isImagePaint(paint)) { | ||
| out.push({ node_id, target, paint }); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return out; | ||
| } | ||
|
|
||
| /** | ||
| * Returns unique image `src` references used by image paints. | ||
| * | ||
| * - Persisted: typically `hex16` (content hash id) | ||
| * - Runtime: typically `blob:` or `mem://...` | ||
| */ | ||
| image_srcs(): string[] { | ||
| return Array.from( | ||
| new Set( | ||
| this.image_paints() | ||
| .map((p) => p.paint.src) | ||
| .filter(Boolean) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether image nodes exist and how they're shaped in schema usage.
rg -n "type:\s*\"image\"|interface\s+ImageNode|ImageNode" -SRepository: gridaco/grida
Length of output: 11002
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check ImageNode interface definition and src property
echo "=== ImageNode interface definition ==="
sed -n '2232,2245p' packages/grida-canvas-schema/grida.ts
echo ""
echo "=== ComputedImageNode interface definition ==="
sed -n '2246,2250p' packages/grida-canvas-schema/grida.ts
echo ""
echo "=== Search for Editor.archivedir() and image_srcs() usage ==="
rg -n "archivedir|image_srcs" editor/grida-canvas/editor.ts -B 2 -A 3 | head -80Repository: gridaco/grida
Length of output: 1465
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find ISourceValue interface definition
echo "=== ISourceValue interface ==="
rg -n "interface ISourceValue|ISourceValue" packages/grida-canvas-schema -A 5 | head -30
echo ""
echo "=== Check DocumentStateQuery class definition ==="
rg -n "class DocumentStateQuery" editor/grida-canvas/query/index.ts -A 5
echo ""
echo "=== View image_srcs() and image_paints() full implementation ==="
sed -n '616,668p' editor/grida-canvas/query/index.tsRepository: gridaco/grida
Length of output: 3701
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check VideoNode interface ==="
sed -n '2284,2305p' packages/grida-canvas-schema/grida.ts
echo ""
echo "=== Search for video node usage in documents ==="
rg -n 'type:\s*"video"' packages/grida-tree/__tests__/ editor/ | head -20
echo ""
echo "=== Check if image_srcs() name is misleading (should it include video?) ==="
rg -n "image_srcs|image src" editor/grida-canvas/query/index.ts | head -10Repository: gridaco/grida
Length of output: 1416
Add image and video nodes to image_srcs() for complete persistence coverage
image_srcs() is used by Editor.archivedir() for persistence, but currently scans only fill/stroke paints. The method misses image nodes and video nodes (both with src properties), risking incomplete archives/OPFS saves.
Proposed fix (extend to include image and video node srcs)
@@
image_srcs(): string[] {
- return Array.from(
- new Set(
- this.image_paints()
- .map((p) => p.paint.src)
- .filter(Boolean)
- )
- );
+ const srcs = new Set<string>();
+
+ // Image paints (fills + strokes)
+ for (const p of this.image_paints()) {
+ if (p.paint.src) srcs.add(p.paint.src);
+ }
+
+ // Image and video nodes
+ for (const node_id of this.nodeids) {
+ const node = this.nodes[node_id];
+ if (!node) continue;
+ if (node.type === "image" || node.type === "video") {
+ const src = (node as any).src;
+ if (typeof src === "string" && src) srcs.add(src);
+ }
+ }
+
+ return Array.from(srcs);
}🤖 Prompt for AI Agents
In `@editor/grida-canvas/query/index.ts` around lines 616 - 668, image_srcs()
currently collects srcs only from image paints via image_paints(), missing
node-level images/videos; update image_srcs() to also iterate this.nodeids and
collect src from nodes that are image or video types (e.g., check node.type ===
"image" || node.type === "video" or use cg.isImageNode / cg.isVideoNode if
available) and include their node.src values (filtering falsy) into the Set so
archivedir()/persistence covers both paint-based and node-based media sources.
1 participant
New Features
Bug Fixes
Refactor