diff --git a/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable.md b/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable.md new file mode 100644 index 00000000..8f9b7cd5 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable.md @@ -0,0 +1,58 @@ +# PRD: FU-P12-T2-1 — Fix stacking click event listeners in `updateLatencyTable` + +**Status:** In Progress +**Priority:** P3 +**Depends on:** P12-T2 ✅ +**Date:** 2026-02-16 + +--- + +## Problem + +`updateLatencyTable(toolLatency)` is called on every polling cycle (~2 s) from +`handleMetricsUpdate`. Each invocation clears `tbody.innerHTML` and rebuilds rows, +but it also calls `tbody.addEventListener("click", handler)` unconditionally. Since +`tbody` is the same DOM node across calls (only its children are replaced), each +call stacks another identical click handler on it. After N refresh cycles, a single +button click fires the toggle logic N times, triggering N `fetchParamPatterns` +requests for the same row. + +## Root Cause + +```js +// dashboard.js:332 — inside updateLatencyTable, called every ~2 s +tbody.addEventListener("click", function (e) { … }); +``` + +The listener is registered inside the function that is called on every poll, with no +guard against re-registration. + +## Fix + +Move the click listener to `setupEventHandlers()`, which runs once at init. Attach +it to `el("latency-table")` (the stable `` element) instead of `tbody`, using +the same event-delegation pattern (`e.target.closest(".param-toggle-btn")`). Remove +the `tbody.addEventListener(...)` block from `updateLatencyTable`. + +## Deliverables + +- `src/mcpbridge_wrapper/webui/static/dashboard.js` — listener moved to + `setupEventHandlers`, removed from `updateLatencyTable`. + +## Acceptance Criteria + +- `updateLatencyTable` contains no `addEventListener` call. +- `setupEventHandlers` registers exactly one delegated click handler on + `el("latency-table")`. +- Toggle expand/collapse behaviour is unchanged after the fix. +- `fetchParamPatterns` is called exactly once per button click regardless of how + many times `updateLatencyTable` has been called before. +- All existing tests pass (`pytest`, `ruff check src/`). + +## Out of Scope + +- Changes to `fetchParamPatterns`, row rendering, or any other part of the dashboard. + +--- +**Archived:** 2026-02-16 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Validation_Report.md b/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Validation_Report.md new file mode 100644 index 00000000..4b891305 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/FU-P12-T2-1_Validation_Report.md @@ -0,0 +1,41 @@ +# Validation Report: FU-P12-T2-1 + +**Task:** Fix stacking click event listeners in `updateLatencyTable` +**Date:** 2026-02-16 +**Verdict:** PASS + +--- + +## Change Summary + +Moved the delegated click handler for `.param-toggle-btn` out of +`updateLatencyTable` (which runs every ~2 s) into `setupEventHandlers` (which runs +once at init). The handler is now attached to `el("latency-table")` instead of +`tbody`, keeping the same event-delegation pattern. + +**File changed:** `src/mcpbridge_wrapper/webui/static/dashboard.js` +- Removed `tbody.addEventListener("click", …)` block from `updateLatencyTable` (lines 332-350 prior to fix) +- Added equivalent `el("latency-table").addEventListener("click", …)` in `setupEventHandlers` (lines 570-588) + +--- + +## Quality Gates + +| Gate | Result | +|------|--------| +| `pytest` | ✅ 465 passed, 5 skipped | +| `ruff check src/` | ✅ All checks passed | +| `pytest --cov` | ✅ 95.6% (≥ 90% required) | +| `mypy src/` | N/A — not configured | + +--- + +## Acceptance Criteria Verification + +| Criterion | Status | +|-----------|--------| +| `updateLatencyTable` contains no `addEventListener` call | ✅ | +| `setupEventHandlers` registers exactly one delegated click handler on `el("latency-table")` | ✅ | +| Toggle expand/collapse behaviour unchanged | ✅ (same handler logic) | +| `fetchParamPatterns` called exactly once per click regardless of prior refresh count | ✅ (single registered listener) | +| All existing tests pass | ✅ 465 passed | diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index b5d824b7..083c88dc 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-16 (FU-P11-T1-1) +**Last Updated:** 2026-02-16 ## Archived Tasks @@ -101,6 +101,7 @@ | FU-P11-T2-1 | [FU-P11-T2-1_Push_Session_Data_via_WebSocket/](FU-P11-T2-1_Push_Session_Data_via_WebSocket/) | 2026-02-16 | PASS | | FU-P11-T2-2 | [FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/](FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/) | 2026-02-16 | PASS | | FU-P11-T1-1 | [FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/](FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/) | 2026-02-16 | PASS | +| FU-P12-T2-1 | [FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/](FU-P12-T2-1_Fix_stacking_click_listeners_in_updateLatencyTable/) | 2026-02-16 | PASS | ## Historical Artifacts @@ -165,6 +166,7 @@ | [REVIEW_P12-T2_param_frequency_analysis.md](P12-T2_Add_Tool_Parameter_Frequency_Analysis/REVIEW_P12-T2_param_frequency_analysis.md) | Review report for P12-T2 | | [REVIEW_FU-P11-T2-2_limit_query_param.md](_Historical/REVIEW_FU-P11-T2-2_limit_query_param.md) | Review report for FU-P11-T2-2 | | [REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md](_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md) | Review report for FU-P11-T1-1 | +| [REVIEW_FU-P12-T2-1_stacking_click_listeners.md](_Historical/REVIEW_FU-P12-T2-1_stacking_click_listeners.md) | Review report for FU-P12-T2-1 | ## Archive Log @@ -284,3 +286,5 @@ | 2026-02-16 | FU-P11-T2-2 | Archived REVIEW_FU-P11-T2-2_limit_query_param report | | 2026-02-16 | FU-P11-T1-1 | Archived Refactor_FakeWebUIConfig_to_MagicMock (PASS) | | 2026-02-16 | FU-P11-T1-1 | Archived REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor report | +| 2026-02-16 | FU-P12-T2-1 | Archived Fix_stacking_click_listeners_in_updateLatencyTable (PASS) | +| 2026-02-16 | FU-P12-T2-1 | Archived REVIEW_FU-P12-T2-1_stacking_click_listeners report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T2-1_stacking_click_listeners.md b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T2-1_stacking_click_listeners.md new file mode 100644 index 00000000..9a361ff4 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P12-T2-1_stacking_click_listeners.md @@ -0,0 +1,53 @@ +## REVIEW REPORT — FU-P12-T2-1: Fix stacking click listeners in updateLatencyTable + +**Scope:** origin/main..HEAD +**Files:** 1 (dashboard.js) +**Date:** 2026-02-16 + +--- + +### Summary Verdict + +- [x] Approve + +--- + +### Critical Issues + +None. + +--- + +### Secondary Issues + +None. + +--- + +### Architectural Notes + +- The fix correctly moves the delegated click handler from `updateLatencyTable` + (called every ~2 s by the polling loop) into `setupEventHandlers` (called once at + `init()`), eliminating listener stacking entirely. +- Attaching to `el("latency-table")` (the stable `
` element) rather than + `tbody` (whose children are rebuilt on each poll) is the right approach: the table + element is present in the static HTML and survives all DOM rebuilds. +- Event delegation via `e.target.closest(".param-toggle-btn")` is preserved + unchanged, so the handler correctly targets dynamically created buttons regardless + of when they were inserted. +- The change is a pure refactor (no logic changes); the handler body is identical to + the original. + +--- + +### Tests + +- 465 tests pass; 5 skipped. Coverage 95.6% (requirement: ≥90%). +- No JS unit tests exist for dashboard.js — this is consistent with the existing + test suite which covers only the Python backend. No gap introduced. + +--- + +### Next Steps + +- No actionable issues found. FOLLOW-UP skipped. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index c419fe90..f1ba38c1 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -4,15 +4,14 @@ The previously selected task has been archived. ## Recently Archived +- 2026-02-16 — FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` (PASS) - 2026-02-16 — FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` (PASS) - 2026-02-16 — FU-P11-T2-2: Add `limit` query param to `GET /api/sessions` (PASS) - 2026-02-16 — FU-P11-T2-1: Push session data via WebSocket (PASS) - 2026-02-16 — P12-T2: Add Tool Parameter Frequency Analysis (PASS) -- 2026-02-15 — P11-T4: Add Keyboard Shortcuts & Command Palette (PASS) ## Suggested Next Tasks -- FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` (P3, depends on P12-T2 ✅) - FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (P3, depends on P12-T1 ✅) - FU-P12-T3-2: Add `error_code` column to audit CSV export (P3, depends on P12-T3 ✅) - FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (P3, depends on P12-T1 ✅) diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index eb0052f9..283fd3df 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -1993,7 +1993,7 @@ Phase 9 Follow-up Backlog --- -#### FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` +#### ✅ FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` - **Description:** The `tbody.addEventListener("click", ...)` call inside `updateLatencyTable()` in `dashboard.js` is re-registered on every metrics refresh (every 1 second), causing listeners to accumulate. Move the click handler to a one-time setup function called once during dashboard initialization, or guard with a `data-listener-attached` sentinel attribute on the tbody element. - **Priority:** P3 - **Dependencies:** P12-T2 diff --git a/src/mcpbridge_wrapper/webui/static/dashboard.js b/src/mcpbridge_wrapper/webui/static/dashboard.js index b8328439..9d76d386 100644 --- a/src/mcpbridge_wrapper/webui/static/dashboard.js +++ b/src/mcpbridge_wrapper/webui/static/dashboard.js @@ -328,26 +328,6 @@ + "Loading\u2026"; tbody.appendChild(detailTr); }); - - tbody.addEventListener("click", function (e) { - var btn = e.target.closest(".param-toggle-btn"); - if (!btn) return; - var targetId = btn.getAttribute("data-target"); - var toolName = btn.getAttribute("data-tool"); - var detailRow = document.getElementById(targetId); - if (!detailRow) return; - var isOpen = detailRow.style.display !== "none"; - if (isOpen) { - detailRow.style.display = "none"; - btn.innerHTML = "▶"; - btn.setAttribute("aria-expanded", "false"); - } else { - detailRow.style.display = ""; - btn.innerHTML = "▼"; - btn.setAttribute("aria-expanded", "true"); - fetchParamPatterns(toolName, "patterns-" + targetId); - } - }); } function fetchParamPatterns(toolName, containerId) { @@ -585,6 +565,26 @@ auditPage = 0; loadAuditLogs(); }); + + el("latency-table").addEventListener("click", function (e) { + var btn = e.target.closest(".param-toggle-btn"); + if (!btn) return; + var targetId = btn.getAttribute("data-target"); + var toolName = btn.getAttribute("data-tool"); + var detailRow = document.getElementById(targetId); + if (!detailRow) return; + var isOpen = detailRow.style.display !== "none"; + if (isOpen) { + detailRow.style.display = "none"; + btn.innerHTML = "▶"; + btn.setAttribute("aria-expanded", "false"); + } else { + detailRow.style.display = ""; + btn.innerHTML = "▼"; + btn.setAttribute("aria-expanded", "true"); + fetchParamPatterns(toolName, "patterns-" + targetId); + } + }); } // --- Session Timeline ---