diff --git a/SPECS/ARCHIVE/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.md b/SPECS/ARCHIVE/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.md new file mode 100644 index 00000000..985de061 --- /dev/null +++ b/SPECS/ARCHIVE/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.md @@ -0,0 +1,45 @@ +# PRD: BUG-T13 — Per-Tool Latency Statistics does not show params when `capture_params` is false + +## Objective +Improve the Per-Tool Latency Statistics UX so the dashboard clearly explains why parameter data is missing when parameter capture is disabled by configuration. + +## Background +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. + +## Deliverables +- Add a visible disabled-state hint in the Per-Tool Latency Statistics table when parameter capture is disabled. +- Keep the existing detailed parameter table behavior unchanged when parameter capture is enabled. +- Add regression tests that verify the disabled-state hint behavior in served dashboard frontend code. +- Update any related docs only if implementation text needs adjustment. + +## Dependencies +- Existing `/api/config` payload exposing `metrics.capture_params`. +- Existing per-tool latency table rendering logic in `src/mcpbridge_wrapper/webui/static/dashboard.js`. + +## Acceptance Criteria +- [ ] When `capture_params` is `false`, the Per-Tool Latency Statistics section shows an explicit hint that parameter capture is disabled. +- [ ] The hint includes clear enablement guidance (set `metrics.capture_params: true` via web UI config). +- [ ] When `capture_params` is `true`, the hint is hidden and normal parameter details continue to render. +- [ ] Frontend regression tests cover the disabled-state hint behavior. +- [ ] Required quality gates pass: `pytest`, `ruff check src/`, `mypy src/`, `pytest --cov` (coverage >= 90%). + +## Validation Plan +1. Add/extend frontend-oriented unit tests for dashboard rendering logic in `tests/unit/webui/test_server.py`. +2. Run required quality gates and capture outputs in `SPECS/INPROGRESS/BUG-T13_Validation_Report.md`. +3. Perform a brief manual read-through of generated dashboard HTML/JS responses for the disabled-state string and condition. + +## Implementation Plan +### Phase 1: Frontend disabled-state messaging +- Update per-tool latency table rendering logic in `dashboard.js` to conditionally render a disabled-state row/message when parameter capture is disabled. +- Ensure the message is scoped to parameter detail content and does not affect latency/call count metrics. + +### Phase 2: Backend configuration wiring verification +- Confirm dashboard config payload consumption already provides `capture_params` and wire the condition in rendering code if needed. + +### Phase 3: Regression coverage and quality gates +- Add assertions in tests for disabled/enabled rendering states. +- Run quality gates and record evidence. + +--- +**Archived:** 2026-02-25 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false/BUG-T13_Validation_Report.md b/SPECS/ARCHIVE/BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false/BUG-T13_Validation_Report.md new file mode 100644 index 00000000..acd78bcf --- /dev/null +++ b/SPECS/ARCHIVE/BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false/BUG-T13_Validation_Report.md @@ -0,0 +1,45 @@ +# Validation Report: BUG-T13 + +## Task +Per-Tool Latency Statistics does not show params when `capture_params` is false. + +## Implementation Summary +- Added a dashboard config fetch path in `dashboard.js` to read `/api/config` and track `metrics.capture_params` on the frontend. +- Added a table-level disabled-state hint row for Per-Tool Latency Statistics when parameter capture is disabled. +- Disabled parameter toggle buttons in that state and guarded click handling against disabled toggles. +- Updated empty-pattern messaging to distinguish between: + - capture disabled (`metrics.capture_params: true` guidance), and + - capture enabled but no data yet. +- Added styling for disabled toggle state and the disabled-hint row. +- Added regression assertions in `tests/unit/webui/test_server.py` covering: + - config exposure of `metrics.capture_params`, + - frontend config fetch and conditional disabled-state rendering logic. + +## Quality Gates + +### 1) `PYTHONPATH=src pytest` +- Result: PASS +- Evidence: `637 passed, 5 skipped` + +### 2) `ruff check src/` +- Result: PASS +- Evidence: `All checks passed!` + +### 3) `PYTHONPATH=src mypy src/` +- Result: PASS +- Evidence: `Success: no issues found in 18 source files` + +### 4) `PYTHONPATH=src pytest --cov` +- Result: PASS +- Evidence: + - `637 passed, 5 skipped` + - `Required test coverage of 90.0% reached` + - `Total coverage: 91.33%` + +## Manual Validation Notes +- When `capture_params` is disabled, the latency table now immediately shows a clear configuration hint instead of only surfacing messaging after row interaction. +- Param toggle controls are non-interactive in disabled mode to match visible state. +- Existing expanded-row behavior for enabled mode remains covered by regression assertions. + +## Verdict +PASS diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 0c5f29ea..9331d5d0 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,11 +1,12 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-25 (BUG-T11_Chart_Request_Timeline_never_shows_actual_events) +**Last Updated:** 2026-02-25 (BUG-T13_Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false) ## Archived Tasks | Task ID | Folder | Archived | Verdict | |---------|--------|----------|---------| +| 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 | | BUG-T11 | [BUG-T11_Chart_Request_Timeline_never_shows_actual_events/](BUG-T11_Chart_Request_Timeline_never_shows_actual_events/) | 2026-02-25 | PASS | | BUG-T12 | [BUG-T12_Audit_Log_does_not_show_new_calls/](BUG-T12_Audit_Log_does_not_show_new_calls/) | 2026-02-20 | PASS | | 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 @@ | [REVIEW_bug_t18_workplan_entry.md](_Historical/REVIEW_bug_t18_workplan_entry.md) | Review report for BUG-T18 | | [REVIEW_bug_t17_audit_log_rows_stay_unfolded.md](_Historical/REVIEW_bug_t17_audit_log_rows_stay_unfolded.md) | Review report for BUG-T17 | | [REVIEW_bug_t14_latency_rows.md](_Historical/REVIEW_bug_t14_latency_rows.md) | Review report for BUG-T14 | +| [REVIEW_bug_t13_capture_params_hint.md](_Historical/REVIEW_bug_t13_capture_params_hint.md) | Review report for BUG-T13 | | [REVIEW_bug_t11_request_timeline.md](_Historical/REVIEW_bug_t11_request_timeline.md) | Review report for BUG-T11 | | [REVIEW_bug_t10.md](_Historical/REVIEW_bug_t10.md) | Review report for BUG-T10 | @@ -259,6 +261,8 @@ | Date | Task ID | Action | |------|---------|--------| +| 2026-02-25 | BUG-T13 | Archived REVIEW_bug_t13_capture_params_hint report | +| 2026-02-25 | BUG-T13 | Archived Per-Tool_Latency_Statistics_does_not_show_params_when_capture_params_is_false (PASS) | | 2026-02-25 | BUG-T11 | Archived REVIEW_bug_t11_request_timeline report | | 2026-02-25 | BUG-T11 | Archived Chart_Request_Timeline_never_shows_actual_events (PASS) | | 2026-02-20 | BUG-T12 | Archived REVIEW_BUG-T12_audit_log_live_updates report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_bug_t13_capture_params_hint.md b/SPECS/ARCHIVE/_Historical/REVIEW_bug_t13_capture_params_hint.md new file mode 100644 index 00000000..02f05705 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_bug_t13_capture_params_hint.md @@ -0,0 +1,33 @@ +## REVIEW REPORT — BUG-T13 Capture Params Hint + +**Scope:** origin/main..HEAD +**Files:** 8 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues +- None. + +### Secondary Issues +- None. + +### Architectural Notes +- 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. +- Disabled toggles and guarded click handling keep interaction behavior aligned with configuration state. +- Existing expanded-row state handling remains intact for enabled mode. + +### Tests +- Added/updated regression coverage: + - `tests/unit/webui/test_server.py` +- Quality gates pass: + - `PYTHONPATH=src pytest` PASS (`637 passed, 5 skipped`) + - `ruff check src/` PASS + - `PYTHONPATH=src mypy src/` PASS (`Success: no issues found in 18 source files`) + - `PYTHONPATH=src pytest --cov` PASS (`91.33%`, threshold 90%) + +### Next Steps +- FOLLOW-UP skipped: no actionable findings identified. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index bff6cc1c..bee0d014 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -2,12 +2,11 @@ ## Recently Archived +- **BUG-T13** — Per-Tool Latency Statistics does not show params when `capture_params` is false (2026-02-25, PASS) - **BUG-T11** — Chart Request Timeline never shows actual events (2026-02-25, PASS) - **BUG-T12** — Audit Log does not show new calls (2026-02-20, PASS) -- **BUG-T17** — Rows in Audit Log table automatically fold after user unfolds them (2026-02-20, PASS) ## Suggested Next Tasks -- BUG-T13 — Per-Tool Latency Statistics does not show params when `capture_params` is false - BUG-T19 — Audit Log and Session Timeline are inconsistent with tool charts in multi-process runs - BUG-T20 — Session Timeline can show negative duration due to incorrect entry ordering diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 1ca6d88f..7a607b00 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -1395,9 +1395,10 @@ Export audit log via `/api/audit/export/json` or `/api/audit/export/csv` for a s ### BUG-T13: Per-Tool Latency Statistics does not show params when `capture_params` is false - **Type:** Bug / Web UI / Configuration -- **Status:** 🔴 Open +- **Status:** ✅ Fixed (2026-02-25) - **Priority:** P2 - **Discovered:** 2026-02-18 +- **Completed:** 2026-02-25 - **Component:** Web UI Dashboard (`webui/static/`, per-tool latency table), `webui/config.py` - **Affected Clients:** All clients using Web UI dashboard with default config - **Affected Surface:** Per-Tool Latency Statistics table @@ -1428,9 +1429,9 @@ No tooltip, label, or hint explains that capture_params must be enabled. 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). #### Resolution Path -- [ ] 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") -- [ ] 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 -- [ ] Add a test asserting the hint is present when `capture_params` is false +- [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") +- [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 +- [x] Add a test asserting the hint is present when `capture_params` is false #### Related Items - **P12-T2** ✅ — Add Tool Parameter Frequency Analysis; the feature this bug surfaces diff --git a/src/mcpbridge_wrapper/webui/static/dashboard.css b/src/mcpbridge_wrapper/webui/static/dashboard.css index b3901288..ee67d4ea 100644 --- a/src/mcpbridge_wrapper/webui/static/dashboard.css +++ b/src/mcpbridge_wrapper/webui/static/dashboard.css @@ -650,6 +650,22 @@ kbd { color: var(--accent-blue); } +.param-toggle-btn-disabled { + cursor: not-allowed; + opacity: 0.5; +} + +.param-toggle-btn-disabled:hover { + color: var(--text-secondary); +} + +.param-disabled-hint-row td { + background: var(--bg-secondary); + color: var(--text-secondary); + font-size: 0.85rem; + padding: 8px 12px; +} + /* Expandable detail row for param patterns */ .param-detail-row td { background: var(--bg-secondary); diff --git a/src/mcpbridge_wrapper/webui/static/dashboard.js b/src/mcpbridge_wrapper/webui/static/dashboard.js index 329c43a9..2229e06f 100644 --- a/src/mcpbridge_wrapper/webui/static/dashboard.js +++ b/src/mcpbridge_wrapper/webui/static/dashboard.js @@ -13,6 +13,8 @@ var latencyExpandedRows = Object.create(null); var latestAuditRefreshRequest = 0; var lastSeenTotalRequests = null; + var captureParamsEnabled = null; + var latestToolLatencySummary = Object.create(null); // --- Theme --- var THEME_COLORS = { @@ -245,6 +247,34 @@ return document.getElementById(id); } + function paramsCaptureDisabled() { + return captureParamsEnabled === false; + } + + function renderCaptureParamsDisabledHint(tbody) { + var hint = document.createElement("tr"); + hint.className = "param-disabled-hint-row"; + hint.innerHTML = "Parameter pattern capture is disabled. Enable metrics.capture_params: true in Web UI config to view parameter data."; + tbody.appendChild(hint); + } + + function loadDashboardConfig() { + fetch("/api/config") + .then(function (r) { return r.json(); }) + .then(function (data) { + var metricsConfig = data && typeof data === "object" ? data.metrics : null; + if (metricsConfig && typeof metricsConfig.capture_params === "boolean") { + captureParamsEnabled = metricsConfig.capture_params; + } else { + captureParamsEnabled = null; + } + updateLatencyTable(latestToolLatencySummary); + }) + .catch(function () { + captureParamsEnabled = null; + }); + } + // --- Chart Initialization --- function initCharts() { // Tool usage bar chart @@ -538,22 +568,32 @@ Object.keys(latencyExpandedRows).forEach(function (tool) { expandedRows[tool] = true; }); + latestToolLatencySummary = toolLatency || {}; + var paramsDisabled = paramsCaptureDisabled(); var nextExpandedRows = Object.create(null); tbody.innerHTML = ""; - var tools = Object.keys(toolLatency).sort(); + var tools = Object.keys(latestToolLatencySummary).sort(); if (tools.length === 0) { latencyExpandedRows = Object.create(null); tbody.innerHTML = "No latency data"; return; } + if (paramsDisabled) { + renderCaptureParamsDisabledHint(tbody); + } tools.forEach(function (tool) { - var s = toolLatency[tool]; + var s = latestToolLatencySummary[tool]; var rowId = "param-row-" + tool.replace(/[^a-zA-Z0-9]/g, "_"); + var toggleHtml = " "; + if (paramsDisabled) { + toggleHtml = " "; + } var tr = document.createElement("tr"); tr.innerHTML = "" - + " " + tool + + toggleHtml + tool + "" + "" + s.count + "" + "" + s.avg_ms.toFixed(1) + "" @@ -568,11 +608,15 @@ detailTr.id = rowId; detailTr.className = "param-detail-row"; detailTr.style.display = "none"; + var detailHtml = "Loading\u2026"; + if (paramsDisabled) { + detailHtml = "Parameter capture is disabled. Enable metrics.capture_params: true in Web UI config."; + } detailTr.innerHTML = "
" - + "Loading\u2026
"; + + detailHtml + ""; tbody.appendChild(detailTr); - if (expandedRows[tool]) { + if (!paramsDisabled && expandedRows[tool]) { detailTr.style.display = ""; var toggleBtn = tr.querySelector(".param-toggle-btn"); if (toggleBtn) { @@ -593,7 +637,11 @@ var container = document.getElementById(containerId); if (!container) return; if (!data.patterns || data.patterns.length === 0) { - container.innerHTML = "No parameter patterns captured. Enable capture_params in config."; + if (paramsCaptureDisabled()) { + container.innerHTML = "Parameter capture is disabled. Enable metrics.capture_params: true in Web UI config."; + } else { + container.innerHTML = "No parameter patterns captured yet."; + } return; } var html = ""; @@ -613,7 +661,8 @@ updateKPIs(data.summary); updateToolCharts(data.summary.tool_counts); updateErrorBreakdownChart(data.summary.error_counts_by_code || {}); - updateLatencyTable(data.summary.tool_latency); + latestToolLatencySummary = data.summary.tool_latency || {}; + updateLatencyTable(latestToolLatencySummary); updateTimeline(data.timeseries); updateLatencyChart(data.timeseries); if (data.sessions !== undefined) { @@ -888,6 +937,7 @@ el("latency-table").addEventListener("click", function (e) { var btn = e.target.closest(".param-toggle-btn"); if (!btn) return; + if (btn.disabled || btn.getAttribute("aria-disabled") === "true") return; var targetId = btn.getAttribute("data-target"); var toolName = btn.getAttribute("data-tool"); var detailRow = document.getElementById(targetId); @@ -1079,6 +1129,7 @@ window.addEventListener("resize", updateDoughnutLegendLayout); setupEventHandlers(); initKeyboardShortcuts(); + loadDashboardConfig(); connectWebSocket(); startPolling(); loadAuditLogs(); diff --git a/tests/unit/webui/test_server.py b/tests/unit/webui/test_server.py index 596a7a1a..b44911dd 100644 --- a/tests/unit/webui/test_server.py +++ b/tests/unit/webui/test_server.py @@ -147,6 +147,8 @@ def test_get_config(self, client): data = response.json() assert "host" in data assert "port" in data + assert "metrics" in data + assert data["metrics"]["capture_params"] is False # Password should be masked assert data["auth"]["password"] == "********" @@ -243,12 +245,28 @@ def test_dashboard_js_preserves_latency_row_expansion_state(self, client): assert "var latencyExpandedRows = Object.create(null);" in response.text assert "function collectExpandedLatencyRows(tbody)" in response.text assert "Object.keys(latencyExpandedRows).forEach(function (tool) {" in response.text - assert "if (expandedRows[tool]) {" in response.text + assert "if (!paramsDisabled && expandedRows[tool]) {" in response.text assert "nextExpandedRows[tool] = true;" in response.text assert "latencyExpandedRows = nextExpandedRows;" in response.text assert "delete latencyExpandedRows[toolName];" in response.text assert "latencyExpandedRows[toolName] = true;" in response.text + def test_dashboard_js_shows_capture_params_disabled_hint(self, client): + """Latency table surfaces a disabled-state hint when capture_params is off.""" + response = client.get("/static/dashboard.js") + assert response.status_code == 200 + assert "var captureParamsEnabled = null;" in response.text + assert "function loadDashboardConfig() {" in response.text + assert 'fetch("/api/config")' in response.text + assert "function paramsCaptureDisabled() {" in response.text + assert "renderCaptureParamsDisabledHint(tbody);" in response.text + assert "param-disabled-hint-row" in response.text + assert "param-toggle-btn param-toggle-btn-disabled" in response.text + expected_disabled_guard = ( + 'if (btn.disabled || btn.getAttribute("aria-disabled") === "true") return;' + ) + assert expected_disabled_guard in response.text + def test_websocket_metrics_update_includes_sessions(self, client, audit): """WebSocket metrics_update message includes sessions key.""" with client.websocket_connect("/ws/metrics") as websocket:
Parameter KeysCount