feat(desktop): add a pinned canvas tab#221
feat(desktop): add a pinned canvas tab#221framethespace wants to merge 2 commits intoOpenCoworkAI:mainfrom
Conversation
Signed-off-by: framethespace <68256458+framethespace@users.noreply.github.com>
Signed-off-by: framethespace <68256458+framethespace@users.noreply.github.com>
2fea064 to
edf2dc7
Compare
There was a problem hiding this comment.
Findings
-
[Blocker] Canvas state is written to disk without an app-level schema wrapper, which violates the repo rule that everything persisted on disk must carry a
schemaVersion.scene.excalidraw.jsonis written as raw Excalidraw JSON andimports.jsonis written as a bare array, so any future migration of canvas metadata becomes guesswork instead of an explicit versioned upgrade path. Evidenceapps/desktop/src/main/canvas-ipc.ts:107,apps/desktop/src/main/canvas-ipc.ts:108,apps/desktop/src/renderer/src/store.ts:614
Suggested fix:const payload = { schemaVersion: 1, sceneJson, importedFiles, lastGeneratedCanvasRevision, }; await writeFile(canvasStatePath(designId), JSON.stringify(payload, null, 2), 'utf8');
-
[Blocker] Canvas load/save/export failures are silently downgraded to empty state or empty attachments, so the user can continue with a prompt that claims to include canvas context while nothing was actually attached. That breaks the project's “no silent fallbacks” rule and makes data loss hard to diagnose. Evidence
apps/desktop/src/main/canvas-ipc.ts:83,apps/desktop/src/renderer/src/store.ts:2868,apps/desktop/src/renderer/src/store.ts:2912,apps/desktop/src/renderer/src/store.ts:2946,apps/desktop/src/renderer/src/store.ts:1682
Suggested fix:try { return await window.codesign.canvas.writeContextFiles({ designId, files: artifacts }); } catch (err) { const message = err instanceof Error ? err.message : String(err); get().reportableErrorToast({ code: 'CANVAS_CONTEXT_EXPORT_FAILED', scope: 'canvas', title: tr('canvas.contextExportFailed'), description: message, }); throw new Error(`Failed to export canvas context: ${message}`); }
-
[Major] The “only resend when changed” check is reset on every design load, so a saved-but-unchanged canvas is treated as dirty after any app restart or design switch.
loadCanvasStateForCurrentDesign()restorescanvasRevisionto1andlastGeneratedCanvasRevisionto0, andsendPrompt()uses that pair to decide whether to reattach the canvas. That means unchanged canvas context is resent indefinitely across sessions, which contradicts the feature contract and adds avoidable tokens/cost. Evidenceapps/desktop/src/renderer/src/store.ts:1621,apps/desktop/src/renderer/src/store.ts:1623,apps/desktop/src/renderer/src/store.ts:2865,apps/desktop/src/renderer/src/store.ts:2866
Suggested fix:interface PersistedCanvasStateV1 { schemaVersion: 1; sceneJson: string | null; importedFiles: LocalInputFile[]; lastGeneratedCanvasRevision: number; } set({ canvasRevision: hasSavedCanvas ? saved.lastGeneratedCanvasRevision : 0, lastGeneratedCanvasRevision: saved.lastGeneratedCanvasRevision, });
Summary
- Review mode: initial. Three issues found: two hard-constraint regressions (
schemaVersionmissing on new on-disk canvas state, silent fallbacks on canvas failures) and one behavioral regression where unchanged canvas context is resent after reload/switch.
Testing
- Not run (automation:
pnpmis not available in this runner). Missing coverage: no test exercises the reload/switch path forlastGeneratedCanvasRevisionpersistence; current canvas tests only cover same-session behavior inapps/desktop/src/renderer/src/store.test.ts:261.
open-codesign Bot
| const importedFiles = parseImportedFiles(record['importedFiles']); | ||
| await mkdir(canvasStateDir(designId), { recursive: true }); | ||
| await Promise.all([ | ||
| writeFile(canvasScenePath(designId), sceneJson ?? '', 'utf8'), |
There was a problem hiding this comment.
[Blocker] This writes raw Excalidraw JSON / a bare imports array directly to disk with no app-level schemaVersion. The repo rule is explicit that persisted disk formats must be versioned, and parseCanvasScene() currently assumes this raw shape forever.
Suggested fix:
const payload = {
schemaVersion: 1,
sceneJson,
importedFiles,
lastGeneratedCanvasRevision,
};
await writeFile(canvasStatePath(designId), JSON.stringify(payload, null, 2), 'utf8');| files: artifacts, | ||
| }); | ||
| } catch (err) { | ||
| console.warn('[open-codesign] buildCanvasContextFiles failed:', err); |
There was a problem hiding this comment.
[Blocker] Returning [] here turns a real export failure into a silent no-op. sendPrompt() keeps going and can still show the canvas badge, so the user gets no signal that their canvas context was dropped.
Suggested fix:
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
get().reportableErrorToast({
code: 'CANVAS_CONTEXT_EXPORT_FAILED',
scope: 'canvas',
title: tr('canvas.contextExportFailed'),
description: message,
});
throw new Error(`Failed to export canvas context: ${message}`);
}| canvasImportedFiles: nextImportedFiles, | ||
| canvasSceneLoaded: true, | ||
| canvasSeed: s.canvasSeed + 1, | ||
| canvasRevision: hasSavedCanvas ? 1 : 0, |
There was a problem hiding this comment.
[Major] This resets dirty tracking after every reload/switch: any non-empty saved canvas comes back as canvasRevision = 1 and lastGeneratedCanvasRevision = 0, so sendPrompt() reattaches unchanged canvas context forever across sessions.
Suggested fix:
set({
canvasRevision: hasSavedCanvas ? saved.lastGeneratedCanvasRevision : 0,
lastGeneratedCanvasRevision: saved.lastGeneratedCanvasRevision,
});
Summary
This adds a pinned
Canvastab to the desktop workspace so people can sketch rough layouts, drop in references, and send that visual context back with the next prompt instead of starting every iteration from a blank text box. The aim is to make the design loop feel a little more natural and collaborative while still keeping the output code-native and local-first.Compatibility / upgradeability / no bloat / elegance: all green.
Type of change
Linked issue
N/A
Checklist
docs/VISION.md,docs/PRINCIPLES.md, andCLAUDE.mdbefore startinggit commit -s)pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleDependency additions (if any)
@excalidraw/excalidraw@^0.18.1
Screenshots / recordings (UI changes)