feat(mcp): reject persisted histogram tiles with unsupported aggFns in query_tile - #2796
feat(mcp): reject persisted histogram tiles with unsupported aggFns in query_tile#2796pulpdrew wants to merge 1 commit into
Conversation
…n query_tile Co-authored-by: Drew Davis <pulpdrew@gmail.com>
🦋 Changeset detectedLatest commit: 548c25d 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds pre-execution validation for persisted metric tiles queried through MCP, returning an actionable user error when histogram or exponential-histogram tiles use unsupported aggregation functions.
Confidence Score: 5/5The PR appears safe to merge, with the new guard consistently validating the persisted external tile representation before execution. The guard uses the same metric constraints as MCP query and dashboard-write inputs, correctly adapts persisted delta representation, preserves valid metric configurations, and returns invalid configurations through the established user-error path.
|
| Filename | Overview |
|---|---|
| packages/api/src/mcp/tools/dashboards/validation.ts | Adds persisted builder-tile metric validation using the existing metric-select rules while skipping raw SQL and non-metric configurations. |
| packages/api/src/mcp/tools/dashboards/queryTile.ts | Runs the new validation before tile execution and returns violations as categorized MCP user errors. |
| packages/api/src/mcp/tests/dashboards/validation.test.ts | Covers supported and unsupported histogram aggregations, unaffected metric kinds, ignored tile shapes, labels, and multi-item collection. |
| packages/api/src/mcp/tests/dashboards/queryTile.int.test.ts | Verifies that a directly persisted legacy histogram tile using avg is rejected before renderer execution. |
| .changeset/mcp-histogram-query-tile-guard.md | Records the user-facing MCP validation behavior as an API patch release. |
Sequence Diagram
sequenceDiagram
participant Client as MCP client
participant Tool as clickstack_query_tile
participant Mongo as Dashboard storage
participant Validator as Metric tile validator
participant Runner as runConfigTile
participant CH as ClickHouse
Client->>Tool: Query persisted tile
Tool->>Mongo: Load team-scoped dashboard
Mongo-->>Tool: Persisted tile config
Tool->>Validator: Validate metric select items
alt Unsupported histogram aggregation
Validator-->>Tool: Actionable validation errors
Tool-->>Client: mcpUserError
else Valid configuration
Validator-->>Tool: No errors
Tool->>Runner: Execute tile
Runner->>CH: Render and run query
CH-->>Client: Query result
end
Reviews (1): Last reviewed commit: "feat(mcp): reject persisted histogram ti..." | Re-trigger Greptile
E2E Test Results✅ All tests passed • 266 passed • 1 skipped • 1104s
Tests ran across 4 shards in parallel. |
Why
Histogram and exponential histogram metrics only support
countandquantileaggregations in the ClickHouse renderer. The MCP query and dashboard save/patch tools already reject invalid aggFns for these metric kinds at input time (added in #2705 viagetMetricSelectIssues). This mirrors the chart-builder UI change that hides those options.The one remaining MCP gap was
clickstack_query_tile: it loads and executes an already-persisted tile without re-validating it. A tile created outside MCP validation — via the REST API, the UI (pre-dating the UI guard), or legacy data — could hold a histogram metric withavg/sum/min/max, and running it surfaced an opaque ClickHouse render error ("<aggFn> is not supported for histograms currently") instead of an actionable message.What changed
mcp/tools/dashboards/validation.ts: AddedgetMetricTileAggFnErrors(tiles), which walks each builder tile'sselectitems and reuses the existinggetMetricSelectIssuesrules. It inspects only metric select items (those carrying ametricType), maps the persistedperiodAggFn: 'delta'back to theisDeltaflag, and skips raw SQL / search / heatmap / log / trace items. Returns one human-readable error per offending item.mcp/tools/dashboards/queryTile.ts: Before executing a tile,clickstack_query_tilenow runsgetMetricTileAggFnErrors([tile])and returns anmcpUserErrorwhen the persisted config is invalid, instead of letting the query fail opaquely downstream.No change to the already-validated surfaces (
clickstack_timeseries,clickstack_table,clickstack_save_dashboard,clickstack_patch_dashboard) — those continue to reject invalid histogram aggFns at input time.Testing
packages/api/src/mcp/__tests__/dashboards/validation.test.ts— added coverage forgetMetricTileAggFnErrors(histogram/exp-histogram rejection of avg/sum/min/max, acceptance of quantile+level and count, gauge unaffected, non-metric/raw-SQL items ignored, positional labels, multi-item collection). 30/30 passing.packages/api/src/mcp/__tests__/dashboards/queryTile.int.test.ts— added a test that seeds a histogram tile withavgdirectly into MongoDB (bypassing MCP save validation) and assertsclickstack_query_tilerejects it with"Histogram metrics only support …". 15/15 passing (make dev-int FILE=queryTile).yarn tsc --noEmit(packages/api) — clean.yarn lint:fix— no errors.Linear Issue: HDX-4991