Restore prose-level fade animation for Transparent Stream - #5506
11 commits merged into
Conversation
|
| Filename | Overview |
|---|---|
| static/messages.js | Core fade logic: new Transparent Stream path in _renderStreamingFadeMarkdown appends plain text to assistantBody as a tracking buffer, routes anchor prose through _streamFadeRenderer for markdown-aware fade spans; regular _fadeTextEffect path unchanged (still uses _smdWrite). _streamFadeAppendText is defined but has no call sites in the current code. |
| static/ui.js | Adds _bindTransparentFadeCleanup, _appendTransparentFadeText, and _refreshTransparentFadeProseRow to preserve the transparent live-row DOM and append only delta fade spans; candidateIsFadeProse correctly gates on .msg-body.stream-fade-active which is set by _anchorProseIncrementalNode; wrote variable replaced by renderedRows.length for clarity. |
| static/style.css | Animation duration extended from 240ms to 620ms with a softer cubic-bezier and three-stop opacity keyframe; default --stream-fade-ms updated to match. Minimal risk — pure CSS change. |
| tests/test_smooth_text_fade.py | Adds five new test cases: fade routing predicate, reduce-motion gating, hidden-body plain-text behaviour, anchor prose receiving revealed text, and append-without-replacing invariant. Updates existing tests for new timing constants. |
| tests/test_issue5367_transparent_live_row_reconcile.py | Extends FakeElement to handle document fragments and track node moves; adds fade-prose reconciliation assertions verifying that existing DOM nodes are preserved and only delta fade spans are appended. |
| tests/test_live_to_final_anchor_visible_order.py | Minor assertion update to match the new anchorProcessText variable name introduced by this PR; no logic change. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant SC as scheduleRender
participant RSFM as _renderStreamingFadeMarkdown
participant AB as assistantBody
participant UAP as _upsertAnchorProcessProse
participant APIN as _anchorProseIncrementalNode
participant SFR as _streamFadeRenderer
participant RLAST as _renderLiveAnchorScene
participant RTL as _refreshTransparentLiveRow
participant RTFP as _refreshTransparentFadeProseRow
participant TR as TransparentLiveRow
SC->>RSFM: displayText
alt Transparent Stream
RSFM->>AB: createTextNode(delta) plain text buffer
else Regular fadeTextEffect
RSFM->>AB: _smdWrite with fade spans
end
RSFM-->>SC: _streamFadeDomText
SC->>UAP: anchorProcessText
UAP->>APIN: key, revealed text
APIN->>SFR: wrap new words in fade spans
SFR-->>APIN: anchor node with stream-fade-active
UAP->>RLAST: render transparent scene
RLAST->>RTL: candidate node
RTL->>RTFP: "candidateIsFadeProse=true"
RTFP->>TR: _appendTransparentFadeText delta only
TR-->>TR: animationend cleanup to text nodes
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant SC as scheduleRender
participant RSFM as _renderStreamingFadeMarkdown
participant AB as assistantBody
participant UAP as _upsertAnchorProcessProse
participant APIN as _anchorProseIncrementalNode
participant SFR as _streamFadeRenderer
participant RLAST as _renderLiveAnchorScene
participant RTL as _refreshTransparentLiveRow
participant RTFP as _refreshTransparentFadeProseRow
participant TR as TransparentLiveRow
SC->>RSFM: displayText
alt Transparent Stream
RSFM->>AB: createTextNode(delta) plain text buffer
else Regular fadeTextEffect
RSFM->>AB: _smdWrite with fade spans
end
RSFM-->>SC: _streamFadeDomText
SC->>UAP: anchorProcessText
UAP->>APIN: key, revealed text
APIN->>SFR: wrap new words in fade spans
SFR-->>APIN: anchor node with stream-fade-active
UAP->>RLAST: render transparent scene
RLAST->>RTL: candidate node
RTL->>RTFP: "candidateIsFadeProse=true"
RTFP->>TR: _appendTransparentFadeText delta only
TR-->>TR: animationend cleanup to text nodes
Reviews (5): Last reviewed commit: "fix(#5493): honor reduced motion for tra..." | Re-trigger Greptile
33035f6 to
0cdcf3a
Compare
|
Addressed the remaining Greptile concerns in |
0cdcf3a to
c849f69
Compare
🔬 Gate certification — RED ⛔ (reduced-motion only half-disabled + hidden per-word DOM growth on long streams — 2 SILENT) + visible motion → NathanCertified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (reproduce) | SHIP-WITH-FIXES — 2 SILENT; I confirmed SILENT-1 by inspection |
| Full pytest suite | 2 failed / 11921 passed — both non-defects (nous env flake + test_issue4536 isolation flake) |
| Fade/reconcile/anchor tests | ✅ 74 passed |
Findings
⛔ SILENT (I CONFIRMED) — prefers-reduced-motion does NOT fully disable the fade (static/messages.js:4099): _shouldUseLiveProseFade() returns _shouldUseStreamFade() || _shouldUseTransparentStreamFade() with NO reduced-motion guard — the helper _streamFadeReduceMotionEnabled() exists (4107) but the predicate never consults it. So a reduced-motion user still gets the JS-paced word reveal + the done drain delay; only the CSS opacity animation (style.css:6788) is disabled. Motion isn't actually off. Fix (Codex-exact): return !_streamFadeReduceMotionEnabled() && (_shouldUseStreamFade() || _shouldUseTransparentStreamFade()); + a reduced-motion regression test.
⛔ SILENT (Codex, verified) — long replies accumulate hidden per-word span nodes (static/messages.js:4392 / ui.js:11019): the transparent branch appends .stream-fade-word spans into the legacy assistantBody, which is then HIDDEN while the visible anchor row owns rendering. Hidden/display:none animated spans can't be relied on to fire animationend, so the cleanup at messages.js:4128 may never run for that duplicate hidden DOM → unbounded hidden-node growth on long transparent-stream replies. Fix: don't create fade spans in the hidden legacy assistantBody for Transparent Stream — append plain text / only update _streamFadeDomText, leaving animated spans only in the visible anchor prose row.
✅ Good (keep): coexists with shipped #5400 identity-reconcile + #5454 (0 transparent-event-enter refs — no flicker reintroduced); reconcile path extended not replaced; CSS reduced-motion media query present (just needs the JS gate too); 74 fade/reconcile/anchor tests pass.
Recommendation to the next agent / author
RED — gate-fail/changes-requested (2 SILENT): (1) gate _shouldUseLiveProseFade() on !_streamFadeReduceMotionEnabled() so reduced-motion fully disables the JS-paced reveal + drain, not just the CSS opacity; (2) don't append .stream-fade-word spans into the hidden legacy assistantBody (they can't fire animationend → hidden-node leak on long streams) — keep animated spans only in the visible anchor prose row. The fade itself is well-built and coexists with the shipped transparent-stream work. THEN, per Nathan's motion preference, park for his visual sign-off — he'll want to SEE the fade (record a GIF/video of a streaming reply, not a still). concept 4/5 (nice restore of the prose fade #5493). Author @rodboev (T1). crit=2.
_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; SILENT-1 confirmed (_shouldUseLiveProseFade lacks the streamFadeReduceMotionEnabled guard that exists at 4107), SILENT-2 hidden-span animationend-leak Codex-verified via ui.js:11019 legacy-body-hidden; coexists w/ #5400/#5454 (0 entrance-keyframe refs), 74 tests. Visible motion → Nathan. Cert valid for sha:c849f697.
c849f69 to
0c1bdce
Compare
|
Addressed in
|
🔬 Gate certification — GREEN (engineering) ✅ · CONVERGED (both round-1 SILENTs fixed) · ⏸️ visible motion → Nathan (GIF)Certified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (reproduce) | SAFE TO SHIP — 0 findings; both round-1 SILENTs confirmed fixed |
| Full pytest suite | 2 failed / 11988 passed — both non-defects (nous env flake + test_issue4536 isolation flake); no #5513 collateral (doesn't touch routes.py) |
| Fade/reconcile/anchor tests | ✅ 81 passed |
Findings — both round-1 SILENTs CLOSED
✅ SILENT-1 (reduced-motion) FIXED: _shouldUseLiveProseFade() now returns !_streamFadeReduceMotionEnabled() && (_shouldUseStreamFade() || _shouldUseTransparentStreamFade()) — reduced-motion users get NO JS-paced word reveal / drain (previously only the CSS opacity was disabled). Full motion-off honored.
✅ SILENT-2 (hidden-span leak) FIXED: the reconcile no longer appends .stream-fade-word animated spans into the hidden legacy assistantBody — it tracks _streamFadeDomText + renders animated spans in the visible target (final render via renderMd sanitized), and the hidden body gets textContent/plain-text. So no orphaned animated nodes in a hidden subtree that can't fire animationend → cleanup is reachable, no node growth on long streams.
✅ Coexists w/ shipped work: 0 transparent-event-enter refs (no flicker/entrance-replay reintroduced vs #5400/#5454), live→final anchor order + #5493 preserve-anchor-prose intact, renderMd-sanitized (no XSS via the fade path). Codex SAFE, 81 tests.
Recommendation to the next agent
Engineering-GREEN — merge from branch gate-rebase/5506-prose-fade-transparent-stream (sha:ba066960), NOT the PR's stale head 0c1bdce0 — but PARK for Nathan's VISUAL sign-off. Both my round-1 findings are fixed (reduced-motion fully off, no hidden-span leak), it coexists with the shipped transparent-stream work, and Codex is SAFE. Per Nathan's loading-state/motion preference, he wants to SEE the fade in motion — record a GIF/video of a streaming reply (the prose word-fade), not a still, for his approval; also verify a reduced-motion capture shows NO animation. concept 4/5 (nice prose-fade restore #5493; converged cleanly). Author @rodboev (T1). crit=2.
_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; both round-1 SILENTs verified fixed (_shouldUseLiveProseFade gated on !streamFadeReduceMotionEnabled; animated spans only in visible target, hidden body textContent — no animationend leak), coexists w/ #5400/#5454 (0 entrance-keyframe refs), Codex SAFE + 81 tests + suite green bar 2 known flakes. Visible motion → Nathan (GIF). Cert valid for sha:ba066960.
8cdb4ea
|
Shipped in v0.51.893 (deployed live). Thanks @rodboev — Transparent Stream now fades newly-streamed prose word-by-word again, without the row-level flicker #5367 removed. Reduced-motion is honored at both the JS and CSS layers. We went with the simpler design where the fade is part of Transparent Stream regardless of the off-by-default 'Fade text effect' toggle (coupling them fought the settings-persistence round-trip); the toggle description now says so. 🙏 |
Re-adds the per-word prose fade for newly-streamed assistant text in Transparent Stream WITHOUT reintroducing the row-level entrance flicker (nesquena#5367). Reduced-motion honored at both JS and CSS layers; thinking/tool rows not animated. Option A: the fade is part of Transparent Stream regardless of the off-by-default 'Fade text effect' toggle; settings description clarified accordingly. Gate: Codex SAFE, Fable SHIP-UX, suite 12158/0, browser-smoke clean. Co-authored-by: Rod Boev <rod.boev@gmail.com>
Thinking Path
What Changed
static/messages.js: route Transparent Stream prose through the fade renderer when appropriate, append only newly revealed words as fade spans, and clean completed fade spans back to text nodes.static/ui.js: preserve transparent live-row DOM during refresh so previous words do not restart their animation.static/style.css: keep streamed fade rows stationary while letting newly appended prose fade in.tests/test_smooth_text_fade.py: cover prose fade routing, appended span behavior, cleanup, and preference-independent Transparent Stream fade behavior.tests/test_issue5367_transparent_live_row_reconcile.py,tests/test_live_to_final_anchor_visible_order.py, andtests/test_streaming_markdown.py: keep the transparent live-row, visible-order, and streaming markdown invariants covered.Why It Matters
Transparent Stream keeps the no-flicker row behavior from #5367 while restoring the smooth prose fade expected from streaming assistant text. New words fade in, already visible words stay stable, and completed animation spans collapse back into ordinary text.
Verification
python -m pytest tests/test_smooth_text_fade.py tests/test_issue5367_transparent_live_row_reconcile.py tests/test_live_to_final_anchor_visible_order.py tests/test_issue3820_chat_activity_display_mode.py tests/test_streaming_markdown.pynpx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"http://localhost:8787with a real streamed response in Transparent Stream. New words fade in, previous words do not reanimate, and completed fade spans clean back to plain text.Full-suite CI context, not a required local check unless requested:
pytest tests/ -v --timeout=60.Risks / Follow-ups
transparent-event-enteranimation remains absent, so thinking rows, tool rows, and old-event opacity behavior stay outside this change.Upstream
Closes #5493.
Model Used
GPT-5 via Codex CLI