fix(web): stop the transcript scrolling itself on every streamed chunk - #4884
fix(web): stop the transcript scrolling itself on every streamed chunk#4884AsimNet wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding: a leftover temporary Effect script (packages/contracts/tmp-rexec.ts) was committed alongside the reduce-motion/auto-scroll change. The rest of the changed TypeScript (contracts schema additions, reducedMotion.ts, timeline/settings wiring) follows the Effect conventions — no service, layer, or error-model violations found.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
This looks like a leftover debugging bridge rather than part of the reduce-motion / auto-scroll change, and it lands in a shared package at a non-canonical location: it sits outside packages/contracts/src, so the package tsconfig.json (include: ["src"]) never typechecks it, and it hardcodes a private host plus several as any casts. Suggest dropping it from this PR.
If it does need to live in the repo, it would be worth moving it to a typechecked scripts location and tidying the Effect usage while it is still small: import * as RpcClient from "effect/unstable/rpc/RpcClient" instead of the barrel named import, Effect.provide(protocolLayer) instead of manually Layer.build-ing the context, and a typed failure instead of Effect.catchCause((cause) => Effect.logDebug(String(cause))), which swallows every failure and defect into a stringified message.
Posted via Macroscope — Effect Service Conventions
| let printedUpTo = 0; | ||
| let streaming = false; | ||
|
|
||
| const onChunk = (chunk: string) => |
There was a problem hiding this comment.
🟡 Medium contracts/tmp-rexec.ts:89
onChunk appends every terminal byte to buf and never trims data that has already been written to stdout. A long-running or high-output remote command causes the process to retain the command's entire output for up to the one-hour default timeout; sufficiently large build/test output exhausts memory and crashes the helper. Once the consumed prefix has been written to stdout, it should be discarded from buf, keeping only the small suffix needed to detect a split END marker.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/tmp-rexec.ts around line 89:
`onChunk` appends every terminal byte to `buf` and never trims data that has already been written to stdout. A long-running or high-output remote command causes the process to retain the command's entire output for up to the one-hour default timeout; sufficiently large build/test output exhausts memory and crashes the helper. Once the consumed prefix has been written to stdout, it should be discarded from `buf`, keeping only the small suffix needed to detect a split `END` marker.
| Effect.runPromise(Effect.scoped(program) as any).then( | ||
| (code) => process.exit(Number(code)), | ||
| (err) => { | ||
| console.error("\n[rexec] failed:", err?.message ?? err); | ||
| process.exit(1); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🟡 Medium contracts/tmp-rexec.ts:149
process.exit() is called immediately after writing the final stdout chunk, which can silently truncate output when stdout is piped and the write is still queued. Node forces termination on process.exit() regardless of pending stream writes, so the exit code is reported correctly but the command's output is incomplete. Set process.exitCode and let the event loop drain naturally, or await process.stdout before exiting.
| Effect.runPromise(Effect.scoped(program) as any).then( | |
| (code) => process.exit(Number(code)), | |
| (err) => { | |
| console.error("\n[rexec] failed:", err?.message ?? err); | |
| process.exit(1); | |
| }, | |
| ); | |
| Effect.runPromise(Effect.scoped(program) as any).then( | |
| (code) => { | |
| process.exitCode = Number(code); | |
| }, | |
| (err) => { | |
| console.error("\n[rexec] failed:", err?.message ?? err); | |
| process.exitCode = 1; | |
| }, | |
| ); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/tmp-rexec.ts around lines 149-155:
`process.exit()` is called immediately after writing the final stdout chunk, which can silently truncate output when stdout is piped and the write is still queued. Node forces termination on `process.exit()` regardless of pending stream writes, so the exit code is reported correctly but the command's output is incomplete. Set `process.exitCode` and let the event loop drain naturally, or await `process.stdout` before exiting.
| else if (argv[i] === "--") (cmdParts.push(...argv.slice(i + 1)), (i = argv.length)); | ||
| else cmdParts.push(argv[i]!); | ||
| } | ||
| const command = cmdParts.join(" "); |
There was a problem hiding this comment.
🟡 Medium contracts/tmp-rexec.ts:33
cmdParts.join(" ") discards argument boundaries, so any argument containing spaces is split into separate words on the remote shell. For example, -- printf '%s\n' 'hello world' sends printf %s\n hello world, making hello and world separate arguments instead of one. Similarly, -- bash -lc 'echo one; echo two' no longer passes the script as a single -c argument, changing the command's semantics. Any argument with whitespace, glob characters, or other shell-significant text executes incorrectly unless the caller manually adds literal shell quoting that a normal shell would have stripped. Consider joining with proper shell escaping (e.g. shlex-style quoting) instead of a plain space.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/contracts/tmp-rexec.ts around line 33:
`cmdParts.join(" ")` discards argument boundaries, so any argument containing spaces is split into separate words on the remote shell. For example, `-- printf '%s\n' 'hello world'` sends `printf %s\n hello world`, making `hello` and `world` separate arguments instead of one. Similarly, `-- bash -lc 'echo one; echo two'` no longer passes the script as a single `-c` argument, changing the command's semantics. Any argument with whitespace, glob characters, or other shell-significant text executes incorrectly unless the caller manually adds literal shell quoting that a normal shell would have stripped. Consider joining with proper shell escaping (e.g. `shlex`-style quoting) instead of a plain space.
![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 high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c01c834d2a4cd4db2335ce667da9c8fc53b561c. Configure here.
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. This PR introduces new user-facing settings and changes the default transcript scrolling behavior from on to off. Additionally, open review comments identify potential CSS issues with undefined variables and incomplete animation coverage that should be verified before merging. You can customize Macroscope's approvability policy. Learn more. |
![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: 8c01c834d2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : // --safe-area-inset-start/end resolve to the physical side that | ||
| // matches the reading direction, so the notch inset lands on the | ||
| // right edge in RTL instead of following the LTR assumption. | ||
| "workspace-topbar ps-[calc(var(--safe-area-inset-start)+0.75rem)] pe-[calc(var(--safe-area-inset-end)+0.75rem)] sm:ps-[calc(var(--safe-area-inset-start)+1.25rem)] sm:pe-[calc(var(--safe-area-inset-end)+1.25rem)]", |
There was a problem hiding this comment.
On every non-Electron chat surface, these padding declarations are invalid because neither --safe-area-inset-start nor --safe-area-inset-end is defined anywhere in the repository. Browsers therefore discard all four declarations, removing the previous 0.75rem/1.25rem horizontal header padding and placing its controls against the viewport edges; define the variables or retain declarations based on the existing env(safe-area-inset-*) values.
Useful? React with 👍 / 👎.
| html[data-reduce-motion="true"] [class*="animate-status-pulse"], | ||
| html[data-reduce-motion="true"] [class*="animate-status-ping"], | ||
| html[data-reduce-motion="true"] [class*="animate-sidebar-working-text"], | ||
| html[data-reduce-motion="true"] [class*="animate-skeleton"], | ||
| html[data-reduce-motion="true"] [class*="animate-pulse"] { |
There was a problem hiding this comment.
When the in-app Reduce motion switch is enabled without the OS media preference, this selector list does not match the infinite [animation:bounce_2.4s_ease-in-out_infinite] on ProviderInstanceCard.tsx:626; its motion-reduce:animate-none only responds to the OS preference. Consequently the newly advertised setting still leaves a continuously repainting indicator bouncing, so include that animation in the data-reduce-motion path.
AGENTS.md reference: AGENTS.md:L128-L134
Useful? React with 👍 / 👎.
| const BASE = process.env.T3_BASE ?? "http://100.77.192.114:3773"; | ||
| const TOKEN = process.env.T3_TOKEN ?? ""; | ||
| const THREAD_ID = process.env.T3_THREAD ?? "claude-remote-build"; |
There was a problem hiding this comment.
This commit adds an unreferenced maintainer scratch utility that opens a terminal and executes arbitrary shell commands against a hard-coded private T3 endpoint. It is unrelated to the web scrolling change, embeds environment-specific identifiers in distributed source, and introduces runtime logic into the contracts package; remove it from the commit rather than shipping it as supported repository code.
AGENTS.md reference: AGENTS.md:L119-L124
Useful? React with 👍 / 👎.
The chat transcript follows the live edge while an agent streams, which fires a scroll once per chunk. Over a long session that continuous movement is tiring to read, and there was no way to turn it off. Adds two per-device settings. `chatAutoScroll` gates live-follow and defaults to off: the transcript holds still, the send-time anchor still positions the new turn once, and the scroll-to-end pill becomes the way back. `reduceMotion` forces the reduced-motion path on regardless of the OS preference, making scrolls instant and stopping the looping indicator animations that otherwise never stop repainting while an agent works. The pill's visibility guard needed care rather than a simple gate. Leaving the live edge without a user gesture means a programmatic scroll is still settling, not that the reader moved: with follow on, live-follow owns the position for as long as it holds it; with it off, only the window just after a thread switch or a send is settling, because LegendList reports not-at-end while initialScrollAtEnd lands. Suppressing indefinitely in that mode would hide the pill exactly when it is the only way back, and not suppressing at all would flash it on every thread switch. Reduce motion also covers the provider-update indicator, whose arbitrary [animation:bounce_...] class the animate-* selectors never matched and whose motion-reduce: variant only answers to the OS preference. Spinners are deliberately left spinning — a frozen spinner reads as a hung app. Written by Claude Opus 4.5 in Claude Code.
8c01c83 to
c376c94
Compare
|
Thanks — all four findings were real. Fixed and force-pushed as one clean commit.
Undefined Scroll pill flashing on thread switch — also real, and the fix needed more than the gate I had. Leaving the live edge without a user gesture means a programmatic scroll is still settling rather than the reader moving away. With follow on, live-follow owns the position for as long as it holds it. With it off, only the window just after a thread switch or a send is settling, because LegendList reports not-at-end while Bouncing provider-update indicator — right, and it undercut the whole point of the setting.
|
|
Closing as part of the open-PR backlog sweep (wave 1). Reason: Jul conflicted transcript auto-scroll; streaming list virtualization landed later. Reopen if this is still wanted and you’re willing to rebase onto current |
2 participants
The chat transcript follows the live edge while an agent streams, which fires a scroll once per chunk. Over a long session that continuous movement is tiring to read, and there is no way to turn it off.
How it's fixed
Two per-device client settings in Settings → Appearance:
chatAutoScrollgates live-follow, defaulting to off. The transcript holds still while output streams; the send-time anchor still positions the new turn once, so you are not left looking at old messages; and the scroll-to-end pill becomes the way back to the edge.reduceMotionforces the reduced-motion path on regardless of the OS preference — scrolls become instant instead of animated, and the looping indicator animations stop (status pulse, skeleton shimmer, working indicator, ultrathink gradients).Two details worth review attention:
The pill's visibility guard needed the setting too. It treats "left the live edge" as the list settling rather than the reader navigating — which is only true while something is actually following. Without that, turning follow off hid the pill exactly when it became the only way back.
Spinners are deliberately left spinning under reduce-motion. A frozen spinner reads as a hung app, which is worse than the motion it removes.
Verification
tsgo --noEmit,vp test run --project unit(1666 tests),vp lint, andvp buildall pass on this branch against currentmain.MessagesTimeline.test.tsxgains a case asserting the list only re-pins to the end when the setting is on.Screenshots
The new rows in Settings → Appearance:
This change is behavioural rather than visual — the difference is motion during streaming, which a still image can't show. Happy to record a short clip if that's wanted before review.
Written by Claude Opus 4.5 in Claude Code.
Note
Medium Risk
Default-off live-follow changes behavior for all users during streaming, and scroll-mode/pill logic in ChatView is easy to regress; changes are UI-only with no auth or data impact.
Overview
Behavior change: streaming transcripts no longer auto-follow the live edge by default. A new
chatAutoScrollclient setting (default off) gates per-chunk live-follow inChatViewand LegendListmaintainScrollAtEndinMessagesTimeline. Send-time turn anchoring still runs once; the scroll-to-end pill is shown when the reader is behind the edge, with settling guards updated so the pill is not suppressed when follow is off.Adds
reduceMotion(default off) plus a sharedprefersReducedMotionhelper anddata-reduce-motionon<html>so scroll jumps (anchor, pill, minimap), draft-hero / mobile composer transitions, and looping status animations respect the setting or OSprefers-reduced-motion(spinners stay animated).Settings → Appearance exposes both toggles, restore-defaults, and user docs. Chat timeline tests assert list re-pin only when auto-scroll is enabled. Minor RTL-oriented class swaps (
ps/ms/start,force-ltron tool output).Reviewed by Cursor Bugbot for commit c376c94. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Stop transcript auto-scrolling on every streamed chunk with a new "Follow agent output" setting
chatAutoScrollandreduceMotionboolean client settings (both defaulting tofalse) insettings.ts; whenchatAutoScrollis off the message list no longer re-pins to the end during streaming.LIVE_FOLLOW_SETTLE_MS) inChatView.tsxto suppress the scroll-to-end pill from flashing during thread switches and post-send transitions.prefersReducedMotionutility inreducedMotion.tsthat checks both the new setting and the OSprefers-reduced-motionmedia query; mirrors the result tohtml[data-reduce-motion]for CSS.index.cssdisables continuous animations (pulses, shimmers, view transitions) when reduced motion is active.SettingsPanels.tsx.chatAutoScroll: false, meaning auto-follow of streaming output is off by default for all users.Macroscope summarized c376c94.