Skip to content

Reduce echo output latency - #179

Merged
ThomasK33 merged 1 commit into
mainfrom
implement-issue-161
Jun 26, 2026
Merged

Reduce echo output latency#179
ThomasK33 merged 1 commit into
mainfrom
implement-issue-161

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

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

  • Added a bounded awaitingEcho state in Terminal.
  • Mark echo-pending before onData fires for keyboard input, paste(), and input(data, true).
  • Render synchronously once in writeInternal() after the WASM write/response processing, then clear the marker.
  • Added regression coverage for user input, keyboard input, paste, synchronous onData echoes, programmatic input, disabled stdin, and subsequent bulk output.

Validation

  • bun run build — passed; regenerated ghostty-vt.wasm so tests could load the WASM artifact in this checkout.
  • bun run fmt && bun run lint && bun run typecheck && bun test && bun run build — passed.
    • Prettier: all matched files use Prettier style.
    • Biome: checked 62 files, no fixes applied.
    • Tests: 338 pass across 8 files.
    • Build: ghostty-vt.wasm built and Vite library build completed.
  • Workflow verifier: passed with no P1-P3 findings.

Verifier findings

The implementation-loop verifier reviewed the issue context and final commit, confirmed the changed paths are limited to lib/terminal.ts and lib/terminal.test.ts, and found no P1-P3 issues. It also dogfooded the interactive demo with PORT=8000 bun run demo:dev, typed echo issue-161-latency, and ran high-output printf '%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 because gh-image had no user_session cookie or GH_SESSION_TOKEN.


📋 Implementation Plan

Implementation Plan for #161 — Reduce latency for echo output

Issue summary

  • Issue: coder/ghostty-web#161 — “Reduce latency for echo output”
  • State/labels at planning time: open; accepted, feature, triage:done
  • Requested behavior: when user input is sent to stdin, the first output write that comes back from the PTY should render synchronously instead of waiting for the next requestAnimationFrame tick. This reduces perceived keystroke echo latency while avoiding synchronous rendering for bulk stdout.
  • Recommended scope: implement a small internal Terminal state 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

  • Live issue details and comments were read with gh issue view 161 --repo coder/ghostty-web --comments and gh issue view 161 --repo coder/ghostty-web --json ....
  • GitHub issue page was opened for live confirmation; it matches the CLI output: the issue asks for an awaitingEcho boolean set on stdin sends and consumed by a synchronous render on the next write.
  • Related issue search was run for echo latency, render latency, and sluggish typing; only Reduce latency for echo output #161 matched the first two searches.
  • Referenced adjacent issue [Feature] WebGL Renderer #155 was checked and is a separate WebGL renderer feature request, not part of this implementation.
  • The linked fork commit diegosouzapw/ghostty-web@740452a was inspected as prior art. It changes only lib/terminal.ts and lib/terminal.test.ts for the same awaitingEcho pattern; use it as guidance, not as a blind patch.
  • Repository context was investigated by Explore agents and spot-checked locally. Relevant files/symbols:
    • lib/terminal.ts
      • component/event fields around lines 65–105
      • InputHandler callback in open() around lines 448–461
      • write()/writeInternal() around lines 541–596
      • paste() around lines 617–633
      • input() around lines 641–656
      • startRenderLoop() around lines 1155–1182
    • lib/renderer.ts
      • CanvasRenderer.render(...) around lines 267–503; it clears dirty flags via buffer.clearDirty() after rendering
    • lib/terminal.test.ts
      • existing paste() tests around lines 438–496
      • existing input() tests around lines 554–637
      • keyboard/disableStdin tests around lines 2581–2725
      • write behavior tests around lines 2854–2911
    • lib/test-helpers.ts
      • createIsolatedTerminal(...) helper around lines 32–37
    • demo/bin/demo.js and demo/README.md
      • bun run demo:dev runs the demo through Vite with the /ws PTY handler attached on the same origin/port.

Non-goals / scope boundaries

  • Do not implement a new renderer, WebGL path, render scheduler, or general performance framework.
  • Do not change lib/renderer.ts unless tests reveal the current render() signature has changed unexpectedly.
  • Do not change demo server/Vite proxy behavior. The repo already has bun run demo:dev for the interactive PTY demo.
  • Do not create or modify GitHub issues unless a new, distinct out-of-scope problem is discovered during implementation. If that happens, first search existing issues for the same root problem, comment on an existing issue if found, or create a new needs-triage issue 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.

  1. Add a focused describe('echo latency optimization', ...) block to lib/terminal.test.ts, near the existing write behavior/input tests.
  2. Use createIsolatedTerminal({ cols: 80, rows: 24 }), create a DOM container in beforeEach, call term.open(container), and always term.dispose() in cleanup.
  3. Spy on term.renderer!.render after term.open(container) so the synchronous initial render from startRenderLoop() is not counted.
  4. Keep assertions black-box where practical by counting synchronous renderer.render(...) calls, rather than depending primarily on a private flag.
  5. Add tests covering:
    • input('x', true) followed by write('x') triggers exactly one synchronous render during writeInternal().
    • A subsequent 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 following write('x') does not synchronously render.
    • paste('hello') followed by write('hello') triggers one synchronous render.
    • disableStdin: true blocks the echo marker path for both input(data, true) and paste(data); a following write should not synchronously render.
    • Dispatch a real KeyboardEvent('keydown', { key: 'a', code: 'KeyA', keyCode: 65, bubbles: true, cancelable: true }) on the opened container, verify onData fires as existing tests do, then write('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:

const renderer = term.renderer!;
const originalRender = renderer.render;
const renderArgs: Array<Parameters<typeof renderer.render>> = [];
renderer.render = ((...args: Parameters<typeof renderer.render>) => {
  renderArgs.push(args);
  return originalRender.call(renderer, ...args);
}) as typeof renderer.render;

try {
  term.input('x', true);
  expect(renderArgs).toHaveLength(0);

  term.write('x');
  expect(renderArgs).toHaveLength(1);
  expect(renderArgs[0][0]).toBe(term.wasmTerm);
  expect(renderArgs[0][1]).toBe(false); // no forced full redraw

  term.write('bulk output');
  expect(renderArgs).toHaveLength(1);
} finally {
  renderer.render = originalRender as typeof renderer.render;
  term.dispose();
}

Implementation note for tests: avoid await between setting up the spy and asserting the synchronous render, because the background rAF loop can legitimately render after yielding to the event loop. Use try/finally when monkeypatching renderer.render so the original method is restored and the terminal is disposed even if an assertion fails.

Phase 2 — Add the minimal awaitingEcho state in Terminal

Quality gate: after this phase, run the new targeted tests and confirm they pass before broadening validation.

  1. In lib/terminal.ts, add a private field near the lifecycle/write state fields:
private awaitingEcho = false;

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.

  1. Set this.awaitingEcho = true only after stdin is confirmed enabled and immediately before emitting user-originated data. “Before emitting” matters because an onData listener may synchronously echo via term.write(data), so the flag must already be set before this.dataEmitter.fire(data) runs:
    • In the InputHandler callback inside open(), after disableStdin check and selection clearing, before this.dataEmitter.fire(data).
    • In paste(data), after assertOpen() and disableStdin check, before the bracketed-paste branch. Set once, not separately in both branches.
    • In input(data, wasUserInput), only in the wasUserInput === true branch, before this.dataEmitter.fire(data).
  2. Do not set the flag for:
    • input(data, false) / programmatic writes
    • write() / writeln() output paths
    • terminal-generated responses emitted by processTerminalResponses()
    • mouse tracking data unless it already flows through the InputHandler data callback; the callback-level marker is acceptable because it is still user-originated input.
  3. At the end of 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:
if (this.awaitingEcho) {
  this.awaitingEcho = false;
  if (this.renderer && this.wasmTerm) {
    this.renderer.render(this.wasmTerm, false, this.viewportY, this, this.scrollbarOpacity);
  }
}

Important details:

  • Clear awaitingEcho before rendering so a thrown render cannot leave the terminal in a permanent sync-render mode.
  • Preserve existing write callback behavior; callbacks should still be queued with requestAnimationFrame(callback) as they are today.
  • Pass the same render arguments used by the normal render loop: this.wasmTerm, false, this.viewportY, this, this.scrollbarOpacity.
  • Do not force a full render. Let WASM dirty tracking limit work to rows changed by the echo.

Phase 3 — Full automated validation

Quality gate: do not claim success until these pass, or report the exact blocker.

Run, in this order:

bun install
bun test lib/terminal.test.ts
bun run typecheck
bun test
bun run fmt
bun run lint
bun run build

Before final handoff/PR, run the repository’s full required sequence:

bun run fmt && bun run lint && bun run typecheck && bun test && bun run build

If bun test hangs after reporting results, capture the visible pass/fail summary and the hang behavior; do not mark validation complete if pass/fail is unclear. If bun run build or bun run demo:dev is blocked by a missing ghostty-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.

  1. Start the interactive demo in dev mode:
bun run demo:dev

This starts Vite on http://localhost:8000/demo/ with the WebSocket PTY handler attached at /ws on the same origin.

  1. Use browser automation (prefer the agent-browser skill/tooling in Exec mode) to open http://localhost:8000/demo/.
  2. Confirm the terminal connects to a real shell. Capture a screenshot showing the connected demo before typing.
  3. Record a short video/WebM/GIF while typing a simple command slowly, for example:
echo issue-161-latency

The 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:

printf '%s\n' {1..200}
# or
ls -la

The purpose is to confirm no obvious regression or main-thread stutter from synchronous rendering on bulk stdout.
6. Save and attach reviewable artifacts:

  • screenshot before typing / connected demo
  • screen recording of typing
  • screenshot or recording after high-output command
  • optional DevTools Performance screenshot if it clearly shows the keydown-to-canvas update path
  1. If the environment is headless or browser automation/video capture is unavailable, explicitly report that blocker and include the dev server logs plus automated validation results. If the demo cannot start because 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

  • User-originated input paths mark the terminal as awaiting an echo:
    • keyboard/InputHandler data callback
    • paste(data)
    • input(data, true)
  • Non-user/programmatic paths do not mark the terminal as awaiting an echo:
    • input(data, false)
    • write()/writeln()
    • internal terminal response handling
  • The first writeInternal() after the marker is set renders synchronously via CanvasRenderer.render(this.wasmTerm, false, this.viewportY, this, this.scrollbarOpacity), with tests asserting the wasmTerm argument and forceAll === false.
  • The marker is cleared immediately, so subsequent stdout chunks do not synchronously render unless another user input occurs.
  • disableStdin prevents both user data emission and the echo marker.
  • Existing render loop behavior remains intact; normal rAF rendering still runs and write callbacks keep their current scheduling semantics.
  • Regression tests cover the sync-render-on-echo behavior and the no-sync-render bulk/programmatic cases.
  • Full validation passes: formatting, lint, typecheck, tests, and build.
  • Dogfooding evidence is captured for the interactive demo, including screenshots and a recording when browser automation is available.

Risks and mitigations

  • Accidental sync render for every stdout chunk: clear awaitingEcho before the render call and assert with a test that a second write does not increment the sync render count.
  • Password prompts/no-echo modes: a keypress may set the marker but no echo arrives; the next output may consume the marker and render once. This is acceptable because it is one bounded render, not a persistent mode.
  • Unrelated background output arrives before the echo: it may consume the marker. This is still bounded to one synchronous render and avoids trying to parse PTY semantics in the terminal frontend.
  • rAF noise in tests: install the render spy after open() and do not yield between the triggering input and assertions. If needed, dispose immediately after assertions.
  • Private state test brittleness: prefer render-call observations over direct awaitingEcho assertions. Direct private-field checks are acceptable only as supplementary checks if they materially reduce ambiguity.
  • Out-of-scope performance temptations: keep the patch limited to lib/terminal.ts and lib/terminal.test.ts unless a verified compile/test issue requires a minimal adjacent adjustment.

Advisor review

  • The plan was reviewed with the advisor, revised to strengthen keyboard coverage, render-argument assertions, try/finally test cleanup, synchronous-echo listener handling, and environment blocker reporting.
  • The advisor gave final approval with no further required changes.

PR/handoff notes for the implementer

  • Keep the diff small and reviewable.
  • Mention Reduce latency for echo output #161 in the PR body and explain that the change is deliberately limited to one synchronous render after user input.
  • Include validation output and dogfooding artifacts in the PR or handoff.
  • If creating a PR, include the required generated-by footer and append this plan in a collapsible details block according to repo/user preferences.

Generated with mux • Model: openai:gpt-5.5 • Thinking: xhigh

Change-Id: I046454a6ef3f44d7671de0ca0b21f85e25509ddc
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 065984a455

ℹ️ 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".

@ThomasK33
ThomasK33 merged commit 24b3c80 into main Jun 26, 2026
5 checks passed
@ThomasK33
ThomasK33 deleted the implement-issue-161 branch June 26, 2026 16:24
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce latency for echo output

2 participants