fix(mcp): steer agents toward builder query tools instead of raw SQL (HDX-4892)

.changeset/mcp-steer-away-from-raw-sql.md

@@ -0,0 +1,5 @@
+---
+'@hyperdx/api': patch
+---
+
+fix(mcp): steer agents toward builder query tools instead of raw SQL (HDX-4892). Telemetry showed agents (notebook investigations) using `clickstack_sql` for ~73% of data queries — usually for single-source aggregations, top-N, and time-series that the builder tools express more reliably (raw SQL also had ~2x the error rate). Reworded the `clickstack_sql` description to mark it a last resort, added a reciprocal "prefer me over SQL" nudge to `clickstack_table`, `clickstack_timeseries`, and `clickstack_search`, and added a server-level `instructions` tool-selection policy so the guidance is surfaced on `initialize` rather than only via the opt-in `query_guide` prompt.

MCP.md

@@ -147,6 +147,11 @@ with:
 | `clickstack_save_webhook`        | Create (omit id) or update (provide id) a webhook notification destination (slack, generic, or incidentio) |
 | `clickstack_delete_webhook`      | Permanently delete a webhook by ID (blocked while alerts still reference it)                   |
 
+> **Maintainers:** when adding/removing/renaming a query tool, update this table
+> and — for builder (non-SQL) tools — the hand-curated `BUILDER_TOOLS_LIST` in
+> `packages/api/src/mcp/tools/query/builderCatalog.ts`. It isn't auto-derived, so
+> it drifts silently.
+
 ### Metric Sources
 
 `clickstack_timeseries`, `clickstack_table`, and the dashboard builder tile

packages/api/src/mcp/mcpServer.ts

@@ -5,18 +5,46 @@ import { CODE_VERSION } from '@/config';
 import dashboardPrompts from './prompts/dashboards/index';
 import alertsTools from './tools/alerts/index';
 import dashboardsTools from './tools/dashboards/index';
+import {
+  BUILDER_TOOLS_LIST,
+  SQL_FALLBACK_CRITERIA,
+} from './tools/query/builderCatalog';
 import queryTools from './tools/query/index';
 import savedSearchesTools from './tools/savedSearches/index';
 import sourcesTools from './tools/sources/index';
 import traceTools from './tools/trace/index';
 import { McpContext } from './tools/types';
 import { createRegisterTool } from './utils/registerTool';
 
+const SERVER_INSTRUCTIONS = [
+  'ClickStack observability MCP server for querying logs, metrics, traces, and',
+  'sessions, and for managing dashboards, alerts, sources, and saved searches.',
+  '',
+  'TOOL SELECTION POLICY — read before querying:',
+  'Default to the builder query tools. They are more reliable and produce',
+  'structured, chart-ready results:',
+  BUILDER_TOOLS_LIST,
+  '',
+  'Do NOT reach for clickstack_sql first. Raw SQL is a last resort, reserved for',
+  `queries the builder tools genuinely cannot express — ${SQL_FALLBACK_CRITERIA}.`,
+  'A single-source aggregation, top-N, time-series, or row browse must always',
+  'use a builder tool.',
+  '',
+  'Discovery flow: clickstack_list_sources → clickstack_describe_source → query.',
+  'Fetch the query_guide prompt for detailed syntax, and create_dashboard for',
+  'dashboard authoring.',
+].join('\n');
+
 export function createServer(context: McpContext) {
-  const server = new McpServer({
-    name: 'clickstack',
-    version: `${CODE_VERSION}-beta`,
-  });
+  const server = new McpServer(
+    {
+      name: 'clickstack',
+      version: `${CODE_VERSION}-beta`,
+    },
+    {
+      instructions: SERVER_INSTRUCTIONS,
+    },
+  );
 
   const registerTool = createRegisterTool(server, context);
   const registrar = { server, context, registerTool };

packages/api/src/mcp/tools/query/builderCatalog.ts

@@ -0,0 +1,31 @@
+/**
+ * Shared "prefer the builder tools over raw SQL" steering, defined once so the
+ * server instructions, the clickstack_sql description, and the builder tools'
+ * descriptions can't drift apart.
+ */
+
+/** Bulleted list of the builder (non-SQL) query tools and what they're for. */
+export const BUILDER_TOOLS_LIST = [
+  '  • clickstack_table — aggregations, top-N, single-value KPIs, breakdowns',
+  '  • clickstack_timeseries — trends / metrics over time',
+  '  • clickstack_search — browsing individual log/trace rows',
+  '  • clickstack_event_patterns — recurring log/event pattern discovery',
+  '  • clickstack_event_deltas — attributes that differ between two row groups',
+  '  • clickstack_emerging_signals — patterns new or gone vs a baseline window',
+  '  • clickstack_trace_waterfall — one trace as a parent/child span tree',
+  '  • clickstack_trace_top_time_consuming_operations — slowest child operations in a trace',
+].join('\n');
+
+/** When raw SQL is actually warranted. */
+export const SQL_FALLBACK_CRITERIA =
+  'JOINs, sub-queries, CTEs, window functions, tables not registered as sources, or ' +
+  'summary-type metrics (the metricTables.summary table on a metric source, which the ' +
+  'builder tools cannot query)';
+
+/**
+ * Reciprocal nudge dropped into each builder tool's description. Keep it
+ * generic so a single string works for every builder tool.
+ */
+export const PREFER_BUILDER_OVER_SQL_NUDGE =
+  'PREFER THIS over clickstack_sql. Only drop to raw SQL for things the ' +
+  `builder tools cannot express — ${SQL_FALLBACK_CRITERIA}.`;

packages/api/src/mcp/tools/query/eventDeltas.ts

@@ -18,6 +18,7 @@ import type { ToolRegistrar } from '@/mcp/tools/types';
 import { mcpUserError } from '@/mcp/utils/errors';
 import { trimToolResponse } from '@/utils/trimToolResponse';
 
+import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
 import { clickHouseErrorResult, parseTimeRange } from './helpers';
 
 // ─── Schema ──────────────────────────────────────────────────────────────────
@@ -180,6 +181,8 @@ export function registerEventDeltas({ context, registerTool }: ToolRegistrar) {
         'specific attribute (use clickstack_table groupBy), when you want raw ' +
         'rows (use clickstack_search), or when you need a time-series shape ' +
         '(use clickstack_timeseries).\n\n' +
+        PREFER_BUILDER_OVER_SQL_NUDGE +
+        '\n\n' +
         'OUTPUT SHAPE: an array of properties, each with rank, key, score, ' +
         'semanticBoost (true for well-known OTel attrs like service.name / ' +
         'http.method / error.type / status), targetCount and baselineCount ' +

packages/api/src/mcp/tools/query/eventPatterns.ts

@@ -3,6 +3,7 @@ import { z } from 'zod';
 import type { ToolRegistrar } from '@/mcp/tools/types';
 import { mcpUserError } from '@/mcp/utils/errors';
 
+import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
 import { parseTimeRange } from './helpers';
 import { runEventPatterns } from './runEventPatterns';
 import {
@@ -86,6 +87,8 @@ export function registerEventPatterns({
         'Each pattern includes a "whereSnippet" — use it as the "where" parameter in ' +
         'a follow-up clickstack_search call to browse matching raw events.\n\n' +
         'Requires sourceId — call clickstack_list_sources then clickstack_describe_source first.\n\n' +
+        PREFER_BUILDER_OVER_SQL_NUDGE +
+        '\n\n' +
         'When to use which tool:\n' +
         '  - clickstack_event_patterns: clustering / recurring shapes / noise analysis\n' +
         '  - clickstack_search: raw individual rows\n' +

packages/api/src/mcp/tools/query/search.ts

@@ -5,6 +5,7 @@ import { mcpUserError } from '@/mcp/utils/errors';
 import logger from '@/utils/logger';
 import { trimToolResponse } from '@/utils/trimToolResponse';
 
+import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
 import { denoiseSearchResults } from './denoise';
 import { buildTile, parseTimeRange, runConfigTile } from './helpers';
 import {
@@ -67,6 +68,8 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
         'Browse individual log/event/trace rows. ' +
         'Use this when you need to see raw events, investigate specific log lines, ' +
         'or drill into individual records matching a filter.\n\n' +
+        PREFER_BUILDER_OVER_SQL_NUDGE +
+        '\n\n' +
         'Requires sourceId — call clickstack_list_sources then clickstack_describe_source first.\n\n' +
         'For aggregated metrics, use clickstack_table instead. ' +
         'For pattern discovery, use clickstack_event_patterns instead.\n\n' +

packages/api/src/mcp/tools/query/sql.ts

@@ -3,6 +3,7 @@ import { z } from 'zod';
 import type { ToolRegistrar } from '@/mcp/tools/types';
 import { mcpUserError } from '@/mcp/utils/errors';
 
+import { BUILDER_TOOLS_LIST, SQL_FALLBACK_CRITERIA } from './builderCatalog';
 import { buildTile, parseTimeRange, runConfigTile } from './helpers';
 import { endTimeSchema, startTimeSchema } from './schemas';
 
@@ -62,18 +63,20 @@ export function registerSql({ context, registerTool }: ToolRegistrar) {
       // Enforces ClickHouse readonly=2, so effectively read-only.
       annotations: { readOnlyHint: true },
       description:
-        'Execute raw ClickHouse SQL. ' +
-        'ADVANCED: only use this when you need capabilities the builder tools cannot express — ' +
-        'JOINs, sub-queries, CTEs, querying tables not registered as sources, or looking at ' +
-        'summary-type metrics (the metricTables.summary table on a metric source, which the ' +
-        'builder tools cannot query).\n\n' +
+        'Execute raw ClickHouse SQL. LAST-RESORT TOOL — do NOT reach for this first.\n\n' +
+        'Default to the builder tools for querying; they are more reliable and produce ' +
+        'richer, structured results:\n' +
+        BUILDER_TOOLS_LIST +
+        '\n\n' +
+        'ONLY use raw SQL when the query genuinely cannot be expressed by a builder tool — ' +
+        `i.e. it requires ${SQL_FALLBACK_CRITERIA}. ` +
+        'A single-table aggregation, top-N, time-series, or row browse is ALWAYS a ' +
+        'builder-tool job, never raw SQL. If you are unsure, try the builder tool first and only ' +
+        'fall back to SQL if it cannot express what you need.\n\n' +
         'Requires connectionId (not sourceId) — call clickstack_list_sources to find connections. ' +
         'Call clickstack_describe_source to discover column names before writing SQL.\n\n' +
         'Results are always returned as table rows — for time-series semantics, ' +
-        'include a time column and ORDER BY it in your SQL.\n\n' +
-        'For standard aggregations use clickstack_table. ' +
-        'For time-series charts use clickstack_timeseries. ' +
-        'For browsing rows use clickstack_search.',
+        'include a time column and ORDER BY it in your SQL.',
       inputSchema: sqlSchema,
     },
     async input => {

packages/api/src/mcp/tools/query/table.ts

@@ -3,6 +3,7 @@ import { z } from 'zod';
 import type { ToolRegistrar } from '@/mcp/tools/types';
 import { mcpUserError } from '@/mcp/utils/errors';
 
+import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
 import {
   annotateIncreaseTopNHint,
   buildTile,
@@ -209,6 +210,8 @@ export function registerTable({ context, registerTool }: ToolRegistrar) {
         'Compute aggregated metrics as a table, single number, pie chart, or bar chart. ' +
         'Use this for grouped aggregations, top-N queries, single-value KPIs, ' +
         'or proportional breakdowns.\n\n' +
+        PREFER_BUILDER_OVER_SQL_NUDGE +
+        '\n\n' +
         'Requires sourceId — call clickstack_list_sources then clickstack_describe_source first.\n\n' +
         'Use the top-level "where" to scope the entire query (e.g. filter by service). ' +
         'Each select item can also have its own "where" for per-metric cohort ' +

packages/api/src/mcp/tools/query/timeseries.ts

@@ -4,6 +4,7 @@ import { z } from 'zod';
 import type { ToolRegistrar } from '@/mcp/tools/types';
 import { mcpUserError } from '@/mcp/utils/errors';
 
+import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
 import {
   annotateIncreaseTopNHint,
   appendHint,
@@ -83,6 +84,8 @@ export function registerTimeseries({ context, registerTool }: ToolRegistrar) {
         'Plot metrics over time as a line or stacked bar chart. ' +
         'Use this when you need to visualize trends, compare time-series, ' +
         'or monitor metric changes over a time window.\n\n' +
+        PREFER_BUILDER_OVER_SQL_NUDGE +
+        '\n\n' +
         'Requires sourceId — call clickstack_list_sources then clickstack_describe_source first. ' +
         'Each select item defines one plotted series.\n\n' +
         'Column naming: top-level columns are PascalCase (Duration, StatusCode). ' +

packages/hdx-eval/src/__tests__/preflight.test.ts

@@ -0,0 +1,73 @@
+import { probeMcp } from '@/harness/preflight';
+import type { McpDefinition } from '@/harness/types';
+
+const httpDef: McpDefinition = {
+  type: 'http',
+  url: 'http://localhost:30199/mcp',
+  headers: { Authorization: 'Bearer test' },
+  toolPattern: 'mcp__hyperdx__*',
+  label: 'HyperDX',
+};
+
+/** Mock fetch so each queued value is returned as an SSE JSON-RPC response. */
+function mockFetch(...jsonRpc: unknown[]) {
+  const fn = jest.fn();
+  for (const payload of jsonRpc) {
+    fn.mockResolvedValueOnce({
+      status: 200,
+      headers: { get: () => null },
+      text: async () => `event: message\ndata: ${JSON.stringify(payload)}\n\n`,
+    });
+  }
+  global.fetch = fn as unknown as typeof fetch;
+}
+
+const originalFetch = global.fetch;
+afterEach(() => {
+  global.fetch = originalFetch;
+});
+
+describe('probeMcp', () => {
+  it('returns ok with a tool count when initialize + tools/list succeed', async () => {
+    mockFetch(
+      { result: {} },
+      { result: { tools: [{ name: 'a' }, { name: 'b' }] } },
+    );
+    expect(await probeMcp('hyperdx', httpDef)).toEqual({
+      mcp: 'hyperdx',
+      ok: true,
+      toolCount: 2,
+    });
+  });
+
+  it('fails when the server is reachable but serves zero tools', async () => {
+    mockFetch({ result: {} }, { result: { tools: [] } });
+    const res = await probeMcp('hyperdx', httpDef);
+    expect(res.ok).toBe(false);
+    expect(res.error).toMatch(/0 tools/);
+  });
+
+  it('fails with an actionable message when the connection is refused', async () => {
+    global.fetch = jest
+      .fn()
+      .mockRejectedValueOnce(
+        new Error('fetch failed'),
+      ) as unknown as typeof fetch;
+    const res = await probeMcp('hyperdx', httpDef);
+    expect(res.ok).toBe(false);
+    expect(res.error).toMatch(/could not connect.*API server/s);
+  });
+
+  it('treats stdio MCPs as ok without probing', async () => {
+    const stdioDef: McpDefinition = {
+      type: 'stdio',
+      command: 'uvx',
+      toolPattern: 'mcp__clickhouse__*',
+      label: 'ClickHouse',
+    };
+    expect(await probeMcp('clickhouse', stdioDef)).toEqual({
+      mcp: 'clickhouse',
+      ok: true,
+    });
+  });
+});

packages/hdx-eval/src/cli.ts

@@ -17,6 +17,7 @@ import {
   resolveBatchDir,
 } from './grading/grade';
 import { DEFAULT_JUDGE_SPEC, parseJudgeSpec } from './grading/judgeModel';
+import { preflightMcps } from './harness/preflight';
 import { runCell } from './harness/runRun';
 import { type McpKind, PLUGIN_NONE, type PromptVariant } from './harness/types';
 import {
@@ -464,6 +465,14 @@ program
     '--no-judge',
     'Run programmatic checks only during auto-grading (skip LLM judge)',
   )
+  .option(
+    '--no-preflight',
+    'Skip the MCP reachability preflight check. By default the runner probes ' +
+      'each MCP server (initialize + tools/list) before spawning any agents ' +
+      'and aborts if a server is unreachable or serves no tools — this ' +
+      'prevents an entire batch of silent zero-tool-call runs when an API ' +
+      'server is down.',
+  )
   .option(
     '--email <email>',
     `HyperDX account email for post-run inspection (default: ${DEFAULT_EVAL_EMAIL})`,
@@ -495,6 +504,7 @@ program
         report: boolean;
         judgeModel?: string;
         judge: boolean;
+        preflight: boolean;
         email: string;
         password: string;
       },
@@ -616,6 +626,38 @@ program
         anchorMs = anchor.anchorMs;
       }
 
+      // Preflight: probe every MCP server before doing any expensive or
+      // destructive setup (seed detection / reseeding) and before spawning
+      // any agents. A dead API server (or one serving zero tools) otherwise
+      // yields an entire batch of silent zero-tool-call runs that look like
+      // real (bad) scores but actually reflect an unreachable server —
+      // wasting a full suite. Running it first also avoids truncating and
+      // repopulating scenario tables (via --reseed/--live) only to abort.
+      if (cmdOpts.preflight) {
+        const probes = await preflightMcps(
+          mcpKinds.map(k => ({ mcp: k, def: getMcpDefinition(config, k) })),
+        );
+        for (const p of probes) {
+          if (p.ok) {
+            const tools =
+              p.toolCount !== undefined ? ` (${p.toolCount} tools)` : '';
+            console.log(`Preflight ${p.mcp}: OK${tools}`);
+          } else {
+            console.error(`Preflight ${p.mcp}: FAILED — ${p.error}`);
+          }
+        }
+        const failed = probes.filter(p => !p.ok);
+        if (failed.length > 0) {
+          throw new Error(
+            `MCP preflight failed for: ${failed
+              .map(p => p.mcp)
+              .join(', ')}. Fix the server(s) above (usually: the dev stack ` +
+              `for that slot is not running) and re-run, or pass ` +
+              `--no-preflight to bypass this check.`,
+          );
+        }
+      }
+
       // ── Re-seed ───────────────────────────────────────────────────
       // Default: skip reseed IF data already exists. Auto-seeds on first
       // run when scenario tables are empty or missing.