Skip to content

fix(ui): open help and menus without first-use suspension - #1079

Merged
elucid merged 3 commits into
mainfrom
perf/overlay-first-open
Sep 10, 2026
Merged

fix(ui): open help and menus without first-use suspension#1079
elucid merged 3 commits into
mainfrom
perf/overlay-first-open

Conversation

@elucid

@elucid elucid commented Sep 9, 2026

Copy link
Copy Markdown
Member

Follow-up to #1063. First of two first-interaction fixes; independent of #1078 (Shiki WASM decode).

Problem

The first ? (help) or F10 (menu) after launch takes ~310–360 ms to appear, on every host and in the compiled binary; reopening takes 5–25 ms. Waiting a few seconds before the first press doesn't help, and diff size doesn't matter.

Cause: HelpDialog and MenuDropdown were React.lazy components inside <Suspense fallback={null}>. The module import resolves in 3–6 ms, but React 19's reconciler throttles the commit after a fallback (FALLBACK_THROTTLE_MS = 300, present in both dev and production builds), so the real tree commits ~301 ms after the null fallback. act() bypasses that throttle, which is why no unit test or bench:interaction-latency run ever saw it. Full diagnosis with instrumented timelines in the linked issue thread.

Approach

Import the two components statically. Their import graphs are small (command/key helpers and text helpers — no ThemeSelector, AgentSkill, Shiki or Pierre), so this doesn't move any heavy code onto the startup path: launch → first frame is unchanged and the npm bundle shrinks by 869 bytes. ThemeSelector and AgentSkill keep their own lazy + Suspense wrappers and are not touched (see follow-ups).

Second commit: two existing PTY tests pressed Escape to close the menu and immediately sent the next key. With the menu now closing quickly, that exposed a harness race (OpenTUI buffers a lone Escape until its parser timeout and may join the following escape-prefixed bytes). They now wait for the menu to disappear from the screen before sending more input. Production key routing is unchanged.

Measurements (median ms, n=5; before → after)

Host / build Overlay First open at first frame First open after 2 s idle Reopen Launch → first frame
macOS binary help 320 → 18 306 → 10 4 → 5 172 → 173
macOS binary menu 318 → 15 305 → 4 3 → 3 166 → 167
Linux binary help 364 → 80 316 → 33 20 → 15 459 → 445
Linux binary menu 361 → 69 312 → 18 13 → 8 451 → 457

The remaining Linux first-frame cost is mostly the synchronous Shiki WASM decode that #1078 removes.

Tests / checks

  • New test/pty/dialog-first-open-integration.test.ts: raw Bun.Terminal, one-file git fixture, watch off, fixed theme, no extensions/update/broker. Sends the key in the first visible-frame callback (no warmup/press/waitIdle), then verifies close and reopen from the parsed screen. Four fresh processes: help/menu × immediate/2 s-idle. Gates: ≤200 ms immediate, ≤150 ms idle, first − reopen ≤150 ms. Fails 80/80 cases on main, passes 80/80 here (macOS + Linux, source + binary). Raw bytes and final screen are kept on failure.
  • bun run typecheck, deps:check, lint, format:check — pass.
  • bun run test:integration — Linux 170 pass / 1 skip; macOS 5 failures reproduce on main (TMPDIR trust-state cases, signal-lifecycle cases).
  • bun run test:tty-smoke — Linux 10/10; plus a real-TTY keyboard/mouse smoke.
  • bun run test — same pre-existing failures as main.
  • Not run on Windows (PTY tests are Unix-only).

Follow-ups (not in this PR)

  • ThemeSelector still has the same first-use behaviour (measured ~306–361 ms). AgentSkill is lazy and unmeasured. Both need their own measurement before deciding static vs. preloaded.
  • CI runner variance for the new latency gates hasn't been characterized; budgets have ~2.5× headroom over the slowest measured host.

Keep lightweight help and menu chrome synchronously available so the first open does not wait on React’s Suspense fallback throttle. Preserve lazy loading for the other overlays.

Add raw-PTY first-frame and idle-first latency coverage with screen-verified close/reopen, bounded process-group cleanup, and optional compiled-binary coverage.
Faster first-open rendering exposes tests that treat a short output-idle wait as proof a lone Escape has been parsed. Observe the closed menu before sending F8, hunk navigation, or an Escape-prefixed Alt binding instead of relying on the old suspension delay.
@vercel

vercel Bot commented Sep 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Preview Sep 10, 2026 2:23am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes lazy Suspense boundaries from the lightweight help and menu overlays so their first visible commit is no longer throttled. It also adds a raw-PTY latency regression suite and makes existing PTY tests wait for menu closure before sending escape-prefixed input.

  • Statically imports HelpDialog and MenuDropdown.
  • Measures immediate, idle, and repeat overlay-open latency in fresh processes.
  • Synchronizes follow-up PTY input with screen-verified menu closure.

Confidence Score: 4/5

The runtime change appears sound, but the repository’s explicit test-import requirement must be satisfied before merging.

The static overlay imports are lightweight and the PTY synchronization changes follow established behavior; the only accepted issue is the new test’s prohibited dynamic import.

Files Needing Attention: test/pty/dialog-first-open-integration.test.ts

Important Files Changed

Filename Overview
packages/hunk/src/ui/App.tsx Replaces lazy help and menu rendering with lightweight static imports while retaining lazy loading for heavier dialogs.
test/pty/dialog-first-open-integration.test.ts Adds first-use latency coverage with a raw PTY, but its in-test dynamic import violates the repository testing instruction.
test/pty/file-views-integration.test.ts Waits for the menu marker to disappear before sending subsequent escape-prefixed input.
test/pty/key-routing.test.ts Verifies the menu has closed before sending the Alt-key sequence used to open help.
Prompt To Fix All With AI
### Issue 1
test/pty/dialog-first-open-integration.test.ts:56-58
**Dynamic import violates test rule**

This test loads `PersistentTerminal` with `await import(...)` inside the test callback. The repository testing directive requires imports to be hoisted to the top level and explicitly forbids dynamic imports in tests. This requirement must be satisfied before merging while preserving the Unix-only platform handling.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test(pty): verify menu closure before su..." | Re-trigger Greptile

Comment thread test/pty/dialog-first-open-integration.test.ts Outdated
@elucid
elucid merged commit 33eb12f into main Sep 10, 2026
14 checks passed
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.

1 participant