Skip to content

Highlighting: Make the worker the default and warm it during startup - #1105

Open
masonmcelvain wants to merge 7 commits into
modem-dev:mainfrom
masonmcelvain:fast-default
Open

masonmcelvain wants to merge 7 commits into
modem-dev:mainfrom
masonmcelvain:fast-default

Conversation

@masonmcelvain

@masonmcelvain masonmcelvain commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Syntax highlighting now runs in the worker by default, and the worker starts compiling the review's grammars during startup, before the OpenTUI import. On a small diff the default path used to block keyboard input for roughly a quarter second after the review was already on screen: the first file of each language paid one-time Oniguruma grammar compilation on the main thread, and source-backed highlighting tokenizes whole files. That work now happens in the worker while the renderer loads.

Changes

  • Warm during startup. New preload worker request (protocol v5) resolves one theme and grammar and tokenizes a short sample so the grammar's root, comment, and string rules compile. main.tsx sends one preload per distinct changeset language (review order, capped at 8) after bootstrap and before importing OpenTUI. Render requests queue ahead of waiting preloads, so a visible file waits behind at most one grammar. Warm-up is best-effort and skipped where the worker is not used.
  • Worker by default. Interactive highlighting offloads wherever supportsHighlightWorkerOffload() holds and the theme has no syntax_scopes overrides. Inline stays for Windows compiled binaries, scope-override themes, and static piped output. The offloadLargeDiff prop plumbing from App down to the highlight hooks is removed; loadHighlightedDiff keeps an offload option for the static pager, and the document service consults eligibility alone.
  • HIGHLIGHT_WORKER_MIN_LINES removed. It was checked against source-backed metadata so nearly every real file already qualified, and with the worker as the default any file below it would have paid the full grammar compile on the main thread for a handful of lines.
  • --fast is a no-op. Still parsed so aliases and existing invocations do not error; hidden from --help and the public reference; the changeset notes the deprecation for removal in a later release.
  • Permanent worker refusals fall through to inline. Non-retryable worker errors (unsupported-language and friends) now take the inline path, which re-renders as plain text and caches the result, instead of being marked retryable and re-requested on every scroll while the file sits in the prefetch halo.
  • Tests and benchmark. A PTY test launches a three-grammar review, sends page-down just after first paint, and requires the answer within max(60ms, 4× the settled answer) for the same key once color has landed. Cherry-picks the spawn-inclusive startup benchmark from Startup: begin loading OpenTUI once the plan is interactive, overlapping git bootstrap #1062's investigation and extends it with startup_first_color_ms and startup_post_paint_key_max_ms.

Measurements

Source mode, Linux x64, benchmarks/startup-first-frame.ts, 7 launches per side, same machine:

Metric main this branch
startup_first_frame_ms 435 462
startup_first_color_ms 639 561
startup_post_paint_key_max_ms 105 34

The +28ms first frame is the worker thread starting alongside the OpenTUI import, the same order as the issue's prototype. The PTY latency test measured the first page-down at 72–105ms on main and 6–28ms here across repeated runs.

Validation

  • bun run typecheck, bun run lint, bun run deps:check, bun run check:docs
  • bun run test: 2245 pass, 4 fail. All four reproduce on unmodified main in this environment: test/cli/entrypoint.test.ts and test/cli/log.test.ts pick up my user config.toml (watch = true), packages/hunk-jj/src/source.test.ts needs jj on PATH, and brokerClient.test.ts > authenticates after a successor becomes healthy is a 5s timeout flake.
  • bun run test:integration: 184 pass, 1 fail. The failure is watch-start-integration.test.ts's teardown assertion (child.exitCode still null 500ms after SIGKILL), which fails 2 of 4 runs on main as well.
  • PTY smoke of hunk diff and hunk show HEAD~1 on this repository at 140×30, unified layout, github-dark-default; navigation and color land as before.
  • Static hunk show HEAD~1 | head, hunk --fast show HEAD~1 | head, and hunk diff --help from source.
  • Not tested: macOS, Windows. The Windows compiled-binary path is unchanged by design (supportsHighlightWorkerOffload still gates it, warm-up skips it).

Follow-ups

  • fix(ui): bound and surface large-diff highlight retries #785 (bounded worker retries) should land first or alongside.
  • hunk log's interactive history surface does not warm the worker; it has no file list when the plan is selected, so its first review still starts a cold worker.
  • Remove --fast in a later release.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Sep 15, 2026

Copy link
Copy Markdown

@masonmcelvain is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@masonmcelvain
masonmcelvain marked this pull request as ready for review September 15, 2026 17:39
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

masonmcelvain and others added 7 commits September 29, 2026 14:48
Every first-frame benchmark starts its timer after the renderer is
already imported, so none of them can observe OpenTUI's module
evaluation or its native-library load, which profiles put at roughly
120ms of the ~440ms first frame. Launch `hunk diff` in a real PTY on a
small dirty repo and time the first painted review frame from process
spawn instead. The script answers the terminal background probe the
way a real terminal would so the number reflects a normal launch, and
HUNK_BENCHMARK_EXECUTABLE points it at a compiled binary. It joins the
default suite so the release gate covers startup end to end.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MJ5s3ZBwp9FwFhpee2egXg
On a small diff the default highlight path blocked keyboard input for
about a quarter second after the review was on screen: the first file
of each language paid one-time Oniguruma grammar compilation on the
main thread, and source-backed highlighting tokenizes whole files, so
the plain frame appeared and then nothing responded until color landed.

Add a `preload` request to the worker protocol that resolves one theme
and grammar and tokenizes a short sample so the grammar's root, comment,
and string rules compile in the worker. The interactive entrypoint
sends one preload per distinct language in the changeset (review order,
capped at eight) right after bootstrap and before the OpenTUI import,
so compilation overlaps renderer startup. Render requests queue ahead
of waiting preloads, so a file that needs color mid-warm-up waits for
at most one grammar. Warm-up is best-effort and skipped where the
worker is not used at all: Windows compiled binaries and scope-override
themes.

The protocol version bumps to 5. `themeSupportsHighlightWorker` moves
the scope-override check beside the syntax theme naming so the loader,
document eligibility, and warm-up share one predicate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Interactive highlighting now uses the syntax worker wherever it is
supported instead of behind `--fast`. The inline path remains the
fallback for Windows compiled binaries, custom themes with scope
overrides, and static (piped) rendering, which gains nothing from a
worker.

The 40-line minimum goes away. It was checked against source-backed
metadata, so nearly every real file already qualified, and with the
worker as the default any file that fell below it would have paid the
full grammar compile on the main thread for a handful of lines. Every
interactive request now takes one path, so grammar compilation happens
in the worker exactly once.

`--fast` stays accepted as a no-op so existing aliases and configs do
not error; it is hidden from `--help` and the public reference and will
be removed in a later release. The `offloadLargeDiff` prop plumbing
from App down to the highlight hooks is removed, since the decision is
no longer per launch; `loadHighlightedDiff` keeps an `offload` option
for the static pager, and the document service consults eligibility
alone. Tests that need the inline path deterministically use the real
eligibility policy under a runtime without worker support, and test
files that register worker doubles now dispose them.

Measured with the startup benchmark on this branch versus main (source
mode, Linux, 7 launches each): the slowest key answer in the 400ms
after first paint drops from 105ms to 34ms, first syntax color lands
78ms sooner (639ms to 561ms from spawn), and the first frame costs
about 28ms more while the worker thread starts alongside OpenTUI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The worker path was covered by a PTY test that sent a key while a large
file highlighted under `--fast`, but nothing exercised the default path
on the small multi-language diff where inline grammar compilation
blocked input. Add a PTY test that launches a three-grammar review,
sends page-down just after the first paint, and requires the answer to
arrive within four times the settled answer for the same key once color
has landed (or 60ms, whichever is larger). On the inline path the first
answer was 4-15x slower; through the worker it matches the settled one.

Extend the startup benchmark from the same launch: report the first
keyword-colored cell from spawn and the slowest answer among page-down
keys sent from first paint through the next 400ms, each answer counted
only when an unseen row appears so highlight repaints never count.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`loadHighlightedDiff` marked every worker rejection retryable, and
retryable results deliberately stay out of the shared highlight cache
so a recreated worker can try again. With the worker now the default
path, a permanent refusal such as an extension-registered language
Pierre has no grammar for was re-requested on every scroll while the
file sat in the prefetch halo, redoing source reads and posting the
full metadata to the worker each time.

Non-retryable worker errors now fall through to the inline path, which
already re-renders an unsupported language as plain text and caches
that result once. Retryable failures keep their plain-row placeholder.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The compiled highlight worker control fixture asserts the protocol
version by hand, since importing the private worker protocol would
cross the CLI fixture's packaging boundary. The preload request bumped
that version to 5, so the macOS compiled portability job failed on the
old literal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
check:pack requires packages/hunk/README.md to match the repository
README, which dropped the `hunk --fast` example.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Highlighting: make --fast the default with warm workers

1 participant