Skip to content

perf(desktop): bound transcript rendering and eliminate scroll LoAF - #4206

Open
Astro-Han wants to merge 12 commits into
apache:mainfrom
Astro-Han:fix/desktop-transcript-bounded-range
Open

perf(desktop): bound transcript rendering and eliminate scroll LoAF#4206
Astro-Han wants to merge 12 commits into
apache:mainfrom
Astro-Han:fix/desktop-transcript-bounded-range

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

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). DesktopTranscriptRangeStore is now a reference-stable immutable projection of that Host range, React renders every active Turn, and Chromium content-visibility skips 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-rail data-active / aria-current changes invalidated the whole chat subtree. AppShell already 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-visibility

useChatScroll remains the sole scroll authority and Astryx remains the sole streaming display scheduler.

Performance

Predeclared primary metrics were CDP TaskDuration and RecalcStyleDuration. A is starting main 00a15abd; B's measured production candidate is d866253a. Final head 1b798e98d9d48d43784fff7e7d2d7ab1894bf687 differs 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.

Pair Task A / B (ms) Gain Recalc A / B (ms) Gain B frame P95 B >50 ms LoAF
1 809.9 / 263.5 67.5% 373.6 / 24.3 93.5% 10.1 ms 0
2 692.8 / 182.8 73.6% 321.4 / 17.3 94.6% 10.2 ms 0
3 888.8 / 237.6 73.3% 408.9 / 23.2 94.3% 9.9 ms 0
4 781.0 / 272.0 65.2% 362.6 / 23.9 93.4% 10.1 ms 0
5 706.1 / 181.9 74.2% 330.3 / 17.7 94.6% 10.2 ms 0
6 766.2 / 261.6 65.9% 348.8 / 27.9 92.0% 10.0 ms 0
7 703.9 / 272.0 61.4% 334.8 / 25.4 92.4% 9.9 ms 0
Metric A median / P95 / P99 B median / P95 / P99 Median gain A / B IQR Wins
Task 766.2 / 865.1 / 884.1 261.6 / 272.0 / 272.0 65.9% 90.5 / 57.6 7/7
RecalcStyle 348.8 / 398.3 / 406.8 23.9 / 27.2 / 27.8 93.1% 35.6 / 4.4 7/7

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=1

It 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

Scoped concept Before After
Transcript semantic authorities 1 1
Persistent transcript state added 0 0
Runtime transcript state owners 3 2
Message/layout representations 3 2
Transcript-scroll observer instances 7 5
rAF scheduler owners 3 2

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 AppShell boolean and stores no independent state.

Diff against A: 26 files, +1,259 / -1,430 (net -171 lines).

Verification

  • npm run build:with-deps on A and B
  • Desktop and UI typechecks; Biome on changed files; git diff --check
  • Desktop transcript range-store unit tests: 28/28
  • UI focused unit tests: 18/18
  • transcript-scroll + prompt-rail E2E: 19/19, including nested near-start ownership
  • onboarding viewport E2E: 2/2
  • repeated prompt-jump landing: 10/10
  • LoAF unsupported capability test + supported warm measurement
  • exact-head 640-Turn repeated-paging stress with executable bounds
  • final-head 7-pair interleaved production CDP A/B

The 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:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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: Codex trailers.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

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
@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 29, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review August 29, 2026 19:34

@zhiiw zhiiw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts groups resident messages by turn (turnless messages each get their own sequence:N group, 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. #refreshSequenceBounds is O(1) off the order array.
  • The :has() removal is a real LoAF fix: projecting data-maka-onboarding from AppShell (which already owns showOnboardingHero) onto the layout element removes the descendant-:has() invalidation of the whole chat subtree on every prompt-rail aria-current change.
  • content-visibility: auto uses the right containment pattern: contain-intrinsic-block-size: auto 280px keeps the last-rendered size, so scroll height does not collapse for already-rendered turns.
  • Scroll authority is preserved: useChatScroll remains the only writer; the new nested-scroller wheel guard walks composedPath and 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 ({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 test check 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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants