feat(review): surface loading tasks under Ready (no ✓) and open their window on click - #576
Merged
Conversation
… window on click A review-routed task that begins loading onto a review slot now appears under "Ready for Review" immediately, rendered with a ▶ (not the ✓, which stays gated on the serving marker) and no location. Previously a loading task lived under "Queued for Review" until the load finished, so the row vanished from Ready during the whole checkout/install/build and its ✓ + placement only appeared once serving. Clicking a review task now branches on its lifecycle sub-state instead of raw section membership: - pending-load (not on a slot) → open the slot-selection load prompt (unchanged) - loading or serving (already on a slot) → open/attach its review interface directly, no dialog Key decisions (not asked about): - Moved the Loading arm of `split_review_sections` from `queued` to `ready`; the ✓ was already gated on `ReviewState::Serving` in `Row::decoration`, so a Loading row in the Ready section paints ▶ with no ✓ for free — no renderer change to the checkmark itself. - Added `App::review_substate`, which sources the sub-state from the in-flight `review_job` first (authoritative Loading before the next 750ms refresh records the slot assignment, so a double-click mid-load opens the booting window rather than re-prompting) then the section entries' own `state`. Unknown ids fall back to Pending so a stale click re-prompts rather than opening a nonexistent window. Kept the existing "a review load is already in progress…" guard in `open_review_load_prompt` intact as the belt-and-braces backstop. - Reused `open_ready_review` (not the private `open_loaded_review_interface`) for the loading/serving click branch: it was already the Ready-click entry point and handles the NeedsLaunch/Loading/RemoteFallback outcomes. Tests: reworked `review_tasks_split_by_serving_state` and the stale-marker test for the new placement; added `review_substate_drives_the_click_branch` (covers the loading-click-opens-window no-prompt branch, incl. the in-flight-job override); extended the sidebar render test with a loading row asserting ▶ under Ready with no ✓ and no location. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
jlong
deleted the
jlong/review-sidebar-show-a-loading-task-under-ready-for-review-no-checkmark-until-serving-and-open-its-window-on-click-instead-of-re-prompting-to-load
branch
August 17, 2026 13:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task
The review sidebar has two sections, "Ready for Review" and "Queued for Review" (
crates/shelbi-tui/src/app.rs:500and:519), and the Ready rows carry a✓(crates/shelbi-tui/src/sidebar.rs). A review-routed task moves through three sub-states: pending-load (in review, no slot yet), loading (on a slot, installing/building), serving (up, ready to inspect). Clicking a sidebar task callsopen_review_load_prompt(app.rs:706→:724), which shows the slot-selection dialog and starts a load.Two problems while a task is loading: it isn't surfaced under "Ready for Review" until the load finishes, and clicking it re-opens the load prompt even though it's already loading onto a slot.
Current Behavior
✓and the Ready placement appear together only once it's serving.open_review_load_promptand shows the "pick a review slot / load" dialog. For a loading task this is wrong: it's already on a slot being built.Expected Behavior
✓. The✓appears only once the slot is serving (ready to inspect). (So "Ready for Review" contains loading rows without a checkmark and serving rows with a checkmark.)open_loaded_review_interfacepath,app.rs:877), with no load prompt.Technical Details
app.rs:706) on the task's review sub-state: pending-load →open_review_load_prompt; loading or serving → open the review interface directly (no dialog). Source the sub-state from the assigned review slot's workspace state / the in-flight load job (poll_review_load,WorkspaceState::Servinginactivity.rs), not just section membership.app.rs:500/:519): a task that is loading belongs in "Ready for Review"; gate the✓on serving, not merely on being in the Ready section (sidebar.rscheckmark render).app.rs:726) intact.Acceptance Criteria
✓.✓on that row appears only once the slot is serving (ready to inspect).cargo build/test/clippygreen; sidebar/app tests cover the loading-row-without-checkmark placement and the loading-click-opens-window (no-prompt) branch.Auto-opened by Shelbi — review at: /Users/jlong/.shelbi/projects/shelbi/tasks/review-sidebar-show-a-loading-task-under-ready-for-review-no-checkmark-until-serving-and-open-its-window-on-click-instead-of-re-prompting-to-load.md