P12-T3: Add Error Classification & Categorization

SPECS/ARCHIVE/INDEX.md

@@ -1,6 +1,6 @@
 # mcpbridge-wrapper Tasks Archive
 
-**Last Updated:** 2026-02-15 (P12-T1)
+**Last Updated:** 2026-02-15 (P12-T3)
 
 ## Archived Tasks
 
@@ -95,6 +95,7 @@
 | P11-T3 | [P11-T3_Add_Dashboard_Theme_Toggle/](P11-T3_Add_Dashboard_Theme_Toggle/) | 2026-02-15 | PASS |
 | P11-T4 | [P11-T4_Add_Keyboard_Shortcuts_Command_Palette/](P11-T4_Add_Keyboard_Shortcuts_Command_Palette/) | 2026-02-15 | PASS |
 | P12-T1 | [P12-T1_Add_MCP_Client_Identification/](P12-T1_Add_MCP_Client_Identification/) | 2026-02-15 | PASS |
+| P12-T3 | [P12-T3_Add_Error_Classification_and_Categorization/](P12-T3_Add_Error_Classification_and_Categorization/) | 2026-02-15 | PASS |
 
 ## Historical Artifacts
 
@@ -154,6 +155,7 @@
 | [REVIEW_P11-T3_dashboard_theme_toggle.md](P11-T3_Add_Dashboard_Theme_Toggle/REVIEW_P11-T3_dashboard_theme_toggle.md) | Review report for P11-T3 |
 | [REVIEW_P11-T4_keyboard_shortcuts.md](P11-T4_Add_Keyboard_Shortcuts_Command_Palette/REVIEW_P11-T4_keyboard_shortcuts.md) | Review report for P11-T4 |
 | [REVIEW_P12-T1_mcp_client_identification.md](P12-T1_Add_MCP_Client_Identification/REVIEW_P12-T1_mcp_client_identification.md) | Review report for P12-T1 |
+| [REVIEW_P12-T3_error_classification_categorization.md](P12-T3_Add_Error_Classification_and_Categorization/REVIEW_P12-T3_error_classification_categorization.md) | Review report for P12-T3 |
 
 ## Archive Log
 
@@ -261,3 +263,5 @@
 | 2026-02-15 | P11-T4 | Archived REVIEW_P11-T4_keyboard_shortcuts report |
 | 2026-02-15 | P12-T1 | Archived Add_MCP_Client_Identification (PASS) |
 | 2026-02-15 | P12-T1 | Archived REVIEW_P12-T1_mcp_client_identification report |
+| 2026-02-15 | P12-T3 | Archived Add_Error_Classification_and_Categorization (PASS) |
+| 2026-02-15 | P12-T3 | Archived REVIEW_P12-T3_error_classification_categorization report |

SPECS/ARCHIVE/P12-T3_Add_Error_Classification_and_Categorization/P12-T3_Add_Error_Classification_and_Categorization.md

