feat(desktop): slides template workflow for agent-authored decks#949
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
WalkthroughThis PR updates workspace creation to mint empty projects, adds scratch-seed and template-context handling through the agent path, reworks desktop startup around presets and slides templates, and adds SVG resource materialization for slide editing. ChangesDesktop workspace, session startup, and agent flow
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
softmarshmallow
force-pushed
the
feat/desktop-home-redesign
branch
from
ff98a14 to
84109c4
Compare
softmarshmallow
changed the title
…oject auto-create Home redesign slice on top of the merged public/ origin tree (#952). What lands: - Slides preset gallery now renders the REAL bundled `.canvas` decks served from /templates/slides/ (the public/slides-templates publishing unit), hover-scrubbing actual pages via slides-template-loader.ts (code-split; fflate + dotcanvas kept out of the home chunk). PlaceholderSlide is gone. - editor syncs the origin tree on predev/prebuild (`sync:public`); /public/templates/ is gitignored on the host. - Preset rail + chip, workspace picker, reference gallery, composer wiring for the reference-first home. - Daemon `POST /workspaces/create` mints an EMPTY project (no seed, no manifest) — document creation is deferred to the agent's first turn. This REMOVES the prior seed write authority (manifest-injection surface); SECURITY.md GRIDA-SEC-004 updated in lockstep (4 rules -> 3). Boundary tests green (140/140). Known gap (picked up next — see PR body): picking a slides template only PREFILLS the composer prompt today; it does NOT yet materialize the deck into the project. The intended flow (create project -> copy the `.canvas` bundle down -> open the deck) is designed but unimplemented.
softmarshmallow
force-pushed
the
feat/desktop-home-redesign
branch
from
84109c4 to
434ab77
Compare
softmarshmallow
changed the title
coderabbitai
Bot
added
ai
![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: e92e49ebfc
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
editor/lib/agent-chat/build-agent-send.ts (1)
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend
SendMessageFnto cover the{role, parts}arm and eliminate theas nevercast.The local
SendMessageFntype only covers the{text, files}form, so the{role, parts}path at line 113 requiresas never. If the AI SDK'ssendMessagecontract or the parts shape diverges, TypeScript won't catch it. A union return type would restore compile-time safety here.♻️ Suggested type extension
export type SendMessageFn = ( - message: { text: string; files?: FileUIPart[] }, + message: + | { text: string; files?: FileUIPart[] } + | { role: "user"; parts: unknown[] }, options?: { body?: AgentSendBody } ) => void | Promise<void>;Then at the call site:
- void sendMessage({ role: "user", parts } as never, { body }); + void sendMessage({ role: "user", parts }, { body });Also applies to: 104-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editor/lib/agent-chat/build-agent-send.ts` around lines 57 - 60, `SendMessageFn` only types the `{text, files}` payload, so the `{role, parts}` send path in `build-agent-send` is forced through an unsafe cast. Extend `SendMessageFn` to accept both message shapes used by the send flow, and update the call site in `buildAgentSend`/`sendMessage` to pass the typed `{role, parts}` object directly without `as never`; keep the return type aligned with the AI SDK contract so TypeScript can validate both branches.packages/grida-ai-agent/src/protocol/context.ts (1)
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
USER_TEMPLATE_SELECTIONas the computed key inCONTEXT_MARKERS.The registry key
"data-user_template_selection"duplicates the value ofUSER_TEMPLATE_SELECTION. If either changes independently, the lookup inmessage-view.tssilently breaks. Using the constant as a computed property key makes the dependency explicit.♻️ Proposed refactor
export const CONTEXT_MARKERS: Readonly<Record<string, string>> = { - "data-user_template_selection": "user_template_selection", + [USER_TEMPLATE_SELECTION]: "user_template_selection", };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/grida-ai-agent/src/protocol/context.ts` around lines 25 - 27, Update CONTEXT_MARKERS in context.ts to use USER_TEMPLATE_SELECTION as the computed property key instead of a duplicated string literal, so the registry stays aligned with the constant used by message-view.ts. Keep the existing value mapping to "user_template_selection", and make sure the dependency is explicit by referencing USER_TEMPLATE_SELECTION directly in the CONTEXT_MARKERS object definition.editor/scaffolds/desktop/canvas/slide-svg-resources.ts (1)
77-95: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSequential resource reads in the materialization loop.
Each
<image>/<feImage>href is read one at a time viaawaitinside thefor...ofloop. Since each read is independent (differentrelPath, own map entry), these could be issued concurrently withPromise.allto cut round-trip latency for slides with multiple images.♻️ Proposed refactor
- for (const attr of doc.attributes()) { - const href = attr.value; - const relPath = resolveResourceRelPath({ - href, - bundleBasePath: options.bundleBasePath, - slideRelPath: options.slideRelPath, - }); - if (!relPath) continue; - - try { - const { base64 } = await readFileBytes(options.workspaceId, relPath); - const materialized = toDataUrl(relPath, base64, projectionId, sequence++); - restores.set(materialized, escapeXmlAttribute(href)); - attr.set(materialized); - } catch { - // Per `#960` this is a render projection, not document validation. A - // missing/oversized/binary asset should leave only that image broken. - } - } + await Promise.all( + doc.attributes().map(async (attr) => { + const href = attr.value; + const relPath = resolveResourceRelPath({ + href, + bundleBasePath: options.bundleBasePath, + slideRelPath: options.slideRelPath, + }); + if (!relPath) return; + + try { + const { base64 } = await readFileBytes(options.workspaceId, relPath); + const materialized = toDataUrl(relPath, base64, projectionId, sequence++); + restores.set(materialized, escapeXmlAttribute(href)); + attr.set(materialized); + } catch { + // Per `#960` this is a render projection, not document validation. + } + }) + );Note:
sequence++under concurrent execution is non-deterministic ordering but still unique per call, so uniqueness is preserved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editor/scaffolds/desktop/canvas/slide-svg-resources.ts` around lines 77 - 95, The resource materialization loop in slide-svg-resources is performing independent file reads sequentially inside the for...of iteration, which can add unnecessary latency for slides with multiple images. Refactor the logic around resolveResourceRelPath, readFileBytes, and toDataUrl so each valid href is processed concurrently with Promise.all while still preserving the existing restores map updates and per-item error swallowing behavior. Keep sequence++ unique per materialized resource, and ensure the attr.set and restores.set calls still happen for each successful read.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@editor/scaffolds/desktop/home/slides-template-loader.ts`:
- Around line 117-143: The loadSlidesTemplates memoization currently caches a
rejected promise when the initial index.json fetch or parsing fails, which
prevents any retry for the rest of the session. Update loadSlidesTemplates and
its memo assignment so that failures clear memo back to null before rethrowing,
allowing a later call to retry instead of permanently reusing the rejected
promise. Use the existing memo variable and the loadSlidesTemplates function as
the place to implement this retry-safe behavior.
In `@fixtures/prompts/slides-prompts.txt`:
- Line 28: The prompt text is not self-sufficient because it asks for a future
paste of meeting notes, which leaves the fixture incomplete. Update the prompt
in the slides prompt fixture to stand alone by replacing the placeholder with a
concrete notes stub or example meeting notes, or move this wording into an
explicitly multi-turn prompt fixture. Use the prompt text itself as the target
to fix, since there is no code symbol here.
---
Nitpick comments:
In `@editor/lib/agent-chat/build-agent-send.ts`:
- Around line 57-60: `SendMessageFn` only types the `{text, files}` payload, so
the `{role, parts}` send path in `build-agent-send` is forced through an unsafe
cast. Extend `SendMessageFn` to accept both message shapes used by the send
flow, and update the call site in `buildAgentSend`/`sendMessage` to pass the
typed `{role, parts}` object directly without `as never`; keep the return type
aligned with the AI SDK contract so TypeScript can validate both branches.
In `@editor/scaffolds/desktop/canvas/slide-svg-resources.ts`:
- Around line 77-95: The resource materialization loop in slide-svg-resources is
performing independent file reads sequentially inside the for...of iteration,
which can add unnecessary latency for slides with multiple images. Refactor the
logic around resolveResourceRelPath, readFileBytes, and toDataUrl so each valid
href is processed concurrently with Promise.all while still preserving the
existing restores map updates and per-item error swallowing behavior. Keep
sequence++ unique per materialized resource, and ensure the attr.set and
restores.set calls still happen for each successful read.
In `@packages/grida-ai-agent/src/protocol/context.ts`:
- Around line 25-27: Update CONTEXT_MARKERS in context.ts to use
USER_TEMPLATE_SELECTION as the computed property key instead of a duplicated
string literal, so the registry stays aligned with the constant used by
message-view.ts. Keep the existing value mapping to "user_template_selection",
and make sure the dependency is explicit by referencing USER_TEMPLATE_SELECTION
directly in the CONTEXT_MARKERS object definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06a55810-a201-4c50-801b-45c919eafc25
⛔ Files ignored due to path filters (4)
editor/public/assets/desktop-home/presets/image.svgis excluded by!**/*.svgeditor/public/assets/desktop-home/presets/slides.svgis excluded by!**/*.svgeditor/public/assets/desktop-home/presets/video.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
SECURITY.mdeditor/.gitignoreeditor/app/desktop/welcome/page.tsxeditor/kits/agent-chat/message.tsxeditor/kits/composer/composer-react.tsxeditor/lib/agent-chat/bridge-transport.test.tseditor/lib/agent-chat/bridge-transport.tseditor/lib/agent-chat/build-agent-send.test.tseditor/lib/agent-chat/build-agent-send.tseditor/lib/agent-chat/index.tseditor/lib/desktop/bridge.tseditor/lib/desktop/welcome-handoff.tseditor/package.jsoneditor/scaffolds/desktop/canvas/canvas-shell.tsxeditor/scaffolds/desktop/canvas/slide-surface.tsxeditor/scaffolds/desktop/canvas/slide-svg-resources.test.tseditor/scaffolds/desktop/canvas/slide-svg-resources.tseditor/scaffolds/desktop/home/application-preset.tseditor/scaffolds/desktop/home/preset-chip.tsxeditor/scaffolds/desktop/home/preset-rail.tsxeditor/scaffolds/desktop/home/reference-gallery.tsxeditor/scaffolds/desktop/home/slide-scrub-preview.tsxeditor/scaffolds/desktop/home/slides-template-gallery.tsxeditor/scaffolds/desktop/home/slides-template-loader.tseditor/scaffolds/desktop/home/slides-template-preview-dialog.tsxeditor/scaffolds/desktop/home/workspace-picker.tsxeditor/scaffolds/desktop/workbench/agent-pane.tsxeditor/scaffolds/desktop/workbench/use-workspace-file-save.tsfixtures/prompts/README.mdfixtures/prompts/slides-prompts.txtfixtures/prompts/slides-scenarios.jsonlpackages/grida-ai-agent/src/index.tspackages/grida-ai-agent/src/protocol/context.tspackages/grida-ai-agent/src/protocol/run.tspackages/grida-ai-agent/src/runtime/index.tspackages/grida-ai-agent/src/runtime/message-view.test.tspackages/grida-ai-agent/src/runtime/message-view.tspackages/grida-ai-agent/src/runtime/run-input.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.test.tspackages/grida-ai-agent/src/runtime/workspace-agent-bindings.tspackages/grida-daemon/src/http/routes/workspaces.create.test.tspackages/grida-daemon/src/http/routes/workspaces.tspackages/grida-daemon/src/protocol/resources.tspackages/grida-daemon/src/server.tspackages/grida-daemon/src/workspaces.test.tspackages/grida-daemon/src/workspaces.tspublic/package.jsonpublic/turbo.jsonskills/dotcanvas/SKILL.mdskills/slides/SKILL.md
The prior fix relaxed the composer's empty-text guard for `allowEmptySubmit`, but `composer.submit()` was still called without `allow_empty`, so the composer core returned null for a blank editor and `submit()` bailed at `if (!message) return` BEFORE that relaxed guard — a picked-template Send with no typed ask was still dropped and the template lost. Pass `allow_empty: allowEmptySubmit` into `composer.submit()` (and widen the composer handle's `submit` type to the core's `createMessage` params) so the empty message is produced and the guard below does the gating.
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.
Summary
This PR completes the desktop slides template flow on top of the filesystem-backed skills system and the repo-root
public/origin tree.The desktop home now treats a picked slides template as concrete context for the first agent turn: the real bundled
.canvasdeck is loaded for gallery preview, passed through the handoff as template context, and seeded into the agent scratch workspace so the agent can adapt the selected deck instead of reconstructing a deck from a prompt alone.It also includes the host-side desktop slides workaround for SVG image resources tracked in
gridaco/grida#960: saved slide SVGs remain portable, while the desktop slides editor materializes bundle-relative image references into renderabledata:URLs before handing SVG text to@grida/svg-editor.What Changed
.canvastemplates:/templates/slides/workspaces.createProjectno longer seeds a guessed documentimage/feImagehref and xlink href) for editor rendering.canvasbundle rootdata:URLs before write.canvasis a bundle folderWhy
A typed prompt is ambiguous, so the home should not guess whether to create a board, deck, or another document shape. A clicked template is different: the user selected a concrete deck. This PR uses that concrete deck as first-turn context while keeping actual document creation agent-led and filesystem-backed.
The SVG image change is intentionally scoped as a host workaround.
@grida/svg-editorcurrently accepts SVG text without a first-class resource mapping mechanism, so relative image hrefs inside.canvasbundles do not naturally resolve in the editor. This PR keeps that limitation contained to desktop slides and links the workaround togridaco/grida#960for later removal when the editor owns resource resolution.Verification
git diff --checkpnpm exec oxfmt --check ...pnpm --filter editor test -- scaffolds/desktop/canvas/slide-svg-resources.test.tspnpm --filter editor typecheckoxfmt/oxlintpassed for the four latest commitsNotes: