From b12975f035b60ef25eea2abac7ed0fe1fe987e8c Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Sun, 17 May 2026 23:32:13 +0300
Subject: [PATCH 01/11] feat: Add centralized error management system
 (pt_error_handler)

ApplicationErrorHandler singleton wraps pt_errors.ErrorHandler with
per-severity notification callbacks, module suppression, and a global
error bus. Module-level shortcuts (handle, on_critical, on_error,
configure_gui_alerts) make adoption drop-in for existing modules.

- 18 unit tests, all passing
---
 app/pt_error_handler.py   | 249 ++++++++++++++++++++++++++++++++++++++
 app/test_error_handler.py | 211 ++++++++++++++++++++++++++++++++
 2 files changed, 460 insertions(+)
 create mode 100644 app/pt_error_handler.py
 create mode 100644 app/test_error_handler.py

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
new file mode 100644
index 00000000..560d8e80
--- /dev/null
+++ b/app/pt_error_handler.py
@@ -0,0 +1,249 @@
+"""
+PowerTraderAI+ Centralized Error Management System
+Single application-wide error handler with notification routing and
+classification. All modules should use `get_handler()` instead of
+creating their own ErrorHandler instances.
+
+Usage:
+    from pt_error_handler import get_handler, on_critical, handle
+
+    # Register a GUI alert callback for critical errors
+    on_critical(lambda report: show_alert(report.user_message))
+
+    # Handle an exception anywhere in the codebase
+    try:
+        risky_operation()
+    except Exception as exc:
+        handle(exc, context={"operation": "risky_operation"})
+
+    # Or via the global handler directly
+    handler = get_handler()
+    handler.handle_error(exc)
+"""
+
+import logging
+import threading
+from typing import Callable, Dict, List, Optional
+
+from pt_errors import (
+    ErrorCategory,
+    ErrorHandler,
+    ErrorReport,
+    ErrorSeverity,
+    PowerTraderError,
+)
+
+logger = logging.getLogger(__name__)
+
+# ---------------------------------------------------------------------------
+# Notification callback type alias
+# ---------------------------------------------------------------------------
+NotificationCallback = Callable[[ErrorReport], None]
+
+
+# ---------------------------------------------------------------------------
+# ApplicationErrorHandler — singleton wrapping ErrorHandler
+# ---------------------------------------------------------------------------
+class ApplicationErrorHandler:
+    """
+    Application-wide singleton error handler.
+
+    Wraps pt_errors.ErrorHandler and adds:
+    - Per-severity notification callbacks (e.g. pop GUI alerts for CRITICAL)
+    - Global error bus: all modules share one instance
+    - Thread-safe callback registration / invocation
+    - Error suppression rules for known noisy errors
+    """
+
+    _instance: Optional["ApplicationErrorHandler"] = None
+    _lock = threading.Lock()
+
+    def __new__(cls) -> "ApplicationErrorHandler":
+        with cls._lock:
+            if cls._instance is None:
+                cls._instance = super().__new__(cls)
+                cls._instance._initialised = False
+            return cls._instance
+
+    def _ensure_init(self) -> None:
+        if self._initialised:
+            return
+        self._handler = ErrorHandler(logger=logging.getLogger("pt.errors"))
+        self._callbacks: Dict[ErrorSeverity, List[NotificationCallback]] = {
+            sev: [] for sev in ErrorSeverity
+        }
+        self._suppressed_modules: set = set()
+        self._cb_lock = threading.Lock()
+        self._initialised = True
+
+    # ------------------------------------------------------------------
+    # Callback registration
+    # ------------------------------------------------------------------
+    def register_callback(
+        self, severity: ErrorSeverity, callback: NotificationCallback
+    ) -> None:
+        """
+        Register a callback fired whenever an error of `severity` is handled.
+        Callbacks must be non-blocking (offload heavy work to a thread).
+        """
+        self._ensure_init()
+        with self._cb_lock:
+            self._callbacks[severity].append(callback)
+
+    def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None:
+        """Clear callbacks for a severity level (or all levels if None)."""
+        self._ensure_init()
+        with self._cb_lock:
+            if severity:
+                self._callbacks[severity].clear()
+            else:
+                for cbs in self._callbacks.values():
+                    cbs.clear()
+
+    # ------------------------------------------------------------------
+    # Suppression
+    # ------------------------------------------------------------------
+    def suppress_module(self, module_name: str) -> None:
+        """Suppress notifications for errors originating from `module_name`."""
+        self._ensure_init()
+        self._suppressed_modules.add(module_name)
+
+    def unsuppress_module(self, module_name: str) -> None:
+        self._ensure_init()
+        self._suppressed_modules.discard(module_name)
+
+    # ------------------------------------------------------------------
+    # Core handle
+    # ------------------------------------------------------------------
+    def handle(
+        self,
+        error: Exception,
+        context: Optional[Dict] = None,
+        reraise: bool = False,
+    ) -> ErrorReport:
+        """
+        Handle an exception: classify, log, fire callbacks.
+
+        Args:
+            error: The exception to handle
+            context: Additional key/value context for the report
+            reraise: If True, re-raise the exception after handling
+
+        Returns:
+            ErrorReport with full classification and metadata
+        """
+        self._ensure_init()
+        report = self._handler.handle_error(error, context=context)
+        self._fire_callbacks(report)
+        if reraise:
+            raise error
+        return report
+
+    def handle_critical(
+        self, error: Exception, context: Optional[Dict] = None
+    ) -> ErrorReport:
+        """Handle and immediately fire all CRITICAL callbacks."""
+        self._ensure_init()
+        report = self._handler.handle_error(error, context=context)
+        # Force severity to CRITICAL for escalation
+        if report.severity != ErrorSeverity.CRITICAL:
+            report.severity = ErrorSeverity.CRITICAL
+        self._fire_callbacks(report)
+        return report
+
+    def _fire_callbacks(self, report: ErrorReport) -> None:
+        self._ensure_init()
+        if report.module in self._suppressed_modules:
+            return
+        with self._cb_lock:
+            callbacks = list(self._callbacks.get(report.severity, []))
+        for cb in callbacks:
+            try:
+                cb(report)
+            except Exception as cb_exc:
+                logger.warning("Error callback raised an exception: %s", cb_exc)
+
+    # ------------------------------------------------------------------
+    # Query / stats
+    # ------------------------------------------------------------------
+    def get_summary(self) -> Dict:
+        self._ensure_init()
+        return self._handler.get_error_summary()
+
+    def get_recent_errors(self, limit: int = 20) -> List[ErrorReport]:
+        self._ensure_init()
+        return self._handler.error_reports[-limit:]
+
+    def get_critical_errors(self) -> List[ErrorReport]:
+        self._ensure_init()
+        return [
+            r for r in self._handler.error_reports
+            if r.severity == ErrorSeverity.CRITICAL
+        ]
+
+    def clear_history(self) -> None:
+        """Clear in-memory error history (does NOT affect logs)."""
+        self._ensure_init()
+        self._handler.error_reports.clear()
+        self._handler.error_counts.clear()
+
+    @classmethod
+    def reset_singleton(cls) -> None:
+        """Reset singleton — for testing only."""
+        with cls._lock:
+            cls._instance = None
+
+
+# ---------------------------------------------------------------------------
+# Module-level convenience API
+# ---------------------------------------------------------------------------
+def get_handler() -> ApplicationErrorHandler:
+    """Return the application-wide singleton error handler."""
+    h = ApplicationErrorHandler()
+    h._ensure_init()
+    return h
+
+
+def handle(
+    error: Exception,
+    context: Optional[Dict] = None,
+    reraise: bool = False,
+) -> ErrorReport:
+    """Module-level shortcut: handle(exc) from anywhere."""
+    return get_handler().handle(error, context=context, reraise=reraise)
+
+
+def on_critical(callback: NotificationCallback) -> None:
+    """Register a callback that fires on every CRITICAL error."""
+    get_handler().register_callback(ErrorSeverity.CRITICAL, callback)
+
+
+def on_error(callback: NotificationCallback) -> None:
+    """Register a callback that fires on HIGH severity errors."""
+    get_handler().register_callback(ErrorSeverity.HIGH, callback)
+
+
+def on_warning(callback: NotificationCallback) -> None:
+    """Register a callback that fires on MEDIUM severity errors."""
+    get_handler().register_callback(ErrorSeverity.MEDIUM, callback)
+
+
+def configure_gui_alerts(
+    critical_callback: Optional[NotificationCallback] = None,
+    error_callback: Optional[NotificationCallback] = None,
+) -> None:
+    """
+    Convenience: register GUI alert callbacks for CRITICAL and HIGH errors.
+    Call once during application startup.
+
+    Example:
+        configure_gui_alerts(
+            critical_callback=lambda r: messagebox.showerror("Critical", r.user_message),
+            error_callback=lambda r: messagebox.showwarning("Error", r.user_message),
+        )
+    """
+    handler = get_handler()
+    if critical_callback:
+        handler.register_callback(ErrorSeverity.CRITICAL, critical_callback)
+    if error_callback:
+        handler.register_callback(ErrorSeverity.HIGH, error_callback)
diff --git a/app/test_error_handler.py b/app/test_error_handler.py
new file mode 100644
index 00000000..30ee0227
--- /dev/null
+++ b/app/test_error_handler.py
@@ -0,0 +1,211 @@
+"""Tests for pt_error_handler centralized error management system."""
+
+import unittest
+from unittest.mock import MagicMock
+
+from pt_error_handler import (
+    ApplicationErrorHandler,
+    configure_gui_alerts,
+    get_handler,
+    handle,
+    on_critical,
+    on_error,
+    on_warning,
+)
+from pt_errors import ErrorCategory, ErrorSeverity, TradingError
+
+
+class TestApplicationErrorHandlerSingleton(unittest.TestCase):
+
+    def setUp(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_singleton_same_instance(self):
+        h1 = get_handler()
+        h2 = get_handler()
+        self.assertIs(h1, h2)
+
+    def test_initialises_on_first_call(self):
+        h = get_handler()
+        self.assertTrue(h._initialised)
+
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+
+class TestErrorHandling(unittest.TestCase):
+
+    def setUp(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_handle_returns_report(self):
+        try:
+            raise ValueError("test error")
+        except ValueError as exc:
+            report = handle(exc)
+        self.assertIsNotNone(report)
+        self.assertEqual(report.exception_type, "ValueError")
+
+    def test_handle_with_context(self):
+        try:
+            raise RuntimeError("ctx test")
+        except RuntimeError as exc:
+            report = handle(exc, context={"operation": "test_op"})
+        self.assertEqual(report.context.get("operation"), "test_op")
+
+    def test_handle_reraise(self):
+        with self.assertRaises(ValueError):
+            try:
+                raise ValueError("reraise me")
+            except ValueError as exc:
+                handle(exc, reraise=True)
+
+    def test_handle_critical_forces_severity(self):
+        handler = get_handler()
+        try:
+            raise ValueError("low but critical context")
+        except ValueError as exc:
+            report = handler.handle_critical(exc)
+        self.assertEqual(report.severity, ErrorSeverity.CRITICAL)
+
+    def test_powertrader_error_preserved_severity(self):
+        try:
+            raise TradingError("market closed", severity=ErrorSeverity.HIGH)
+        except TradingError as exc:
+            report = handle(exc)
+        self.assertEqual(report.severity, ErrorSeverity.HIGH)
+        self.assertEqual(report.category, ErrorCategory.TRADING_ERROR)
+
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+
+class TestCallbacks(unittest.TestCase):
+
+    def setUp(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_critical_callback_fired(self):
+        cb = MagicMock()
+        on_critical(cb)
+        try:
+            raise RuntimeError("critical!")
+        except RuntimeError as exc:
+            get_handler().handle_critical(exc)
+        cb.assert_called_once()
+
+    def test_callback_receives_report(self):
+        received = []
+        on_error(lambda r: received.append(r))
+        try:
+            raise TradingError("order failed", severity=ErrorSeverity.HIGH)
+        except TradingError as exc:
+            handle(exc)
+        self.assertEqual(len(received), 1)
+        self.assertEqual(received[0].exception_type, "TradingError")
+
+    def test_warning_callback_fired(self):
+        cb = MagicMock()
+        on_warning(cb)
+        try:
+            raise ValueError("medium issue")
+        except ValueError as exc:
+            # ValueError → MEDIUM severity via classification
+            handle(exc)
+        # May or may not fire depending on classification — just ensure no crash
+        # and callback is registered
+        self.assertTrue(True)
+
+    def test_callback_exception_does_not_propagate(self):
+        def bad_cb(report):
+            raise RuntimeError("callback exploded")
+        on_critical(bad_cb)
+        try:
+            get_handler().handle_critical(ValueError("test"))
+        except RuntimeError:
+            self.fail("Callback exception should not propagate")
+
+    def test_unregister_all(self):
+        cb = MagicMock()
+        on_critical(cb)
+        get_handler().unregister_all(ErrorSeverity.CRITICAL)
+        get_handler().handle_critical(ValueError("test"))
+        cb.assert_not_called()
+
+    def test_suppress_module(self):
+        cb = MagicMock()
+        on_critical(cb)
+        get_handler().suppress_module("pt_trader")
+        try:
+            raise ValueError("suppressed")
+        except ValueError as exc:
+            report = get_handler().handle_critical(exc)
+            # Manually override module for test
+            report.module = "pt_trader"
+        # Callback already fired before we changed module — just test no crash
+        self.assertTrue(True)
+
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+
+class TestQueryAPI(unittest.TestCase):
+
+    def setUp(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_get_summary_empty(self):
+        summary = get_handler().get_summary()
+        self.assertEqual(summary["total_errors"], 0)
+
+    def test_get_recent_errors(self):
+        for i in range(5):
+            try:
+                raise ValueError(f"error {i}")
+            except ValueError as exc:
+                handle(exc)
+        recent = get_handler().get_recent_errors(limit=3)
+        self.assertEqual(len(recent), 3)
+
+    def test_get_critical_errors_filtered(self):
+        try:
+            raise ValueError("normal")
+        except ValueError as exc:
+            handle(exc)
+        try:
+            raise TradingError("critical trade fail")
+        except TradingError as exc:
+            get_handler().handle_critical(exc)
+        criticals = get_handler().get_critical_errors()
+        self.assertEqual(len(criticals), 1)
+
+    def test_clear_history(self):
+        try:
+            raise ValueError("to clear")
+        except ValueError as exc:
+            handle(exc)
+        get_handler().clear_history()
+        self.assertEqual(len(get_handler().get_recent_errors()), 0)
+
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+
+class TestConfigureGuiAlerts(unittest.TestCase):
+
+    def setUp(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_configure_gui_alerts_registers_callbacks(self):
+        critical_cb = MagicMock()
+        error_cb = MagicMock()
+        configure_gui_alerts(critical_callback=critical_cb, error_callback=error_cb)
+        get_handler().handle_critical(ValueError("gui test"))
+        critical_cb.assert_called_once()
+
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+
+if __name__ == "__main__":
+    unittest.main()

From 0c2d1c3567ce2c2a62c17b687747ec4c988253f2 Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Sun, 17 May 2026 23:42:34 +0300
Subject: [PATCH 02/11] fix: Address Copilot review - error handler thread
 safety and test coverage

- _ensure_init uses double-checked locking (class-level _init_lock) to be thread-safe
- handle_critical updates error_counts for CRITICAL so get_critical_errors is consistent
- ErrorCategory/PowerTraderError unused imports removed
- configure_gui_alerts uses 'is not None' instead of truthiness check for callbacks
- Callback exceptions now logged with exc_info=True for full traceback
- Tests: non-assertive tests replaced with deterministic assertions
- Tests: suppress_module test patches handle_error to set module before _fire_callbacks
- Tests: added warning callback, unregister_all, gui alerts error-level tests
---
 app/pt_error_handler.py   |  68 ++++++++++++++---------
 app/test_error_handler.py | 111 +++++++++++++++++++++++++++-----------
 2 files changed, 124 insertions(+), 55 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index 560d8e80..eda29391 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -26,11 +26,9 @@
 from typing import Callable, Dict, List, Optional
 
 from pt_errors import (
-    ErrorCategory,
     ErrorHandler,
     ErrorReport,
     ErrorSeverity,
-    PowerTraderError,
 )
 
 logger = logging.getLogger(__name__)
@@ -51,30 +49,35 @@ class ApplicationErrorHandler:
     Wraps pt_errors.ErrorHandler and adds:
     - Per-severity notification callbacks (e.g. pop GUI alerts for CRITICAL)
     - Global error bus: all modules share one instance
-    - Thread-safe callback registration / invocation
-    - Error suppression rules for known noisy errors
+    - Thread-safe init, callback registration, and invocation
+    - Error suppression rules for known noisy modules
     """
 
     _instance: Optional["ApplicationErrorHandler"] = None
-    _lock = threading.Lock()
+    _class_lock = threading.Lock()     # Guards singleton creation
+    _init_lock = threading.Lock()      # Guards lazy initialisation
 
     def __new__(cls) -> "ApplicationErrorHandler":
-        with cls._lock:
+        with cls._class_lock:
             if cls._instance is None:
                 cls._instance = super().__new__(cls)
                 cls._instance._initialised = False
             return cls._instance
 
     def _ensure_init(self) -> None:
+        """Thread-safe double-checked lazy initialisation."""
         if self._initialised:
             return
-        self._handler = ErrorHandler(logger=logging.getLogger("pt.errors"))
-        self._callbacks: Dict[ErrorSeverity, List[NotificationCallback]] = {
-            sev: [] for sev in ErrorSeverity
-        }
-        self._suppressed_modules: set = set()
-        self._cb_lock = threading.Lock()
-        self._initialised = True
+        with self._init_lock:
+            if self._initialised:   # re-check after acquiring lock
+                return
+            self._handler = ErrorHandler(logger=logging.getLogger("pt.errors"))
+            self._callbacks: Dict[ErrorSeverity, List[NotificationCallback]] = {
+                sev: [] for sev in ErrorSeverity
+            }
+            self._suppressed_modules: set = set()
+            self._cb_lock = threading.Lock()
+            self._initialised = True
 
     # ------------------------------------------------------------------
     # Callback registration
@@ -91,10 +94,10 @@ def register_callback(
             self._callbacks[severity].append(callback)
 
     def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None:
-        """Clear callbacks for a severity level (or all levels if None)."""
+        """Clear callbacks for a severity level, or all levels if severity is None."""
         self._ensure_init()
         with self._cb_lock:
-            if severity:
+            if severity is not None:
                 self._callbacks[severity].clear()
             else:
                 for cbs in self._callbacks.values():
@@ -104,7 +107,7 @@ def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None:
     # Suppression
     # ------------------------------------------------------------------
     def suppress_module(self, module_name: str) -> None:
-        """Suppress notifications for errors originating from `module_name`."""
+        """Suppress notification callbacks for errors from `module_name`."""
         self._ensure_init()
         self._suppressed_modules.add(module_name)
 
@@ -142,12 +145,26 @@ def handle(
     def handle_critical(
         self, error: Exception, context: Optional[Dict] = None
     ) -> ErrorReport:
-        """Handle and immediately fire all CRITICAL callbacks."""
+        """
+        Handle an exception and escalate to CRITICAL severity regardless of
+        automatic classification. Fires CRITICAL-level callbacks only.
+
+        The underlying ErrorHandler classifies and logs the error first; the
+        severity field on the returned ErrorReport is then set to CRITICAL so
+        that `get_critical_errors()` and the CRITICAL callback list both see
+        the escalated severity consistently.
+        """
         self._ensure_init()
         report = self._handler.handle_error(error, context=context)
-        # Force severity to CRITICAL for escalation
+
         if report.severity != ErrorSeverity.CRITICAL:
+            # Update the stored report so stats and `get_critical_errors()` agree
             report.severity = ErrorSeverity.CRITICAL
+            # Also update the in-place error_counts to include CRITICAL
+            self._handler.error_counts[ErrorSeverity.CRITICAL.value] = (
+                self._handler.error_counts.get(ErrorSeverity.CRITICAL.value, 0) + 1
+            )
+
         self._fire_callbacks(report)
         return report
 
@@ -160,8 +177,9 @@ def _fire_callbacks(self, report: ErrorReport) -> None:
         for cb in callbacks:
             try:
                 cb(report)
-            except Exception as cb_exc:
-                logger.warning("Error callback raised an exception: %s", cb_exc)
+            except Exception:
+                # Log full traceback so callback bugs are diagnosable
+                logger.warning("Error callback raised an exception", exc_info=True)
 
     # ------------------------------------------------------------------
     # Query / stats
@@ -182,15 +200,15 @@ def get_critical_errors(self) -> List[ErrorReport]:
         ]
 
     def clear_history(self) -> None:
-        """Clear in-memory error history (does NOT affect logs)."""
+        """Clear in-memory error history (does NOT affect log files)."""
         self._ensure_init()
         self._handler.error_reports.clear()
         self._handler.error_counts.clear()
 
     @classmethod
     def reset_singleton(cls) -> None:
-        """Reset singleton — for testing only."""
-        with cls._lock:
+        """Destroy singleton and force re-init on next call. For testing only."""
+        with cls._class_lock:
             cls._instance = None
 
 
@@ -243,7 +261,7 @@ def configure_gui_alerts(
         )
     """
     handler = get_handler()
-    if critical_callback:
+    if critical_callback is not None:
         handler.register_callback(ErrorSeverity.CRITICAL, critical_callback)
-    if error_callback:
+    if error_callback is not None:
         handler.register_callback(ErrorSeverity.HIGH, error_callback)
diff --git a/app/test_error_handler.py b/app/test_error_handler.py
index 30ee0227..bbe389b4 100644
--- a/app/test_error_handler.py
+++ b/app/test_error_handler.py
@@ -12,7 +12,7 @@
     on_error,
     on_warning,
 )
-from pt_errors import ErrorCategory, ErrorSeverity, TradingError
+from pt_errors import ErrorSeverity, TradingError, ErrorCategory
 
 
 class TestApplicationErrorHandlerSingleton(unittest.TestCase):
@@ -20,6 +20,9 @@ class TestApplicationErrorHandlerSingleton(unittest.TestCase):
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
     def test_singleton_same_instance(self):
         h1 = get_handler()
         h2 = get_handler()
@@ -29,15 +32,15 @@ def test_initialises_on_first_call(self):
         h = get_handler()
         self.assertTrue(h._initialised)
 
-    def tearDown(self):
-        ApplicationErrorHandler.reset_singleton()
-
 
 class TestErrorHandling(unittest.TestCase):
 
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
     def test_handle_returns_report(self):
         try:
             raise ValueError("test error")
@@ -68,6 +71,17 @@ def test_handle_critical_forces_severity(self):
             report = handler.handle_critical(exc)
         self.assertEqual(report.severity, ErrorSeverity.CRITICAL)
 
+    def test_handle_critical_updates_error_counts(self):
+        """get_critical_errors must see the escalated severity."""
+        handler = get_handler()
+        try:
+            raise ValueError("escalated")
+        except ValueError as exc:
+            handler.handle_critical(exc)
+        criticals = handler.get_critical_errors()
+        self.assertEqual(len(criticals), 1)
+        self.assertEqual(criticals[0].severity, ErrorSeverity.CRITICAL)
+
     def test_powertrader_error_preserved_severity(self):
         try:
             raise TradingError("market closed", severity=ErrorSeverity.HIGH)
@@ -76,15 +90,15 @@ def test_powertrader_error_preserved_severity(self):
         self.assertEqual(report.severity, ErrorSeverity.HIGH)
         self.assertEqual(report.category, ErrorCategory.TRADING_ERROR)
 
-    def tearDown(self):
-        ApplicationErrorHandler.reset_singleton()
-
 
 class TestCallbacks(unittest.TestCase):
 
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
     def test_critical_callback_fired(self):
         cb = MagicMock()
         on_critical(cb)
@@ -104,17 +118,16 @@ def test_callback_receives_report(self):
         self.assertEqual(len(received), 1)
         self.assertEqual(received[0].exception_type, "TradingError")
 
-    def test_warning_callback_fired(self):
+    def test_warning_callback_fired_for_medium_error(self):
+        """MEDIUM-severity TradingError must trigger on_warning callbacks."""
         cb = MagicMock()
         on_warning(cb)
         try:
-            raise ValueError("medium issue")
-        except ValueError as exc:
-            # ValueError → MEDIUM severity via classification
+            raise TradingError("minor issue", severity=ErrorSeverity.MEDIUM)
+        except TradingError as exc:
             handle(exc)
-        # May or may not fire depending on classification — just ensure no crash
-        # and callback is registered
-        self.assertTrue(True)
+        cb.assert_called_once()
+        self.assertEqual(cb.call_args[0][0].severity, ErrorSeverity.MEDIUM)
 
     def test_callback_exception_does_not_propagate(self):
         def bad_cb(report):
@@ -125,28 +138,52 @@ def bad_cb(report):
         except RuntimeError:
             self.fail("Callback exception should not propagate")
 
-    def test_unregister_all(self):
+    def test_unregister_all_specific_severity(self):
         cb = MagicMock()
         on_critical(cb)
         get_handler().unregister_all(ErrorSeverity.CRITICAL)
         get_handler().handle_critical(ValueError("test"))
         cb.assert_not_called()
 
-    def test_suppress_module(self):
+    def test_unregister_all_clears_every_severity(self):
+        cb_crit = MagicMock()
+        cb_high = MagicMock()
+        on_critical(cb_crit)
+        on_error(cb_high)
+        get_handler().unregister_all()   # no argument → all
+        get_handler().handle_critical(ValueError("x"))
+        try:
+            raise TradingError("y", severity=ErrorSeverity.HIGH)
+        except TradingError as exc:
+            handle(exc)
+        cb_crit.assert_not_called()
+        cb_high.assert_not_called()
+
+    def test_suppress_module_blocks_callbacks(self):
+        """Callbacks must NOT fire when the error originates from a suppressed module."""
         cb = MagicMock()
         on_critical(cb)
         get_handler().suppress_module("pt_trader")
+
+        # Build an exception that looks like it came from pt_trader:
+        # ErrorHandler captures the caller's frame, so we monkeypatch
+        # the report module after handle_error but before _fire_callbacks
+        # by wrapping _handler.handle_error.
+        original_handle_error = get_handler()._handler.handle_error
+
+        def patched_handle_error(error, context=None):
+            report = original_handle_error(error, context=context)
+            report.module = "pt_trader"   # simulate origin in suppressed module
+            return report
+
+        get_handler()._handler.handle_error = patched_handle_error
+
         try:
-            raise ValueError("suppressed")
+            raise ValueError("from suppressed module")
         except ValueError as exc:
-            report = get_handler().handle_critical(exc)
-            # Manually override module for test
-            report.module = "pt_trader"
-        # Callback already fired before we changed module — just test no crash
-        self.assertTrue(True)
+            get_handler().handle_critical(exc)
 
-    def tearDown(self):
-        ApplicationErrorHandler.reset_singleton()
+        cb.assert_not_called()
 
 
 class TestQueryAPI(unittest.TestCase):
@@ -154,6 +191,9 @@ class TestQueryAPI(unittest.TestCase):
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
     def test_get_summary_empty(self):
         summary = get_handler().get_summary()
         self.assertEqual(summary["total_errors"], 0)
@@ -187,24 +227,35 @@ def test_clear_history(self):
         get_handler().clear_history()
         self.assertEqual(len(get_handler().get_recent_errors()), 0)
 
-    def tearDown(self):
-        ApplicationErrorHandler.reset_singleton()
-
 
 class TestConfigureGuiAlerts(unittest.TestCase):
 
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
-    def test_configure_gui_alerts_registers_callbacks(self):
+    def tearDown(self):
+        ApplicationErrorHandler.reset_singleton()
+
+    def test_configure_gui_alerts_registers_critical(self):
         critical_cb = MagicMock()
         error_cb = MagicMock()
         configure_gui_alerts(critical_callback=critical_cb, error_callback=error_cb)
         get_handler().handle_critical(ValueError("gui test"))
         critical_cb.assert_called_once()
+        error_cb.assert_not_called()
 
-    def tearDown(self):
-        ApplicationErrorHandler.reset_singleton()
+    def test_configure_gui_alerts_registers_error(self):
+        error_cb = MagicMock()
+        configure_gui_alerts(error_callback=error_cb)
+        try:
+            raise TradingError("high err", severity=ErrorSeverity.HIGH)
+        except TradingError as exc:
+            handle(exc)
+        error_cb.assert_called_once()
+
+    def test_configure_gui_alerts_none_callbacks_safe(self):
+        """Passing None should not raise."""
+        configure_gui_alerts(critical_callback=None, error_callback=None)
 
 
 if __name__ == "__main__":

From 337b55a62f7dfd84eba0f010bdeb3e710e87c480 Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Mon, 18 May 2026 03:57:03 +0300
Subject: [PATCH 03/11] fix: Black formatting

---
 app/pt_error_handler.py   |  9 +++++----
 app/test_error_handler.py | 10 +++-------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index eda29391..56ff3173 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -54,8 +54,8 @@ class ApplicationErrorHandler:
     """
 
     _instance: Optional["ApplicationErrorHandler"] = None
-    _class_lock = threading.Lock()     # Guards singleton creation
-    _init_lock = threading.Lock()      # Guards lazy initialisation
+    _class_lock = threading.Lock()  # Guards singleton creation
+    _init_lock = threading.Lock()  # Guards lazy initialisation
 
     def __new__(cls) -> "ApplicationErrorHandler":
         with cls._class_lock:
@@ -69,7 +69,7 @@ def _ensure_init(self) -> None:
         if self._initialised:
             return
         with self._init_lock:
-            if self._initialised:   # re-check after acquiring lock
+            if self._initialised:  # re-check after acquiring lock
                 return
             self._handler = ErrorHandler(logger=logging.getLogger("pt.errors"))
             self._callbacks: Dict[ErrorSeverity, List[NotificationCallback]] = {
@@ -195,7 +195,8 @@ def get_recent_errors(self, limit: int = 20) -> List[ErrorReport]:
     def get_critical_errors(self) -> List[ErrorReport]:
         self._ensure_init()
         return [
-            r for r in self._handler.error_reports
+            r
+            for r in self._handler.error_reports
             if r.severity == ErrorSeverity.CRITICAL
         ]
 
diff --git a/app/test_error_handler.py b/app/test_error_handler.py
index bbe389b4..24d75df6 100644
--- a/app/test_error_handler.py
+++ b/app/test_error_handler.py
@@ -16,7 +16,6 @@
 
 
 class TestApplicationErrorHandlerSingleton(unittest.TestCase):
-
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
@@ -34,7 +33,6 @@ def test_initialises_on_first_call(self):
 
 
 class TestErrorHandling(unittest.TestCase):
-
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
@@ -92,7 +90,6 @@ def test_powertrader_error_preserved_severity(self):
 
 
 class TestCallbacks(unittest.TestCase):
-
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
@@ -132,6 +129,7 @@ def test_warning_callback_fired_for_medium_error(self):
     def test_callback_exception_does_not_propagate(self):
         def bad_cb(report):
             raise RuntimeError("callback exploded")
+
         on_critical(bad_cb)
         try:
             get_handler().handle_critical(ValueError("test"))
@@ -150,7 +148,7 @@ def test_unregister_all_clears_every_severity(self):
         cb_high = MagicMock()
         on_critical(cb_crit)
         on_error(cb_high)
-        get_handler().unregister_all()   # no argument → all
+        get_handler().unregister_all()  # no argument → all
         get_handler().handle_critical(ValueError("x"))
         try:
             raise TradingError("y", severity=ErrorSeverity.HIGH)
@@ -173,7 +171,7 @@ def test_suppress_module_blocks_callbacks(self):
 
         def patched_handle_error(error, context=None):
             report = original_handle_error(error, context=context)
-            report.module = "pt_trader"   # simulate origin in suppressed module
+            report.module = "pt_trader"  # simulate origin in suppressed module
             return report
 
         get_handler()._handler.handle_error = patched_handle_error
@@ -187,7 +185,6 @@ def patched_handle_error(error, context=None):
 
 
 class TestQueryAPI(unittest.TestCase):
-
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 
@@ -229,7 +226,6 @@ def test_clear_history(self):
 
 
 class TestConfigureGuiAlerts(unittest.TestCase):
-
     def setUp(self):
         ApplicationErrorHandler.reset_singleton()
 

From 4a1dba4a585512cfec828949679cbbbbfe39193d Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Tue, 19 May 2026 11:45:49 +0300
Subject: [PATCH 04/11] fix: address Copilot round-2 review on centralized
 error management

- Fix module docstring example: handler.handle_error() -> handler.handle()
- handle(reraise=True): use sys.exc_info() to detect if error is the
  active exception; bare raise preserves traceback exactly when called
  from an except block, fallback to raise error otherwise
- handle_critical: decrement original severity count before incrementing
  CRITICAL to prevent double-counting in error_counts/get_summary()
- _suppressed_modules: protect add/discard/read with _cb_lock so
  concurrent suppress_module/unsuppress_module/_fire_callbacks cannot race
---
 app/pt_error_handler.py | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index 56ff3173..96ea9086 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -18,10 +18,11 @@
 
     # Or via the global handler directly
     handler = get_handler()
-    handler.handle_error(exc)
+    handler.handle(exc)
 """
 
 import logging
+import sys
 import threading
 from typing import Callable, Dict, List, Optional
 
@@ -109,11 +110,13 @@ def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None:
     def suppress_module(self, module_name: str) -> None:
         """Suppress notification callbacks for errors from `module_name`."""
         self._ensure_init()
-        self._suppressed_modules.add(module_name)
+        with self._cb_lock:
+            self._suppressed_modules.add(module_name)
 
     def unsuppress_module(self, module_name: str) -> None:
         self._ensure_init()
-        self._suppressed_modules.discard(module_name)
+        with self._cb_lock:
+            self._suppressed_modules.discard(module_name)
 
     # ------------------------------------------------------------------
     # Core handle
@@ -139,6 +142,13 @@ def handle(
         report = self._handler.handle_error(error, context=context)
         self._fire_callbacks(report)
         if reraise:
+            # If error is the currently active exception (called from inside an except
+            # block), bare raise preserves the original traceback exactly.
+            # Otherwise fall back to raise error which uses the traceback already
+            # attached to the exception object.
+            _, active_exc, _ = sys.exc_info()
+            if active_exc is error:
+                raise
             raise error
         return report
 
@@ -158,9 +168,13 @@ def handle_critical(
         report = self._handler.handle_error(error, context=context)
 
         if report.severity != ErrorSeverity.CRITICAL:
-            # Update the stored report so stats and `get_critical_errors()` agree
+            original_severity = report.severity
+            # Update the stored report so stats and get_critical_errors() agree
             report.severity = ErrorSeverity.CRITICAL
-            # Also update the in-place error_counts to include CRITICAL
+            # Move the count from original severity to CRITICAL - no double-counting
+            orig_key = original_severity.value
+            if self._handler.error_counts.get(orig_key, 0) > 0:
+                self._handler.error_counts[orig_key] -= 1
             self._handler.error_counts[ErrorSeverity.CRITICAL.value] = (
                 self._handler.error_counts.get(ErrorSeverity.CRITICAL.value, 0) + 1
             )
@@ -170,9 +184,9 @@ def handle_critical(
 
     def _fire_callbacks(self, report: ErrorReport) -> None:
         self._ensure_init()
-        if report.module in self._suppressed_modules:
-            return
         with self._cb_lock:
+            if report.module in self._suppressed_modules:
+                return
             callbacks = list(self._callbacks.get(report.severity, []))
         for cb in callbacks:
             try:

From ad8bbfdc636b5eacdd75a061261f75c4f6b07e4e Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Thu, 21 May 2026 11:53:37 +0300
Subject: [PATCH 05/11] fix: address Copilot/owner round-3 review on error
 handling

- pt_errors.ErrorHandler.handle_error: new severity_override parameter
  applied BEFORE logging so escalated errors are recorded at the final
  severity from the start, not at the original severity followed by a
  post-hoc mutation.
- pt_errors.ErrorHandler.error_counts: changed to nested
  Dict[category, Dict[severity, int]] so callers can ask
  "how many TRADING_ERROR/HIGH events" without scanning error_reports.
  Added ErrorHandler.category_total() helper for flat totals.
- get_error_summary: now derives the flat 'categories' / 'severities'
  views from the nested counts so a single structure drives every
  reporting surface; also exposes 'by_category_severity' for callers
  that want the nested breakdown.
- ApplicationErrorHandler.handle_critical: simplified to call
  handle_error(..., severity_override=CRITICAL); removes the manual
  error_counts arithmetic and the post-handle severity mutation.
- ApplicationErrorHandler suppression: normalize both stored entries
  and ErrorReport.module by stripping a trailing '.py' so callers can
  pass either 'pt_trader' or 'pt_trader.py' and either form matches
  the filename that ErrorHandler captures from the traceback.
- test_error_handler: replace the bare monkeypatch in
  test_suppress_module_blocks_callbacks with patch.object +
  addCleanup so the original method is always restored. Added
  test_suppress_module_accepts_filename_and_module_name.
---
 app/pt_error_handler.py   | 57 +++++++++++++++----------
 app/pt_errors.py          | 87 +++++++++++++++++++++++++++++----------
 app/test_error_handler.py | 41 +++++++++++++-----
 3 files changed, 130 insertions(+), 55 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index 96ea9086..887cf697 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -108,15 +108,21 @@ def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None:
     # Suppression
     # ------------------------------------------------------------------
     def suppress_module(self, module_name: str) -> None:
-        """Suppress notification callbacks for errors from `module_name`."""
+        """Suppress notification callbacks for errors originating from
+        ``module_name``.
+
+        Pass the bare module name (``"pt_trader"``), not the filename —
+        :class:`ErrorReport` stores the filename (``"pt_trader.py"``) but
+        suppression normalizes both sides so either spelling works.
+        """
         self._ensure_init()
         with self._cb_lock:
-            self._suppressed_modules.add(module_name)
+            self._suppressed_modules.add(self._normalize_module(module_name))
 
     def unsuppress_module(self, module_name: str) -> None:
         self._ensure_init()
         with self._cb_lock:
-            self._suppressed_modules.discard(module_name)
+            self._suppressed_modules.discard(self._normalize_module(module_name))
 
     # ------------------------------------------------------------------
     # Core handle
@@ -159,33 +165,25 @@ def handle_critical(
         Handle an exception and escalate to CRITICAL severity regardless of
         automatic classification. Fires CRITICAL-level callbacks only.
 
-        The underlying ErrorHandler classifies and logs the error first; the
-        severity field on the returned ErrorReport is then set to CRITICAL so
-        that `get_critical_errors()` and the CRITICAL callback list both see
-        the escalated severity consistently.
+        Uses ``ErrorHandler.handle_error(..., severity_override=CRITICAL)`` so
+        the underlying logger records the error at CRITICAL from the start —
+        no post-hoc severity mutation, no double-counting in
+        ``error_counts``, and ``get_critical_errors()`` sees the escalated
+        severity consistently.
         """
         self._ensure_init()
-        report = self._handler.handle_error(error, context=context)
-
-        if report.severity != ErrorSeverity.CRITICAL:
-            original_severity = report.severity
-            # Update the stored report so stats and get_critical_errors() agree
-            report.severity = ErrorSeverity.CRITICAL
-            # Move the count from original severity to CRITICAL - no double-counting
-            orig_key = original_severity.value
-            if self._handler.error_counts.get(orig_key, 0) > 0:
-                self._handler.error_counts[orig_key] -= 1
-            self._handler.error_counts[ErrorSeverity.CRITICAL.value] = (
-                self._handler.error_counts.get(ErrorSeverity.CRITICAL.value, 0) + 1
-            )
-
+        report = self._handler.handle_error(
+            error,
+            context=context,
+            severity_override=ErrorSeverity.CRITICAL,
+        )
         self._fire_callbacks(report)
         return report
 
     def _fire_callbacks(self, report: ErrorReport) -> None:
         self._ensure_init()
         with self._cb_lock:
-            if report.module in self._suppressed_modules:
+            if self._is_module_suppressed(report.module):
                 return
             callbacks = list(self._callbacks.get(report.severity, []))
         for cb in callbacks:
@@ -195,6 +193,21 @@ def _fire_callbacks(self, report: ErrorReport) -> None:
                 # Log full traceback so callback bugs are diagnosable
                 logger.warning("Error callback raised an exception", exc_info=True)
 
+    @staticmethod
+    def _normalize_module(name: Optional[str]) -> Optional[str]:
+        """Strip a trailing ``.py`` so callers can pass either the module
+        name (``pt_trader``) or the filename (``pt_trader.py``) when
+        registering suppression — :class:`ErrorReport` carries the filename
+        but a module name reads more naturally in code that calls
+        :meth:`suppress_module`."""
+        if not name:
+            return name
+        return name[:-3] if name.endswith(".py") else name
+
+    def _is_module_suppressed(self, module: Optional[str]) -> bool:
+        normalized = self._normalize_module(module)
+        return normalized is not None and normalized in self._suppressed_modules
+
     # ------------------------------------------------------------------
     # Query / stats
     # ------------------------------------------------------------------
diff --git a/app/pt_errors.py b/app/pt_errors.py
index d57e19a2..c2626df1 100644
--- a/app/pt_errors.py
+++ b/app/pt_errors.py
@@ -205,7 +205,8 @@ class ErrorHandler:
     def __init__(self, logger: Optional[logging.Logger] = None):
         self.logger = logger or logging.getLogger(__name__)
         self.error_reports: List[ErrorReport] = []
-        self.error_counts: Dict[str, int] = {}
+        # Nested counters: category.value -> severity.value -> count.
+        self.error_counts: Dict[str, Dict[str, int]] = {}
 
         # Recovery suggestions for common errors
         self.recovery_suggestions = {
@@ -226,7 +227,10 @@ def generate_error_id(self) -> str:
         return f"PT_{datetime.now().strftime('%Y%m%d_%H%M%S')}_{str(uuid.uuid4())[:8]}"
 
     def handle_error(
-        self, error: Exception, context: Dict[str, Any] = None
+        self,
+        error: Exception,
+        context: Dict[str, Any] = None,
+        severity_override: Optional[ErrorSeverity] = None,
     ) -> ErrorReport:
         """
         Handle and report an error with comprehensive logging.
@@ -234,6 +238,12 @@ def handle_error(
         Args:
             error: The exception that occurred
             context: Additional context information
+            severity_override: If provided, replaces the auto-classified or
+                PowerTraderError-declared severity *before* logging and counter
+                updates. This lets escalation paths (e.g.
+                ``ApplicationErrorHandler.handle_critical``) ensure the error
+                is logged at the escalated level instead of at the original
+                level followed by a post-hoc mutation.
 
         Returns:
             ErrorReport: Detailed error report
@@ -258,6 +268,11 @@ def handle_error(
             severity = self._determine_severity(error, category)
             error_context = context or {}
 
+        # Apply caller override BEFORE logging/counting so logs and stats
+        # both reflect the final severity from the start.
+        if severity_override is not None:
+            severity = severity_override
+
         # Create error report
         error_report = ErrorReport(
             error_id=self.generate_error_id(),
@@ -277,12 +292,12 @@ def handle_error(
             ),
         )
 
-        # Log the error
+        # Log the error (now at the final, possibly overridden severity)
         self._log_error(error_report)
 
         # Store error report
         self.error_reports.append(error_report)
-        self._update_error_counts(category)
+        self._update_error_counts(category, severity)
 
         return error_report
 
@@ -381,33 +396,61 @@ def _log_error(self, error_report: ErrorReport) -> None:
                 f"[{error_report.error_id}] Traceback:\\n{error_report.traceback_str}"
             )
 
-    def _update_error_counts(self, category: ErrorCategory) -> None:
-        """Update error count statistics."""
-        key = category.value
-        self.error_counts[key] = self.error_counts.get(key, 0) + 1
+    def _update_error_counts(
+        self, category: ErrorCategory, severity: ErrorSeverity
+    ) -> None:
+        """Update nested error counters: category → severity → count.
+
+        ``error_counts`` is a ``Dict[str, Dict[str, int]]`` so callers can ask
+        e.g. how many TRADING_ERROR/HIGH events happened without having to
+        scan ``error_reports`` themselves. A flat per-category total is
+        available via :meth:`category_total` for callers that don't care
+        about severity breakdown.
+        """
+        cat_key = category.value
+        sev_key = severity.value
+        bucket = self.error_counts.setdefault(cat_key, {})
+        bucket[sev_key] = bucket.get(sev_key, 0) + 1
+
+    def category_total(self, category: ErrorCategory) -> int:
+        """Return total count for ``category`` across all severities."""
+        return sum(self.error_counts.get(category.value, {}).values())
 
     def get_error_summary(self) -> Dict[str, Any]:
-        """Get summary of error statistics."""
+        """Get summary of error statistics.
+
+        Returns a dict with:
+        - ``total_errors``: total handled
+        - ``categories``: flat category → count (sum across severities)
+        - ``severities``: flat severity → count (sum across categories)
+        - ``by_category_severity``: nested category → severity → count
+          (the raw structure of ``error_counts``)
+        """
         total_errors = len(self.error_reports)
         if total_errors == 0:
-            return {"total_errors": 0, "categories": {}, "severities": {}}
-
-        # Count by category
-        category_counts = {}
-        severity_counts = {}
-
-        for report in self.error_reports:
-            category_counts[report.category.value] = (
-                category_counts.get(report.category.value, 0) + 1
-            )
-            severity_counts[report.severity.value] = (
-                severity_counts.get(report.severity.value, 0) + 1
-            )
+            return {
+                "total_errors": 0,
+                "categories": {},
+                "severities": {},
+                "by_category_severity": {},
+            }
+
+        # Derive flat views from the nested error_counts so a single source
+        # of truth (the nested dict) drives every reporting surface.
+        category_counts: Dict[str, int] = {}
+        severity_counts: Dict[str, int] = {}
+        for cat_key, sev_bucket in self.error_counts.items():
+            category_counts[cat_key] = sum(sev_bucket.values())
+            for sev_key, count in sev_bucket.items():
+                severity_counts[sev_key] = severity_counts.get(sev_key, 0) + count
 
         return {
             "total_errors": total_errors,
             "categories": category_counts,
             "severities": severity_counts,
+            "by_category_severity": {
+                cat: dict(sev_bucket) for cat, sev_bucket in self.error_counts.items()
+            },
             "recent_errors": [
                 {
                     "id": report.error_id,
diff --git a/app/test_error_handler.py b/app/test_error_handler.py
index 24d75df6..1adb6f38 100644
--- a/app/test_error_handler.py
+++ b/app/test_error_handler.py
@@ -1,7 +1,7 @@
 """Tests for pt_error_handler centralized error management system."""
 
 import unittest
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 from pt_error_handler import (
     ApplicationErrorHandler,
@@ -163,18 +163,25 @@ def test_suppress_module_blocks_callbacks(self):
         on_critical(cb)
         get_handler().suppress_module("pt_trader")
 
-        # Build an exception that looks like it came from pt_trader:
-        # ErrorHandler captures the caller's frame, so we monkeypatch
-        # the report module after handle_error but before _fire_callbacks
-        # by wrapping _handler.handle_error.
-        original_handle_error = get_handler()._handler.handle_error
-
-        def patched_handle_error(error, context=None):
-            report = original_handle_error(error, context=context)
-            report.module = "pt_trader"  # simulate origin in suppressed module
+        # ErrorHandler captures the test file's frame as report.module, so
+        # patch handle_error to override the module to simulate origin in
+        # the suppressed source file. Use patch.object so the override is
+        # restored via addCleanup regardless of test outcome (no manual
+        # try/finally, no leakage to other tests in this class).
+        original = get_handler()._handler.handle_error
+
+        def patched(error, context=None, severity_override=None):
+            report = original(
+                error, context=context, severity_override=severity_override
+            )
+            report.module = "pt_trader.py"  # filename form, suppression normalizes
             return report
 
-        get_handler()._handler.handle_error = patched_handle_error
+        patcher = patch.object(
+            get_handler()._handler, "handle_error", side_effect=patched
+        )
+        patcher.start()
+        self.addCleanup(patcher.stop)
 
         try:
             raise ValueError("from suppressed module")
@@ -183,6 +190,18 @@ def patched_handle_error(error, context=None):
 
         cb.assert_not_called()
 
+    def test_suppress_module_accepts_filename_and_module_name(self):
+        """Suppression normalizes — passing either 'pt_trader' or 'pt_trader.py'
+        suppresses errors reported as either form."""
+        h = get_handler()
+        # Both spellings collapse to the same suppressed entry
+        h.suppress_module("pt_trader")
+        h.suppress_module("pt_trader.py")
+        self.assertTrue(h._is_module_suppressed("pt_trader"))
+        self.assertTrue(h._is_module_suppressed("pt_trader.py"))
+        h.unsuppress_module("pt_trader.py")
+        self.assertFalse(h._is_module_suppressed("pt_trader"))
+
 
 class TestQueryAPI(unittest.TestCase):
     def setUp(self):

From 25f5a5bf03ef1178239102e144e97edc644dcf0b Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Sat, 23 May 2026 16:22:42 +0300
Subject: [PATCH 06/11] fix: preserve flat error_counts shape for backward
 compatibility

External callers reading ErrorHandler.error_counts directly expected
Dict[str, int]. The earlier change to Dict[str, Dict[str, int]] was a
silent BC break. Restore flat counter; expose nested breakdown via new
error_counts_by_severity attribute. Both updated atomically so they
cannot drift.
---
 app/pt_errors.py | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/app/pt_errors.py b/app/pt_errors.py
index c2626df1..1063c3be 100644
--- a/app/pt_errors.py
+++ b/app/pt_errors.py
@@ -205,8 +205,13 @@ class ErrorHandler:
     def __init__(self, logger: Optional[logging.Logger] = None):
         self.logger = logger or logging.getLogger(__name__)
         self.error_reports: List[ErrorReport] = []
-        # Nested counters: category.value -> severity.value -> count.
-        self.error_counts: Dict[str, Dict[str, int]] = {}
+        # Flat per-category total counter (preserved for backward compatibility
+        # with external callers that read this attribute directly).
+        self.error_counts: Dict[str, int] = {}
+        # Richer nested counter: category.value -> severity.value -> count.
+        # Both counters are updated atomically by _update_error_counts so they
+        # cannot drift out of sync.
+        self.error_counts_by_severity: Dict[str, Dict[str, int]] = {}
 
         # Recovery suggestions for common errors
         self.recovery_suggestions = {
@@ -399,32 +404,31 @@ def _log_error(self, error_report: ErrorReport) -> None:
     def _update_error_counts(
         self, category: ErrorCategory, severity: ErrorSeverity
     ) -> None:
-        """Update nested error counters: category → severity → count.
+        """Update both the flat per-category counter and the nested
+        category → severity → count counter in lockstep.
 
-        ``error_counts`` is a ``Dict[str, Dict[str, int]]`` so callers can ask
-        e.g. how many TRADING_ERROR/HIGH events happened without having to
-        scan ``error_reports`` themselves. A flat per-category total is
-        available via :meth:`category_total` for callers that don't care
-        about severity breakdown.
+        ``error_counts`` stays a ``Dict[str, int]`` to preserve backward
+        compatibility for external callers that read it directly. The richer
+        breakdown lives on ``error_counts_by_severity``.
         """
         cat_key = category.value
         sev_key = severity.value
-        bucket = self.error_counts.setdefault(cat_key, {})
+        self.error_counts[cat_key] = self.error_counts.get(cat_key, 0) + 1
+        bucket = self.error_counts_by_severity.setdefault(cat_key, {})
         bucket[sev_key] = bucket.get(sev_key, 0) + 1
 
     def category_total(self, category: ErrorCategory) -> int:
         """Return total count for ``category`` across all severities."""
-        return sum(self.error_counts.get(category.value, {}).values())
+        return self.error_counts.get(category.value, 0)
 
     def get_error_summary(self) -> Dict[str, Any]:
         """Get summary of error statistics.
 
         Returns a dict with:
         - ``total_errors``: total handled
-        - ``categories``: flat category → count (sum across severities)
+        - ``categories``: flat category → count
         - ``severities``: flat severity → count (sum across categories)
         - ``by_category_severity``: nested category → severity → count
-          (the raw structure of ``error_counts``)
         """
         total_errors = len(self.error_reports)
         if total_errors == 0:
@@ -435,21 +439,18 @@ def get_error_summary(self) -> Dict[str, Any]:
                 "by_category_severity": {},
             }
 
-        # Derive flat views from the nested error_counts so a single source
-        # of truth (the nested dict) drives every reporting surface.
-        category_counts: Dict[str, int] = {}
         severity_counts: Dict[str, int] = {}
-        for cat_key, sev_bucket in self.error_counts.items():
-            category_counts[cat_key] = sum(sev_bucket.values())
+        for sev_bucket in self.error_counts_by_severity.values():
             for sev_key, count in sev_bucket.items():
                 severity_counts[sev_key] = severity_counts.get(sev_key, 0) + count
 
         return {
             "total_errors": total_errors,
-            "categories": category_counts,
+            "categories": dict(self.error_counts),
             "severities": severity_counts,
             "by_category_severity": {
-                cat: dict(sev_bucket) for cat, sev_bucket in self.error_counts.items()
+                cat: dict(sev_bucket)
+                for cat, sev_bucket in self.error_counts_by_severity.items()
             },
             "recent_errors": [
                 {

From 32071273a9086dbc40ef6f2bac80877d77793f93 Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Sat, 23 May 2026 17:01:06 +0300
Subject: [PATCH 07/11] fix: address Copilot review on PR 81 (docstring scope,
 empty-state API)

- pt_error_handler.py module docstring: the prior example called
  handler.handle(exc) outside an except block; on Python 3 the
  exception variable is only bound inside the except block, so the
  snippet would raise UnboundLocalError if pasted as-is. Wrap the
  example in a try/except so it is copy-paste safe.

- pt_errors.get_error_summary(): the zero-error branch omitted the
  recent_errors key, forcing callers to special-case empty state.
  Always return recent_errors (as []) so the API shape is consistent.
---
 app/pt_error_handler.py | 10 +++++++---
 app/pt_errors.py        |  3 +++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index 887cf697..c2151d14 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -16,9 +16,13 @@
     except Exception as exc:
         handle(exc, context={"operation": "risky_operation"})
 
-    # Or via the global handler directly
-    handler = get_handler()
-    handler.handle(exc)
+    # Or via the global handler directly. The exception variable is only
+    # bound inside the `except` block on Python 3 (PEP 3134), so callers
+    # must invoke handle/handle_error from within that block.
+    try:
+        risky_operation()
+    except Exception as exc:
+        get_handler().handle(exc)
 """
 
 import logging
diff --git a/app/pt_errors.py b/app/pt_errors.py
index 1063c3be..8d03e340 100644
--- a/app/pt_errors.py
+++ b/app/pt_errors.py
@@ -432,11 +432,14 @@ def get_error_summary(self) -> Dict[str, Any]:
         """
         total_errors = len(self.error_reports)
         if total_errors == 0:
+            # Always include `recent_errors` so the API shape stays consistent
+            # and callers can iterate without a special case for empty state.
             return {
                 "total_errors": 0,
                 "categories": {},
                 "severities": {},
                 "by_category_severity": {},
+                "recent_errors": [],
             }
 
         severity_counts: Dict[str, int] = {}

From 19405e07c9bb60132421bc69cca2510bac523f80 Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Sun, 24 May 2026 13:25:55 +0300
Subject: [PATCH 08/11] fix: address Copilot review on PR 81 (typo,
 clear_history, thread safety)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- pt_error_handler.py:2 — strip stray '+' from module title so generated
  docs render 'PowerTraderAI Centralized Error Management System'.
- clear_history() now also clears error_counts_by_severity so
  get_summary().severities / by_category_severity start from zero after a
  reset instead of compounding across clears.
- ApplicationErrorHandler now holds a dedicated _state_lock around every
  call that mutates or reads the underlying ErrorHandler state
  (handle, handle_critical, get_summary, get_recent_errors,
  get_critical_errors, clear_history) so the thread-safety promise in the
  class docstring actually holds under concurrent access. Callbacks still
  fire outside the lock to avoid holding it across user code.
---
 app/pt_error_handler.py | 53 ++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index c2151d14..efe8a0a0 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -1,5 +1,5 @@
 """
-PowerTraderAI+ Centralized Error Management System
+PowerTraderAI Centralized Error Management System
 Single application-wide error handler with notification routing and
 classification. All modules should use `get_handler()` instead of
 creating their own ErrorHandler instances.
@@ -82,6 +82,11 @@ def _ensure_init(self) -> None:
             }
             self._suppressed_modules: set = set()
             self._cb_lock = threading.Lock()
+            # Guards mutation/read of the underlying ErrorHandler state
+            # (error_reports, error_counts, error_counts_by_severity) so
+            # concurrent handle()/clear_history()/get_summary() calls can't
+            # lose increments or observe partially-updated counters.
+            self._state_lock = threading.Lock()
             self._initialised = True
 
     # ------------------------------------------------------------------
@@ -149,7 +154,8 @@ def handle(
             ErrorReport with full classification and metadata
         """
         self._ensure_init()
-        report = self._handler.handle_error(error, context=context)
+        with self._state_lock:
+            report = self._handler.handle_error(error, context=context)
         self._fire_callbacks(report)
         if reraise:
             # If error is the currently active exception (called from inside an except
@@ -176,11 +182,12 @@ def handle_critical(
         severity consistently.
         """
         self._ensure_init()
-        report = self._handler.handle_error(
-            error,
-            context=context,
-            severity_override=ErrorSeverity.CRITICAL,
-        )
+        with self._state_lock:
+            report = self._handler.handle_error(
+                error,
+                context=context,
+                severity_override=ErrorSeverity.CRITICAL,
+            )
         self._fire_callbacks(report)
         return report
 
@@ -217,25 +224,37 @@ def _is_module_suppressed(self, module: Optional[str]) -> bool:
     # ------------------------------------------------------------------
     def get_summary(self) -> Dict:
         self._ensure_init()
-        return self._handler.get_error_summary()
+        with self._state_lock:
+            return self._handler.get_error_summary()
 
     def get_recent_errors(self, limit: int = 20) -> List[ErrorReport]:
         self._ensure_init()
-        return self._handler.error_reports[-limit:]
+        with self._state_lock:
+            return self._handler.error_reports[-limit:]
 
     def get_critical_errors(self) -> List[ErrorReport]:
         self._ensure_init()
-        return [
-            r
-            for r in self._handler.error_reports
-            if r.severity == ErrorSeverity.CRITICAL
-        ]
+        with self._state_lock:
+            return [
+                r
+                for r in self._handler.error_reports
+                if r.severity == ErrorSeverity.CRITICAL
+            ]
 
     def clear_history(self) -> None:
-        """Clear in-memory error history (does NOT affect log files)."""
+        """Clear in-memory error history (does NOT affect log files).
+
+        Resets every accumulator on the underlying ErrorHandler so that
+        subsequent ``get_summary()`` calls don't return stale severity totals
+        from the cleared run. Without clearing ``error_counts_by_severity``,
+        ``severities`` and ``by_category_severity`` would compound forever
+        across clears.
+        """
         self._ensure_init()
-        self._handler.error_reports.clear()
-        self._handler.error_counts.clear()
+        with self._state_lock:
+            self._handler.error_reports.clear()
+            self._handler.error_counts.clear()
+            self._handler.error_counts_by_severity.clear()
 
     @classmethod
     def reset_singleton(cls) -> None:

From 23eee9753ac8b1770af05400d117bd4ccdb0380f Mon Sep 17 00:00:00 2001
From: Simon Jackson <admin@jacksonfamily.me>
Date: Mon, 25 May 2026 19:18:14 +0100
Subject: [PATCH 09/11] Potential fix for pull request finding

comment only, easily rectified

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Simon Jackson <admin@jacksonfamily.me>
---
 app/pt_error_handler.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index efe8a0a0..b66bd866 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -18,7 +18,7 @@
 
     # Or via the global handler directly. The exception variable is only
     # bound inside the `except` block on Python 3 (PEP 3134), so callers
-    # must invoke handle/handle_error from within that block.
+    # must invoke handle()/get_handler().handle() from within that block.
     try:
         risky_operation()
     except Exception as exc:

From 901ee974a5600459262fb0e31ab2ca548d40edfc Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Tue, 26 May 2026 15:15:26 +0300
Subject: [PATCH 10/11] Address review: guard get_recent_errors against
 non-positive limit, correct counter thread-safety comment

- get_recent_errors() now returns [] for limit <= 0 instead of returning a
  wrong subset via negative-index slicing; added regression test.
- Corrected the error_counts comment in ErrorHandler: the two counters stay
  consistent within a single _update_error_counts call, but ErrorHandler is
  not itself thread-safe (the ApplicationErrorHandler wrapper serialises
  access via _state_lock).
---
 app/pt_error_handler.py   |  2 ++
 app/pt_errors.py          |  6 ++++--
 app/test_error_handler.py | 10 ++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index b66bd866..440cab11 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -229,6 +229,8 @@ def get_summary(self) -> Dict:
 
     def get_recent_errors(self, limit: int = 20) -> List[ErrorReport]:
         self._ensure_init()
+        if limit <= 0:
+            return []
         with self._state_lock:
             return self._handler.error_reports[-limit:]
 
diff --git a/app/pt_errors.py b/app/pt_errors.py
index 8d03e340..025db5a3 100644
--- a/app/pt_errors.py
+++ b/app/pt_errors.py
@@ -209,8 +209,10 @@ def __init__(self, logger: Optional[logging.Logger] = None):
         # with external callers that read this attribute directly).
         self.error_counts: Dict[str, int] = {}
         # Richer nested counter: category.value -> severity.value -> count.
-        # Both counters are updated atomically by _update_error_counts so they
-        # cannot drift out of sync.
+        # Both counters are bumped together in the single _update_error_counts
+        # call path, so they stay consistent for any one update. ErrorHandler
+        # is not itself thread-safe; concurrent callers must serialise access
+        # (the ApplicationErrorHandler wrapper does this via its _state_lock).
         self.error_counts_by_severity: Dict[str, Dict[str, int]] = {}
 
         # Recovery suggestions for common errors
diff --git a/app/test_error_handler.py b/app/test_error_handler.py
index 1adb6f38..77f7b75c 100644
--- a/app/test_error_handler.py
+++ b/app/test_error_handler.py
@@ -223,6 +223,16 @@ def test_get_recent_errors(self):
         recent = get_handler().get_recent_errors(limit=3)
         self.assertEqual(len(recent), 3)
 
+    def test_get_recent_errors_non_positive_limit(self):
+        """limit <= 0 returns [] rather than a wrong negative-slice subset."""
+        for i in range(5):
+            try:
+                raise ValueError(f"error {i}")
+            except ValueError as exc:
+                handle(exc)
+        self.assertEqual(get_handler().get_recent_errors(limit=0), [])
+        self.assertEqual(get_handler().get_recent_errors(limit=-3), [])
+
     def test_get_critical_errors_filtered(self):
         try:
             raise ValueError("normal")

From 9bd7c6e2ea1ef76bc0e9916f757032d56a9f7c69 Mon Sep 17 00:00:00 2001
From: Ibrahim Elsayed <ibrahim_alsaied@outlook.com>
Date: Tue, 26 May 2026 17:04:35 +0300
Subject: [PATCH 11/11] Align suppress_module docstring with code: accepts
 module name or filename

Doc said 'not the filename' then 'either spelling works'; code normalizes
both, so the docstring now states either form is accepted. Also drops a
stray em dash.
---
 app/pt_error_handler.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py
index 440cab11..d6db5a59 100644
--- a/app/pt_error_handler.py
+++ b/app/pt_error_handler.py
@@ -120,9 +120,9 @@ def suppress_module(self, module_name: str) -> None:
         """Suppress notification callbacks for errors originating from
         ``module_name``.
 
-        Pass the bare module name (``"pt_trader"``), not the filename —
-        :class:`ErrorReport` stores the filename (``"pt_trader.py"``) but
-        suppression normalizes both sides so either spelling works.
+        Accepts either the bare module name (``"pt_trader"``) or the filename
+        (``"pt_trader.py"``). :class:`ErrorReport` stores the filename, and
+        suppression normalizes both sides, so either spelling works.
         """
         self._ensure_init()
         with self._cb_lock: