fix(vertical-tabs): show agent status badges in summary view (#9868)#10179
fix(vertical-tabs): show agent status badges in summary view (#9868)#10179lonexreb wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
…dev#9868) The vertical-tabs summary view dropped the agent status icon/badge for both Oz and CLI agents — making it impossible to tell from the tab list whether an agent was in progress, done, errored, cancelled, or blocked without opening the agent itself. This regressed the documented invariant that "pane icons in the vertical tabs panel should display a small status badge overlay reflecting each agent's current state." Two cooperating bugs in app/src/workspace/view/vertical_tabs.rs: 1. SummaryPaneKind::OzAgent and CLIAgent dropped the upstream status when projecting from the shared IconWithStatusVariant in summary_pane_kind() (the .. discard pattern). 2. The reverse projection in ambient_agent_variant() — the function that hands the variant back to render_icon_with_status — only matched is_ambient: true and hardcoded status: None. Non-ambient agents fell through to a status-less inline render path. So: ambient agents had their status thrown away, and non-ambient agents never even reached the status-aware renderer. Fix: - Add status: Option<ConversationStatus> to both agent variants of SummaryPaneKind and capture it in summary_pane_kind(). - Rename ambient_agent_variant to summary_pane_kind_to_status_variant, match both ambient and non-ambient cases, and pass the captured status straight through to IconWithStatusVariant. - Route both ambient and non-ambient agent kinds through the status- aware path in render_summary_pane_kind_icon_circle. The remaining inline branch covers only non-agent pane kinds; agent kinds are marked unreachable!() so a future refactor can't silently regress the badge. - Preserve the existing dedup-by-kind semantics in select_summary_pane_kind_icons via a new helper same_kind_ignoring_status. Two Oz agents with different statuses still collapse to one icon (status is metadata, not identity), with the first pane's status preserved on the surviving entry. Tests: - Update the two existing tests that constructed agent variants to pass status: None. - Add four regression tests in vertical_tabs_tests.rs: * status propagation for OzAgent * status propagation for CLIAgent * non-ambient agents still produce a status-aware variant * non-agent kinds correctly return None (caller falls back to inline rendering) - Add a dedup test asserting two same-kind agents with different statuses collapse to one icon and preserve the first status. All 71 vertical-tabs tests pass; cargo check + clippy clean. The shared agent-icon helpers in app/src/ui_components/agent_icon.rs and the cross-surface invariants in agent_icon_tests.rs are unchanged.
github-actions
Bot
added
the
external-contributor
|
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 carries agent status through the vertical-tabs summary projection and routes agent summary icons through the shared status-aware renderer.
Concerns
- Mixed-kind summary rows can still hide an agent status badge: pair-icon rendering overlays the secondary kind in the same bottom-right slot used by
render_icon_with_status, so an agent that is the primary summary icon can have its badge covered.
Verdict
Found: 0 critical, 1 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
| // kinds fall through to the inline circle rendering below. | ||
| // See #9868: previously only ambient kinds went through this path AND the | ||
| // status was hardcoded to None, so the badge never rendered in summary view. | ||
| if let Some(variant) = summary_pane_kind_to_status_variant(&kind) { |
There was a problem hiding this comment.
…es (warpdotdev#10179) Bot review on the previous commit caught a real layout bug: the pair-icon rendering in render_summary_pane_kind_icons places the secondary kind in the bottom-right slot of the primary, but render_icon_with_status puts the agent status badge in the same BR slot via attach_status_overlay. When an agent is the primary in a Pair (e.g., agent + terminal in the same tab group), the secondary icon overlays the badge and hides it, which defeats the whole purpose of warpdotdev#9868. Fix: when summary_pane_kind_to_status_variant(&primary).is_some() — i.e., the primary is an agent that paints a status badge — anchor the secondary at the top-right corner instead of the bottom-right. The 45° offset and ring treatment are unchanged; only the corner moves. Non- agent primaries keep the existing BR placement (no badge to occlude). The decision is gated on the same helper that drives the status-aware render path, so a future change to which kinds report a badge automatically updates the layout in lockstep — no second source of truth. Test: regression test pair_icon_placement_avoids_badge_for_agent_primary pins the contract by asserting which kinds produce a status-aware variant. Covers all four agent shapes (Oz/CLI × ambient/non-ambient) plus four non-agent kinds. 72 vertical-tabs tests pass; clippy clean.
|
Pushed a fix for the BR-overlap concern (commit 27c6fbb). The bug the bot caught: Fix: when The placement decision is gated on the same helper that drives the status-aware render path, so a future change to which kinds report a badge automatically updates the layout in lockstep — no second source of truth. Test: new regression test /oz-review |
|
I'm re-reviewing this pull request in response to a review 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 |
There was a problem hiding this comment.
Overview
This PR preserves agent status in vertical-tabs summary kind projection and routes agent summary icons through the shared status-aware renderer.
Concerns
- The new top-right secondary placement reuses the bottom-right positive Y offset, so agent-primary pair icons will place the secondary down into the primary icon instead of up/right away from the status badge.
Verdict
Found: 0 critical, 1 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
| @@ -3647,8 +3726,8 @@ fn render_summary_pane_kind_icons( | |||
| OffsetPositioning::offset_from_parent( | |||
| vec2f(secondary_corner_offset, secondary_corner_offset), | |||
There was a problem hiding this comment.
TopRight anchoring needs a negative Y offset; keeping the bottom-right positive Y offset moves the secondary icon down into the primary instead of up/right away from the status badge.
| vec2f(secondary_corner_offset, secondary_corner_offset), | |
| vec2f( | |
| secondary_corner_offset, | |
| if primary_has_status_badge { | |
| -secondary_corner_offset | |
| } else { | |
| secondary_corner_offset | |
| }, | |
| ), |
…dev#10179 round 2) Round-2 review caught that my previous TR placement reused the BR- quadrant positive-Y offset. pathfinder uses positive-Y-down, so for the BR anchor (+x, +y) walks the secondary into the BR quadrant correctly. But for the new TR anchor, positive Y pushes the secondary DOWN into the primary icon instead of UP into the TR quadrant. Fix: when primary_has_status_badge is true, the offset Y component flips to -secondary_corner_offset. The X offset and the magnitude are unchanged. Pulled out the per-branch values into a 3-tuple so the asymmetry is visible at the call site. 72 vertical-tabs tests still pass.
|
Round-3 fix pushed (commit 2c0ae44): the TR placement was reusing the BR-quadrant positive-Y offset, so secondary went DOWN into the primary instead of UP. Flipped Y to negative for the TR branch (X unchanged, magnitude unchanged). 72 vertical-tabs tests still pass. /oz-review |
|
I'm re-reviewing this pull request in response to a review 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 |
There was a problem hiding this comment.
Overview
This PR preserves agent conversation status through the vertical-tabs summary projection, routes Oz and CLI agent summary icons through the status-aware renderer, adjusts pair-icon placement to avoid the badge area, and adds regression coverage for status propagation and deduping.
Concerns
- This is a user-visible UI/layout change, but the provided PR context does not include screenshots or a video showing the summary-view status badges and pair-icon placement end to end. For faster review, please upload screenshots or a video of the feature working end to end.
Verdict
Found: 0 critical, 0 important, 1 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
|
Re: screenshots — I don't have a runnable build environment for Warp here (no full Xcode/Metal toolchain in CI-equivalent state), so I can't capture live screenshots myself. The geometric correctness is covered by:
Happy to add a Playwright/visual snapshot if there's an existing harness for that — point me at it and I can wire it in. /oz-review |
|
I'm re-reviewing this pull request in response to a review 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 |
There was a problem hiding this comment.
Overview
This PR preserves agent conversation status through the vertical-tabs summary icon projection, routes Oz/CLI agent summary icons through the status-aware renderer, adjusts paired icon placement to avoid the badge slot, and adds regression coverage for status propagation and deduplication semantics.
Concerns
- This is a user-visible UI/layout change, but the PR description does not include screenshots or video showing summary-view agent badges for the affected ambient and non-ambient states. For faster review, please upload screenshots or a video of the feature working end to end.
Verdict
Found: 0 critical, 1 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
1 participant
Summary
Fixes #9868 — the vertical-tabs summary view dropped the agent status icon/badge for both Oz and CLI agents, making it impossible to tell from the tab list whether an agent was in progress, done, errored, cancelled, or blocked without opening the agent itself.
Root cause
Two cooperating bugs in
app/src/workspace/view/vertical_tabs.rs:SummaryPaneKind::OzAgent/CLIAgentdropped the upstream status when projecting from the sharedIconWithStatusVariantinsummary_pane_kind()— the..discard pattern at vertical_tabs.rs:2522,2525.ambient_agent_variant()— the function that hands the variant back torender_icon_with_status— only matchedis_ambient: trueand hardcodedstatus: None. Non-ambient agents fell through to a status-less inline render path entirely.So ambient agents had their status thrown away, and non-ambient agents never even reached the status-aware renderer.
Fix
status: Option<ConversationStatus>to both agent variants ofSummaryPaneKindand capture it insummary_pane_kind().ambient_agent_variant→summary_pane_kind_to_status_variant, match both ambient and non-ambient cases, and pass the captured status straight through toIconWithStatusVariant.render_summary_pane_kind_icon_circle. The remaining inline branch covers only non-agent pane kinds; agent kinds are markedunreachable!()so a future refactor can't silently regress the badge.select_summary_pane_kind_iconsvia a new helpersame_kind_ignoring_status. Two Oz agents with different statuses still collapse to one icon (status is metadata, not identity), with the first pane's status preserved on the surviving entry.Tests
status: None.vertical_tabs_tests.rs:OzAgentCLIAgentNoneso the caller falls back to inline rendering71 vertical-tabs tests pass; cargo check + clippy clean.
Test plan
cargo test -p warp --lib vertical_tabspassesOut of scope
The shared agent-icon helpers in
app/src/ui_components/agent_icon.rsand the cross-surface invariants inagent_icon_tests.rsare unchanged. The bug was strictly in the summary-view projection, not in the upstream variant derivation.Closes #9868.