Skip to content

fix: Add sourceId to MCP Raw SQL Tile schema - #2424

Merged
kodiakhq[bot] merged 3 commits into
mainfrom
drew/improve-mcp-dashboard-raw-sql-source
Jun 5, 2026
Merged

kodiakhq[bot] merged 3 commits into
mainfrom
drew/improve-mcp-dashboard-raw-sql-source

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR updates the MCP server's dashboard schemas/prompts to allow agents to set source ID for raw sql chart configs. This is required for certain macros to work.

Screenshots or video

Claude now generates dashboards with Raw SQL Tiles with the Source ID set:

Screenshot 2026-06-05 at 10 55 29 AM

How to test on Vercel preview

This can be tested locally by asking your agent to generate a dashboard with some Raw SQL Tiles.

References

  • Linear Issue: Closes HDX-4463
  • Related PRs:

@vercel

vercel Bot commented Jun 5, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Jun 5, 2026 3:35pm
hyperdx-storybook Ready Ready Preview, Comment Jun 5, 2026 3:35pm

Request Review

@changeset-bot

changeset-bot Bot commented Jun 5, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d234b4b

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

This PR includes changesets to release 4 packages
Name Type
@hyperdx/common-utils Patch
@hyperdx/api Patch
@hyperdx/app Patch
@hyperdx/otel-collector Patch

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

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

github-actions Bot commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

  • Critical-path files (1):
    • packages/api/src/routers/external-api/v2/utils/dashboards.ts
  • Cross-layer change: touches backend (packages/api) + shared utils (packages/common-utils)

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: 7
  • Production lines changed: 151 (+ 373 in test files, excluded from tier calculation)
  • Branch: drew/improve-mcp-dashboard-raw-sql-source
  • Author: pulpdrew

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

@greptile-apps

greptile-apps Bot commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds sourceId as an optional field to the MCP raw SQL tile schema and wires in a validation guard that rejects tiles using $__filters or $__sourceTable without a corresponding sourceId. Prompt content is also updated to instruct agents to always set sourceId on single-table SQL tiles.

  • SOURCE_DEPENDENT_MACROS constant introduced in common-utils to enumerate the two macros that require a source; validation.ts uses substring matching against sqlTemplate to detect usage and reports offending tile names and macro names in the error.
  • Guard applied consistently across createDashboard, updateDashboard, and patchDashboard before the existing schema/tile validation step; isRawSqlExternalTileConfig is exported from the router utils to support this.
  • Prompt examples updated with sourceId + $__filters on all single-table SQL tiles; schema descriptions updated to document the $__sourceTable macro and the sourceId requirement.

Confidence Score: 5/5

Safe to merge — the changes are additive and well-tested; the new validation guard only rejects configurations that would silently misbehave at render time.

The new sourceId field is optional in the schema and the existing save/patch paths were already converting it to the internal source field. The validation guard is defensive-only: it fires exclusively when a tile uses a macro that requires a source but omits sourceId, a case that was previously a silent failure. The constant, helper functions, and calling code are all covered by unit and integration tests. No pre-existing behaviour is altered for tiles that don't use source-dependent macros.

No files require special attention.

Important Files Changed

Filename Overview
packages/api/src/mcp/tools/dashboards/validation.ts New file: exports getRawSqlTilesMissingRequiredSource and getRawSqlMissingSourceError — cleanly guards against $__filters/$__sourceTable being used without sourceId
packages/api/src/mcp/tools/dashboards/schemas.ts Adds optional sourceId field to mcpSqlTileSchema with detailed description; sqlTemplate description updated to document $__sourceTable macro and sourceId requirement
packages/api/src/mcp/tools/dashboards/saveDashboard.ts getRawSqlMissingSourceError guard added to both createDashboard and updateDashboard paths before the existing tile validation
packages/api/src/mcp/tools/dashboards/patchDashboard.ts getRawSqlMissingSourceError check added after mergedTile construction and before validateDashboardTiles, correctly using the merged (not existing) config
packages/common-utils/src/macros.ts Adds SOURCE_DEPENDENT_MACROS constant listing $__filters and $__sourceTable with clear documentation on why each requires a source
packages/api/src/routers/external-api/v2/utils/dashboards.ts isRawSqlExternalTileConfig exported so the validation module can use it; no logic changes
packages/api/src/mcp/prompts/dashboards/content.ts Prompt updated to instruct agents to always set sourceId on single-table raw SQL tiles and add $__filters; example tiles updated accordingly
packages/api/src/mcp/tests/dashboards/validation.test.ts New test file with comprehensive unit coverage of getRawSqlTilesMissingRequiredSource and getRawSqlMissingSourceError across 8 cases
packages/api/src/mcp/tests/dashboards/saveDashboard.test.ts Four integration tests added: sourceId round-trips for $__filters, rejection without sourceId for $__filters and $__sourceTable, and no-sourceId pass for multi-table queries
packages/api/src/mcp/tests/dashboards/patchDashboard.test.ts Two integration tests added covering patch rejection when sourceId is missing and pass-through when sourceId is present on a SQL tile using $__filters

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MCP Tool Input\nsave / update / patch] --> B[Zod Schema Parse\nmcpSqlTileSchema includes sourceId]
    B -->|invalid| C[Return schema error]
    B -->|valid| D[getRawSqlMissingSourceError]
    D --> E{SQL tile uses\n$__filters or\n$__sourceTable?}
    E -->|No / sourceId set| F[validateDashboardTiles\nexisting validator]
    E -->|Yes, no sourceId| G[Return mcpError\nnaming tile + macros]
    F -->|invalid| H[Return validation error]
    F -->|valid| I[convertExternalTilesToInternal\nsourceId to source field]
    I --> J[Save to MongoDB\nsource ObjectId stored]
