feat(status): stack concurrent session status bubbles - #20
Conversation
📝 WalkthroughWalkthroughThe live-status system now tracks multiple concurrent sessions, renders them as expandable stacked cards, and synchronizes dynamic content height with the Tauri status window. Window interaction, accessibility attributes, styling, and controller tests were updated accordingly. ChangesLive status stack
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant AgentEvents
participant LiveStatusController
participant StatusRenderer
participant TauriStatusWindow
AgentEvents->>LiveStatusController: Submit session event
LiveStatusController->>LiveStatusController: Update and order session statuses
LiveStatusController->>StatusRenderer: Emit status array
StatusRenderer->>StatusRenderer: Render stacked session cards
StatusRenderer->>TauriStatusWindow: Sync content height
TauriStatusWindow-->>StatusRenderer: Show or hide status window
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/status.ts (1)
77-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilent error swallowing in the render queue.
.catch(() => undefined)keeps the chain alive but hides failures frominvoke("sync_status_window", ...)orshow()/hide(). Aconsole.errorwould make window-sync regressions diagnosable without changing behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/status.ts` around lines 77 - 79, Update queueRender to retain the existing render queue recovery behavior while logging failures from render, including invoke("sync_status_window", ...) or show()/hide(), via console.error before resolving the catch handler. Do not otherwise change the render sequencing or error propagation behavior.src-tauri/src/lib.rs (1)
244-256: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHold the height lock only while reading/updating it.
The guard is kept across
set_size, monitor queries, andset_position, so concurrentsync_status_windowinvocations serialize on GUI IPC, and a panic anywhere in that span poisons the mutex permanently (every later sync then returns an error). Copying the value out and dropping the guard immediately also makes the explicitdropat Line 290 unnecessary.🔒 Narrow the critical section
- let state = app.state::<StatusWindowState>(); - let mut content_height_state = state.0.lock().map_err(|error| error.to_string())?; - if let Some(requested) = content_height.filter(|height| height.is_finite()) { - *content_height_state = requested.clamp(96.0, 4096.0); - } - let content_height = *content_height_state; + let content_height = { + let state = app.state::<StatusWindowState>(); + let mut stored = state.0.lock().map_err(|error| error.to_string())?; + if let Some(requested) = content_height.filter(|height| height.is_finite()) { + *stored = requested.clamp(96.0, 4096.0); + } + *stored + };and drop the trailing
drop(content_height_state);at Line 290.Also applies to: 289-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/lib.rs` around lines 244 - 256, Limit the content height mutex guard in sync_status_window to the block that filters, clamps, and updates the stored height, then copy the resulting value out before constructing the window size or performing GUI operations. Remove the trailing drop(content_height_state) since the guard will already be released, and preserve the existing error propagation for lock failures.src/live-status.ts (1)
104-122: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
titlesandlatestEventsare never pruned per session.
clear(sessionId)removes only thesessionsentry, so both maps keep growing one entry per session id for the lifetime of the process. RetaininglatestEventsis required by the "no resurrect" behavior, but consider an eviction cap (e.g. LRU of N recent sessions) so long-running instances don't accumulate indefinitely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/live-status.ts` around lines 104 - 122, Update clear(sessionId) to remove the session’s entry from titles and retain latestEvents only through a bounded recent-session cache, preserving the no-resurrect behavior. Add eviction for oldest latestEvents entries when the configured cap is exceeded, using existing ordering state such as updateOrder where appropriate. Ensure clear() and dispose() fully reset both maps and related eviction metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/status.html`:
- Line 10: Separate the toggle control semantics from the `#live-status` live
region: remove role="button", tabindex, aria-expanded, and the root aria-label
from `#live-status` so its per-session .status-title and .status-detail content
remains accessible. Add a distinct toggle control for expanding/collapsing
statuses, or ensure that control is disabled whenever
controller.getStatuses().length is less than 2.
In `@src/status.ts`:
- Line 93: Update loadConfig in the flow calling
render(controller.getStatuses()) to route the status rendering through
queueRender instead of invoking render directly. Await the queued operation so
config reloads remain serialized with all other render paths and preserve the
existing status update behavior.
In `@src/styles.css`:
- Around line 26-38: Update the status window interaction styling so transparent
padding and rounded-corner gaps do not capture pointer input after
`.status-page` becomes interactive. Scope pointer events to
`.status-card-surface`, preserving the status-card toggle behavior while
allowing clicks outside the visible surface to pass through.
- Line 27: Update the `#live-status` focus styling to provide a visible
:focus-visible indicator for keyboard users instead of leaving outline: none
without a replacement. Preserve the existing default appearance and ensure the
indicator is clearly distinguishable when the tabindex="0" element receives
keyboard focus.
---
Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 244-256: Limit the content height mutex guard in
sync_status_window to the block that filters, clamps, and updates the stored
height, then copy the resulting value out before constructing the window size or
performing GUI operations. Remove the trailing drop(content_height_state) since
the guard will already be released, and preserve the existing error propagation
for lock failures.
In `@src/live-status.ts`:
- Around line 104-122: Update clear(sessionId) to remove the session’s entry
from titles and retain latestEvents only through a bounded recent-session cache,
preserving the no-resurrect behavior. Add eviction for oldest latestEvents
entries when the configured cap is exceeded, using existing ordering state such
as updateOrder where appropriate. Ensure clear() and dispose() fully reset both
maps and related eviction metadata.
In `@src/status.ts`:
- Around line 77-79: Update queueRender to retain the existing render queue
recovery behavior while logging failures from render, including
invoke("sync_status_window", ...) or show()/hide(), via console.error before
resolving the catch handler. Do not otherwise change the render sequencing or
error propagation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac6ed7cc-1550-4f4a-9ec6-7aa8b9668446
📒 Files selected for processing (8)
src-tauri/src/lib.rssrc-tauri/tauri.conf.jsonsrc/live-status.test.tssrc/live-status.tssrc/main.tssrc/status.htmlsrc/status.tssrc/styles.css
| .status-page { margin: 0; width: 100vw; height: 100vh; overflow: hidden; user-select: none; } | ||
| #live-status { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); } | ||
| .status-card { | ||
| position: absolute; | ||
| right: 10px; | ||
| bottom: 12px; | ||
| left: 10px; | ||
| height: 72px; | ||
| transform: translateY(var(--stack-collapsed-y, 0)) scale(var(--stack-scale, 1)); | ||
| transform-origin: bottom center; | ||
| transition: transform 320ms cubic-bezier(.22, 1, .36, 1), filter 220ms ease; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Status window now swallows clicks in its transparent areas.
Dropping pointer-events: none from .status-page (paired with set_ignore_cursor_events(false) in src-tauri/src/lib.rs Line 242) makes the full 400×--content-height always-on-top window capture pointer input, including the transparent padding and the rounded-corner gaps around the cards. Anything underneath is no longer clickable there. Scoping interactivity to .status-card-surface would keep the toggle working without blocking the desktop.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles.css` around lines 26 - 38, Update the status window interaction
styling so transparent padding and rounded-corner gaps do not capture pointer
input after `.status-page` becomes interactive. Scope pointer events to
`.status-card-surface`, preserving the status-card toggle behavior while
allowing clicks outside the visible surface to pass through.
| .status-page { margin: 0; width: 100vw; height: 100vh; overflow: hidden; pointer-events: none; user-select: none; } | ||
| #live-status { width: 400px; height: 96px; padding: 8px 10px 12px; zoom: var(--bubble-scale, 1); } | ||
| .status-page { margin: 0; width: 100vw; height: 100vh; overflow: hidden; user-select: none; } | ||
| #live-status { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
outline: none removes the only focus indicator on a keyboard-operable element.
#live-status is tabindex="0" with an Enter/Space handler, so suppressing the outline without a :focus-visible replacement leaves keyboard users with no focus feedback.
⌨️ Add a visible focus style
-#live-status { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); }
+#live-status { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); }
+#live-status:focus-visible .status-card-surface { outline: 2px solid `#4a90d9`; outline-offset: 2px; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #live-status { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); } | |
| `#live-status` { position: absolute; bottom: 0; left: 0; width: 400px; height: var(--content-height, 96px); padding: 8px 10px 12px; outline: none; cursor: pointer; zoom: var(--bubble-scale, 1); } | |
| `#live-status`:focus-visible .status-card-surface { outline: 2px solid `#4a90d9`; outline-offset: 2px; } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles.css` at line 27, Update the `#live-status` focus styling to provide
a visible :focus-visible indicator for keyboard users instead of leaving
outline: none without a replacement. Preserve the existing default appearance
and ensure the indicator is clearly distinguishable when the tabindex="0"
element receives keyboard focus.
LRainner
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/status.ts (1)
42-66: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the list DOM order newest-first.
Existing cards are visually restacked but never moved in
#status-stack, so itsrole="list"order remains creation order after a session receives a newer update. Append each card during this loop to align assistive-technology order with the visual stack.Proposed fix
card.querySelector<HTMLElement>(".status-title")!.textContent = config.codex.showTaskSummary ? status.title : "Codex"; card.querySelector<HTMLElement>(".status-detail")!.textContent = status.detail; + stack.append(card); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/status.ts` around lines 42 - 66, Update the status-card loop around statuses.forEach so every processed card is appended to stack after its content and styling are updated, including existing cards. This must reorder `#status-stack` DOM children newest-first to match the visual stack and assistive-technology list order while preserving the existing card creation and update behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/status.ts`:
- Around line 42-66: Update the status-card loop around statuses.forEach so
every processed card is appended to stack after its content and styling are
updated, including existing cards. This must reorder `#status-stack` DOM children
newest-first to match the visual stack and assistive-technology list order while
preserving the existing card creation and update behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6ac6dbe-a67c-403e-91bf-215cd1805f65
📒 Files selected for processing (3)
src/status.htmlsrc/status.tssrc/styles.css
1 participant
Summary
Testing
Summary by CodeRabbit