improve(ui): flatten chat tool-call rows into a scannable list#99763
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 12:35 PM ET / 16:35 UTC. Summary Reproducibility: yes. Current main source and the linked issue describe the nested, decorated tool-call presentation, and the PR's screenshots show the before/after visual path; I did not run a live browser in this read-only review. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Refresh the PR so GitHub sees the focused 5-file UI-only delta, preserve the flat tool-row behavior, then merge only after maintainer visual acceptance and required checks pass. Do we have a high-confidence way to reproduce the issue? Yes. Current main source and the linked issue describe the nested, decorated tool-call presentation, and the PR's screenshots show the before/after visual path; I did not run a live browser in this read-only review. Is this the best way to solve the issue? Yes, after rebase. Removing duplicated legacy CSS and flattening the existing tool-card presentation is narrower than adding a config mode; the remaining decision is maintainer visual acceptance, not a different code layer. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 280e8d487627. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
e93318d to
392de88
Compare
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d76f012b4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `telegram sendRichMessage rejected invalid entity; falling back to plain text: ${errText}`, | ||
| ); | ||
| return await sendPlainFallback(richFallbackText); | ||
| return await sendPlainFallback(fallbackPlan.plainText); |
There was a problem hiding this comment.
Split rich fallback text before sendMessage
When rich Telegram replies are chunked for the 32,768-char rich limit, the plain fallback can still exceed the normal 4,000-char sendMessage limit. This path computes fallbackPlan.chunks but discards it and sends fallbackPlan.plainText as one message, unlike the durable rich-send path in send.ts; any long rich reply that hits the new rich/entity parse fallback fails again as message-too-long instead of degrading to plain chunks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d84255609
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? api.editMessageText(chatId, messageId, fallbackPlan.plainText, plainTextParams) | ||
| : api.editMessageText(chatId, messageId, fallbackPlan.plainText), |
There was a problem hiding this comment.
Split rich edit fallback before calling editMessageText
For the newly handled rich HTML parse fallback, this passes the entire fallbackPlan.plainText into the normal editMessageText API. Telegram limits editMessageText.text to 1–4096 characters after entity parsing (https://core.telegram.org/bots/api#editmessagetext), while rich messages here can be up to 32,768 chars, so a long rich edit that should degrade to plain text will fail again as message-too-long instead of updating; the send path above uses fallbackPlan.chunks, so this edit path needs an equivalent bounded fallback or replacement strategy.
Useful? React with 👍 / 👎.
Tool calls in Control UI chat rendered as triple-nested cards: activity group card + tool-shell bubble chrome + gradient summary pill with a 0 8px 22px drop shadow. Collapsed rows now render as flat single-line rows with ellipsis, the activity group uses a flat header plus a left rule, and expanded detail keeps soft tinted blocks without card chrome. Also deletes the dead legacy .chat-tool-card__details/__output CSS and the unused renderToolCardSidebar path with its never-taken renderToolDataBlock branches. Closes #99760
2d84255 to
8909ebf
Compare
|
Merged via squash.
|
Closes #99760
What Problem This Solves
Tool calls in the Control UI chat render as heavily decorated nested cards: the
Activity: N toolsgroup draws a bordered card, every tool message inside it draws another bubble box, and the summary button on top adds an accent gradient, an inset highlight, and a0 8px 22pxdrop shadow. Long tool summaries wrap across several lines. Dense agent sessions become hard to scan — a single 8-tool activity group fills an entire viewport.Part of the box stacking is a CSS ordering bug:
components.cssbegins with@import "./chat.css", so its legacy.chat-bubbleand.chat-tool-cardblocks load afterchat/grouped.css/chat/tool-cards.cssand override the intended.chat-bubble--tool-shellresets (padding: 0; border: 0; background: transparent). Each tool row therefore leaks a full bubble box underneath the summary pill.Why This Change Was Made
Tool rows are now flat one-line rows (chevron + tool icon + label + mono detail, ellipsized), the activity group header is a flat clickable row, and its body is grouped by a thin left rule instead of a card. Expanded tool detail keeps soft tinted input/output blocks without card chrome; canvas/document previews keep their frame because they embed external content. Error states remain explicit (red icon/label plus
Errorbadge, error groups still auto-expand). The duplicated legacy chat-bubble/tool-card CSS incomponents.cssis deleted and the canonical.chat-bubblebase inchat/grouped.cssis reconciled to today's computed style, so normal user/assistant bubbles render unchanged. Dead render paths (renderToolCardSidebar, never-takenrenderToolDataBlockbranches) and their orphaned CSS are removed.User Impact
Chat sessions with tool activity are dramatically denser and calmer: an 8-tool activity group shrinks from roughly a full screen of stacked cards to a few compact rows, in both light and dark themes. Collapsed rows clamp to one line; the full command and output remain one click away. No behavior changes: expand/collapse, error badges, sidebar open, and previews all work as before.
Evidence
Before/after with the same seeded conversation (mock gateway fixture, light and dark, collapsed/expanded/detail states):
Validation:
node scripts/run-vitest.mjs run ui/src/ui/chat/tool-cards.test.ts ui/src/ui/chat/tool-cards.node.test.ts ui/src/ui/chat/grouped-render.test.ts ui/src/ui/chat/build-chat-items.test.ts— 4 files, 153 tests passedpnpm tsgo:core— cleanoxfmt --checkon all touched files — cleanui/src/test-helpers/control-ui-e2e.ts), viewport 1290×1180,deviceScaleFactor: 2Net diff: −720 lines (340 insertions, 1060 deletions).