mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
Merge pull request #14640 from NousResearch/bb/fix-tui-glyph-ghosting
fix(ui-tui): heal post-resize alt-screen drift
This commit is contained in:
commit
b6ca3c28dc
10 changed files with 240 additions and 15 deletions
|
|
@ -257,6 +257,7 @@ function ScrollBox({ children, ref, stickyScroll, ...style }: PropsWithChildren<
|
|||
|
||||
if (el) {
|
||||
el.scrollTop ??= 0
|
||||
el.notifyScrollChange = notify
|
||||
}
|
||||
}}
|
||||
style={{
|
||||
|
|
|
|||
|
|
@ -72,6 +72,7 @@ export type DOMElement = {
|
|||
scrollViewportHeight?: number
|
||||
scrollViewportTop?: number
|
||||
stickyScroll?: boolean
|
||||
notifyScrollChange?: () => void
|
||||
// Set by ScrollBox.scrollToElement; render-node-to-output reads
|
||||
// el.yogaNode.getComputedTop() (FRESH — same Yoga pass as scrollHeight)
|
||||
// and sets scrollTop = top + offset, then clears this. Unlike an
|
||||
|
|
|
|||
|
|
@ -245,6 +245,7 @@ export default class Ink {
|
|||
// microtask. Dims are captured sync in handleResize; only the
|
||||
// expensive tree rebuild defers.
|
||||
private pendingResizeRender = false
|
||||
private resizeSettleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
// Fold synchronous re-entry (selection fanout, onFrame callback)
|
||||
// into one follow-up microtask instead of stacking renders.
|
||||
|
|
@ -439,6 +440,11 @@ export default class Ink {
|
|||
this.drainTimer = null
|
||||
}
|
||||
|
||||
if (this.resizeSettleTimer !== null) {
|
||||
clearTimeout(this.resizeSettleTimer)
|
||||
this.resizeSettleTimer = null
|
||||
}
|
||||
|
||||
// Alt screen: reset frame buffers so the next render repaints from
|
||||
// scratch (prevFrameContaminated → every cell written, wrapped in
|
||||
// BSU/ESU — old content stays visible until the new frame swaps
|
||||
|
|
@ -456,6 +462,20 @@ export default class Ink {
|
|||
|
||||
this.resetFramesForAltScreen()
|
||||
this.needsEraseBeforePaint = true
|
||||
|
||||
// One last repaint after the resize burst settles closes any host-side
|
||||
// reflow drift the normal diff path can't see.
|
||||
this.resizeSettleTimer = setTimeout(() => {
|
||||
this.resizeSettleTimer = null
|
||||
|
||||
if (!this.canAltScreenRepaint()) {
|
||||
return
|
||||
}
|
||||
|
||||
this.resetFramesForAltScreen()
|
||||
this.needsEraseBeforePaint = true
|
||||
this.render(this.currentNode!)
|
||||
}, 160)
|
||||
}
|
||||
|
||||
// Already queued: later events in this burst updated dims/alt-screen
|
||||
|
|
@ -477,6 +497,17 @@ export default class Ink {
|
|||
this.render(this.currentNode)
|
||||
})
|
||||
}
|
||||
|
||||
private canAltScreenRepaint(): boolean {
|
||||
return (
|
||||
!this.isUnmounted &&
|
||||
!this.isPaused &&
|
||||
this.altScreenActive &&
|
||||
!!this.options.stdout.isTTY &&
|
||||
this.currentNode !== null
|
||||
)
|
||||
}
|
||||
|
||||
resolveExitPromise: () => void = () => {}
|
||||
rejectExitPromise: (reason?: Error) => void = () => {}
|
||||
unsubscribeExit: () => void = () => {}
|
||||
|
|
@ -1935,6 +1966,11 @@ export default class Ink {
|
|||
this.drainTimer = null
|
||||
}
|
||||
|
||||
if (this.resizeSettleTimer !== null) {
|
||||
clearTimeout(this.resizeSettleTimer)
|
||||
this.resizeSettleTimer = null
|
||||
}
|
||||
|
||||
reconciler.updateContainerSync(null, this.container, null, noop)
|
||||
reconciler.flushSyncWork()
|
||||
instances.delete(this.options.stdout)
|
||||
|
|
|
|||
115
ui-tui/packages/hermes-ink/src/ink/log-update.test.ts
Normal file
115
ui-tui/packages/hermes-ink/src/ink/log-update.test.ts
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { Frame } from './frame.js'
|
||||
import { LogUpdate } from './log-update.js'
|
||||
import { CellWidth, CharPool, createScreen, HyperlinkPool, type Screen, setCellAt, StylePool } from './screen.js'
|
||||
|
||||
/**
|
||||
* Contract tests for LogUpdate.render() — the diff-to-ANSI path that owns
|
||||
* whether the terminal picks up each React commit correctly.
|
||||
*
|
||||
* These tests pin down a few load-bearing invariants so that any fix for
|
||||
* the "scattered letters after rapid resize" artifact in xterm.js hosts
|
||||
* can be grounded against them.
|
||||
*/
|
||||
|
||||
const stylePool = new StylePool()
|
||||
const charPool = new CharPool()
|
||||
const hyperlinkPool = new HyperlinkPool()
|
||||
|
||||
const mkScreen = (w: number, h: number) => createScreen(w, h, stylePool, charPool, hyperlinkPool)
|
||||
|
||||
const paint = (screen: Screen, y: number, text: string) => {
|
||||
for (let x = 0; x < text.length; x++) {
|
||||
setCellAt(screen, x, y, {
|
||||
char: text[x]!,
|
||||
styleId: stylePool.none,
|
||||
width: CellWidth.Narrow,
|
||||
hyperlink: undefined
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const mkFrame = (screen: Screen, viewportW: number, viewportH: number): Frame => ({
|
||||
screen,
|
||||
viewport: { width: viewportW, height: viewportH },
|
||||
cursor: { x: 0, y: 0, visible: true }
|
||||
})
|
||||
|
||||
const stdoutOnly = (diff: ReturnType<LogUpdate['render']>) =>
|
||||
diff
|
||||
.filter(p => p.type === 'stdout')
|
||||
.map(p => (p as { type: 'stdout'; content: string }).content)
|
||||
.join('')
|
||||
|
||||
describe('LogUpdate.render diff contract', () => {
|
||||
it('emits only changed cells when most rows match', () => {
|
||||
const w = 20
|
||||
const h = 4
|
||||
const prev = mkScreen(w, h)
|
||||
paint(prev, 0, 'HELLO')
|
||||
paint(prev, 1, 'WORLD')
|
||||
paint(prev, 2, 'STAYSHERE')
|
||||
|
||||
const next = mkScreen(w, h)
|
||||
paint(next, 0, 'HELLO')
|
||||
paint(next, 1, 'CHANGE')
|
||||
paint(next, 2, 'STAYSHERE')
|
||||
next.damage = { x: 0, y: 0, width: w, height: h }
|
||||
|
||||
const log = new LogUpdate({ isTTY: true, stylePool })
|
||||
const diff = log.render(mkFrame(prev, w, h), mkFrame(next, w, h), true, false)
|
||||
|
||||
const written = stdoutOnly(diff)
|
||||
expect(written).toContain('CHANGE')
|
||||
expect(written).not.toContain('HELLO')
|
||||
expect(written).not.toContain('STAYSHERE')
|
||||
})
|
||||
|
||||
it('width change emits a clearTerminal patch before repainting', () => {
|
||||
const prevW = 20
|
||||
const nextW = 15
|
||||
const h = 3
|
||||
|
||||
const prev = mkScreen(prevW, h)
|
||||
paint(prev, 0, 'thiswaswiderrow')
|
||||
|
||||
const next = mkScreen(nextW, h)
|
||||
paint(next, 0, 'shorterrownow')
|
||||
next.damage = { x: 0, y: 0, width: nextW, height: h }
|
||||
|
||||
const log = new LogUpdate({ isTTY: true, stylePool })
|
||||
const diff = log.render(mkFrame(prev, prevW, h), mkFrame(next, nextW, h), true, false)
|
||||
|
||||
expect(diff.some(p => p.type === 'clearTerminal')).toBe(true)
|
||||
expect(stdoutOnly(diff)).toContain('shorterrownow')
|
||||
})
|
||||
|
||||
it('drift repro: identical prev/next emits no heal, even when the physical terminal is stale', () => {
|
||||
// Load-bearing theory for the rapid-resize scattered-letter bug: if the
|
||||
// physical terminal has stale cells that prev.screen doesn't know about
|
||||
// (e.g. resize-induced reflow wrote past ink's tracked range), the
|
||||
// renderer has no signal to heal them. LogUpdate.render only sees
|
||||
// prev/next — no view of the physical terminal — so when prev==next,
|
||||
// it emits nothing and any orphaned glyphs survive.
|
||||
//
|
||||
// The fix path is upstream of this diff: either (a) defensively
|
||||
// full-repaint on xterm.js frames where prevFrameContaminated is set,
|
||||
// or (b) close the drift window so prev.screen cannot diverge.
|
||||
const w = 20
|
||||
const h = 3
|
||||
|
||||
const prev = mkScreen(w, h)
|
||||
paint(prev, 0, 'same')
|
||||
|
||||
const next = mkScreen(w, h)
|
||||
paint(next, 0, 'same')
|
||||
next.damage = { x: 0, y: 0, width: w, height: h }
|
||||
|
||||
const log = new LogUpdate({ isTTY: true, stylePool })
|
||||
const diff = log.render(mkFrame(prev, w, h), mkFrame(next, w, h), true, false)
|
||||
|
||||
expect(stdoutOnly(diff)).toBe('')
|
||||
expect(diff.some(p => p.type === 'clearTerminal')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -761,6 +761,7 @@ function renderNodeToOutput(
|
|||
// active text selection by the same delta (native terminal behavior:
|
||||
// view keeps scrolling, highlight walks up with the text).
|
||||
const scrollTopBeforeFollow = node.scrollTop ?? 0
|
||||
const stickyBeforeFollow = node.stickyScroll
|
||||
|
||||
const sticky = node.stickyScroll ?? Boolean(node.attributes['stickyScroll'])
|
||||
|
||||
|
|
@ -863,6 +864,10 @@ function renderNodeToOutput(
|
|||
scrollDrainNode = node
|
||||
}
|
||||
|
||||
if ((node.scrollTop ?? 0) !== scrollTopBeforeFollow || node.stickyScroll !== stickyBeforeFollow) {
|
||||
node.notifyScrollChange?.()
|
||||
}
|
||||
|
||||
scrollTop = clamped
|
||||
|
||||
if (content && contentYoga) {
|
||||
|
|
|
|||
31
ui-tui/src/__tests__/viewport.test.ts
Normal file
31
ui-tui/src/__tests__/viewport.test.ts
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { stickyPromptFromViewport } from '../domain/viewport.js'
|
||||
|
||||
describe('stickyPromptFromViewport', () => {
|
||||
it('hides the sticky prompt when a newer user message is already visible', () => {
|
||||
const messages = [
|
||||
{ role: 'user' as const, text: 'older prompt' },
|
||||
{ role: 'assistant' as const, text: 'older answer' },
|
||||
{ role: 'user' as const, text: 'current prompt' },
|
||||
{ role: 'assistant' as const, text: 'current answer' }
|
||||
]
|
||||
|
||||
const offsets = [0, 2, 10, 12, 20]
|
||||
|
||||
expect(stickyPromptFromViewport(messages, offsets, 8, 16, false)).toBe('')
|
||||
})
|
||||
|
||||
it('shows the latest user message above the viewport when no user message is visible', () => {
|
||||
const messages = [
|
||||
{ role: 'user' as const, text: 'older prompt' },
|
||||
{ role: 'assistant' as const, text: 'older answer' },
|
||||
{ role: 'user' as const, text: 'current prompt' },
|
||||
{ role: 'assistant' as const, text: 'current answer' }
|
||||
]
|
||||
|
||||
const offsets = [0, 2, 10, 12, 20]
|
||||
|
||||
expect(stickyPromptFromViewport(messages, offsets, 16, 20, false)).toBe('current prompt')
|
||||
})
|
||||
})
|
||||
|
|
@ -658,11 +658,34 @@ export function useMainApp(gw: GatewayClient) {
|
|||
[cols, composerActions, composerState, empty, pagerPageSize, submit]
|
||||
)
|
||||
|
||||
const appProgress = useMemo(
|
||||
const liveTailVisible = (() => {
|
||||
const s = scrollRef.current
|
||||
|
||||
if (!s) {
|
||||
return true
|
||||
}
|
||||
|
||||
const top = Math.max(0, s.getScrollTop() + s.getPendingDelta())
|
||||
const vp = Math.max(0, s.getViewportHeight())
|
||||
const total = Math.max(vp, s.getScrollHeight())
|
||||
|
||||
return top + vp >= total - 3
|
||||
})()
|
||||
|
||||
const liveProgress = useMemo(
|
||||
() => ({ ...turn, showProgressArea, showStreamingArea: Boolean(turn.streaming) }),
|
||||
[turn, showProgressArea]
|
||||
)
|
||||
|
||||
const frozenProgressRef = useRef(liveProgress)
|
||||
|
||||
// Freeze the offscreen live tail so scroll doesn't rebuild unseen streaming UI.
|
||||
if (liveTailVisible || !ui.busy) {
|
||||
frozenProgressRef.current = liveProgress
|
||||
}
|
||||
|
||||
const appProgress = liveTailVisible || !ui.busy ? liveProgress : frozenProgressRef.current
|
||||
|
||||
const cwd = ui.info?.cwd || process.env.HERMES_CWD || process.cwd()
|
||||
const gitBranch = useGitBranch(cwd)
|
||||
|
||||
|
|
|
|||
|
|
@ -249,22 +249,15 @@ export function StickyPromptTracker({ messages, offsets, scrollRef, onChange }:
|
|||
useSyncExternalStore(
|
||||
useCallback((cb: () => void) => scrollRef.current?.subscribe(cb) ?? (() => {}), [scrollRef]),
|
||||
() => {
|
||||
const s = scrollRef.current
|
||||
const { atBottom, top } = getStickyViewport(scrollRef.current)
|
||||
|
||||
if (!s) {
|
||||
return NaN
|
||||
}
|
||||
|
||||
const top = Math.max(0, s.getScrollTop() + s.getPendingDelta())
|
||||
|
||||
return s.isSticky() ? -1 - top : top
|
||||
return atBottom ? -1 - top : top
|
||||
},
|
||||
() => NaN
|
||||
)
|
||||
|
||||
const s = scrollRef.current
|
||||
const top = Math.max(0, (s?.getScrollTop() ?? 0) + (s?.getPendingDelta() ?? 0))
|
||||
const text = stickyPromptFromViewport(messages, offsets, top, s?.isSticky() ?? true)
|
||||
const { atBottom, bottom, top } = getStickyViewport(scrollRef.current)
|
||||
const text = stickyPromptFromViewport(messages, offsets, top, bottom, atBottom)
|
||||
|
||||
useEffect(() => onChange(text), [onChange, text])
|
||||
|
||||
|
|
@ -389,3 +382,15 @@ interface TranscriptScrollbarProps {
|
|||
scrollRef: RefObject<ScrollBoxHandle | null>
|
||||
t: Theme
|
||||
}
|
||||
|
||||
function getStickyViewport(s?: ScrollBoxHandle | null) {
|
||||
const top = Math.max(0, (s?.getScrollTop() ?? 0) + (s?.getPendingDelta() ?? 0))
|
||||
const vp = Math.max(0, s?.getViewportHeight() ?? 0)
|
||||
const total = Math.max(vp, s?.getScrollHeight() ?? vp)
|
||||
|
||||
return {
|
||||
atBottom: (s?.isSticky() ?? true) || top + vp >= total - 2,
|
||||
bottom: top + vp,
|
||||
top
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -237,6 +237,8 @@ const ComposerPane = memo(function ComposerPane({
|
|||
)}
|
||||
|
||||
{!composer.empty && !ui.sid && <Text color={ui.theme.color.dim}>⚕ {ui.status}</Text>}
|
||||
|
||||
<StatusRulePane at="bottom" composer={composer} status={status} />
|
||||
</NoSelect>
|
||||
)
|
||||
})
|
||||
|
|
@ -320,8 +322,6 @@ export const AppLayout = memo(function AppLayout({
|
|||
/>
|
||||
|
||||
<ComposerPane actions={actions} composer={composer} status={status} />
|
||||
|
||||
<StatusRulePane at="bottom" composer={composer} status={status} />
|
||||
</>
|
||||
)}
|
||||
</Box>
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ export const stickyPromptFromViewport = (
|
|||
messages: readonly Msg[],
|
||||
offsets: ArrayLike<number>,
|
||||
top: number,
|
||||
bottom: number,
|
||||
sticky: boolean
|
||||
) => {
|
||||
if (sticky || !messages.length) {
|
||||
|
|
@ -26,8 +27,15 @@ export const stickyPromptFromViewport = (
|
|||
}
|
||||
|
||||
const first = Math.max(0, Math.min(messages.length - 1, upperBound(offsets, top) - 1))
|
||||
const last = Math.max(first, Math.min(messages.length - 1, upperBound(offsets, bottom) - 1))
|
||||
|
||||
for (let i = first; i >= 0; i--) {
|
||||
for (let i = first; i <= last; i++) {
|
||||
if (messages[i]?.role === 'user') {
|
||||
return ''
|
||||
}
|
||||
}
|
||||
|
||||
for (let i = first - 1; i >= 0; i--) {
|
||||
if (messages[i]?.role !== 'user') {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue