Converge the mouse ladder slice 4: panels — activity bar, sidebar, terminal, bottom tabs, quickfix

[JDonaghy]

Continuation of #733, slice 4 of 6. #733 declared a 6-slice plan and was
closed when slice 1's PR merged — the remaining slices were never filed. This issue is
one of them. Measured on develop @ 44882e9 (2026-09-02):

handle_mouse (src/tui_main/mouse.rs) 3,044 · handle_mouse_event
(src/tui_main/shell_app.rs) 365 · handle_mouse_click_msg (src/gtk/mod.rs) 919 ·
handle_mouse_drag_msg 356 · try_route_sidebar_mouse_event 177 — 4,861 lines total,
against the 4,994 #733 was filed on. Slice 1 converged 133.

Locate everything below by symbol, not by line number — #733's own body warns that
its numbers drift, and they already have.

The rungs

Rung	TUI (mouse.rs banner)	GTK
activity bar	── Activity bar ──	handle_mouse_click_msg arm
sidebar panel area	── Sidebar panel area ──	try_route_sidebar_mouse_event (177 lines)
right-click sidebar routing (#575)	── #575: right-click sidebar panel routing ──	separate arm
terminal panel	── Terminal panel click ──	handle_mouse_click_msg arm
SC / ext-panel hover	── SC button hover ──, ── Ext panel hover ──	— blank
quickfix	inside the sidebar arm	handle_mouse_click_msg arm

This is the slice that owns #544 (GTK sidebar clicks) and the mechanism behind
#499 / #484 (ext-panel section headers only partly toggling on click): the ext-panel
hover and header rungs exist on TUI and are blank on GTK, so a fix on one side is invisible
on the other.

The engine already owns the actions — handle_dap_sidebar_action_click (dap_ops.rs),
handle_ext_panel_double_click (ext_panel.rs), handle_terminal_pane_click
(terminal_ops.rs). This slice is the ladder, not the handlers.

Acceptance criteria

Inherited verbatim from #733 — these are the rules that make a slice done:

• The rung is implemented once, in shared code (src/render.rs), and called from both
  backends.
• Both backends' bespoke arms for that rung are deleted in the same PR. A migration that
  leaves both paths is not done (GOALS.md, "How to use this doc").
• Black-box coverage on both backends — TuiDriver via driver_with_shell(TuiShellApp, …)
  in src/tui_main/shell_app.rs, and GtkDriver in src/gtk/testing.rs. Assert on rendered
  output, never on state being populated (CLAUDE.md rule 1).
• The PR states the new tests were verified RED against unfixed develop (CLAUDE.md rule 2).
• Combined production line count of the entry points above strictly decreases; the PR states
  before/after. (Converge the two mouse routers — ~4,800 lines implementing the same precedence ladder twice #733 slice 1 met this only barely — 4,994 → 4,861 — and Converge frame composition — ~4,500 lines laying down the same surfaces in two hand-kept orders #735 slice 1 did not
  meet it at all. State the numbers.)
• cargo build && cargo test && cargo clippy -- -D warnings && cargo fmt --check EXIT=0.

Scope discipline

This is one slice. Do not attempt the rungs belonging to the sibling issues listed in
Sequencing — they conflict in the same files and are queued behind this one on purpose.

Files

• src/render.rs (the shared router)
• src/tui_main/mouse.rs, src/tui_main/shell_app.rs
• src/gtk/mod.rs
• src/gtk/testing.rs (GtkDriver coverage)

Sequencing

After slice 3 (dividers). try_route_sidebar_mouse_event should be gone, or reduced to a
native-plumbing shim, by the end of this slice.

Relates: #544, #499, #484.

Milestone

#7 Platform-Neutral

[JDonaghy]

Coordinator Assignment

Machine: dell64
Repo: vimcode
Files: (unspecified)
Do not touch: (nothing else in flight)

Briefing

Issue #754: Converge the mouse ladder slice 4: panels — activity bar, sidebar, terminal, bottom tabs, quickfix

Continuation of #733, slice 4 of 6. #733 declared a 6-slice plan and was
closed when slice 1's PR merged — the remaining slices were never filed. This issue is
one of them. Measured on develop @ 44882e9 (2026-09-02):

handle_mouse (src/tui_main/mouse.rs) 3,044 · handle_mouse_event
(src/tui_main/shell_app.rs) 365 · handle_mouse_click_msg (src/gtk/mod.rs) 919 ·
handle_mouse_drag_msg 356 · try_route_sidebar_mouse_event 177 — 4,861 lines total,
against the 4,994 #733 was filed on. Slice 1 converged 133.

Locate everything below by symbol, not by line number — #733's own body warns that
its numbers drift, and they already have.

The rungs

Rung	TUI (mouse.rs banner)	GTK
activity bar	── Activity bar ──	handle_mouse_click_msg arm
sidebar panel area	── Sidebar panel area ──	try_route_sidebar_mouse_event (177 lines)
right-click sidebar routing (#575)	── #575: right-click sidebar panel routing ──	separate arm
terminal panel	── Terminal panel click ──	handle_mouse_click_msg arm
SC / ext-panel hover	── SC button hover ──, ── Ext panel hover ──	— blank
quickfix	inside the sidebar arm	handle_mouse_click_msg arm

This is the slice that owns #544 (GTK sidebar clicks) and the mechanism behind
#499 / #484 (ext-panel section headers only partly toggling on click): the ext-panel
hover and header rungs exist on TUI and are blank on GTK, so a fix on one side is invisible
on the other.

The engine already owns the actions — handle_dap_sidebar_action_click (dap_ops.rs),
handle_ext_panel_double_click (ext_panel.rs), handle_terminal_pane_click
(terminal_ops.rs). This slice is the ladder, not the handlers.

Acceptance criteria

Inherited verbatim from #733 — these are the rules that make a slice done:

• The rung is implemented once, in shared code (src/render.rs), and called from both
  backends.
• Both backends' bespoke arms for that rung are deleted in the same PR. A migration that
  leaves both paths is not done (GOALS.md, "How to use this doc").
• Black-box coverage on both backends — TuiDriver via driver_with_shell(TuiShellApp, …)
  in src/tui_main/shell_app.rs, and GtkDriver in src/gtk/testing.rs. Assert on rendered
  output, never on state being populated (CLAUDE.md rule 1).
• The PR states the new tests were verified RED against unfixed develop (CLAUDE.md rule 2).
• Combined production line count of the entry points above strictly decreases; the PR states
  before/after. (Converge the two mouse routers — ~4,800 lines implementing the same precedence ladder twice #733 slice 1 met this only barely — 4,994 → 4,861 — and Converge frame composition — ~4,500 lines laying down the same surfaces in two hand-kept orders #735 slice 1 did not
  meet it at all. State the numbers.)
• cargo build && cargo test && cargo clippy -- -D warnings && cargo fmt --check EXIT=0.

Scope discipline

This is one slice. Do not attempt the rungs belonging to the sibling issues listed in
Sequencing — they conflict in the same files and are queued behind this one on purpose.

Files

• src/render.rs (the shared router)
• src/tui_main/mouse.rs, src/tui_main/shell_app.rs
• src/gtk/mod.rs
• src/gtk/testing.rs (GtkDriver coverage)

Sequencing

After slice 3 (dividers). try_route_sidebar_mouse_event should be gone, or reduced to a
native-plumbing shim, by the end of this slice.

Relates: #544, #499, #484.

Milestone

#7 Platform-Neutral

[JDonaghy]

Coordinator: Assignment Complete

Machine: dell64
Status: done
Exit code: 0
Duration: 31m 14s
Log: /home/john/.coord/logs/e6cc1a067016.log

[JDonaghy]

Coordinator: Assignment Complete

Machine: dell64
Status: done
Exit code: 0
Duration: 3m 26s
Log: /home/john/.coord/logs/52019113f83a.log

[JDonaghy]

Coordinator: Assignment Complete

Machine: precision
Status: done
Exit code: 0
Duration: —

Summary

Verdict: request-changes

Review findings

Blocking findings

• The "sidebar panel area" rung — the one the issue names explicitly and gives a hard exit
  condition for — is not actually converged. try_route_sidebar_mouse_event
  (src/gtk/mod.rs, ~line 4192 post-PR) goes from 177 lines to 170: the only change is
  swapping the match active_id.as_str() { PANEL_EXPLORER => ..., ... } string match for
  match &owner { render::SidebarOwner::Explorer => ..., ... }. Every arm's body —
  self.explorer_ui_event(...), self.route_debug_sidebar_event(...),
  self.route_sc_sidebar_event(...), self.route_ai_sidebar_event(...),
  engine.handle_ext_sidebar_ui_event(...), the picker/context-menu precedence checks, the
  drag-capture bookkeeping — is untouched, byte-for-byte identical to develop. I diffed
  explorer_ui_event, route_debug_sidebar_event, route_sc_sidebar_event and
  route_ai_sidebar_event before/after the PR and they are unchanged. TUI's mirror — the
  owner == render::SidebarOwner::X chain inside handle_mouse (src/tui_main/mouse.rs,
  the block starting around the old active_panel_is(PANEL_EXPLORER) site) — got the same
  swap and nothing else; the explorer-row-click math, the DAP sidebar dispatch, the SC event
  dispatch, etc. are all still inline, TUI-only code.

  This directly contradicts three things the issue and repo docs make non-negotiable:

  • The issue's own Sequencing note: "try_route_sidebar_mouse_event should be gone, or
    reduced to a native-plumbing shim, by the end of this slice." 170 lines of full per-panel
    dispatch logic is not a shim.
  • The Acceptance Criteria: "The rung is implemented once, in shared code (src/render.rs),
    and called from both backends. Both backends' bespoke arms for that rung are deleted in
    the same PR. A migration that leaves both paths is not done" (GOALS.md, "How to use this
    doc").
  • The PR's own commit message (256bb19): "Both backends' bespoke arms are deleted in this
    commit" — true for the activity-bar, bottom-panel and hover rungs, but not true for the
    sidebar-panel-area rung, which the issue's rung table lists by name and by line count
    (177 lines) as one of the things this slice owns.

  What did get shared here — render::sidebar_owner(), the "who owns the sidebar body"
  precedence — is real and correctly used by both the click router, the hover router and (per
  its own doc comment) the painter. But that's the input to the rung, not the rung itself; the
  actual per-panel event routing this issue calls out is still fully duplicated on both
  backends.

• render::apply_activity_panel_switch's own doc comment states it fixes two real,
  previously backend-specific bugs by sharing the code: "Extension panel input dies intermittently: scrollwheel never works, and the scrollbar/? stop responding after a help-popup dismiss or an editor focus round-trip #637's focus clear was TUI-only" (a
  bug that now no longer exists on GTK because GTK calls the shared function), and "the
  re-entry guard was GTK-only" (now fixed on TUI for the same reason). These are genuine
  user-visible behavior changes on the backend that used to lack the fix, not pure refactors.
  No black-box test in either src/gtk/testing.rs or src/tui_main/shell_app.rs demonstrates
  either fix (e.g. "switching from the Extensions marketplace panel to a plugin panel on GTK
  clears the stale ext_sidebar_has_focus intercept" or "clicking an already-active plugin
  panel's activity-bar icon twice on TUI doesn't reset its scroll/re-fire panel_focus"). Per
  CLAUDE.md ("Every PR that changes user-visible behaviour must ship a black-box test... The
  adversarial reviewer rejects behaviour-changing PRs that lack one") and this review's brief
  ("A behaviour change with no driver test is a request-changes"), this needs coverage before
  merge — the two black-box tests actually shipped (bottom_panel_tab_strip_click_...,
  source_control_toolbar_button_highlights_on_hover, plus the TUI quickfix-band test) cover
  the bottom-panel and hover rungs well, but nothing exercises the activity-panel-switch fixes
  the PR itself claims to be making.

Non-blocking concerns

• I could not reach the PR body text itself (no gh/web access from this review session), so
  I can't confirm whether it states the required before/after production line counts for the
  five named entry points. I measured them independently against the PR's own merge-base
  (043f15c, i.e. after slice 3): handle_mouse 2766→2632, handle_mouse_event 360→360
  (unchanged), handle_mouse_click_msg 548→520, handle_mouse_drag_msg 292→292
  (unchanged), try_route_sidebar_mouse_event 177→170. Combined: 4143→3974, a decrease of
  169 lines (~4%) — technically satisfies "strictly decreases," but two of the five entry
  points show zero movement at all, which lines up with the sidebar-rung gap above: nothing
  meaningful was removed from handle_mouse_event (GTK's ShellApp::handle, where the new
  hover call was added, offsetting whatever else shrank) or from handle_mouse_drag_msg
  (untouched by this PR). Worth the coordinator double-checking the PR body actually states
  real numbers rather than the issue's stale pre-slice baseline (4861 total, measured at
  44882e9 before any of slices 1–3 ran).

• The right-click sidebar routing rung (TUI: wrong context menu opens in Debug/Search/Git panels; File-menu dropdown doesn't dismiss on outside click #575), also named in the issue's rung table, gets the
  same treatment as the sidebar-panel-area rung: TUI's arm (mouse.rs, the block guarding
  explorer-tree right-click) now checks render::sidebar_owner(engine) == render::SidebarOwner::Explorer instead of engine.active_panel_is(PANEL_EXPLORER), but no
  new shared function exists for it and GTK's side is untouched. Consistent with — and
  reinforces — the main sidebar-panel-area finding above rather than being separate scope.

• GTK's new route_sidebar_hover call site (in the MouseMoved arm of ShellApp::handle,
  near the CSD-titlebar comment) only runs when ctx.layout.sidebar_content_bounds is
  Some, i.e. it's skipped entirely whenever nothing painted a sidebar this frame. TUI instead
  calls route_sidebar_hover on every Moved event and lets the function's own
  sidebar_visible parameter decide the dismiss path. In practice this looks equivalent (no
  sidebar geometry ⇒ nothing to hover), but it does mean GTK never exercises the "sidebar just
  closed, drop the stale hover card" branch through this call site the way TUI does. Probably
  fine since apply_activity_panel_switch / toggle_sidebar likely clear hover state via a
  different path, but worth a second look — not blocking on the evidence I could gather.

Nits

None.

[JDonaghy]

Review Complete — ⚠️ Changes Requested

Reviewer findings could not be posted directly to PR #771 (gh pr review was rejected — likely a self-review restriction). Findings are reproduced here.

Note: a completion comment was posted earlier but findings could not be extracted at that time. These are the retroactive findings.

Blocking findings

• The "sidebar panel area" rung — the one the issue names explicitly and gives a hard exit condition for — is not actually converged. try_route_sidebar_mouse_event (src/gtk/mod.rs, ~line 4192 post-PR) goes from 177 lines to 170: the only change is swapping the match active_id.as_str() { PANEL_EXPLORER => ..., ... } string match for match &owner { render::SidebarOwner::Explorer => ..., ... }. Every arm's body — self.explorer_ui_event(...), self.route_debug_sidebar_event(...), self.route_sc_sidebar_event(...), self.route_ai_sidebar_event(...), engine.handle_ext_sidebar_ui_event(...), the picker/context-menu precedence checks, the drag-capture bookkeeping — is untouched, byte-for-byte identical to develop. I diffed explorer_ui_event, route_debug_sidebar_event, route_sc_sidebar_event and route_ai_sidebar_event before/after the PR and they are unchanged. TUI's mirror — the owner == render::SidebarOwner::X chain inside handle_mouse (src/tui_main/mouse.rs, the block starting around the old active_panel_is(PANEL_EXPLORER) site) — got the same swap and nothing else; the explorer-row-click math, the DAP sidebar dispatch, the SC event dispatch, etc. are all still inline, TUI-only code.

  This directly contradicts three things the issue and repo docs make non-negotiable:

  • The issue's own Sequencing note: "try_route_sidebar_mouse_event should be gone, or reduced to a native-plumbing shim, by the end of this slice." 170 lines of full per-panel dispatch logic is not a shim.
  • The Acceptance Criteria: "The rung is implemented once, in shared code (src/render.rs), and called from both backends. Both backends' bespoke arms for that rung are deleted in the same PR. A migration that leaves both paths is not done" (GOALS.md, "How to use this doc").
  • The PR's own commit message (256bb19): "Both backends' bespoke arms are deleted in this commit" — true for the activity-bar, bottom-panel and hover rungs, but not true for the sidebar-panel-area rung, which the issue's rung table lists by name and by line count (177 lines) as one of the things this slice owns.

  What did get shared here — render::sidebar_owner(), the "who owns the sidebar body" precedence — is real and correctly used by both the click router, the hover router and (per its own doc comment) the painter. But that's the input to the rung, not the rung itself; the actual per-panel event routing this issue calls out is still fully duplicated on both backends.

• render::apply_activity_panel_switch's own doc comment states it fixes two real, previously backend-specific bugs by sharing the code: "Extension panel input dies intermittently: scrollwheel never works, and the scrollbar/? stop responding after a help-popup dismiss or an editor focus round-trip #637's focus clear was TUI-only" (a bug that now no longer exists on GTK because GTK calls the shared function), and "the re-entry guard was GTK-only" (now fixed on TUI for the same reason). These are genuine user-visible behavior changes on the backend that used to lack the fix, not pure refactors. No black-box test in either src/gtk/testing.rs or src/tui_main/shell_app.rs demonstrates either fix (e.g. "switching from the Extensions marketplace panel to a plugin panel on GTK clears the stale ext_sidebar_has_focus intercept" or "clicking an already-active plugin panel's activity-bar icon twice on TUI doesn't reset its scroll/re-fire panel_focus"). Per CLAUDE.md ("Every PR that changes user-visible behaviour must ship a black-box test... The adversarial reviewer rejects behaviour-changing PRs that lack one") and this review's brief ("A behaviour change with no driver test is a request-changes"), this needs coverage before merge — the two black-box tests actually shipped (bottom_panel_tab_strip_click_..., source_control_toolbar_button_highlights_on_hover, plus the TUI quickfix-band test) cover the bottom-panel and hover rungs well, but nothing exercises the activity-panel-switch fixes the PR itself claims to be making.

Non-blocking concerns

• I could not reach the PR body text itself (no gh/web access from this review session), so I can't confirm whether it states the required before/after production line counts for the five named entry points. I measured them independently against the PR's own merge-base (043f15c, i.e. after slice 3): handle_mouse 2766→2632, handle_mouse_event 360→360 (unchanged), handle_mouse_click_msg 548→520, handle_mouse_drag_msg 292→292 (unchanged), try_route_sidebar_mouse_event 177→170. Combined: 4143→3974, a decrease of 169 lines (~4%) — technically satisfies "strictly decreases," but two of the five entry points show zero movement at all, which lines up with the sidebar-rung gap above: nothing meaningful was removed from handle_mouse_event (GTK's ShellApp::handle, where the new hover call was added, offsetting whatever else shrank) or from handle_mouse_drag_msg (untouched by this PR). Worth the coordinator double-checking the PR body actually states real numbers rather than the issue's stale pre-slice baseline (4861 total, measured at 44882e9 before any of slices 1–3 ran).

• The right-click sidebar routing rung (TUI: wrong context menu opens in Debug/Search/Git panels; File-menu dropdown doesn't dismiss on outside click #575), also named in the issue's rung table, gets the same treatment as the sidebar-panel-area rung: TUI's arm (mouse.rs, the block guarding explorer-tree right-click) now checks render::sidebar_owner(engine) == render::SidebarOwner::Explorer instead of engine.active_panel_is(PANEL_EXPLORER), but no new shared function exists for it and GTK's side is untouched. Consistent with — and reinforces — the main sidebar-panel-area finding above rather than being separate scope.

• GTK's new route_sidebar_hover call site (in the MouseMoved arm of ShellApp::handle, near the CSD-titlebar comment) only runs when ctx.layout.sidebar_content_bounds is Some, i.e. it's skipped entirely whenever nothing painted a sidebar this frame. TUI instead calls route_sidebar_hover on every Moved event and lets the function's own sidebar_visible parameter decide the dismiss path. In practice this looks equivalent (no sidebar geometry ⇒ nothing to hover), but it does mean GTK never exercises the "sidebar just closed, drop the stale hover card" branch through this call site the way TUI does. Probably fine since apply_activity_panel_switch / toggle_sidebar likely clear hover state via a different path, but worth a second look — not blocking on the evidence I could gather.

Nits

None.