Skip to content

feat(mcp): support ad hoc dashboard filters in the MCP dashboard tools - #3660

Open
knudtty wants to merge 3 commits into
aaron/adhoc-external-apifrom
aaron/adhoc-mcp
Open

knudtty wants to merge 3 commits into
aaron/adhoc-external-apifrom
aaron/adhoc-mcp

Conversation

@knudtty

@knudtty knudtty commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Stacked on #3659.

Summary

The MCP dashboard tools now create, read, and update dashboards with ADHOC filters. The previous PR left MCP hiding them. A side effect was that a clickstack_save_dashboard update that sent filters silently deleted any ad hoc filters already on the dashboard. This PR fixes that.

  • Input schemas: clickstack_save_dashboard accepts ADHOC filters, and clickstack_save_dashboard / clickstack_patch_dashboard accept adhoc entries in savedFilterValues. Validation reuses the external API schemas, so MCP and REST stay in sync; only the descriptions are MCP-specific. sourceType: "promql" is offered only while PromQL is enabled.
  • Reads: convertToExternalDashboard no longer has the temporary hideAdhoc option. hidePromql now hides PromQL ad hoc filters as well as PROMETHEUS_LABEL filters, through a shared isPromqlFilter helper.
  • Updates with PromQL off: hidden PromQL ad hoc filters are kept on update, like hidden PROMETHEUS_LABEL filters. The existing variable-name clash check also covers them.
  • Warnings: a saved adhoc entry produces a warning when its name matches no ad hoc filter, or when it uses an operator its filter's source type can't apply. The UI would silently ignore both.
  • Prompt docs: the create-dashboard prompt gets a rule 9h and two gotchas. The query guide documents the ad hoc filter shape. The PromQL dashboard section covers PromQL ad hoc filters.

How to reproduce

Starting point: yarn dev, with an MCP client connected to the local MCP server.

On main:

  1. Create a dashboard with an ad hoc filter in the UI, then call clickstack_get_dashboard. The filter is missing from filters.
  2. Call clickstack_save_dashboard with that dashboard's id and any filters array. The ad hoc filter is deleted.

On this branch:

  1. clickstack_get_dashboard returns the ad hoc filter and its adhoc saved conditions.
  2. clickstack_save_dashboard with filters: [{ "type": "ADHOC", "name": "Conditions", "sourceType": "sql", "sources": ["<trace-source-id>"] }] and savedFilterValues: [{ "type": "adhoc", "name": "Conditions", "conditions": [{ "key": "ServiceName", "operator": "IN", "value": "api" }] }] creates it. Opening the dashboard shows the condition applied.
  3. With ENABLE_PROMQL off, a PromQL ad hoc filter is absent from clickstack_get_dashboard and survives a save that leaves it out.

How to test on Vercel preview

N/A. API-only change.

Testing

  • make dev-int FILE=src/mcp/__tests__/dashboards passes. New cases cover a SQL round trip through save, get and save, a PromQL round trip, source-kind rejections, hiding and keeping PromQL ad hoc filters with PromQL off, and the warnings.
  • make dev-int FILE=src/routers/external-api/__tests__/dashboards.int.test.ts passes.

References

Save, get, and patch now handle ADHOC filters and their adhoc saved
conditions. PromQL ad hoc filters are offered only while PromQL is enabled;
otherwise reads hide them and updates keep them, as for PROMETHEUS_LABEL
filters. Saved conditions naming no ad hoc filter, or using an operator their
filter can't apply, warn. Drops the temporary hideAdhoc option.
@knudtty
knudtty requested a review from a team as a code owner November 13, 2026 21:36
@vercel

vercel Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Nov 13, 2026 10:00pm UTC
hyperdx-storybook Ready Ready Preview Nov 13, 2026 10:00pm UTC

Request Review

@changeset-bot

changeset-bot Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a61fc5e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Minor
@hyperdx/app Minor
@hyperdx/otel-collector Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@knudtty
knudtty added this pull request to stack #3661 November 13, 2026 21:37
@github-actions

Copy link
Copy Markdown
Contributor

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

  • 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
  • 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
  • Anything about surrounding code, or asking you to widen the change (hoist a helper,
    dedupe with another file, fix other call sites): never fix here, whatever the
    severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

1 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 0 minor

None could be anchored to changed lines; they are listed below.

