Skip to content

fix: split the artifact-frame notice copy by cause (#6489) - #6513

Merged
bolichen97 merged 1 commit into
mainfrom
fix/artifact-docsilent-copy-6489
Aug 28, 2026
Merged

fix: split the artifact-frame notice copy by cause (#6489)#6513
bolichen97 merged 1 commit into
mainfrom
fix/artifact-docsilent-copy-6489

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

ArtifactBody.tsx rendered 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 || docSilent collapsed a known failure and an ambiguous state into one failure claim.

Change (per the issue's specified fix):

  • failed path unchanged: could_not_render + Retry + RotateCw.
  • docSilent path: 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). failed wins when both are set (the more specific diagnosis).
  • i18n: both keys added to en.manual.json, translated across all 11 locale catalogs (terminology matched to each locale's existing artifactBody entries), en-XA regenerated.
  • Hardening that fell out of adversarial review (5 rounds, dual model-pinned reviewers, see below):
    • useSandboxDoc gained a pending flag; the notice button is disabled={pending} instead of clearing docSilent at click time — a re-mint can resolve with the same URL string (a React no-op: no new load, nothing re-arms the silence window) or hang, and a cleared notice would strand the reader on a dead frame with no affordance.
    • pending has a 15s ceiling so a wedged POST cannot pin the disabled button for the life of the mount.
    • failed no longer clears eagerly at retry start (the accessible name of the control the user just pressed must not flip mid-flight when failed && docSilent); it clears on a successful settle. pending is 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-0 headroom for the longer copy in wordy locales.

Tests

website/src/test/ArtifactBody.iframeBlob.test.tsx (all mutation-verified — each named mutation reds the pin):

  • Existing docSilent assertions updated to the split strings; failed-mint test unchanged (pins the failed path kept its copy).
  • Different-copy pin: failed and docSilent render different message AND action text — merging the branches back reds it.
  • Precedence pin: failed wins when both states are set (reachable: content-change re-mint fails while docSilent is up; the old url survives a failed mint so no new load clears 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.
  • Same-url pin: a re-mint resolving with the same spent URL keeps the notice mounted and re-enables the button after the settle (asserted on the node held from before the click) — clearing docSilent on click, or dropping setPending(false) from the resolve path, reds it.
  • In-flight pin: the button is disabled during the round trip and re-enabled after a same-url settle.

Full frontend suite: 1612 files, 25403 passed. i18n gate chain: 19 checks PASS (I18N_BASE_REF diff-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:

before docSilent

After — cause-neutral copy + honest action:

after docSilent

Failed state, before/after (unchanged copy — pinned by test):

before failed

after failed

Adversarial Review (pre-push)

  • Rounds: 5 (hard cap), dual blind model-pinned reviewers per round (GPT + Opus lanes mirroring the CI review workflows), fresh context each round, fix delta re-reviewed every round.
  • Findings: 18 total — 13 adopted & fixed (incl. 2 Critical: click-clear stranding the reader affordance-less on a same-url/hung re-mint; mid-flight accessible-name flip), 2 rebutted with evidence, 2 noise, 1 initially declined then adopted (next line).
  • Initially declined, then adopted after the CI UX lane independently escalated it: the raw btn btn-sm buttons 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: the role="status" live region voices the existing "Rendering…" string while a re-mint is in flight, so pending is audible as well as visible (no new i18n strings).
  • Accepted-and-deferred (per the First Principles lane's own wording): replacing the 15s MINT_PENDING_CEILING_MS timer with a timeout on the sandboxDocUrl fetch 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.

@CrysisDeu
CrysisDeu requested a review from a team August 28, 2026 09:25
@CrysisDeu
CrysisDeu requested a review from a team as a code owner August 28, 2026 09:25
@CrysisDeu
CrysisDeu requested a review from buluoray August 28, 2026 09:25
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 9827a8896f9eef91e6f39512c7ef8c90ce556bd2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 9827a8896f9eef91e6f39512c7ef8c90ce556bd2 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done. Every claim in the change checked out: pending has 3 real consumers, the btn/btn-sm class genuinely has no CSS rule (only .btn-sweep exists) and zero unfixed usages remain, temp-screenshots/ is a documented commit convention (README + 2,620 tracked files), and 292 sibling capture-*.mjs scripts make the harness a convention too. Final review:

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 ships

Intent: 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.

  1. Silent-frame notice now says "no longer showing" instead of claiming a render failure — justified (the fix; Artifact frame claims a render failure when the reader followed a link inside it #6489)
  2. Its action relabeled "Show artifact" with an Eye glyph — justified (the arrow/label carried the failure claim)
  3. Failed-mint copy wins when both states are set — justified (more specific diagnosis; premise-pinned test)
  4. Both strings land in all 12 locale catalogs — justified (zero-tolerance i18n gates, documented invariant)
  5. Recovery button disables while a re-mint is in flight (3 surfaces) — rides along, declared; same-URL re-mint is a React no-op that would otherwise strand the reader (counted: 3 consumers of pending)
  6. Failure notice no longer flips mid-flight; clears on settle — rides along, declared
  7. Disabled state self-releases after 15s on a wedged mint — rides along, declared; symptom-level with the fetch-timeout cause explicitly deferred and its residual gap named
  8. Notice buttons render as real buttons, not bare text — rides along, declared; verified no .btn/.btn-sm CSS exists, 0 unfixed btn btn-sm siblings remain
  9. Screen readers hear "Rendering…" while a re-mint runs — rides along, declared; reuses an existing key
  10. Screenshot harness + 4 before/after PNGs committed — justified (repo convention: temp-screenshots/README.md, 292 sibling capture scripts)

[FIRST-PRINCIPLES-REVIEWED] 9827a88

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of 9827a8896f9eef91e6f39512c7ef8c90ce556bd2 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • In ArtifactBody.tsx, the docSilent banner has no dismiss: a reader who deliberately followed a link inside the artifact keeps a permanent strip occluding the top of the page they chose, and the only affordance ("Show artifact") destroys that page — add a small dismiss (X) next to Btn so the deliberate-navigation cause has a non-destructive exit.

[UX-REVIEWED] 9827a88

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 9827a8896f9eef91e6f39512c7ef8c90ce556bd2 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 9827a88

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 9827a8896f9eef91e6f39512c7ef8c90ce556bd2: <one-sentence reason>

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 9827a8896f9eef91e6f39512c7ef8c90ce556bd2 — this comment is updated in place on each push.

Review details

The single candidate rests on a <Btn> (which defaults to type="submit") being mounted inside a <form>. The candidate itself admits no form ancestor exists on any artifact/widget render path, and I confirmed the same: the <form> elements in website/src are all in unrelated panels (WebPreviewPanel, PreferencesTab, MochiPage, reminder inputs), none of which render ArtifactBody/WidgetFrame. RemoteArtifactDetailPage already used <Btn> without a type pre-PR, and website/AGENTS.md explicitly prescribes <Btn> over a raw <button>. No concrete input (a) reaches a submit path, so the candidate dies under falsification. No further grounded defect surfaced.

No findings.

[OPUS-REVIEWED] 9827a88

Verdict parsed from the review's SHA-scoped output markers for commit 9827a8896f9eef91e6f39512c7ef8c90ce556bd2.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 9827a8896f9eef91e6f39512c7ef8c90ce556bd2: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 28, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 28, 2026
@CrysisDeu
CrysisDeu force-pushed the fix/artifact-docsilent-copy-6489 branch from 4b7c7ec to 57376f1 Compare August 28, 2026 10:20
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 28, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

ai-review-disposition — first-principles (CONCERNS on 57376f139b3e359b3bbb167ecb59aacf9031de0b)

Per-finding disposition:

  1. Description staleness (the body recorded the <Btn> swap as declined and the AT acknowledgment as missing, both of which this head ships) — fixed. The Adversarial Review section now records: the Btn swap as initially-declined-then-adopted after the UX lane independently escalated it, and the live-region "Rendering…" announcement as the closure of the AT residual. The body edit does not re-trigger this lane, hence this disposition comment.

  2. Subtraction: replace MINT_PENDING_CEILING_MS with a timeout on the sandboxDocUrl fetchaccepted-and-deferred, matching the lane's own framing ("unless the late-arriving-success repair is worth keeping"). It is worth keeping here: the non-aborting ceiling preserves late-arriving successes (both settle paths stay valid after release — documented on the constant), whereas an aborting fetch timeout is a behavior change to a shared hook with four consumers and would surface failed on slow-but-successful mints. The wedged-first-mint gap the timeout would additionally close predates this PR (also documented on the constant). Doing the timeout properly — picking an abort budget, auditing the four consumers for slow-mint tolerance, and deleting the ceiling — is a self-contained follow-up rather than a rider on a copy-split fix. The trade-off comment the lane asked for is already on the constant (useSandboxDoc.ts, MINT_PENDING_CEILING_MS doc: late settles stay valid; wedged first mint named as the gap the ceiling cannot restore).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 28, 2026
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
@CrysisDeu
CrysisDeu force-pushed the fix/artifact-docsilent-copy-6489 branch from 57376f1 to 9827a88 Compare August 28, 2026 12:10
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 28, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) August 28, 2026 22:14
@bolichen97
bolichen97 merged commit 3ab5e96 into main Aug 28, 2026
73 of 75 checks passed
@bolichen97
bolichen97 deleted the fix/artifact-docsilent-copy-6489 branch August 28, 2026 22:27
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Artifact frame claims a render failure when the reader followed a link inside it

2 participants