-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(chat): stop the streaming transcript floor inventing scroll space #6527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+191
−13
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
90 changes: 90 additions & 0 deletions
90
apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/sizer-floor.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| */ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { nextSizerFloor } from '@/app/workspace/[workspaceId]/home/components/mothership-chat/sizer-floor' | ||
|
|
||
| /** Matches the transcript scroller's `pt-4 pb-2`. */ | ||
| const PADDING = { paddingTop: 16, paddingBottom: 8 } | ||
| /** A viewport scrolled to the bottom of 2000px of content: extent resolves to 2000. */ | ||
| const PINNED = { scrollTop: 1424, clientHeight: 600, ...PADDING } | ||
|
|
||
| describe('nextSizerFloor', () => { | ||
| it('follows the scrolled-to extent while it stays under the high-water mark', () => { | ||
| const { floor } = nextSizerFloor({ | ||
| ...PINNED, | ||
| previousHighWater: 5000, | ||
| appliedFloor: 0, | ||
| contentHeight: 5000, | ||
| }) | ||
| expect(floor).toBe(2000) | ||
| }) | ||
|
|
||
| it('floors fractional scroll offsets down so the sizer cannot round above the extent', () => { | ||
| const { floor } = nextSizerFloor({ | ||
| scrollTop: 100.7, | ||
| clientHeight: 600, | ||
| ...PADDING, | ||
| previousHighWater: 5000, | ||
| appliedFloor: 0, | ||
| contentHeight: 5000, | ||
| }) | ||
| expect(floor).toBe(676) | ||
| }) | ||
|
|
||
| it('never returns a negative floor for a container smaller than its padding', () => { | ||
| const { floor } = nextSizerFloor({ | ||
| scrollTop: 0, | ||
| clientHeight: 8, | ||
| ...PADDING, | ||
| previousHighWater: 500, | ||
| appliedFloor: 0, | ||
| contentHeight: 500, | ||
| }) | ||
| expect(floor).toBe(0) | ||
| }) | ||
|
|
||
| it('never exceeds the content height when the transcript is shorter than the viewport', () => { | ||
| const { floor } = nextSizerFloor({ | ||
| scrollTop: 0, | ||
| clientHeight: 600, | ||
| ...PADDING, | ||
| previousHighWater: 0, | ||
| appliedFloor: 0, | ||
| contentHeight: 180, | ||
| }) | ||
| expect(floor).toBe(180) | ||
| }) | ||
|
|
||
| it('holds the high-water mark when content re-measures smaller mid-turn', () => { | ||
| const { floor, highWater } = nextSizerFloor({ | ||
| ...PINNED, | ||
| previousHighWater: 2000, | ||
| appliedFloor: 2000, | ||
| contentHeight: 1940, | ||
| }) | ||
| expect(highWater).toBe(2000) | ||
| expect(floor).toBe(2000) | ||
| }) | ||
|
|
||
| it('carries undrained debt across a turn boundary that interrupts the drain', () => { | ||
| const { floor, highWater } = nextSizerFloor({ | ||
| ...PINNED, | ||
| previousHighWater: 0, | ||
| appliedFloor: 1985, | ||
| contentHeight: 1940, | ||
| }) | ||
| expect(highWater).toBe(1985) | ||
| expect(floor).toBe(1985) | ||
| }) | ||
|
|
||
| it('raises the high-water mark as content grows', () => { | ||
| const { highWater } = nextSizerFloor({ | ||
| ...PINNED, | ||
| previousHighWater: 1200, | ||
| appliedFloor: 1200, | ||
| contentHeight: 1600, | ||
| }) | ||
| expect(highWater).toBe(1600) | ||
| }) | ||
| }) |
64 changes: 64 additions & 0 deletions
64
apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/sizer-floor.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| interface SizerFloorInput { | ||
| /** High-water mark carried from the previous commit of this turn. */ | ||
| previousHighWater: number | ||
| /** Floor currently written to the sizer, including one a drain has not finished releasing. */ | ||
| appliedFloor: number | ||
| /** Natural sizer height — `virtualizer.getTotalSize()`. */ | ||
| contentHeight: number | ||
| scrollTop: number | ||
| clientHeight: number | ||
| paddingTop: number | ||
| paddingBottom: number | ||
| } | ||
|
|
||
| interface SizerFloorResult { | ||
| /** Floor to write to the sizer's `min-height`. */ | ||
| floor: number | ||
| /** High-water mark to carry into the next commit. */ | ||
| highWater: number | ||
| } | ||
|
|
||
| /** | ||
| * Floor height for the transcript sizer while a turn streams — the value that | ||
| * keeps `scrollHeight` from dipping below the scrolled-to extent when a row | ||
| * transiently re-measures smaller — together with the high-water mark that | ||
| * bounds it. | ||
| * | ||
| * The extent (`scrollTop + clientHeight`) is the viewport's bottom edge, which | ||
| * sits BELOW the content whenever the transcript is shorter than the viewport — | ||
| * early in a turn, or in any short chat. Flooring at the raw extent invents | ||
| * scrollable space no content occupies, which stays invisible only while the | ||
| * container keeps its height. The moment it shrinks mid-turn — the composer | ||
| * growing, the queued-message banner appearing, a window or panel resize — that | ||
| * space becomes real scrollable room, the bottom-pin scrolls into it, and the | ||
| * transcript is dragged upward until the floor drains at the end of the turn. | ||
| * | ||
| * The high-water mark is what the extent is clamped to, and it folds in the | ||
| * APPLIED floor as well as the live content height. Both terms are load-bearing: | ||
| * | ||
| * - Live content alone would release the debt on the very commit that created | ||
| * it, since holding space a shrink just took away is the floor's whole purpose. | ||
| * - Ignoring the applied floor would dump undrained debt in a single frame when | ||
| * a queued message re-engages the floor mid-drain — the end-of-turn jump the | ||
| * eased drain exists to prevent. | ||
| * | ||
| * Together they say: never exceed the space content has actually held this turn. | ||
| * | ||
| * `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 is holding `scrollHeight`. | ||
| */ | ||
| export function nextSizerFloor({ | ||
| previousHighWater, | ||
| appliedFloor, | ||
| contentHeight, | ||
| scrollTop, | ||
| clientHeight, | ||
| paddingTop, | ||
| paddingBottom, | ||
| }: SizerFloorInput): SizerFloorResult { | ||
| const highWater = Math.max(previousHighWater, contentHeight, appliedFloor) | ||
| const extent = Math.max(0, Math.floor(scrollTop + clientHeight - paddingTop - paddingBottom)) | ||
| return { floor: Math.min(extent, highWater), highWater } | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.