fix(stream): stop tearing live progress words across block boundaries - #7082
5 commits merged into
Conversation
|
| Filename | Overview |
|---|---|
| static/ui.js | Reconciles live prose using source-space cursors, block-local insertion, and parser-owned row adoption; the previously reported Markdown and pending-tail defects are addressed. |
| static/messages.js | Exposes the stateless rendered-prefix muting helper needed to preserve already-visible fade state during reconciliation. |
| tests/test_live_prose_fade_cursor_source_space.py | Adds focused real-parser regression coverage for word integrity, Markdown preservation, pending parser text, and live-node identity. |
| tests/test_issue5367_transparent_live_row_reconcile.py | Updates the extracted reconciler harness with the new append-target helper dependency. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Streamed source delta] --> B[Incremental Markdown parser]
B --> C[Parser-owned live row]
C --> D{Existing source cursor?}
D -->|Yes| E[Append source-space delta]
D -->|No| F[Adopt or preserve parsed DOM]
E --> G[Trailing Markdown block]
F --> G
G --> H[Intact live prose and stable fade nodes]
Reviews (6): Last reviewed commit: "fix(stream): adopt parser-owned fade row..." | Re-trigger Greptile
|
Reading One edge in the append target
const _TRANSPARENT_FADE_BLOCK_TAGS = new Set([
'P','DIV','H1','H2','H3','H4','H5','H6','BLOCKQUOTE','LI','UL','OL','PRE','TABLE',
]);For the reported prose case the trailing child is a SuggestionEither drop the pure-container tags ( A cheap way to pin this: extend Prose path itself looks correct — this is only about the non- |
nesquena-hermes
left a comment
There was a problem hiding this comment.
Really solid fix, @ruizanthony — the root-cause analysis is exactly right and the gate confirms it. The regression is real and this restores the universal live-streaming convention (ChatGPT / Claude / Codex all keep a partially-streamed word in-flow; master tore it across the block boundary as "Ces deu" / "x fichiers").
Gate results:
- Codex: SAFE TO SHIP — verified exact cursor behavior across ASCII/emoji/CJK, live-to-final + reattach paths, default-render isolation; no regression risk.
- Non-vacuous proof: your two regression tests FAIL on clean master (the tear is genuinely present) and PASS on the fix — confirmed by re-running them against
origin/master. Plus 1028 focused stream/fade/transparent tests green. - Fable UX: SHIP-WITH-UX-FIXES — confirms the fix is in the right layer (row reconciler, not a CSS band-aid), reuses the existing fade machinery, no regression to the fade animation / thinking rows / settled message, and correct under RTL. Caret unaffected (it's suppressed in fade mode).
One minor UX-polish fix before it ships, and it's in the app's own established pattern:
Should-fix — mute the already-rendered prefix on the no-cursor rebuild branch
The no-cursor rebuild branch of _refreshTransparentFadeProseRow (the branch that now fires in exactly the scenario that used to tear, since incremental rows never set data-stream-fade-text) clears the body and re-wraps all words as .is-new. So the whole visible row dips to opacity 0 and fades back over ~620ms, and because every node is replaced, native scroll anchoring may pick a new anchor (the one-time vertical bounce the #6257 comment in _bindTransparentFadeCleanup warns about).
This is strictly better than the persistent torn word, it's one-time per row, and reduced-motion users never see it — but it contradicts the app's own #6783 standard, where messages.js (_streamFadeMuteRenderedPrefix, messages.js:4997) deliberately mutes exactly this "replay the fade on ALL visible words at once" defect. The ui.js rebuild branch has no such mute, and the helper is currently closure-private.
Fix (~3 lines): on the rebuild branch, snapshot body.textContent before clearing, then apply the messages.js _streamFadeMuteRenderedPrefix idiom so only genuinely-new tail words animate. Export it on window the same way __anchorProseIncrementalNode is exposed. This kills both the one-time whole-row fade flash and the all-new-nodes scroll-anchor bounce, and it also repairs the pre-existing rewind case where messages.js carefully mutes the fade on the new node only for the ui.js reconciler to rebuild it as all-is-new.
Once that lands, this is a clean ship on the crown-jewel streaming surface. Re-push and I'll re-gate + capture the before/after mid-stream proof for the maintainer. Nice work tracking this to the one-character parser lag.
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
nesquena-hermes
left a comment
There was a problem hiding this comment.
@ruizanthony, the original word-tearing fix still holds, and the two follow-up commits do fix the requested full-row fade replay. The re-gate found one objective regression in the new parser-DOM clone path, so this exact head still needs a correction.
Must fix: keep one parser/DOM authority after the cursorless rebuild
static/ui.js:_refreshTransparentFadeProseRow now clones the candidate streaming-Markdown body into the preserved visible row, then stores a trimmed source cursor. On the next source growth, hasCursor is true, so the normal branch sends the source delta to _appendTransparentFadeText as raw text. The live parser keeps updating the detached candidate body, not the visible clone.
A sandboxed diagnostic drove the real vendored parser through this sequence:
Hello **
Hello **world** done
At the second refresh, the candidate parser DOM contained <strong>world</strong>, but the visible preserved row had no <strong> and displayed literal **world**. The cursor then advanced to the full source, so the parser-owned structure could not repair that live row until later canonical settlement.
Please keep one authority after the clone transition. Either make the fresh parser-owned candidate row/body the visible keyed owner, or continue reconciling later updates from the candidate's parsed DOM. Do not clone parser output and then append later source bytes through _appendTransparentFadeText. Preserve the rendered-prefix mute behavior, and keep parser pending/MEDIA tails owned by the parser.
Please also add a regression to tests/test_live_prose_fade_cursor_source_space.py that performs two reconciliations across incomplete-to-complete Markdown using the real parser. It should assert that the visible row contains <strong>world</strong>, contains no literal **, has every byte exactly once, and stays correct through finalization. The current five tests pass, but they stop at the initial clone or append only punctuation, so they miss this transition.
Gate evidence at bf087f8b505e51444af0080e34ed5fd2aba4d2a0:
- threat scan: CLEAN
- focused sandbox suite: 5 passed
- reviewer diagnostic: deterministic real-parser reproduction
- worktree returned clean after the diagnostic
Once this lands, the existing Nathan visual-sign-off requirement still applies because this changes visible live Markdown/fade behavior.
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
bf087f8 to
3dbe47b
Compare
|
Re-certification request on exact head The 2026-08-18 must-fix is addressed in
Regression added: Local focused slice on this head: 44 passed ( Please re-gate this exact head. Nathan visual sign-off still applies. |
🔬 Gate certification — RED ⛔ (prior fixes closed; post-rewind fade identity still regresses)Certified contributor head: Verdict: What I ran
The suite residuals are inherited box/topology failures: atomic group ownership, four linked-worktree gitignore checks, one connection-error lifecycle case, keyless LM Studio readiness, two sandbox-home path checks, and two wheel-build setup errors. The exact pinned-base control reproduced every identity. Prior findings — CLOSED ✅
MUST-FIX — post-rewind growth replaces and prematurely settles the prior animated tail (SILENT)Site: Production reachability is the rewind/cache-miss graph where the keyed visible row remains Current schedule:
Independent reviewer reproduction through the real production reconciler: after the first frame The all-mode live gate staying GREEN does not disprove this finding: its ordinary fixture remained on the steady Exact fix and regressionPromote/adopt the actual parser-owned candidate row into the existing row's DOM position once instead of copying its children into the old row. Preserve the old row's relevant interactive/position state, apply rendered-prefix muting to the actual parser body, and return the adopted candidate so later keyed renders hit Add a deterministic production-reconciler regression with two later growth frames after the separate-owner transition:
UX and screenshotsFable's code review is RecommendationDo not merge this head. Preserve @ruizanthony's correct word-tear and parser-authority repairs, fix the one remaining post-rewind identity transition, add the two-growth animation-lifetime regression, then run a full exact-head re-gate and the required final motion/visual proof. Gate-certifier layer only. No merge, tag, deploy, close, or contributor-branch write was performed. This certificate is valid only for contributor head |
|
Post-rewind fade identity shipped on exact head
Regression: |
🔬 Gate certification — GREEN ✅ (engineering converged; crown-jewel motion approval hold)Certified contributor head: Verdict: Authoritative gate
The inherited 9 failures + 2 errors are not PR-owned. Prior RED — CLOSED ✅The old head copied At this head:
The independent reviewer probe observed:
The real-parser tests also preserve one parsed Crown-jewel visible approval holdThe automated stream gate is green, but it does not deliberately force the rare rewind/separate-owner adoption while recording the fade. Before release-lane action, the standing visual gate still requires final realistic evidence:
Do not send a sparse still as approval evidence; this defect class needs motion plus scroll metrics. Once that proof is captured and visually approved, this exact head is ready for the release agent's final fresh-head check. RecommendationEngineering GREEN. Keep the PR on visible-approval hold, not T1/ship-ready, until final motion evidence is approved. Preserve @ruizanthony's attribution. No code change is requested by this gate. Gate-certifier layer only. No merge, tag, deploy, close, or contributor-branch write was performed. This certificate is valid only for contributor head |
Re-gate GREEN — the parser-authority fix closed the clone regressionRe-gated the converged head (
Because this changes visible live-Markdown/fade behavior, it carries the standing visual sign-off requirement — captured live transparent-stream frames and routing them for UX + maintainer review before it lands. Thanks @ruizanthony — clean convergence on the fix-spec. |
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
0cc15af to
ab4d2bc
Compare
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
ab4d2bc to
30a0417
Compare
Transparent-stream progress rows are built incrementally by
`_anchorProseIncrementalNode`, which streams source text into the vendored
streaming-markdown parser. That parser always holds the most recently written
character in its pending buffer, so a live row's rendered `.msg-body` text
trails its source text by one character.
`_refreshTransparentFadeProseRow` resumed appending from the
`data-stream-fade-text` cursor but fell back to `body.textContent` when the
attribute was absent — exactly the case for incrementally built rows, which
never set it. That fallback mixed two coordinate spaces (rendered text vs
source text), so the computed delta started one character early, in the middle
of a word. The delta was then appended as a sibling of the parser's open `<p>`,
and since `<p>` is a block box the tail rendered on its own line: a single word
split across two lines ("Ces deu" / "x fichiers").
Two independent fixes:
1. Resume strictly from the source-space cursor. When no cursor exists we
cannot know how much source is already rendered, so rebuild the body
instead of guessing a delta from rendered text.
2. Append into the trailing block element rather than the `.msg-body` root, so
a continuation of the current sentence stays on the same line even if a
cursor is ever stale.
Regression tests drive the real vendored parser to build a live row exactly as
production does, assert the parser really does lag its source, and fail if any
word is torn across a block boundary. The existing reconcile harness gains the
new helper dependency.
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
… markdown rebuild Greptile P1 follow-up on the fade-row rebuild: when a cursorless live markdown row adopts the incremental node's cloned DOM, that DOM lags the source text by the streaming parser's held-back tail (pending/text buffers). Keeping the full source-length resume cursor made the next reconciliation append nothing, so the pending characters stayed missing from the live row until settlement. Read the held-back length from the parser bound on the candidate body (_smdBindParserIdentity) and trim the source-space cursor by exactly that length, so the cursor matches the adopted content and the next reconciliation re-appends the pending tail. Regression: rebuild while the parser holds an unrendered tail, then reconcile again — the pending text must appear in the live row before settlement.
Cursorless markdown rebuilds cloned the live parser tree, then later source growth appended raw bytes through _appendTransparentFadeText. Incomplete emphasis (Hello ** / Hello **world**) became literal **world** while the candidate already had <strong>world</strong>. Parser-owned candidates now keep reconciling from the parsed DOM. Pending and MEDIA tails stay on the parser. Rendered-prefix mute is unchanged.
After rewind/cache-miss the keyed visible row stayed distinct from the persistent parser candidate, so every later growth frame deep-cloned the parser DOM into the old row. That cut off the previous tail word's ~620ms fade and broke word-node identity. Promote the parser-owned candidate into the existing row's DOM position once, mute the already-rendered prefix on the real parser body, and return the adopted node so later keyed renders hit existing === node. Regression: two post-transition growth frames keep the first new tail span as the same .is-new node until an explicit animationend.
30a0417 to
69e4fa3
Compare
7d56234
|
Shipped in exp-v0.52.278 — thank you @ruizanthony! 🎉 Live progress text in Transparent Stream no longer tears mid-word across block boundaries ( Full gate on this crown-jewel streaming surface:
Credited via |
…w rebuild Review follow-ups for nesquena#7082: - Maintainer should-fix: the no-cursor rebuild branch of _refreshTransparentFadeProseRow cleared the body and re-wrapped every word as .is-new, so the whole visible row dipped to opacity 0 and faded back (~620ms), and wholesale node replacement invited the one-time scroll-anchor bounce (nesquena#6257). Now the rendered text is snapshotted before the clear and the messages.js _streamFadeMuteRenderedPrefix idiom is re-applied after the rebuild, so only genuinely-new tail words animate. The helper is exported on window (__streamFadeMuteRenderedPrefix) the same way __anchorProseIncrementalNode is. - Greptile P1: the same branch passed raw source text to _appendTransparentFadeText, flattening live markdown rows to literal syntax (links/emphasis/headings lost until settlement). The rebuild now adopts a deep clone of the candidate's parsed .msg-body (the incremental streaming-markdown output) when one exists, so the parsed DOM survives; the plain-text rebuild remains only as fallback for candidates without a parsed body. The resume cursor stays in source space when rendered text diverges from source (markdown), and shrinks to the rendered prefix for plain prose so the parser's held-back character is re-appended on the next delta instead of dropped. Regression tests: rebuild must not re-animate the already-rendered prefix (tail-only .is-new), and rebuild must keep parsed <a>/<strong> DOM with no literal markdown in the visible text. The two existing tests of this PR are unchanged and still pass.
Summary
Live progress rows in Transparent Stream can be torn in the middle of a word, with the tail rendered on its own line:
The streamed text itself is intact — this is purely a client-side render defect. The server sends clean deltas (
"\n\nCes"," deux fichiers passent…") and the persisted final message is correct.Root cause
Two changes landed on the same day and are individually correct, but interact badly:
smddata-stream-fade-textbody.textContentThe vendored streaming-markdown parser always holds the most recently written character in its pending buffer, so a live row's rendered text trails its source text by exactly one character:
_refreshTransparentFadeProseRow()computed its resume point as:For incrementally built rows the attribute is absent, so the fallback applies and the cursor is expressed in rendered-text space while
nextTextis in source-text space. The resulting deltanextText.slice(currentText.length)starts one character early — mid-word — and_appendTransparentFadeText()appended it to the root of.msg-body, i.e. as a sibling of the parser's open<p>. A<p>is a block box, so the tail starts a new line box.Observed DOM before this patch:
Fix
Two independent layers, so a single stale cursor can no longer tear a word:
body.textContentis no longer used as a cursor substitute. When no cursor exists we cannot know how much of the source is already rendered, so the body is rebuilt rather than guessing a delta._transparentFadeAppendTarget) instead of at the.msg-bodyroot, so a continuation of the current sentence stays on the same line.Behaviour that is deliberately preserved:
.stream-fade-wordspans, which fix(stream): preserve prose fade nodes to prevent scroll bounce #6257/fix(stream): preserve prose fade nodes to prevent scroll bounce #6264 rely on for scroll-anchor stability;prefers-reduced-motionplain-text path.Tests
New
tests/test_live_prose_fade_cursor_source_space.pydrives the real vendored parser to construct a live row exactly as production does, then runs the real reconciler over it:RED on
dc3bf44ewithout thestatic/ui.jschange:GREEN with it.
tests/test_issue5367_transparent_live_row_reconcile.pyextracts_appendTransparentFadeTextinto a Node harness, so it gains the new helper dependency (_transparentFadeAppendTargetplus its block-tag set). Without that it would throwReferenceError.Verification
python -m pytest tests/test_live_prose_fade_cursor_source_space.py \ tests/test_issue5367_transparent_live_row_reconcile.py -q # adjacent streaming / anchor-scene suites python -m pytest tests/test_anchor_fallback_ownership.py \ tests/test_anchor_scene_persistence.py \ tests/test_issue3397_transparent_stream_prose_segments.py \ tests/test_issue3820_chat_activity_display_mode.py \ tests/test_issue5700_worklog_event_timestamps.py \ tests/test_issue5720_reasoning_owner.py \ tests/test_issue5749_transparent_stream_prefix_dedupe.py \ tests/test_issue5854_anchor_scene_split.py \ tests/test_issue5966_transparent_stream_lazy_dom.py \ tests/test_smooth_text_fade.py tests/test_smd_media_in_stream.py \ tests/test_live_activity_timeline.py tests/test_live_anchor_progress_echo.py \ tests/test_live_anchor_stable_run_identity.py \ tests/test_live_to_final_anchor_visible_order.py \ tests/test_stable_assistant_turn_anchor_normalizer.py \ tests/test_stable_assistant_turn_anchor_phase0.py \ tests/test_stable_assistant_turn_anchor_registry.py \ tests/test_issue6220_id_linked_tool_anchor_hydration.py -q node --check static/ui.js python scripts/ruff_lint.py --diff origin/master415 tests pass, no regressions.
Notes
Touches only the transparent-stream live prose render path; no server, session, or persistence change. Independent of #6264 (scroll bounce), which does not modify the cursor or the append target — whichever lands second rebases trivially.