Skip to content

fix(#5367): preserve transparent stream live rows on rerender - #5400

Closed
rodboev wants to merge 4 commits into
nesquena:masterfrom
rodboev:pr/5367-transparent-live-row-reconcile
Closed

rodboev wants to merge 4 commits into
nesquena:masterfrom
rodboev:pr/5367-transparent-live-row-reconcile

Conversation

@rodboev

@rodboev rodboev commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Thinking Path

  • Transparent Stream already projects live activity rows with stable identity, but the renderer discarded that identity by removing every live-owned transparent row on each streamed update.
  • The visible flicker came from that teardown: recreated .transparent-event-row nodes replayed their entrance animation even when they represented the same row that was already on screen.
  • The fix keeps that row-identity reconcile in the browser renderer, then rehydrates the preserved row's interactive state after the body refresh so the stable row keeps working copy and disclosure controls while new rows still get the existing entrance animation.

What Changed

  • static/ui.js: keep the Transparent Stream live-row reconcile path in _renderLiveAnchorActivitySceneTransparent(...), drop the dead inline helper fallbacks, and update _refreshTransparentLiveRow(...) so a preserved row carries the candidate _tcData, rebinds the header and copy controls after the innerHTML swap, and restores the preserved card/detail state.
  • tests/test_issue5367_transparent_live_row_reconcile.py: extend the browserless DOM harness with a same-key rerender regression that proves the preserved row keeps working copy/header handlers and copies the refreshed _tcData payload after reconcile.

Why It Matters

Transparent Stream should stay visually stable while the model is generating, and the preserved row still needs to behave like the fresh render it replaced. This keeps the no-blink reconcile fix without silently breaking copy or disclosure behavior mid-stream.

Verification

pytest tests/test_issue5367_transparent_live_row_reconcile.py tests/test_live_to_final_anchor_visible_order.py tests/test_stable_assistant_turn_anchor_registry.py -v --timeout=60
npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"

Full-suite CI context, not a required local check unless requested: pytest tests/ -v --timeout=60.

Upstream

Closes #5367.

Fix shape follows the maintainer root-cause analysis and accepted fix directions at #5367 (comment).

Model Used

GPT 5.5 via Codex CLI

@greptile-apps

greptile-apps Bot commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a flicker regression (#5367) in the Transparent Stream renderer by replacing the tear-down-and-rebuild strategy with an in-place DOM reconciliation pass. On each streamed update, same-key live rows are now preserved and patched in place while stale rows are pruned and new rows are inserted in scene order, just before the live status footer.

  • static/ui.js: Adds _transparentLiveRowKey, _transparentLiveRowsCompatible, _transparentLiveRowAttributePairs, _transparentLiveRowInteractiveState, _rehydrateTransparentLiveRow, and _refreshTransparentLiveRow helpers, and rewires _renderLiveAnchorActivitySceneTransparent to use a two-pass reconcile (collect → clean → patch/insert → prune stale).
  • tests/test_issue5367_transparent_live_row_reconcile.py: New browserless DOM-harness test that exercises same-key preservation, stale-row removal, keyless-row rebuilding, and interactive-state rehydration across multiple consecutive renders.
  • tests/test_live_to_final_anchor_visible_order.py: Updated to eval the new helpers so the existing ordering tests remain valid against the changed renderer.

Confidence Score: 5/5

The change is safe to merge — it narrows a previously destructive teardown to a targeted reconcile with no risk of data loss or incorrect render state.

The reconciliation logic is tight: data-live-stream-owned co-occurs with data-anchor-scene-row (confirmed at line 10534/10542), so removing the former from the legacy cleanup selector is safe. The key function encodes stream, row-id, role, and source, making compatible rows unambiguous. Interactive state (card-open, detail-mode, _tcData, copy-button handler) is correctly snapshotted before innerHTML replacement and rehydrated after. The new test suite covers same-key reuse, stale removal, keyless row rebuilding, and full rehydration in a Node harness that exercises the real production code.

No files require special attention.

Important Files Changed

Filename Overview
static/ui.js Adds six helper functions and rewrites the transparent live row rendering loop to reconcile by key identity instead of unconditionally removing and recreating all rows; all attribute sync, interactive-state preservation, and stale-node cleanup paths are correctly handled.
tests/test_issue5367_transparent_live_row_reconcile.py New 726-line browserless regression covering same-key row reuse, stale row removal, keyless row rebuilding, and copy-button/tcData/card-open rehydration; correctly skipped when Node is not on PATH.
tests/test_live_to_final_anchor_visible_order.py Adds eval calls for the six new helper functions so the existing visible-order tests run correctly against the updated renderer; indentation change is harmless in a JS string context.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Stream as Streamed Update
    participant Renderer as _renderLiveAnchorActivitySceneTransparent
    participant DOM as Live DOM
    participant Map as preserveByKey Map

    Stream->>Renderer: new scene (activity_rows)
    Renderer->>DOM: query .transparent-event-row[data-live-stream-owned]
    DOM-->>Map: store matched rows by key (streamId+rowId+role+source)
    Renderer->>DOM: remove [data-anchor-scene-owner] elements
    Renderer->>DOM: remove [data-anchor-scene-row] not in preserveByKey

    loop for each row in new scene
        Renderer->>DOM: "_anchorSceneTransparentNodeForRow -> node"
        Renderer->>Map: get existing by key(node)
        alt existing found and compatible
            Renderer->>DOM: _refreshTransparentLiveRow - sync attrs + innerHTML
            Renderer->>DOM: _rehydrateTransparentLiveRow - rewire handlers and state
            Renderer->>Map: delete key from map
            Renderer->>DOM: insertBefore(existing, liveFooter)
        else no existing match
            Renderer->>DOM: insertBefore(node, liveFooter)
        end
    end

    Renderer->>Map: forEach remaining stale node
    Map->>DOM: stale.remove()
Loading
%%{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 Stream as Streamed Update
    participant Renderer as _renderLiveAnchorActivitySceneTransparent
    participant DOM as Live DOM
    participant Map as preserveByKey Map

    Stream->>Renderer: new scene (activity_rows)
    Renderer->>DOM: query .transparent-event-row[data-live-stream-owned]
    DOM-->>Map: store matched rows by key (streamId+rowId+role+source)
    Renderer->>DOM: remove [data-anchor-scene-owner] elements
    Renderer->>DOM: remove [data-anchor-scene-row] not in preserveByKey

    loop for each row in new scene
        Renderer->>DOM: "_anchorSceneTransparentNodeForRow -> node"
        Renderer->>Map: get existing by key(node)
        alt existing found and compatible
            Renderer->>DOM: _refreshTransparentLiveRow - sync attrs + innerHTML
            Renderer->>DOM: _rehydrateTransparentLiveRow - rewire handlers and state
            Renderer->>Map: delete key from map
            Renderer->>DOM: insertBefore(existing, liveFooter)
        else no existing match
            Renderer->>DOM: insertBefore(node, liveFooter)
        end
    end

    Renderer->>Map: forEach remaining stale node
    Map->>DOM: stale.remove()
Loading

Reviews (3): Last reviewed commit: "fix(#5367): rehydrate preserved transpar..." | Re-trigger Greptile

Comment thread static/ui.js
Comment thread static/ui.js
Comment thread static/ui.js Outdated
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Read the full static/ui.js diff against origin/master plus the new test harness, and cross-checked against the competing CSS-only PR #5406 (both close #5367).

Summary

This is the maintainer's fix direction #2 ("reconcile instead of teardown") — the durable one. Instead of removing every [data-live-stream-owned="1"] row on each streamed event, it snapshots existing rows into a preserveByKey map keyed by a stable composite (streamId \u0000 rowId \u0000 role \u0000 sourceEventType), skips those during the cleanup sweep, updates matching rows in place via _refreshTransparentLiveRow, and only removes genuinely stale rows at the end. Because the outer .transparent-event-row element survives across renders, transparent-event-enter never replays on a stable row, so the blink stops without touching CSS. That preserves the entrance animation for genuinely new rows — the property #5406 gives up.

Code reference

The core swap at static/ui.js (was: unconditional [data-live-stream-owned="1"] removal at master ui.js:10837):

const key = keyTransparentLiveRow(node, activeStreamId);
const existing = key ? preserveByKey.get(key) : null;
const renderedNode = existing && transparentLiveRowsCompatible(existing, node)
  ? refreshTransparentLiveRow(existing, node)
  : node;
if(existing) preserveByKey.delete(key);

The key includes data-anchor-source-event-type, which is correct: the builder stamps all four fields at ui.js:10559-10562, so a tool row that changes source_event_type won't be wrongly reused.

Two things worth a look

1. innerHTML copy drops the interactive DOM identity of preserved rows. _refreshTransparentLiveRow does existing.innerHTML = node.innerHTML. For tool/thinking rows the header toggle is wired via .onclick handlers (_wireTransparentHeaderToggle, ui.js:9583) attached to the .tool-card-header element. Overwriting innerHTML replaces those header nodes with fresh, unwired ones. In the settled re-render path this is re-bound by _rehydrateTransparentStreamDom (ui.js:9685), but the live transparent loop only calls _syncTransparentEventControls at ui.js:10861 afterward — I don't see it re-running _wireTransparentHeaderToggle over the rebuilt row bodies. Worth confirming a tool card's expand/collapse still works mid-stream after a reconcile (click the header while the model is still generating). If it doesn't, the fix is to re-decorate/re-wire the refreshed row rather than reuse it wholesale, or to update in place without blowing away innerHTML.

2. The inline-fallback duplication is dead weight. Each helper is defined twice — once as a top-level function _transparentLiveRowKey(...) etc., and once as an inline typeof _x === 'function' ? _x : (…) fallback inside the render function. Since all four are top-level declarations in the same file, the typeof guard is always true and the fallbacks never run (the test evals the top-level versions, confirming that path). It's ~50 lines of unreachable code that will drift from the real helpers over time. I'd drop the fallbacks and call the top-level functions directly. Greptile flagged the same dead-branch concern on preserveByKey.delete for incompatible rows.

Test plan

The Node DOM harness (tests/test_issue5367_transparent_live_row_reconcile.py) evals the real top-level helpers and asserts same-key preservation, stale removal, and footer-ordered insertion — good coverage of the reconcile logic. It doesn't exercise header re-wiring though, which is the gap in item #1. A manual check (expand a tool card mid-stream, then let another event arrive, then click it again) would close that.

Net: structurally this is the better long-term fix vs #5406, assuming the innerHTML re-wire question checks out. Flagging that both PRs target the same issue so they aren't both merged.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

🔬 Gate certification — RED ⛔ (the key-reconcile is well-built, but in-place innerHTML breaks the copy button + tool-data on rerender)

Certified head: sha:8cb66643 (rebased onto current master, git apply clean) · PR: #5400 · rodboev, fix(#5367): preserve transparent stream live rows on rerender
Verdict: The key-based reconciliation is the right approach and correctly built (key-collision first-wins, orphan cleanup, no DOM leak). But the full gate found a real SILENT regression it introduces: _refreshTransparentLiveRow updates the row via innerHTML, which drops the copy button's property-bound handlers and the row's _tcData tool payload — so copy stops working (or copies wrong data) after a reconciled rerender. Rebind + carry the expando, then ship.

What I ran (rebased worktree /tmp/wt-rebase-5400)

Gate Result
Rebase onto current master ✅ git apply clean; node -c OK
Codex (reproduce) SHIP-WITH-FIXES — 1 SILENT (copy/tool-data break); I confirmed by inspection
Full pytest suite 1 failed / 11771 passed — the 1 failure is the pre-existing nous-picker env flake (unrelated)
PR's own test ✅ 1/1 (but it doesn't exercise copy-after-rerender — which is why it's green while the defect exists)
Reconciliation review (mine) key-collision + orphan cleanup correct (below); innerHTML handler-drop confirmed (below)

Findings

✅ The key-reconciliation core is well-built: key = data-anchor-row-id+role+source-event-type; preserveByKey Map with first-wins on collision (if(key && !preserveByKey.has(key))); matched nodes updated in place + deleted from the map (consumed); leftover map entries removed (preserveByKey.forEach(stale=>stale.remove())) + scene-owner/el.remove() cleanup → no DOM leak, no dup, no dropped live row. This correctly fixes the #5367 destroy-and-recreate drop.

⛔ SILENT (I CONFIRMED) — copy button + tool-data break on reconciled rerender (static/ui.js:11015): _refreshTransparentLiveRow does existing.innerHTML = node.innerHTML. But _attachCopyButton binds the copy control via DOM properties — btn.onclick=handler; btn.onkeydown=... (ui.js:9532-9535), NOT addEventListener/delegation. innerHTML assignment replaces the button DOM with freshly-parsed markup that carries no onclick/onkeydown properties → the .transparent-event-copy button silently stops working after a reconcile. Additionally the row's _tcData expando (tool payload, ui.js:14681) isn't carried from the candidate node → copied payload can be stale/wrong even if rebound. So the reconcile preserves the row but guts the interactive controls inside it. The PR's test doesn't catch this (it verifies rows survive, not that copy still works post-rerender).

  • Fix (Codex-exact): after refreshing the row HTML, copy/delete existing._tcData from the candidate node, and rebind the refreshed row header with _wireTransparentHeaderToggle() + _attachCopyButton() while restoring expanded/detail state. Add a test that reconciles a row then asserts the copy button still fires + copies the right _tcData.

Recommendation to the next agent

RED — gate-fail/changes-requested (1 SILENT fix): rebind the copy button + header toggle and carry _tcData after the in-place innerHTML refresh. The reconciliation architecture is correct (key-collision, orphan cleanup, no leak) — this is the classic "innerHTML drops JS-property-bound handlers + expandos" trap on the in-place update path. Add a copy-after-rerender regression test. Converges fast. concept 4/5 (#5367 real streaming-render fix). Author @rodboev (T1). crit=3. (Good catch: the PR's green test verifies row survival but not that the row's interactive controls survive — the exact blind spot of an innerHTML-based in-place update.)


_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; reconcile core verified sound (key-collision/orphan/no-leak), innerHTML handler+tcData drop confirmed by inspecting _attachCopyButton (onclick/onkeydown properties, not addEventListener). Cert valid for sha:8cb66643.

@nesquena-hermes nesquena-hermes added gate-fail Gate found blocking issue(s); fix-spec in comment; awaiting fix/re-push changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC) labels Jul 2, 2026
@rodboev

rodboev commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Updated in 743e45ee. static/ui.js:10852-10867 keeps the reconcile path in place, and static/ui.js:10915-10968 now snapshots the preserved row state, carries the candidate _tcData, re-runs _wireTransparentHeaderToggle(...) plus _attachCopyButton(...) after the innerHTML swap, and restores the preserved card/detail state instead of leaving the refreshed subtree unwired. I also dropped the dead inline helper fallbacks from the live renderer, so it now calls the shared top-level helpers directly.

I extended tests/test_issue5367_transparent_live_row_reconcile.py:365-722 to rerender a same-key tool row and assert the preserved node keeps working copy and header handlers, carries the refreshed _tcData, and copies the new payload instead of the stale one.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

🔬 Gate certification — GREEN ✅ · CONVERGED (bounce closed)

Certified head: sha:4e74de45 (clean rebase, branch gate-rebase/5400-transparent-row-reconcile) · PR: #5400 · rodboev, fix(#5367): preserve transparent stream live rows on rerender
Verdict: Bounce closed — my prior CORE (the in-place innerHTML refresh dropped the copy button's property-bound handlers + the row's _tcData) is fixed by a new _rehydrateTransparentLiveRow that carries _tcData, rebinds the copy/toggle handlers, and restores expanded state after the refresh. Codex SAFE, suite green, and the exact copy-after-rerender regression test is now present.

What I ran (rebased worktree /tmp/wt-rebase-5400b)

Gate Result
Rebase onto current master ✅ git apply clean
Codex (reproduce) SAFE TO SHIP — gated the rebased worktree, 0 findings
Full pytest suite 1 failed / 11772 passed — the 1 failure is the pre-existing nous env flake (unrelated)
PR own tests ✅ 61/61 (test_issue5367_transparent_live_row_reconcile + test_live_to_final_anchor_visible_order)

Findings

✅ CONVERGED — my copy-button/tool-data CORE is fixed: after existing.innerHTML = node.innerHTML, the new _rehydrateTransparentLiveRow(existing, node, preservedState) runs and (1) carries _tcData — existing._tcData = node._tcData (or delete if the new node has none, so no stale payload), (2) rebinds the header controls — _wireTransparentHeaderToggle(header) + _attachCopyButton(header) (re-attaching the onclick/onkeydown property handlers that innerHTML stripped), and (3) restores expanded/detail state — preservedState.expanded (captured from card.classList.contains('open')/data-expanded before the refresh) re-applied via _setTransparentCardOpen. So a reconciled rerender now preserves the row AND its interactive controls + tool payload + expansion. The key-reconcile core (collision first-wins, orphan cleanup, no DOM leak) remains intact from my prior review.

  • Regression coverage added: test_transparent_live_scene_rehydrates_copy_button_and_tcdata_after_reconcile drives a reconcile then asserts the copy button is rebound + _tcData carried to the kept row — exactly the blind spot the original survival-only test missed. 61/61 pass. Codex SAFE (0 findings).

Recommendation to the next agent

Ready to merge — use branch gate-rebase/5400-transparent-row-reconcile (sha:4e74de45), NOT the PR's stale head 743e45ee. The copy-button/tool-data/expanded-state rehydration closes my CORE, with the exact copy-after-rerender regression test; key-reconcile core intact; Codex SAFE + 61 tests + suite green. Crown-jewel streaming-render fix (#5367 live rows no longer dropped on rerender). Visible = streaming render (behavior test-covered). concept 4/5. Credit @rodboev (co-authored). crit=3.


Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; copy-button + _tcData + expanded-state rehydration verified (reading _rehydrateTransparentLiveRow + the new regression test), Codex SAFE + 61 tests + suite green bar the nous flake. Cert valid for sha:4e74de45.

@nesquena-hermes nesquena-hermes added gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent and removed changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address gate-fail Gate found blocking issue(s); fix-spec in comment; awaiting fix/re-push labels Jul 2, 2026
nesquena-hermes added a commit that referenced this pull request Jul 2, 2026
Release — Transparent Stream flicker/row-drop fix (#5400, closes #5367)
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.827 🎬 — thanks @rodboev!

Merged via release #5433, with Nathan's sign-off (motion fix — proof is the Codex repro + the 61-test regression file, since a still can't show absence-of-flicker on the live streaming surface).

Full gate (rebased onto current master v0.51.826):

  • Codex adversarial (crown-jewel brief: row-drop, dead copy button, key-collision, non-transparent/virtualization regression, settled/replay): SAFE TO SHIP, 0 findings.
  • Full pytest suite green (11794 passed). PR-own tests 61/61. Browser-smoke + ESLint runtime + node -c: CLEAN.
  • Mechanism code-verified: _refreshTransparentLiveRow captures expanded/detail state before the innerHTML swap; _rehydrateTransparentLiveRow carries _tcData, rebinds the copy + header-toggle handlers innerHTML strips, and restores expansion — closing the earlier gate bounce (dropped copy handlers) with an exact copy-after-rerender regression test.

Live rows are now reconciled by identity across rerenders (matching rows refreshed in place, stale rows removed, only new rows animate), so Transparent Stream no longer flickers or drops rows mid-response.

Credit preserved via Co-authored-by. Closes #5367.

pull Bot pushed a commit to jw5812018/hermes-webui that referenced this pull request Jul 2, 2026
… (rehydrate controls)

Clean rebase of rodboev's nesquena#5400 (rebase-first).

Co-authored-by: rodboev <rodboev@users.noreply.github.com>
pull Bot pushed a commit to jw5812018/hermes-webui that referenced this pull request Jul 2, 2026
pull Bot pushed a commit to jw5812018/hermes-webui that referenced this pull request Jul 2, 2026
ruizanthony pushed a commit to ruizanthony/hermes-webui that referenced this pull request Jul 3, 2026
… row entrance animation (residual nesquena#5367)

Removes the #liveAssistantTurn .transparent-event-row entrance animation
rule and its now-unused @Keyframes transparent-event-enter, eliminating the
entrance-animation replay on streaming rebuilds that remained after the
nesquena#5400 identity-reconcile fix. Depth-fade [data-transparent-fade] rules
preserved (avoids the nesquena#5406 opacity-clobber). Follow-up to already-closed nesquena#5367.

Co-authored-by: Rod Boev <rodboev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Model Response Flickering when using Activity Display “Transparent Stream”

2 participants