Distinguish between agent conversation model update types#9864
Distinguish between agent conversation model update types#9864lucieleblanc merged 3 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 enriches conversation status update events with previous/new status and precomputed status-filter bucket details, while preserving current subscriber behavior. The model and subscriber updates are consistent with the existing status mapping, and the added unit coverage exercises restored events, bucket transitions, same-bucket re-emissions, and status-to-filter mapping.
Concerns
- No blocking correctness, security, error-handling, or performance concerns found in the changed diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| Restored, | ||
| /// The conversation's status was set. | ||
| StatusSet { | ||
| prev_filter: StatusFilter, |
There was a problem hiding this comment.
Thoughts on storing AgentRunDisplayStatus here instead of StatusFilter? We can use from_conversation_status to get it, and means we don't need a new conversation_status_filter method since AgentRunDisplayStatus already has a status_filter
There was a problem hiding this comment.
Changed, thank you!
| is_restored: bool, | ||
| /// The conversation's status before this update, if known. | ||
| /// Restoration events do not have a previous status. | ||
| prev_status: Option<ConversationStatus>, | ||
| /// The conversation's status after this update. | ||
| new_status: ConversationStatus, |
There was a problem hiding this comment.
nit: what do you think about something like
enum ConversationStatusUpdate {
Restored,
Changed { prev_status: ConversationStatus },
}
UpdatedConversationStatus {
// ...
update: ConversationStatusUpdate,
new_status: ConversationStatus
}since prev_status and is_restored are pretty coupled
There was a problem hiding this comment.
I like this a lot! Changed, thanks
Co-Authored-By: Oz <oz-agent@warp.dev>
lucieleblanc
force-pushed
the
lucie/app-4329-conversation-update-details
branch
from
6b2457e to
2dfff48
Compare
…#9864) Co-authored-by: Oz <oz-agent@warp.dev>
3 participants
Closes #10076
Description
The "Runs" agent management view rebuilds its entire card list whenever any conversation's status changes. Most of those events are visually no-ops (re-emitted statuses or status changes that don't cross the active filter), but subscribers can't tell because the event carries no detail.
This PR is the first of two and only changes the shape of the conversation status update event. Subscribers continue to behave exactly as before. The follow-up PR uses the new payload to skip rebuilds when possible.
Concretely:
BlocklistAIHistoryEvent::UpdatedConversationStatusnow carries the previous and newConversationStatus. Restoration events reportprev_status: Nonesince restoration doesn't change the underlying status.AgentConversationsModelEvent::ConversationUpdatednow carries aconversation_idand a typedConversationUpdateKindof eitherRestoredorStatusSet { prev_filter, new_filter }. Filter buckets are precomputed at emission time so subscribers don't have to recompute membership.conversation_status_filtermapsConversationStatusto itsStatusFilterbucket.{ .. }patterns and ignore the new fields. The variant is annotated with#[allow(dead_code)]until the follow-up PR lands.Testing
Added/updated four unit tests on the model:
Restoredemission, changing filter buckets, same-bucket re-emission, and theConversationStatus-->StatusFiltermapping.Server API dependencies
No server dependencies.