Skip to content

bench-terminal-resize: pane check samples each window's frames; unsampled or lagging terminals fail - #36223

Closed
lawrencecchen wants to merge 6 commits into
feat-cmux-nextfrom
fix/m16b-pane-check-window-frames
Closed

lawrencecchen wants to merge 6 commits into
feat-cmux-nextfrom
fix/m16b-pane-check-window-frames

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #36109: until it merges, this diff also shows its commit (135c183). The follow-up is the four commits after it. Review of #36109 (comment 6650044486) plus frame-owner-s4's later rules.

Summary

  • Sampler on the window's frames. The pane check sampled on the screen's display link (FrameScheduler.app). Each window that shows a followed terminal now has a FrameClient on FrameScheduler.forWindow(window), the clock its springs and frame presenter tick on. An observer after Core Animation's commit (beforeWaiting, order CFIndex.max) samples only the windows whose frame ticked in that turn. A sample is the geometry that turn's commit carried for that window's frame. The doc comment says exactly this.
  • An unsampled terminal fails. A terminal shown at the round's start with 0 sampled frames fails the round (" was not sampled"). It used to pass on the other terminals' frames.
  • A lagging frame fails. frame-owner-s4 set this rule under the m14 contract: a late frame is a failure. A frame whose layer shows another size than its pane after that frame's commit fails the round. The message names the terminal, its lagging frames and the first sizes. lagging_frames stays in the report.
  • The docstring is fixed. It named DebugTerminalPaneFollow, which does not exist; it now names TerminalPaneFollowProbe.
  • New data to explain lag. Each terminal adds in_flight: frames where a size was asked of Ghostty and was not shown yet. The stop reply adds follow_presenter: the frame presenters' passes, presented, keeps, group waits and fallbacks in the run. The bench carries both into each round, and the arm summaries add in_flight_frames and presenter_keeps.

Bench only. This PR adds no CI job, step, gate or budget. The probe stays in CmuxNextTerminal, owned by PL-ghostty (session cmux-b6).

Observed / Not run

  • Red, then green:

    • python3 scripts/cmux-next/tests/test_bench_terminal_resize.py: test_a_terminal_not_sampled_in_the_motion_fails fails at 31c3daf and passes at 6a87567.
    • test_a_lagging_frame_fails_and_names_the_terminal_its_frames_and_sizes fails at 0c16e59 and passes at ed6ba20.
    • All 27 tests pass at the head.
  • Fleet step ad3ccfc1219e74f7aeb7b804 at 6a87567 compiled CmuxNext, and the 4 TerminalPaneFollowTallyTests passed, including the new in-flight test.

  • A/B --ab=motion.frameOwner.terminals, keys and keys-stress, 2 rounds per arm, at 6a87567 (same Swift as the head) on cmux23-perf: step d557f84ae28f821bd9f55bc1. Held and lagging frames are summed over the round's terminals:

    layout arm round frames held lagging in flight presenter (passes / presented / keeps) Ghostty host presents (fresh / late)
    keys off 176, 183 144, 140 144, 140 0, 0 0 / 0 / 0 -
    keys on 182, 173 0, 0 0, 0 0, 0 28 / 112 / 0 per round 112 / 0 per round
    keys-stress off 147, 156 168, 182 168, 182 0, 0 0 / 0 / 0 -
    keys-stress on 150, 140 0, 0 0, 0 0, 0 20 / 140 / 0 per round 140 / 0 per round

    In this bench build, the ON key rounds still show 0 lagging frames. frame-owner-s4 expected lagging in most spring frames, from pixel-perfect's trace ("presenter request, host frame pending"). I checked whether the 0 comes from sampling at the wrong time. Two counters that do not depend on when the sampler reads say it does not:

    • The window's frame presenter kept no frame: keeps 0, fallbacks 0, and all 112 (keys) or 140 (keys-stress) requests per round were presented in their own pass.
    • Ghostty counted 112 or 140 fresh host presents and 0 late presents per round.

    The sampler also never saw a size in flight after a commit (in_flight 0). So in the optimized bench build (--config=bench) on cmux23, each key-spring frame's new size was drawn and presented in the commit of the same turn. I did not reproduce pixel-perfect's one-tick lag. The likely cause is a different build or display. A known gap: if the host view itself lags its pane, this check cannot see it, because "pane" here means the host's bounds.

  • With this head's rules, the ON key rounds above pass and the OFF rounds fail (held and lagging). A run with lagging > 0 would now fail. That path is covered only by the unit test, not by a bench run.

  • I moved the run off two minis. cmux17-perf showed idle but did not claim step 4fb4e24064b35b4ba1a0e766 for 35 minutes. cmux22-perf was held for a capture pool. I cancelled both steps.

  • Not run: drag layouts on this head, and films.

Changelog

none


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

The pane check for bench-terminal-resize now samples each window's own frame clock, so a terminal that is never sampled, or that shows a size other than its pane's, fails the round.

  • Each window that shows a followed terminal gets a frame client on its own clock; sampling reads the geometry that turn's commit carried.
  • A terminal shown at the round's start with 0 sampled frames fails the round; it used to pass on the other terminals' frames.
  • A frame whose layer shows another size than its pane's after that frame's commit fails; the message names the terminal, its lagging frames, and first sizes.
  • The stop reply adds per-terminal in-flight frames and frame-presenter counts; arm summaries add in-flight frames and presenter keeps.
  • A held drag (holds_terminals) is data only and no longer excuses a round.

Bench only: no CI job, step, gate, or budget changes.

Written for commit ed6ba20. Summary will update on new commits.

View guided diff Turn on auto-fix

…s not change size with its pane

debug.terminal_display now also samples, once per display frame after the
frame's commit, every terminal shown at start: its pane's size
(TerminalHostView.surfaceSize(forHostSize:)), the size its host asked of it
(the frame owner's wanted frame, else the view's frame) and the size its layer
shows. The stop reply adds terminals_followed (per terminal: frames, held,
lagging, the first 3 of each) and follow_frames; the other keys stay. Nothing
runs while the probe is stopped. The probe moves to CmuxNextTerminal
(TerminalDisplayProbe), next to the geometry it reads; the App keeps the
route.

terminal_resize_verdict.problems fails a drag or key round with a held frame,
naming the terminal, its held frames and the first sizes; a round with no pane
check fails too. The held-drag excuse (holds_terminals) is gone: it stays as
data. Lagging frames are data only. Arm summaries add held and lagging frames.

Tests: scripts/cmux-next/tests/test_bench_terminal_resize.py (26 pass);
TerminalPaneFollowTallyTests (fleet).
…round must fail the round

Review of #36109, finding 2. Red: the round passes today (problems() returns []).
…n unsampled terminal fails

Review of #36109 (frame-owner-s4):
1. The pane check sampled on the screen's display link (FrameScheduler.app). Each window that shows a followed
   terminal now gets a client on its own frame clock (FrameScheduler.forWindow), the clock its springs and its frame
   presenter tick on, and the after-commit observer samples only the windows whose frame ticked in that turn.
2. terminal_resize_verdict: a terminal shown at the round's start with 0 sampled frames fails the round
   ("was not sampled"), instead of passing on the other terminals' frames.
3. The verdict's docstring named DebugTerminalPaneFollow, which does not exist; it names TerminalPaneFollowProbe.

Data for the lagging question: per terminal `in_flight` (frames with a size asked of Ghostty and not shown yet), and
`follow_presenter` in the stop reply (the frame presenters' passes, presented, keeps, group waits and fallbacks in the
run). The bench carries both into each round and the arm summaries.

Tests: test_bench_terminal_resize.py 27 pass (the new unsampled test was red at 31c3daf);
TerminalPaneFollowTallyTests (fleet).
…round

frame-owner-s4 (m14 contract: a late frame is a failure). Red: lagging frames are data only today.
frame-owner-s4, 2026-11-05 (the m14 contract: a late frame is a failure). A frame whose layer shows another size than
its pane after that frame's commit fails the round, with a message naming the terminal, its lagging frames and the
first sizes; lagging_frames stays in the report. Expected red: ON key rounds until draw ahead and the window frame
sync land.

Tests: test_bench_terminal_resize.py 27 pass (the lagging test was red at 0c16e59).
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 5, 2026
@cursor

cursor Bot commented Nov 5, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review ed6ba20: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr36223 -f ref=fix/m16b-pane-check-window-frames -f platform=macos -f backend_mode=local

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent subagent): SHIP at ed6ba20

Scope: the 4 commits on top of 135c183 (#36109, now merged). I read the code, ran the Python tests, and read the A/B fleet log. I did not build Swift.

Verdict: SHIP. The sampler is correct for what it claims, and the A/B data supports the ON result. Findings below are follow-ups. None blocks.

1. Sample timing (correct).

  • Each followed window gets its own FrameClient on FrameScheduler.forWindow(window). That is the same clock the presenter uses (TerminalPaneFollow.swift:172-178, TerminalFramePresenter.swift:72-74).
  • The sample observer runs at beforeWaiting, CFIndex.max (TerminalPaneFollow.swift:185). That is after the presenter pass (1_999_999, TerminalFramePresenter.swift:30) and after CA's commit (2_000_000).
  • The pass sets the present and the new bounds together under CATransaction.lock (TerminalFramePresenter.swift:203-209). So when the switch is ON and a request is presented, the sample reads the bounds that this commit sent with its frame.
  • A size asked inside the commit's display cycle reads as in_flight with old bounds, so it counts as lagging. Good.

2. What it can still miss (follow-ups):

  • a. shown is the layer's bounds, not the size of the IOSurface in layer.contents (TerminalPaneFollow.swift:94-96). Any path that lands bounds without a frame shows the old pixels at the new bounds, and the sample reads it as "not lagging". These paths are:

    • presenter fallback after keepLimit;
    • the NONE/HIDDEN land (TerminalFramePresenter.swift:215-224);
    • every frame with the switch OFF.

    With the switch ON, a fallback comes only after 6 keeps, and those keeps are lagging frames, so the round fails anyway. Cheap extra safety: fail a round when presenter.fallbacks > 0.

  • b. Only turns in which the window's clock ticked are sampled (TerminalPaneFollow.swift:212-222). A commit from a turn with no tick (an input turn or a socket turn) can change the pane area and keep the old frame. That commit stays on screen until the next tick turn and is never sampled. presenter.keeps counts it, but keeps are data only (terminal_resize_arms.py:67). Fix: fail a round when keeps > 0, or also sample any turn whose commit changed a followed terminal's geometry.

    In the A/B (d557f84ae28f821bd9f55bc1, built at 6a87567; the head adds only Python): keys and keys-stress ON had 0 held, 0 lagging, 0 in_flight, 0 keeps and 0 fallbacks. Presented counts were 112 and 140 per round. So the ON result holds on both signals.

  • c. Nit: for id in clocks.keys { clocks[id]?.pending = false ... } (:213-215) changes the dictionary while its keys view holds the storage. That copies the dictionary once per sampled turn, which is a small per-frame allocation in a debug probe. Use clocks.values[index] in place, or make WindowClock a class.

  • d. TerminalFramePresenter.forWindow(window) in start makes a presenter on a window that has none (OFF arm). This is harmless because its client is lazy.

3. Rules, cost and threads (OK).

  • Nothing is installed until start. The probe stays off the main thread's blocking paths, and it reads geometry only.
  • Per-turn cost is O(followed windows). Per-frame cost is O(that window's terminals).
  • stop skips clients that are not active, so a second stop does not add the presenter counts twice.
  • The 600 s expiry stays.

4. Tests (not vacuous; red for the right reason).

  • At 31c3daf, test_a_terminal_not_sampled_in_the_motion_fails fails with AssertionError: 0 != 1 : [] (no problem was reported).
  • At 0c16e59, test_a_lagging_frame_fails_and_names_the_terminal_its_frames_and_sizes fails the same way.
  • At the head, python3 -I scripts/cmux-next/tests/test_bench_terminal_resize.py gives 27 OK.
  • The new Swift test aSizeAskedOfGhosttyAndNotShownCountsAsInFlight checks in_flight, lagging and held. I did not run it, and it has no fleet step id in the PR.

5. No CI job, gate, perf-gate row or budget is added. No .github or perf-gate files change. Report keys are only added: in_flight, follow_presenter/presenter, in_flight_frames, presenter_keeps. No key is renamed.

6. Arch and tables.

  • arch-ratchet.py --base 135c1834597: no count rises.
  • swift-logic-lines.py --missing: no TerminalPaneFollow* row is missing.
  • TerminalPaneFollowPresenterCounts has a table row.

7. CI at the head.

8. With #36242. There is no code dependency. #36242 changes DebugFrameProbe (debug.frames) only, and this sampler does not read it. The two PRs conflict only in scripts/cmux-next/terminal_resize_arms.py (the docstring and the arm setdefault dict). Whichever PR lands second keeps both key sets.

Review (independent subagent): SHIP at ed6ba20

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Replaced by #36288. #36109 merged as one squashed commit, so this branch conflicted with feat-cmux-next. #36288 carries the same four commits, cherry-picked onto feat; their files are byte-equal to this head (ed6ba20).

This branch was previously deployed

1 inactive deployment
ci-routing-agent — ed6ba207 Deployed Nov 5, 2026 by lawrencecchen via namespace autofix #14955
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant