Studio: keep the streaming render harness the perf PRs kept rebuilding by danielhanchen · Pull Request #8969 · unslothai/unsloth · GitHub
Skip to content

Studio: keep the streaming render harness the perf PRs kept rebuilding - #8969

Merged
danielhanchen merged 11 commits into
mainfrom
perf-stream-harness
Aug 18, 2026
Merged

Studio: keep the streaming render harness the perf PRs kept rebuilding#8969
danielhanchen merged 11 commits into
mainfrom
perf-stream-harness

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Aug 16, 2026

Copy link
Copy Markdown
Member

Four merged PRs moved the chat streaming render path, and each built a throwaway harness to prove it:

PR What it fixed Reported
#7892 Streamdown's transition starvation 9.86s to 0.34s longest freeze
#8750 incremental Markdown parsing O(n) per update to tail only
#8845 publish coalescing 4.01s to 0.62s longest stall
#8935 incremental fence tokenization 21x fewer characters to Shiki

Nothing was left behind that would notice the next regression, and each measurement had to rediscover the same methodology. #8845 needed four attempts before its numbers meant anything: a real model gave the two sides different essays, generating the stream in-page competed for the throttled CPU budget, a CSP silently blocked the helper server, and a wall-clock quiet window declared a reply finished in the middle of a freeze.

This keeps the harness.

What it runs

smoke-stream-pacing.html mounts the real MarkdownText inside a real assistant-ui local runtime, in the same shape as the existing smoke-research.html and smoke-autoscroll.html: a vite entry, no backend, no auth, no GPU, no model. A local runtime rather than a bare component because MarkdownText reads its part from assistant-ui context and its BlockComponent reads useAuiState, and because assistant-ui's own update scheduling is where #7892's starvation lived.

tests/studio/playwright_stream_pacing.py drives it under CPU throttling and reports longest stall, long-task total and count, frames over 33 ms, and time to fully painted.

Three methodology points are carried over deliberately, each from a way a measurement went wrong before:

  • The reply is a fixed string built by repetition. The renderer's cost is superlinear in length, so comparing two different essays says nothing.
  • "Settled" is counted in frames without growth, not in wall-clock time. A freeze blocks the frame loop too, so a frame counter cannot tick through one, which is exactly what a 1.5s quiet window failed to do.
  • The finish verdict is computed in the page, not by polling from the driver. Every round trip is itself slowed by the throttling.

Choosing the budgets

A budget picked by feel is not a gate. Two merged fixes were reverted in this harness and measured on two different machines, and they move the two numbers in opposite directions:

main #8750 reverted #7892 reverted
long tasks 5.0 to 8.0s 13.1s / 74.4s 6.7 to 7.9s
long task count 49 to 71 144 / 598 14 to 23
longest stall 1.05 to 1.23s 0.97 to 1.40s 5.03 to 6.35s

Reverting #8750 (incremental Markdown parsing) blows up the long-task total and leaves the longest stall alone. Reverting #7892 (the Streamdown animated configuration that keeps block updates out of an interruptible transition) does the exact opposite: the stall goes 4 to 5x while the long-task total stays inside the clean range.

So both budgets are load-bearing, and a single headline metric would have missed one of these two outright. Every mutated run exits 1 on the budget you would expect and every clean run exits 0, over sixteen clean runs, six with #8750 reverted and five with #7892 reverted.

Machine spread, and why this does not gate yet

Clean long-task readings are 5,029 and 5,901ms on one machine and 6,687 to 8,003ms over five runs on another. So the number varies around 60% across boxes even though a single box repeats to within around 15%, and the headroom over the 10,000ms budget is 1.7x on the first machine but only 1.25x on the second.

The budget is deliberately not raised to buy that headroom back, because the same two machines read the #8750 revert as 13,059ms and 74,353ms. A budget loose enough to be comfortable on the slower box stops catching that regression on the faster one. That tension is the whole reason the step keeps continue-on-error: true: tighten it from observed runner numbers, not from any one machine.

The longest-stall budget needs no such hedging. 2,500ms sits at 2.0x above the worst clean run and 2.0x below the mildest #7892-reverted run, and the stall metric repeats to within 4% on a single box.

What stops it reading a false pass

A harness that cannot fail is worse than nothing, so every number the budgets rest on is asserted to have measured something, and every number is scoped to the stream rather than to the page:

  • at least 90% of the characters sent were painted, and the arrival count matches the rate this claims to feed. Without these, a page that rendered nothing scores a perfect zero on every budget.
  • the reply is still there at the end. paintedChars is a high-water mark and only ever climbs, so a completion render that truncated the bubble would leave the peak behind and pass the floor on a DOM that no longer held the reply. The length present at settlement is recorded separately and checked against the same floor.
  • long tasks were actually observed, and the engine actually supports the entry type. observe({type: "longtask"}) is specified to abort silently on an engine that lacks the type rather than throw, so a try/catch around it never fires: longtask is Chromium-only, and under firefox or webkit the long-task total stayed at 0 and sailed under the budget. Support is detected with PerformanceObserver.supportedEntryTypes, recorded, and failed on.
  • CPU throttling was actually applied. Unthrottled, the renderer keeps up with any rate this can feed and every budget passes on any tree.
  • a stall that never ends is still a stall. The longest stall used to be written only when a later paint closed it, so a freeze running to the end of the stream went unrecorded while its missing tail hid inside the 90% floor. The stall in progress is measured instead, capped at the moment the stream ended. The cap matters in both directions: a freeze spanning that moment blocks the frame loop across it, so the first frame afterwards already sees an ended stream and the whole interval would otherwise be skipped, while measuring past it would count the quiet frames the settle check itself needs and report a stall on every healthy run. That rule lives in smoke-stream-pacing-stall.ts with its own tests, two of which fail if the previous rule is put back.

Long tasks and slow frames are counted only from the moment the stream is asked for, so page load and module evaluation are not budgeted as render cost. That was measured at one entry of about 140ms, roughly 2.6% of a clean total. A long task carries the start time of its whole task, so the message append is handed to a later task, or runtime startup and the first publish would sort before the window and be dropped as page load.

A broken page fails fast rather than hanging: with the fixture deliberately crashed on load the driver exits 1 in 65s on the readiness wait, well before the 300s settle deadline.

Chromium only, and deliberately

Both of the things that make this a measurement are Chromium-only: Emulation.setCPUThrottlingRate is reached over CDP, which Playwright exposes for Chromium alone, and longtask entries exist in no other engine (the Gecko bug is open, WebKit has never shipped them). Verified against all three engines: the page renders fine under firefox and webkit, new_cdp_session raises CDP session is only available in Chromium, and supportedEntryTypes excludes longtask without observe() throwing. Hence the explicit support check rather than a catch.

This step will not execute yet

The step is added after Browser smoke for ANSI tool output, which is currently failing on main and does not carry continue-on-error. A failing step without it skips every step after it in the job, so this one reports as skipped rather than running, which is what happened on a replicated CI run of this branch. That is being handled separately and is not fixed here, but it is worth stating plainly: until it is resolved, a green Frontend CI is not evidence that this harness ran.

It is also why the budgets are still calibrated from developer machines rather than from runner numbers.

Limitation

createStreamPublishGate from #8845 lives in the Studio chat adapter, not in this render path, so this harness does not exercise it. What it covers is everything downstream of the yield: the Streamdown configuration, the incremental Markdown cache, the fence tokenizer, and assistant-ui's update scheduling.

A separate finding, not fixed here

The entry opens with a deliberate side-effect import of the sidebar organization store. Without it the harness renders nothing and the page throws Cannot access 'SIDEBAR_ORGANIZATION_STORAGE_KEY' before initialization.

The underlying cycle is real app code, not a harness artefact: the chat barrel re-exports chat-page, which reaches the tour barrel, whose confetti helper imports the settings barrel, which imports the General tab, which imports SIDEBAR_ORGANIZATION_STORAGE_KEY back out of the chat barrel and reads it at module evaluation time in a top-level const PREFS_KEYS array. The app survives only because app-sidebar.tsx happens to import the theme toggler, and so the settings barrel, above its own chat import. Moving that one import block up reproduces the same white screen in the real app.

So this is a latent app bug worth its own change, not something to fix in a test-only PR. #8966 makes the settings tabs lazy, which cuts that edge and makes the workaround here unnecessary; the harness was verified to load clean with #8966 applied and the workaround removed. Whichever lands first, the root fix is for the General tab to read the constant from the store module rather than through the barrel.

Testing

  • python3 tests/studio/playwright_stream_pacing.py: sixteen clean runs, six with Fix Studio CPU saturation on long streaming replies #8750 reverted, five with Studio: prevent long streaming Markdown stalls #7892 reverted, numbers above
  • all three Playwright engines probed for CDP and longtask availability
  • npm test (2,787 passed, including 5 new for the stall rule), npm run typecheck, npm run build
  • dist/ confirmed to contain no reference to the smoke entry, and the entry is not in index.html's import closure, so neither the bundle nor the 75 MB budget moves
  • pytest tests/studio/test_playwright_server_lifecycle.py tests/studio/test_autoscroll_harness_contract.py (27 passed) with the harness registered in both
  • ruff check on the driver, npx eslint on the harness entry, clean

The harness had stopped loading, and CI could not tell us

Recorded here because it is the kind of thing that comes back silently.

After this branch merged main, the smoke page stopped loading entirely. Loading smoke-stream-pacing.html on the branch head produced an empty #root and a single page error:

Cannot access 'MarkdownText' before initialization

and the driver then died on a 60 second wait_for_function timeout. Every other smoke page on the same tree loaded fine, so it was specific to this one.

The cause is a second edge in the cycle described above, added by main rather than by this branch. #9014 introduced a module-scope literal in thread.tsx:

const ASSISTANT_PART_COMPONENTS = {
  Text: MarkdownText,

This harness enters the graph at markdown-text.tsx, which imports the chat barrel, which reaches chat-page, which reaches thread.tsx, which reads MarkdownText while that binding is still in its temporal dead zone. Module bodies evaluate once and live bindings start uninitialized, so a cycle is only harmless while every access is deferred into a function body, and this one is not. The workaround already at the top of the entry broke the first edge; main added a second and the trick went stale.

Fixed by importing the chat barrel before MarkdownText, mirroring the app's own entry order.

The part worth stating plainly: CI stayed green through all of it. The step that runs this harness carries continue-on-error: true, so a page that could not load, and a driver that timed out after 60 seconds, both reported as a passing job. The budgets this PR exists to enforce were not being enforced; nothing was being measured at all.

That argues for distinguishing the two failure kinds at the exit-code level, so that "the page is dead" can gate the job while the timing budgets stay advisory. That is a change to the workflow rather than to this harness, so it is not made here, but the current arrangement means a green Frontend CI is not evidence that this harness ran, let alone passed.

Separately, smoke-stream-pacing.html is a new HTML entry and did not load /crypto-boot.js, which #9075 added a test for. That was the live red on this PR and is fixed in the same commit.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

The long-task total is the metric the budgets turn on, and it reads 0 both
when the render is free and when the observer never ran. observe({type:
"longtask"}) is specified to abort silently on an engine that lacks the
entry type rather than throw, so the try/catch around it never fired: under
firefox or webkit the harness scored a perfect zero and exited 0. Detect
support with PerformanceObserver.supportedEntryTypes, record it, and fail
the run when no long tasks were seen or when throttling was disabled.

Also:
- add the entry to tsconfig.app.json, which lists the smoke entries one by
  one, so npm run typecheck actually covers the 270 lines it was reported
  against
- write the JSON under logs/ like every sibling harness instead of dropping
  an untracked stream-pacing.json in the repo root, and create the directory
- treat an exported-but-empty SMOKE_BASE_URL as unset, matching the siblings;
  it drove "" as the base URL and burned the full readiness timeout
- register the harness in the two contract tests, which is what surfaced the
  SMOKE_BASE_URL bug, and pin the new guards there
- record the second mutation: reverting #7892 moves the longest stall 4-5x
  while leaving the long-task total inside the clean range, the exact
  opposite of reverting #8750, so both budgets are load-bearing
buffered: true replays whatever the performance timeline already held, so
module evaluation and the first React render landed in the budgeted total:
one entry, ~140ms, about 2.6% of a clean run here, and larger on a cold or
loaded runner. Nothing filtered by startTime and run() reset nothing, so a
slow page load read as a slow renderer.

Open the measurement window in run() and drop entries that began before it.
Pre-stream share goes 2.6% to 0.00% while the stream's own tasks are
unchanged (60 and 52 entries over two clean runs), and reverting #8750 still
fails the budget at 52,465ms.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

Comments and docstrings only, no code change. Every measured number, PR
reference and causal reason is kept verbatim.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: f71569c161

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

paintedChars is a high-water mark and only ever climbs, so a completion
render that truncated the bubble would leave the peak behind and the 90%
workload floor would still pass on a DOM that no longer held the reply.
Record what is on screen at settlement and check that too. Measured equal
to the peak today (24,033 both), so this is a guard rather than a live
discrepancy, and it is pinned in the harness contract test.

Also count slow frames only inside the measurement window and reset the
counter in run(), the same rule long tasks now follow. Contamination
measured at 0 of 286 here, but an external server or a slower box need not
be 0 and the number is meant to be comparable across them.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

… the stream

Two holes left by the measurement window added in the previous commits.

A long task carries the start time of its whole task, so appending the user
message in the same task that assigned measureFrom stamped runtime startup
and the first publish as earlier than the window and dropped them as page
load. Hand the append to a later task so the work that begins the stream
sorts inside it.

longestStallMs was only ever written when a later paint closed the stall, so
a freeze that ran to the end of the stream was never recorded: the tail can
go missing inside the 90% floor and the quiet-frame loop then calls it
settled. Measure the stall in progress while text is still arriving, which is
what the number means, and not afterwards, where the settle window's own
quiet frames would read as a freeze.

Clean runs unchanged (stall 933 to 1,050ms, long tasks 4,749 to 5,159ms over
three) and both mutations still caught: #7892 reverted fails the stall at
5,233ms, #8750 reverted fails long tasks at 52,263ms.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: bef0cf102c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comments only, no code change. Every measured number and every causal
reason is kept.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

The stall in progress was measured only while text was still arriving. A
freeze that spans the moment the stream ends blocks the frame loop across
it, so the first frame afterwards already observes a non-null
streamEndedAtMs and the whole frozen interval was skipped. With the lost
tail able to hide inside the 90% workload floor, thirty quiet frames then
settled the reply and the run reported a short longest stall, which is the
one shape this number exists to catch.

Cap the interval at the absolute stream-end timestamp instead. A freeze
across that moment is recorded in full, and the stall stops growing once
there is no more text to wait for, so the settle check's own quiet frames
are still not counted as a freeze.

The rule moves into smoke-stream-pacing-stall.ts so it can be tested
without importing the harness entry, which mounts React on import. The new
tests cover the spanning freeze, the settle-window bound, idempotence and
late tail paint; restoring the previous rule fails two of the five.

Clean runs unchanged (stall 967 to 983ms, long tasks 5,442 to 5,842ms) and
both mutations still caught: #7892 reverted fails the stall at 5,017ms,
#8750 reverted fails long tasks at 63,687ms.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 16, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 0bcf6a330d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64baedbaf7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}
const painted = paintedChars();
if (painted > state.paintedChars) {
const stall = now - lastGrowthAt;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cap late-growth stalls at the stream end

When the stream ends during a main-thread freeze and the first frame afterward observes newly painted text, this growth branch records now - lastGrowthAt, bypassing the stream-end cap used by the no-growth branch below. The resulting longestStallMs includes arbitrary post-stream delay even though the metric and its 2,500 ms budget are defined only while text is arriving, so a late final paint can produce a false budget failure; apply the same stream-end cap when closing a stall on growth.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Measured this both ways and the cap is what would break the number, not what fixes it, so I am leaving the growth branch uncapped.

The asymmetry is deliberate. The no-growth branch has to cap at stream end or the 30 quiet frames the settle check needs would be reported as a stall on every healthy run. The growth branch must not, because a growth event is a real paint: the interval it closes ends at the moment arrived text finally reached the screen. Capping it would make never painted score better than painted late.

Numbers, Chromium at 6x throttle. In a burst config, chunkChars 4000, the last text arrives at 2547ms and the bubble does not grow again until 4521ms. The branch as written reports 2017ms. The proposed cap reports 967ms and discards 1974ms of arrived-but-unpainted text, which is exactly the freeze class this metric exists to catch. In the shipped 96-char config the cap changes longestStallMs by 0.0ms, 1200.1ms either way, because the maximum came from 1060 to 2260ms, mid-stream, with 43.2ms of post-stream exposure against a 2500ms budget.

There is also no idle-time vector: post-stream, paintedChars only rises when the DOM textContent grows, so the growth branch cannot fire while idle, and the driver stops polling as soon as done flips.

Separately, your item made me re-run the harness end to end, which turned up that the page had stopped loading at all after the main merge. Fixed in 3d0bbce.

Merging main brought in ASSISTANT_PART_COMPONENTS, which thread.tsx builds
at module scope with Text: MarkdownText. Entering the markdown-text ->
features/chat -> chat-page -> thread cycle from markdown-text runs that
object literal while the MarkdownText binding is still in its temporal dead
zone, so the page died with Cannot access MarkdownText before initialization
and rendered nothing. Import the chat barrel first, as the app's entry does.

The page is also a new HTML entry, so it has to load the crypto polyfill
before its module entry like every other one.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

@danielhanchen
danielhanchen merged commit 1f09a17 into main Aug 18, 2026
35 of 39 checks passed
@danielhanchen
danielhanchen deleted the perf-stream-harness branch August 18, 2026 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant