fix(desktop): persist generated artifacts so designs reload correctly#93
Merged
fix(desktop): persist generated artifacts so designs reload correctly#93
Conversation
Generated artifacts were never written to design_snapshots, so reopening a design showed only an empty assistant message wrapper while the SQLite designs.db reported `design_snapshots: 0 rows`. The renderer's persist path saved messages + thumbnail but skipped snapshot creation entirely. Now applyGenerateSuccess (and applyInlineComment) feed the produced artifact into persistDesignState, which writes a new design_snapshots row chained to the previous one (`type: 'initial'` for the first generation, `'edit'` afterwards). switchDesign already rehydrates previewHtml from the latest snapshot, so reopening a design now restores the artifact instead of a blank preview. Existing schema is reused — no schema_version bump or migration needed.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Findings
- [Major] Silent fallback hides unsupported artifact types and can persist invalid snapshot metadata without surfacing an error, which conflicts with the no-silent-fallback constraint. Evidence
apps/desktop/src/renderer/src/store.ts:289andapps/desktop/src/renderer/src/store.ts:322.
Suggested fix:function toSnapshotArtifactType(coreType: string | undefined): 'html' | 'react' | 'svg' { switch (coreType) { case undefined: case 'html': case 'slides': case 'bundle': return 'html'; case 'svg': return 'svg'; case 'react': return 'react'; default: throw new Error(`Unsupported artifact type for snapshot persistence: ${coreType}`); } }
Summary
- Review mode: initial
- 1 issue found in added/modified lines.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
open-codesign Bot
| if (coreType === 'svg') return 'svg'; | ||
| if (coreType === 'react') return 'react'; | ||
| return 'html'; | ||
| } |
Contributor
There was a problem hiding this comment.
[Major] This default return 'html' silently accepts any unknown artifact type. Please make this mapping exhaustive and throw on truly unsupported types so persistence errors are surfaced instead of hidden.
function toSnapshotArtifactType(coreType: string | undefined): 'html' | 'react' | 'svg' {
switch (coreType) {
case undefined:
case 'html':
case 'slides':
case 'bundle':
return 'html';
case 'svg':
return 'svg';
case 'react':
return 'react';
default:
throw new Error(`Unsupported artifact type for snapshot persistence: ${coreType}`);
}
}
Collaborator
Author
|
Addressed Codex's [Major] silent-fallback finding on |
Contributor
There was a problem hiding this comment.
Findings
- No high-confidence issues found in added/modified lines.
Summary
- Review mode: follow-up after new commits
- Prior bot finding on silent artifact-type fallback appears resolved by exhaustive mapping + explicit throw in
apps/desktop/src/renderer/src/store.ts:291. docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.- Residual risk: no assertion in this diff for parent chaining across multiple persisted snapshots (initial -> edit) or the
applyInlineCommentpersistence path.
Testing
- Not run (automation). Suggested additions: Vitest coverage for sequential snapshot writes verifying
parentIdchaining and oneapplyInlineCommentpersistence case.
open-codesign Bot
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.
Bug
After a successful generation the renderer showed the artifact, but reopening the design later showed only an empty assistant message wrapper (
```html\n\n```). Inspection of~/Library/Application Support/@open-codesign/desktop/designs.dbconfirmed the cause:designs: 2 rowsdesign_messages: contained the user prompt + an assistant text wrapperdesign_snapshots: 0 rowsThe
generate.okIPC log reportedartifacts: 1, so the artifact reached the renderer fine — but the persist path (persistDesignState) only wrote messages + thumbnail and never created a snapshot.switchDesignreadspreviewHtmlfrom the latest snapshot, so the preview came back empty on reload.Fix (option a — write to
design_snapshots)That table is already purpose-built (
artifact_type,artifact_source,prompt,parent_idchain) and the IPC channelsnapshots:v1:createwas already wired through preload. We:type,content,prompt,message) intopersistDesignStateafter bothapplyGenerateSuccessandapplyInlineComment.parentId; first generation usestype: 'initial', subsequent onestype: 'edit'.Artifact.type('html' | 'svg' | 'slides' | 'bundle') onto the snapshot column's allowed values ('html' | 'react' | 'svg');slides/bundlefall through to'html'because their on-disk source is HTML — keeps the CHECK constraint stable, no migration needed.switchDesignalready rehydratespreviewHtmlfromsnapshots[0].artifactSource, so the reload path now restores the artifact correctly.Option (b) — embedding the source into the assistant message body — was rejected because it would duplicate large HTML blobs into chat history, defeat existing snapshot lineage / parent_id chaining, and waste the schema we already pay for.
Tests
New Vitest in
apps/desktop/src/renderer/src/store.test.tscovering the full round-trip:generateto return one HTML artifact.sendPromptagainst a knowncurrentDesignId.snapshots.createwas invoked with{ type: 'initial', artifactType: 'html', artifactSource, prompt, parentId: null }and that the in-memory snapshots store now holds the row.switchDesign, and assertpreviewHtml+messagesare restored.All 289 desktop tests pass; lint + typecheck clean.
PRINCIPLES check
snapshots:v1:*channels andDesignSnapshotV1.schemaVersion = 1.