fix(ui): rework turn changes panel and lift assistant footer - #844
Merged
Conversation
Three small regressions in the turn changes panel surfaced after PR #704: - The "Some shell changes were not individually captured" line added noise to mixed/uncaptured turns. Removed; the uncaptured kind now renders no panel at all. - Fully-undone turns rendered "0 files changed +0 -0" plus a separate "Undone" pill. Replaced with a single "N files undone" summary. - Panel was glued to surrounding prose with margin-top: 4. Bumped to --space-md (12, the breathe tier) so the panel reads as a turn-internal side-effect rather than a section switch. While fixing DOM order so the panel sits above the assistant footer, the footer's home (inside text-part - a generic part) turned out to be the root cause. Lifted the footer out: - text.tsx loses ~60 lines of meta / duration / model / copy / hover logic and is now a pure markdown renderer. - New AssistantTurnFooter component owned by SessionTurn. SessionTurn finds the last non-empty assistant text part, passes {message, text} to the footer, and renders it after the panel. Gated on !working() so no footer appears during streaming. - The showAssistantCopyPartID / turnDurationMs prop chain (only ever fed the embedded footer) is removed from MessagePartProps, message-router, AssistantParts, AssistantMessageDisplay, MessageProps. - Footer keeps hover-only visibility, now gated on the session-turn-message-container hover. - user-message.tsx is untouched; it already owns its own copy button independent of text.tsx. Footer form (left-aligned, agent / model / duration) is intentionally unchanged here. DESIGN.md L519-520 calls for a different form (right-aligned, completion time only, [Copy] [Fork]); follow-up.
Contributor
📝 WalkthroughWalkthroughThis PR refactors assistant message footer functionality by extracting copy-to-clipboard and metadata display from individual text parts into a dedicated ChangesFooter Refactoring and Component Contract Updates
Session Turn-Changes UI and Visibility Logic
E2E Test Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
app
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the assistant message UI by introducing a dedicated AssistantTurnFooter component and consolidating the copy-to-clipboard and metadata display logic. It also updates the session turn changes panel to improve the display of undone turns and removes the 'uncaptured' shell changes indicator. A regression was identified in the footer logic where the 'Copy Response' button now only copies the last text part instead of the entire response; a suggestion was provided to aggregate all text parts across assistant messages.
Perf delta summaryComparator: pass
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/session-turn-changes-panel.tsx`:
- Around line 120-124: The omitted-lines notice is rendered outside the
isUndoneTurn() conditional so it appears for both undone and non-undone turns;
move the Show block that renders the omitted notice (the span with
data-slot="session-turn-changes-omitted" and the i18n.t call using
props.turnChange.omittedCount) into the non-undone fallback branch of the Show
that checks isUndoneTurn(props.turnChange) so it only displays when the turn is
not undone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ace4e5e1-2733-4525-ab04-4403e4588b62
📒 Files selected for processing (15)
packages/app/e2e/snap/session-turn-changes.snap.tspackages/ui/src/components/assistant-turn-footer.tsxpackages/ui/src/components/message-part-registry.test.tspackages/ui/src/components/message-part.csspackages/ui/src/components/message-part/assistant-message-display.tsxpackages/ui/src/components/message-part/assistant-parts.tsxpackages/ui/src/components/message-part/message-router.tsxpackages/ui/src/components/message-part/parts/text.tsxpackages/ui/src/components/message-part/registry.tspackages/ui/src/components/session-turn-changes-panel.tsxpackages/ui/src/components/session-turn-changes.tspackages/ui/src/components/session-turn.csspackages/ui/src/components/session-turn.tsxpackages/ui/src/i18n/en.tspackages/ui/src/i18n/zh.ts
💤 Files with no reviewable changes (5)
- packages/ui/src/components/message-part/message-router.tsx
- packages/ui/src/components/message-part.css
- packages/ui/src/components/message-part/assistant-parts.tsx
- packages/ui/src/components/message-part/registry.ts
- packages/ui/src/components/message-part/assistant-message-display.tsx
Adds e2e regression coverage for the lifted AssistantTurnFooter: hover-only reveal, clipboard copy of the trimmed assistant text, and DOM ordering below the turn changes panel in a mixed text+patch turn.
… helper Introduces a tag-aware turnChangeFiles helper so that downstream UI code (panel, hasVisibleTurnChanges, turnChangeAction) no longer pattern-matches on TurnChangeDisplay.kind directly. Future file-bearing variants only need to opt in inside this helper.
Adds a mixed-kind fixture pass to session-turn-changes snap: apply_patch + shell touch in the same turn. Asserts the panel renders the captured file row while the uncaptured diagnostic copy stays suppressed, pinning the no-"未逐个捕获" rule against regression.
The clipboard try/catch contract moved out of message-part/ when the footer was lifted to turn level, so the hardening grep no longer scanned the file that actually holds the guard. Pulls assistant-turn-footer.tsx into the hardening source set and bumps the expected match count to 2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix three small regressions in the turn changes panel and lift the assistant footer (copy / model / duration) out of
text-partto its own turn-level component, so the panel sits between assistant prose and footer in the natural reading order.Why
After #704 the turn changes panel had three rough edges users hit immediately: a noisy "Some shell changes were not individually captured" line, a "0 files changed +0 -0 Undone" state when everything was undone, and the panel glued tight to surrounding chat. Fixing the third one revealed that the panel was rendered after the assistant footer in DOM, which itself was caused by the footer living inside
text-part— a generic part renderer that was carrying turn-level metadata it shouldn't own.The fix removes the noise, gives the "fully undone" state a single readable summary, and lifts the footer out of
text-partso the natural turn order isprose → panel → footer. As a side effect,text-partbecomes a pure markdown renderer (~60 fewer lines) and theshowAssistantCopyPartID/turnDurationMsprop chain that only existed to feed the embedded footer is removed across the part registry.Related Issue
None. Captured as a follow-up to #704; the bigger Turn Changes visual redesign is tracked separately in #823.
Human Review Status
Pending
Review Focus
SessionTurn:AssistantParts → SessionTurnChangesPanel → AssistantTurnFooter, with the footer gated on!working()so it doesn't appear during streaming.assistantFooterTargetmemo returns{message, text}for the last non-empty assistant text part so the footer's meta (agent / model / duration) is read from the message that owns the text being copied, not from "the last assistant message" which can end on a tool part.session-turn-message-container:hoverinstead of the per-part wrapper.margin-top: 12pxfollows the "breathe" tier in DESIGN.md (paragraph-to-paragraph), because the panel is a side-effect of the same turn, not a section switch — see the new "Turn-internal product modules" paragraph added to DESIGN.md (local-only file, not in this diff).message-part-registry.test.tsclipboard hardening grep stays at try-count 2 after the lift: the scanned source set now includesassistant-turn-footer.tsxalongsidemessage-part/, so removing the footer'stry { await navigator.clipboard.writeText }guard still fails the test.turnChangeFilesis the single tag-narrowing point forTurnChangeDisplay.kind; the panel and helpers consume that helper, not the union directly.Footer form (left-aligned
Agent · Model · 8s) is intentionally unchanged in this PR. DESIGN.md L519-520 specifies a different form (right-aligned, completion time only,[Copy] [Fork]) which is a follow-up.Risk Notes
!working()) is preserved so the footer doesn't flash in during partial responses.user-message.tsxis untouched — it already owned its own copy button viarenderMetaAndActionsand never went throughtext-part. The deadif (props.message.role !== "assistant")branch in the oldtext-partfooter code is removed alongside the rest.How To Verify
Screenshots or Recordings
docs/design/preview/screenshots/session-turn-changes.png(regenerated bybun run snap session-turn-changes):count(session-turn-changes) === 0instead of capturing a panel screenshot, because that kind renders no panelChecklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Release Notes
New Features
Style
Refactor
Tests