Skip to content

feat(mcp): insights_view MCP App — time-series explorer (KLA-402)#19

Merged
jrennichjc merged 2 commits into
mainfrom
juergen/mcp-app-insights
Apr 28, 2026
Merged

feat(mcp): insights_view MCP App — time-series explorer (KLA-402)#19
jrennichjc merged 2 commits into
mainfrom
juergen/mcp-app-insights

Conversation

@jklaassenjc
Copy link
Copy Markdown
Collaborator

@jklaassenjc jklaassenjc commented Apr 24, 2026

Summary

  • Adds `insights_view` MCP App: interactive Directory Insights time-series with filters, top-users sidebar, and event preview
  • Introduces `addToolWithMetaTyped[In]` — generic variant of `addToolWithMeta` so parameterized apps get an auto-derived JSON input schema
  • Server aggregates into time buckets (size auto-chosen from window); client just renders
  • Stacked on top of #18; rebase to main once that lands

UX

Interactive filter form (service / last / event_type / user), stacked bars by time bucket with legend, top-10-users list alongside. Refresh re-runs the query.

Scope

New:

  • `internal/mcp/apps_insights.go` — typed args, fetch + aggregate, registration
  • `internal/mcp/apps_html/insights.html` — renderer (uses jcApp scaffolding from refactor(mcp): 1-file-per-app pattern for MCP Apps (KLA-401) #18, no duplicated postMessage)
  • `internal/mcp/apps_insights_test.go` — 6 tests covering bucketing, window parsing, aggregation, filter, UI metadata, resource injection

Modified:

  • `internal/mcp/apps.go` — adds `addToolWithMetaTyped` helper and calls `registerInsightsView` from `registerAppTools`
  • `internal/mcp/tools_test.go` — tool count bumps 195 → 196, adds `insights_view` to expected list

Key design choices

  • 10k event cap per call. Directory Insights windows can be millions of events; we use `CountEvents` for the true total and `QueryEvents(Limit: 10000)` for the sample, surfacing a warning when truncated so the UI can flag partial data
  • Server-side bucketing keeps the UI bundle small and avoids shipping raw events to the browser. Bucket size auto-picked: 5m / 1h / 6h / 1d depending on window span
  • Local duration parsing (`parseLastDuration`) instead of `api.ParseTimeRange` for the `Last` argument, so start/end both anchor on the same `nowFunc` and tests stay deterministic
  • `addToolWithMetaTyped` is generic on `In any` so each app declares its own typed args struct (KLA-403..406 will use this too)

Test plan

🤖 Generated with Claude Code


Note

Medium Risk
Adds a new MCP App tool that queries and aggregates Directory Insights events (including time-window parsing and sampling limits), which could affect performance/behavior of Insights calls. Also changes the tool-registration helper to support generic typed inputs, potentially impacting how future tools’ schemas/handlers are derived.

Overview
Introduces a new MCP App, insights_view, that serves a ui://jc/insights HTML dashboard and a parameterized tool to fetch/aggregate Directory Insights events into time buckets with top-user rankings, preview events, and truncation warnings (10k event cap).

Refactors app tool registration by adding generic addToolWithMetaTyped[In] (auto-derived JSON input schema) and keeping addToolWithMeta as a no-args wrapper; registerAppTools now registers this typed-input app explicitly. Tests are added/updated to cover window parsing, bucketing/aggregation, UI metadata/resource injection, and the expected tool list/count.

Reviewed by Cursor Bugbot for commit daf3c88. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread internal/mcp/apps_html/insights.html
@jklaassenjc jklaassenjc force-pushed the juergen/mcp-apps-refactor branch from fc1e1c6 to b6ef360 Compare April 24, 2026 21:39
@jklaassenjc jklaassenjc force-pushed the juergen/mcp-app-insights branch from 6fdbc29 to e8bdee1 Compare April 24, 2026 21:39
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Reviewed by Cursor Bugbot for commit e8bdee1. Configure here.

Comment thread internal/mcp/apps.go
Adds the Directory Insights time-series dashboard as an interactive MCP App.
Parameters mirror `jc insights query` (service, event_type, last, start, end,
user); the rendered app shows a stacked bar chart by time bucket with a
top-users ranking alongside.

Server side (apps_insights.go):
- insightsViewArgs: typed inputs so the JSON schema auto-derives; LLMs see
  the same shape they already know from the CLI
- fetchInsightsViewData: CountEvents for true total, QueryEvents capped at
  10k for the sample, aggregate into bins keyed by bucket start (bucket
  size auto-chosen from window: 5m/1h/6h/1d), tally top 10 users, keep
  20-event preview
- resolveInsightsWindow: parses last/start/end, defaults to 24h, validates
  start < end; uses local duration parser anchored on our nowFunc so tests
  stay deterministic (api.ParseTimeRange uses its own wall-clock)
- bucketSizeFor: 5m / 1h / 6h / 1d depending on window length

addToolWithMetaTyped[In any]: generic variant of addToolWithMeta so apps
that take parameters (insights_view now, user_view / device_view next) get
an auto-derived JSON input schema from the typed arg struct. Still wires
rate limiting, audit logging, and Meta with _meta.ui.resourceUri.

Client side (apps_html/insights.html):
- Filter form (service, last, event_type, user) with Apply + Refresh buttons
- Proactive fetch on load, re-fetches on filter submit via jcApp.callTool
- Per-bucket stacked bar chart with event-type color legend; top-users list
- Warning banner when the window is large enough that we sampled vs
  fetched all events
- Uses the shared jcApp scaffolding from common.js; no inline postMessage
  re-implementation. Purely content + rendering.

Tests (apps_insights_test.go):
- TestBucketSizeFor — auto-picked bucket size
- TestResolveInsightsWindow — defaults, Last override, start/end ordering
- TestFetchInsightsViewData_Aggregates — fixture of 5 events, verifies total,
  event type set, top-user ranking, bin count, sum of bin counts
- TestFetchInsightsViewData_EventTypeFilter — filter narrows correctly
- TestInsightsView_HasUIMetadata — tool exposes _meta.ui.resourceUri
- TestInsightsResource_ServesHTMLWithInjection — ui:// resource has
  common.js injected and the marker stripped

Stacks on juergen/mcp-apps-refactor (PR #18) since it depends on
addToolWithMetaTyped and the renderAppHTML helper introduced there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jklaassenjc jklaassenjc force-pushed the juergen/mcp-apps-refactor branch from b6ef360 to a5cddd4 Compare April 24, 2026 21:47
Medium: insights.html onToolResult race
  The initialized flag was set to true BEFORE the try-catch in
  onToolResult. If parseToolResult threw or returned null, the flag was
  already set and the setTimeout fallback (which uses the more robust
  load() path) never fired — the UI locked on the loading spinner with
  no recovery. Now the flag only flips after a successful render.

Low: consolidate addToolWithMeta to delegate
  addToolWithMeta and addToolWithMetaTyped had ~25 lines of identical
  rate-limiting + audit-logging + tool-filtering wrapper. Any future fix
  would need to land in both places. Collapsed to a 1-line shim that
  calls the typed variant with struct{}, so the wrapper exists once.
  Call sites unchanged (s.addToolWithMeta still works).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jklaassenjc jklaassenjc force-pushed the juergen/mcp-app-insights branch from e8bdee1 to daf3c88 Compare April 24, 2026 21:49
@jklaassenjc
Copy link
Copy Markdown
Collaborator Author

Addressed both Bugbot findings on this PR in daf3c88:

  • Medium — insights.html initialized flag only flips after a successful render now; a thrown parseToolResult or null payload leaves initialized=false so the setTimeout fallback can recover (previously the UI stuck on the loading spinner with no way out).
  • LowaddToolWithMeta now delegates to addToolWithMetaTyped[struct{}], dropping ~25 lines of duplicated wrapper logic. Call sites (s.addToolWithMeta(...)) are unchanged.

jrennichjc
jrennichjc previously approved these changes Apr 28, 2026
Base automatically changed from juergen/mcp-apps-refactor to main April 28, 2026 01:37
@jrennichjc jrennichjc dismissed their stale review April 28, 2026 01:37

The base branch was changed.

@jrennichjc jrennichjc merged commit d1449e5 into main Apr 28, 2026
3 of 4 checks passed
@jrennichjc jrennichjc deleted the juergen/mcp-app-insights branch April 28, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants