fix(mothership): stop-button transitions freeze in place#5838
fix(mothership): stop-button transitions freeze in place#5838TheodoreSpeaks wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview "Stopped by user" no longer renders in the main segment list; it appears in the same tail slot as the thinking shimmer (and actions), so stopping mid-"Thinking…" reads as an in-place swap instead of the shimmer disappearing while a row appears higher up. Sizer floor during streaming uses Reviewed by Cursor Bugbot for commit fff6119. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR changes how stopped Mothership responses are rendered and how their transcript scrolling settles. The main changes are:
Confidence Score: 4/5Stopping during the final row animation can still trigger the settle chase, and the new floor rounding can shift fractional scroll positions.
apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx Important Files Changed
Reviews (1): Last reviewed commit: "fix(mothership): swap stopped row into t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| useEffect(() => { | ||
| if (isStreamActive) stopRequestedRef.current = false | ||
| }, [isStreamActive]) |
There was a problem hiding this comment.
When a user stops while lastRowAnimating is true, auto-scroll remains active through isStreamActive || lastRowAnimating, but this effect clears the stop marker as soon as the stream ends. The later auto-scroll teardown then sees shouldFollowSettle as true and runs the settle chase, so the transcript can move after the user pressed stop.
Context Used: Custom hook patterns and best practices (source)
| // Math.floor, not the raw float: a fractional min-height can round | ||
| // scrollHeight 1px ABOVE the scrolled-to extent, and that phantom 1px gap | ||
| // re-derives 1px higher after every chase step — a visible 1px/frame | ||
| // upward creep whenever the floor is what's holding scrollHeight. | ||
| const floor = Math.max( | ||
| 0, | ||
| Math.floor(el.scrollTop + el.clientHeight - padding.top - padding.bottom) |
There was a problem hiding this comment.
When scrollTop is fractional and the sizer floor is load-bearing, flooring the required extent can make scrollHeight smaller than scrollTop + clientHeight by almost one CSS pixel. The browser then clamps the scroll position, and the next layout pass derives another floor from that shifted position, which can cause a small jump or repeated drift on fractional-pixel displays.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fff6119. Configure here.
| }, [isStreamActive]) | ||
| const { ref: autoScrollRef } = useAutoScroll(isStreamActive || lastRowAnimating, { | ||
| shouldFollowSettle: () => !stopRequestedRef.current, | ||
| }) |
There was a problem hiding this comment.
Stop still chases before settle gate
Medium Severity
shouldFollowSettle only skips the post-stream kickUntil on teardown, but auto-scroll stays armed while lastRowAnimating is still true. lastRowAnimating clears in a useEffect after paint, so when the stopped row and actions mount, the live MutationObserver can still chase.kick() for at least a frame before teardown cancels it — a visible nudge after stop despite the freeze intent.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit fff6119. Configure here.
1 participant
Summary
shouldFollowSettle), so the transcript freezes where it ismin-heightroundedscrollHeightabove the scrolled-to extent; nowMath.floored)Type of Change
Testing
Tested manually on a local dev server (stop mid-thinking, credential-request reply); vitest 110/110 in the touched tree; lint + strict boundary validation + typecheck clean
Checklist