Skip to content

feat(design-canvas): visual asset editor surface — scene model, MCP toolset, konva editor, templates#52

Merged
drewstone merged 8 commits into
mainfrom
feat/design-canvas
Jun 12, 2026
Merged

feat(design-canvas): visual asset editor surface — scene model, MCP toolset, konva editor, templates#52
drewstone merged 8 commits into
mainfrom
feat/design-canvas

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

What

@tangle-network/agent-app@0.13.0 — the visual-asset twin of the sequences surface:

  • ./design-canvas — JSON scene model (pages → typed elements: rect/ellipse/line/text/image/video/group; z-order = array order; per-page bleed + guides; template slots), closed 15-op union with validate/apply, optimistic-revision store contract (stale-rev throws, one retry), template instantiation + channel size presets (square/portrait/story/landscape/OG/A4-print), ~22-tool MCP channel (get_scene_state with per-element AABBs, add/set/move/resize/rotate/group/page ops, bind_slot/apply_data, instantiate_template, create_export).
  • ./design-canvas/drizzle — schema factory (json document + rev + isTemplate) and scoped store.
  • ./design-canvas-reactDesignCanvasEditor: Konva workspace (drag/resize/rotate via Transformer with scale-baking, marquee, inline text editing, snap-to-grid/element/guide with live snap lines, wheel zoom-to-cursor + pan) and chrome (selection-aware toolbar, layers panel with lock/hide/reorder/group tree, rulers with drag-out guides, multi-page strip with thumbnails, bleed/trim overlay, zoom controls) on one shared command stack — gesture-coalesced undo/redo, every action a durable schema operation.
  • ./tools — shared createMcpToolHandler JSON-RPC envelope extracted; sequences refactored onto it (regression suite green, public API unchanged).

Verification

  • 1189/1189 tests (incl. 314-test sequences regression), tsc clean, dist import-verified konva/react/drizzle-free at the root entry
  • Adversarial review (correctness / protocol+scoping / ergonomics+feature-audit) + fix pass: 10 findings fixed (rotated-group AABB, redo index capture, full-page undo restore, template slot rebinding, stale-rev retry scoping, …)

Consumers

Stacked adoption PRs in creative-agent and gtm-agent. Publish 0.13.0 after merge to unblock them.

…es, konva editor — full visual-asset surface

Integrates all eight design-canvas sub-agent deliverables into a shippable
package slice: barrels, tsup entries, package.json 0.13.0 exports/peerDeps,
and cross-agent seam fixes (TS2783 ElementNode listening duplicate, TS4104
SEQUENCE_MCP_TOOLS readonly mismatch, mcp-rpc exported from tools barrel).
- elementExtent group: track minX/minY so rotated-child negative AABB doesn't zero-clip
- storeApplyScenePlan: only retry on stale-rev errors; rethrow everything else
- reorderElementCommand/reorderPageCommand: capture origin index once, guard redo calls
- deletePageCommand: restore full page (elements + guides + bleed) via buildRestoreOps(); bleed via set_page_props
- mcp-tools: optionalPositiveInteger → optionalNonNegativeInteger; set_page_guides array guard; double-call slot fix; instantiate_template 4-phase unbind/dup/apply/rebind avoids ID prediction
- SceneCommandStack.undo/redo return SceneCommand; DesignCanvas persists inverseOperations on undo
- Workspace: dragOriginRef carries real AABB dims so snap uses actual element size
- ElementNode: 256-entry LRU bound on module-level image cache
- contracts.ts: SceneCommandStack.undo/redo typed to return SceneCommand
- New tests: reorder undo-after-redo, deletePageCommand full restore, group extent negative, stale-rev retry guard, instantiate_template slots, set_page_guides type validation
…ome and workspace share one command stack

Extract WorkspaceView (injectable stack) from Workspace.tsx, add
DesignCanvasEditor as the batteries-included composition products mount,
thread the chrome's stack through renderWorkspace ctx, and update lazy
entries so DesignCanvasLazy loads the composed editor.

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — 9ae80152

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-12T22:40:10Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Value Audit — unjustified

Verdict unjustified
Concerns 16 (3 strong-concern, 5 medium, 4 medium-concern, 2 low, 2 weak-concern)
Heuristic 0.1s
Duplication 0.1s
Interrogation 492.5s (2 bridge agents)
Total 492.7s

🔎 Heuristic Signals

🟡 Cruft: commented out code src/design-canvas/export-presets.ts

+// Export quality presets

🟡 Cruft: magic number added src/design-canvas-react/components/InlineTextEditor.tsx

  •    zIndex: 1000,
    

📦 Duplication Scan

