Skip to content

perf(cli): render one smaller page of transcript messages - #1550

Merged
obadakhalili merged 1 commit into
CodebuffAI:mainfrom
Salhi-zh:perf/cli-transcript-page
Oct 23, 2026
Merged

obadakhalili merged 1 commit into
CodebuffAI:mainfrom
Salhi-zh:perf/cli-transcript-page

Conversation

@Salhi-zh

Copy link
Copy Markdown
Contributor

Why

A re-render in opentui lays out every rendered node, and the cost is linear in that count whether or not the node changed:

case (1,500 spans) median per frame
re-render + paint 59.2 ms
renderOnce with nothing re-rendered 0.1 ms

So the cost is not a per-frame paint charge — an untouched tree is nearly free. It is paid per re-render, which is why memoizing cannot rescue it: a page of 15 memoized messages measured 15.0 ms against 13.1 ms for the same page unmemoized at 1,800 nodes. @opentui/core@0.3.4 has no culling, so the only lever is how many nodes a page holds.

A streamed chunk re-renders the whole page. At the real density (~770 nodes per message) one page measures:

page nodes median per frame
15 messages (today) 11,550 266.5 ms
6 messages 4,620 104.4 ms

What

MESSAGE_BATCH_SIZE 15 → 6, with the measurement recorded next to the constant so the next reader knows it is a frame-cost knob and not a layout preference. LoadPreviousButton already renders the hidden count and loads a page at a time (handleLoadPreviousMessages adds MESSAGE_BATCH_SIZE), so the rest of the history is one click away and each load stays cheap.

Verification

  • bun run typecheck → exit 0
  • bun test over all non-integration files: 3958 pass, 20 fail — the same 20 import errors main has in a partial checkout (Failed to load CLI environment … Cannot find module '../../../packages/internal/src/env'); main alone measures 3951 pass / 20 fail.
  • Frame timings above are bun bench runs against @opentui/core's test renderer, not the app harness; the ratio is what the change buys.

Trade-off

Fewer messages visible before clicking "load previous". If 6 is too aggressive, the number is independent of the mechanism — say which page size you want and I will re-measure it.

A re-render lays out every rendered node, and the cost is linear in that
count whether or not the node changed: an unchanged 1,500-node transcript
measures ~60ms per frame against ~0.1ms when nothing re-renders, so
memoizing cannot rescue it and a streamed chunk re-renders the whole page.

At the real density (~770 nodes per message) one page of 15 messages costs
~266ms per frame against ~104ms for six, so a long conversation falls
behind its own output. LoadPreviousButton already offers the rest of the
history, so the smaller page trades a click of history for frames that keep
up with the stream.
@codebuff-team

Copy link
Copy Markdown
Contributor

The underlying observation is useful: opentui's re-render cost is apparently linear in node count regardless of diffing, so the page size for streamed transcript rendering is a real knob worth tuning. The comment you added documenting why 15 was chosen (or wasn't) is a genuine improvement over the bare constant.

That said, this is a one-line behavioral change justified entirely by numbers in the PR description that aren't reproducible from the diff itself. There's no bench script checked in, no test asserting the frame-cost claim, and no way for a maintainer to verify 6 vs. 15 vs. some other value without re-running your informal bun bench session against the test renderer. For a perf-sensitive magic number like this, I'd expect either:

  1. A committed micro-benchmark (even a simple one) in the repo so the number is falsifiable later when opentui or the message renderer changes, or
  2. The batch size made configurable/adaptive rather than hardcoded differently, since the right value plausibly depends on terminal size, message density, or future opentui versions.

Also 15 → 6 is a steep cut to default history visibility — more than half of users' visible scrollback disappears behind a click. That's a real UX regression that should be weighed against the frame-rate win explicitly by a maintainer, ideally with the two numbers (265ms @ 15 vs 104ms @ 6) coming from something in-repo rather than only prose.

Good instinct, right file, but needs a verifiable benchmark and probably a smaller first cut (e.g. 15 → 10) with room to iterate, rather than committing to 6 based on out-of-tree measurements.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Oct 23, 2026
@obadakhalili
obadakhalili merged commit b960b31 into CodebuffAI:main Oct 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants