From 6e375fb9630af0e28affbda03d0b591690b28e57 Mon Sep 17 00:00:00 2001 From: "dayuan.jiang" Date: Sun, 11 Oct 2026 12:11:32 +0900 Subject: [PATCH] refactor(canvas): review fixes for the busy flag, the comparison note and the bundle size check - busyReason had no reader: the chat engine sets isBusy through the store's generic set, like every other flag. - The isSameDocument comment says the two "same document" rules disagree in both directions, so neither is a subset of the other. - The import boundary test bundles minified with one pako (the MCP core resolves its own copy) and caps the canvas core at 160 KB (134 KB now). --- components/chat/chat-engine.tsx | 2 +- lib/diagram-diff.ts | 5 ++++- stores/canvas-store.ts | 7 +------ tests/unit/canvas-import-boundary.test.ts | 12 +++++++++++- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/components/chat/chat-engine.tsx b/components/chat/chat-engine.tsx index 5f6ed5f9..5a868b90 100644 --- a/components/chat/chat-engine.tsx +++ b/components/chat/chat-engine.tsx @@ -729,7 +729,7 @@ export function ChatEngineProvider({ // Canvas components (version cards, the compare dialog, "ask AI") read // the busy flag from the canvas store useEffect(() => { - useCanvasStore.getState().setBusy(isBusy ? "chat" : null) + useCanvasStore.getState().set({ isBusy }) }, [isBusy]) // The page goes (another language mounts a new one): the answer stops, // so it cannot reach the next page's canvas diff --git a/lib/diagram-diff.ts b/lib/diagram-diff.ts index 7ab186de..aa0c3d30 100644 --- a/lib/diagram-diff.ts +++ b/lib/diagram-diff.ts @@ -288,7 +288,10 @@ export function sameFileVars(a: string | null, b: string | null): boolean { * page settings and file variables. This is the web app's one rule for "is * this version on the canvas" (versions, compare, one-step commits). The * MCP server's edit gate has its own (contentFingerprint): it compares the - * cells as written, and leaves page settings and file variables out. + * cells as written, and leaves page settings and file variables out. The + * two disagree both ways (a background change is a difference only here; + * a geometry written "40.0" and "40" is one only there), so neither is a + * subset of the other. */ export function isSameDocument(a: string, b: string): boolean { const pagesOf = (doc: Document) => { diff --git a/stores/canvas-store.ts b/stores/canvas-store.ts index aae5bed9..d2f93be9 100644 --- a/stores/canvas-store.ts +++ b/stores/canvas-store.ts @@ -33,10 +33,7 @@ interface CanvasState { /** Something is changing the canvas (an answer streams in, a sync * runs): version actions and "ask AI" wait */ isBusy: boolean - /** Who set the busy flag ("chat", "sync"); null when not busy */ - busyReason: string | null - set: (partial: Partial>) => void - setBusy: (reason: string | null) => void + set: (partial: Partial>) => void } export const useCanvasStore = create((set) => ({ @@ -48,7 +45,5 @@ export const useCanvasStore = create((set) => ({ isFreehand: false, isDrawioPopupOpen: false, isBusy: false, - busyReason: null, set: (partial) => set(partial), - setBusy: (reason) => set({ isBusy: reason !== null, busyReason: reason }), })) diff --git a/tests/unit/canvas-import-boundary.test.ts b/tests/unit/canvas-import-boundary.test.ts index 16d71047..2c3be7cd 100644 --- a/tests/unit/canvas-import-boundary.test.ts +++ b/tests/unit/canvas-import-boundary.test.ts @@ -122,8 +122,11 @@ describe("canvas import boundary", () => { platform: "browser", format: "esm", jsx: "automatic", - alias: { "@": root }, + // The MCP core (packages/mcp-server/src) resolves pako from its + // own node_modules; one copy, as the shell build must do + alias: { "@": root, pako: path.join(root, "node_modules/pako") }, external: ["react", "react-dom", "next/*"], + minify: true, logLevel: "silent", }) const inputs = result.metafile.inputs @@ -145,5 +148,12 @@ describe("canvas import boundary", () => { .map((specifier) => `${file} -> ${specifier}`), ) expect(nextImports).toEqual([]) + // The canvas core stays small enough for the MCP's browser shell + // (134 KB minified, pako once) + const stage = Object.values(result.metafile.outputs).find( + (output) => + output.entryPoint === "components/canvas/canvas-stage.tsx", + ) + expect(stage?.bytes).toBeLessThan(160 * 1024) }) })