🟠 'uuid' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'isTypingTarget' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'stack' already exists elsewhere in the repo ``

New definition in this PR. Also found in: .claude/skills/eval-architect/SKILL.md, .claude/skills/eval-bootstrap/SKILL.md, README.md, create-agent-app/template/CUSTOMIZE.md, create-agent-app/template/_wrangler.toml. Verify this is a new context, not an accidental re-implementation.

🟠 'editorState' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'commit' already exists elsewhere in the repo ``

New definition in this PR. Also found in: AGENTS.md, create-agent-app/template/.dev.vars.example, create-agent-app/template/knowledge/README.md, src/missions/engine.ts, src/missions/service.ts. Verify this is a new context, not an accidental re-implementation.

💰 Value Audit

🟠 AGENTS.md module map is stale after adding three new subpaths [maintenance_debt] ``

The PR adds ./design-canvas, ./design-canvas/drizzle, and ./design-canvas-react (src/index.ts:32-36, tsup.config.ts:32-34, package.json exports), but AGENTS.md's module map (AGENTS.md:21-35) still only lists /sequences and /sequences-react. The project rule says: 'When you add a module... Wire tsup.config.ts + package.json exports + src/index.ts' and AGENTS.md must be kept up-to-date. Update AGENTS.md:21-35 to add rows for the three new subpaths and note Konva/react/drizzle optional-pe

🟡 Sequences MCP handler exports an unused protocol-version constant [duplication] ``

src/sequences/mcp-handler.ts:26-29 exports SEQUENCES_MCP_PROTOCOL_VERSIONS, but a repo-wide grep shows it is referenced nowhere. The refactor extracted the envelope into createMcpToolHandler, which already owns the canonical versions at src/tools/mcp-rpc.ts:20. Remove the leftover constant to avoid drift.

🟡 Command-stack engines duplicate generic undo/redo scaffolding [duplication] ``

src/design-canvas-react/engine/command-stack.ts and src/sequences-react/engine/command-stack.ts both reimplement an identical bounded undo/redo store (history limit, listener set, execute/undo/redo, reset, notify). The domain commands differ, but the generic machinery does not. Consider extracting a tiny createImmutableCommandStack<Document, View> helper to remove the ~100-line duplication and make future surfaces cheaper.

🎯 Usefulness Audit

🔴 Workspace.tsx persist() corrupts undo history with a synthetic rollback command [real_world_viability] ``

