Reduce echo output latency - #179
Merged
Merged
Conversation
Change-Id: I046454a6ef3f44d7671de0ca0b21f85e25509ddc Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
matifali
approved these changes
Jun 26, 2026
piclaw-bot
pushed a commit
to rcarmo/ghostty-web
that referenced
this pull request
Jul 29, 2026
Signed-off-by: Thomas Kosiewski <tk@coder.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #161.
This reduces perceived keystroke echo latency by tracking when user-originated input has been emitted to stdin and synchronously rendering the first subsequent write that may contain the PTY echo. The marker is consumed immediately, so bulk stdout continues to rely on the existing animation-frame render loop.
Changes
awaitingEchostate inTerminal.onDatafires for keyboard input,paste(), andinput(data, true).writeInternal()after the WASM write/response processing, then clear the marker.onDataechoes, programmatic input, disabled stdin, and subsequent bulk output.Validation
bun run build— passed; regeneratedghostty-vt.wasmso tests could load the WASM artifact in this checkout.bun run fmt && bun run lint && bun run typecheck && bun test && bun run build— passed.ghostty-vt.wasmbuilt and Vite library build completed.Verifier findings
The implementation-loop verifier reviewed the issue context and final commit, confirmed the changed paths are limited to
lib/terminal.tsandlib/terminal.test.ts, and found no P1-P3 issues. It also dogfooded the interactive demo withPORT=8000 bun run demo:dev, typedecho issue-161-latency, and ran high-outputprintf '%s\n' {1..120}without obvious regression.Dogfooding screenshots/video were captured locally under
/tmp/issue161-dogfood/. Uploading those artifacts to GitHub from this headless worker was blocked becausegh-imagehad nouser_sessioncookie orGH_SESSION_TOKEN.📋 Implementation Plan
Implementation Plan for #161 — Reduce latency for echo output
Issue summary
coder/ghostty-web#161— “Reduce latency for echo output”accepted,feature,triage:donerequestAnimationFrametick. This reduces perceived keystroke echo latency while avoiding synchronous rendering for bulk stdout.Terminalstate flag (awaitingEcho) and targeted regression tests. Do not expand into renderer architecture work, WebGL work, PTY/demo server changes, Vite proxy changes, or broader performance tuning.Evidence reviewed
gh issue view 161 --repo coder/ghostty-web --commentsandgh issue view 161 --repo coder/ghostty-web --json ....awaitingEchoboolean set on stdin sends and consumed by a synchronous render on the next write.echo latency,render latency, andsluggish typing; only Reduce latency for echo output #161 matched the first two searches.diegosouzapw/ghostty-web@740452awas inspected as prior art. It changes onlylib/terminal.tsandlib/terminal.test.tsfor the sameawaitingEchopattern; use it as guidance, not as a blind patch.lib/terminal.tsInputHandlercallback inopen()around lines 448–461write()/writeInternal()around lines 541–596paste()around lines 617–633input()around lines 641–656startRenderLoop()around lines 1155–1182lib/renderer.tsCanvasRenderer.render(...)around lines 267–503; it clears dirty flags viabuffer.clearDirty()after renderinglib/terminal.test.tspaste()tests around lines 438–496input()tests around lines 554–637disableStdintests around lines 2581–2725lib/test-helpers.tscreateIsolatedTerminal(...)helper around lines 32–37demo/bin/demo.jsanddemo/README.mdbun run demo:devruns the demo through Vite with the/wsPTY handler attached on the same origin/port.Non-goals / scope boundaries
lib/renderer.tsunless tests reveal the currentrender()signature has changed unexpectedly.bun run demo:devfor the interactive PTY demo.needs-triageissue only if none exists.Recommended implementation approach
Phase 1 — Add regression tests first
Quality gate: run the targeted tests before implementation and confirm the new echo-latency tests fail for the expected reason.
describe('echo latency optimization', ...)block tolib/terminal.test.ts, near the existing write behavior/input tests.createIsolatedTerminal({ cols: 80, rows: 24 }), create a DOM container inbeforeEach, callterm.open(container), and alwaysterm.dispose()in cleanup.term.renderer!.renderafterterm.open(container)so the synchronous initial render fromstartRenderLoop()is not counted.renderer.render(...)calls, rather than depending primarily on a private flag.input('x', true)followed bywrite('x')triggers exactly one synchronous render duringwriteInternal().write('more output')without another user input does not trigger another synchronous render.input('x', false)writes/programmatic input without setting the echo path; a followingwrite('x')does not synchronously render.paste('hello')followed bywrite('hello')triggers one synchronous render.disableStdin: trueblocks the echo marker path for bothinput(data, true)andpaste(data); a following write should not synchronously render.KeyboardEvent('keydown', { key: 'a', code: 'KeyA', keyCode: 65, bubbles: true, cancelable: true })on the opened container, verifyonDatafires as existing tests do, thenwrite('a')and assert one synchronous render. Existing tests already use this pattern; keep a fallback to callback/input-path coverage only if this demonstrably flakes in Happy DOM, and document the reason.Suggested assertion pattern:
Implementation note for tests: avoid
awaitbetween setting up the spy and asserting the synchronous render, because the background rAF loop can legitimately render after yielding to the event loop. Usetry/finallywhen monkeypatchingrenderer.renderso the original method is restored and the terminal is disposed even if an assertion fails.Phase 2 — Add the minimal
awaitingEchostate inTerminalQuality gate: after this phase, run the new targeted tests and confirm they pass before broadening validation.
lib/terminal.ts, add a private field near the lifecycle/write state fields:A short comment is fine if it clarifies that the flag represents “user input has been emitted to stdin; consume on the next incoming write.” Avoid a long issue-history comment.
this.awaitingEcho = trueonly after stdin is confirmed enabled and immediately before emitting user-originated data. “Before emitting” matters because anonDatalistener may synchronously echo viaterm.write(data), so the flag must already be set beforethis.dataEmitter.fire(data)runs:InputHandlercallback insideopen(), afterdisableStdincheck and selection clearing, beforethis.dataEmitter.fire(data).paste(data), afterassertOpen()anddisableStdincheck, before the bracketed-paste branch. Set once, not separately in both branches.input(data, wasUserInput), only in thewasUserInput === truebranch, beforethis.dataEmitter.fire(data).input(data, false)/ programmatic writeswrite()/writeln()output pathsprocessTerminalResponses()InputHandlerdata callback; the callback-level marker is acceptable because it is still user-originated input.writeInternal()—after the WASM write, terminal responses, bell detection, link invalidation, auto-scroll, title checks, and existing callback scheduling—consume the flag and synchronously render once:Important details:
awaitingEchobefore rendering so a thrown render cannot leave the terminal in a permanent sync-render mode.requestAnimationFrame(callback)as they are today.this.wasmTerm,false,this.viewportY,this,this.scrollbarOpacity.Phase 3 — Full automated validation
Quality gate: do not claim success until these pass, or report the exact blocker.
Run, in this order:
Before final handoff/PR, run the repository’s full required sequence:
If
bun testhangs after reporting results, capture the visible pass/fail summary and the hang behavior; do not mark validation complete if pass/fail is unclear. Ifbun run buildorbun run demo:devis blocked by a missingghostty-vt.wasm, Zig/mise setup, or another local toolchain issue, report the exact command and error instead of broadening this task to fix the environment.Phase 4 — Dogfooding and reviewable evidence
Quality gate: collect reviewable UI evidence after automated checks pass.
This starts Vite on
http://localhost:8000/demo/with the WebSocket PTY handler attached at/wson the same origin.agent-browserskill/tooling in Exec mode) to openhttp://localhost:8000/demo/.echo issue-161-latencyThe recording should show typed characters echoing promptly and the command output appearing normally.
5. Run a higher-output command such as one of the following and capture a second screenshot or short clip:
The purpose is to confirm no obvious regression or main-thread stutter from synchronous rendering on bulk stdout.
6. Save and attach reviewable artifacts:
ghostty-vt.wasm, Zig, or another local toolchain prerequisite is missing, capture the exact failure and stop dogfooding there. Do not fabricate visual evidence.Acceptance criteria
paste(data)input(data, true)input(data, false)write()/writeln()writeInternal()after the marker is set renders synchronously viaCanvasRenderer.render(this.wasmTerm, false, this.viewportY, this, this.scrollbarOpacity), with tests asserting thewasmTermargument andforceAll === false.disableStdinprevents both user data emission and the echo marker.Risks and mitigations
awaitingEchobefore the render call and assert with a test that a second write does not increment the sync render count.open()and do not yield between the triggering input and assertions. If needed, dispose immediately after assertions.awaitingEchoassertions. Direct private-field checks are acceptable only as supplementary checks if they materially reduce ambiguity.lib/terminal.tsandlib/terminal.test.tsunless a verified compile/test issue requires a minimal adjacent adjustment.Advisor review
try/finallytest cleanup, synchronous-echo listener handling, and environment blocker reporting.PR/handoff notes for the implementer
Generated with
mux• Model:openai:gpt-5.5• Thinking:xhigh