fix(dashboard): promote the widget and remote artifact frames to their own layer - #7931
Conversation
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSUX-level review of This PR is a pure rendering-reliability fix: it adds UX-Verdict: PASS Invisible paint-reliability fix — no new copy, controls, or layout; it only makes blank widget/artifact frames render, with the load reveal preserved. [UX-REVIEWED] a0d0849 |
Design Review (Fable 5) — ✅ PASSDesign-level review of Verified: Design-Verdict: PASS Closes a real, proven paint-skip gap by applying the sibling frame's measured remedy to the three consumers it missed; scope and limits are stated honestly. Suggestions
[DESIGN-REVIEWED] a0d0849 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All repository facts are verified. I have what I need to emit the review. First-Principles-Verdict: PASS Three siblings of a measured, issue-reported paint defect get the exact remedy their fourth sibling already carries — nothing rides along, no new surface. What this change shipsIntent: make agent-generated widgets and artifacts actually paint in the desktop app instead of showing a permanently blank box (#6176). This is a FIX.
Counts run: Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] a0d0849 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
f3535a6 to
32e5e6b
Compare
…g layer Inline chat widgets, gallery thumbnails and the remote artifact detail view all render model-authored HTML in a sandboxed frame loaded from a gateway-minted document, exactly as the local artifact frame does. That frame was promoted to its own compositing layer after an engine was measured laying a document out, running its scripts and reporting a correct height while rasterizing nothing -- a correctly sized, visible frame painting an empty box. The other three consumers of the shared mint were left out, so the inline-widget surface named first in #6176 still carried the gap. All four frames now carry the translateZ(0) promotion. On the gallery thumbnail it is composed onto the existing scale() rather than replacing it: a 2D transform makes a stacking context but does not force layer promotion, and the scale is that frame's whole geometry. Nothing about the sandbox flags, the CSP, or the single-use document channel changes. Scope is the four consumers of that mint. Frames that build their document inline and never reach the mint are outside it and are filed as #8037. A macOS desktop reproduction is still outstanding, so this states the mechanism it fixes rather than claiming the whole of the report. Refs #6176
32e5e6b to
a0d0849
Compare
buluoray
left a comment
There was a problem hiding this comment.
Source-verified review of head a0d0849361523b10cc7ff5e301be4bb74e54331e (fresh files pulled from the GitHub API at that SHA, not from any shared local checkout).
Verdict: approve. 0 blocking, 0 non-blocking.
What the change does: adds transform: translateZ(0) to three of the four useSandboxDoc iframe surfaces (inline WidgetFrame, RemoteArtifactDetailPage, and WidgetThumb), matching the promotion ArtifactBody already carries. On WidgetThumb it is composed onto the existing scale(...) rather than replacing it, so the thumbnail geometry is preserved.
Independent verification (did not rely on the PR body or bot verdicts):
- Effective, not inert. Each
transformlands on the inlinestyleof the actual rendered iframe. TheclassNameon each frame (w-full border-none bg-card ...) carries notransformutility, so the inline value is not overridden.translateZ(0)forces a real compositing layer (the 3D form; a bare 2Dscaleonly makes a stacking context), which is exactly why the thumb composes rather than replaces. - No overlay painted under the frame.
WidgetFrame: the progress indicator isabsolute inset-0 z-10and the expand backdrop isfixed -z-10; the promoted iframe sits at z-index auto (0), so it paints below the z-10 overlay and above the -z-10 backdrop — order unchanged.WidgetThumb: the iframe is the sole child of arelative overflow-hiddenwrapper, and its stacking context is nested inside that wrapper, so parent-level card badges still paint above it;overflow-hiddenstill clips the scaled frame and nocontainwas added.RemoteArtifactDetailPage: the iframe is the sole child of arounded-xl overflow-hiddenwrapper; the comments sidebar is a separate flex column, not an overlap. - opacity-on-load reveal preserved.
WidgetFramekeepsopacity: iframeLoaded ? 1 : 0, and the new "still reveals on load" test guards that the promotion is additive rather than a replacement of the gate. - New assertions redden on revert.
WidgetFrame.test.tsxandRemoteArtifactDetailPageCoverage.test.tsxmount and assertstyle.transform === 'translateZ(0)'(empty on revert). TheArtifactsPage.narrowSingleAxissource assertion requires the composedscale(${scale}) translateZ(0)form and rejects the baretranslateZ(0), so both a dropped promotion and a clobbered scale redden. - No blocking-rule (AUTOSDE) surface: no new controls, icons, strings, or layout; no sandbox/CSP change;
website/AGENTS.mdmandates no spec update for this subsystem.
Could not verify without a browser: the actual pixel result on a real iOS WebKit device, and the GPU compositing-layer memory footprint of promoting many inline widget frames on a long transcript. The transcript is virtualized so only visible/overscan frames mount, and an iframe already carries its own document, so the marginal layer cost is modest and matches the already-shipped ArtifactBody pattern — but I am flagging it as unmeasured rather than confirmed.
Mochi's WidgetFrame and the meetings AgentPanel render model-authored HTML in sandboxed frames whose documents are built inline and passed to srcDoc, so they never reach the gateway sandbox-doc mint that #7931 covered. Both frames carried no compositing promotion, leaving them exposed to the same measured failure mode: an engine lays the document out, runs its scripts, reports a correct height, and never rasterizes it -- a correctly sized, visible frame painting an empty box, silent by construction. Port #7931's exact remedy onto both sites: transform: translateZ(0) added to each iframe's inline style. Neither frame carried an existing transform, so the plain 3D form suffices; nothing about sandbox flags, CSP, or how the documents are built changes, matching the invariants #7931 kept. Tests mirror #7931's promotion assertions: a new mochi test renders WidgetFrame and pins the transform plus the survival of its existing inline sizing, and the AgentPanel html-output suite gains the same pin. The regression is invisible in Chromium and in a test DOM, so the style assertions are the whole guard. Closes #8037
…8076) Mochi's WidgetFrame and the meetings AgentPanel render model-authored HTML in sandboxed frames whose documents are built inline and passed to srcDoc, so they never reach the gateway sandbox-doc mint that #7931 covered. Both frames carried no compositing promotion, leaving them exposed to the same measured failure mode: an engine lays the document out, runs its scripts, reports a correct height, and never rasterizes it -- a correctly sized, visible frame painting an empty box, silent by construction. Port #7931's exact remedy onto both sites: transform: translateZ(0) added to each iframe's inline style. Neither frame carried an existing transform, so the plain 3D form suffices; nothing about sandbox flags, CSP, or how the documents are built changes, matching the invariants #7931 kept. Tests mirror #7931's promotion assertions: a new mochi test renders WidgetFrame and pins the transform plus the survival of its existing inline sizing, and the AgentPanel html-output suite gains the same pin. The regression is invisible in Chromium and in a test DOM, so the style assertions are the whole guard. Closes #8037 Co-authored-by: Kiro Crew <bolichen97@users.noreply.github.com>
…rame (#8105) The capsule's metrics readout is an inline segment, not a popover: one button both shows the readings and toggles them away. Its open branch pushed a segment only when the system-metrics query had ERRORED, so the third state -- open, no frame, no error -- pushed nothing at all. `sysMetrics` is undefined for the whole of the first fetch AND for the retry window of a failing one, because react-query reports `isError` only once its retries are spent. On a host where /api/system hangs or never produces a frame the readout was therefore logically open with its toggle absent from the DOM, and the click aimed at it landed on the capsule's background: the reported "the cpu, metrics etc. doesn't open". Every open state now pushes a toggle. The pending one carries an em dash per metric rather than a spinner, which is the distinction the sibling usage segment already draws in this same capsule and for the same reason -- a spinner asserts a fetch is about to land, and on a host that never reports metrics that claim never comes true. It reuses the loaded branch's own "no valid reading" glyph, so the two open states differ in opacity rather than in shape. Second half, for the "something left in the background" artifact. The header carries `backdrop-filter`, and the two readout groups inside it carry `container-type: inline-size`, which implies layout containment. When a capsule segment mounted or unmounted inside one, the strip it vacated had to be re-sampled through that blur, and damage from a contained subtree did not always reach a backdrop sharing a layer with the content beneath it -- the old blurred pixels stayed on screen. `.topbar-glass` is now promoted to its own compositing layer, the same remedy #7931 applied to the sandbox-doc frames, and the reason the notification popover portals to <body> to escape this containing block rather than living inside it. Two spellings, because focus mode drives this element's `transform` from an inline style that wins over the rule: `backface-visibility:hidden` promotes on its own and nothing writes it. The two halves meet: the first one also removes the segment MOUNT the second one is sensitive to. The capsule used to gain a button and a divider when the frame landed; it now only re-renders text inside a button that was already there. No rung in the collapse ladder #7851 repaired is touched. The capture script asserts the actions group's content box -- what the container queries measure -- is byte-identical across the absent, pending and loaded scenes, and that nothing outside the capsule moves when the frame arrives. Scope is the two symptoms #7851 did not cover. The artifact half states the mechanism it fixes rather than claiming the whole of the report: a promoted layer and an unpromoted one render the same correct pixels, so no screenshot from this Linux host can distinguish them, and the reporter's macOS reproduction is outstanding. Its contract is pinned from the stylesheet source instead. Closes #7967 Co-authored-by: Joe Guo <zejiangg@amazon.com>
Problem / Motivation
In the desktop app, agent-generated widgets and HTML artifacts render as a blank
area while the same content against the same gateway renders correctly in a
browser opened side by side (#6176). The content is generated correctly; it is
simply not displayed, and nothing surfaces an error.
One hook,
useSandboxDoc, mints a gateway-served document URL and fourcomponents load it into a sandboxed frame:
ArtifactBody.tsx(artifact detail, Artifacts tab)WidgetFrame.tsx(inline chat widget)RemoteArtifactDetailPage.tsx(remote artifact detail)library/ArtifactThumbs.tsx(gallery thumbnail)That is the scope of this change: the four consumers of that mint, three of
which were missing the promotion. Model-authored HTML also reaches the screen
through frames that build their document inline and never touch the mint, so a
fix here cannot reach them; two such frames are filed separately as #8037.
#6461 promoted the artifact frame with
transform: translateZ(0)aftermeasuring an engine that laid a document out, ran its injected scripts and
reported a correct 385px height while rasterizing nothing: a correctly sized,
visible frame painting an empty box, with four unrelated post-load
invalidations each making it appear. Its three siblings load the same kind of
document, through the same mint, behind the same opacity-on-load reveal, and
were left un-promoted -- so the inline-widget surface #6176 names first still
carried the gap the artifact surface had closed.
Why it matters
All four frames reveal at
opacity: 0until their ownloadfires, and adocument served with an opaque origin cannot be observed by the parent. A frame
that lays out but never paints is therefore indistinguishable from one still
loading: the reader gets a permanently empty box with no error and no retry
affordance. Inline widgets are the primary way generated visual output is read,
and on the gallery thumbnail the same failure repeats across every card at once.
The desktop shell makes this class of defect more reachable than a browser tab
does. It hosts the dashboard in a
WebContentsViewinside aBaseWindow(
website/electron/window-lifecycle.js), a different compositing host from anordinary tab, which is consistent with the report that a browser against the
same gateway paints what the desktop window does not.
What changed (motivation -> approach -> change)
Symptom: a sandboxed document frame is laid out and never rasterized, which
presents as an empty box rather than an error. Cause of the remaining instances:
the promotion that fixes it was applied to one of the four frames that share the
mint. Change: apply the identical promotion to the other three.
transform: translateZ(0)is the one remedy of the four measured in #6461 thatneeds no timing -- the others must fire after
load, which races a slowdocument. The promotion is additive: the opacity-on-load reveal stays, because
removing it would show the browser's own canvas for the length of the document
fetch, which some engines paint white regardless of the element background.
On the gallery thumbnail the promotion is composed onto that frame's
existing
scale(...)transform rather than replacing it. Two reasons thedistinction is load-bearing: the scale is the thumbnail's entire geometry, so
clobbering it would render every card at full width inside a column-width box;
and a 2D transform creates a stacking context but does not force a
compositing layer the way the 3D form does, which is why that frame needed the
change at all despite already carrying a transform.
translateZ(0)adds novisual offset with no perspective set.
Deliberately not in scope:
Content-Security-Policy: sandboxresponse header, theframe-ancestorsvalue, and the single-use document channel are all untouched. Nothing here
loosens the sandbox, adds
allow-same-origin, or relaxes a CSP.srcDocframes outside the mint, filed as Model-authored srcDoc frames in mochi and meetings can render blank (no compositing promotion, outside the sandbox-doc mint) #8037. They are adifferent code path in different apps; folding them in would put three app
surfaces in a channel fix.
colorScheme, which the artifact frame and the thumbnail both already set, isnot added to
WidgetFrame: it addresses a first-paint canvas flash, not askipped paint, and that frame already covers the flash with
bg-cardplus theopacity gate.
outcome) is pre-existing and independent of this change.
Tests
website/src/test/WidgetFrame.test.tsx-- the frame carries the promotion;and, separately, it still reveals on load rather than having the promotion
displace the opacity gate.
website/src/test/ArtifactsPage.narrowSingleAxis.test.tsx-- the thumbnailcarries the promotion composed onto its scale, asserted both ways so a
future edit cannot drop the scale and keep the promotion.
website/src/test/RemoteArtifactDetailPageCoverage.test.tsx-- extends theexisting sandboxed-document assertion on the html body with the promotion.
Every new guard is mutation-verified: removing the property turns the suite red
(1 failed of 40 for the widget frame, 1 of 10 for the thumbnail) and restoring
it turns it green again. That verification is the point of the assertions -- as
#6461's own test comment records, dropping the property looks completely
harmless in Chromium, so nothing else in the suite would catch a silent revert.
Run on the touched files only, against the rebased tree:
vitest runon the five touched-surface suites -- 128 passed (the siblingartifact-frame suite is included to show its existing guard still holds)
tsc --noEmit-- cleaneslinton the six touched files -- 0 errors; the 3 warnings(
RemoteArtifactDetailPage.tsxx2,ArtifactThumbs.tsxx1) are pre-existingand byte-identical on the base, with only line numbers shifted by this diff
npm run i18n:checkwithI18N_BASE_REFset to the merge base -- all 10checks ok, including the zero-tolerance
added-lines,vs-base,unit-added-linesandunit-vs-baseManual verification
Not performed, and this is a limit worth stating plainly rather than papering
over. The report in #6176 is on the macOS desktop build; this work was done on
Linux, so the desktop shell that ships was never exercised and no console or CSP
output from a reproducing session was captured. The evidence here is
source-level and test-level.
What this PR establishes: three of the four frames on the sandbox-doc mint
lacked a promotion their sibling has for exactly this symptom class, and they
now carry it. The change cannot regress anything, since a promoted frame renders
identically on any engine that was already painting it.
What remains outstanding: a macOS desktop reproduction, which is what would show
whether a skipped paint is the whole story there. A reviewer with that build can
settle it in one pass with the three evidence items from the triage comment on
the issue -- the status of the
/sandbox-doc/<id>/<token>request in the desktopdevtools Network tab, any
frame-ancestorsorsandboxconsole line, andwhether the Windows and Linux desktop builds reproduce.
Screenshots / video
Why no screenshot:
translateZ(0)is a non-rendering compositing hint -- onany engine that already paints the frame, the rendered output before and after
this change is identical, so a before/after pair would be two identical images.
The change is observable only on an engine that skips the frame's first paint,
and that is the environment this branch could not exercise (see Manual
verification). The repository's own guard for the sibling frame records the same
constraint:
ArtifactBody.iframeBlob.test.tsxsays "only an iOS device canobserve the regression, so this assertion is the whole guard: dropping the
property looks completely harmless in Chromium." A Chromium screenshot pair
would misrepresent the evidence rather than add to it, which is why this uses
the marker instead of attaching one.
Related Issues
Refs #6176
#8037 covers the two inline-
srcDocframes outside this mint, found whilereviewing this PR.
Context, not duplicates: #6461 introduced the promotion this PR extends, #6481
fixed the contradictions #6461 left, and #6184 was an earlier attempt withdrawn
by its author after the spent-URL causal chain was falsified. This PR does not
revive that chain -- it changes nothing about the document channel.
Pattern harvest
Rule candidate: review-prompt
Pattern: a remedy applied at one consumer of a shared hook while its sibling
consumers keep the defect.
useSandboxDochas four call sites; #6461 fixed one.This PR is its own worked example of why the rule is needed, twice. Its first
revision enumerated three consumers and fixed two, and a First Principles review
caught that the count was wrong --
library/ArtifactThumbs.tsxwas a fourthconsumer, already carrying a 2D transform that looks like a promotion and is
not. The same review then caught this description overstating its own reach,
which is how #8037 was found. So the generalizable check is mechanical rather
than a reminder to be careful: grep every call site of the shared hook, state for
each whether it is fixed or why it is not, and say plainly which surfaces reach
the same screen WITHOUT going through that hook. The guard matters most when the
defect is invisible in the development engine, because then no local run
distinguishes a fixed consumer from an unfixed one.
Checklist