Repository navigation
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: a61fc5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
PR Review
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
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Duplicate code analysisNo new duplicate code introduced by this PR. jscpd compared this branch against |
🔴 Tier 4 — CriticalTouches 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:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
E2E Test Results✅ All tests passed • 466 passed • 1 skipped • 1494s
Tests ran across 4 shards in parallel. |
Deep Review✅ No critical (P0/P1) issues found in the coverage completed so far. This change exposes
🟡 P2 — recommended
🔵 P3 nitpicks (1)
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 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 |
Stacked on #3659.
Summary
The MCP dashboard tools now create, read, and update dashboards with
ADHOCfilters. The previous PR left MCP hiding them. A side effect was that aclickstack_save_dashboardupdate that sentfilterssilently deleted any ad hoc filters already on the dashboard. This PR fixes that.clickstack_save_dashboardacceptsADHOCfilters, andclickstack_save_dashboard/clickstack_patch_dashboardacceptadhocentries insavedFilterValues. 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.convertToExternalDashboardno longer has the temporaryhideAdhocoption.hidePromqlnow hides PromQL ad hoc filters as well asPROMETHEUS_LABELfilters, through a sharedisPromqlFilterhelper.PROMETHEUS_LABELfilters. The existing variable-name clash check also covers them.adhocentry 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.How to reproduce
Starting point:
yarn dev, with an MCP client connected to the local MCP server.On
main:clickstack_get_dashboard. The filter is missing fromfilters.clickstack_save_dashboardwith that dashboard'sidand anyfiltersarray. The ad hoc filter is deleted.On this branch:
clickstack_get_dashboardreturns the ad hoc filter and itsadhocsaved conditions.clickstack_save_dashboardwithfilters: [{ "type": "ADHOC", "name": "Conditions", "sourceType": "sql", "sources": ["<trace-source-id>"] }]andsavedFilterValues: [{ "type": "adhoc", "name": "Conditions", "conditions": [{ "key": "ServiceName", "operator": "IN", "value": "api" }] }]creates it. Opening the dashboard shows the condition applied.ENABLE_PROMQLoff, a PromQL ad hoc filter is absent fromclickstack_get_dashboardand 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__/dashboardspasses. 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.tspasses.References