refactor: replace Qt overrides with native mechanisms, install app filters once

Per-window installs of the two QApplication-level filters (cursor,
wheel guard) stacked one stale copy per MainWindow; now installed once
per process, parented to the app. closeEvent overrides in the baton
dialogs replaced by finished.connect(timer.stop); keyPressEvent in
VideoGraphicsView replaced by WidgetShortcut QShortcuts; hover
enter/leave overrides in PlayPauseButton replaced by underMouse();
ValueLabel inherits ClickableLabel instead of duplicating its
mousePressEvent. Dead raster-timer branch and a PyQt5-era
event-position fallback deleted; redundant try/except and getattr
guards dropped now that class-level defaults cover pre-init reads.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/aare/gui/main_window.py

@@ -167,10 +167,12 @@ class _AlertBannerHost(QWidget):
 class MainWindow(QMainWindow):
     sample_geometry = Signal(SampleGeometryModel)
 
-    # Set lazily outside __init__ (first use guards with getattr/default);
-    # declared for the basedpyright gate.
+    # Class-level defaults: Qt can call overrides (showEvent, closeEvent)
+    # before __init__ finishes, so these must be readable without a getattr
+    # guard; also typed for the basedpyright gate.
     _session_operations_enabled: bool | None = None
     _default_dock_split_done: bool = False
+    _cleanup_done: bool = False
     _pre_watch_dock_state: QByteArray | None = None
     _pre_watch_visibility: list[tuple[QWidget, bool]] | None = None
 
@@ -207,7 +209,6 @@ class MainWindow(QMainWindow):
         self._beamline_recovery_dialog = None
         self._local_contact_dialog = None
         self._controls_help_dialog = None
-        self._cleanup_done = False
         self._default_window_state = None
         self._pre_automation_window_state = None
         self._pre_automation_left_column_visible = True
@@ -241,18 +242,19 @@ class MainWindow(QMainWindow):
         self.state_manager = UIStateManager("PSI", "AareGUI")
 
         # App-level, not window-level: dialogs and pop-outs get it too.
-        self._clickable_cursor_filter = ClickableCursorFilter(self)
+        # Installed once per process and parented to the app, NOT the window:
+        # the old per-window copies stacked up (one per MainWindow the test
+        # suite builds) and kept filtering every process event after their
+        # parent window died — stale wrappers in the hottest Qt→Python path.
         app = QApplication.instance()
         assert app is not None
-        app.installEventFilter(self._clickable_cursor_filter)
-
-        # Wheel safety: sliders/spin boxes/combos only react to the wheel
-        # while the right mouse button is held; a bare wheel just scrolls
-        # the page — it can never nudge a value or move a motor.
-        self._wheel_value_guard = WheelValueGuard(self)
-        app_instance = QApplication.instance()
-        if app_instance is not None:
-            app_instance.installEventFilter(self._wheel_value_guard)
+        if not app.property("_aare_app_filters_installed"):
+            app.setProperty("_aare_app_filters_installed", True)
+            app.installEventFilter(ClickableCursorFilter(app))
+            # Wheel safety: sliders/spin boxes/combos only react to the wheel
+            # while the right mouse button is held; a bare wheel just scrolls
+            # the page — it can never nudge a value or move a motor.
+            app.installEventFilter(WheelValueGuard(app))
         self.viewer = JFJochDBusClient()
 
         try:
@@ -2992,7 +2994,7 @@ class MainWindow(QMainWindow):
         # default dock split AFTER the real (maximized) geometry exists —
         # the __init__ resizeDocks ran on the pre-show size and Qt hands the
         # scale-up surplus to the sample list, skewing 50/50 into ~80/20.
-        if not getattr(self, "_default_dock_split_done", False):
+        if not self._default_dock_split_done:
             self._default_dock_split_done = True
             if not self.state_manager.settings.value("main_window/state"):
                 QTimer.singleShot(0, self._apply_default_dock_split)
@@ -3041,7 +3043,7 @@ class MainWindow(QMainWindow):
         super().closeEvent(event)
 
     def cleanup(self):
-        if getattr(self, "_cleanup_done", False):
+        if self._cleanup_done:
             return
 
         try:
@@ -3209,21 +3211,21 @@ class MainWindow(QMainWindow):
             obj.setFloating(False)
             event.ignore()
             return True
-        try:
-            if event.type() in {
-                QEvent.Type.MouseButtonPress,
-                QEvent.Type.MouseButtonRelease,
-                QEvent.Type.MouseMove,
-                QEvent.Type.Wheel,
-                QEvent.Type.KeyPress,
-                QEvent.Type.KeyRelease,
-                QEvent.Type.FocusIn,
-                QEvent.Type.TouchBegin,
-                QEvent.Type.TouchUpdate,
-            }:
-                self._mark_user_interaction()
-        except Exception as e:
-            logger.debug(f"GUI interaction event filter error: {e}", exc_info=True)
+        # No try/except: every attribute this touches exists before the first
+        # install, and the one risky call (backend report) guards itself in
+        # _refresh_idle_activity.
+        if event.type() in {
+            QEvent.Type.MouseButtonPress,
+            QEvent.Type.MouseButtonRelease,
+            QEvent.Type.MouseMove,
+            QEvent.Type.Wheel,
+            QEvent.Type.KeyPress,
+            QEvent.Type.KeyRelease,
+            QEvent.Type.FocusIn,
+            QEvent.Type.TouchBegin,
+            QEvent.Type.TouchUpdate,
+        }:
+            self._mark_user_interaction()
         # getattr defaults: this filter also runs for events delivered while
         # __init__ is still building (or teardown is tearing down) the very
         # widgets it inspects — a raise here spams every event and breaks

src/aare/gui/panels/fluorescence_panel.py

@@ -2,7 +2,7 @@ import numpy as np
 from aarecommon.config.logger import setup_logger
 from aarecommon.models.models import DAQStatusModel, FluorescenceSpectrumOutputModel
 from PySide6.QtCharts import QChart, QChartView, QLineSeries, QValueAxis
-from PySide6.QtCore import QEvent, QPointF, Qt, Slot
+from PySide6.QtCore import QEvent, Qt, Slot
 from PySide6.QtGui import QPainter, QPen
 from PySide6.QtWidgets import QGraphicsSimpleTextItem, QGridLayout, QLabel, QWidget
 
@@ -82,8 +82,7 @@ class FluorescencePanel(QWidget):
     def eventFilter(self, obj, event):
         try:
             if obj is self.chart_view.viewport() and event.type() == QEvent.Type.MouseMove:
-                pos = event.position() if hasattr(event, "position") else event.pos()
-                p = QPointF(pos.x(), pos.y())
+                p = event.position()
                 plot = self.chart.plotArea()
                 if not plot.contains(p) or self.series.count() == 0:
                     self.chart_view.setToolTip("")
@@ -94,19 +93,12 @@ class FluorescencePanel(QWidget):
                     (p.x() - plot.left()) / plot.width()
                 )
 
-                # Snap to the largest Y within +/- 3 indices around nearest index
+                # Snap to the largest Y within +/- 10 indices around nearest index
                 center = self._nearest_index(x_val)
                 n = self.series.count()
                 left = max(0, center - 10)
                 right = min(n - 1, center + 10)
-
-                best_i = left
-                best_y = self.series.at(best_i).y()
-                for i in range(left + 1, right + 1):
-                    yi = self.series.at(i).y()
-                    if yi > best_y:
-                        best_y = yi
-                        best_i = i
+                best_i = max(range(left, right + 1), key=lambda i: self.series.at(i).y())
 
                 pt = self.series.at(best_i)
                 self.chart_view.setToolTip(f"Energy {pt.x():.3f} keV counts {pt.y():.3f}")

src/aare/gui/panels/portrait_mode.py

@@ -154,7 +154,6 @@ class LEDStages(QWidget):
 class PlayPauseButton(QPushButton):
     def __init__(self, parent=None):
         super().__init__(parent)
-        self._hovered = False
         self._running = False
         self.setFixedSize(64, 64)
         self.setMouseTracking(True)
@@ -163,14 +162,6 @@ class PlayPauseButton(QPushButton):
         self._running = running
         self.update()
 
-    def enterEvent(self, event):
-        self._hovered = True
-        self.update()
-
-    def leaveEvent(self, event):
-        self._hovered = False
-        self.update()
-
     def paintEvent(self, event):
         p = QPainter(self)
         p.setRenderHint(QPainter.Antialiasing)
