fix(cli): eliminate terminal scroll flicker and rerender storm#310
Conversation
193549e to
366bd6c
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes persistent terminal scroll flicker and excessive re-rendering in the Ink-based CLI shell that made interactive sessions unusable on Windows/PowerShell. It addresses four root causes: Ink's clearTerminal firing on every render, high-frequency animation timers, unstable component keys, and layout jumping. Several pre-existing test failures on Windows are also fixed.
Changes:
- Core rendering architecture (
App.tsx,MessageStream.tsx): Fixed viewport height pinning, moved current messages to live region instead of Static scrollback, added stable timestamp-based keys, added responsivemaxVisible, and added version footer. - Re-render throttling (
AgentPanel.tsx,InputPrompt.tsx,MessageStream.tsx): Reduced animation timer frequencies and added ref-gated state updates to skip no-op re-renders. - Build & test fixes (
bump-build.mjs,terminal.ts, test files): Fixed invalid 4-part semver output, deduplicated terminal hooks with debouncing, and fixed Windows-specific test failures.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/squad-cli/src/cli/shell/components/App.tsx |
Fixed viewport height, stable keys, live region layout, maxVisible, version footer |
packages/squad-cli/src/cli/shell/components/MessageStream.tsx |
Elapsed-time dedup, system message layout, removed @mention fallback |
packages/squad-cli/src/cli/shell/components/AgentPanel.tsx |
Ref-gated elapsed tick to skip no-op re-renders, wider pulse interval |
packages/squad-cli/src/cli/shell/components/InputPrompt.tsx |
Spinner interval widened from 80ms to 150ms |
packages/squad-cli/src/cli/shell/terminal.ts |
Shared useTerminalDimension hook, 150ms debounced resize, 50-row fallback |
packages/squad-cli/src/cli/shell/index.ts |
Documents Ink runtime patches, viewport clear before render |
scripts/bump-build.mjs |
Non-prerelease builds now use valid semver X.Y.Z-build.N |
CHANGELOG.md |
Added [Unreleased] section documenting the rendering fixes |
test/bump-build.test.ts |
Updated assertion for new semver format |
test/cli-packaging-smoke.test.ts |
Added SKIP_BUILD_BUMP=1 to prevent version mutation during test |
test/marketplace.test.ts |
UUID-based non-existent path for Windows compatibility |
test/repl-ux.test.ts |
Pass activityHint prop to match new MessageStream architecture |
366bd6c to
bceeae1
Compare
|
@copilot please re-review the latest changes addressing all 4 review threads. |
bceeae1 to
2e78a94
Compare
|
@copilot please re-review the latest changes. |
2e78a94 to
0a445b4
Compare
|
@copilot please re-review the latest changes. |
0a445b4 to
916617c
Compare
|
@copilot please re-review the latest changes. MQ quality gate applied: regex safety fix in patch script, memoized roleMap, inlined resolvedHint alias, idiomatic Array.find, corrected CHANGELOG description. |
…migration feat: Wave 3 — Documentation Epic complete (bradygaster#182)
Root cause: four independent issues combined into a scroll storm: 1. Ink fullscreen clearTerminal path firing every render cycle 2. ~16 unsynchronized animation re-renders/sec from 3 timers 3. Unstable Static component keys causing Ink remounts 4. Layout shift from height toggling between processing states Changes: - Patch Ink fullscreen path (disable clearTerminal, incrementalRendering, trailing newline) - Widen spinner/animation intervals (80->150ms spinner, 500->800ms pulse) - Share terminal dimension hook with 150ms debounce - Pin root height to prevent logUpdate cursor drift - Keep conversation in live viewport (not Static scrollback) - Stable UUID-based Static keys, responsive maxVisible - Fix bump-build.mjs to produce valid semver prerelease format - Fix marketplace test for Windows path compat Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
916617c to
a8020c7
Compare
Squad Team Review — PR #310Thanks for this, Jon — this is exceptional systems work. You found four independent root causes that our team wouldn't have caught through normal code review. All four required library-internals archaeology across React → Ink → log-update → terminal emulation, and the fixes are surgical. What our team foundFlight (Architecture): The design philosophy shift from React is cheap, render everything to only render when pixels change is the right model for TUI apps. The bounded reactive system with explicit render budgets is now defensible. LGTM. INCO (CLI UX): A+ grade. Stable viewport, smoother animations (slower timers feel faster because they don't cause flicker), and the pinned-input model matches modern chat UIs. The animation slowdowns (spinner 80→150ms, pulse 500→800ms) are the right trade-off — consistency beats speed. FIDO (Quality): Build passes clean. 3,931 tests pass. The two test failures (shell-metrics, remote-control) are pre-existing on main — confirmed by running the same tests against main. Zero regressions introduced by this PR. The four root causes you caught
Follow-ups we'll track separately
Ship it. 🚀 |
Merge upstream/main to incorporate recent improvements: - Fix: Resolve CLI scroll flicker and re-render storms (bradygaster#310) - Docs: Remote Q&A scenario, .squad/ directory explainer, decision framework - Docs: Node.js version alignment, contributor credits - UI: Open Graph/Twitter Cards, Astro Image component, navbar improvements - Release: v0.8.25 with CLI packaging smoke test Preserved Squadro-5 branding in README while incorporating upstream improvements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nt updates Session: 2026-03-10-pr310-merge-and-abstract Changes: - Orchestration logs for PAO (presentation abstract), Flight (arch review), INCO (UX review), FIDO (build+test) - Session log for PR #310 merge and abstract delivery - Cross-agent updates: INCO and FIDO history.md include Ink root cause findings and postinstall patch pattern - Decision inbox: empty (no new decisions) - Team updates: PR #310 merged, 3,931 tests pass (zero regressions), presentation abstract delivered Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes persistent scroll-to-top flicker and excessive re-rendering in the Ink-based CLI shell that made the interactive terminal unusable in PowerShell and standard terminals. The root cause was a cascade of issues: Ink's fullscreen
clearTerminalpath firing on every render, high-frequency animation timers triggering unnecessary React re-renders, unstable component keys causing remounts, and the live viewport layout pushing the input prompt below the fold.Closes #309
Root Cause
Four independent issues combined into the scroll storm:
clearTerminal— Ink's render path calledclearTerminal()(writes\x1b[2J\x1b[3J\x1b[H) on every render cycle, forcing the viewport to scroll-to-topprocessing=true, causing the layout to jump when toggling statesChanges
Core Rendering Architecture (App.tsx)
height={rootHeight}on the root<Box>sologUpdatetracks exactly that many lines — no cursor drift or scroll-to-top<Static>scrollback, preventing the "conversation vanishes on re-render" problemmaxVisible: Derives visible message count from terminal height (Math.floor((terminalHeight - 8) / 3)) so taller terminals show more contexttimestamp + index-at-creationinstead of shifting array indices to prevent Ink remountscontentWidthto Ink'suseStdout().columnsto prevent text overflow/clippingv{version}at the bottom of the viewportRe-render Throttling
AgentPanel.tsx(PulsingDot)AgentPanel.tsx(elapsed)InputPrompt.tsx(spinner)MessageStream.tsx(elapsed)setElapsedMsevery 1slastElapsedSecRefskips setState when rounded seconds unchangedCombined effect: ~16 re-renders/sec down to ~7/sec, with most being no-ops that React skips via referential equality.
Terminal Utilities (terminal.ts)
useTerminalDimension(getter)—useTerminalWidthanduseTerminalHeightwere copy-paste duplicatesprocess.stdoutresize events to batch rapid resize sequencesgetTerminalHeight()falls back to 50 rows (instead of 24) so piped/test environments have enough room for/helpoutputInk Runtime Patches (index.ts)
ink.js:incrementalRendering(standard erase-and-rewrite instead)\nappended to outputclearTerminalscroll-to-top path\x1b[2J\x1b[3J\x1b[H) before Ink render to start from a clean viewportDead Code & Smell Cleanup
@mentionfallback fromMessageStream—App.tsxalready derivesmentionHintand passes it viaactivityHintBuild & Test Fixes
scripts/bump-build.mjs0.8.25.4) rejected by npm as invalid semver0.8.25-build.N)test/bump-build.test.tsX.Y.Z-build.Nformattest/cli-packaging-smoke.test.tsSKIP_BUILD_BUMP=1in test envtest/marketplace.test.ts/nonexistentresolves toD:\nonexistenton Windows (exists)tmpDirtest/repl-ux.test.ts@mentiontests relied on removed MessageStream fallbackactivityHintprop to match new architectureFiles Changed (12 files, +158/-80)
CHANGELOG.mdpackages/squad-cli/src/cli/shell/components/AgentPanel.tsxpackages/squad-cli/src/cli/shell/components/App.tsxpackages/squad-cli/src/cli/shell/components/InputPrompt.tsxpackages/squad-cli/src/cli/shell/components/MessageStream.tsxpackages/squad-cli/src/cli/shell/index.tspackages/squad-cli/src/cli/shell/terminal.tsscripts/bump-build.mjstest/bump-build.test.tstest/cli-packaging-smoke.test.tsSKIP_BUILD_BUMPenv for pack stabilitytest/marketplace.test.tstest/repl-ux.test.tsactivityHintfor @mention testsVerification
npm run buildpasses