Skip to content

ui: stabilize the rendering of tool invocations #26098

Open
zachwinter wants to merge 6 commits into
ggml-org:masterfrom
zachwinter:ui/streaming-perf-pr2
Open

ui: stabilize the rendering of tool invocations #26098
zachwinter wants to merge 6 commits into
ggml-org:masterfrom
zachwinter:ui/streaming-perf-pr2

Conversation

@zachwinter

@zachwinter zachwinter commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR addresses instabilities in the rendering of tool invocations in the llama-server UI.

This is a UI-only PR; no cpp core is touched.

Additional information

acd7a2d verifies a processing-state subscription leak as non-issue (lmk if we should keep this)
f25d4bf hardens tool call parsing; reduces header renders from 19 -> 1; language highlights 3 -> 1
9cdf7fa eradicates 24 blank frames during read-file parsing, reduces header renders from 2 -> 1
9a74042 MarkdownContent 11.50ms -> 2.18ms p95 per streamed token at 26KB
77fce09 edit-file; 16 all-deletion diff frames -> 0 per streamed edit
e1f38cf streaming stability contract across 7 parsers (flicker === gone)

Requirements

Comment thread tools/ui/src/lib/components/app/content/MarkdownContent/MarkdownContent.svelte Outdated
@ServeurpersoCom

ServeurpersoCom commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Can you drop acd7a2d? You measured the subscription leak at 0.005ms and concluded it wasn't worth fixing, so the 146 lines of fixture that prove it are dead weight in a suite that runs in a real browser on every PR. The rest of the perf harness is fine to keep, and streaming-stability.ts earns its lines.

One thing while you're in there: none of the perf fixtures ever close a code fence, they stream either plain paragraphs or a fence that stays open. That's how the endOffset issue got past the numbers.

This conflicts with #26097, which restructures parsePartialJsonArgs into scanPartialJson plus a memo cache. Which one goes first?

@ServeurpersoCom

ServeurpersoCom commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Let's just drop the perf suite entirely, it doesn't earn its place.

Zach Winter and others added 6 commits July 27, 2026 02:43
…fter a closed fence

Co-authored-by: Pascal <4873680+ServeurpersoCom@users.noreply.github.com>
Assisted-by: Claude Fable 5
@zachwinter
zachwinter force-pushed the ui/streaming-perf-pr2 branch from e1f38cf to c52e52c Compare July 27, 2026 07:27
@zachwinter

Copy link
Copy Markdown
Contributor Author

Let's just drop the perf suite entirely, it doesn't earn its place.

@ServeurpersoCom aye! would you like me to add to this PR's scope, or do this in a follow-up?

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

The scope just needs to be narrowly defined; browser rendering performance cannot be effectively tested via CI, it requires a human measured / manual testing to ensure there are no regressions / easy to review and no extraneous code.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants