fix(web): persist BYOK deck artifacts as decks#3594
Conversation
Visual regression reviewHead: 13 changed · 4 unchanged · 0 missing baseline · 0 failed Changed cases
Unchanged cases
Visual diff is advisory only and does not block merging. |
lefarcen
added
size/S
PerishCode
left a comment
PerishCode
left a comment
There was a problem hiding this comment.
@YOMXXX I reviewed the BYOK deck artifact persistence path and the focused regression coverage. The change keeps normal HTML manifests unchanged while allowing deck projects to persist artifact-tag HTML with deck-html renderer metadata and the pptx export affordance, which lines up with the preview/export bug in #1698. Nice focused fix with the regression anchored on the API-mode flow.
|
Hi @YOMXXX! This PR looks quiet for a bit, so I'm checking in to make sure it doesn't get stuck. If you are waiting on review, feedback, or a specific unblock from the team, feel free to say so here. If faster coordination helps, you can also drop into our contributors channel in Discord: We want to help move this forward instead of letting it stall. |
|
Hey @YOMXXX — quick status update so this does not look stuck: @PerishCode has already approved the current head, and CI is green on this branch. From our side this is tracking as a restoration bug fix for #1698 rather than a new product-direction change, so there is not extra review feedback pending here right now. If there is anything specific you want re-checked, feel free to drop it in this thread — otherwise we will keep it moving from the maintainer side. |
|
@YOMXXX friendly reminder: this PR has been waiting on an author response for more than 3 days after reviewer or maintainer feedback. When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no author activity for more than 5 days after feedback may be closed automatically, but they can be reopened when work resumes. |
4 participants
Fixes #1698
Why
I picked this up from the P0 BYOK/API-key-mode PPT preview bug. In API mode the model emits a plain
<artifact type="text/html">...</artifact>payload, and the web persistence path was always stamping that HTML artifact as a generichtmlmanifest.That loses the deck renderer metadata and
pptxexport affordance for PPT projects, which can leave the right preview/export path behaving differently from the CLI/tool-emitted deck path.What users will see
PPT/deck projects generated through API key mode keep their generated HTML artifact as a deck artifact. The preview path receives
deck-htmlrenderer metadata, and the artifact manifest includes thepptxexport option.Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-pack/tools-prflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies); workspace-packagepackage.jsonfiles are out of scope. Include a paragraph on what we get vs. what bytes we ship (seeCONTRIBUTING.md→ Code style)Screenshots
Not applicable; no new UI surface.
Bug fix verification
apps/web/tests/components/ProjectView.api-empty-response.test.tsxmainand green on this branch? The new regression spec failed before the source change on this branch's main-based worktree, then passed after the fix.Validation
pnpm installpnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/components/ProjectView.api-empty-response.test.tsxpnpm --filter @open-design/web typecheckpnpm guardpnpm typecheckgit diff --check