fix(terminal): keep selections aligned while scrolling

[peters]

Purpose

Fix terminal text selections falling behind the pointer when scrolling down, or extending beyond it when scrolling up. Selection endpoints were mapped before same-frame wheel events changed the viewport, and a held pointer outside the terminal body had no repaint cadence to keep auto-scrolling.

Behavior impact

• Remap an active selection endpoint after local scrollback changes while preserving wheel, press, and release event order.
• Continue out-of-body selection auto-scroll every 16 ms only while the viewport can still move, stopping at history boundaries and on release.
• Add regression coverage for both scroll directions, boundaries, repaint lifetime, and same-frame press/release ordering.
• Process primary release events in frame order so a same-frame release-then-press keeps the restarted drag extendable, and narrow the post-scroll replay to local-selection anchors so it can never reopen Ctrl/Cmd+click targets.
• Split the terminal input module into keyboard, routing, and frame-events leaves to stay inside the maintainability line budget.

Test evidence

• cargo fmt --all -- --check
• ./scripts/check-maintainability.sh
• RUSTFLAGS="-D warnings" cargo test --offline --workspace
• RUSTFLAGS="-D warnings" cargo test --offline --workspace --features speech
• cargo clippy --offline --all-targets --features speech,trace-profiling -- -D warnings
• cargo clippy --offline --workspace --lib --bins --examples --features speech -- -D warnings -D clippy::unwrap_used -D clippy::expect_used
• cargo clippy --offline --workspace --all-targets --features speech -- -D warnings -W clippy::pedantic
• Live debug smoke on NVIDIA/Vulkan: held selections stayed pointer-aligned through downward and upward scrolling in Shell and Codex panels; the Claude panel passed the same gesture through its native mouse-reporting history; stationary out-of-body Shell selection continued to the bottom boundary and stopped; workspace fit and live window resize remained correct.

[Copilot]

Pull request overview

Fixes terminal selection alignment during scrolling and adds continuous out-of-body auto-scroll.

Changes:

• Remaps selection endpoints after viewport scrolling.
• Adds timed auto-scroll repainting.
• Adds regression tests for scrolling, boundaries, and event ordering.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
input.rs	Integrates scroll-aware selection handling.
input/viewport_scroll.rs	Adds selection scrolling and endpoint helpers.
input/selection_tests.rs	Adds selection-scroll regression coverage.

Comments suppressed due to low confidence (1)

crates/horizon-ui/src/terminal_widget/input.rs:453

• This schedules a repaint after 16 ms but does not throttle the scroll operation itself. Any earlier repaint (for example, while another UI animation calls request_repaint) invokes this block again and scrolls immediately, so out-of-body selection speed becomes frame-rate dependent and can substantially exceed the intended 16 ms cadence. Track the next eligible auto-scroll time in the drag state and only scroll once that deadline has elapsed; the repaint test should also advance frame time to verify the throttle.

        if auto_scrolled {
            pointer.ui_ctx.request_repaint_after(SELECTION_AUTO_SCROLL_INTERVAL);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

crates/horizon-ui/src/terminal_widget/input.rs:349

• Primary transitions are still not fully order-aware. For an input batch press inside → release → press outside, body_primary_press_pos retains the first press while the final pointer state is down. This sets drag_restarted_by_press, suppresses completion, and leaves a selection drag active toward the outside pointer even though the local press was already released. Track the primary-button lifecycle in event order so a later non-local press cannot revive an earlier in-body press.

    let mut drag_restarted_by_press = false;
    if let Some(pos) = frame_events.body_primary_press_pos

[peters]

Code review (max effort): findings fixed in 5efd1db

Overview

The PR correctly identifies why selections drift while scrolling: start_selection/update_selection convert viewport coordinates to buffer points using the display offset at call time, so any same-frame scrollback change invalidates endpoints mapped earlier in the frame. The fix layers three mechanisms: post-scroll endpoint remapping (update_active_selection_after_scrollback), an in-order replay of claimed primary events against wheel scrolls, and a 16 ms repaint cadence for out-of-body auto-scroll that stops at history boundaries by comparing scrollback before/after. All three are sound, and the test harness (headless egui pass over a disconnected snapshot panel) is a genuinely good addition; it makes sub-frame event orderings deterministically testable, which no manual smoke can do.

Findings (all fixed)

1. Same-frame release→press killed the restarted drag (Copilot's thread, confirmed and slightly deeper than stated). handle_terminal_body_pointer_actions treated any primary release event in the frame as drag-ending, even when a later press restarted the drag, so the new selection could never be extended by the still-held pointer. Fixed by processing primary transitions order-aware: release_completes_drag is true only when the release is the frame's final primary transition or no in-body press restarted the drag.
2. [wheel, release, press] frames mis-remapped the new selection. The release replay ran for any claimed release, so it would clamp the new selection's endpoint to the old release position and consume the scroll flag, defeating the press re-anchor. The release replay now only fires for the drag-completing release (release_replay_index).
3. Replayed presses could re-open Ctrl/Cmd+click targets. The claimed-press replay re-ran all of handle_pointer_button, including the clickable branch: a same-frame wheel-before-Ctrl+press on a URL called open_url twice (reachable under touchpad momentum scrolling). The replay is now start_local_selection_at only, gated on local_primary_selection_allowed, so it can never re-trigger URL opens or PTY button reports.
4. Maintainability budget. input.rs was over the 1000-line production guardrail once the fixes landed. Split per AGENTS.md into single-purpose leaves: keyboard.rs (keyboard forwarding + its tests), routing.rs (mode/modifier routing predicates, paired with mouse_tests.rs), frame_events.rs (egui event-list scanning + PointerFrameEvents::collect). input.rs is now the pointer orchestrator (~640 production lines); no behavior changes from the split.

Regression coverage added

• release_then_press_in_one_frame_keeps_the_new_drag_extendable
• release_wheel_and_press_in_one_frame_anchor_the_new_drag_post_scroll
• primary_release_only_ends_frame_when_it_is_the_last_transition, final_primary_release_index_finds_the_last_release_event

The two harness tests fail on f73091b and pass on 5efd1db. These orderings live inside a single ~16 ms frame, so coverage is deterministic-test-based by necessity.

Verified good (no action)

• Boundary detection via before/after scrollback() comparison is correct: scroll_display(Scroll::Delta) clamps internally, so "did it move" is the right signal to stop the repaint loop at history edges.
• Repaint scheduling is gated on actual viewport movement, so a held pointer at the boundary stops burning frames (selection_auto_scroll_repaints_only_while_held_drag_can_move_viewport pins this).
• Hot-path cost: the new event scans run only for panels that pass should_handle_terminal_pointer, add no allocations, and are O(events).
• Press-before-wheel vs wheel-before-press semantics are correct because selections are buffer-anchored; the test pair pins both directions.

Known pre-existing limitations (out of scope, noted for the record)

• A rapid release→press mid-drag replaces the old selection before maybe_copy_selection_to_primary reads it, so the finished selection's X11 primary copy is dropped. Would need eager text capture; low value.
• The committed Cargo.lock is stale relative to the surge-core v1.0.0-beta.52 bump in ci: bump surge-core from v1.0.0-beta.50 to v1.0.0-beta.52 #255, so every local build rewrites it. Worth a one-line lockfile sync PR on main.

Test evidence (worktree of this branch)

cargo fmt --check, check-maintainability.sh, check-version-sync.sh, RUSTFLAGS="-D warnings" cargo test --workspace (default + --features speech), blocking clippy (speech,trace-profiling), strict clippy (unwrap_used/expect_used), pedantic clippy: all green.

[peters]

SMOKE-TEST REQUEST macOS/Metal — plan: docs/testing/2026-07-31-macos-terminal-selection-scrollback-smoke.md — scope: selection/scrollback ordering, outside auto-scroll motion, release/re-entry, and mouse-reporting routing

[Copilot]

🟡 Not ready to approve

An outside release can trigger an additional auto-scroll step after its cadence deadline.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

crates/horizon-ui/src/terminal_widget/input.rs:437

• A release outside the body can still perform another auto-scroll step when its cadence deadline is ready, because the release path calls the same scrolling helper used while the button is held. This makes the viewport and final endpoint move on the release event, despite auto-scroll being required to stop on release. Finalize the endpoint without scrolling here; wheel-induced remapping is still handled later by the event replay.

        update_active_selection_drag(panel, pointer, selection_drag, pos, false);

• Files reviewed: 9/10 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[peters]

SMOKE-TEST REPORT (macOS 26.5.2 / Apple M4 Max Metal)

• Exact pushed head 4d930bf: pass — debug binary launched from the PR worktree with an isolated HOME/config and the exact PID/Quartz window scoped for automation.
• Baseline selection/copy and wheel-up/wheel-down while held: pass — copied numbered markers remained contiguous and aligned with the visible selection.
• Same-frame transition stress: pass — repeated inside release → outside press and inside release → fresh inside press sequences caused no endpoint extension, stale auto-scroll, crash, or stuck drag; focused RawInput tests are the decisive same-egui-frame evidence.
• Timed outside auto-scroll: pass — exact-window video captured continuous live-to-oldest movement; high-frequency native dragged events did not bypass the tested 16 ms deadline; release stopped continuation.
• Boundaries and re-entry: pass — oldest and live boundaries stopped cleanly, body re-entry cancelled outside scrolling, leaving/re-entering the native window then releasing left no stuck state.
• Mouse reporting: pass — Vim mouse=a accepted normal primary input, Shift created/copyable local selection with wheel input, and the immediate Shift-local release → Alt+primary sequence reached Vim (cursor/mode changed).
• Ordinary workflows: pass — typing, scrollback, workspace Fit, live panel resize, and fresh selection after stress remained functional.
• Persistence/relaunch: pass — the same binary restored the disposable session with no selection highlight or drag state persisted.
• Runtime diagnostics: pass — no panic, warning, error, or failed markers in the scoped log.
  Summary: all requested macOS lanes pass on 4d930bf; no additional live-smoke defects remain. Evidence is retained outside the repository under /tmp/horizon-pr258-smoke.ES9qXN/evidence.
  SMOKE-TEST: DONE

[Copilot]

🟡 Not ready to approve

Same-frame release-then-press handling can lose or incorrectly replace the completed Linux primary selection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

crates/horizon-ui/src/terminal_widget/input.rs:412

• A release that precedes the last local press is discarded here: PointerFrameEvents reports no completing release, and start_local_selection_at has already overwritten the old terminal selection. Consequently, a completed drag followed by a new press in the same frame never copies the completed text to the Linux primary selection (or drag_stopped_by copies the newly started selection instead). Preserve/copy the selection at each release before processing a later press, while still leaving the restarted drag active.

    if let Some(pos) = frame_events.body_primary_press_pos {
        pointer.interaction.body.request_focus();
        // Collection verified this exact event's modifiers and terminal mode
        // route it to local selection. The frame's final modifiers may belong
        // to a later outside press.
        start_local_selection_at(panel, pointer, pos);
        selection_drag.start(panel.id, pos);

• Files reviewed: 8/9 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.