dogfood: record the modifier tour's hover steps as trees, not frames

[teamleaderleo]

The modifier tour shot a plain-hover and a cmd-hover frame of the same cell. No tour frame can show the difference those two are named for, and the reason is structural rather than about any one affordance.

holding() wraps only its own step in XCUIElement.perform(withKeyModifiers:) (cmuxUITests/DogfoodScenarioUITests.swift), and the wait and shot that follow are separate steps. Command is already released by the time the capture happens, flagsChanged forwards that release to Ghostty, and the link highlight goes with it.

That covers both affordances on this path:

• GHOSTTY_ACTION_MOUSE_OVER_LINK raises TerminalLinkHoverIndicatorView, a HUD badge pinned to the terminal's bottom-leading corner. Its default hover_mods is ctrlOrSuper, so it is gone by capture time. This is a genuine on-screen affordance, not a cursor, and it still cannot be photographed here.
• A pointing-hand cursor is invisible twice over, because neither window.screenshot() nor XCUIScreen.main.screenshot() draws the pointer at all.

Measured, on both pairs this tour has published

Pair	Pixels differing	What they are
60656d76	3354, 0.70% of the image	block cursor blink phase, plus a strip of macOS Dock icons overlapping the window for one capture
15792/cf91daf7	30, inside one 9x16 cell	the same blinking cursor

No badge in any of the four frames. Worth noting on its own: a window.screenshot() image is not clean of other applications' chrome.

Change

Both hover steps stay and still exercise the hover path. Their record becomes a tree, which is an honest downgrade and not a better home: a tree is captured after the same modifier release, so it does not hold the hover state either. This is media-noise cleanup.

shots: 01-cmd-click-opens-example, 02-cmd-click-opens-github
trees: 01-cmd-click-terminal-example, 01-plain-hover-example, 01-cmd-hover-example,
       01-cmd-click-opens-example, 02-cmd-click-terminal-github, 02-cmd-click-opens-github

pick_key_shots publishes at most four frames, so the two deleted ones were taking half the slots a reviewer sees. Because this edits the tour's own file, select_tours runs this tour on this PR, so the new media is self-verifying.

The affordance itself belongs in an XCTest asserting hover state directly, which is where #15259 already puts it.

This supersedes the reasoning I gave on #15421 and the first version of this description, both of which named the cursor as the only affordance and missed the badge.

Summary by CodeRabbit

• Tests
  • Updated the modifier-click tour’s hover checks to capture structural snapshots instead of screenshots. Existing snapshot names are unchanged; no end-user behavior changes are included in this update.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 66d4b525-ee25-461d-90d8-ea9a20ed4770

📥 Commits

Reviewing files that changed from the base of the PR and between 02dac3c and 1c9906c.

📒 Files selected for processing (1)

• dogfood/scenarios/modifier-clicks-tour.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The plain-hover and command-hover steps in the modifier-clicks tour now capture tree snapshots instead of screenshots. Their snapshot names remain unchanged.

Changes

Modifier-clicks tour

Layer / File(s)	Summary
Update hover captures dogfood/scenarios/modifier-clicks-tour.json	The plain-hover and command-hover steps capture tree snapshots instead of screenshots. Their snapshot names remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 1c990

The two hover steps now produce text accessibility-tree attachments instead of screenshots while retaining their names. No actionable breakage is established; the change is mergeable subject to normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 1c990

The change affects 1 system.

Changed systems: dogfood

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — dogfood (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in dogfood/scenarios/modifier-clicks-tour.json: The plain-hover step now records a tree snapshot instead of a screenshot.
• observed — Modified behavior in dogfood/scenarios/modifier-clicks-tour.json: The command-hover step now records a tree snapshot instead of a screenshot.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description gives a detailed and relevant rationale, but it does not follow the repository template. It omits the required Summary, Testing, Changelog, Demo Video, and Checklist sections.	Organize the existing explanation under the required template headings. Add explicit Testing commands and results, set Changelog to "none" because this is an internal-only change, state whether a demo is not applicable or attach one, and co…

✅ Passed checks (24 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS: The pull request changes only two capture declarations in dogfood/scenarios/modifier-clicks-tour.json, from shot to tree. It does not change Cloud terminal creation, cmux-tui clients, tran…
Cmux Swift Actor Isolation	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two shot captures with tree captures and introduces no Swift changes. Therefore, it cannot introduce or wor…
Cmux Swift Blocking Runtime	✅ Passed	The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two shot entries with tree entries. The diff contains no Swift files and introduces no blocking or timing-b…
Cmux Browser Automation Off-Main	✅ Passed	PASS. The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two shot captures with tree captures. It does not change browser.* socket commands, worker routing,…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It changes two capture entries from shot to tree and adds no Swift code or agent-history loading path. The expens…
Cmux Cache Substitution Correctness	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json, replacing two shot capture entries with tree entries. It contains no production Swift, TypeScript, or JavaScript …
Cmux No Hacky Sleeps	✅ Passed	The pull request changes only dogfood/scenarios/modifier-clicks-tour.json, replacing two shot captures with tree captures. It introduces no sleep, timer, polling, retry, or delayed-dispatch logi…
Cmux Algorithmic Complexity	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. The two changes replace shot entries with tree entries in scenario data. No production Swift, TypeScript, JavaScr…
Cmux Swift Concurrency	✅ Passed	The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. The two-line diff replaces shot with tree for two capture steps. It adds no Swift code and does not introduce any legac…
Cmux Swift @Concurrent	✅ Passed	PASS. The authoritative pull-request diff changes only dogfood/scenarios/modifier-clicks-tour.json, replacing two shot entries with tree entries. It introduces no Swift code, async function, act…
Cmux Swift Package Boundaries	✅ Passed	The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two shot entries with tree entries. The diff contains no Swift, SwiftPM, or Xcode project changes, so the S…
Cmux Swiftpm Lockfiles	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json, replacing two shot fields with tree fields. It changes no SwiftPM package, Package.swift, .gitignore, workflo…
Cmux Swift Logging	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It adds no Swift changes and adds or modifies no logging statements covered by `.github/review-bot-rules/swift-loggin…
Cmux User-Facing Error Privacy	✅ Passed	PASS: The PR changes only two capture keys in dogfood/scenarios/modifier-clicks-tour.json, from shot to tree. The scenario documentation defines these as screenshot and accessibility-tree record…
Cmux Full Internationalization	✅ Passed	PASS: The PR changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two capture fields from shot to tree and adds no user-facing text, Swift localization keys, string-catalog entr…
Cmux Swiftui State Layout	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two shot entries with tree entries and introduces no SwiftUI or Swift source changes. The SwiftUI state…
Cmux Architecture Rethink	✅ Passed	PASS: The PR changes only dogfood/scenarios/modifier-clicks-tour.json. It does not change Swift code or introduce timing repairs, state ownership, bridge code, or duplicate UI wiring covered by the …
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. It replaces two "shot" entries with "tree" entries. It adds or materially changes no Swift, NSWindow, NSPanel, NSWindow…
Cmux Source Artifacts	✅ Passed	The PR changes only dogfood/scenarios/modifier-clicks-tour.json, an existing checked-in dogfood scenario fixture. The diff changes two capture declarations from shot to tree; it does not add log…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS: The pull request changes only dogfood/scenarios/modifier-clicks-tour.json. The authoritative diff contains no Swift files under a production Sources/ path, so this production-source seam che…
Title check	✅ Passed	The title clearly and concisely describes the main change: recording the modifier tour's hover steps as trees instead of screenshots.

Full details: Description check

Resolution

Organize the existing explanation under the required template headings. Add explicit Testing commands and results, set Changelog to "none" because this is an internal-only change, state whether a demo is not applicable or attach one, and complete the applicable checklist items.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[teamleaderleo]

Review

Review subagent on e3ce546915, told to be skeptical rather than agreeable. It cleared the change and rejected my reasoning for it, which was the right call on both counts. Head is now 1c9906cb19.

Fixed

1. The body cited a symbol that does not exist in this base. I named gitHubReferenceIsUnderPointer as the affordance behind these frames. Case-insensitive grep for gitHubReference across Sources/ and Packages/ on main returns zero hits. That symbol lives on Show the cmd-hover affordance over GitHub references #15259's branch, which is where I had been reading, and it has nothing to do with a tour that prints plain https://example.com/ URLs. Removed.
2. "The affordance is a cursor shape" was wrong for this path. Cmd-hovering a URL raises TerminalLinkHoverIndicatorView, a HUD badge with the URL text pinned to the terminal's bottom-leading corner, driven by GHOSTTY_ACTION_MOUSE_OVER_LINK, whose default hover_mods is ctrlOrSuper per docs/ghostty-fork.md. It is on-screen, it is load-bearing (GhosttyNSView+RevealInFinder.swift reads linkHoverIndicatorView.url as the app's record of the hovered link), and a screenshot could in principle show it. My blanket claim ignored it.
3. The actual mechanism, which is more general than what I wrote. holding() wraps only its own step in XCUIElement.perform(withKeyModifiers:); the wait and shot that follow are separate steps. Command is released before the capture, flagsChanged forwards the release to Ghostty, and the highlight clears. So no tour frame can capture any modifier-held state, badge or cursor. That is worth more than the narrow claim it replaces.
4. I was overselling the replacement. The body implied a tree is where this affordance can be asserted instead. It is not: a tree is captured after the same release and does not hold hover state either. Reworded as an honest downgrade and media-noise cleanup.

Body and commit message both rewritten around the corrected reasoning.

Confirmed, with the checks I would have wanted

• tree is a first-class step kind, decoded and executed as an attachment; a tree and a shot sharing a name cannot collide, because attachment names carry the 1-based step label, and the two kinds write to different directories (frames/ versus attachments/) in scripts/ci/e2e-frames.py. All 22 steps enumerated, zero duplicate names.
• Nothing else in the repo references 01-plain-hover-example or 01-cmd-hover-example.
• No registry update needed: the execution registry discovers only tests/test_*.py, and tours are discovered at head SHA. python3 -m unittest tests.test_ci_pr_media passes, 64 tests.
• pick_key_shots executed directly: 4 own shots before, exactly 2 after, both published, no thinning. Cross-checked against the last published manifest.json. The GIF still renders, since SKIP_IN_GIF drops only 99-final-screen and two moving frames is the floor.
• The surviving 01-cmd-click-opens-example frame still shows the terminal full of URLs beside the opened browser, so the GIF loses no context.

Independent measurement

The reviewer pulled a different published pair than I did, 15792/cf91daf7, and diffed it: 30 pixels over a single 9x16 cell, the blinking prompt cursor, no badge. That agrees with my 60656d76 pair, where the larger 3354-pixel delta was the same cursor plus an overlapping Dock strip. Two pairs, two capture sessions, same answer.

Left

• Nothing blocking. The diff is still the same two lines.

[github-actions]

Dogfood tours of 1c9906cb

modifier-clicks-tour at 1c9906cb, on an app compiled for the tour: passed (run)

Key frames of modifier-clicks-tour at 1c9906c

_{Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.}

[teamleaderleo]

Dogfood evidence is published and I looked at both surviving frames.

14-01-cmd-click-opens-example: the terminal is full of https://example.com/, and the embedded browser beside it is on an Example Domain tab at https://example.com/. 21-02-cmd-click-opens-github: same terminal filled with https://github.com/manaflow-ai/cmux, embedded browser on the loaded repo page. Both are cmd-click results, captured after the modifier was released, which is the whole point: they show an outcome, not a held state.

Neither shows a hover badge or a changed cursor, which is what the two removed frames were supposed to show and never could. The GIF still renders from the two moving frames.

Green at 1c9906cb, 64 checks, no failures. Merging on green under the fix rule.

[github-actions]

Merge receipt for 1c9906cb19: every check was green at merge (17 verified; 20 skipped by policy). Full suite runs on main after merge.