fix(tui): preserve Codex wheel locations - #261
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
📝 WalkthroughWalkthroughCodex wheel scrolling now sends coordinate-aware SGR mouse reports when the input layer is ready. Alternate-scroll behavior remains unchanged for other sessions, and dead sessions still scroll emulator history. ChangesCodex wheel forwarding
Estimated code review effort: 3 (Moderate) | ~15 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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.
🧹 Nitpick comments (1)
src/tui/src/ui/harness_pane/tests/session.rs (1)
324-331: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the down-wheel branch.
This test sends only
up = true, so it checks button64but notBUTTON_WHEEL_DOWN(65) or the^[[Bcursor-key fallback. Add a down-wheel assertion or equivalent coverage in the mouse tests.As per coding guidelines, “Cover new branches with tests and maintain the repository's 80% line-coverage gate.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tui/src/ui/harness_pane/tests/session.rs` around lines 324 - 331, Extend the mouse-event tests around the existing “Codex to receive a location-aware wheel event” assertion to send a down-wheel event (`BUTTON_WHEEL_DOWN`/button 65) and verify it produces the expected location-aware sequence without falling back to the `^[[B` cursor key or navigating history. Preserve the existing up-wheel coverage and assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/tui/src/ui/harness_pane/tests/session.rs`:
- Around line 324-331: Extend the mouse-event tests around the existing “Codex
to receive a location-aware wheel event” assertion to send a down-wheel event
(`BUTTON_WHEEL_DOWN`/button 65) and verify it produces the expected
location-aware sequence without falling back to the `^[[B` cursor key or
navigating history. Preserve the existing up-wheel coverage and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b4af7a5-8d89-4607-b642-4ca584b99c93
📒 Files selected for processing (3)
src/tui/src/ui/harness_pane/mod.rssrc/tui/src/ui/harness_pane/mouse.rssrc/tui/src/ui/harness_pane/tests/session.rs
Summary
Root cause
Current Codex releases handle mouse events but do not negotiate terminal mouse reporting. Medulla therefore selected its Codex compatibility fallback, converted wheel notches into Up/Down keys, and necessarily discarded the pointer coordinates. Codex interpreted those keys as history/navigation input instead of scrolling the screen under the pointer.
Validation
Summary by CodeRabbit