fix(chat): remove doubled gap under assistant messages - #5926
Conversation
The assistant action bar reserves its own height with `min-h-7.5` so a bar revealed on hover does not shift the transcript, and cancels that reservation with a matching `-mb-7.5` so it costs nothing in flow. The two halves had been separated: `-mb-7.5` sat on the message root, paired with a `pb-7.5` added for `content-visibility: auto`'s implied `contain: paint`. There the two merely cancel each other, leaving the footer's `min-h-7.5` uncompensated — a dead 30px band under every assistant turn, on top of the message group's `gap-y-6`. Consecutive replies sat ~54px apart instead of 24px. Put the compensation back on the element that reserves the height, and comment both pairs so they are not conflated again. The root's paint-box pair is unchanged and still required: with the footer's `-mb` restored, the bar paints past the content box and `contain: paint` would otherwise clip it. Adds a regression test asserting the invariant (reserved === compensated, on the same element) rather than the literal value, so retuning the bar's height stays free while decoupling the halves does not.
tinysweeper
Bot
added
the
priority: p3
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0053 · 63,707 in / 563 out · 0 cached (0%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-flash · 265 embedded
critique: $0.0019 · 22,958 in / 146 out · 0 cached (0%) · deepseek/deepseek-v4-flash
security: $0.0019 · 22,916 in / 248 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tests: $0.0010 · 12,093 in / 82 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0005 · 5,740 in / 87 out · 0 cached (0%) · deepseek/deepseek-v4-flash
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe assistant message action bar now uses the message-group gap to compensate for its reserved height. A Vitest test verifies this spacing relationship for multiple assistant replies. ChangesAssistant action bar spacing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized chat spacing fix preserves action-bar behavior and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 884d66db30
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Codex review on the previous commit: compensating the footer's full `min-h-7.5` pulls the bar 30px into a gap that `gap-y-6` only makes 24px tall, so the bar's bottom 6px paint over the next message's first line — at the same left inset (`ms-2` on the bar, `px-2` on the content). The bar is `pt-1.5` plus a `size-6` button, so it is exactly 30px and the overflow is real, not marginal. Hand back the gap rather than the whole reservation: `-mb-6 min-h-7.5`. The bar now fills the 24px gap exactly and consecutive assistant turns sit 30px apart — still well down from the 54px the uncompensated reservation produced, with no overlap. Retargets the regression test at the property that actually matters: the amount pulled back equals the message group's `gap-y-*`, read off both elements rather than hard-coded. That fails in both directions — pulling back nothing (the original dead-band bug) and pulling back more than the gap (this overlap) — and both were verified to fail before restoring the fix.
How this change flows0 changed behaviours across 8 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 34 further behaviours left out to keep the diagram readable. flowchart LR
n0["ThreadRoot"]:::impacted
n1["Composer"]:::impacted
n2["ThreadComponentsContext"]:::impacted
n3["Thread"]:::impacted
n4["ComposerAction"]:::impacted
n5["AssistantMessage"]:::impacted
n0 -->|uses| n1
n0 -->|uses| n2
n1 -->|uses| n2
n1 -->|uses| n4
n3 -->|uses| n0
n3 -->|uses| n2
n4 -->|uses| n2
n5 -->|uses| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
|
@codex review The P2 you raised on |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
Summary
-mbonto the footer that carriesmin-h-7.5, sized to the message group's gap so the bar is drawn inside the gap instead of stacking on top of it. Assistant turns go from ~54px apart to 30px.gap-y-*), which fails in both directions.Problem
AssistantMessagereserves the action bar's height withmin-h-7.5so a bar revealed on hover does not shift the transcript. That reservation is meant to be cancelled immediately by a matching-mb-7.5on the same element, so it costs nothing in flow — the pattern is still intact inpages/dev/assistant-ui-demo/BaseDemo.tsx:599, comment included:In
thread.tsxthe two halves had drifted apart.-mb-7.5ended up on the message root, paired with apb-7.5added socontent-visibility: auto(which impliescontain: paint) would not clip the bar. On the root those two simply cancel each other and reclaim nothing, leaving the footer'smin-h-7.5uncompensated:BaseDemo.tsxfootermin-h-7.5-mb-7.5thread.tsxfooter (before)min-h-7.5thread.tsxrootpb-7.5-mb-7.5Result: 30px of dead space under every assistant turn, stacked on the message group's
gap-y-6(24px) — so ~54px between consecutive replies, and the same band between a reply and a following tool group.Solution
Put the compensation back on the element that reserves the height, sized to the gap it is being pulled into:
Assistant turns now sit 30px apart, down from ~54px.
Why
-mb-6and not the full-mb-7.5. Full compensation is what upstream prescribes (BaseDemo.tsx:599, "forpt-[n]use-mb-[n + 6]"), and it was the first form of this fix — Codex caught it in review. The bar ispt-1.5plus asize-6button, so it is exactly 30px, whilegap-y-6makes the gap only 24px. Pulling back all 30px drags the bar 6px deeper than the gap is tall, and the bar (ms-2) and the next message's content (px-2) share an 8px left inset — so the tail lands on the following reply's first line rather than beside it. Handing back the gap instead makes the bar fill it exactly:-mbstep-mb-7.5(full)-mb-6(this PR)Notes on what deliberately did not change:
-mb-7.5 pb-7.5stays. It is paint-only and is load-bearing again: with the footer's-mbrestored the bar paints past the root's content box, andcontain: paintwould clip it without that padding. Both pairs are now commented so a future reader does not conflate the paint allowance with the spacing compensation.gap-y-6is untouched. It also governs user↔assistant spacing, so retuning it is a separate design call rather than part of this fix.The test reads the
-mb-*step off the footer and thegap-y-*step offaui_message-groupand asserts they match, so it pins the relationship rather than either literal — retuning either stays free, breaking their correspondence does not. Verified non-vacuous in both directions before landing: no-mbfails withexpected null to be 6, and-mb-7.5fails withexpected 7.5 to be 6.Submission Checklist
thread.actionBarSpacing.test.tsx, covering both failure directions (no compensation → dead band; over-compensation → overlap), each confirmed failing before landing. jsdom performs no layout, so the utilities are the only observable; they are read as a relationship between two elements rather than asserted as class-string literals.ACTION_BAR_HEIGHTassignment) is hit by the new test;thread.tsxwent from 0% to ~67% statements. The rest of the diff is comments.N/A: behaviour-only change, no feature row added, removed or renamed.## Related—N/A: no matrix feature IDs affected.useExternalStoreRuntime.N/A: CSS-only spacing fix, no release-cut surface changed.Closes #NNN—N/A: no tracking issue; reported directly.Impact
Related
gap-y-6atthread.tsx:248is the remaining knob if 30px still reads loose — note it now sets both the turn spacing and the space the action bar is drawn into, and it also moves user↔assistant spacing, so it is left as a separate design call.BaseDemo.tsx:599still carries the fully-compensated form and the same latent overlap; left alone as it is a dev-only demo page.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/assistant-message-action-bar-spacingValidation Run
pnpm --filter openhuman-app format:check— prettier clean on both touched files; eslint clean.pnpm typecheckvitest run src/components/assistant-ui/ src/features/conversations/components/— 27 files, 248 tests passed.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
contain: paintfrom clipping it; the bar stays clear of neighbouring messages.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
Tests