(MOT-3964) feat: show live turn phases in console chat while waiting - #477
Conversation
The chat's thinking shimmer walked straight from submit to a generic "thinking…"/"dispatching <model>" line with no hint whether the message was accepted, queued, or waiting on the provider. Surface the real phases end to end: - console: translate the harness::send response (accepted/merged/queued) and harness::turn-started — both previously discarded — into a new turn-status stream event that drives the shimmer detail; server status_reason stays highest precedence. - session-manager: session::set-status stores status_reason on working (live phase) as well as error (failure cause) and re-emits status-changed when only the reason changes; no-op now requires status AND reason unchanged. Feature contract, golden schema, internals.md and tech-spec updated. - harness: turn_loop stamps "preparing context" at step start and "waiting for <model>" right before the router::chat RPC. Also restore the missing SelectOption.title field: PermissionModePicker passes title since #469 but the field never landed, so tsc -b (and the console worker build) fails on a clean checkout.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 41 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThe change propagates turn lifecycle phases from session and harness status updates through backend stream events into ChatView shimmer details. It also forwards optional titles through Select options and styles the new stream event in EventLog. ChangesTurn phase lifecycle
Select option metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TurnLoop
participant RealStream
participant Translator
participant ChatView
TurnLoop->>RealStream: Emit turn-started and send-resolved events
RealStream->>Translator: Forward stream source events
Translator->>ChatView: Emit turn-status phase
ChatView->>ChatView: Update shimmer detail
ChatView->>ChatView: Clear phase when content starts or stream ends
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
harness/src/turn_loop.rs (2)
169-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStatus update now unconditional, comment still says "First-step bookkeeping".
The
set_status(..., Some("preparing context"))call was pulled out of thepayload.step == 0block and now runs on every step, but the preceding comment header still describes it as first-step-only bookkeeping. This is now stale/misleading relative toemit_started/run_pre_turnbelow it, which remain first-step-only.🤖 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 `@harness/src/turn_loop.rs` around lines 169 - 173, Update the stale comment above the unconditional set_status call to describe per-step status preparation rather than first-step-only bookkeeping, while keeping the existing emit_started and run_pre_turn comments or guards explicitly tied to payload.step == 0.
385-391: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winInformational status RPC blocks the provider round-trip.
set_status(..., waiting_reason)is awaited synchronously right beforerouter.chat, on every step of every turn. Unlike the earlierappend/update_messagecalls (which must complete before the streamed entry exists), this write is purely informational for the UI phase line — nothing downstream depends on its completion before calling the provider. Awaiting it here adds this RPC's latency to time-to-first-token for every single generation step, which runs counter to the PR's goal of surfacing wait-phase visibility without slowing down the actual wait.Consider firing it without blocking the critical path (e.g. spawn it, since
sessionis alreadyCloneper its use at line 354).⚡ Proposed fix to stop blocking on the status update
- let waiting_reason = format!("waiting for {}", record.options.model); - let _ = session - .set_status(&record.session_id, "working", Some(&waiting_reason)) - .await; + let waiting_reason = format!("waiting for {}", record.options.model); + // Fire-and-forget: purely informational for the UI phase line, must not + // add latency to the provider round-trip. + let status_session = session.clone(); + let status_session_id = record.session_id.clone(); + tokio::spawn(async move { + let _ = status_session + .set_status(&status_session_id, "working", Some(&waiting_reason)) + .await; + });Please confirm whether
session::set-statusis a local in-process dispatch (negligible latency) or a networked call — if the former, this may be a non-issue in practice, but the current code adds an unconditional extra await on the hottest path in the harness regardless.🤖 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 `@harness/src/turn_loop.rs` around lines 385 - 391, Make the informational status update non-blocking before the provider round-trip: in the turn-loop code around waiting_reason and session.set_status, clone the session and spawn the set_status future instead of awaiting it synchronously, while preserving the existing "working" status and message. Ensure the spawned task handles or intentionally ignores the RPC result without delaying the subsequent router.chat call.
🤖 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 `@console/web/src/components/chat/ChatView.tsx`:
- Around line 1165-1169: Update the `turn-status` case in the ChatView event
handler to prevent stale `accepted`, `merged`, or `queued` updates from
overwriting an already-recorded `started` phase. Preserve `turnPhase` as
`started` once received, while retaining the existing behavior of mapping
`queued` to `null` when no start has occurred.
---
Nitpick comments:
In `@harness/src/turn_loop.rs`:
- Around line 169-173: Update the stale comment above the unconditional
set_status call to describe per-step status preparation rather than
first-step-only bookkeeping, while keeping the existing emit_started and
run_pre_turn comments or guards explicitly tied to payload.step == 0.
- Around line 385-391: Make the informational status update non-blocking before
the provider round-trip: in the turn-loop code around waiting_reason and
session.set_status, clone the session and spawn the set_status future instead of
awaiting it synchronously, while preserving the existing "working" status and
message. Ensure the spawned task handles or intentionally ignores the RPC result
without delaying the subsequent router.chat call.
🪄 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
Run ID: 6080ef1c-822a-4e6c-989f-81af05c9c8d1
📒 Files selected for processing (14)
console/web/src/components/chat/ChatView.tsxconsole/web/src/components/ui/Select.tsxconsole/web/src/lib/backend/real.tsconsole/web/src/lib/backend/translate.test.tsconsole/web/src/lib/backend/translate.tsconsole/web/src/lib/backend/types.tsconsole/web/src/stories/playground/EventLog.tsxharness/src/turn_loop.rssession-manager/architecture/internals.mdsession-manager/src/functions/set_status.rssession-manager/src/service.rssession-manager/tests/features/status.featuresession-manager/tests/golden/schemas/session.set-status.jsontech-specs/2026-06-agentic/session-manager.md
| case 'turn-status': { | ||
| // `queued` renders in the queued-messages strip, not the shimmer. | ||
| setTurnPhase(event.phase === 'queued' ? null : event.phase) | ||
| break | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant files and surrounding code paths.
git ls-files | rg '^(console/web/src/components/chat/ChatView\.tsx|.*real\.ts|.*turn_loop\.rs|.*send.*ts|.*status.*ts)$'
echo '--- ChatView.tsx ---'
wc -l console/web/src/components/chat/ChatView.tsx
sed -n '1120,1205p' console/web/src/components/chat/ChatView.tsx
echo '--- search turn-status handling ---'
rg -n "turn-status|setTurnPhase|phaseDetail|turnPhase" console/web/src/components/chat/ChatView.tsx console/web/src -g '!**/dist/**' -g '!**/build/**'
echo '--- real.ts candidates ---'
fd -a 'real.ts' console/web/src
for f in $(fd -a 'real.ts' console/web/src); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,260p' "$f" | rg -n "turn-status|send-resolved|onStarted|phase|harness::send|status_reason|queued|accepted|merged|started"
done
echo '--- turn_loop.rs candidates ---'
fd -a 'turn_loop.rs' .
for f in $(fd -a 'turn_loop.rs' .); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,260p' "$f" | rg -n "emit_started|started|queued|accepted|merged|turn-status|status_reason|phase"
doneRepository: iii-hq/workers
Length of output: 8073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- console/web/src/lib/backend/translate.ts ---'
sed -n '1,140p' console/web/src/lib/backend/translate.ts
echo '--- console/web/src/lib/backend/real.ts (relevant sections) ---'
sed -n '180,260p' console/web/src/lib/backend/real.ts
echo '...'
sed -n '300,420p' console/web/src/lib/backend/real.ts
echo '--- harness/src/turn_loop.rs (relevant sections) ---'
sed -n '150,240p' harness/src/turn_loop.rs
echo '...'
sed -n '180,240p' harness/src/turn_loop.rs
echo '--- ChatView phase rendering ---'
sed -n '1290,1325p' console/web/src/components/chat/ChatView.tsxRepository: iii-hq/workers
Length of output: 18958
Guard turnPhase against out-of-order updates. turn-status events come from two async paths: started from the live turn-start subscription, and accepted/merged/queued from the separate harness::send response. If the response lands after the turn has already started, it can overwrite started and show a stale pre-content phase. Keep started monotonic here.
🤖 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 `@console/web/src/components/chat/ChatView.tsx` around lines 1165 - 1169,
Update the `turn-status` case in the ChatView event handler to prevent stale
`accepted`, `merged`, or `queued` updates from overwriting an already-recorded
`started` phase. Preserve `turnPhase` as `started` once received, while
retaining the existing behavior of mapping `queued` to `null` when no start has
occurred.
What
Sending a message in the console chat used to show a bare "thinking…" (or "dispatching ") line until the first tokens arrived. The shimmer now walks the real lifecycle, live:
sending…harness::sendacceptedqueued — waiting to start…harness::sendmerged into a running turnadded to the running turn…preparing contextwaiting for <model>The two server-driven phases come from session state, so they survive reloads and show in every tab viewing the session.
How
harness::sendresponse andharness::turn-started— both already delivered to the console and previously discarded — translate to a newturn-statusstream event feeding the shimmer's detail line. Serverstatus_reasonremains highest precedence, and phase text is only trusted while the transcript still ends at the user's message (content arrives via session events, not stream events).session::set-statusstoresstatus_reasononworking(live phase detail) as well aserror(failure cause), and re-emitssession::status-changedwhen only the reason changes; the no-op now requires status AND stored reason unchanged. Contract updated instatus.feature(2 new scenarios), the golden schema,architecture/internals.md, andtech-specs/2026-06-agentic/session-manager.md.turn_loopstampspreparing contextat step start andwaiting for <model>immediately before therouter::chatRPC — the window users actually wait in.Also restores the missing
SelectOption.titlefield:PermissionModePickerpassestitlesince #469 but the field never landed, sotsc -b(and therefore the console worker'sbuild.rsweb build) fails on a clean checkout of main.Test plan
console/web:pnpm build(tsc -b + vite),pnpm test— 946 tests pass, incl. new translator cases forsend-resolved/turn-startedsession-manager:cargo test— 118 BDD scenarios pass, incl. 2 new (working-phase reason re-emit; same status+reason no-op); goldens regeneratedharness:cargo build,cargo fmt --checksending… → queued — waiting to start… → waiting for claude-sonnet-5 → (tokens)on a real Sonnet 5 sendFixes MOT-3964
Summary by CodeRabbit
New Features
Bug Fixes
Documentation