Findings outside the changed lines

  • 🟠 packages/api/src/mcp/tools/dashboards/saveDashboard.ts:343 — With PromQL off, a get→save round trip falsely tells the agent to remove the saved conditions of a hidden PromQL ad hoc filter → convertToExternalDashboard (packages/api/src/routers/external-api/v2/utils/dashboards.ts:914) hides PromQL ad hoc filters but still returns their adhoc savedFilterValues. When the agent sends those back with filters, getSavedFilterValueWarnings checks them against effectiveFilters = filters. That list leaves out the hiddenFilters the code adds back at line 414, so the agent gets a warning that the entry matches no ADHOC filter and should be fixed or removed, even though the filter is kept. Acting on that warning deletes the hidden filter's defaults. Fix: compute the warnings against [...requestFilters, ...hiddenFilters] (or the final setPayload.filters), and add a case to the 'keeps hidden PromQL ad hoc filters' test that round-trips savedFilterValues. Also decide whether hidePromql should hide those saved values, since the filter itself is hidden.

Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.

@github-actions

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 108 in total (1.18%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Nov 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

  • Critical-path files (1) — tenancy, public API, or shipped database config:
    • packages/api/src/routers/external-api/v2/utils/dashboards.ts

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 13
  • Production lines changed: 296 (+ 240 in test files, excluded from tier calculation)
  • Critical-path lines changed: 29
  • Branch: aaron/adhoc-mcp
  • Author: knudtty

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

github-actions Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 466 passed • 1 skipped • 1494s

Status Count
✅ Passed 466
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical (P0/P1) issues found in the coverage completed so far.

This change exposes ADHOC dashboard filters through the MCP tools and reuses the external-API Zod schemas so MCP and REST validation stay in sync. Direct verification of the central mechanics confirms the three load-bearing claims hold:

  • The removed helpers (getMcpCompatibleFilters, getMcpCompatibleFilterValues, hideAdhoc) have no remaining callers in packages/api/src, so the deletions are clean.
  • The variable-name clash check in updateDashboard genuinely covers hidden PromQL ad hoc filters: stored ad hoc filters carry isVariableEnabled: true, so they pass isFilterVariableEnabled and participate in both requestVariableNames and hiddenFilters.
  • The ad hoc warning path in getSavedFilterValueWarnings is correctly separate from the variable path, because getDashboardVariableFilters explicitly skips ADHOC filters (filters.ts:1290), so an ad hoc variableName never leaks into declaredNames.

🟡 P2 — recommended

  • packages/api/src/routers/external-api/v2/utils/dashboards.ts:763 — the new exported isPromqlFilter ("a filter that only works while PromQL is enabled") shares its exact name with a different private isPromqlFilter in packages/common-utils/src/filters.ts:1182 that tests a BroadcastFilter's type === 'promql'; the two unrelated meanings invite confusion for future readers importing across these modules.
    • Fix: rename the exported helper to convey intent, e.g. isPromqlOnlyFilter or requiresPromql.
🔵 P3 nitpicks (1)
  • packages/api/src/mcp/tools/dashboards/validation.ts:242 — the filters parameter type (FilterForVariableDeclaration & { sourceType?: AdhocFilterSourceType })[] is an ad-hoc intersection that papers over the real DashboardFilter shape; it works but reads as a type patch rather than a modeled input.
    • Fix: accept the proper filter union (or a narrowed DashboardFilter[]) so the type === 'ADHOC' narrowing is first-class.

Reviewers (2 of 10 returned before output deadline): previous-comments, learnings-researcher, plus orchestrator diff analysis.

Coverage note: The correctness, adversarial, api-contract, kieran-typescript, testing, maintainability, project-standards, and agent-native reviewers were dispatched but had not returned when structured output was required; findings above come from the completed reviewers and direct orchestrator verification of the diff, so deeper per-persona analysis of those lenses is not reflected here. previous-comments reviewer confirmed the 4 prior PR comments are all automated bot output (Vercel, changeset-bot, jscpd, pr-triage) with no actionable feedback. learnings-researcher found no applicable prior learnings (the only docs/solutions/ tree is plugin-scoped, not ClickStack application code).

Testing gaps: No explicit regression test was observed for the variable-name clash between a request filter and a hidden PromQL ad hoc filter when PromQL is disabled; the keep-hidden-on-update path is tested, but the clash rejection on that same path is not.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — a61fc5e0 Deployed Nov 13, 2026 by vercel[bot]
Preview – hyperdx-storybook — a61fc5e0 Deployed Nov 13, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant