fix(transparent-stream): defer settled tool-detail DOM + cap per-turn rows (#5966) - #5974
nesquena-hermes wants to merge 6 commits into
Conversation
… rows (#5966) Reported by @curtisszmania-cytocync. Transparent Stream built a DOM node for every activity event of every settled turn at load, and eagerly materialized each tool row's full detail body — a 15+ prompt reasoning-heavy history produced thousands of subtrees and tipped the tab into a multi-GB freeze. The #5860/#5839 lazy-worklog fix did not cover Transparent Stream (it never collapses). Two-part surgical fix in static/ui.js, mirroring #5860's proven machinery: - Row-detail deferral: a settled+collapsed transparent tool row renders header-only; its heavy .tool-card-detail body builds on first expand (_materializeTransparentToolDetail), recovering the tool call from the scene after an innerHTML cache round-trip. Same anchor-suppressed post-processing as the eager path, so an expanded row is byte-identical. - Per-turn row cap: a settled turn with >CAP+SLACK (30+10) rows renders the last 30 behind a clean 'Show earlier steps (N)' affordance that materializes the prefix in place with viewport compensation. Just-settled turn (keep-open token) and already-revealed turns are exempt; true Trace tool-count is stashed so the label stays honest; Expand-all reveals capped rows; cache-restore re-wires the affordance. Live rendering is untouched. Server-side #5858 (scene serialize-after-messages) already covers the container RSS half on the experimental channel; this is the browser-DOM half.
…igned) Constrain the affordance to fit-content width + align-self:flex-start so it reads as a light inline control on the activity rail, not a full-width bar.
|
| Filename | Overview |
|---|---|
| static/ui.js | Adds row capping, lazy tool details, cache recovery, and persistent reveal state; revealed prefix rows still omit their scene-owner index. |
| static/style.css | Adds responsive and reduced-motion styling for the earlier-steps control. |
| static/i18n.js | Adds singular and plural labels for the earlier-steps control. |
| tests/test_issue5966_transparent_stream_lazy_dom.py | Covers cap behavior and recovery wiring but does not assert owner metadata on rows inserted by the reveal loop. |
Reviews (5): Last reviewed commit: "fix(review): extract _transparentToolRow..." | Re-trigger Greptile
Codex SHIP-ONLY-WITH-FIXES (3 SILENT defects), all fixed + browser-verified: - F1: buildToolCard PRE-BUILDS .tool-card-detail when a tool has args/output, so the old `!detail` defer guard skipped exactly the heavy rows. Defer now fires on settled+collapsed+has-detail and STRIPS any prebuilt body (verified: 400 shell tool rows -> 0 detail bodies mounted; expand still materializes byte-identically). - F2: stamp owner rawIdx on rows + affordance; dataset recovery and cache-restore rebind resolve the scene-owning message (multi-segment turns own it on a later segment, not the first) with a scene-owning-segment fallback. - F3: persist revealed-turn state in a session/owner-keyed Set + invalidate the session HTML cache on reveal, so a rebuild/switch-away never re-caps a turn the user expanded (verified: reveal 30->400, full re-render stays 400). Fable SHIP-UX fast-follows: t()-with-fallback label (+ en locale keys), 640px touch breakpoint, stale comment fixed. Tests: 21 pass (added has-detail deferral+strip, detail-less no-defer, owner-idx recovery, persistent-reveal, i18n).
Codex re-gate SHIP-ONLY-WITH-FIXES — 2 more SILENT defects, both fixed + browser-verified: - F1(r2): _materializeTransparentToolDetail rebuilt via the thinner _transparentToolDetailHtml(), dropping buildToolCard's richer detail (diff coloring / show-more / canonical shell-command lead). Now transplants buildToolCard(tc)'s own .tool-card-detail node so an expanded deferred row is byte-identical to the eager path (verified: canonical structure + 595-char body). - F2(r2): after an in-session renderMessages() rebuild (next send) an already-open settled tool row got re-deferred, and disclosure-restore only toggled .open → open-but-empty card. _setWorklogDetailDisclosureOpen now materializes a deferred transparent row BEFORE toggling open (verified: expand → rebuild → still open WITH content). Likely the same defect the RegressionGate caught intermittently. Tests: 22 pass (added canonical-detail + disclosure-restore-materialize).
The transparent-stream row-cap "Show N earlier steps" pill added two new i18n keys (show_earlier_steps, show_earlier_step_one) to the English block only, so the strict locale-parity tests (zh/ko/ru/zh-Hant/...) failed with missing-key assertions and CI shard 0 went red across 3.11/3.12/3.13. Add both keys to all 14 non-English locale blocks with the English string as fallback (the repo's established add-with-English-fallback convention for new UI keys). Inserted after each block's collapse_all anchor. All 197 locale tests + the #5966 suite green; node --check clean.
#5966 added a _transparentToolRowHasDetail() call into the settled transparent render (_anchorSceneTransparentNodeForRow) to decide whether to defer a collapsed tool row's detail body. The pre-existing #5700 timestamp test extracts that render fn into a node harness but didn't extract the new helper, so the extracted fn hit a ReferenceError → node exited non-zero → 3 #5700 tests failed (RED across CI shards; the PR updated its own #5966 harness but missed this one). Add `eval(extractFunc('_transparentToolRowHasDetail'))` to the harness. The helper self-guards its _toolActionKind/_toolCardAllowsDetail deps (returns true when absent), and the #5700 timestamp lives on the row HEADER (rendered even when detail is deferred), so all 20 #5700 assertions pass unchanged — confirming this was a harness gap, not a timestamp regression.
| const frag=document.createDocumentFragment(); | ||
| for(let idx=0;idx<stopIdx;idx+=1){ | ||
| const node=_anchorSceneTransparentNodeForRow(rows[idx],{settled:true,finalAnswer,liveTokenFinalPrefixEligible:idx>lastNonTerminalWorkRowIndex}); | ||
| if(node){ node.setAttribute('data-earlier-revealed','1'); frag.appendChild(node); } |
There was a problem hiding this comment.
Rows inserted by this reveal loop still lack data-anchor-owner-idx. After these rows are saved to the HTML cache and restored, their _deferredToolCall properties are gone. Expanding a revealed tool row in a multi-segment turn can then search the first scene-bearing segment instead of the owning segment, fail to find the row ID, and leave the tool detail empty. Stamp these rows with rawIdx, as the initial settled-render path does.
| if(node){ node.setAttribute('data-earlier-revealed','1'); frag.appendChild(node); } | |
| if(node){ | |
| node.setAttribute('data-earlier-revealed','1'); | |
| node.setAttribute('data-anchor-owner-idx',String(rawIdx)); | |
| frag.appendChild(node); | |
| } |
Context Used: AGENTS.md (source)
nesquena
left a comment
There was a problem hiding this comment.
Review — end-to-end ✅ (approved after two CI-red fixes I pushed)
Independent review of #5966 (@curtisszmania-cytocync): chats consuming 28GB+ and freezing the host with Transparent Stream enabled — the settled render built a DOM node for every activity event of every settled turn at load and eagerly materialized each tool row's full detail body, so a 15+ prompt history produced thousands of subtrees → multi-GB freeze. This is the Transparent-Stream counterpart to the compact-worklog fix #5860/#5839 (which explicitly didn't cover it).
What this ships
static/ui.js (row-detail deferral + per-turn cap), static/style.css, static/i18n.js, tests/test_issue5966_transparent_stream_lazy_dom.py. Agent-authored (nesquena-hermes). MERGEABLE, CI green (18/18 after my fixes).
What I caught — CI was red, two issues, both fixed
- Locale parity (
7d4f7fb8): the new "Show N earlier steps" pill addedshow_earlier_steps+show_earlier_step_oneto the English block only → the strict parity tests (zh/ko/ru/zh-Hant) failed missing-key across CI shards. Added both to all 14 non-English blocks (English fallback, the established convention) after eachcollapse_allanchor. 197 locale tests green. - #5700 harness
ReferenceError(197ca695): #5966 added a_transparentToolRowHasDetail(tc)call into_anchorSceneTransparentNodeForRow, but the pre-existingtest_issue5700_worklog_event_timestampsnode harness (which extracts that render fn) didn't extract the new helper → the extracted fn hit a ReferenceError → 3 #5700 tests failed (the PR updated its own #5966 harness but missed this one — same class as prior harness-gap CI-reds). Added theextractFuncline; confirmed it was a harness gap, not a timestamp regression — the #5700 timestamp lives on the row header (rendered even when detail is deferred), so all 20 assertions pass unchanged, and the helper self-guards its own deps.
The fix — traced correct, reuses proven #5860 machinery
A. Row-detail deferral: a settled, collapsed transparent tool row renders header-only; _materializeTransparentToolDetail(row) builds the .tool-card-detail on first expand — idempotent (clears the deferred flag), recovering the tool call from _deferredToolCall or, after an innerHTML cache round-trip, from data-anchor-row-id → S.messages (the #5839 recovery pattern). The detail transplants buildToolCard's own output with the same anchor-suppressed post-processing → an expanded row is byte-identical to the eager path. Live/already-open rows unchanged. ✓
B. Per-turn cap: _TRANSPARENT_SETTLED_ROW_CAP=30 + _SLACK=10 → a turn with >40 rows renders only the last 30 behind a "Show N earlier steps" pill that materializes the prefix with viewport compensation; exemptions for the just-settled turn (keep-open token) and any turn revealed this session; the true Trace tool-count is stashed so "Trace: N tools" stays honest; "Expand all" reveals capped rows first; cache-restore re-wires the affordance. ✓
No XSS: the only interpolating innerHTML (the pill label, line 375) is esc()'d; everything else is static tab/chevron markup or buildToolCard's (hardened) output. ✓
Edge-case matrix
| Scenario | Behavior |
|---|---|
| Settled collapsed tool row | header-only; detail built on first expand ✅ |
| Expand a deferred row | byte-identical to eager (buildToolCard transplant) ✅ |
After innerHTML cache restore |
recovers via data-anchor-row-id → S.messages (#5839) ✅ |
| Turn ≤40 rows | never capped ✅ (boundary-tested: 40 uncapped / 41 capped) |
| Turn >40 rows | last 30 + "Show N earlier" pill, position held ✅ |
| Just-settled turn / already-revealed | exempt from cap ✅ |
| Trace label while capped | honest "N tools" (stashed count) ✅ |
Tests
tests/test_issue5966_transparent_stream_lazy_dom.py— 22/22 (source-assertions for both parts + cache-recovery/count/expand-all/rehydrate wiring + a node-vm cap-arithmetic harness: 400→30 mounted, 12-row turn never capped, slack boundary 40/41). RED-on-master verified (21 fail on master).- #5700 timestamp suite — 20/20 after the harness fix; 197 locale tests green.
node --checkclean. Full suite: 12687 passed / 0 failed in 421s (deselected the known darwin CRLF flake). CI green (18/18).- The PR's real-browser measurement: 12,494 → 639 transcript DOM nodes (−94.9%) on a 400-tool turn, Trace label honest, position held on reveal.
Recommendation
✅ Approved after fixes. Parked at approval — ready for the release agent's merge/tag pipeline.
The load-bearing browser-DOM half of the Transparent-Stream freeze, faithfully reusing the #5860/#5839 deferral + #5839 cache-recovery machinery (byte-identical expanded rows, XSS-clean, honest Trace count, viewport-held reveal). The two CI-reds were a locale-parity miss and a pre-existing test-harness gap (not a behavioral regression — verified) — both fixed. Mutation-proven tests RED on master; full suite clean. Ship.
…cap (#5974, #5966) (#5981) * fix(transparent-stream): defer settled tool-detail DOM + cap per-turn rows (#5966) Reported by @curtisszmania-cytocync. Transparent Stream built a DOM node for every activity event of every settled turn at load, and eagerly materialized each tool row's full detail body — a 15+ prompt reasoning-heavy history produced thousands of subtrees and tipped the tab into a multi-GB freeze. The #5860/#5839 lazy-worklog fix did not cover Transparent Stream (it never collapses). Two-part surgical fix in static/ui.js, mirroring #5860's proven machinery: - Row-detail deferral: a settled+collapsed transparent tool row renders header-only; its heavy .tool-card-detail body builds on first expand (_materializeTransparentToolDetail), recovering the tool call from the scene after an innerHTML cache round-trip. Same anchor-suppressed post-processing as the eager path, so an expanded row is byte-identical. - Per-turn row cap: a settled turn with >CAP+SLACK (30+10) rows renders the last 30 behind a clean 'Show earlier steps (N)' affordance that materializes the prefix in place with viewport compensation. Just-settled turn (keep-open token) and already-revealed turns are exempt; true Trace tool-count is stashed so the label stays honest; Expand-all reveals capped rows; cache-restore re-wires the affordance. Live rendering is untouched. Server-side #5858 (scene serialize-after-messages) already covers the container RSS half on the experimental channel; this is the browser-DOM half. * style(#5966): compact 'Show earlier steps' pill (hug content, left-aligned) Constrain the affordance to fit-content width + align-self:flex-start so it reads as a light inline control on the activity rail, not a full-width bar. * fix(#5966): address Codex gate findings + Fable UX fast-follows Codex SHIP-ONLY-WITH-FIXES (3 SILENT defects), all fixed + browser-verified: - F1: buildToolCard PRE-BUILDS .tool-card-detail when a tool has args/output, so the old `!detail` defer guard skipped exactly the heavy rows. Defer now fires on settled+collapsed+has-detail and STRIPS any prebuilt body (verified: 400 shell tool rows -> 0 detail bodies mounted; expand still materializes byte-identically). - F2: stamp owner rawIdx on rows + affordance; dataset recovery and cache-restore rebind resolve the scene-owning message (multi-segment turns own it on a later segment, not the first) with a scene-owning-segment fallback. - F3: persist revealed-turn state in a session/owner-keyed Set + invalidate the session HTML cache on reveal, so a rebuild/switch-away never re-caps a turn the user expanded (verified: reveal 30->400, full re-render stays 400). Fable SHIP-UX fast-follows: t()-with-fallback label (+ en locale keys), 640px touch breakpoint, stale comment fixed. Tests: 21 pass (added has-detail deferral+strip, detail-less no-defer, owner-idx recovery, persistent-reveal, i18n). * fix(#5966): address Codex re-gate findings (r2) Codex re-gate SHIP-ONLY-WITH-FIXES — 2 more SILENT defects, both fixed + browser-verified: - F1(r2): _materializeTransparentToolDetail rebuilt via the thinner _transparentToolDetailHtml(), dropping buildToolCard's richer detail (diff coloring / show-more / canonical shell-command lead). Now transplants buildToolCard(tc)'s own .tool-card-detail node so an expanded deferred row is byte-identical to the eager path (verified: canonical structure + 595-char body). - F2(r2): after an in-session renderMessages() rebuild (next send) an already-open settled tool row got re-deferred, and disclosure-restore only toggled .open → open-but-empty card. _setWorklogDetailDisclosureOpen now materializes a deferred transparent row BEFORE toggling open (verified: expand → rebuild → still open WITH content). Likely the same defect the RegressionGate caught intermittently. Tests: 22 pass (added canonical-detail + disclosure-restore-materialize). * fix(review): add show_earlier_steps i18n keys to all locales (#5966) The transparent-stream row-cap "Show N earlier steps" pill added two new i18n keys (show_earlier_steps, show_earlier_step_one) to the English block only, so the strict locale-parity tests (zh/ko/ru/zh-Hant/...) failed with missing-key assertions and CI shard 0 went red across 3.11/3.12/3.13. Add both keys to all 14 non-English locale blocks with the English string as fallback (the repo's established add-with-English-fallback convention for new UI keys). Inserted after each block's collapse_all anchor. All 197 locale tests + the #5966 suite green; node --check clean. * fix(review): extract _transparentToolRowHasDetail into the #5700 harness #5966 added a _transparentToolRowHasDetail() call into the settled transparent render (_anchorSceneTransparentNodeForRow) to decide whether to defer a collapsed tool row's detail body. The pre-existing #5700 timestamp test extracts that render fn into a node harness but didn't extract the new helper, so the extracted fn hit a ReferenceError → node exited non-zero → 3 #5700 tests failed (RED across CI shards; the PR updated its own #5966 harness but missed this one). Add `eval(extractFunc('_transparentToolRowHasDetail'))` to the harness. The helper self-guards its _toolActionKind/_toolCardAllowsDetail deps (returns true when absent), and the #5700 timestamp lives on the row HEADER (rendered even when detail is deferred), so all 20 #5700 assertions pass unchanged — confirming this was a harness gap, not a timestamp regression. * fix(#5974): hide Show-earlier-steps pill when a capped transparent turn is collapsed (Fable UX gate) Fable UX gate SHIP-WITH-UX-FIXES: the .transparent-earlier-steps pill wasn't in the .assistant-turn[data-transparent-turn-collapsed=1] hide rule, so collapsing a capped turn via the name-tag toggle left an orphan pill. Added it to the hide rule. (i18n finding was already resolved — both pill keys present in all 15 locales w/ English fallback + show_earlier_step_one is live at ui.js:12918, not dead.) * Release exp-v0.52.50: transparent-stream lazy settled DOM + per-turn cap (#5974, #5966) --------- Co-authored-by: nesquena-hermes <agent@nesquena-hermes> Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
|
Shipped in exp-v0.52.50 (closes #5966). Transparent Stream now defers settled tool-detail DOM + caps per-turn rows — the 28GB freeze is fixed. Full gate: nesquena independent review APPROVED (−94.9% DOM), Codex SAFE, stream-regression-gate GREEN all modes, Fable UX/arch SHIP (orphan-pill-on-collapse fix applied), CI 18/18. Thanks @curtisszmania-cytocync for the detailed report. |
Closes #5966.
Reported by @curtisszmania-cytocync: chats consuming 28GB+ and freezing the host, "typically when a chat has 15+ user prompts," with Transparent Stream enabled, worst "before the chat fully loads."
Root cause
Transparent Stream never collapses its activity, so the settled render built a DOM node for every activity event of every settled turn at load, and eagerly materialized each tool row's full detail body (tool input/output HTML + Prism/KaTeX/Mermaid post-processing). A 15+ prompt reasoning-heavy history produced thousands of subtrees synchronously → the multi-GB tab freeze. The message virtualizer's unit is messages, not activity rows, and a normal cold load (~30 messages) sits below its 80-row threshold and never engages — so nothing bounded the per-turn row fan-out.
The compact-worklog freeze fix (#5860/#5839) deferred collapsed-worklog DOM, but explicitly did not cover Transparent Stream. This PR is the Transparent-Stream counterpart, reusing the same proven deferral machinery.
Fix (two parts,
static/ui.js+static/style.css)A. Row-detail deferral — a settled, collapsed transparent tool row renders header-only (name, summary preview, status, chevron); its heavy
.tool-card-detailbody is built on first expand (_materializeTransparentToolDetail). The tool call is recovered from the in-memory stash, or fromdata-anchor-row-id → S.messagesscene after an innerHTML cache round-trip (#5839 recovery pattern). Same anchor-suppressed post-processing as the eager path, so an expanded row is byte-identical. Live and already-open rows are unchanged.B. Per-turn row cap — a settled turn with
> CAP+SLACK(30+10) rows renders only the last 30, behind a clean "Show earlier steps (N)" pill (styled on the existing load-earlier idiom) that materializes the omitted prefix in place with viewport compensation (reader's position held). Exemptions: the just-settled turn (keep-open token — no STREAM_DONE shrink) and any turn already revealed this session. The true Trace tool-count is stashed so "Trace: N tools" stays honest while capped; "Expand all" reveals capped rows first; cache-restore re-wires the affordance.Server-side #5858 (scene serialize-after-messages) already covers the container-RSS half on the experimental channel; this is the load-bearing browser-DOM half for a Transparent-Stream user.
Evidence
Real-browser measurement (400-tool settled turn, Transparent Stream), master vs this branch:
Interactions verified live (0 console/JS errors): "Show 370 earlier steps" reveals all 400 with the previously-visible row held at the exact same screen position (Δ 0px); expanding a deferred row builds only that row's detail; even fully revealed, unopened rows keep their detail deferred.
Crown-jewel streaming RegressionGate: transparent_stream and compact_worklog both CLEAN across the full lifecycle (stream / after-done / switch-away-back / reload-after-done / reload-mid-stream), tool-expand shows input+output, no flicker, no alternating.
Tests
tests/test_issue5966_transparent_stream_lazy_dom.py— 15 tests: source-assertions for both parts + the cache-recovery/count/expand-all/rehydrate wiring, plus a behavioral node-vm harness proving the cap arithmetic bounds mounted rows (400→30) while a normal 12-row turn is never capped (non-vacuity) and the slack boundary (40 uncapped / 41 capped) holds.