Repository navigation
fix(activity): count calls once, so Activity and Usage agree - #1048
Merged
Merged
Conversation
The Activity Log header and the Dashboard's Usage tab reported different
totals for the same 24-hour window on the same instance. On a repro
instance the same traffic produced three numbers within seconds:
Activity header 51 calls (every activity record in the window)
Usage "Activity over time" 25 (calls the user made)
Usage "Tool calls" tile 19 (a client-side sum of `tools`)
The two Usage numbers sat on the same screen, one directly above the
other.
Root causes, one per surface:
* handleActivitySummary counted every record type as the header's
total, and the frontend labelled that total "N calls". Security
scans, quarantine auto-approvals and system starts are rows in the
log, not calls.
* Usage.vue summed `tools` client-side. That list is
lifetime-cumulative (the window only filters which tools appear),
upstream-only (mcpproxy's own retrieve_tools/describe_tool calls
vanish) and truncated to top-N (the tail is silently dropped).
Both now draw on one definition — storage.CountsAsCall — which the usage
aggregate already applied implicitly and now consults explicitly, so the
timeline, the summary endpoint, the Web UI and the CLI cannot drift
apart. The summary gains call_count/call_error_count; the usage response
gains total_calls/total_errors, computed server-side as the sum of the
timeline it carries. The event total is still reported, under its own
name, because it is the paginator's denominator.
After, on the same instance: Activity reads "83 events · 50 calls · 12
errors" and Usage reads "Calls 50 · Errors 12 · 24.0%".
Audit finding F1 (also F24) — #1046
Deploying mcpproxy-docs with
|
| Latest commit: |
967825a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://94dbccae.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-f1-activity-usage-call-p.mcpproxy-docs.pages.dev |
ActivityFilter.ExcludeCallToolSuccess hides EVERY call_tool_* record, not only successful ones — a dispatched call emits its own tool_call record under the same request id whatever the outcome, and a shed one is written by the limiter. So there is no pre-dispatch failure the counter loses; the exclusion matches the list's own.
…tract Cross-model review flagged that the usage endpoint serves a snapshot behind observability.usage_cache_ttl (5s) while the summary endpoint counts live, so a call can reach the Activity Log a few seconds before the Usage tiles. That is inherent to the endpoint never scanning the log per request (SC-005), it is bounded, and freshness_ms already discloses it — so it is documented on the contract and in the parity test rather than traded away.
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 32832813359 --repo smart-mcp-proxy/mcpproxy-go
|
usage_cache_ttl is hot-reloadable and it is the documented bound on how far the Usage figures may lag the Activity Log. The cache banked an absolute expiry from the TTL in force when the entry was written, so lowering the TTL left entries alive past the new bound — an operator tightening the bound got no effect until the old expiry passed. Entries now record when they were stored and freshness is judged against the TTL in force at read time. Found by cross-model review of the F1 parity work.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
TestUsageCacheHonoursAHotReloadedTTL asserted a 1ns TTL rejects an entry written moments earlier. On a Windows runner two adjacent time.Now() calls can report zero elapsed time, so nothing was older than any TTL and the test failed there. It now back-dates the entry by a minute and checks a lowered TTL rejects it and a raised one accepts it — no sleeping, no dependence on clock granularity.
![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 25, 2026
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.
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 25, 2026
…1052) * fix(webui): make the Activity and Usage numbers say what they mean 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. * fix(webui): widen the run identity to everything the folded row prints 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. * fix(activity): stop dropping the scan findings summary, and clamp the 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. * fix(webui): three tie-breaks and one export that would come back empty 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.
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 P0 from the Web UI UX audit (finding F1, also F24) — #1046.
The Activity Log and the Usage tab reported different totals for the same 24-hour window. A repro instance produced three numbers from the same traffic, two of them on the same screen:
51 calls · 6 errors83 events · 50 calls · 12 errors19(31.6% error rate)Calls 50(24.0%)2550Root cause
Two independent miscounts, one per surface:
handleActivitySummary(internal/httpapi/activity.go) counts every activity record in the window, and the frontend labelled that totalN calls. Security scans, quarantine auto-approvals and system starts are rows in the log, not calls.Usage.vuesummedtoolsclient-side. That list is lifetime-cumulative (the window only filters which tools appear), upstream-only (mcpproxy's ownretrieve_tools/describe_toolcalls vanish) and truncated to top-N — so it also disagreed with the histogram printed directly beneath it.Fix
One definition of "a call the user made" —
storage.CountsAsCall— consulted by both surfaces. The usage aggregate already applied these rules implicitly for its timeline; it now delegates to the shared contract, so timeline, summary endpoint, Web UI and CLI cannot drift apart.ActivitySummaryResponsegainscall_count/call_error_count.UsageAggregateResponsegainstotal_calls/total_errors, computed server-side as the sum of the timeline it carries.mcpproxy activity statsno longer prints the event total as "Total Calls".Tests
internal/httpapi/activity_call_parity_test.go— one record set, both endpoints, asserts the two totals are equal (and that the event total is deliberately larger).internal/storage/activity_call_population_test.go— the admission table.internal/runtime/usage_glance_parity_test.go— the timeline is exactly the shared population; the existing tray-glance oracle still agrees independently (itsdefaultcase was extended to mirrorGlanceSelection.qualifies).frontend/tests/unit/activity-usage-call-parity.spec.ts— header labelling and the Usage headline.Verified:
-racegreen for httpapi/runtime/storage/contracts/server; both editions build; golangci-lint v20 issues; 673 frontend unit tests green; live Playwright check on an isolated instance confirms both rendered surfaces now print 50.