@@ -178,7 +169,10 @@ class PlayPauseButton(QPushButton):
         cx, cy = rect.width() / 2, rect.height() / 2
         r = min(rect.width(), rect.height()) / 2 - 2
 
-        bg_color = qcolor(WHITE) if self._hovered else QColor(ACCENT)
+        # underMouse() instead of enter/leave overrides tracking a _hovered
+        # flag: QPushButton already repaints on hover (WA_Hover), so the
+        # two extra Qt→Python callbacks bought nothing.
+        bg_color = qcolor(WHITE) if self.underMouse() else QColor(ACCENT)
         p.setBrush(bg_color)
         p.setPen(Qt.NoPen)
         p.drawEllipse(QPointF(cx, cy), r, r)

src/aare/gui/widgets/baton_request_dialog.py

@@ -180,6 +180,9 @@ class BatonRequestDialog(QDialog):
         self._timer = QTimer(self)
         self._timer.setInterval(1000)
         self._timer.timeout.connect(self._tick)
+        # finished fires on accept/reject/close alike — replaces the
+        # closeEvent override that existed only to stop this timer.
+        self.finished.connect(self._timer.stop)
         self._timer.start()
 
     def _tick(self):
@@ -221,22 +224,13 @@ class BatonRequestDialog(QDialog):
             self._on_accept()
 
     def _on_accept(self):
-        self._timer.stop()
         self.accepted_signal.emit()
         self.accept()
 
     def _on_refuse(self):
-        self._timer.stop()
         self.refused_signal.emit()
         self.reject()
 
-    def closeEvent(self, event):
-        """Closing the dialog counts as ignoring = auto-accept on timeout."""
-        # Don't emit anything here - let the timeout handle it
-        # or the SSE stream will close the dialog when resolved
-        self._timer.stop()
-        super().closeEvent(event)
-
 
 class BatonPendingDialog(QDialog):
     """
@@ -331,6 +325,9 @@ class BatonPendingDialog(QDialog):
         self._timer = QTimer(self)
         self._timer.setInterval(1000)
         self._timer.timeout.connect(self._tick)
+        # finished fires on accept/reject/close alike — replaces the
+        # closeEvent override that existed only to stop this timer.
+        self.finished.connect(self._timer.stop)
         self._timer.start()
 
     def _tick(self):
@@ -364,10 +361,5 @@ class BatonPendingDialog(QDialog):
         # Keep cancel button so they can abort the wait if they change their mind
 
     def _on_cancel(self):
-        self._timer.stop()
         self.cancelled_signal.emit()
         self.reject()
-
-    def closeEvent(self, event):
-        self._timer.stop()
-        super().closeEvent(event)

src/aare/gui/widgets/camera_image.py

@@ -609,12 +609,6 @@ class SampleCameraImageLabel(QGraphicsView):
         if self._pending_load_pos is not None:
             self.load_image.emit(self._pending_load_pos)
             self._pending_load_pos = None
-            return
-
-        if self._pending_load_pos is not None:
-            self.load_image.emit(self._pending_load_pos)
-            self._pending_load_pos = None
-            self.raster_timer.start(self.raster_timer_interval)
 
     def mouseReleaseEvent(self, event):
         if not self._camera_interaction_enabled():

src/aare/gui/widgets/motor_move_group.py

@@ -86,24 +86,22 @@ class MotorMoveGroup(QObject):
             self._set_state(name, "pending")
 
     def eventFilter(self, obj, event):
-        if event.type() == QEvent.Type.KeyPress and event.key() in (
-            Qt.Key.Key_Return,
-            Qt.Key.Key_Enter,
+        if (
+            event.type() == QEvent.Type.KeyPress
+            and event.key() in (Qt.Key.Key_Return, Qt.Key.Key_Enter)
+            and obj in self._boxes.values()
         ):
-            for box in self._boxes.values():
-                if obj is box:
-                    try:
-                        value = float(box.text())
-                    except ValueError:
-                        break
-                    bottom = box.range_validator.bottom()
-                    if value < bottom:
-                        QToolTip.showText(
-                            box.mapToGlobal(QPoint(0, box.height())),
-                            f"Too small — minimum value: {box.to_string(bottom)}",
-                            box,
-                        )
-                    break
+            try:
+                value = float(obj.text())
+            except ValueError:
+                return super().eventFilter(obj, event)
+            bottom = obj.range_validator.bottom()
+            if value < bottom:
+                QToolTip.showText(
+                    obj.mapToGlobal(QPoint(0, obj.height())),
+                    f"Too small — minimum value: {obj.to_string(bottom)}",
+                    obj,
+                )
         return super().eventFilter(obj, event)
 
     @Slot()

src/aare/gui/widgets/value_label.py

@@ -1,9 +1,9 @@
-from PySide6.QtCore import Qt, Signal
-from PySide6.QtWidgets import QLabel
+from aare.gui.widgets.clickable_label import ClickableLabel
 
 
-class ValueLabel(QLabel):
-    clicked = Signal()
+class ValueLabel(ClickableLabel):
+    # clicked signal + left-click mousePressEvent inherited from
+    # ClickableLabel — this class only adds the "descr: value unit" text.
 
     def __init__(self, text: str, unit: str = "", parent=None):
         super().__init__(parent)
@@ -17,9 +17,3 @@ class ValueLabel(QLabel):
             )
         else:
             self.setText(f"{self._descr}: <b>{s}</b> {self._unit}")
-
-    def mousePressEvent(self, event):
-        if event.button() == Qt.MouseButton.LeftButton:
-            self.clicked.emit()
-        else:
-            super().mousePressEvent(event)

src/aare/gui/widgets/video_image.py

@@ -1,5 +1,5 @@
 from PySide6.QtCore import QRectF, Qt, Slot
-from PySide6.QtGui import QImage, QPainter, QPixmap
+from PySide6.QtGui import QImage, QKeySequence, QPainter, QPixmap, QShortcut
 from PySide6.QtWidgets import QGraphicsPixmapItem, QGraphicsScene, QGraphicsView
 
 from aare.gui.widgets.busy_overlay import BusyOverlayStyle, draw_busy_status_text
@@ -34,6 +34,20 @@ class VideoGraphicsView(QGraphicsView):
 
         self._busy_overlay_style: BusyOverlayStyle | None = None
 
+        # QShortcut instead of a keyPressEvent override: one fewer Qt→Python
+        # callback on the render path, same focus behavior (WidgetShortcut =
+        # active only while the view has focus).
+        for key, slot in (
+            (Qt.Key.Key_F, self.fit_to_view),
+            (Qt.Key.Key_R, self.reset_zoom),
+            (Qt.Key.Key_Plus, self.zoom_in),
+            (Qt.Key.Key_Equal, self.zoom_in),
+            (Qt.Key.Key_Minus, self.zoom_out),
+        ):
+            shortcut = QShortcut(QKeySequence(key), self)
+            shortcut.setContext(Qt.ShortcutContext.WidgetShortcut)
+            shortcut.activated.connect(slot)
+
     @Slot(QImage)
     def update_frame(self, qt_image: QImage):
         """Update the video frame"""
@@ -92,19 +106,6 @@ class VideoGraphicsView(QGraphicsView):
             # Normal scrolling
             super().wheelEvent(event)
 
-    def keyPressEvent(self, event):
-        """Handle keyboard shortcuts"""
-        if event.key() == Qt.Key.Key_F:
-            self.fit_to_view()
-        elif event.key() == Qt.Key.Key_R:
-            self.reset_zoom()
-        elif event.key() == Qt.Key.Key_Plus or event.key() == Qt.Key.Key_Equal:
-            self.zoom_in()
-        elif event.key() == Qt.Key.Key_Minus:
-            self.zoom_out()
-        else:
-            super().keyPressEvent(event)
-
     def drawForeground(self, painter: QPainter, rect: QRectF):
         super().drawForeground(painter, rect)
 

tests/unit/gui/test_qt_override_reduction.py

@@ -0,0 +1,103 @@
+"""Covers the code paths touched by the reduce-overwriting-qt-method
+refactor: Qt-native replacements (signals, shortcuts, underMouse) for
+virtual-method overrides, so the diff-coverage gate sees them executed."""
+
+from PySide6.QtCore import QEvent, QPointF, Qt
+from PySide6.QtGui import QMouseEvent
+
+from aare.gui.panels.fluorescence_panel import FluorescencePanel
+from aare.gui.panels.portrait_mode import PlayPauseButton
+from aare.gui.widgets.baton_request_dialog import BatonPendingDialog, BatonRequestDialog
+from aare.gui.widgets.value_label import ValueLabel
+from aare.gui.widgets.video_image import VideoGraphicsView
+
+
+def test_baton_dialogs_stop_timer_via_finished(qtbot):
+    """finished.connect replaced the closeEvent overrides: the timer must
+    stop on accept, reject AND plain close — the path closeEvent used to
+    handle."""
+    req = BatonRequestDialog("someone")
+    qtbot.addWidget(req)
+    assert req._timer.isActive()
+    req._on_accept()
+    assert not req._timer.isActive()
+
+    req2 = BatonRequestDialog("someone")
+    qtbot.addWidget(req2)
+    req2._on_refuse()
+    assert not req2._timer.isActive()
+
+    # close() only delivers a close event to a SHOWN dialog — same held for
+    # the old closeEvent override, so showing first keeps the test honest.
+    pend = BatonPendingDialog("user")
+    qtbot.addWidget(pend)
+    pend.show()
+    qtbot.waitExposed(pend)
+    assert pend._timer.isActive()
+    pend.close()
+    assert not pend._timer.isActive()
+
+
+def test_fluorescence_hover_snaps_to_peak(qtbot):
+    """Drives a MouseMove through the viewport filter: event.position()
+    (the PyQt5-era hasattr fallback is gone) and the max(range) peak snap."""
+    panel = FluorescencePanel()
+    qtbot.addWidget(panel)
+    panel.resize(500, 400)
+    panel.show()
+    qtbot.waitExposed(panel)
+
+    panel.axis_x.setRange(0.0, 10.0)
+    panel.axis_y.setRange(0.0, 100.0)
+    for i in range(50):
+        panel.series.append(i * 0.2, 90.0 if i == 25 else 10.0)
+
+    plot = panel.chart.plotArea()
+    pos = QPointF(plot.center())
+    ev = QMouseEvent(
+        QEvent.Type.MouseMove,
+        pos,
+        panel.chart_view.viewport().mapToGlobal(pos.toPoint()),
+        Qt.MouseButton.NoButton,
+        Qt.MouseButton.NoButton,
+        Qt.KeyboardModifier.NoModifier,
+    )
+    assert panel.eventFilter(panel.chart_view.viewport(), ev) is False
+    assert "keV" in panel.chart_view.toolTip()
+
+
+def test_video_view_shortcuts_replace_keypress_override(qtbot):
+    view = VideoGraphicsView()
+    qtbot.addWidget(view)
+    view.show()
+    qtbot.waitExposed(view)
+    # WidgetShortcut context needs real focus; offscreen grants it only
+    # after the window is active.
+    view.activateWindow()
+    view.setFocus()
+    qtbot.waitUntil(view.hasFocus, timeout=2000)
+
+    qtbot.keyClick(view, Qt.Key.Key_Plus)
+    assert view.zoom_factor > 1.0
+    qtbot.keyClick(view, Qt.Key.Key_R)
+    assert view.zoom_factor == 1.0
+    qtbot.keyClick(view, Qt.Key.Key_Minus)
+    assert view.zoom_factor < 1.0
+    qtbot.keyClick(view, Qt.Key.Key_F)  # fit_to_view: just must not raise
+
+
+def test_play_pause_button_paints_without_hover_overrides(qtbot):
+    btn = PlayPauseButton()
+    qtbot.addWidget(btn)
+    btn.set_running(True)
+    # grab() forces a real paintEvent pass over the underMouse() branch
+    assert not btn.grab().isNull()
+
+
+def test_value_label_inherits_click(qtbot):
+    label = ValueLabel("Energy", "keV")
+    qtbot.addWidget(label)
+    label.set_value("12.4")
+    assert "12.4" in label.text()
+    with qtbot.waitSignal(label.clicked, timeout=1000):
+        qtbot.mousePress(label, Qt.MouseButton.LeftButton)