src/design-canvas-react/components/Workspace.tsx:229-246 handles a rejected save by executing a synthetic 'rollback' command whose operations() returns the inverse of the failed command. This pushes a fake command onto the shared undo stack instead of undoing the original one. After a failed gesture the stack is [original, rollback]; pressing Undo in the chrome (DesignCanvas.tsx:391-412) pops rollback, re-executes the forward move locally, and sends command.inverseOperations() (the forward move

🔴 useCommitCommand can undo the wrong command if the user edits during a pending save [real_world_viability] ``

src/design-canvas-react/components/DesignCanvas.tsx:140-144 checks stack.canUndo() before rolling back a failed save, but it never verifies that the command whose save failed is still on top of the undo stack. If the user executes another edit while the async onApplyOperations is in flight, the rollback will undo the newer command instead of the failed one. Fix: capture the pending command or the stack depth at commit time and only undo when that command is still on top.

🔴 Test fixture adds an 'id' field that does not exist on SceneDocument [real_world_viability] ``

src/design-canvas/model.ts:18-24 defines SceneDocument without an id field, yet tests/design-canvas/design-canvas-editor.test.ts:17-20 returns { id: 'doc-1', ... } as a SceneDocument. With tsconfig.json strict: true this is an excess-property type error, contradicting the PR claim that tsc is clean and tests pass. Fix: remove id from the fixture, or add a document id to SceneDocument if products actually need it.

🟠 Snap-to-grid/element/guide only draws guides; it does not actually snap the element [product_fit] ``

src/design-canvas-react/components/Workspace.tsx:353-375 computes a SnapResult during drag but only stores it via setActiveSnap() for guide rendering. handleElementDragEnd (lines 377-417) persists the raw finalX/finalY from Konva and never applies snapResult.x / snapResult.y. The PR advertises 'snap-to-grid/element/guide', but currently the guides are purely cosmetic. Fix: apply the snap engine's result to the dragged element position before persisting (and optionally update the live node positi

🟠 Inconsistent slot semantics: apply_data accepts line/group slots but template helpers reject them [api_ergonomics] ``

src/design-canvas/apply.ts:439-446 recolors line and group elements when apply_data targets their slots, while src/design-canvas/templates.ts:61-66 throws for slots on line or group elements. A document with a slotted line or group can be mutated by apply_data but cannot be used with instantiateTemplate / validateBindings. Fix: pick one policy — either allow line/group color slots everywhere and update fillKindForElementKind, or reject them in apply_data validation.

🟠 Workspace gesture failures are rolled back silently with no user-facing error [real_world_viability] ``

src/design-canvas-react/components/Workspace.tsx:229-246 catches onApplyOperations rejections, executes a rollback, and swallows the error. Unlike DesignCanvas.tsx's useCommitCommand, which calls setError(), a user whose drag/delete/group save fails sees the canvas snap back with no explanation. Fix: propagate the error to the chrome's error surface (or the host) so users know why their edit disappeared.


What this audit checks
Pass What it asks
Heuristic Is the PR body explaining why? Vague title? Whitespace-only diff?
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit Is the problem real? Proportional? Already solved? Worth the maintenance cost?
Usefulness Audit Will it be used? Right abstraction? Fits the product? Works in practice?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260612T225005Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — 8fd4100f

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-12T23:01:57Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — d26363de

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-12T23:02:12Z

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Value Audit — unjustified

Verdict unjustified
Concerns 15 (4 strong-concern, 5 medium, 2 medium-concern, 2 low, 2 weak-concern)
Heuristic 0.2s
Duplication 0.1s
Interrogation 343.9s (2 bridge agents)
Total 344.2s

🔎 Heuristic Signals

🟡 Cruft: commented out code src/design-canvas/export-presets.ts

+// Export quality presets

🟡 Cruft: magic number added src/design-canvas-react/components/InlineTextEditor.tsx

  •    zIndex: 1000,
    

📦 Duplication Scan

🟠 'uuid' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'isTypingTarget' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'stack' already exists elsewhere in the repo ``

New definition in this PR. Also found in: .claude/skills/eval-architect/SKILL.md, .claude/skills/eval-bootstrap/SKILL.md, README.md, create-agent-app/template/CUSTOMIZE.md, create-agent-app/template/_wrangler.toml. Verify this is a new context, not an accidental re-implementation.

🟠 'editorState' already exists elsewhere in the repo ``

New definition in this PR. Also found in: src/sequences-react/components/TimelineEditor.tsx. Verify this is a new context, not an accidental re-implementation.

🟠 'commit' already exists elsewhere in the repo ``

New definition in this PR. Also found in: AGENTS.md, create-agent-app/template/.dev.vars.example, create-agent-app/template/knowledge/README.md, src/missions/engine.ts, src/missions/service.ts. Verify this is a new context, not an accidental re-implementation.

🎯 Usefulness Audit

🔴 React export hook is public but never wired [real_world_viability] ``

DesignCanvasProps.onExport is defined in src/design-canvas-react/contracts.ts:146, destructured in src/design-canvas-react/components/DesignCanvas.tsx:165, and never called anywhere in the React tree (src/design-canvas-react/components/Toolbar.tsx has no export UI). A consumer providing onExport will expect a built-in export flow, but nothing triggers it. Either wire an export button/menu to exportPageDataUrl and call onExport, or remove the prop until the UI exists.

🔴 Keyboard shortcuts fire twice when the workspace is focused [real_world_viability] ``

WorkspaceView attaches onKeyDown handlers to its own div in src/design-canvas-react/components/Workspace.tsx:469-590 (Delete/Backspace, Mod+Z/Y), while DesignCanvas also installs a global window.addEventListener('keydown', ...) in src/design-canvas-react/components/DesignCanvas.tsx:439-468 for the same keys. Neither calls stopPropagation(), so when the workspace div has focus a single Delete/Undo keypress is handled twice. Remove one layer or call stopPropagation() in WorkspaceView

🔴 Optimistic rollback can undo the wrong edit under lag [real_world_viability] ``

useCommitCommand in src/design-canvas-react/components/DesignCanvas.tsx:118-155 executes the command locally, then awaits onApplyOperations. If the user makes another edit before the server rejects, the catch block still calls stack.undo(), which undoes the top of the stack—the newer, unrelated edit. WorkspaceView.persist in src/design-canvas-react/components/Workspace.tsx:229-246 has the same flaw. Track the pending command identity and only roll back that exact command, or hold a write

🟠 AGENTS.md module map was not updated for the new module [product_fit] ``

The repo's AGENTS.md module map (AGENTS.md:21-35) ends at /trace and /crypto.../stream; the new /design-canvas and /design-canvas-react subpaths are absent. The 'When you add a module' instructions (AGENTS.md:70-75) explicitly require updating the module map. Add rows documenting what each new subpath owns and composes so future agents and reviewers know where the surface lives.

🟠 Slot mutation has inconsistent validation between paths [api_ergonomics] ``

bind_slot validates slot uniqueness (src/design-canvas/validate.ts:246-263), but set_attrs can also change slot via the base-attr allow-list (src/design-canvas/validate.ts:134-143) without that uniqueness check, allowing duplicate slots. Also, apply_data supports group recoloring (src/design-canvas/apply.ts:449-466) while listTemplateSlots rejects group slots (src/design-canvas/templates.ts:61-67). Decide whether group slots are valid and route all slot changes through bind_slot vali

💰 Value Audit

🔴 Slot binding is implemented twice and set_attrs/add_element can create duplicate slots [duplication] ``

Only validateBindSlot (src/design-canvas/validate.ts:246-263) enforces document-wide slot-name uniqueness. However, slot is also a base attribute, so set_attrs (validate.ts:134-143) and add_element (validate.ts:111-132) accept it without uniqueness validation. A UI or model call that writes slot through set_attrs can therefore introduce duplicate slot names, which later makes collectSlots throw during apply_data (src/design-canvas/apply.ts:415-425) and breaks template filling. At the same time,

🟡 Generic editor primitives are reimplemented alongside the sequences surface [duplication] ``

The design-canvas editor adds its own immutable command stack (src/design-canvas-react/engine/command-stack.ts), zoom/pan math (src/design-canvas-react/engine/zoom-pan.ts), and snap engine (src/design-canvas-react/engine/snap.ts) that mirror the sequences editor's src/sequences-react/engine/command-stack.ts, zoom.ts, and snap.ts. The pattern (history limit, undo/redo, reset, screen-to-doc transforms, threshold/zoom snapping) is shared but not extracted. Because the two surfaces are twins this is

🟡 React chrome components are large and tightly coupled [proportion] ``

DesignCanvas.tsx is 655 lines and Workspace.tsx is 901 lines, bundling chrome layout, keyboard handling, gesture state, and Konva rendering. This is proportional to shipping a full editor, but future changes will be harder to review and test than smaller components. If the surface evolves, split Workspace.tsx into pan/marquee/drag subcomponents and DesignCanvas.tsx into toolbar/page/layer wiring.


What this audit checks
Pass What it asks
Heuristic Is the PR body explaining why? Vague title? Whitespace-only diff?
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit Is the problem real? Proportional? Already solved? Worth the maintenance cost?
Usefulness Audit Will it be used? Right abstraction? Fits the product? Works in practice?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260612T230937Z

Workspace.tsx persist() and DesignCanvas.tsx useCommitCommand both had broken
rollback logic:

1. persist() called stack.execute(synthetic-rollback-command) on save rejection,
   pushing a phantom 'rollback' entry onto the undo stack. The redo stack was
   cleared and the user gained a spurious undo step that re-applied the failed
   change.

2. useCommitCommand called stack.undo() blindly on rejection. If the user
   executed commands B or C while command A's save was in-flight, stack.undo()
   undid the newest command (C), not the failed one (A).

Fix: add SceneCommandStack.rollback(command) which locates the specific command
by reference in the undo stack, applies its inverse to the current state, splices
it out, and clears the redo stack. For non-overlapping attribute edits the inverse
commutes correctly past subsequent commands so their effects are preserved. Both
persist() and useCommitCommand now call stack.rollback(command) with the captured
command reference.

Tests in tests/design-canvas/rollback-correctness.test.ts cover:
- pre-fix evidence showing the old broken patterns
- rollback with no interleaving (undo stack empty, doc restored)
- rollback with interleaved independent command (B effect preserved, A removed)
- no-op safety when command not in stack (stale rejection handler)
- idempotence (double-fire safe)
- redo stack cleared on rollback
- rollback of middle command (B) leaves A in history

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — 7bfba72a

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-12T23:13:32Z

@drewstone

Copy link
Copy Markdown
Contributor Author

Reviewed + the one P3 fixed (dropped the prior-version narrative in the AABB-seed comment). Verdict: merge — the 16k is mechanism not baked product UI (konva editor is an optional subpath excluded from the root barrel; scene model is pure domain-parametrized data; MCP reuses the shared createMcpToolHandler; same factoring as /sequences). konva is an optional peer with correct SSR isolation. The 37 local test failures are a better-sqlite3 native-binding env issue (rebuild → 1198/1198 pass); CI build green. Publishing 0.13.0 on merge to unblock the consumers (#339/#249).

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — ac2007fb

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-12T23:20:10Z

@drewstone drewstone merged commit 287e886 into main Jun 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants