perf(desktop): bound transcript rendering and eliminate scroll LoAF - #4206
perf(desktop): bound transcript rendering and eliminate scroll LoAF#4206Astro-Han wants to merge 12 commits into
Conversation
Make Runtime Host own the bounded Turn range, keep immutable stable Renderer projections, and delegate offscreen rendering to Chromium content visibility. Delete the Renderer row virtualizer, height index, spacers, and their compensation state. Generated-by: Codex
Record reproducible warm-scroll, memory, DOM, session-switch, and repeated-paging evidence without participating in product state. Generated-by: Codex
Project the existing onboarding composition state directly onto ChatLayout so prompt-rail aria-current updates no longer invalidate the full transcript through ancestor :has selectors. Generated-by: Codex
Poll both sides of the prompt landing tolerance in one sample so faster transcript styling cannot expose an intermediate smooth-scroll position as a failure. Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
zhiiw
left a comment
There was a problem hiding this comment.
Reviewed at exact head d866253a (verified unchanged at review time).
Not approving: the PR's own evidence test is red on the hosted runner. See the inline [P1].
What I verified before hitting that gate:
- Eviction is now turn-grouped with a protected anchor:
desktop-transcript-replica.tsgroups resident messages by turn (turnless messages each get their ownsequence:Ngroup, so control traffic cannot starve the turn budget), never evicts the protected turn, and evicts from the edge farther from it. Fail-safe direction: when nothing evictable remains, it stops rather than corrupting. - The renderer snapshot is genuinely reference-stable: the range store keeps incremental order arrays, caches the frozen snapshot per committed batch, and compares durable records by their stored encoded form instead of re-stringifying.
#refreshSequenceBoundsis O(1) off the order array. - The
:has()removal is a real LoAF fix: projectingdata-maka-onboardingfrom AppShell (which already ownsshowOnboardingHero) onto the layout element removes the descendant-:has()invalidation of the whole chat subtree on every prompt-railaria-currentchange. content-visibility: autouses the right containment pattern:contain-intrinsic-block-size: auto 280pxkeeps the last-rendered size, so scroll height does not collapse for already-rendered turns.- Scroll authority is preserved:
useChatScrollremains the only writer; the new nested-scroller wheel guard walkscomposedPathand declines when an inner scroller can still consume the gesture — the right shape, and the 1px anchoring workaround is retained.
Executed on a real Windows machine at this head: clean rebuild (including the install-time model-metadata generation this head requires), Desktop main build, then the range-store / streaming-handoff / prompt-rail suites 54/54.
Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.
简体中文
不批:PR 自己的证据测试在 hosted runner 上红(详见行内 P1)。闸前已核实:逐出改为按 Turn 分组并保护锚点 Turn(无 turnId 的消息各自成组,控制流量不会饿死 Turn 预算),方向 fail-safe;renderer 快照真引用稳定(增量有序数组 + 按提交批次缓存冻结快照 + 用存好的编码串比对);:has() 移除是真 LoAF 修复(AppShell 直接投影 onboarding 布尔);content-visibility 用了正确的 auto + 上次渲染尺寸模式;滚动单写者未变,嵌套滚轮守卫形状正确。本机真 Windows 干净重建 + 三套件 54/54。
| expect(await scrollButtonOffered(page)).toBe(false); | ||
| }); | ||
|
|
||
| test('a nested scroller near the history boundary does not request an earlier range', async ({ |
There was a problem hiding this comment.
[P1] This test fails on the hosted CI runner at this head (test job, Desktop e2e step, run 33270284081): locator.hover on [data-nested-history-scroller="true"] times out after 30 s, so the element never becomes actionable there. The PR's own verification reports the Desktop E2E green locally (macOS), so the behavior differs between the author's machine and the hosted Linux/Electron environment — until that gap is reconciled, the PR's evidence does not hold on the merge gate.
One plausible mechanism worth checking first: the injected box is position: fixed inside a [data-turn-id] subtree that now carries content-visibility: auto — a fixed descendant of a skipped (offscreen) subtree is not rendered, and the anchor turn chosen here sits at the very top edge of the scrollport. Whether it is that or something else, this needs a head where the hosted runner is green.
Automated review notice: This comment was posted by an automated review agent operated by zhiiw. It is not an independent human review and does not replace one.
Generated-by: Codex
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed at exact head 1b798e98d9d48d43784fff7e7d2d7ab1894bf687.
Requesting changes for one P1 inline: the new protected-Turn eviction guarantee does not complete a Turn across transcript page boundaries, so reopening a session can still render only the tail of its latest Turn.
Validation performed:
npm ci, full test build, Desktop renderer build, and Desktop typecheck passed.- Transcript range/streaming unit tests passed (45/45).
- The affected Desktop E2E set passed 21 tests with 3 opt-in perf/stress tests skipped; one scroll timing case failed once and passed on isolated rerun. The hosted
testcheck is green on this exact head. - A synthetic merge with current
main(827b3fdf) built, typechecked, passed the same 45 unit tests, and passed focused prompt-range/nested-scroll E2E tests (2/2).
Not independently rerun: the opt-in same-build CDP performance and 600+ Turn stress scenarios.
Codex-assisted review performed under the maintainer-approved review workflow.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| replica.#evictToBudget( | ||
| undefined, | ||
| 'oldest', | ||
| replica.#newestTurnSequence() ?? replica.#durableThrough ?? undefined, |
There was a problem hiding this comment.
[P1] Complete the protected Turn across transcript pages
protectedSequence can only protect records already returned by this decoded page. Desktop opens with a 16 KiB transcript bootstrap, and decodeTranscriptPage() follows nextCursor only far enough to finish the currently fragmented message; it then returns the cursor for older records. Reproducing with a latest Turn containing a prompt plus a 20 KiB assistant message leaves only the assistant in the replica (hasOlder: true), so reopening the session renders an incomplete latest Turn. The new oversized-Turn test does not exercise this because its fake decoder returns both records at once despite the real page limit.
Please make initial/range loading continue until the protected Turn boundary is complete (or provide a turn-aware paging primitive) before applying eviction, and cover it with real page-fragment/cursor behavior.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Summary
Desktop kept accumulated transcript history in the Renderer and added a row virtualizer to bound mounted Turns. That left memory proportional to history, rebuilt the snapshot with full sort/clone work, and made scrolling depend on a second height/spacer state machine.
This PR moves the bound to the existing Runtime Host transcript replica (10 Turns / 512 KiB).
DesktopTranscriptRangeStoreis now a reference-stable immutable projection of that Host range, React renders every active Turn, and Chromiumcontent-visibilityskips offscreen rendering. The row virtualizer, height index, spacers, compensation state, Renderer per-batch full sort/clone, and their observers/scheduler are deleted with no fallback path.The remaining warm-scroll LoAF came from three onboarding
:has()selectors: prompt-raildata-active/aria-currentchanges invalidated the whole chat subtree.AppShellalready owns onboarding visibility, so it now projects that boolean directly onto the layout element.The authority pipeline remains one-way:
Runtime Host bounded range -> DesktopTranscriptRangeStore immutable projection -> stable Turn DOM -> Chromium content-visibilityuseChatScrollremains the sole scroll authority and Astryx remains the sole streaming display scheduler.Performance
Predeclared primary metrics were CDP
TaskDurationandRecalcStyleDuration. A is startingmain00a15abd; B's measured production candidate isd866253a. Final head1b798e98d9d48d43784fff7e7d2d7ab1894bf687differs only in this PR's Playwright E2E fixture, so production build inputs and the read-only CDP script (f000c697...) are byte-identical. Both used production builds, Electron 43.4.1, the same 120-Turn fixture, a 1000x700 window, one worker, and the same Apple M5 Pro / 64 GiB machine. Runs were paired and interleaved:A1-B1, B2-A2, A3-B3, B4-A4, A5-B5, B6-A6, A7-B7.The weakest paired gains exceed A's IQR/median noise. All 14 runs reported
loafSupported:true; B had zero frames over 12.5 ms and zero >50 ms warm LoAF. Secondary median changes stayed inside the 10% gate: heap +0.50%, Nodes +4.21%, DOM +3.45%, listeners +3.93%; session switch improved 14.66%.Final-head stress is directly reproducible without editing source:
cd apps/desktop && MAKA_TRANSCRIPT_STRESS=1 npx playwright test --config e2e/playwright.config.ts e2e/native-transcript-perf.spec.ts --grep '600\+ Turn' --workers=1It completed two full 640-Turn sweeps (126 / 126 successful pages, 28 forced-GC samples). Mounted max was 10; Nodes were 2,800-3,006 with same-stage growth <=3.43%. Using the predeclared 10% secondary resource gate, first-sweep heap endpoint/projected growth was 3.05% / 5.34%, and second-sweep growth was 2.54% / 5.20%. Both sweeps use every sample from iteration 0 through the terminal oldest page; negative slopes are treated as reclamation, not growth. A retain-all mutation failed the same gate at 189.35%.
Entropy ledger
The remaining owners are the Runtime Host range and its disposable Renderer projection. New derived state is limited to bounded ordered identities plus one invalidated immutable snapshot reference. The direct onboarding data attribute projects an existing
AppShellboolean and stores no independent state.Diff against A: 26 files, +1,259 / -1,430 (net -171 lines).
Verification
npm run build:with-depson A and Bgit diff --checkThe E2E coverage includes history anchors, prompt navigation, return latest, streaming follow, scroll-away, stable Turn identity, focus/selection, browser find/accessibility, and session switching.
Risks
History paging now evicts whole Turns around the visible sequence anchor rather than retaining every visited message. A protected active Turn may exceed the 512 KiB range target but remains bounded by the existing 16 MiB message cap. Prompt navigation outside the active range reloads around its Host sequence. The affected paging, navigation, streaming, nested scrolling, selection/focus, find/accessibility, and session-switch paths are covered above.
AI use
Select exactly one:
Tool(s) and scope: Codex diagnosed the performance traces, implemented the change, and authored tests and the read-only benchmark harness. Material commits include
Generated-by: Codextrailers.Checklist
Does this PR entail a change in behavior?