Repository navigation
Conversation
The session screen could paint the loading skeleton and a Connection lost line in the same frame, with the last skeleton bubble sliced by the row, and the load-error card restated the same failure again beside Send. A lost agent transport now owns the body: the SDK's own connection-lost code resolves to a retryable terminal failure with the existing connection-trouble copy and Retry, whatever type the status indicator carries, and an exhausted app-wide transport leg does the same while the open still has nothing to show. While the body states a failure, the fixed footer row and the composer reason stay silent, so one failure is stated once with the Retry that resolves it. The skeleton's bubbles can shrink instead of overflowing a body shorter than they are.
|
bot: Device proof on the Android emulator against the local dev stack, with the session-ingest worker stopped so the session open fails. Before the change the load-error card and the composer reason state the same failure twice; after the change the card states it once with its Retry and no footer row. Design gate contact sheet for the four standard cells (0 errors, 4 pre-existing touch-target warnings). |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit replaces the fixed-width Assumptions: Files Reviewed (4 files)
Previous Review Summaries (8 snapshots, latest commit 81d14cf)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 81d14cf)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit paints the loading value slot as one bar; its only new concern is that the fixed Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Previous review (commit 630f55d)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last review removes the Assumptions: Files Reviewed (1 file)
Previous review (commit 924a3c1)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit cd4741e)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
The earlier Previous review (commit 14f4a7f)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit Assumptions: Files Reviewed (6 files)
Scope note: the incremental commit also adds the pill's dash slots and Previous review (commit bd993bf)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit Assumptions: Files Reviewed (3 files)
Resolved since the previous review: the shared Previous review (commit 1cd5ee4)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe skeleton and app-wide Retry fixes land correctly, but the new Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 06e4594)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe session-state consolidation is sound, but the skeleton Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Reviewed by deepseek-flash-latest · Input: 66.5K · Output: 12.8K · Cached: 688.9K Review guidance: REVIEW.md from base branch |
Two review findings on the lost-connection state. The skeleton rows were the flex items that overflowed, and each bubble sat in an auto-height wrapper, so flexShrink on the bubble could not reduce anything. The bubbles are now the list's own flex items, each with its margin and alignment, and the container clips, so a body shorter than the placeholder compresses the shapes instead of letting one be sliced. A spent reconnect budget is only cleared by resetting the shared transport leg, so the terminal error now carries which recovery its Retry runs: reopening the session for a failure the session reported, and the connection retry for a lost leg.
|
bot: Re-ran the design gate at the new head and added the placeholder capture.
|
The recovery a lost transport's Retry runs depends on which transport it was: a session whose own transport dropped reopens, while the shared app-wide leg needs its spent reconnect budget reset. Pass the action per branch instead of returning the connection retry for both.
While a session transcript was still loading, the screen painted an editable Message composer at the bottom. The open's transport has not answered in that window, so a read-only session replaced that composer with the read-only footer as soon as it resolved: one screen showing writing controls it did not have, then a jump to the state it did have. The bottom slot is now the session's own. Until the transcript resolves — and while no failure, agent status or pending ask means the session has already spoken — the slot holds the read-only footer's area, at the footer's own geometry, so the footer lands where the placeholder was. The composer keeps its place behind blocking cards and failures. The header context pill showed a bare ring in that same window, then grew its value when usage arrived. A session that has not reported usage now keeps both value slots, marked with the dash, so the pill fills the slots it already showed instead of changing shape late.
The type names this module's own selector output; nothing outside it reads it, and the repository's unused-export check rejects it while it is exported.
|
bot: The automatic design review hands this pull request to the owner at head 630f55d.
|
|
bot: Design gate at
Measured in the same run from the cell dumps: the loading pill is 96.4 dp in every cell (a 28.2 dp ring and one 38.5 dp bar), the settled pill with a reported cost is 97.1 dp, and the merge base's loading frame is a bare 51.0 dp ring. |
|
bot: Design gate re-run for head
The loading value row now takes the settled row's own width instead of a fixed one. Measured in the same run: the row is 40.0 dp, the bar fills it at |









Evidence
Evidence: demonstrated — on emulator-5554 at the default text size and density against the local dev stack, in the fixture session
ses_00000000000LiveCliStream01with its transcript stalled: the body now shows the retryable card with its Retry instead of the skeleton and aConnection lostline in the same frame, the reserved read-only footer paints its note line and button bar in the one gray that stands off the footer's own fill (46 px and 126 px, the footer's own 17.5 dp line box and 48 dp action floor), and the header pill's value row holds the width its values take — 40.0 dp, one bar filling it while the open loads, the same row once it settles — so the pill is 97.9 dp while loading and 97.1 dp settled, where the merge base2b4f203d3cpaints a bare 51.0 dp ring in the loading frame.Problem
The session screen had no single owner for a lost agent transport:
resolveSessionTerminalErroronly treated a status indicator of typeerroras the body's failure, and the SDK's own lost-transport code (agent-connection-lost) carried catalog copy instead of a class, so the classifier called itunknown: copy written for an unknown load failure and no Retry.Connection lostin the same frame — a loading state and a failure on one screen.The session could not be loaded. Retry first.beside Send), so one failure was stated twice with one meaning.Five defects of the same window survived that change, and the design review named them across its successive reviews:
mutedequals thesecondaryfill of the footer the area stands in for, so the two bars painted the container's own colour.Change
apps/mobile/src/components/agents/session-terminal-error.tsCouldn't load this session, messageConnection trouble. Please retry in a moment., Retry. No new copy; both strings already exist in every mobile locale.isRetryableTerminalFailureanswers true for that code, so the other surfaces agree.connectionLostfact for the app-wide transport leg, which counts only while the open still has nothing to show.apps/mobile/src/components/agents/session-detail-content.tsxapps/mobile/src/components/agents/session-detail-skeleton.tsxSessionReadOnlyFooterPlaceholderholds the read-only footer's area — its container, a line and a button bar at the footer's own child sizes — so the footer lands on its own geometry instead of replacing a composer.bg-muted-soft, the one gray that differs from thebg-secondaryfill the area reserves, so the reserved area reads as loading instead of as an empty block.apps/mobile/src/components/agents/context-usage-display.tsloading, which the screen paints as one bar over that row; after the change a dash beside a bar no longer reads as half-painted content.apps/mobile/src/components/agents/session-context-metrics.tsxbg-muted-softbar fills a full-bleed box over it, so the placeholder takes the settled row's own text metrics at any font scale instead of a width of its own.session-terminal-error.test.tscovers the lost code on every indicator type and the app-wide leg, plus the loaded-transcript exemption;session-detail-content.test.tscovers the reported frame (skeleton replaced by one retryable card, no footer line, no send reason), the rewritten footer expectations, and the bottom slot (held area with no composer, then the read-only footer on that same frame);context-usage-display.test.tsandsession-context-metrics.test.tscover the dash-marked slots and the single loading bar that replaces the dash while the row loads;session-detail-skeleton.test.tspins the reserved footer's geometry and the fill that stands off it, and fails if a bar goes back to the container's own colour.Verification
npx vitest run src/components/agents/inapps/mobile: 294 files, 5258 tests pass.pnpm --filter kilo-app run typecheck: clean. oxlint (apps/mobile/.oxlintrc.json) and oxfmt: clean on every changed file.ses_00000000000LiveCliStream01:2b4f203d3c: the open's loading frame, in all four design-gate cells, paints the Message composer with mic and send and a 51.0 dp context ring; the session then settles read-only with theThis is a read-only sessionfooter and Continue, and the composer is replaced by it.14f4a7f594: the same loading frame paints the reserved footer area (no composer) and a 78.1 dp pill with both value slots, but the reserved footer's two bars and the pill's second dash are indistinguishable from their own surfaces.630f55d95: with the API held for 20 s so the loading window can be captured frame by frame, the reserved footer paints two bars — 46 px of note line and 126 px of button floor — and the pill holds a 73 px cost bar at the baseline of its dash. With the API refused, the same screen settles into the retryable failure with the dash in that same slot, and the pill keeps its width (97.1 dp / 97.5 dp / 97.9 dp; frame measurements in## Evidence).2581362723: the loading frame paints no dash. In all four cells the value row is 40.0 dp and the bar fills it —[346.3, 93.0, 40.0, 7.2]inside the row at[346.3, 89.5, 40.0, 14.1], vertically centred, mirrored to[25.5, 93.0]in the Arabic cell. The pill is 97.9 dp while loading and 97.1 dp once the open settles with its cost (—8.4 dp at[910, 235],$0.4027.4 dp at[941, 235]), where the merge base's same frame is a 51.0 dp ring.Connection lostline; after it, neither — the retryable card with its Retry instead. Covered for indicator typeserror,info,progressandwarning, and for an exhausted app-wide transport leg.2581362723, on the agent chat session route for the dev fixture session: every cell clean (this run is the head run uploaded for this commit).2b4f203d3cand reloading the app: 4 cells, 0 errors and its four pre-existingVoiceInputButtontouch-target warnings, which this head does not paint because its loading frame carries no composer.Limitations
The exact dark cell from the reporting run also needs the socket to close while the transcript fetch is still pending. In the local stack both ride port 8800, so a stopped worker fails the fetch first (the error-atom state) and a dropped port hangs both: that split cannot be staged on this VM. The indicator path is therefore proved in the mounted suite, and the device run covers the loading, the read-only and the load-error states.
The design gate's default observation window closes the cell before this fixture's transcript settles, so the runs above show either the first painted frame or the loading frame; the settled read-only frame was measured on the same device with a longer observation window (
--observe-ms 7000), which also reports the transcript's own scroll-in as layout-shift errors on the merge base and on this branch alike. The loading frame in the runs above is the gate's own capture right after the link opens, while this fixture's transcript is still unresolved; an earlier head held that window open with a delayed API instead, and that capture also showed the app's own slow-load card once the delay outlasted its threshold.Production prerequisites
None.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Both: retryable error for a lost transport AND a non-clipping skeleton for ordinary loading
Confidence: 0.55 · Precedent: Fix the root cause and the class of the issue: "if anything repeats the same mistake over and over again, fix the class" (00eb2a1d3e4b)
Answer: Yes: transient class, existing 'Connection trouble' copy, Retry present
Confidence: 0.80 · Precedent: Reuse as much as possible of the current implementation: "reuse as much as possible from the current impl" (0a2b68365a4b); plans cover the retryable-error state (df491946927a)
Answer: Suppress both while the body states the failure
Confidence: 0.70 · Precedent: Remove redundant copy and keep surfaces consistent: "go make the public dashboard a delight to use" (0e57ea66f95b); best-in-class mobile UX (00eb2a1d3e4b)
Answer: Yes, while the transcript is still loading
Confidence: 0.60 · Precedent: Fix the class on every affected variant: "fix this for all shared services model pickers on all surfaces" (75f8833fe8c1)
Answer: Fix: hide the dash while loading, one bar spanning the 40.2 dp value row
Confidence: 0.85 · Precedent: UX rule 'Stop mobile design and UX defects before they land, with a design gate': session 6341f41c09ab, owner wrote 'I very often have design/ux fixes sessions - I don't want to have those'. Also, pushing after an approval is fine: session df491946927a, 'you can push after an approval without issue'.