fix: split the artifact-frame notice copy by cause (#6489) - #6513
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Cause-neutral copy is the only honest option an opaque sandbox permits, and the pending/no-click-clear mechanics correctly guard the reader's sole recovery affordance. [DESIGN-REVIEWED] 9827a88 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All verification is done. Every claim in the change checked out: First-Principles-Verdict: PASS The copy split fixes the named lie at cause level, and every rider is declared, harm-named, and fixes all its counted siblings at once. What this change shipsIntent: stop telling a reader who followed a link inside an artifact that it "couldn't render," and stop offering a "Retry" that destroys their page — a FIX.
[FIRST-PRINCIPLES-REVIEWED] 9827a88 |
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS Honest, state-accurate copy split with real pending feedback — the notice now says only what the surface can verify, and the action names its outcome. Suggestions
[UX-REVIEWED] 9827a88 |
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: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe single candidate rests on a No findings. [OPUS-REVIEWED] 9827a88 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
4b7c7ec to
57376f1
Compare
|
ai-review-disposition — first-principles (CONCERNS on Per-finding disposition:
|
The frame notice rendered one message ('Couldn't render this artifact' + Retry) for two states: failed (the mint failed - the claim is accurate) and docSilent (the frame loaded a document that never reported a height). docSilent is either an engine renavigation 404ing the single-use doc URL or the reader deliberately following a link inside the sandbox; the two are indistinguishable from outside an opaque origin, so the failure claim was wrong half the time and 'Retry' destroyed a page the reader chose to open.
failed keeps its copy. docSilent now says 'This artifact is no longer showing' with a 'Show artifact' action (same re-mint handler, labeled by what it does). failed wins when both are set. Two new keys under components.artifactBody across all locale catalogs + en-XA regen; tests updated plus a pin that the two states render different copy.
Closes #6489
57376f1 to
9827a88
Compare
Problem / Motivation
ArtifactBody.tsxrendered ONE notice — "Couldn't render this artifact" + Retry (components.artifactBody.could_not_render) — for two different states:failed: the document mint itself failed. The copy is accurate.docSilent: the frame loaded but its document never reported a height past the grace window. This has two indistinguishable causes: an engine renavigation 404ing the single-use doc URL (notice correct), and the reader deliberately clicking a link inside the sandboxed frame (notice false — and "Retry" re-mints, destroying the page they chose to open).The frame is an opaque-origin sandbox, so the causes cannot be told apart; the copy must therefore not assert a failure. Deliberately deferred from #6481 because the copy lands in all locale files under the zero-tolerance i18n gates.
Why it matters
Any reader who follows a link inside an artifact gets told the artifact "couldn't render" — a false failure claim — and the offered "Retry" silently destroys the page they navigated to. The surface lies about what happened and then punishes the user for believing it.
What changed (motivation → approach → change)
Symptom → root cause: one notice keyed on
failed || docSilentcollapsed a known failure and an ambiguous state into one failure claim.Change (per the issue's specified fix):
failedpath unchanged:could_not_render+ Retry + RotateCw.docSilentpath: new cause-neutral copy "This artifact is no longer showing" (no_longer_showing) with a "Show artifact" action (show_artifact) — same re-mint handler, labeled by what it does for the user, with an Eye glyph instead of the reload arrow (the arrow carried the failure claim the string stopped making).failedwins when both are set (the more specific diagnosis).en.manual.json, translated across all 11 locale catalogs (terminology matched to each locale's existingartifactBodyentries),en-XAregenerated.useSandboxDocgained apendingflag; the notice button isdisabled={pending}instead of clearingdocSilentat click time — a re-mint can resolve with the same URL string (a React no-op: no newload, nothing re-arms the silence window) or hang, and a cleared notice would strand the reader on a dead frame with no affordance.pendinghas a 15s ceiling so a wedged POST cannot pin the disabled button for the life of the mount.failedno longer clears eagerly at retry start (the accessible name of the control the user just pressed must not flip mid-flight whenfailed && docSilent); it clears on a successful settle.pendingis the click acknowledgment, wired into the WidgetFrame and RemoteArtifactDetailPage retry buttons too.role="status"on the notice text span (not the container, which would re-announce the button's name as status prose);flex-wrap/min-w-0headroom for the longer copy in wordy locales.Tests
website/src/test/ArtifactBody.iframeBlob.test.tsx(all mutation-verified — each named mutation reds the pin):failedwins when both states are set (reachable: content-change re-mint fails while docSilent is up; the old url survives a failed mint so no newloadclears docSilent), with premise pins (second mint attempted, same iframe src) so it cannot green as a tautology — inverting the ternaries to key on docSilent reds it.setPending(false)from the resolve path, reds it.Full frontend suite: 1612 files, 25403 passed. i18n gate chain: 19 checks PASS (
I18N_BASE_REFdiff-scoped included).npx tsc -b, eslint, build: clean. Backend: zero backend lines changed (tree byte-identical to main); isort/flake8/mypy clean.Manual verification
Screenshot harness (
website/scripts/capture-artifact-notice-split.mjs, committed): real built SPA behind the fixture stub server, both notice states driven end-to-end (mint 500 → failed; minted doc without the height reporter → docSilent past the 3s grace window).Screenshots / video
Before (main) — docSilent falsely claims a render failure over a page the reader navigated to:
After — cause-neutral copy + honest action:
Failed state, before/after (unchanged copy — pinned by test):
Adversarial Review (pre-push)
btn btn-smbuttons are an inert class (no CSS rule exists behind them), so the sole recovery affordance rendered as bare text — both notice buttons (ArtifactBody + WidgetFrame) now use the design-system<Btn>. The AT residual was closed the same way: therole="status"live region voices the existing "Rendering…" string while a re-mint is in flight, sopendingis audible as well as visible (no new i18n strings).MINT_PENDING_CEILING_MStimer with a timeout on thesandboxDocUrlfetch itself. Kept the ceiling deliberately: a non-aborting release preserves late-arriving successes (both settle paths stay valid, documented in the constant's comment), while an aborting fetch timeout would trade that for closing the wedged-first-mint gap — a behavior change to a shared hook that deserves its own PR.Closes #6489
no linked issue: N/A — linked above.