mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
NeilandOrca
97cb32c1cc
* fix(terminal): release an abandoned synchronized-output frame on reveal Alt-screen agent TUIs (OpenCode/OpenTUI, Codex, grok) bracket every repaint in `?2026h … ?2026l`. Hiding a pane mid-bracket — which a worktree switch or cold-park lands on routinely, since these brackets are written many times a second — leaves xterm's `decPrivateModes.synchronizedOutput` latched. RenderService.refreshRows checks that latch *before* rendering, so while it holds, every repaint Orca owns is a no-op: the forced render-pause repaint, the plain `refresh()` fallback, and the shared glyph-atlas rebuild all render zero rows while the xterm buffer is perfectly correct. Release the latch at the two reveal repaint entry points so those repaints actually paint. Also adds an OpenCode-shaped alt-screen e2e fixture and spec. The existing inline-TUI convergence spec covers the normal-buffer shape (live block glued to the bottom, history scrolling into scrollback); this covers the full-screen alternate-buffer shape, where nothing scrolls and so no row ever self-heals through the scroll path. Scope note: xterm arms a 1s watchdog that clears this latch on its own, so this closes a bounded window rather than the whole STA-2694 report. The e2e spec passes with and without the production change for that reason; the unit tests are what pin the behavior. Refs STA-2694. * fix(terminal): clear the render model on the plain-refocus repaint path `schedulePaneRevealPresent` — the atlas-preserving path a plain window refocus takes — only called `terminal.refresh()`. xterm's renderers are diff-based: `_updateModel` early-continues on any cell whose code/fg/bg/ext still match the cached model, so a refresh repaints nothing for a pane whose buffer never changed. When an occluded window loses its canvas contents while that model stays populated, the refresh skips exactly the cells that went stale and the pane keeps compositing pre-hide pixels — until a window resize reallocates the model, which is the repair users find by hand. Clear the model first (`RenderService.clear()` → renderer `clear()` → `_clearModel(true)`) so the refresh becomes a guaranteed full repaint. That drops cached cells and glyph vertices but NOT the texture atlas, which is shared by every same-config terminal and whose mid-stream wipe re-arms xterm's page-merge garble race (xterm.js #4480) — the reason this path is atlas-preserving in the first place. Also covers the DOM-renderer fallback in `resetWebglTextureAtlas`: `clearTextureAtlas()` is what invalidated the model on the WebGL path, so a pane without an addon had nothing invalidate it and hit the same skip. Scope note: the e2e spec guards buffer/geometry convergence across the hide/reveal boundaries and adds idle-agent and headful desktop-hide cases, but it cannot observe a stale canvas — both oracles built for that (canvas-vs-buffer ink sampling, screenshot-vs-forced-repaint) were proven blind by injecting the defect, and the spec header documents why. The unit tests pin the ordering and the atlas-preservation invariant. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * docs(terminal): hand off the STA-2694 reveal-artifact investigation Records both fixed defects with their xterm mechanisms, the reveal/wake call graph, why every e2e oracle for a stale canvas was proven blind, how to arm the in-app render-desync sentinel on real hardware, and the one unverified lead (dimension staleness) that would explain why a window resize specifically is the repair users find. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * Revert "fix(terminal): clear the render model on the plain-refocus repaint path" This reverts commit0f7ec4458d. * test(terminal): add a draw-command oracle for reveal repaints, and correct the STA-2694 scope Every pixel oracle tried for STA-2694 was blind: `drawImage` on a non-preserveDrawingBuffer WebGL canvas returns a re-rendered copy, and Playwright's screenshot drives a fresh compositor frame that heals a stale paint before capture. Reading pixels is self-defeating here — the read triggers the repaint that hides the bug. Count the WebGL draw commands instead, by wrapping GlyphRenderer.updateCell and gl.drawElementsInstanced on the live pane. A draw command cannot be healed after the fact, so "did the reveal actually repaint?" becomes directly observable. Teeth-verified: removing releaseAbandonedSynchronizedOutput from schedulePaneRevealPresent fails the stranded-latch test. Two findings, both of which change previously-committed claims: 1. The 1s watchdog does NOT bound the synchronized-output defect. It is armed only inside `bufferRows`, and `refreshRows` returns at its `_isPaused` check first — so while a pane is occluded nothing reaches `bufferRows` and no timer is ever pending. A pane hidden mid-`?2026h` holds the latch with no watchdog behind it, indefinitely. ed1eaf55f1's "closes a bounded window" scope note was wrong; this is the unbounded garble the report describes, and the fix closes it. Corrected in the module doc comment. 2. It refutes the diff-based-staleness hypothesis behind0f7ec4458d(reverted in8d5eacecb4). `_updateModel` does early-continue per unchanged cell, but `GlyphRenderer.render` then copies vertices for EVERY row up to `lineLengths[y]` and issues ONE full-viewport draw — measured identical instance counts (562) for a diff-skipped and a model-cleared refresh, with updateCell at 0 vs 561. The DOM renderer likewise replaceChildren()s every row unconditionally. Clearing the model could not change what reached the screen, and `_clearModel(true)` zeroes every glyph vertex while `RenderService.clear()` fires no repaint of its own — so it opened a blank-viewport window (also asserted here) for no benefit. Also keeps the idle-agent and headful desktop-hide cases from the reverted commit, since those were independent of the refuted production change, and rewrites the alt-screen spec header to point paint questions at this oracle. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * docs(terminal): rewrite the STA-2694 handoff after the refutation Records that the garble window is unbounded (the 1s watchdog never arms for an occluded pane), that the diff-based-staleness hypothesis was refuted by measurement and reverted, why pixel oracles are structurally blind here, and the two leads now closed by measurement (dimension staleness, lazy atlas bindings). Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * test(terminal): capture visual proof of the STA-2694 stale paint The earlier screenshot oracles were blind because they compared a revealed pane against a repaired one and both ran the same repaint code. Capturing the defect directly works instead, because the mechanism is self-preserving: while synchronizedOutput is latched, refreshRows returns before reaching the renderer, so a compositor frame just re-composites the existing canvas texture and the stale pixels survive the screenshot rather than being healed by it. Latch a frame, write a full new frame the pane cannot paint, and capture. The screenshot comes back byte-identical to the pre-hide one while the buffer holds the new frame — the buffer/screen divergence users report — and differs after the reveal repaint runs. Asserts both halves, so it fails if either the defect stops reproducing or the fix stops repairing it. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * test(terminal): note where the xterm gate-order double is pinned for real The unit double encodes RenderService's paused-then-latch gate order, which can drift on an xterm upgrade. Point at the e2e oracle that pins the same order against the real renderer, so a future upgrade has a trail to the authoritative check. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * test(terminal): add a perf budget for the synchronized-output release releaseAbandonedSynchronizedOutput runs inside resetWebglTextureAtlas, which a streaming alt-screen TUI can reach through the terminal-output atlas recovery path — not only on reveal. Measure rather than assert that this costs nothing. Steady state (a TUI that closes every frame it opens): 200 bracketed frames produce zero releases, zero extra draw calls, and an unmeasurable early-out cost. Worst case (every reveal finds a latched frame): 50 latched atlas resets at 0.08ms each. Both are asserted with headroom, so the guard catches a future change that makes this scan the buffer per pane rather than flaking on machine speed. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> * test(terminal): address review — drive real code paths, close vacuity gaps CodeRabbit caught a genuine tautology in the perf budget: it timed a hand-copied mirror of the early-out rather than the shipped function, so the assertion would have held even if the real code grew a buffer scan. Driving resetWebglTextureAtlases instead moved the measured cost from ~0 to ~0.03ms per call, which is the honest number for the whole recovery; bound re-set to 0.4ms (10x measured). Other review fixes: - Assert the draw counts both perf tests were measuring and logging but never checking, so the 'no extra draws' titles now mean something. - Fail fast when decPrivateModes is unavailable; previously the latched test would pass without ever exercising the fix. - Re-check the latch right after the worktree switch in the mid-frame test: the pane is visible until then, so the 1s watchdog can arm and clear it before the hide, making the run vacuous. - Count scheduleRevealPresent invocations instead of returning a literal true, so a missing test hook no longer masquerades as a production failure. - Assert the latch clears on every reveal iteration, not just the last. - Make the fixture heartbeat write atomic (tmp + rename); writeFileSync truncates first, so a reader could see '' and read it as frame 0. - Relabel assertRevealPixelsNeedNoRepair as the weak secondary check it is; it contradicted the file header by calling itself 'the decisive paint assertion'. Refs STA-2694. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>