Skip to content

Commit 2f28aec

Browse files
authored
Merge pull request #106 from SoundBlaster/codex/BUG-T13-per-tool-latency-params-hint
BUG-T13: Per-Tool Latency Statistics does not show params when capture_params is false
2 parents b631d3b + fa0c3ec commit 2f28aec

9 files changed

Lines changed: 229 additions & 17 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# PRD: BUG-T13 — Per-Tool Latency Statistics does not show params when `capture_params` is false
2+
3+
## Objective
4+
Improve the Per-Tool Latency Statistics UX so the dashboard clearly explains why parameter data is missing when parameter capture is disabled by configuration.
5+
6+
## Background
7+
When `metrics.capture_params` is `false` (default), the backend intentionally avoids parameter capture for privacy. The current UI silently omits parameter details, which looks like broken behavior instead of an intentional configuration state.
8+
9+
## Deliverables
10+
- Add a visible disabled-state hint in the Per-Tool Latency Statistics table when parameter capture is disabled.
11+
- Keep the existing detailed parameter table behavior unchanged when parameter capture is enabled.
12+
- Add regression tests that verify the disabled-state hint behavior in served dashboard frontend code.
13+
- Update any related docs only if implementation text needs adjustment.
14+
15+
## Dependencies
16+
- Existing `/api/config` payload exposing `metrics.capture_params`.
17+
- Existing per-tool latency table rendering logic in `src/mcpbridge_wrapper/webui/static/dashboard.js`.
18+
19+
## Acceptance Criteria
20+
- [ ] When `capture_params` is `false`, the Per-Tool Latency Statistics section shows an explicit hint that parameter capture is disabled.
21+
- [ ] The hint includes clear enablement guidance (set `metrics.capture_params: true` via web UI config).
22+
- [ ] When `capture_params` is `true`, the hint is hidden and normal parameter details continue to render.
23+
- [ ] Frontend regression tests cover the disabled-state hint behavior.
24+
- [ ] Required quality gates pass: `pytest`, `ruff check src/`, `mypy src/`, `pytest --cov` (coverage >= 90%).
25+
26+
## Validation Plan
27+
1. Add/extend frontend-oriented unit tests for dashboard rendering logic in `tests/unit/webui/test_server.py`.
28+
2. Run required quality gates and capture outputs in `SPECS/INPROGRESS/BUG-T13_Validation_Report.md`.
29+
3. Perform a brief manual read-through of generated dashboard HTML/JS responses for the disabled-state string and condition.
30+
31+
## Implementation Plan
32+
### Phase 1: Frontend disabled-state messaging
33+
- Update per-tool latency table rendering logic in `dashboard.js` to conditionally render a disabled-state row/message when parameter capture is disabled.
34+
- Ensure the message is scoped to parameter detail content and does not affect latency/call count metrics.
35+
36+
### Phase 2: Backend configuration wiring verification
37+
- Confirm dashboard config payload consumption already provides `capture_params` and wire the condition in rendering code if needed.
38+
39+
### Phase 3: Regression coverage and quality gates
40+
- Add assertions in tests for disabled/enabled rendering states.
41+
- Run quality gates and record evidence.
42+
43+
---
44+
**Archived:** 2026-02-25
45+
**Verdict:** PASS
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Validation Report: BUG-T13
2+
3+
## Task
4+
Per-Tool Latency Statistics does not show params when `capture_params` is false.
5+
6+
## Implementation Summary
7+
- Added a dashboard config fetch path in `dashboard.js` to read `/api/config` and track `metrics.capture_params` on the frontend.
8+
- Added a table-level disabled-state hint row for Per-Tool Latency Statistics when parameter capture is disabled.
9+
- Disabled parameter toggle buttons in that state and guarded click handling against disabled toggles.
10+
- Updated empty-pattern messaging to distinguish between:
11+
- capture disabled (`metrics.capture_params: true` guidance), and
12+
- capture enabled but no data yet.
13+
- Added styling for disabled toggle state and the disabled-hint row.
14+
- Added regression assertions in `tests/unit/webui/test_server.py` covering:
15+
- config exposure of `metrics.capture_params`,
16+
- frontend config fetch and conditional disabled-state rendering logic.
17+
18+
## Quality Gates
19+
20+
### 1) `PYTHONPATH=src pytest`
21+
- Result: PASS
22+
- Evidence: `637 passed, 5 skipped`
23+
24+
### 2) `ruff check src/`
25+
- Result: PASS
26+
- Evidence: `All checks passed!`
27+
28+
### 3) `PYTHONPATH=src mypy src/`
29+
- Result: PASS
30+
- Evidence: `Success: no issues found in 18 source files`
31+
32+
### 4) `PYTHONPATH=src pytest --cov`
33+
- Result: PASS
34+
- Evidence:
35+
- `637 passed, 5 skipped`
36+
- `Required test coverage of 90.0% reached`
37+
- `Total coverage: 91.33%`
38+
39+
## Manual Validation Notes
40+
- When `capture_params` is disabled, the latency table now immediately shows a clear configuration hint instead of only surfacing messaging after row interaction.
41+
- Param toggle controls are non-interactive in disabled mode to match visible state.
42+
- Existing expanded-row behavior for enabled mode remains covered by regression assertions.
43+
44+
## Verdict
45+
PASS

