ci: let the typing-lag harness measure the text-input path - #216
Merged
Conversation
The lag harness repeats PROGRAMA_LAG_KEY_COMBO, hardcoded to "up" in CI. An arrow key produces no committed text, so shouldRefreshAfterTextInput never becomes true and forceRefresh never runs -- meaning the harness can measure the per-event hit-test half of the keystroke path but is structurally blind to the text-input half. Adds a lag_combo workflow_dispatch input. A printable key such as "a" parses to a valid combo, is not intercepted by any configured shortcut (they all require command), and travels the same NSApp.sendEvent -> keyDown -> interpretKeyEvents -> insertText route as a real keystroke, so it does reach forceRefresh. Push behaviour is unchanged: inputs is empty there and the expression falls back to "up". Chose this over driving surface.send_text, which calls ghostty_surface_text and forceRefresh straight from the RPC handler and bypasses NSEvent and the responder chain -- it would measure an artifact rather than the keystroke path. Refs #183
arzafran
added a commit
that referenced
this pull request
Jul 30, 2026
After every printable keystroke the keyDown path called forceRefresh, which reasserts the surface's display id and runs forceRefreshSurface()'s geometry reconciliation. Both exist for topology changes -- split close/reparent, and the stuck-vsync state after wake-from-sleep -- and a character cannot cause either. Typing changes grid contents, not the surface's size or which display it is on. Adds requestRedrawAfterInput(), which only calls ghostty_surface_refresh, and uses it at the text-input call site. forceRefresh is unchanged and still used everywhere else. The early-out conditions are deliberately identical to forceRefresh's, so when a redraw happens is unchanged; only the work done differs. This half was unmeasurable until now: the lag harness sends an arrow key, which never satisfies shouldRefreshAfterTextInput, so forceRefresh never ran under it. #216 added a lag_combo input so a printable key can drive the same harness through the real responder chain. Refs #183
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
1 participant
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.
Refs #183. Unblocks the remaining half of that issue.
What this does
The lag harness repeats
PROGRAMA_LAG_KEY_COMBO, hardcoded toupin CI. An arrow key produces no committed text, soshouldRefreshAfterTextInputnever becomes true andforceRefreshnever runs. The harness can measure the per-event hit-test half of the keystroke path — which is what #207 fixed and measured — but is structurally blind to the text-input half.That's why #207 shipped only half of #183: the other half was unmeasurable, and the issue is explicit that this path doesn't get merged on eyeball.
Adds a
lag_combodispatch input so a printable key can be used instead.Why a bare letter measures the real path
"a"parses to a valid combo (keyCode 0) — the parser maps every letter, not just named keys..commandexceptctrl+1-9.NSApp.sendEventand travelskeyDown→interpretKeyEvents→insertText, exactly like a real keystroke.shouldSendTextreturns true,shouldRefreshAfterTextInputbecomes true, andforceRefresh(reason: "keyDown.textInput")fires — the precise call the remaining fix would replace.What I rejected
Driving
surface.send_textinstead. It callsghostty_surface_textandforceRefreshdirectly from the RPC handler, bypassingNSEventand the responder chain — so it would measure an artifact rather than the keystroke path, and a fix could look like it worked while changing nothing real.Not doing yet
Adding a permanent second CI lane for the text-input combo. Running both combos against one app instance would pollute the second run's baseline with the first run's 20 churn workspaces, so it needs either a relaunch or a restructured step. Worth doing once we know the numbers justify it — this PR is the minimum needed to get those numbers.
Test plan
lag_comboinput present with defaultupinputsis empty on push, expression falls back toupgh workflow run ci.yml --ref <branch> -f lag_combo=aproduces a run whose lag output reflects the text-input path