Repository navigation
Conversation
…robe The probe missed CJK entirely: isEchoingKey drops a composing keydown, so typing Hangul produced zero samples and the percentiles read as "no latency". A composed commit is the keystroke that carries the echo, so compositionend is where the timer starts. Crediting only the oldest pending keystroke also biased the numbers: a TUI redraws on its own clock, so a fast burst shares one redraw, and the rest stayed pending forever — the surviving samples were the ones that had waited longest. The whole waiting set is credited now, with `coalescing` saying how many shared the write and bytes attributed to each. Adds a main-thread blocking census (long tasks), an IME-vs-direct split with a fixed-bucket histogram, and dispatchMs, which separates the renderer's share from the host round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…umbers Two shared slots let one sample carry another's figures. The coalescing count lived on the pane, so a second write parsing before the render that drained the first batch overwrote it, and the earlier samples reported the later batch's size. It is stored per keystroke now, assigned where the echo is marked. The blocking census measured up to whichever moment census() was called, including idle time after stop() detached the observer, which divided the same blocked milliseconds by a longer window and understated the share. The window closes at stop(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe typing-latency probe now supports direct and IME input classification. Echo samples include source, dispatch latency, coalescing, and write metrics. Source-specific reports include bounded percentiles, paint histograms, and IME commit counts. The probe also records main-thread long-task statistics and exposes them in reports. Latency samples use fixed buckets with an overflow bucket. Tests cover grouped echo attribution and independent coalescing counts. Merge Risk: ⚪ Minimal · up to This PR updates localized renderer-side typing diagnostics without changing application behavior, privileges, or external interfaces. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is detailed and covers the change, rationale, linked issue, visual proof, testing, AI disclosure, review considerations, and known gaps. The template checklist and separate Notes section are not reproduced, but the required information is otherwise substantially complete. Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation The added histogram, dispatch, input-source, main-thread blocking, coalescing, and percentile logic directly support the missing measurements and defects described in issue
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 546b60a6-d3ab-4064-83c0-e1f01258c533
📒 Files selected for processing (6)
src/renderer/src/lib/typing-latency-diagnostic-summary.tssrc/renderer/src/lib/typing-latency-diagnostic.tssrc/renderer/src/lib/typing-latency-echo-instrumentation.test.tssrc/renderer/src/lib/typing-latency-echo-instrumentation.tssrc/renderer/src/lib/typing-latency-input-source.tssrc/renderer/src/lib/typing-latency-main-thread-blocking.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
… commit characters The two summaries sit side by side in one report and disagreed: the echo summary is nearest-rank, the blocking census took floor(n * fraction), so with 20 samples its p50 read the 11th value and its p95 read the maximum. IME commit size counted UTF-16 units, so a supplementary-plane commit counted as two characters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ELI5
There is a built-in stopwatch for "how long from keypress to seeing the character" (
window.__orcaTypingDiagnostic). Typing Korean produced zero readings, so it looked like Korean had no latency at all. It also threw away most of a fast burst and kept the slowest ones. This fixes the stopwatch, then adds the readings that say where the time went.What Changed
CJK input is sampled at all.
isEchoingKeyreturns false whileevent.isComposingis true, so a composed character was never recorded. Composing jamo genuinely send nothing to the pty — the commit is what carries the echo — socompositionendstarts the timer, and IME keydowns are now excluded explicitly rather than by accident.Fast bursts are no longer dropped. A TUI redraws on its own frame clock, so typing faster than that clock puts several keystrokes into one redraw.
oldestUnparsedcredited only the oldest to that redraw and the rest stayed pending forever, so the surviving samples were the ones that had waited longest and p50 was biased upward. The whole waiting set is credited now;coalescingon the sample says how many shared the write, and each reports the redraw's byte count so the two divide.Two shared slots that let one sample carry another's figures. The coalescing count lived on the pane, so a second write parsing before the render that drained the first batch overwrote it — it is stored per keystroke now, assigned where the echo is marked. The blocking census measured up to whenever
census()was called, including idle time afterstop()had detached the observer — the window closes atstop().New readings.
mainThreadBlocking— long-task count, blocked milliseconds, and share of the window. Answers "was the renderer even free to serve this keystroke", which the echo timings alone cannotbyInputSource— IME commits and plain keypresses summarized separately, plusimeCommitChars. Pooling them hides which of the two is slowdispatchMs— keystroke to the moment xterm handed bytes toward the pty, separating the renderer's share from the host round tripechoPaintHistogram— fixed buckets rather than derived ones, because the paths a keystroke can take sit at known delays (a ~16-32ms fast release vs a 250ms hold-safety fallback vs a 1s coalesce). Percentiles average two modes into one number that matches neitherWhy
This is the tool the other two fixes were measured with, so it had to be right first. Concretely: with the old probe, an investigation into Korean typing latency reads zero samples and concludes there is nothing to fix — which is the opposite of the truth.
Linked Issue
Fixes #16950
Visual Proof
N/A— no UI. This code is inert untilwindow.__orcaTypingDiagnostic.start()is called from the renderer console.What the fixed probe reports, for reference (agent pane, idle, 16 composed syllables):
The 58-vs-6 bytes split is the kind of thing the old probe could not surface, and it is what identifies the agent TUI's own redraw — not Orca — as the dominant per-syllable cost in that pane.
Testing
typing-latency-echo-instrumentation.test.tsgained coverage for several keystrokes resolving through one redraw, and for the ordering regression: two writes parsing before a single render must report[2, 2, 1], not[1, 1, 1]or[1, 1, ...]from a shared slot.Platform: Windows 11. All of it is renderer-side probe logic and platform-independent.
pnpm testcould not run in my environment — theensure-native-runtime --runtime=nodeprestep fails to rebuildnode-ptyfor Node 24 — so vitest was invoked directly.AI Disclosure
Written with Claude Opus 5 (Claude Code).
Review
Cross-platform — no platform branch;
PerformanceObserverwith thelongtaskentry type is feature-detected and degrades tosupported: falsewith zeroed counts rather than throwing.SSH / remote — renderer-local. No transport, pty, or execution-host code touched.
Agents / integrations — agent-neutral.
byInputSourcesplits by how the character was entered, not by which agent is running.Backwards compatibility —
TypingLatencyReportgains fields; existing ones keep their meaning.recordKeystroketakes a thirdsourceargument, and its only callers are in this file's module group.Performance — inert until
start(). While running, samples are bounded to a rolling 2000 per series, and the write wrapper is restored on detach.Security — no new IPC, no new input path. It reads keystroke timings and byte counts, never keystroke content.
Known gaps
bytesPerKeystrokereports the whole echoing write for every keystroke that shared it, withcoalescingas the divisor. That is deliberate — pre-dividing would lose "how many bytes did one redraw cost" — but the field name invites the other reading, and a consumer that ignorescoalescingwill overcount.