Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# PRD: FU-P12-T3-2 — Add `error_code` column to audit CSV export

**Created:** 2026-02-19
**Priority:** P3
**Branch:** `codex/feature/FU-P12-T3-2-error-code-csv-export`
**Status:** PLAN

---

## 1. Problem Statement

`AuditLogger.export_csv()` uses a fixed CSV header list that excludes `error_code`.
As a result, exported audit files silently drop a useful diagnostic field even
when entries contain structured error metadata.

---

## 2. Scope

### In Scope
- Add `error_code` to CSV export headers in `AuditLogger.export_csv()`.
- Ensure rows include `error_code` values when present.
- Ensure rows emit empty strings for `error_code` when absent.
- Add/adjust unit tests in `tests/unit/webui/test_audit.py`.

### Out of Scope
- Audit schema redesign.
- New export formats.
- Dashboard UI changes.

---

## 3. Deliverables

1. CSV export update
- `src/mcpbridge_wrapper/webui/audit.py`
- `error_code` present in exported CSV columns.

2. Test coverage
- `tests/unit/webui/test_audit.py`
- Assertions for both present and missing `error_code` cases.

3. Validation artifact
- `SPECS/INPROGRESS/FU-P12-T3-2_Validation_Report.md`

---

## 4. Acceptance Criteria

- [ ] CSV export includes `error_code` column.
- [ ] Entries without `error_code` render empty string in that column.
- [ ] Existing CSV tests still pass.
- [ ] `pytest` passes.
- [ ] `ruff check src/` passes.
- [ ] `mypy src/` passes.
- [ ] `pytest --cov` reports coverage >= 90%.

---

## 5. Dependencies

- P12-T3 ✅

---

## 6. Risks and Mitigations

- **Risk:** Column order changes may break fragile tests or downstream CSV
consumers expecting a fixed order.
- **Mitigation:** Keep existing order and append `error_code` at the end to
minimize disruption.

---

## 7. Validation Plan

1. Update export column list to include `error_code`.
2. Add/adjust test fixtures and assertions for CSV header + row values.
3. Run required quality gates and document results in validation report.

---
**Archived:** 2026-02-19
**Verdict:** PASS
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Validation Report — FU-P12-T3-2

**Task:** FU-P12-T3-2 — Add `error_code` column to audit CSV export
**Date:** 2026-02-19
**Verdict:** PASS

## Scope

- Added `error_code` to `AuditLogger.export_csv()` fieldnames.
- Updated audit CSV tests to validate populated and empty `error_code` values.

## Files Changed

- `src/mcpbridge_wrapper/webui/audit.py`
- `tests/unit/webui/test_audit.py`

## Required Quality Gates

- `pytest`
- Result: **PASS** (`586 passed, 5 skipped, 2 warnings`)
- `ruff check src/`
- Result: **PASS** (`All checks passed!`)
- `mypy src/`
- Result: **PASS** (`Success: no issues found in 18 source files`)
- `pytest --cov`
- Result: **PASS** (`586 passed, 5 skipped, 2 warnings`; total coverage **92.18%**, threshold 90%)

## Acceptance Criteria Status

- [x] CSV export includes `error_code` column.
- [x] Entries without `error_code` show empty string for the column.
- [x] Existing CSV tests still pass.

## Notes

- Existing third-party deprecation warnings from `websockets` / `uvicorn` were observed during test runs and are unrelated to this task.
6 changes: 5 additions & 1 deletion SPECS/ARCHIVE/INDEX.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# mcpbridge-wrapper Tasks Archive

**Last Updated:** 2026-02-18 (REVIEW_fu_p12_t1_3_multi_client_widgets_archived)
**Last Updated:** 2026-02-19 (REVIEW_fu_p12_t3_2_error_code_csv_export_archived)

## Archived Tasks

Expand Down Expand Up @@ -118,6 +118,7 @@
| FU-P12-T1-1 | [FU-P12-T1-1_Remove_or_document_MCPInitializeParams_in_schemas/](FU-P12-T1-1_Remove_or_document_MCPInitializeParams_in_schemas/) | 2026-02-18 | PASS |
| FU-P12-T1-2 | [FU-P12-T1-2_Add_code_comment_clarifying_stdin-only_client_capture_in_on_request/](FU-P12-T1-2_Add_code_comment_clarifying_stdin-only_client_capture_in_on_request/) | 2026-02-18 | PASS |
| FU-P12-T1-3 | [FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/](FU-P12-T1-3_Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client/) | 2026-02-18 | PASS |
| FU-P12-T3-2 | [FU-P12-T3-2_Add_error_code_column_to_audit_CSV_export/](FU-P12-T3-2_Add_error_code_column_to_audit_CSV_export/) | 2026-02-19 | PASS |

## Historical Artifacts

Expand Down Expand Up @@ -199,6 +200,7 @@
| [REVIEW_FU-P12-T1-1_mcpinitializeparams.md](_Historical/REVIEW_FU-P12-T1-1_mcpinitializeparams.md) | Review report for FU-P12-T1-1 |
| [REVIEW_FU-P12-T1-2_stdin_capture_comment.md](_Historical/REVIEW_FU-P12-T1-2_stdin_capture_comment.md) | Review report for FU-P12-T1-2 |
| [REVIEW_FU-P12-T1-3_multi_client_widgets.md](_Historical/REVIEW_FU-P12-T1-3_multi_client_widgets.md) | Review report for FU-P12-T1-3 |
| [REVIEW_FU-P12-T3-2_error_code_csv_export.md](_Historical/REVIEW_FU-P12-T3-2_error_code_csv_export.md) | Review report for FU-P12-T3-2 |

## Archive Log

Expand Down Expand Up @@ -352,3 +354,5 @@
| 2026-02-18 | FU-P12-T1-2 | Archived REVIEW_FU-P12-T1-2_stdin_capture_comment report |
| 2026-02-18 | FU-P12-T1-3 | Archived Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client (PASS) |
| 2026-02-18 | FU-P12-T1-3 | Archived REVIEW_FU-P12-T1-3_multi_client_widgets report |
| 2026-02-19 | FU-P12-T3-2 | Archived Add_error_code_column_to_audit_CSV_export (PASS) |
| 2026-02-19 | FU-P12-T3-2 | Archived REVIEW_FU-P12-T3-2_error_code_csv_export report |
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
## REVIEW REPORT — FU-P12-T3-2 error code CSV export

**Scope:** origin/main..HEAD
**Files:** 7

### Summary Verdict
- [x] Approve
- [ ] Approve with comments
- [ ] Request changes
- [ ] Block

### Critical Issues
- None.

### Secondary Issues
- None.

### Architectural Notes
- Change is narrowly scoped and preserves existing audit logging behavior while
improving export completeness for downstream analysis.

### Tests
- Quality gates rerun and passing:
- `pytest` (`586 passed, 5 skipped, 2 warnings`)
- `ruff check src/` (`All checks passed!`)
- `mypy src/` (`Success: no issues found in 18 source files`)
- `pytest --cov` (`92.18%`, threshold `>=90%`)

### Next Steps
- No actionable follow-up items identified.
- FOLLOW-UP step can be skipped for this task.
4 changes: 2 additions & 2 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@

## Recently Archived

- 2026-02-19 — FU-P12-T3-2: Add `error_code` column to audit CSV export (PASS)
- 2026-02-18 — FU-P12-T1-3: Show multi-client widgets in Web UI instead of single overwritten active client (PASS)
- 2026-02-18 — FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (PASS)
- 2026-02-18 — FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (PASS)
- 2026-02-18 — FU-BUG-T7-1: Cap `pending_methods` map to guard against unbounded growth (PASS)
- 2026-02-18 — FU-P13-T2-2: Move PID file write to after successful upstream launch (PASS)
- 2026-02-18 — FU-P13-T2-1: Replace run_forever() polling loop with asyncio.Event-based wait (PASS)

## Suggested Next Tasks

- P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1)
- FU-P12-T1-4 — Make `IN FLIGHT` KPI reflect real in-flight requests in shared-metrics mode (P2)
- FU-P12-T3-2Add `error_code` column to audit CSV export (P3)
- FU-P12-T1-5Cap `_clients` dict and prune `client_identities` to prevent unbounded growth (P2)
9 changes: 5 additions & 4 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -2312,7 +2312,8 @@ Phase 9 Follow-up Backlog

---

#### FU-P12-T3-2: Add `error_code` column to audit CSV export
#### ✅ FU-P12-T3-2: Add `error_code` column to audit CSV export
- **Status:** ✅ Completed (2026-02-19)
- **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
Expand All @@ -2321,9 +2322,9 @@ Phase 9 Follow-up Backlog
- 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
- [x] CSV export includes `error_code` column
- [x] Entries without `error_code` show empty string for the column
- [x] Existing CSV tests still pass

---

Expand Down
1 change: 1 addition & 0 deletions src/mcpbridge_wrapper/webui/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ def export_csv(self, limit: Optional[int] = None) -> str:
"request_id",
"latency_ms",
"error",
"error_code",
]
writer = csv.DictWriter(output, fieldnames=fieldnames, extrasaction="ignore")
writer.writeheader()
Expand Down
24 changes: 20 additions & 4 deletions tests/unit/webui/test_audit.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Tests for webui audit module."""

import csv
import io
import json
import os
import tempfile
Expand Down Expand Up @@ -125,15 +127,29 @@ def test_export_json_with_limit(self):

def test_export_csv(self):
"""Test exporting entries as CSV."""
with tempfile.TemporaryDirectory() as tmpdir:
audit = AuditLogger(log_dir=tmpdir)
audit.log("XcodeRead", request_id="123", latency_ms=50.0, error_code=-32601)

csv_str = audit.export_csv()
rows = list(csv.DictReader(io.StringIO(csv_str)))
assert rows
assert "error_code" in rows[0]
assert rows[0]["tool"] == "XcodeRead"
assert rows[0]["request_id"] == "123"
assert rows[0]["error_code"] == "-32601"
audit.close()

def test_export_csv_empty_error_code_column(self):
"""CSV exports empty string when an entry has no error_code."""
with tempfile.TemporaryDirectory() as tmpdir:
audit = AuditLogger(log_dir=tmpdir)
audit.log("XcodeRead", request_id="123", latency_ms=50.0)

csv_str = audit.export_csv()
assert "timestamp_iso" in csv_str
assert "tool" in csv_str
assert "XcodeRead" in csv_str
assert "123" in csv_str
rows = list(csv.DictReader(io.StringIO(csv_str)))
assert rows
assert rows[0]["error_code"] == ""
audit.close()

def test_export_csv_empty(self):
Expand Down