Skip to content

Load deferred tool outputs in one request without scroll animation - #231

Merged
stippi merged 3 commits into
mainfrom
fix/deferred-output-scroll
Oct 8, 2026
Merged

stippi merged 3 commits into
mainfrom
fix/deferred-output-scroll

Conversation

@stippi

@stippi stippi commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Problem

Opening a session with several expanded tool cards whose outputs are stored as blobs replayed the "scroll to bottom" animation several times.

Cause: every expanded block with a deferred output requested it on its own, and each answer came back as UiEvent::UpdateToolStatus — the live agent path, which calls auto_scroll_if_following and starts the spring animation. scroll_to_end only holds until the next layout (it then becomes a regular top anchor), so each arriving output that grew a visible block pushed the bottom out of view again, and the animation chased it — once per output.

A second issue: those answers carried no session ID and were applied to whatever session was shown when they arrived, including triggering the animation there. Tool IDs are only unique per session.

Changes

  • Core: SessionService::load_tool_outputs(session_id, tool_ids) replaces load_tool_output and reads all requested blobs in one call. A result that can't be read (or an unknown tool ID) is left out with a warning instead of failing the batch.
  • New event: UiEvent::ToolOutputsLoaded { session_id, results } instead of reusing UpdateToolStatus. ACP ignores it.
  • GPUI:
    • Blocks no longer dispatch the request themselves; BlockView::claim_deferred_output hands out the tool ID once, and the event loop collects all of them after applying tool results and sends a single request. Expanding an inline block sends a request for that one ID.
    • ToolOutputsLoaded for a session no longer shown is dropped.
    • On arrival, a list that follows its tail jumps to stay at the bottom (keep_tail_in_view) instead of animating; a list that doesn't follow keeps its anchor.
  • Docs: docs/session-storage.md, AGENTS.md.

Tests

  • loaded_outputs_keep_a_following_transcript_at_the_bottom_without_animating — failed on !animating before the fix (reproduces the bug).
  • outputs_loaded_for_a_session_no_longer_shown_are_dropped
  • Core test updated to the batch API, including an unknown ID being left out.

cargo test --workspace, cargo fmt --check and cargo clippy --all-targets --all-features -D warnings pass. Not yet verified in the running app.

Notes for review

  • In-flight loads are not cancelled on a session switch; the blobs are read and the answer discarded. With one request per session open this stays bounded.
  • In the expand path the block is marked as requested before checking for a session ID / Gpui global. Both always exist in the app; only relevant for tests without the global.

stippi added 3 commits October 8, 2026 15:15
Restored tool results whose outputs were deferred came back one by one as
UpdateToolStatus, the live agent path, which starts the smooth scroll to
the bottom. Opening a session with several expanded cards replayed that
animation once per arriving output.

SessionService::load_tool_outputs now loads all outputs a session view
needs in one call. They arrive as UiEvent::ToolOutputsLoaded, tagged with
their session so GPUI drops answers for a session it no longer shows, and
a transcript that follows its tail jumps to stay at the bottom instead of
animating.
terminate_agent stopped and aborted the run before dropping the sleep
guard and agent lock. The stopped setup task resolves the turn outcome on
another worker thread, so a waiter could observe the outcome while the
wake lock was still held. pending_setup_does_not_keep_its_owner_alive
failed this way on CI ("shutdown leaked the run's wake lock").

Release first, like the run task already does before publishing idle.
a_message_sent_while_the_agent_works_waits_for_it sent its second message
as soon as the session reported AgentRunning. Until the agent's first
iteration starts, it takes a waiting message into its request, so the
message showed up in the transcript instead of waiting, and the test
timed out (136 of 400 runs under load). Wait for WaitingForResponse
instead, as the core test for queued skill triggers already does.

Also put the scroll_state doc comment back above its own method.
@stippi
stippi merged commit 858aebe into main Oct 8, 2026
5 checks passed
@stippi
stippi deleted the fix/deferred-output-scroll branch October 8, 2026 19:10
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