Skip to content

perf(desktop): reduce UI stalls during tool output - #3921

Open
Colafornia wants to merge 3 commits into
apache:mainfrom
Colafornia:fix/desktop-tool-output-jank
Open

perf(desktop): reduce UI stalls during tool output#3921
Colafornia wants to merge 3 commits into
apache:mainfrom
Colafornia:fix/desktop-tool-output-jank

Conversation

@Colafornia

@Colafornia Colafornia commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Live tool output currently publishes every chunk directly to Renderer state. Frequent chunks trigger repeated React updates within one display frame.

This can make the interface feel unresponsive while the model is working. Expanding tool details can lag, and loading animations can drop frames.

  • Before: each tool_output_delta publishes a separate state update.
  • After: deltas received within one animation frame publish as one update.

All output chunks remain ordered and complete. Lifecycle and readiness events remain synchronous and flush pending output before they run. Session teardown discards its queued events so a delayed frame cannot restore cleared state.

No screenshot or recording is included. This problem affects transient frame pacing during live streaming, which static images cannot show reliably.

Verification

  • npm --workspace @maka/desktop run build:main
  • node --test apps/desktop/dist/main/__tests__/streaming-handoff.test.js
  • Scoped Biome check passed for the three affected files.
  • git diff --check
  • Regression tests cover frame batching and delayed frames after session teardown.
  • Manual Desktop testing showed better responsiveness while the model and tools were active.

AI use

Select exactly one:

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

Tool(s) and scope: Codex traced the Renderer event flow, implemented frame batching and the session teardown fix, added regression tests, and ran the listed checks. The affected 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

@Astro-Han Astro-Han 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.

I reviewed this head and found blocking issues.

[P1] Batching caches unbounded raw output before redaction/caps

app-shell-session-events.ts:167-178 enqueues full tool_output_delta into pendingEvents; redaction and size caps only happen in applyToolOutputChunk() at frame time — burst can hold large unredacted data pre-paint → OOM risk.

Fix: redact and bound/coalesce at enqueue, not at paint.

[P2] Delayed frame resurrects cleared session state

Frame callback unconditionally projects pending events; cleanup doesn't drop batch and clear runs separately → delete session then frame creates placeholder turn/tool for deleted ID.

Fix: tie drop/cancel to teardown/delete with regression for delayed-frame-after-clear.

Hosted test/windows_recovery: SUCCESS, but code is NO-GO.

简体中文批量缓存与清理竞争问题。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@Colafornia

Colafornia commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up correction in e9e0bbc8f after reviewing the regression tests and runtime effects:

  • P1 — pushed back after tracing the complete path. Production tool_output_delta events come from createToolOutputDeltaEmitter, which redacts output and limits chunks to 8,192 characters. The Host protocol also bounds each frame, and applyToolOutputChunk keeps the Renderer state redacted and capped. The first response added another enqueue-time redaction pass and a synchronous flush at 200 pending events. Those changes duplicated hot-path work and let a flood bypass animation-frame batching, so they were removed.
  • P2 — retained and fixed at teardown. clearSessionRendererState drops that Session from the shared display batch before clearing UI state. The delayed frame then has no event that can recreate the deleted projection.

Verification:

  • npm --workspace @maka/desktop run build:main
  • node --test apps/desktop/dist/main/__tests__/streaming-handoff.test.js
  • Scoped Biome check for the three affected files
  • git diff --check

The remaining regressions cover the intended frame publication bound and the delayed-frame-after-clear race.

Prevent high-frequency tool output events from publishing renderer state more than once per animation frame.

Generated-by: Codex
Sanitize queued tool output and discard pending events when their session is cleared.

Generated-by: Codex
Remove queue assertions that added duplicate redaction work and synchronous flood flushes outside the frame-batching contract.

Generated-by: Codex
@Colafornia
Colafornia force-pushed the fix/desktop-tool-output-jank branch from e9e0bbc to e98bbf0 Compare August 26, 2026 17:44
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants