feat(web): group live tool activity - #7152
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the changed chat timeline UI (ChangedFilesTree.tsx, MessagesTimeline.tsx, index.css) for shared-primitive usage, Tailwind/CSS ownership and preserved interaction semantics. The AgentSpawnCtaRow migration to Button and the new mask/animation utilities living in index.css both look right. Three concrete issues below: one order-dependent cascade conflict on the new live-activity overlay, one responsive-mode inconsistency in the rebuilt subagent card, and one lost failure affordance.
Posted via Macroscope — UI Consistency
| return ( | ||
| <div className="relative min-h-6 w-fit max-w-full min-w-0 overflow-hidden rounded-md text-sm leading-relaxed"> | ||
| <LiveActivityContent label={label} iconName={iconName} /> | ||
| <div aria-hidden className="live-activity-focus pointer-events-none absolute inset-0"> |
There was a problem hiding this comment.
inset-0 compiles to the inset shorthand and sits in the same utilities layer (same specificity) as live-activity-focus, which owns left, right and width. Whichever Tailwind emits last wins, so the overlay can lose its -4.5rem left offset while live-activity-focus-aligned still applies margin-left: 4.5rem, misaligning the highlighted copy from the base label. Suggest limiting the JSX utility to the axis the CSS utility does not own so the outcome no longer depends on emission order.
| <div aria-hidden className="live-activity-focus pointer-events-none absolute inset-0"> | |
| <div aria-hidden className="live-activity-focus pointer-events-none absolute inset-y-0"> |
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Needs human review 5 blocking correctness issues found. This PR introduces a new feature for grouping live tool activity, adding new row types, summarization logic, and CSS animations. New feature capabilities introducing user-facing behavior changes warrant human review. Additionally, multiple Medium-severity findings remain unresolved. You can customize Macroscope's approvability policy. Learn more. |
34a0b63 to
40c4a79
Compare
40c4a79 to
967e2ea
Compare
967e2ea to
63a901e
Compare
There was a problem hiding this comment.
Reviewed the timeline/changed-files refresh for shared-primitive use, Tailwind ownership, and interaction/accessibility preservation. The three findings from the previous run are addressed (the shimmer overlay no longer sets inset-0 against the CSS utility's own left/width, the subagent card now uses @container/agent-group queries, and the failure icon is exposed via role="img"). Three new issues introduced by this diff are below.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Reviewed the timeline/changed-files refresh for shared-primitive use, Tailwind ownership, and interaction/accessibility preservation. The three findings from the previous run are addressed (the shimmer overlay no longer sets inset-0 against the CSS utility's own left/width, the subagent card now uses @container/agent-group queries, and the failure icon is exposed via role="img"). Three new issues introduced by this diff are below.
Posted via Macroscope — UI Consistency
63a901e to
32e00fa
Compare
32e00fa to
d2095f2
Compare
d2095f2 to
2cfabfe
Compare
|
UI consistency review — inline comments could not be attached to the diff, so the four findings are below. 1. if (detailQuery.isPending && !detail) {
return <PullRequestDetailGhost />;
}This early return replaces the whole panel while The same move also drops the per-tab ghost shape that the deleted comment described — Suggest keeping the ghost inside the content region (where 2. Swapping the manual
If the goal is only to keep the drawer-mode and panel-mode terminal sidebars (both on 3. The Two adjacent controls with the same "toggle that can be unavailable and must explain why" contract now compose two different ways. Suggest giving the terminal toggle the same wrapper, or extracting one small local helper both use, so the availability tooltip behaves the same for either panel. 4.
That Suggest folding the cap into Posted via Macroscope — UI Consistency |
There was a problem hiding this comment.
UI consistency review of the changed timeline rows. Previously flagged items (agent-card geometry/status visibility, live-row failure announcement, expanded-group landmarks, working-row spacing/token, count-toggle metrics, duplicate selectable shimmer label, icon-only agent action) are resolved at this head. One introduced misalignment remains, noted inline.
Posted via Macroscope — UI Consistency
| return ( | ||
| <div | ||
| className={cn( | ||
| "flex min-h-6 min-w-0 items-center gap-1.5 px-1 py-0.5", |
There was a problem hiding this comment.
The new live rows inset at px-1, but every settled row in the same activity column insets at px-0.5: the tool-group summary button (line 1533) and PlainWorkEntryRow (line 2646, whose WorkGroupSection wrapper is -mx-1 px-1, i.e. net zero). So the running tool's icon and label sit 2px right of the identical call once it settles into Used 1 tool, and expanding the live row indents its own children 2px left of the header that discloses them — the column this PR unifies to size-6/size-4/text-sm ends up with two x-origins.
Suggest matching the column's px-0.5 here; the shimmer geometry comes from the live-activity-focus* utilities, so it is unaffected. The gap-1.5 px-1 py-0.5 assertion in MessagesTimeline.test.tsx needs the same update.
| "flex min-h-6 min-w-0 items-center gap-1.5 px-1 py-0.5", | |
| "flex min-h-6 min-w-0 items-center gap-1.5 px-0.5 py-0.5", |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review: one finding on the activity column's type scale and row target for expanded tool-group children.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review of the timeline activity column.
One new finding, posted inline. Several findings from earlier runs on this branch are still present at this head and are not re-commented (see the check details): expanded tool-group children still render at text-[12px] leading-5 / py-0 while every header in the column moved to text-sm leading-relaxed + min-h-6; the live row still insets at px-1 against px-0.5 for the settled rows it turns into; toolGroupSummaryIconName still maps tone-only tool entries to wrench where workEntryIconName gives zap/bot; and the working / turn-fold headers still resolve text-muted-foreground while every activity line they introduce resolves text-secondary-label.
Posted via Macroscope — UI Consistency
| hasFailure: visibleGroupedEntries.some((entry) => | ||
| workEntryDisplayIndicatesToolFailure(entry), |
There was a problem hiding this comment.
hasFailure is only ever rendered by the summary branch of WorkGroupToggleTimelineRow (destructive icon plus the …, tool call failed accessible name, MessagesTimeline.tsx lines 1538-1550). This overflow branch produces the chevron variant (summary: null), which ignores the field entirely, so two collapsed disclosures of the same batch disagree: an all-tool group that hides a failed call announces the failure, while a mixed group that hides the same call stays silent.
The value is also computed over visibleGroupedEntries rather than hiddenEntries, so it would attribute a still-visible failure (which already renders its own X on its own row) to the collapsed set. Suggest scoping it to hiddenEntries and rendering the same failure affordance in the chevron branch, or dropping the field for this variant so the row type only carries state its renderer uses.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review: one finding — the active-turn live tool batch does not apply the error-row exclusion that the settled grouping path applies, so error/denial rows lose their dedicated row treatment while a turn is running.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review of the live tool-activity column: two findings, both on rows introduced by this PR.
Posted via Macroscope — UI Consistency
| <span | ||
| className={cn( | ||
| "flex size-6 shrink-0 items-center justify-center", | ||
| row.hasFailure ? "text-destructive" : "text-icon-muted", | ||
| )} | ||
| role={row.hasFailure ? "img" : undefined} | ||
| aria-label={row.hasFailure ? "Hidden work includes a failure" : undefined} | ||
| > | ||
| {row.hasFailure ? ( | ||
| <WorkEntryIconSvg name="x" className="size-4 shrink-0 stroke-[1.8] opacity-70" /> | ||
| ) : ( | ||
| <ChevronDownIcon | ||
| className={cn( | ||
| "size-4 shrink-0 opacity-70 transition-transform duration-200", | ||
| row.expanded && "rotate-180", | ||
| )} | ||
| /> | ||
| )} | ||
| </span> |
There was a problem hiding this comment.
In this branch the icon slot is the disclosure affordance (the chevron rotates on expand), so swapping it for the failure X drops the expand/collapse indicator — and the state it announces goes stale once the row is open. hasFailure is derived from hiddenEntries regardless of expanded (MessagesTimeline.logic.ts line 902), so an expanded overflow group renders a red X named Hidden work includes a failure while nothing is hidden and every failing row below already carries its own X. The summary branch above is different: there the icon slot is an identity glyph, so replacing it is coherent.
Suggest showing the failure glyph only while collapsed, keeping the chevron as the disclosure indicator when the group is open.
| <span | |
| className={cn( | |
| "flex size-6 shrink-0 items-center justify-center", | |
| row.hasFailure ? "text-destructive" : "text-icon-muted", | |
| )} | |
| role={row.hasFailure ? "img" : undefined} | |
| aria-label={row.hasFailure ? "Hidden work includes a failure" : undefined} | |
| > | |
| {row.hasFailure ? ( | |
| <WorkEntryIconSvg name="x" className="size-4 shrink-0 stroke-[1.8] opacity-70" /> | |
| ) : ( | |
| <ChevronDownIcon | |
| className={cn( | |
| "size-4 shrink-0 opacity-70 transition-transform duration-200", | |
| row.expanded && "rotate-180", | |
| )} | |
| /> | |
| )} | |
| </span> | |
| <span | |
| className={cn( | |
| "flex size-6 shrink-0 items-center justify-center", | |
| row.hasFailure && !row.expanded ? "text-destructive" : "text-icon-muted", | |
| )} | |
| role={row.hasFailure && !row.expanded ? "img" : undefined} | |
| aria-label={row.hasFailure && !row.expanded ? "Hidden work includes a failure" : undefined} | |
| > | |
| {row.hasFailure && !row.expanded ? ( | |
| <WorkEntryIconSvg name="x" className="size-4 shrink-0 stroke-[1.8] opacity-70" /> | |
| ) : ( | |
| <ChevronDownIcon | |
| className={cn( | |
| "size-4 shrink-0 opacity-70 transition-transform duration-200", | |
| row.expanded && "rotate-180", | |
| )} | |
| /> | |
| )} | |
| </span> |
Posted via Macroscope — UI Consistency
| className={cn("block size-4 shrink-0 stroke-[1.8]", !highlighted && "opacity-70")} | ||
| /> | ||
| </span> | ||
| ) : null} |
There was a problem hiding this comment.
ThinkingActivityRow passes no iconName, and the icon slot is only emitted when an icon exists, so Thinking renders with no gutter while every other line in this column reserves one: the work-live row (line 1492), the tool-group summary button (line 1540) and PlainWorkEntryRow (line 2668) all render a size-6 slot before the label. showThinking flips to false the moment the first in-progress tool call lands in the same slot under the working header, so the label visibly jumps ~30px right on the first tool call of a turn.
This PR already establishes the fix pattern one level down — expanded child rows keep the icon slot and mark it invisible instead of dropping it (line 2669). Suggest reserving the gutter here too (the not.toContain('aria-hidden="true" class="size-6 shrink-0"') assertion in MessagesTimeline.test.tsx would need to move to whatever spacer class is chosen).
| ) : null} | |
| ) : ( | |
| <span aria-hidden className="flex size-6 shrink-0" /> | |
| )} |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review of the timeline activity column. Most of the earlier findings are resolved at this head (icon classifiers now agree across live/summary/expanded rows, the failure heuristic is display-only in every renderer, the expanded children moved to the text-sm scale, the working header matches the turn fold, the agent card matches the changed-files card shell, and announceFailure is gated). One remaining interaction gap is inline below.
Posted via Macroscope — UI Consistency
| return ( | ||
| <button | ||
| type="button" | ||
| className="group/live-work flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" |
There was a problem hiding this comment.
The hit area here is the full column (w-full button) but the visible box is the w-fit LiveActivityRow inside it, and the button carries no hover state — so the empty space to the right of Running psql toggles the group with nothing indicating it is interactive. Every other clickable row in this column tints on hover: the tool-group summary button (line 1535), the overflow toggle (line 1569) and PlainWorkEntryRow (line 2664). The summary row is what this same batch becomes the moment the turn settles, so the affordance appears out of nowhere on settle.
Suggest giving the live button the same hover treatment (or dropping w-full so the hit area matches the visible box). Note the live-row test's not.toContain("hover:bg-accent/20") assertion is there to prove no toggle row rendered — tighten it to a work-toggle check if you add the tint. While here, group/live-work (and group/tool-group on line 1535) has no group-*/… consumer anywhere, so the marker can go.
| className="group/live-work flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" | |
| className="flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left transition-colors duration-150 hover:bg-accent/20 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" |
Posted via Macroscope — UI Consistency
| if (input.isWorking && activeTurnHeaderIndex === input.timelineEntries.length) { | ||
| appendWorkingRow(); | ||
| } | ||
| if (input.isWorking) appendActiveWorkRows(); |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.logic.ts:972
The consolidated work-live row is always rendered after the entire timeline, so tool activity appears after later assistant output, plans, or messages even when those entries follow the tool chronologically. Because appendActiveWorkRows() runs only at line 972, insert the row at activeWorkAnchor during the loop and remove the unconditional end append.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/chat/MessagesTimeline.logic.ts around line 972:
The consolidated `work-live` row is always rendered after the entire timeline, so tool activity appears after later assistant output, plans, or messages even when those entries follow the tool chronologically. Because `appendActiveWorkRows()` runs only at line 972, insert the row at `activeWorkAnchor` during the loop and remove the unconditional end append.
There was a problem hiding this comment.
UI consistency review — one finding on the expandable tool row's lost disclosure affordance.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — one finding on the reworked activity rows.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — two shared-treatment findings in the composer drawer work. Both are ownership/consistency suggestions rather than functional breaks.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
The compact 11px drawer action now has two owners: this inline string and APPROVAL_ACTION_CLASS_NAME in chat/ComposerPendingApprovalActions.tsx:14. Both render size="micro" variant="ghost-muted" at 11px in a composer-attached drawer, so the banner action and the approval actions can drift apart even though they sit in the same stack.
sm:text-[11px] is also inert here: micro sets text-xs with no sm: counterpart (ui/button.tsx:32), so the base override already applies at every width.
Suggest expressing this once — a shared exported class constant or a named button size/variant — and leaving only contextual classes at the call site.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — two findings on the composer's new attached-drawer controls.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — one finding on the expanded tool-group child rows.
Posted via Macroscope — UI Consistency
| : workLogEntryIsToolLike(workEntry) | ||
| ? "text-secondary-label" | ||
| : "text-foreground/80"; | ||
| const showEntryIcon = !isExpandedToolGroupEntry || showWarningIndicator || showFailedIndicator; |
There was a problem hiding this comment.
Blanking the icon slot for expanded tool-group children removes the last action indicator these rows have. Line 2615 now collapses the row text to workEntryPreview(...) ?? toolWorkEntryHeading(...), so a child that has a preview no longer renders its verb (Read file, Ran command) as text — and with the slot invisible/aria-hidden an expanded child renders only a bare path, command string, or raw detail.
The same entry keeps its icon when it is revealed by the other disclosure in this column: the overflow +N previous tool calls toggle pushes its rows with isExpandedToolGroupEntry: false (MessagesTimeline.logic.ts line 886), so whether a tool row shows an action glyph depends on which toggle revealed it. It hurts most when summaryKind is mixed (Read 2 files and ran 1 command, header icon hammer): the header describes the batch, not the rows, so nothing tells a read child from a command child.
Suggest keeping the per-entry icon for expanded children — at least when the group summary is mixed — instead of hiding the slot; the slot geometry is already size-6 in both states, so alignment under the header is unaffected.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review: two findings on the composer controls introduced here.
Posted via Macroscope — UI Consistency
|
UI consistency review — inline comments could not be attached to the diff, so the two findings are below. 1.
-const APPROVAL_ACTION_CLASS_NAME = "font-normal text-[11px] sm:text-[11px]";
+const APPROVAL_ACTION_CLASS_NAME = "font-normal text-[11px] max-sm:h-7 max-sm:px-2";2. This row and the stash drawer row ( - "cursor-pointer select-none gap-3 rounded-lg px-3 py-2! hover:bg-transparent hover:text-inherit data-highlighted:bg-transparent data-highlighted:text-inherit",
+ "cursor-pointer select-none gap-3 rounded-lg px-3 py-1.5 hover:bg-transparent hover:text-inherit data-highlighted:bg-transparent data-highlighted:text-inherit",Posted via Macroscope — UI Consistency |
|
UI consistency review — one finding could not be attached inline, so it is below.
"cursor-pointer select-none gap-3 rounded-lg px-3 py-2! hover:bg-transparent hover:text-inherit data-highlighted:bg-transparent data-highlighted:text-inherit",This row and the stash drawer's row ( Suggest expressing this once — a shared class constant, or a Posted via Macroscope — UI Consistency |
|
UI consistency review — inline comments could not be attached to the diff, so the two findings are below. 1. variant="ghost-muted"
className={`${APPROVAL_ACTION_CLASS_NAME} text-destructive-foreground [:hover,[data-pressed]]:text-destructive-foreground`}Decline and Approve pick <Button
size="micro"
- variant="ghost-muted"
- className={`${APPROVAL_ACTION_CLASS_NAME} text-destructive-foreground [:hover,[data-pressed]]:text-destructive-foreground`}
+ variant="ghost"
+ className={`${APPROVAL_ACTION_CLASS_NAME} text-destructive-foreground`}Approve (lines 50-53) is the same case: 2. props.menuOpen && "invisible pointer-events-none",Making the tab It also strands the guard added in Suggest either keeping the tab hit-testable above the open drawer so a second click collapses it (matching the Posted via Macroscope — UI Consistency |
There was a problem hiding this comment.
UI consistency review — one new finding on the live tool row; see the inline comment. Findings already posted on earlier commits of this branch are not repeated.
Posted via Macroscope — UI Consistency
| return ( | ||
| <button | ||
| type="button" | ||
| className="group/live-work flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" |
There was a problem hiding this comment.
This button and the settled tool-group summary button (line 1535) are the same disclosure — both call ctx.onToggleWorkGroup with the same work-group:tool:<turn>:<callId> id and reveal the same children — but only the settled one gets pointer feedback (transition-colors hover:bg-accent/20, also used by the overflow toggle at line 1569 and by expandable PlainWorkEntryRow at line 2664). The live row declares cursor-pointer and aria-expanded with no hover or press state and no chevron, so while a turn is running the control looks inert, then grows a hover tint the moment the batch settles into Used 1 tool. The new test at MessagesTimeline.test.tsx (expect(markup).not.toContain("hover:bg-accent/20")) pins that divergence, so it would need updating with the fix.
group/live-work also has no group-*/live-work consumer anywhere, so it can go.
| className="group/live-work flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" | |
| className="flex min-h-6 w-full max-w-full cursor-pointer items-center rounded-md text-left transition-colors hover:bg-accent/20 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70" |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — one finding on the reworked activity column.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — one new finding on the overflow work-toggle's onlyToolEntries contract. A previously reported item (the disclosure chevron in PlainWorkEntryRow losing its text-icon-muted owner) is still open; not re-posted here.
Posted via Macroscope — UI Consistency
| entry.toolCallId = toolCallId; | ||
| } | ||
| let toolLifecycleStatus = extractWorkLogToolLifecycleStatus(payload); | ||
| if (!toolLifecycleStatus && activity.kind === "tool.started") { |
There was a problem hiding this comment.
🟡 Medium src/session-logic.ts:904
Id-less tool.started activities leave an extra permanently running row after the tool completes. This fallback marks every start as inProgress, but shouldCollapseToolLifecycleEntries refuses the id-less started -> updated sequence, so the update/completion fold separately instead of replacing the original row; only assign the fallback status when toolCallId is present.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/session-logic.ts around line 904:
Id-less `tool.started` activities leave an extra permanently running row after the tool completes. This fallback marks every start as `inProgress`, but `shouldCollapseToolLifecycleEntries` refuses the id-less `started -> updated` sequence, so the update/completion fold separately instead of replacing the original row; only assign the fallback status when `toolCallId` is present.
There was a problem hiding this comment.
UI consistency review — one finding on the approval drawer's accessible state.
Posted via Macroscope — UI Consistency
|
UI consistency review — the inline comment could not be attached to the diff, so the finding is below.
<div
aria-label={detailAriaLabel}
className={cn("flex min-w-0 flex-1 items-center gap-2", className)}
role="group"
>The removed heading ( Suggest naming the group with the approval kind so the state survives regardless of <div
- aria-label={detailAriaLabel}
+ aria-label={fallbackLabel}
className={cn("flex min-w-0 flex-1 items-center gap-2", className)}
role="group"
>
<code
+ aria-label={detailAriaLabel}
className="block max-h-20 min-w-0 flex-1 overflow-auto whitespace-pre font-mono text-[11px] text-foreground/85 [scrollbar-width:thin] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring/70 [&::-webkit-scrollbar]:h-1.5"
data-approval-detail="complete"
tabIndex={0}
>Posted via Macroscope — UI Consistency |
There was a problem hiding this comment.
UI consistency review — one finding on the new tool-group disclosures.
Posted via Macroscope — UI Consistency
| <span | ||
| className={cn( | ||
| "flex size-6 shrink-0 items-center justify-center", | ||
| row.hasFailure ? "text-destructive" : "text-icon-muted", | ||
| )} | ||
| role={row.hasFailure ? "img" : undefined} | ||
| aria-label={row.hasFailure ? "Tool call failed" : undefined} | ||
| > | ||
| <WorkEntryIconSvg | ||
| name={row.hasFailure ? "x" : toolGroupSummaryIconName(row.summaryKind)} |
There was a problem hiding this comment.
The aggregate failure icon here is not gated on collapse, so an expanded summary group swaps its category icon (eye / terminal / square-pen) for a destructive x and announces …, tool call failed while every failing child row below already renders its own x plus Tool call failed (PlainWorkEntryRow, lines 2620-2686).
The overflow toggle immediately below now avoids exactly that with showHiddenFailure = row.hasFailure && !row.expanded (line 1564), so the two disclosures in the same activity column disagree about aggregate failure state — one drops back to its normal icon when the failing rows become visible, the other doesn't. LiveWorkEntryTimelineRow (line 1482) has the same gap for its expanded batch.
Suggest deriving const showGroupFailure = row.hasFailure && !row.expanded; and using it for the icon name, wrapper color, role/aria-label, and the button's aria-label here (and the equivalent in the live row), so the aggregate marker only stands in for hidden rows.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review — one finding on the expanded work-group spacing predicate.
Posted via Macroscope — UI Consistency
1 participant
What changed
Screenshots
Direct parent on the left; this PR on the right. Same viewport and copied application state.
This is the client consumer of #7151. Keeping it at the top isolates the larger tool-call discussion from the five UI PRs below it.
Review notes
Validation
Stack order
Built with GPT-5.6-sol in the Codex harness.
Note
Group live tool activity into an expandable, summarized row in the chat timeline
work-livetimeline row that aggregates in-progress tool calls for the active turn into a single expandable header, replacing per-entry rendering while a turn is running.liveWorkEntryLabel,summarizeToolGroup,commandProgramName, andtokenizeShellCommandutilities to derive labels and icons from tool call metadata.live-activity-focusCSS animation with a horizontal sweep effect and reduced-motion support for the live activity pill UI.(turnId, toolCallId)identity rather than adjacency, preventing cross-turn and cross-tool merges.workingStepLabelandactiveTurnInProgressprops fromMessagesTimeline; the working row now derives its state internally.deriveWorkLogEntriesnow includestool.startedevents as in-progress entries and drops Codex terminal interaction updates; this changes which rows appear in the work log during active tool execution.Macroscope summarized 96c8a4d.
Note
Medium Risk
Large presentation and session-projection changes across web and mobile timelines; row IDs and expansion keys change, and lifecycle merging rules affect what users see during parallel tool calls.
Overview
Reworks how in-progress and settled tool work appears in web and mobile chat timelines: lifecycle events fold by
turnId+toolCallId, including interleaved starts/updates/completions, andtool.startedrows stay visible with commands while runs are active.On web, the timeline adds a
work-liverow that batches the active turn’s tool calls into one expandable line (program-aware labels like “Running npm”, failure affordances, shimmer styling). Settled tool-only groups collapse to summary toggles (“Read 2 files”, local grep vs web search) instead of per-entry rows. Turn folds anchor differently when collapsed vs expanded (steer/superseded turns), the working header drops plan step text in favor of optional Thinking, andderiveActiveWorkStartedAtcan use the latest user message when session/turn pointers lag. Session logic also hides duplicate plan rows, Codex terminal-input noise, and splitsworkEntryDisplayIndicatesToolFailureso command text isn’t treated as error output.Mobile mirrors the same
toolCallId-scoped collapse for interleaved lifecycle rows inthreadActivity.Reviewed by Cursor Bugbot for commit 96c8a4d. Bugbot is set up for automated code reviews on this repo. Configure here.