Loading

Reviews (3): Last reviewed commit: "Merge branch 'main' into drew/improve-mc..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The change is small, additive, and well-tested. sourceId is .optional() (backward compatible), the field already exists on the external raw SQL base schema (packages/api/src/utils/zod.ts:240) so it persists correctly, the guard runs on exactly the config that gets persisted (full config on create/update, the replacement mergedTile.config on patch), and the new pure helpers have thorough unit coverage. No P0/P1 issues.

🟡 P2 -- recommended

  • packages/api/src/mcp/tools/dashboards/schemas.ts:600 -- the new sourceId description asserts it "must belong to the same connection as connectionId," but nothing validates this; team-membership of sourceId and connectionId are each checked independently, so a valid-but-cross-connection sourceId persists and resolves $__sourceTable/$__filters against a table on the wrong connection at render time.
    • Fix: Either validate that the supplied sourceId belongs to the same connection as connectionId in the guard, or soften the description so it does not imply enforcement the system does not provide.
    • agent-native
  • packages/api/src/mcp/tools/dashboards/saveDashboard.ts:281 -- the getRawSqlMissingSourceError guard is wired into updateDashboard but every new save-tool test exercises only the create path, so the duplicated update call site could be removed or broken with no failing test.
    • Fix: Add an integration test that re-saves an existing dashboard by id with a raw SQL tile using $__filters and no sourceId, asserting isError and that persisted tiles are unchanged.
    • testing, correctness, maintainability
🔵 P3 nitpicks (5)
  • packages/api/src/mcp/tools/dashboards/schemas.ts:599 -- the description string concatenates without a separating space, rendering $__sourceTable.The sourceId... in the text the agent reads.
    • Fix: Add a trailing space after $__sourceTable. so the two sentences are separated.
  • packages/api/src/mcp/tools/dashboards/validation.ts -- macro detection uses sqlTemplate.includes(macro), an unanchored substring match that over-rejects when $__filters/$__sourceTable appear inside a SQL string literal or comment, and would false-positive on any future macro that contains these as a prefix.
    • Fix: Match with a word-boundary-aware regex (e.g. /\$__filters\b/) or against the tokenized macro list rather than raw substring containment.
    • correctness, maintainability, api-contract, kieran-typescript
  • packages/api/src/mcp/tools/dashboards/validation.ts -- the source-macro guard is a second tile-validation entry point invoked manually alongside validateDashboardTiles at three mutation sites, so a future fourth mutation path can silently skip it.
    • Fix: Wrap both checks in a single MCP-side validation helper that all save/patch paths call.
    • maintainability
  • packages/common-utils/src/macros.ts:175 -- SOURCE_DEPENDENT_MACROS hardcodes $__filters/$__sourceTable separately from the macro engine's own registry, so a future source-dependent macro must be remembered here or it silently bypasses the guard.
    • Fix: Derive the list from, or add a test cross-referencing it against, the canonical macro registry.
    • maintainability, api-contract
  • packages/api/src/routers/external-api/v2/utils/dashboards.ts -- the new guard is MCP-only, so the same source-less-macro tile can still be saved through the external v2 REST API; this is pre-existing behavior and MCP-scoped by design, noted only for cross-surface awareness.
    • Fix: File a follow-up to apply the same check on the REST path, or document the intentional MCP-only scope.
    • api-contract, project-standards

Reviewers (8): correctness, testing, maintainability, project-standards, api-contract, kieran-typescript, agent-native, learnings-researcher.

Testing gaps: updateDashboard guard call site untested at integration level (create path and pure helper are covered); no test pins the substring-match behavior for a macro token embedded in a string literal/comment; the documented same-connection constraint between sourceId and connectionId is neither enforced nor tested.

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 198 passed • 3 skipped • 1355s

Status Count
✅ Passed 198
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@brandon-pereira brandon-pereira left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - thanks!

@kodiakhq
kodiakhq Bot merged commit ee90738 into main Jun 5, 2026
18 of 19 checks passed
@kodiakhq
kodiakhq Bot deleted the drew/improve-mcp-dashboard-raw-sql-source branch June 5, 2026 15:40

This branch was successfully deployed

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants