Skip to content

Commit ea17537

Browse files
authored
Merge pull request #77 from SoundBlaster/codex/feature/FU-P12-T3-2-error-code-csv-export
FU-P12-T3-2: Add error_code column to audit CSV export
2 parents 500a031 + 4e9b51a commit ea17537

8 files changed

Lines changed: 183 additions & 11 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# PRD: FU-P12-T3-2 — Add `error_code` column to audit CSV export
2+
3+
**Created:** 2026-02-19
4+
**Priority:** P3
5+
**Branch:** `codex/feature/FU-P12-T3-2-error-code-csv-export`
6+
**Status:** PLAN
7+
8+
---
9+
10+
## 1. Problem Statement
11+
12+
`AuditLogger.export_csv()` uses a fixed CSV header list that excludes `error_code`.
13+
As a result, exported audit files silently drop a useful diagnostic field even
14+
when entries contain structured error metadata.
15+
16+
---
17+
18+
## 2. Scope
19+
20+
### In Scope
21+
- Add `error_code` to CSV export headers in `AuditLogger.export_csv()`.
22+
- Ensure rows include `error_code` values when present.
23+
- Ensure rows emit empty strings for `error_code` when absent.
24+
- Add/adjust unit tests in `tests/unit/webui/test_audit.py`.
25+
26+
### Out of Scope
27+
- Audit schema redesign.
28+
- New export formats.
29+
- Dashboard UI changes.
30+
31+
---
32+
33+
## 3. Deliverables
34+
35+
1. CSV export update
36+
- `src/mcpbridge_wrapper/webui/audit.py`
37+
- `error_code` present in exported CSV columns.
38+
39+
2. Test coverage
40+
- `tests/unit/webui/test_audit.py`
41+
- Assertions for both present and missing `error_code` cases.
42+
43+
3. Validation artifact
44+
- `SPECS/INPROGRESS/FU-P12-T3-2_Validation_Report.md`
45+
46+
---
47+
48+
## 4. Acceptance Criteria
49+
50+
- [ ] CSV export includes `error_code` column.
51+
- [ ] Entries without `error_code` render empty string in that column.
52+
- [ ] Existing CSV tests still pass.
53+
- [ ] `pytest` passes.
54+
- [ ] `ruff check src/` passes.
55+
- [ ] `mypy src/` passes.
56+
- [ ] `pytest --cov` reports coverage >= 90%.
57+
58+
---
59+
60+
## 5. Dependencies
61+
62+
- P12-T3 ✅
63+
64+
---
65+
66+
## 6. Risks and Mitigations
67+
68+
- **Risk:** Column order changes may break fragile tests or downstream CSV
69+
consumers expecting a fixed order.
70+
- **Mitigation:** Keep existing order and append `error_code` at the end to
71+
minimize disruption.
72+
73+
---
74+
75+
## 7. Validation Plan
76+
77+
1. Update export column list to include `error_code`.
78+
2. Add/adjust test fixtures and assertions for CSV header + row values.
79+
3. Run required quality gates and document results in validation report.
80+
81+
---
82+
**Archived:** 2026-02-19
83+
**Verdict:** PASS
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Validation Report — FU-P12-T3-2
2+
3+
**Task:** FU-P12-T3-2 — Add `error_code` column to audit CSV export
4+
**Date:** 2026-02-19
5+
**Verdict:** PASS
6+
7+
## Scope
8+
9+
- Added `error_code` to `AuditLogger.export_csv()` fieldnames.
10+
- Updated audit CSV tests to validate populated and empty `error_code` values.
11+
12+
## Files Changed
13+
14+
- `src/mcpbridge_wrapper/webui/audit.py`
15+
- `tests/unit/webui/test_audit.py`
16+
17+
## Required Quality Gates
18+
19+
- `pytest`
20+
- Result: **PASS** (`586 passed, 5 skipped, 2 warnings`)
21+
- `ruff check src/`
22+
- Result: **PASS** (`All checks passed!`)
23+
- `mypy src/`
24+
- Result: **PASS** (`Success: no issues found in 18 source files`)
25+
- `pytest --cov`
26+
- Result: **PASS** (`586 passed, 5 skipped, 2 warnings`; total coverage **92.18%**, threshold 90%)
27+
28+
## Acceptance Criteria Status
29+
30+
- [x] CSV export includes `error_code` column.
31+
- [x] Entries without `error_code` show empty string for the column.
32+
- [x] Existing CSV tests still pass.
33+
34+
## Notes
35+
36+
- Existing third-party deprecation warnings from `websockets` / `uvicorn` were observed during test runs and are unrelated to this task.

SPECS/ARCHIVE/INDEX.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# mcpbridge-wrapper Tasks Archive
22

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

55
## Archived Tasks
66

@@ -118,6 +118,7 @@
118118
| 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 |
119119
| 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 |
120120
| 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 |
121+
| 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 |
121122

122123
## Historical Artifacts
123124

@@ -199,6 +200,7 @@
199200
| [REVIEW_FU-P12-T1-1_mcpinitializeparams.md](_Historical/REVIEW_FU-P12-T1-1_mcpinitializeparams.md) | Review report for FU-P12-T1-1 |
200201
| [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 |
201202
| [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 |
203+
| [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 |
202204

203205
## Archive Log
204206

@@ -352,3 +354,5 @@
352354
| 2026-02-18 | FU-P12-T1-2 | Archived REVIEW_FU-P12-T1-2_stdin_capture_comment report |
353355
| 2026-02-18 | FU-P12-T1-3 | Archived Show_multi-client_widgets_in_Web_UI_instead_of_single_overwritten_active_client (PASS) |
354356
| 2026-02-18 | FU-P12-T1-3 | Archived REVIEW_FU-P12-T1-3_multi_client_widgets report |
357+
| 2026-02-19 | FU-P12-T3-2 | Archived Add_error_code_column_to_audit_CSV_export (PASS) |
358+
| 2026-02-19 | FU-P12-T3-2 | Archived REVIEW_FU-P12-T3-2_error_code_csv_export report |
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
## REVIEW REPORT — FU-P12-T3-2 error code CSV export
2+
3+
**Scope:** origin/main..HEAD
4+
**Files:** 7
5+
6+
### Summary Verdict
7+
- [x] Approve
8+
- [ ] Approve with comments
9+
- [ ] Request changes
10+
- [ ] Block
11+
12+
### Critical Issues
13+
- None.
14+
15+
### Secondary Issues
16+
- None.
17+
18+
### Architectural Notes
19+
- Change is narrowly scoped and preserves existing audit logging behavior while
20+
improving export completeness for downstream analysis.
21+
22+
### Tests
23+
- Quality gates rerun and passing:
24+
- `pytest` (`586 passed, 5 skipped, 2 warnings`)
25+
- `ruff check src/` (`All checks passed!`)
26+
- `mypy src/` (`Success: no issues found in 18 source files`)
27+
- `pytest --cov` (`92.18%`, threshold `>=90%`)
28+
29+
### Next Steps
30+
- No actionable follow-up items identified.
31+
- FOLLOW-UP step can be skipped for this task.

SPECS/INPROGRESS/next.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
## Recently Archived
44

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

1212
## Suggested Next Tasks
1313

1414
- P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1)
1515
- FU-P12-T1-4 — Make `IN FLIGHT` KPI reflect real in-flight requests in shared-metrics mode (P2)
16-
- FU-P12-T3-2Add `error_code` column to audit CSV export (P3)
16+
- FU-P12-T1-5Cap `_clients` dict and prune `client_identities` to prevent unbounded growth (P2)

SPECS/Workplan.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2312,7 +2312,8 @@ Phase 9 Follow-up Backlog
23122312

23132313
---
23142314

2315-
#### FU-P12-T3-2: Add `error_code` column to audit CSV export
2315+
#### ✅ FU-P12-T3-2: Add `error_code` column to audit CSV export
2316+
- **Status:** ✅ Completed (2026-02-19)
23162317
- **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.
23172318
- **Priority:** P3
23182319
- **Dependencies:** P12-T3
@@ -2321,9 +2322,9 @@ Phase 9 Follow-up Backlog
23212322
- Updated `src/mcpbridge_wrapper/webui/audit.py``error_code` added to CSV export columns
23222323
- Updated `tests/unit/webui/test_audit.py` — test verifying `error_code` in CSV output
23232324
- **Acceptance Criteria:**
2324-
- [ ] CSV export includes `error_code` column
2325-
- [ ] Entries without `error_code` show empty string for the column
2326-
- [ ] Existing CSV tests still pass
2325+
- [x] CSV export includes `error_code` column
2326+
- [x] Entries without `error_code` show empty string for the column
2327+
- [x] Existing CSV tests still pass
23272328

23282329
---
23292330

src/mcpbridge_wrapper/webui/audit.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ def export_csv(self, limit: Optional[int] = None) -> str:
314314
"request_id",
315315
"latency_ms",
316316
"error",
317+
"error_code",
317318
]
318319
writer = csv.DictWriter(output, fieldnames=fieldnames, extrasaction="ignore")
319320
writer.writeheader()

tests/unit/webui/test_audit.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Tests for webui audit module."""
22

3+
import csv
4+
import io
35
import json
46
import os
57
import tempfile
@@ -125,15 +127,29 @@ def test_export_json_with_limit(self):
125127

126128
def test_export_csv(self):
127129
"""Test exporting entries as CSV."""
130+
with tempfile.TemporaryDirectory() as tmpdir:
131+
audit = AuditLogger(log_dir=tmpdir)
132+
audit.log("XcodeRead", request_id="123", latency_ms=50.0, error_code=-32601)
133+
134+
csv_str = audit.export_csv()
135+
rows = list(csv.DictReader(io.StringIO(csv_str)))
136+
assert rows
137+
assert "error_code" in rows[0]
138+
assert rows[0]["tool"] == "XcodeRead"
139+
assert rows[0]["request_id"] == "123"
140+
assert rows[0]["error_code"] == "-32601"
141+
audit.close()
142+
143+
def test_export_csv_empty_error_code_column(self):
144+
"""CSV exports empty string when an entry has no error_code."""
128145
with tempfile.TemporaryDirectory() as tmpdir:
129146
audit = AuditLogger(log_dir=tmpdir)
130147
audit.log("XcodeRead", request_id="123", latency_ms=50.0)
131148

132149
csv_str = audit.export_csv()
133-
assert "timestamp_iso" in csv_str
134-
assert "tool" in csv_str
135-
assert "XcodeRead" in csv_str
136-
assert "123" in csv_str
150+
rows = list(csv.DictReader(io.StringIO(csv_str)))
151+
assert rows
152+
assert rows[0]["error_code"] == ""
137153
audit.close()
138154

139155
def test_export_csv_empty(self):

0 commit comments

Comments
 (0)