Handle agent management view updates based on event type#9866
Handle agent management view updates based on event type#9866lucieleblanc merged 5 commits intomasterfrom
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR optimizes the agent management Runs view by routing conversation update events through targeted card refreshes instead of rebuilding the full card list for every status update.
Concerns
- Same-bucket status re-emissions still take the targeted card-update path, so the streaming hot path continues to scan visible cards and rebuild button config on every event rather than becoming a no-op.
- This is a user-visible Runs view behavior change, but the PR description does not include screenshots or video evidence. For faster review, please upload screenshots or a video of the feature working end to end.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| self.get_tasks_from_model(ctx); | ||
| self.refresh_details_panel_if_needed(ctx); | ||
| } else { | ||
| self.update_card_for_conversation(conversation_id, ctx); |
There was a problem hiding this comment.
InProgress status sets scan self.items and rebuild the card button config on every streaming event; make exact status re-emissions a no-op before calling update_card_for_conversation (or carry enough previous/new status detail to distinguish re-emits from real same-bucket changes).
liliwilson
left a comment
liliwilson
left a comment
There was a problem hiding this comment.
Thank you for fixing!! 🙌
|
|
||
| /// Update the action-buttons config for the card backing a given conversation, without | ||
| /// rebuilding the entire list. If no matching card is currently visible, this is a no-op. | ||
| fn update_card_for_conversation( |
There was a problem hiding this comment.
I don't actually think this function is necessary—we read the status for the card at render time, so if we haven't crossed a filter boundary, we shouldn't actually need to rebuild the card at all. A status change for a local conversation (which is the only type triggered from the BlocklistAIHistoryModel) won't change the action buttons available, so we should be good to just refresh the details panel in the else branch above
There was a problem hiding this comment.
That makes sense, thank you!
| /// whether an item is included by this filter. `All` matches every bucket so it | ||
| /// is never crossed; the other variants are crossed when exactly one of the buckets | ||
| /// equals this filter. | ||
| pub(crate) fn is_membership_crossed( |
lucieleblanc
force-pushed
the
lucie/app-4329-agent-management-update-handling
branch
from
6b6b815 to
94885be
Compare
lucieleblanc
force-pushed
the
lucie/app-4329-conversation-update-details
branch
from
6b2457e to
2dfff48
Compare
lucieleblanc
force-pushed
the
lucie/app-4329-agent-management-update-handling
branch
from
94885be to
edc0230
Compare
…nUpdated event details Co-Authored-By: Oz <[email protected]>
lucieleblanc
force-pushed
the
lucie/app-4329-agent-management-update-handling
branch
from
edc0230 to
35c4172
Compare
…9866) Co-authored-by: Oz <[email protected]>
3 participants
Description
The "Runs" agent management view was rebuilding its entire card list every time any conversation's status changed. While an agent is streaming, status events fire many times per second, and a recent heap profile attributed over 2 GB of allocations (plus ~700 MB of allocator churn) to this rebuild path.
Most of those events don't actually change the visible list: they re-emit the same status (typical during streaming) or change the status within the same active-filter bucket. In both cases the list doesn't need to rebuild — cards read their status fresh at render time, and on this path (local conversations) the action buttons don't depend on status.
This PR makes the agent management view branch on the typed payload introduced in #9864:
Allfilter): refresh the open details panel and re-render so the status icon picks up the new value. No list rebuild and no per-card work.A small
StatusFilter::is_membership_crossedhelper centralizes the "did this status transition cross the active filter?" check.This PR also drops the
conversation_idfield from theConversationUpdatedevent, since no subscriber needs it now that the view doesn't target a specific card on this path.Testing
Started a conversation in the background. Opened the agent management view with the
Allfilter selected and confirmed:Working/Done/Failedboundary still updates the visible cards.Server API dependencies
No server dependencies.
Agent Mode
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Reduced memory usage and CPU work in the agent runs management view while a conversation is streaming.