@@ -0,0 +1,114 @@
+# P12-T3: Add Error Classification & Categorization
+
+**Phase:** Phase 12 — Analytics & Insights
+**Priority:** P1
+**Branch:** feature/P12-T3-error-classification-categorization
+**Dependencies:** P10-T1 ✅
+
+---
+
+## Overview
+
+Parse JSON-RPC error codes and messages from responses and categorize them into
+meaningful buckets. Extend the metrics pipeline to track error breakdown, display
+an error doughnut chart on the dashboard, and color-code the audit table error
+column by severity.
+
+---
+
+## Deliverables
+
+### 1. `src/mcpbridge_wrapper/schemas.py`
+- Add `get_error_code()` and `get_error_message()` helper methods to `MCPResponse`
+  that return `Optional[int]` and `Optional[str]` from `self.error.code / .message`.
+
+### 2. `src/mcpbridge_wrapper/webui/metrics.py` (MetricsCollector)
+- Add `_error_counts_by_code: Dict[int, int]` attribute.
+- Extend `record_response` to accept `error_code: Optional[int] = None` and
+  `error_message: Optional[str] = None` parameters.
+- When `error=True` and `error_code` is provided, increment
+  `_error_counts_by_code[error_code]`.
+- Expose `error_counts_by_code` in `get_summary()` output.
+- Reset `_error_counts_by_code` in `reset()`.
+
+### 3. `src/mcpbridge_wrapper/webui/shared_metrics.py` (SharedMetricsStore)
+- Add `error_code INTEGER` and `error_message TEXT` columns to the `requests`
+  SQLite table (via `ALTER TABLE IF NOT EXISTS` pattern in `_ensure_db`).
+- Extend `record_response` to accept and persist `error_code` and `error_message`.
+- Extend `get_summary()` to compute and return `error_counts_by_code: Dict[int, int]`
+  from the DB.
+
+### 4. `src/mcpbridge_wrapper/__main__.py`
+- In the response handler, after `_has_error(line)`, extract the error code/message
+  using `MCPResponse.model_validate_json(line)` (reuse existing parse logic).
+- Pass `error_code` and `error_message` to `metrics.record_response(...)`.
+
+### 5. Error category helper (`src/mcpbridge_wrapper/webui/metrics.py`)
+- Add module-level function `categorize_error(code: Optional[int]) -> str` that
+  returns one of: `"protocol"` (codes -32600 to -32699), `"timeout"` (custom
+  code -32001), `"tool"` (positive codes ≥ 1), `"unknown"` (everything else).
+
+### 6. `src/mcpbridge_wrapper/webui/server.py`
+- Include `error_counts_by_code` in the `/api/metrics/summary` JSON response
+  (already populated from `get_summary()`; just ensure it's passed through).
+
+### 7. `src/mcpbridge_wrapper/webui/static/index.html`
+- Add a `<canvas id="error-breakdown-chart">` container in the dashboard,
+  alongside the existing KPI cards (or replacing "Total Errors" card with a
+  chart section beneath the KPI grid).
+
+### 8. `src/mcpbridge_wrapper/webui/static/dashboard.js`
+- Initialize a Chart.js doughnut chart (`errorBreakdownChart`) using the
+  `error_counts_by_code` data from the metrics summary.
+- Map codes to category labels via `categorizeError(code)`.
+- Color-code audit table error column cells by severity class:
+  - `error-protocol` (red) for -326xx codes
+  - `error-tool` (orange) for positive codes
+  - `error-timeout` (yellow) for -32001
+  - `error-unknown` (grey) for all others
+
+### 9. `src/mcpbridge_wrapper/webui/static/dashboard.css`
+- Add CSS classes: `.error-protocol`, `.error-tool`, `.error-timeout`,
+  `.error-unknown` with appropriate color styling.
+
+### 10. Tests
+- `tests/unit/webui/test_metrics.py`:
+  - Test `categorize_error()` for all four categories.
+  - Test `MetricsCollector.record_response()` with `error_code` tracks in
+    `error_counts_by_code`.
+  - Test `get_summary()` includes `error_counts_by_code`.
+- `tests/unit/webui/test_shared_metrics.py`:
+  - Test `SharedMetricsStore.record_response()` with `error_code` and `error_message`.
+  - Test `get_summary()` returns `error_counts_by_code`.
+- `tests/unit/test_main.py`:
+  - Test that error code is extracted and passed to `metrics.record_response`.
+
+---
+
+## Acceptance Criteria
+
+- [ ] JSON-RPC error code and message are extracted from error responses.
+- [ ] `MetricsCollector.get_summary()` includes `error_counts_by_code`.
+- [ ] `SharedMetricsStore.get_summary()` includes `error_counts_by_code`.
+- [ ] Dashboard displays error breakdown doughnut chart.
+- [ ] Audit table error column color-coded by severity.
+- [ ] `categorize_error()` correctly maps: protocol (-326xx), timeout (-32001),
+  tool (positive), unknown (rest).
+- [ ] All tests pass. Coverage ≥ 90%.
+
+---
+
+## Dependencies
+
+- `MCPResponse` in `schemas.py` already has `error: Optional[MCPError]` with
+  `code: int` and `message: str`. No schema changes needed beyond helpers.
+- Chart.js is already loaded in `index.html`.
+- `_has_error()` in `__main__.py` already parses the response — reuse that parse.
+
+---
+
+## Out of Scope
+
+- Storing full error payloads beyond code + message.
+- Retroactive error categorization for existing audit log entries.
+- WebSocket push for error breakdown (polling is sufficient).

SPECS/ARCHIVE/P12-T3_Add_Error_Classification_and_Categorization/P12-T3_Validation_Report.md

@@ -0,0 +1,51 @@
+# P12-T3 Validation Report — Add Error Classification & Categorization
+
+**Date:** 2026-02-15
+**Branch:** feature/P12-T3-error-classification-categorization
+
+---
+
+## Quality Gate Results
+
+| Gate | Result | Detail |
+|------|--------|--------|
+| `pytest` | ✅ PASS | 437 passed, 5 skipped |
+| `ruff check src/` | ✅ PASS | No issues |
+| `mypy src/` | ✅ PASS | No issues in 13 source files |
+| `pytest --cov` | ✅ PASS | 96.09% (≥ 90% required) |
+
+---
+
+## Acceptance Criteria Check
+
+| Criterion | Status |
+|-----------|--------|
+| JSON-RPC error code and message extracted from error responses | ✅ |
+| `MetricsCollector.get_summary()` includes `error_counts_by_code` | ✅ |
+| `SharedMetricsStore.get_summary()` includes `error_counts_by_code` | ✅ |
+| Dashboard displays error breakdown doughnut chart | ✅ |
+| Audit table error column color-coded by severity | ✅ |
+| `categorize_error()` maps protocol/timeout/tool/unknown correctly | ✅ |
+| All tests pass, coverage ≥ 90% | ✅ |
+
+---
+
+## Files Modified
+
+| File | Change |
+|------|--------|
+| `src/mcpbridge_wrapper/schemas.py` | Added `get_error_code()` and `get_error_message()` methods to `MCPResponse` |
+| `src/mcpbridge_wrapper/__main__.py` | Added `_parse_error_info()`, updated response handler to extract and pass error code/message |
+| `src/mcpbridge_wrapper/webui/metrics.py` | Added `categorize_error()`, `_error_counts_by_code` tracking, extended `record_response` |
+| `src/mcpbridge_wrapper/webui/shared_metrics.py` | Added `error_code`/`error_message` columns, extended `record_response`, `get_summary` includes breakdown |
+| `src/mcpbridge_wrapper/webui/audit.py` | Added `error_code` parameter to `log()` |
+| `src/mcpbridge_wrapper/webui/static/index.html` | Added error breakdown chart canvas |
+| `src/mcpbridge_wrapper/webui/static/dashboard.js` | Added `categorizeError()`, `updateErrorBreakdownChart()`, severity-based audit color coding |
+| `src/mcpbridge_wrapper/webui/static/dashboard.css` | Added `.error-protocol`, `.error-tool`, `.error-timeout`, `.error-unknown` classes |
+| `tests/unit/webui/test_metrics.py` | Added `TestCategorizeError` class and `error_counts_by_code` tests |
+| `tests/unit/webui/test_shared_metrics.py` | Added `error_counts_by_code` tests |
+| `tests/unit/test_main.py` | Added `TestParseErrorInfo` class |
+
+---
+
+## Verdict: PASS

SPECS/ARCHIVE/P12-T3_Add_Error_Classification_and_Categorization/REVIEW_P12-T3_error_classification_categorization.md

@@ -0,0 +1,68 @@
+## REVIEW REPORT — P12-T3 Error Classification & Categorization
+
+**Scope:** origin/main..HEAD
+**Files:** 12 modified (8 src, 4 tests)
+
+---
+
+### Summary Verdict
+
+- [ ] Approve
+- [x] Approve with comments
+- [ ] Request changes
+- [ ] Block
+
+---
+
+### Critical Issues
+
+None.
+
+---
+
+### Secondary Issues
+
+**[Low] `error_message` parameter accepted by `record_response` but unused in `MetricsCollector`**
+
+`MetricsCollector.record_response()` now accepts `error_message: Optional[str]` but never stores or uses it. The in-memory collector tracks only `error_counts_by_code`. While the `SharedMetricsStore` correctly persists `error_message` to SQLite, the in-memory variant silently drops the value. This is fine for current usage (only the DB store is used at runtime) but could cause confusion in tests that rely on `MetricsCollector` directly.
+
+Fix suggestion: Either add a docstring note clarifying the parameter is accepted for API symmetry but not stored, or remove the parameter from `MetricsCollector` if it will never be needed.
+
+**[Low] `categorize_error` duplicated between Python and JavaScript**
+
+The categorization logic (`-32600...-32699` → protocol, `-32001` → timeout, `≥1` → tool) is implemented independently in both `metrics.py` and `dashboard.js`. If the boundaries change, both files must be updated in sync. This is a minor maintenance risk.
+
+Fix suggestion: Add a comment in both locations explicitly cross-referencing each other so future changes don't diverge silently.
+
+**[Low] `error_code` in audit entries is not covered by export CSV**
+
+`audit.py`'s `export_csv()` generates CSV columns from a fixed key set. The new `error_code` field will be missing from CSV exports since the column list is not dynamically derived from entry keys.
+
+Fix suggestion: Either add `error_code` to the CSV column list, or note this as a known limitation in a follow-up task.
+
+---
+
+### Architectural Notes
+
+- The `contextlib.suppress(Exception)` pattern for `ALTER TABLE` migrations is pragmatic but suppresses all exceptions, including unexpected ones like disk-full. This is acceptable for the current scale but would be worth narrowing to `sqlite3.OperationalError` in a production system.
+- The `categorize_error` function is exposed at module level from `metrics.py`, which is the right place — it's domain logic, not UI logic. The JS duplicate is unavoidable for client-side rendering.
+- The error breakdown chart correctly hides itself when no errors have been recorded, providing a clean empty state.
+
+---
+
+### Tests
+
+- 437 tests pass, 5 skipped.
+- Coverage: 96.09% (≥ 90% required).
+- `TestCategorizeError`: 9 tests covering all branches including boundary conditions.
+- `TestParseErrorInfo`: 5 tests covering success, error with code/message, and invalid JSON.
+- `test_error_counts_by_code_*`: 5 tests in `TestMetricsCollector` + 5 tests in `TestSharedMetricsStore`.
+- No missing coverage for the new code paths.
+
+---
+
+### Next Steps
+
+- FU-P12-T3-1: Document or remove unused `error_message` param in `MetricsCollector.record_response` (Low)
+- FU-P12-T3-2: Add `error_code` column to CSV audit export (Low)
+- Consider adding cross-reference comments between Python and JS `categorize_error` implementations.

SPECS/INPROGRESS/next.md

@@ -4,15 +4,16 @@ The previously selected task has been archived.
 
 ## Recently Archived
 
+- 2026-02-15 — P12-T3: Add Error Classification & Categorization (PASS)
 - 2026-02-15 — P12-T1: Add MCP Client Identification (PASS)
 - 2026-02-15 — P11-T4: Add Keyboard Shortcuts & Command Palette (PASS)
 - 2026-02-15 — P11-T3: Add Dashboard Theme Toggle (Dark/Light) (PASS)
 - 2026-02-15 — P11-T2: Add Session Timeline View (PASS)
-- 2026-02-15 — BUG-T8: Audit log cross-process visibility (PASS)
 
 ## Suggested Next Tasks
 
 - P12-T2: Add Tool Parameter Frequency Analysis (P3, depends on P12-T1 ✅)
-- P12-T3: Add Error Classification & Categorization (P1, depends on P10-T1 ✅)
+- FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (P3, depends on P12-T1 ✅)
+- FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (P3, depends on P12-T1 ✅)
 - FU-BUG-T7-1: Cap `pending_methods` map to guard against unbounded growth (P3)
 - FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to `MagicMock(spec=WebUIConfig)` (P3)

SPECS/Workplan.md

@@ -1719,7 +1719,7 @@ Phase 9 Follow-up Backlog
 
 ---
 
-#### P12-T3: Add Error Classification & Categorization
+#### ✅ P12-T3: Add Error Classification & Categorization
 - **Description:** Parse JSON-RPC error codes and messages from responses. Categorize into buckets: protocol errors (-326xx), tool execution errors (Xcode-side failures), timeout errors, connection errors. Extend `record_response` to accept `error_code: Optional[int]` and `error_message: Optional[str]`. New metrics: `error_counts_by_code: Dict[int, int]`. Dashboard: replace single "Total Errors" KPI with error breakdown doughnut chart. Audit table: color-code error column by severity.
 - **Priority:** P1
 - **Dependencies:** P10-T1
@@ -1945,6 +1945,34 @@ Phase 9 Follow-up Backlog
 
 ---
 
+#### FU-P12-T3-1: Document unused `error_message` parameter in `MetricsCollector.record_response`
+- **Description:** `MetricsCollector.record_response()` accepts `error_message: Optional[str]` for API symmetry with `SharedMetricsStore`, but never stores or uses it. Add a docstring note clarifying this parameter is accepted for interface compatibility but not persisted in the in-memory collector.
+- **Priority:** P3
+- **Dependencies:** P12-T3
+- **Parallelizable:** yes
+- **Outputs/Artifacts:**
+  - Updated `src/mcpbridge_wrapper/webui/metrics.py` — docstring clarification on `error_message` parameter
+- **Acceptance Criteria:**
+  - [ ] Docstring clearly notes `error_message` is accepted for API symmetry but not stored in-memory
+  - [ ] No functional changes
+
+---
+
+#### FU-P12-T3-2: Add `error_code` column to audit CSV export
+- **Description:** `AuditLogger.export_csv()` uses a fixed column list that does not include `error_code`. Audit entries with error codes will silently omit this field from CSV exports. Add `error_code` to the CSV column list so it is included when present.
+- **Priority:** P3
+- **Dependencies:** P12-T3
+- **Parallelizable:** yes
+- **Outputs/Artifacts:**
+  - Updated `src/mcpbridge_wrapper/webui/audit.py` — `error_code` added to CSV export columns
+  - Updated `tests/unit/webui/test_audit.py` — test verifying `error_code` in CSV output
+- **Acceptance Criteria:**
+  - [ ] CSV export includes `error_code` column
+  - [ ] Entries without `error_code` show empty string for the column
+  - [ ] Existing CSV tests still pass
+
+---
+
 #### FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)`
 - **Description:** The hand-rolled `_FakeWebUIConfig` class in `tests/unit/test_main.py` must be manually updated every time a new property is added to `WebUIConfig`. Refactor it to use `MagicMock(spec=WebUIConfig)` with only the properties needed by the test wired up, so the spec auto-enforces the real interface and future property additions do not break the test.
 - **Priority:** P3

src/mcpbridge_wrapper/__main__.py

@@ -155,22 +155,35 @@ def _extract_request_id(line: str) -> Optional[str]:
     return None
 
 
-def _has_error(line: str) -> bool:
-    """Check if a JSON-RPC response contains an error.
+def _parse_error_info(line: str) -> Tuple[bool, Optional[int], Optional[str]]:
+    """Parse error status, code, and message from a JSON-RPC response line.
 
     Args:
         line: A line from the bridge output.
 
     Returns:
-        True if the line contains an error response.
+        Tuple of (is_error, error_code, error_message).
     """
     try:
         from mcpbridge_wrapper.schemas import MCPResponse
 
         resp = MCPResponse.model_validate_json(line)
-        return resp.has_error()
+        return resp.has_error(), resp.get_error_code(), resp.get_error_message()
     except Exception:
-        return False
+        return False, None, None
+
+
+def _has_error(line: str) -> bool:
+    """Check if a JSON-RPC response contains an error.
+
+    Args:
+        line: A line from the bridge output.
+
+    Returns:
+        True if the line contains an error response.
+    """
+    is_error, _, _ = _parse_error_info(line)
+    return is_error
 
 
 def main() -> int:
@@ -366,20 +379,23 @@ def signal_handler(signum: int, frame: object) -> None:
                 # This is a response to a tracked request
                 pending_tool_name, pending_start_time = pending_requests.pop(request_id)
                 latency_ms = (time.time() - pending_start_time) * 1000.0
-                is_error = _has_error(line)
+                is_error, error_code, error_message = _parse_error_info(line)
                 metrics.record_response(
                     pending_tool_name,
                     request_id=request_id,
                     error=is_error,
                     latency_ms=latency_ms,
+                    error_code=error_code,
+                    error_message=error_message,
                 )
 
                 if audit is not None:
                     audit.log(
                         tool_name=pending_tool_name,
                         request_id=request_id,
                         latency_ms=latency_ms,
-                        error=str(is_error) if is_error else None,
+                        error=error_message if is_error else None,
+                        error_code=error_code if is_error else None,
                         direction="response",
                     )