SPECS/ARCHIVE/INDEX.md

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

3-
**Last Updated:** 2026-02-25 (BUG-T11_Chart_Request_Timeline_never_shows_actual_events)
3+
**Last Updated:** 2026-02-25 (BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false)
44

55
## Archived Tasks
66

77
| Task ID | Folder | Archived | Verdict |
88
|---------|--------|----------|---------|
9+
| BUG-T13 | [BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false/](BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false/) | 2026-02-25 | PASS |
910
| BUG-T11 | [BUG-T11_Chart_Request_Timeline_never_shows_actual_events/](BUG-T11_Chart_Request_Timeline_never_shows_actual_events/) | 2026-02-25 | PASS |
1011
| BUG-T12 | [BUG-T12_Audit_Log_does_not_show_new_calls/](BUG-T12_Audit_Log_does_not_show_new_calls/) | 2026-02-20 | PASS |
1112
| BUG-T10 | [BUG-T10_Tool_chart_colors_change_on_update_of_tool_type_count/](BUG-T10_Tool_chart_colors_change_on_update_of_tool_type_count/) | 2026-02-20 | PASS |
@@ -250,6 +251,7 @@
250251
| [REVIEW_bug_t18_workplan_entry.md](_Historical/REVIEW_bug_t18_workplan_entry.md) | Review report for BUG-T18 |
251252
| [REVIEW_bug_t17_audit_log_rows_stay_unfolded.md](_Historical/REVIEW_bug_t17_audit_log_rows_stay_unfolded.md) | Review report for BUG-T17 |
252253
| [REVIEW_bug_t14_latency_rows.md](_Historical/REVIEW_bug_t14_latency_rows.md) | Review report for BUG-T14 |
254+
| [REVIEW_bug_t13_capture_params_hint.md](_Historical/REVIEW_bug_t13_capture_params_hint.md) | Review report for BUG-T13 |
253255
| [REVIEW_bug_t11_request_timeline.md](_Historical/REVIEW_bug_t11_request_timeline.md) | Review report for BUG-T11 |
254256

255257
| [REVIEW_bug_t10.md](_Historical/REVIEW_bug_t10.md) | Review report for BUG-T10 |
@@ -259,6 +261,8 @@
259261

260262
| Date | Task ID | Action |
261263
|------|---------|--------|
264+
| 2026-02-25 | BUG-T13 | Archived REVIEW_bug_t13_capture_params_hint report |
265+
| 2026-02-25 | BUG-T13 | Archived Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false (PASS) |
262266
| 2026-02-25 | BUG-T11 | Archived REVIEW_bug_t11_request_timeline report |
263267
| 2026-02-25 | BUG-T11 | Archived Chart_Request_Timeline_never_shows_actual_events (PASS) |
264268
| 2026-02-20 | BUG-T12 | Archived REVIEW_BUG-T12_audit_log_live_updates report |
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
## REVIEW REPORT — BUG-T13 Capture Params Hint
2+
3+
**Scope:** origin/main..HEAD
4+
**Files:** 8
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+
- Frontend now consumes `/api/config` to determine `metrics.capture_params` and renders a stable disabled-state message in the latency table when parameter capture is off.
20+
- Disabled toggles and guarded click handling keep interaction behavior aligned with configuration state.
21+
- Existing expanded-row state handling remains intact for enabled mode.
22+
23+
### Tests
24+
- Added/updated regression coverage:
25+
- `tests/unit/webui/test_server.py`
26+
- Quality gates pass:
27+
- `PYTHONPATH=src pytest` PASS (`637 passed, 5 skipped`)
28+
- `ruff check src/` PASS
29+
- `PYTHONPATH=src mypy src/` PASS (`Success: no issues found in 18 source files`)
30+
- `PYTHONPATH=src pytest --cov` PASS (`91.33%`, threshold 90%)
31+
32+
### Next Steps
33+
- FOLLOW-UP skipped: no actionable findings identified.

