Repository navigation
Agent pane: messages between agents share the tool rows' icon column and text inset - #28796
teamleaderleo wants to merge 4 commits into
Conversation
…ext inset A gallery state (agent-pane.agent-message-row / icon-column) stacks a command row, a message from a Codex session (its logo), this agent's send (the envelope) and a message from a sender with no mark (the bot). The browser test measures each top-level row's icon slot, glyph center, drawn glyph size and first word's x, and fails while a received message's mark sits 8px right of the tool rows' slot, its name 5.5px right of their words, and its logo draws at 14px beside the envelope's 11.5px. Leo: "the alignment i feel like we might as well" Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
Tool rows lay out as a grid: the 16px glyph slot at the row's left edge, the 8px gap, the words at 24px (grid.md 5.5). Messages did not: - a received message's line and sender each added 4px of inline padding, so its mark sat 8px right of the slot and its name started at 28px; the line drops the padding and the mark takes the slot (its hover chips keep their place before the chevron); - a sent message's line is flex, where the tool icon's -1.5px end margin (a no-op in the grid) pulled its words to 22.5px; it is 0 there; - the waiting group's chevron moves into the same slot. A harness logo draws at 12px, the stroke glyphs' drawn size, instead of 14px. Leo: "the alignment i feel like we might as well" Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Passes: CI passes on CI passes on Written by web and size checks: green |
|
Found 3 test failures on Blacksmith runners: Failures
|
Base's #29800 already put the sent, received and tool rows in one icon column (--cv-row-icon, --cv-row-icon-gap, the received line's hover plate inset). Its rules win; this branch keeps the waiting group's chevron in the slot and the sender's harness logo at 12px, now through base's AgentIdentity. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
|
Review (codex-review-box/gpt-6-astra high, lane-place): BLOCK at 9c1ae0b BLOCK at 9c1ae0b Visual verdict: BLOCK — opened both after screenshots and their before counterparts. Alignment improves, but repeated disclosure controls make both themes busier than ChatGPT’s transcript and trigger your explicit blocking rule.
Browser execution was unavailable: the box lacks Bun and webview dependencies. Finding 1 follows from the merged DOM and measurement code. |
… wrapper The icon-column test picked AgentIdentity's outer span before its svg, so the bot sender measured its 16px wrapper against the envelope's svg geometry. Select the svg or image first and fall back to a span only for a text mark. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-63b9964940c20354b639
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webviews/test/transcript-icon-column.test.ts">
<violation number="1" location="webviews/test/transcript-icon-column.test.ts:197">
P2: This regression test does not exercise the waiting-group row it claims to measure. Add queued messages to the icon-column fixture and require a `waiting` row, so the chevron’s shared slot and text inset are actually asserted.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/conversation/PendingMessagesCard.css">
<violation number="1" location="webviews/src/agent-session/acpmux/conversation/PendingMessagesCard.css:36">
P3: This removes padding from physical left rather than inline-start, so an RTL transcript keeps the waiting-group chevron inset from the shared icon edge. Use logical inline padding so the icon is flush on either writing direction.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| ); | ||
| await page.screenshot({ path: path.join(artifacts, `icon-column-${theme.file}.png`), fullPage: true }); | ||
| } | ||
| expect(rows.map((row) => row.kind)).toEqual(expect.arrayContaining(["tool", "sent", "received"])); |
There was a problem hiding this comment.
P2: This regression test does not exercise the waiting-group row it claims to measure. Add queued messages to the icon-column fixture and require a waiting row, so the chevron’s shared slot and text inset are actually asserted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/test/transcript-icon-column.test.ts, line 197:
<comment>This regression test does not exercise the waiting-group row it claims to measure. Add queued messages to the icon-column fixture and require a `waiting` row, so the chevron’s shared slot and text inset are actually asserted.</comment>
<file context>
@@ -0,0 +1,204 @@
+ );
+ await page.screenshot({ path: path.join(artifacts, `icon-column-${theme.file}.png`), fullPage: true });
+ }
+ expect(rows.map((row) => row.kind)).toEqual(expect.arrayContaining(["tool", "sent", "received"]));
+ expect(rows.filter((row) => row.kind === "received").length).toBeGreaterThanOrEqual(2);
+ expect(found).toEqual([]);
</file context>
| /* The trailing room keeps the count clear of the plate's edge; none before the chevron, which | ||
| sits in the tool rows' glyph slot at the row's left edge (design/chat-transcript.md "One icon | ||
| column"). */ | ||
| padding: 0 var(--ds-space-1) 0 0; |
There was a problem hiding this comment.
P3: This removes padding from physical left rather than inline-start, so an RTL transcript keeps the waiting-group chevron inset from the shared icon edge. Use logical inline padding so the icon is flush on either writing direction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/conversation/PendingMessagesCard.css, line 36:
<comment>This removes padding from physical left rather than inline-start, so an RTL transcript keeps the waiting-group chevron inset from the shared icon edge. Use logical inline padding so the icon is flush on either writing direction.</comment>
<file context>
@@ -30,7 +30,10 @@
+ /* The trailing room keeps the count clear of the plate's edge; none before the chevron, which
+ sits in the tool rows' glyph slot at the row's left edge (design/chat-transcript.md "One icon
+ column"). */
+ padding: 0 var(--ds-space-1) 0 0;
border-radius: 6px;
cursor: default;
</file context>
| padding: 0 var(--ds-space-1) 0 0; | |
| padding-inline: 0 var(--ds-space-1); |
|
Codex review
Fresh review started at the new head: |
|
Subsumed by #29124, merged as 9d0d544 with this PR's commits and authorship folded in. The footer's first action glyph, the 'Worked for' label and the agent-message icon now start on the prose's left edge. The shared icon column stays at #29754's 22.5px; #29996 tracks 22.5 vs 24. Leaving it to this PR's owner to close. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Leo: "the alignment i feel like we might as well"
The agent pane's transcript rows (a tool row, a message from another agent, this agent's send, the waiting group's head) each put their glyph in a different place: the received line's padding pushed the sender's mark 8px right of the tool rows' glyph slot and its name to x=28, a sent line's words started 1.5px left of the tool rows', and a harness logo drew at 14px beside 12px stroke glyphs. Now every row shares one icon column and one text inset.
conversation.css).AgentMessageRow.css).PendingMessagesCard.css).IncomingMessageCard.tsx).Spec: design/chat-transcript.md "One icon column" (leo-workspace#1814).
Measured (gallery
agent-pane.agent-message-row#icon-column, Chromium, px from the row's left)Light and dark measure the same. Before, the test reported 8 problems per theme; after, none.
Stills (2x, gallery, synthetic data)
Viewed after image: icon-column-after-light.png and icon-column-after-dark.png. The terminal, Codex logo, envelope and bot glyphs share one center line, and "Ran", "go-live", "Sent to" and "release-notes" all start at the same x. No control repeats per row, and nothing is clipped or off-screen.
Tests
webviews/test/transcript-icon-column.test.ts(new, Chromium, light and dark): each row's glyph slot, glyph center, drawn glyph size and first word's x must match the tool row's. It failed at the red-first commit e579b74 (run https://github.com/manaflow-ai/cmux/actions/runs/41161456159) and passes at 429c1d9 (run https://github.com/manaflow-ai/cmux/actions/runs/41161911610).Changelog
none
Agent session: agent session
🤖 Generated with Claude Code