Repository navigation
fix(webui): make the Activity and Usage numbers say what they mean - #1052
Merged
Merged
Conversation
Audit findings F2, F5, F22, F23, F25, F26, F27 and the Activity/Usage part of F36 (#1046). Builds on #1048's storage.CountsAsCall contract. F2 — the status tiles are a partition again. ActivitySummaryResponse gains other_count: the rows whose status is outside the tool-call vocabulary (a quarantine change stores its action there, a policy decision its verdict). They were in the total and in no tile, so the row read 15+4+0+0 under a 42. The tile list now lives in one pure function, an "Other / internal" tile renders the residual, it filters like every other tile, and Go + Vitest tests assert the sum. F5 — repeats fold. Consecutive rows that agree on everything the collapsed line prints become one expandable run. Status is part of the run identity, so a run can never mix outcomes and "echo x12" can never hide the one call that failed; the sensitive-data flag and the code_execution parent link are in the identity for the same reason. Folding is on by default, remembered, and off whenever the table is sorted by something other than time. Successful rows no longer print "Success" at all — the word stays for screen readers only, so the marked rows in the column are exactly the ones needing a human. F22 — the usage charts stop inventing tools and stop quantizing latency. Names that never completed a call are excluded from "Calls per tool" and "Token sinks" (named in a footnote, still charted under errors, and counted the same way by the "active tools" tile). latencyBucketBoundsMs gains sub-10ms resolution, which is where local MCP servers live and where the old first bucket had none; Percentile now reports whether it hit the unbounded overflow bucket, so a figure renders as a bound ("<= 5 ms") or a floor ("> 10 s") and never as a stopwatch reading. Every chart axis says what it measures. F23 — the tokens-saved headline explains itself. It is a structural estimate over the current tool catalog, so it correctly does not move per call; the label said otherwise. Relabelled per-request, given the derivation on hover, and the Dashboard chip now opens the breakdown that computes it. F25 — a security_scan drawer shows its verdict and findings by severity and offers the server and the Security page. A session id is now a control: filter this log, or open Sessions. F26 — the Intent column paints the operation word beside the glyph, the Sensitive badge carries an accessible name, and the filter panel holds a visible legend for both. F27 — the Connected tile names its denominator ("2 of 3 enabled online"): a switched-off server is not an outage. serverCount.disabled is now asked directly and excludes quarantined servers, so the Dashboard rail stops reporting one server as two problems. F36 — the Sessions "View Activity" button stops clipping its label.
Deploying mcpproxy-docs with
|
| Latest commit: |
13e6646
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5be7a89d.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-ux-activity-usage.mcpproxy-docs.pages.dev |
Cross-model review found three ways a run could fold away something the collapsed line was displaying on the members' behalf: - the intent OPERATION TYPE was not in the key, so a run of call_tool_write calls against a tool could fold under a "read" lead — the two produce records identical in type, server, tool and status. - only the sensitive-data BOOLEAN was in the key, but the badge prints the severity glyph and the detection count, so a critical detection could fold under a low-severity lead. - the key was a space-joined string over free-form fields (server name, tool name, details text), so two different rows could agree on one joined value. It is JSON.stringify now. The reason stays out of the key on purpose — that is what reasonsVary reports. Four regression tests, one per hazard.
… page
Three more from cross-model review.
The settled-scan event carries its rollup as a map[string]int, and
handleSecurityScanSettled read it with a map[string]interface{} type
assertion that never matched — so findings_summary was dropped from
every security_scan record ever written. Nothing noticed because
nothing rendered it; F25's new drawer does. The read now takes the
concrete type (and a JSON round-tripped one), and keeps an EMPTY rollup,
because "the scanners found nothing" and "this record has no summary"
are opposite claims and the drawer must not print the first when it only
knows the second. It now says so, and points at the Security page.
The Activity paginator never clamped currentPage, so folding repeats on
while on a later page — or any refresh that shrinks the list — rendered
an empty table with no hint why. Clamped on the derived page count, so
every path is covered by one rule. Expanded runs and the page now also
reset on a sort, session-filter or grouping change; a run key is a lead
row's id, and a stale one re-expands the wrong run.
The Usage "active tools" figure counts what is charted, so it now admits
the folded tail when the server truncated the list.
Cross-model review, round three. Exporting from the "Other / internal" view sent that synthetic status to an endpoint that matches the closed vocabulary exactly, so the file came back empty — which reads as "there was nothing there". The export drops the status filter instead; a wider export is recoverable. Sorting by p95 could not tell a tool BOUNDED at the last histogram bucket from one that ran PAST it, and the response is truncated to top-N, so the genuinely slow tool could be dropped in favour of one that merely touched the ceiling. The overflow flag now breaks that tie. A failed scan can still carry findings: the debouncer keeps the last non-nil rollup, so a storm where some scanners finished and a later one failed settles as "failed" with real findings attached. The drawer shows them — hiding real findings is the worse error in a security view — and labels them as partial rather than as the scan's verdict.
Member
Author
Cross-model review (opencode / gpt Sol) — 4 roundsRound 1 (backend + shared utils) and rounds 2–3 (views, then the whole diff) found nine issues; all nine are fixed in commits 2–4. The two that mattered most were not in the audit at all: The scan findings summary never reached the record. Three ways a run could fold away what the collapsed row was displaying. The intent operation type was not in the key (a run of Also fixed: Rejected, with reasons
Round 4 returned only the last two, i.e. no new defects. |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 32879945287 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
The document read as an open P0-P3 backlog, but seven PRs merged on 2026-08-25 (#1044, #1048, #1049, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates three things honestly: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Still open: F6 is half fixed. Focus enters the Add Server dialog and submit is in a sticky footer, but Escape still does not close it - AddServerModal.vue has no key handler and JsonViewer.vue is the only component under components/ that handles Escape at all. F28's "key called optional" half is fixed; its three-surfaces state is not. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. Five defects found on main while re-checking are recorded separately, since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
The document read as an open P0-P3 backlog, but eight PRs merged on 2026-08-25 (#1044, #1048, #1049, #1050, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates four things: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F6, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. F28 is called out as partial - its "key called optional" half is fixed, its three-surfaces-at-once state is not. - Corrections to the audit itself: F23 mis-diagnosed a per-request structural estimate as a stale aggregation, and F31's "Manual badge has no tooltip" half was wrong even at audit time (spec-088 trust-mode badge, already titled). - Five defects found on main while re-checking, recorded separately since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. F6 carries a methodology note. Re-checking it produced a false negative: Escape driven through the browser-automation layer never reaches the page's document keydown listener, so the dialog looked stuck open. Dispatched in-page it closes at once, and #1050's useModalA11y is wired into all five <dialog :open> modals. The sweep assertion this document proposes must dispatch in-page, or it tests the automation layer instead of the app. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
Dumbris
added a commit
that referenced
this pull request
Aug 26, 2026
* docs(qa): Web UI UX audit — ranked findings (ux-audit epic) Heuristic + live-instance audit of every routed Web UI view in light/dark, first-run and populated states, at 1440/820/390px. 36 findings ranked P0-P3 with view, evidence, heuristic violated, suggested fix and effort. Audited against standard heuristics plus the house activity-UI rules (grouping, failure-only status, intent reason, histogram/log parity). * docs(qa): record what the Web UI audit's backlog actually resolved to The document read as an open P0-P3 backlog, but eight PRs merged on 2026-08-25 (#1044, #1048, #1049, #1050, #1051, #1052, #1053, #1054) closed it. Anyone opening this file against current main was being handed 36 stale defects. Added a status banner and a Resolution section that separates four things: - Re-checked live on v0.61.0 against a running isolated core with real /mcp traffic: F1, F4, F5, F6, F7, F8, F12, F13, F20, F24, F26, F33 confirmed fixed, each with the observed evidence rather than a PR reference. - Not re-checked: named explicitly rather than implied, so the next reader knows which closures rest on a PR body and which on an observation. F28 is called out as partial - its "key called optional" half is fixed, its three-surfaces-at-once state is not. - Corrections to the audit itself: F23 mis-diagnosed a per-request structural estimate as a stale aggregation, and F31's "Manual badge has no tooltip" half was wrong even at audit time (spec-088 trust-mode badge, already titled). - Five defects found on main while re-checking, recorded separately since they are not regressions of anything in the audit. The first is security-relevant: quarantining a server through the config file leaves its tools in the search index, so retrieve_tools still hands the agent their names and descriptions - which is exactly what quarantine exists to prevent - before the call is refused. Runtime.QuarantineServer purges the index; the reload path detects the same transition (quarantined_changed: true) and does not. F6 carries a methodology note. Re-checking it produced a false negative: Escape driven through the browser-automation layer never reaches the page's document keydown listener, so the dialog looked stuck open. Dispatched in-page it closes at once, and #1050's useModalA11y is wired into all five <dialog :open> modals. The sweep assertion this document proposes must dispatch in-page, or it tests the automation layer instead of the app. Also corrects the record on #1053: its body names F1-F16, which is the tray audit's numbering mis-pasted. #1055 is the tray PR; #1053 touches no Swift file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the Activity & Usage cluster of the Web-UI UX audit (#1046): F2, F5, F22, F23, F25, F26, F27, plus the Activity/Usage item in F36. Builds on the
storage.CountsAsCallcontract landed by #1048.Every finding below was verified live with Playwright against an isolated instance (scratch
--config+--data-dir, port 18491, four servers, ~120 activity rows of generated traffic).F2 — the status tiles did not sum
Total (24h) 42 · Success 15 · Errors 4 · Blocked 0 · Rejected 0— four buckets adding to 19 under a denominator of 42.The four are the tool-call status vocabulary, but the activity log is wider than tool calls: a quarantine change stores its action in
status(tool_auto_approved), a policy decision its verdict (allow). Those rows were in the total and in no bucket.ActivitySummaryResponsenow carriesother_count, computed as thedefaultbranch of the same switch, so the partition holds by construction. The tile list moved into one pure function (statusBucketTiles), an "Other / internal" tile renders the residual with a tooltip explaining what it holds, and it filters like every other tile. A Go test and a Vitest test both assert the sum; live:23 + 6 + 2 + 0 + 23 = 54.F5 — repeats did not group, and every row said "Success"
Consecutive rows that agree on everything the collapsed line prints now fold into one expandable run. The rule that makes the compression safe is that the run identity covers every field the lead row displays on the members' behalf — so status is in it (a run can never mix outcomes;
echo ×12can never hide the one call that failed), and so are the intent operation type and the sensitive-data severity and count. The intent reason is the one displayed field deliberately left out — agents reword it call to call, which would defeat the compression on exactly the logs that need it — so a run whose members disagree shows a+…marker instead.Folding is on by default, remembered per browser, and disabled whenever the table is sorted by something other than time — adjacency only means "repeated" in time order. The paginator counts runs and names the rows behind them.
Successes no longer print "Success" at all: the word is
sr-only, so the marked rows in the Status column are exactly the ones needing a human. Live: 12 identical echo calls with a failure at position 8 rendered asecho ×9/doesnotexist(the only red chip) /echo ×7.F22 — the usage charts invented tools and quantized latency
Invented tools.
everything:doesnotexistandbroken-remote:whateverwere charted as first-class tools with 100% error rates. Those names came from failed calls — an agent's typo, a server that was never reachable. A name that has never once completed a call is excluded from Calls per tool and Token sinks (an error message is a response with bytes, which is how a name that only ever failed ranked as a "token sink"), named in a footnote, and still charted under Errors & latency, where it belongs. The "active tools" tile counts the same population, and admits the folded tail when the server truncates the list. Self-correcting: one success and it reappears.Latency. Every row of the p50/p95 table read exactly
10 mswhile the Activity Log showed 3/4/5 ms for the same calls. A histogram percentile is the upper bound of a bucket, and the first bound was 10 ms — the band local MCP servers live entirely inside.latencyBucketBoundsMsgains1, 2, 5, andPercentilenow also reports whether it landed in the unbounded overflow bucket, so the table renders≤ 5 msor> 10 sand can never be read as a stopwatch again.usageAdmissionVersionis bumped to 5 (the stamp now covers bucket layout, not just admission) so persisted snapshots rebuild rather than carry indices that changed meaning.Axes. Every chart names what it measures: Calls (cumulative), Total response bytes, Error rate (% of calls), plus a line separating the error bars from the latency table beneath them.
Live:
≤ 10 ms / ≤ 5 ms / ≤ 5 ms / ≤ 1 ms / —where the whole column used to read10 ms.F23 — the token-savings headline did not move
It is not supposed to. Tracing the aggregation path:
CalculateTokenSavingsis a structural estimate over the current tool catalog — what it would cost to put every upstream tool in the agent's context versus whatretrieve_toolsreturns for one query. It moves when servers or tools change, never per call. The number was right; the label ("Tokens saved", beside per-window call counts) said cumulative-savings-so-far.Relabelled to Tokens saved per request / smaller tool context per request, given the derivation on hover, and the Dashboard chip is now a control that opens the Token Savings breakdown that computes it.
F25 — the drawer for internal events was a dead end
A
security_scanrow now renders its verdict, its findings by severity, and two ways out (the server, and the Security page). A session id is now a control rather than plain text: Filter this log (preferring the Spec 082 work session) and Open Sessions — Sessions has linked into Activity since 082 and nothing linked back.Building it uncovered why the drawer had nothing to show. The findings summary never reached the record at all:
publishScanSettledputs amap[string]intin the payload andhandleSecurityScanSettledread it with amap[string]interface{}assertion that never matched, so everysecurity_scanrecord ever written hasmetadata: null. Fixed at the read, keeping an empty rollup distinct from an absent one — "the scanners found nothing" and "this record has no summary" are opposite claims, and the drawer must not print the first when it only knows the second. Verified live: a freshly triggered scan now writes{"findings_summary": {}}.F26 — Intent and Sensitive were unlabelled emoji
The Intent column paints the operation word beside the glyph (
📖 read, not📖), the Sensitive badge gains an accessible name, and the filter panel carries a visible legend for both vocabularies. Colour + glyph is no longer the sole encoding (WCAG 1.4.1).F27 — an ambiguous denominator and a double count
Connected read
50% onlinefor 2 of 4 servers, one switched off and one quarantined; a server that is not enabled cannot be online. The tile now states its denominator: "2 of 3 enabled online". (Quarantined servers stay in it:lifecycle.gokeeps them connected for security inspection, so they are supposed to be up.)serverCount.disabledis a new count asked directly — switched off and not quarantined — replacingtotal − connected − quarantined, which was wrong twice: it double-counted a server that was both (the Dashboard rail read one server as two problems) and it labelled an enabled but unreachable server "disabled", hiding the actual fault behind an administrative word.F36 —
View Activityclippedwhitespace-nowrap. Verified:scrollW == clientW, one line.Tests
Percentilecases (sub-10 ms resolution, the overflow floor, an empty histogram, the existing tail-latency case); the p95 overflow tie-break; and the scan-payload reader, including the empty-vs-absent distinction.vue-tscclean,golangci-lintv2 clean, both editions build,internal/{httpapi,runtime,contracts,storage}green under-race.scripts/test-api-e2e.sh— its cleanup blanket-pkills cores it does not own, and other sessions had cores live.Cross-model review
Four rounds (opencode / gpt Sol). Nine findings fixed — see the review note for the full list, including the two that were not in the audit (the dropped scan summary above, and three ways a run could fold away a field it was displaying). Three findings were checked and rejected, with reasons.
Two accepted trade-offs, flagged for a human
everything:add). Intended: it is not evidence of use, it is named in the footnote, and it stays in the errors chart. This is the judgement call most worth a second opinion.API surface
Additive:
ActivitySummaryResponse.other_count,UsageToolStat.p50_exceeds/p95_exceeds.oas/regenerated.