SPECS/INPROGRESS/next.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@
22

33
## Recently Archived
44

5+
- **BUG-T13** — Per-Tool Latency Statistics does not show params when `capture_params` is false (2026-02-25, PASS)
56
- **BUG-T11** — Chart Request Timeline never shows actual events (2026-02-25, PASS)
67
- **BUG-T12** — Audit Log does not show new calls (2026-02-20, PASS)
7-
- **BUG-T17** — Rows in Audit Log table automatically fold after user unfolds them (2026-02-20, PASS)
88

99
## Suggested Next Tasks
1010

11-
- BUG-T13 — Per-Tool Latency Statistics does not show params when `capture_params` is false
1211
- BUG-T19 — Audit Log and Session Timeline are inconsistent with tool charts in multi-process runs
1312
- BUG-T20 — Session Timeline can show negative duration due to incorrect entry ordering

SPECS/Workplan.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,9 +1395,10 @@ Export audit log via `/api/audit/export/json` or `/api/audit/export/csv` for a s
13951395

13961396
### BUG-T13: Per-Tool Latency Statistics does not show params when `capture_params` is false
13971397
- **Type:** Bug / Web UI / Configuration
1398-
- **Status:** 🔴 Open
1398+
- **Status:** ✅ Fixed (2026-02-25)
13991399
- **Priority:** P2
14001400
- **Discovered:** 2026-02-18
1401+
- **Completed:** 2026-02-25
14011402
- **Component:** Web UI Dashboard (`webui/static/`, per-tool latency table), `webui/config.py`
14021403
- **Affected Clients:** All clients using Web UI dashboard with default config
14031404
- **Affected Surface:** Per-Tool Latency Statistics table
@@ -1428,9 +1429,9 @@ No tooltip, label, or hint explains that capture_params must be enabled.
14281429
Enable parameter capture by passing `--web-ui-config` with `metrics.capture_params: true`. See [Web UI Dashboard docs](docs/webui-setup.md#using---web-ui-config-in-mcpjson).
14291430

14301431
#### Resolution Path
1431-
- [ ] Add a disabled-state hint in the Per-Tool Latency Statistics table when `capture_params` is false (e.g. greyed-out column with tooltip "Enable capture_params in webui config to see parameter data")
1432-
- [ ] Expose the current value of `capture_params` in the `/api/config` response (already done) and have the frontend read it to conditionally render the hint
1433-
- [ ] Add a test asserting the hint is present when `capture_params` is false
1432+
- [x] Add a disabled-state hint in the Per-Tool Latency Statistics table when `capture_params` is false (e.g. greyed-out column with tooltip "Enable capture_params in webui config to see parameter data")
1433+
- [x] Expose the current value of `capture_params` in the `/api/config` response (already done) and have the frontend read it to conditionally render the hint
1434+
- [x] Add a test asserting the hint is present when `capture_params` is false
14341435

14351436
#### Related Items
14361437
- **P12-T2** ✅ — Add Tool Parameter Frequency Analysis; the feature this bug surfaces

src/mcpbridge_wrapper/webui/static/dashboard.css

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,22 @@ kbd {
650650
color: var(--accent-blue);
651651
}
652652

653+
.param-toggle-btn-disabled {
654+
cursor: not-allowed;
655+
opacity: 0.5;
656+
}
657+
658+
.param-toggle-btn-disabled:hover {
659+
color: var(--text-secondary);
660+
}
661+
662+
.param-disabled-hint-row td {
663+
background: var(--bg-secondary);
664+
color: var(--text-secondary);
665+
font-size: 0.85rem;
666+
padding: 8px 12px;
667+
}
668+
653669
/* Expandable detail row for param patterns */
654670
.param-detail-row td {
655671
background: var(--bg-secondary);

src/mcpbridge_wrapper/webui/static/dashboard.js

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
var latencyExpandedRows = Object.create(null);
1414
var latestAuditRefreshRequest = 0;
1515
var lastSeenTotalRequests = null;
16+
var captureParamsEnabled = null;
17+
var latestToolLatencySummary = Object.create(null);
1618

1719
// --- Theme ---
1820
var THEME_COLORS = {
@@ -245,6 +247,34 @@
245247
return document.getElementById(id);
246248
}
247249

250+
function paramsCaptureDisabled() {
251+
return captureParamsEnabled === false;
252+
}
253+
254+
function renderCaptureParamsDisabledHint(tbody) {
255+
var hint = document.createElement("tr");
256+
hint.className = "param-disabled-hint-row";
257+
hint.innerHTML = "<td colspan='8'>Parameter pattern capture is disabled. Enable <code>metrics.capture_params: true</code> in Web UI config to view parameter data.</td>";
258+
tbody.appendChild(hint);
259+
}
260+
261+
function loadDashboardConfig() {
262+
fetch("/api/config")
263+
.then(function (r) { return r.json(); })
264+
.then(function (data) {
265+
var metricsConfig = data && typeof data === "object" ? data.metrics : null;
266+
if (metricsConfig && typeof metricsConfig.capture_params === "boolean") {
267+
captureParamsEnabled = metricsConfig.capture_params;
268+
} else {
269+
captureParamsEnabled = null;
270+
}
271+
updateLatencyTable(latestToolLatencySummary);
272+
})
273+
.catch(function () {
274+
captureParamsEnabled = null;
275+
});
276+
}
277+
248278
// --- Chart Initialization ---
249279
function initCharts() {
250280
// Tool usage bar chart
@@ -538,22 +568,32 @@
538568
Object.keys(latencyExpandedRows).forEach(function (tool) {
539569
expandedRows[tool] = true;
540570
});
571+
latestToolLatencySummary = toolLatency || {};
572+
var paramsDisabled = paramsCaptureDisabled();
541573
var nextExpandedRows = Object.create(null);
542574
tbody.innerHTML = "";
543-
var tools = Object.keys(toolLatency).sort();
575+
var tools = Object.keys(latestToolLatencySummary).sort();
544576
if (tools.length === 0) {
545577
latencyExpandedRows = Object.create(null);
546578
tbody.innerHTML = "<tr><td colspan='8' style='text-align:center;color:#8b949e'>No latency data</td></tr>";
547579
return;
548580
}
581+
if (paramsDisabled) {
582+
renderCaptureParamsDisabledHint(tbody);
583+
}
549584
tools.forEach(function (tool) {
550-
var s = toolLatency[tool];
585+
var s = latestToolLatencySummary[tool];
551586
var rowId = "param-row-" + tool.replace(/[^a-zA-Z0-9]/g, "_");
587+
var toggleHtml = "<button class='param-toggle-btn' data-tool='" + tool + "' "
588+
+ "data-target='" + rowId + "' title='Show parameter patterns' "
589+
+ "aria-expanded='false'>&#x25B6;</button> ";
590+
if (paramsDisabled) {
591+
toggleHtml = "<button class='param-toggle-btn param-toggle-btn-disabled' title='Enable metrics.capture_params to view parameter patterns' "
592+
+ "aria-disabled='true' disabled>&#x25B6;</button> ";
593+
}
552594
var tr = document.createElement("tr");
553595
tr.innerHTML = "<td>"
554-
+ "<button class='param-toggle-btn' data-tool='" + tool + "' "
555-
+ "data-target='" + rowId + "' title='Show parameter patterns' "
556-
+ "aria-expanded='false'>&#x25B6;</button> " + tool
596+
+ toggleHtml + tool
557597
+ "</td>"
558598
+ "<td>" + s.count + "</td>"
559599
+ "<td>" + s.avg_ms.toFixed(1) + "</td>"
@@ -568,11 +608,15 @@
568608
detailTr.id = rowId;
569609
detailTr.className = "param-detail-row";
570610
detailTr.style.display = "none";
611+
var detailHtml = "<em style='color:#8b949e'>Loading\u2026</em>";
612+
if (paramsDisabled) {
613+
detailHtml = "<em style='color:#8b949e'>Parameter capture is disabled. Enable <code>metrics.capture_params: true</code> in Web UI config.</em>";
614+
}
571615
detailTr.innerHTML = "<td colspan='8'><div class='param-patterns-container' id='patterns-" + rowId + "'>"
572-
+ "<em style='color:#8b949e'>Loading\u2026</em></div></td>";
616+
+ detailHtml + "</div></td>";
573617
tbody.appendChild(detailTr);
574618

575-
if (expandedRows[tool]) {
619+
if (!paramsDisabled && expandedRows[tool]) {
576620
detailTr.style.display = "";
577621
var toggleBtn = tr.querySelector(".param-toggle-btn");
578622
if (toggleBtn) {
@@ -593,7 +637,11 @@
593637
var container = document.getElementById(containerId);
594638
if (!container) return;
595639
if (!data.patterns || data.patterns.length === 0) {
596-
container.innerHTML = "<em style='color:#8b949e'>No parameter patterns captured. Enable <code>capture_params</code> in config.</em>";
640+
if (paramsCaptureDisabled()) {
641+
container.innerHTML = "<em style='color:#8b949e'>Parameter capture is disabled. Enable <code>metrics.capture_params: true</code> in Web UI config.</em>";
642+
} else {
643+
container.innerHTML = "<em style='color:#8b949e'>No parameter patterns captured yet.</em>";
644+
}
597645
return;
598646
}
599647
var html = "<table class='param-patterns-table'><thead><tr><th>Parameter Keys</th><th>Count</th></tr></thead><tbody>";
@@ -613,7 +661,8 @@
613661
updateKPIs(data.summary);
614662
updateToolCharts(data.summary.tool_counts);
615663
updateErrorBreakdownChart(data.summary.error_counts_by_code || {});
616-
updateLatencyTable(data.summary.tool_latency);
664+
latestToolLatencySummary = data.summary.tool_latency || {};
665+
updateLatencyTable(latestToolLatencySummary);
617666
updateTimeline(data.timeseries);
618667
updateLatencyChart(data.timeseries);
619668
if (data.sessions !== undefined) {
@@ -888,6 +937,7 @@
888937
el("latency-table").addEventListener("click", function (e) {
889938
var btn = e.target.closest(".param-toggle-btn");
890939
if (!btn) return;
940+
if (btn.disabled || btn.getAttribute("aria-disabled") === "true") return;
891941
var targetId = btn.getAttribute("data-target");
892942
var toolName = btn.getAttribute("data-tool");
893943
var detailRow = document.getElementById(targetId);
@@ -1079,6 +1129,7 @@
10791129
window.addEventListener("resize", updateDoughnutLegendLayout);
10801130
setupEventHandlers();
10811131
initKeyboardShortcuts();
1132+
loadDashboardConfig();
10821133
connectWebSocket();
10831134
startPolling();
10841135
loadAuditLogs();

tests/unit/webui/test_server.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ def test_get_config(self, client):
147147
data = response.json()
148148
assert "host" in data
149149
assert "port" in data
150+
assert "metrics" in data
151+
assert data["metrics"]["capture_params"] is False
150152
# Password should be masked
151153
assert data["auth"]["password"] == "********"
152154

@@ -243,12 +245,28 @@ def test_dashboard_js_preserves_latency_row_expansion_state(self, client):
243245
assert "var latencyExpandedRows = Object.create(null);" in response.text
244246
assert "function collectExpandedLatencyRows(tbody)" in response.text
245247
assert "Object.keys(latencyExpandedRows).forEach(function (tool) {" in response.text
246-
assert "if (expandedRows[tool]) {" in response.text
248+
assert "if (!paramsDisabled && expandedRows[tool]) {" in response.text
247249
assert "nextExpandedRows[tool] = true;" in response.text
248250
assert "latencyExpandedRows = nextExpandedRows;" in response.text
249251
assert "delete latencyExpandedRows[toolName];" in response.text
250252
assert "latencyExpandedRows[toolName] = true;" in response.text
251253

254+
def test_dashboard_js_shows_capture_params_disabled_hint(self, client):
255+
"""Latency table surfaces a disabled-state hint when capture_params is off."""
256+
response = client.get("/static/dashboard.js")
257+
assert response.status_code == 200
258+
assert "var captureParamsEnabled = null;" in response.text
259+
assert "function loadDashboardConfig() {" in response.text
260+
assert 'fetch("/api/config")' in response.text
261+
assert "function paramsCaptureDisabled() {" in response.text
262+
assert "renderCaptureParamsDisabledHint(tbody);" in response.text
263+
assert "param-disabled-hint-row" in response.text
264+
assert "param-toggle-btn param-toggle-btn-disabled" in response.text
265+
expected_disabled_guard = (
266+
'if (btn.disabled || btn.getAttribute("aria-disabled") === "true") return;'
267+
)
268+
assert expected_disabled_guard in response.text
269+
252270
def test_websocket_metrics_update_includes_sessions(self, client, audit):
253271
"""WebSocket metrics_update message includes sessions key."""
254272
with client.websocket_connect("/ws/metrics") as websocket:

0 commit comments

Comments
 (0)