From bf377281dcfd24c272ff36162091c14192144702 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 20 Oct 2026 16:56:56 -0600 Subject: [PATCH 1/8] fix(mcp): make event_patterns drill-down snippets executable SQL predicates whereSnippet is now a SQL match() predicate that keeps token order and wildcard positions and includes the caller filter and source table filter. Also: JSONExtractString bodyExpression support, sample coverage and trend bucket metadata, tableFilterExpression applied to pattern mining, and minePatterns reports each cluster's own template. --- .changeset/mcp-event-patterns-drilldown.md | 10 + packages/api/src/fixtures.ts | 1 + .../dashboards/queryTile.int.test.ts | 26 ++ packages/api/src/mcp/__tests__/query.test.ts | 50 +++ .../src/mcp/__tests__/queryTool.int.test.ts | 342 +++++++++++++++++- .../api/src/mcp/prompts/dashboards/content.ts | 46 ++- .../query/__tests__/patternDrilldown.test.ts | 177 +++++++++ .../api/src/mcp/tools/query/eventPatterns.ts | 58 +-- packages/api/src/mcp/tools/query/helpers.ts | 30 +- .../src/mcp/tools/query/patternDrilldown.ts | 111 ++++++ .../src/mcp/tools/query/runEventPatterns.ts | 229 +++++++++--- .../src/__tests__/mine-patterns.test.ts | 113 ++++++ packages/common-utils/src/drain/drain.ts | 5 + .../common-utils/src/drain/mine-patterns.ts | 62 +++- 14 files changed, 1149 insertions(+), 111 deletions(-) create mode 100644 .changeset/mcp-event-patterns-drilldown.md create mode 100644 packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts create mode 100644 packages/api/src/mcp/tools/query/patternDrilldown.ts diff --git a/.changeset/mcp-event-patterns-drilldown.md b/.changeset/mcp-event-patterns-drilldown.md new file mode 100644 index 0000000000..edda3830e4 --- /dev/null +++ b/.changeset/mcp-event-patterns-drilldown.md @@ -0,0 +1,10 @@ +--- +'@hyperdx/api': patch +'@hyperdx/common-utils': patch +--- + +fix: make clickstack_event_patterns drill-down snippets executable + +Each pattern's `whereSnippet` is now a SQL predicate (`whereLanguage: "sql"`) that keeps the template's token order and variable positions and includes the caller's filter and the source table filter, so passing it to `clickstack_search` returns the pattern's events. Before, a template like `Sending <*> rows` produced `Body:"Sending rows"`, which matched nothing, and map fields produced Lucene that failed to parse. `bodyExpression` now also accepts `JSONExtractString(, '', ...)` to mine a field inside JSON bodies, and the tool reports sample coverage and the actual trend bucket interval and boundaries. + +`clickstack_event_patterns`, `clickstack_emerging_signals` and dashboard event-pattern tiles now apply the log source's `tableFilterExpression`, so their counts can drop for sources that set one. `minePatterns` now reports each cluster's own template; before, it could report the template of a different same-length cluster that some members did not fit. diff --git a/packages/api/src/fixtures.ts b/packages/api/src/fixtures.ts index 47d2edafd8..26df966c5a 100644 --- a/packages/api/src/fixtures.ts +++ b/packages/api/src/fixtures.ts @@ -434,6 +434,7 @@ export const bulkInsertLogs = async ( ServiceName: string; SeverityText: string; Timestamp: Date; + LogAttributes?: Record; }[], ) => { if (!config.IS_CI) { diff --git a/packages/api/src/mcp/__tests__/dashboards/queryTile.int.test.ts b/packages/api/src/mcp/__tests__/dashboards/queryTile.int.test.ts index d2c2ab73a9..48de1b8f67 100644 --- a/packages/api/src/mcp/__tests__/dashboards/queryTile.int.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/queryTile.int.test.ts @@ -891,6 +891,30 @@ describe('MCP Dashboard Tools - clickstack_query_tile', () => { }; }; + // clickstack_search has no variable context, so each snippet must carry + // the expanded selection and return only the selected service's rows. + const expectDrilldownServices = async (text: string, service: string) => { + const { patterns } = JSON.parse(text); + expect(patterns.length).toBeGreaterThan(0); + for (const p of patterns) { + expect(p.whereSnippet).not.toContain('$__filter'); + expect(p.whereSnippet).not.toContain('$service'); + const search = await callTool(ctx.client!, 'clickstack_search', { + sourceId: logSourceId, + startTime, + endTime, + where: p.whereSnippet, + whereLanguage: 'sql', + columns: 'ServiceName', + }); + expect(search.isError).toBeFalsy(); + const rows: { ServiceName: string }[] = JSON.parse(getFirstText(search)) + .result.data; + expect(rows.length).toBeGreaterThan(0); + for (const r of rows) expect(r.ServiceName).toBe(service); + } + }; + it('expands $__filter to a no-op on an event_patterns tile with nothing selected', async () => { const dashboard = await saveDashboard([eventPatternsTile()]); @@ -931,6 +955,7 @@ describe('MCP Dashboard Tools - clickstack_query_tile', () => { expect(sampledCount).toBe(2); expect(bodies.some(b => b.includes('checkout'))).toBe(true); expect(bodies.some(b => b.includes('payments'))).toBe(false); + await expectDrilldownServices(getFirstText(result), 'checkout'); }); it('expands a Lucene variable reference on an event_patterns tile', async () => { @@ -953,6 +978,7 @@ describe('MCP Dashboard Tools - clickstack_query_tile', () => { expect(totalCount).toBe(1); expect(bodies.some(b => b.includes('checkout'))).toBe(false); expect(bodies.some(b => b.includes('payments'))).toBe(true); + await expectDrilldownServices(getFirstText(result), 'payments'); }); it('reports an event_patterns macro naming a variable the dashboard does not declare', async () => { diff --git a/packages/api/src/mcp/__tests__/query.test.ts b/packages/api/src/mcp/__tests__/query.test.ts index 0855c40bc7..fca56d7a97 100644 --- a/packages/api/src/mcp/__tests__/query.test.ts +++ b/packages/api/src/mcp/__tests__/query.test.ts @@ -21,6 +21,7 @@ import { INCREASE_TOP_N_CAP, isServerError, mergeWhereIntoSelectItems, + normalizeBodyExpression, parseTimeRange, } from '@/mcp/tools/query/helpers'; import { @@ -107,6 +108,55 @@ describe('parseTimeRange', () => { // ─── mergeWhereIntoSelectItems ─────────────────────────────────────────────── +describe('normalizeBodyExpression', () => { + it.each([ + ['Body', 'Body'], + ['SpanName', 'SpanName'], + ["SpanAttributes['http.url']", "SpanAttributes['http.url']"], + [ + "JSONExtractString(Body, 'message')", + "JSONExtractString(Body, 'message')", + ], + [ + "JSONExtractString(Body,'error', 'message')", + "JSONExtractString(Body, 'error', 'message')", + ], + [ + "JSONExtractString(Body, 'app.user.currency')", + "JSONExtractString(Body, 'app.user.currency')", + ], + [ + "JSONExtractString(LogAttributes['object'], 'regarding', 'kind')", + "JSONExtractString(LogAttributes['object'], 'regarding', 'kind')", + ], + [ + String.raw`JSONExtractString(Body, 'it\'s, ok')`, + String.raw`JSONExtractString(Body, 'it\'s, ok')`, + ], + ])('accepts %s', (expr, expected) => { + expect(normalizeBodyExpression(expr)).toBe(expected); + }); + + it.each([ + 'Body) OR (1=1', + 'lower(Body)', + "JSONExtract(Body, 'message', 'String')", + 'JSONExtractString(Body)', + 'JSONExtractString(Body, message)', + "JSONExtractString(lower(Body), 'message')", + "JSONExtractString(Body, 'm') OR 1=1", + "JSONExtractString(Body, 'm'), Body", + "JSONExtractString(Body, 'm' || 'x')", + "JSONExtractString(Body, 'm')) OR (lower(Body)", + "JSONExtractString(LogAttributes['a'], 'm') OR (1=1", + "JSONExtractString(Body, 'm', (SELECT 1))", + // A stray quote in the column would pair with a key's quotes. + "JSONExtractString(Body', ' OR 1=1 --')", + ])('rejects %s', expr => { + expect(normalizeBodyExpression(expr)).toBeNull(); + }); +}); + describe('mergeWhereIntoSelectItems', () => { it('should return items unchanged when topWhere is empty', () => { const items = [{ where: 'level:error', whereLanguage: 'lucene' as const }]; diff --git a/packages/api/src/mcp/__tests__/queryTool.int.test.ts b/packages/api/src/mcp/__tests__/queryTool.int.test.ts index c4926279e3..d2ea418580 100644 --- a/packages/api/src/mcp/__tests__/queryTool.int.test.ts +++ b/packages/api/src/mcp/__tests__/queryTool.int.test.ts @@ -18,9 +18,10 @@ import { getServer, seedExponentialHistogramMetric, } from '@/fixtures'; +import * as patternDrilldown from '@/mcp/tools/query/patternDrilldown'; import { McpContext } from '@/mcp/tools/types'; import Connection from '@/models/connection'; -import { Source, type SourceDocument } from '@/models/source'; +import { LogSource, Source, type SourceDocument } from '@/models/source'; import Team from '@/models/team'; import { callTool, createTestClient, getFirstText } from './mcpTestUtils'; @@ -687,6 +688,345 @@ describe('MCP Query Tools', () => { expect(hasNonZeroTrend).toBe(true); }); }); + + describe('drill-down, extraction and sampling', () => { + const now = new Date(); + const base = new Date(now.getTime() - 5 * 60 * 1000); + const window = { + startTime: new Date(now.getTime() - 10 * 60 * 1000).toISOString(), + endTime: new Date(now.getTime() + 60 * 1000).toISOString(), + }; + + const tricky = [ + ...['k1', 'k2', 'k3'].map( + k => + String.raw`Lookup "${k}" in C:\data\idx it's (a|b)*[c]+ $1 ^x {n} .?`, + ), + ...['a1', 'b2'].map(id => `Zahlung für ${id} fehlgeschlagen ✓ 日本`), + // Drain splits on these; RE2's \s alone would not. + ...['1', '2'].map(n => `\uFEFFReset ${n}\u00A0done`), + ...['1\u202F234', '5\u202F678'].map(n => `Paid ${n} EUR`), + ]; + + beforeEach(async () => { + let t = 0; + const at = () => new Date(base.getTime() + t++ * 1000); + const logs: Parameters[0] = []; + for (const n of [10, 20, 30, 40, 50, 60]) { + logs.push({ + Body: `Sending ${n} rows`, + ServiceName: 'svc-a', + SeverityText: 'INFO', + Timestamp: at(), + }); + } + for (const n of [1, 2, 3]) { + logs.push({ + Body: `Sending ${n} more rows`, + ServiceName: 'svc-a', + SeverityText: 'INFO', + Timestamp: at(), + }); + } + for (const n of [70, 80, 90, 100]) { + logs.push({ + Body: `Sending ${n} rows`, + ServiceName: 'svc-b', + SeverityText: 'INFO', + Timestamp: at(), + }); + } + for (const body of tricky) { + logs.push({ + Body: body, + ServiceName: 'tricky', + SeverityText: 'WARN', + Timestamp: at(), + }); + } + for (const s of [5, 15, 30]) { + logs.push({ + Body: 'request failed', + ServiceName: 'attrs', + SeverityText: 'ERROR', + Timestamp: at(), + LogAttributes: { 'exception.message': `Timeout after ${s} s` }, + }); + } + logs.push({ + Body: 'request failed', + ServiceName: 'attrs', + SeverityText: 'ERROR', + Timestamp: at(), + }); + for (const id of [123, 456, 789]) { + logs.push({ + Body: JSON.stringify({ + level: 'error', + message: `Payment ${id} declined by issuer`, + }), + ServiceName: 'json', + SeverityText: 'ERROR', + Timestamp: at(), + }); + } + for (const body of [ + JSON.stringify({ message: 5 }), + JSON.stringify({ message: null }), + JSON.stringify({ message: { nested: 'x' } }), + JSON.stringify({ other: 'x' }), + JSON.stringify({ message: '' }), + 'not json at all', + ]) { + logs.push({ + Body: body, + ServiceName: 'json', + SeverityText: 'ERROR', + Timestamp: at(), + }); + } + await bulkInsertLogs(logs); + }); + + const mine = async (args: Record) => { + const result = await callTool(client, 'clickstack_event_patterns', { + sourceId: logSource._id.toString(), + ...window, + ...args, + }); + expect(result.isError).toBeFalsy(); + return JSON.parse(getFirstText(result)); + }; + + const drill = async (output: any, pattern: any) => { + expect(pattern.whereLanguage).toBe('sql'); + expect(pattern.whereSnippet).not.toBe(''); + const { drilldown } = output.summary; + expect(drilldown.tool).toBe('clickstack_search'); + const result = await callTool(client, 'clickstack_search', { + sourceId: drilldown.sourceId, + startTime: drilldown.startTime, + endTime: drilldown.endTime, + where: pattern.whereSnippet, + whereLanguage: pattern.whereLanguage, + columns: 'Body, ServiceName', + maxResults: 200, + }); + expect(result.isError).toBeFalsy(); + const parsed: { + result: { data: { Body: string; ServiceName: string }[] }; + } = JSON.parse(getFirstText(result)); + return parsed.result.data; + }; + + const findPattern = (output: any, template: string) => { + const p = output.patterns.find((x: any) => x.pattern === template); + expect(p).toBeDefined(); + return p; + }; + + it('drills into a template with a mid-message variable and keeps a Lucene filter', async () => { + const output = await mine({ where: 'ServiceName:svc-a' }); + expect(output.summary.drilldown.sourceId).toBe( + logSource._id.toString(), + ); + + const rows = await drill( + output, + findPattern(output, 'Sending <*> rows'), + ); + expect(rows).toHaveLength(6); + for (const r of rows) { + expect(r.Body).toMatch(/^Sending \d+ rows$/); + expect(r.ServiceName).toBe('svc-a'); + } + + const more = await drill( + output, + findPattern(output, 'Sending <*> more rows'), + ); + expect(more).toHaveLength(3); + for (const r of more) expect(r.Body).toMatch(/^Sending \d more rows$/); + }); + + it('keeps a SQL filter in the drill-down', async () => { + const output = await mine({ + where: "ServiceName = 'svc-b'", + whereLanguage: 'sql', + }); + const rows = await drill( + output, + findPattern(output, 'Sending <*> rows'), + ); + expect(rows).toHaveLength(4); + for (const r of rows) expect(r.ServiceName).toBe('svc-b'); + }); + + it('builds predicates that parse and match messages with quotes, backslashes, regex characters and Unicode', async () => { + const output = await mine({ where: 'ServiceName:tricky' }); + expect(output.summary.totalCount).toBe(tricky.length); + expect(output.patterns.length).toBeGreaterThan(0); + + let drilled = 0; + for (const p of output.patterns) { + const rows = await drill(output, p); + // The sample covers every row and the fixture shapes do not overlap, + // so the drill-down must return exactly the cluster's members. + expect(rows).toHaveLength(p.sampleCount); + for (const r of rows) expect(tricky).toContain(r.Body); + drilled += rows.length; + } + expect(drilled).toBe(tricky.length); + }); + + it('mines a map attribute and drills into it with SQL, not Lucene bracket syntax', async () => { + const output = await mine({ + bodyExpression: "LogAttributes['exception.message']", + where: "LogAttributes['exception.message'] != ''", + whereLanguage: 'sql', + }); + expect(output.summary.totalCount).toBe(3); + const p = findPattern(output, 'Timeout after <*> s'); + expect(p.whereSnippet).toContain( + "match(LogAttributes['exception.message'],", + ); + expect(p.whereSnippet).not.toContain("']:"); + + const rows = await drill(output, p); + expect(rows).toHaveLength(3); + for (const r of rows) expect(r.ServiceName).toBe('attrs'); + }); + + it('mines a field inside JSON bodies with JSONExtractString and drills with the same expression', async () => { + const expr = "JSONExtractString(Body, 'message')"; + const output = await mine({ + bodyExpression: expr, + where: `ServiceName = 'json' AND ${expr} != ''`, + whereLanguage: 'sql', + }); + expect(output.summary.bodyColumn).toBe(expr); + // Non-JSON bodies and missing / null / empty values extract to '' and + // are filtered out. How numbers and objects extract varies by + // ClickHouse version, so only the payment strings are asserted. + expect(output.summary.totalCount).toBeGreaterThanOrEqual(3); + const allSamples: string[] = output.patterns.flatMap( + (x: any) => x.samples, + ); + expect(allSamples).not.toContain(''); + + const p = findPattern(output, 'Payment <*> declined by issuer'); + expect(p.sampleCount).toBe(3); + expect(p.whereSnippet).toContain(`match(${expr},`); + const rows = await drill(output, p); + expect(rows).toHaveLength(3); + for (const r of rows) { + expect(JSON.parse(r.Body).message).toMatch(/^Payment \d+ declined/); + } + }); + + it('rejects function calls in bodyExpression other than JSONExtractString over a column', async () => { + for (const bodyExpression of [ + 'lower(Body)', + "JSONExtractString(lower(Body), 'message')", + "JSONExtractString(Body, 'message') OR (1=1", + ]) { + const result = await callTool(client, 'clickstack_event_patterns', { + sourceId: logSource._id.toString(), + bodyExpression, + ...window, + }); + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('bodyExpression'); + } + }); + + it('applies the source table filter to mining and to the drill-down', async () => { + // The field lives on the Log discriminator; Source.updateOne drops it. + await LogSource.updateOne( + { _id: logSource._id }, + { $set: { tableFilterExpression: "ServiceName != 'svc-b'" } }, + ); + const output = await mine({}); + const p = findPattern(output, 'Sending <*> rows'); + expect(p.sampleCount).toBe(6); + expect(p.whereSnippet).toContain("ServiceName != 'svc-b'"); + const rows = await drill(output, p); + expect(rows).toHaveLength(6); + for (const r of rows) expect(r.ServiceName).toBe('svc-a'); + expect(output.summary.drilldown.includesFilters).toBe(true); + expect(output.summary.drilldown.warning).toBeUndefined(); + }); + + it('keeps the table filter and warns when the where filter cannot be rendered as SQL', async () => { + await LogSource.updateOne( + { _id: logSource._id }, + { $set: { tableFilterExpression: "ServiceName != 'svc-b'" } }, + ); + const spy = jest + .spyOn(patternDrilldown, 'userWhereAsSql') + .mockRejectedValueOnce(new Error('cannot render')); + try { + const output = await mine({ where: 'SeverityText:INFO' }); + expect(output.summary.drilldown.includesFilters).toBe(false); + expect(output.summary.drilldown.warning).toContain('table filter'); + const p = findPattern(output, 'Sending <*> rows'); + expect(p.whereSnippet).toContain("ServiceName != 'svc-b'"); + const rows = await drill(output, p); + expect(rows).toHaveLength(6); + for (const r of rows) expect(r.ServiceName).toBe('svc-a'); + } finally { + spy.mockRestore(); + } + }); + + it('separates sampled counts from estimates', async () => { + const output = await mine({ + where: 'ServiceName:svc-a', + sampleSize: 3, + }); + const { summary } = output; + expect(summary.totalCount).toBe(9); + expect(summary.sampledCount).toBe(3); + expect(summary.sampleCoverage).toBeCloseTo(3 / 9, 3); + expect(summary.sampleMultiplier).toBe(3); + const sampledTotal = output.patterns.reduce( + (n: number, p: any) => n + p.sampleCount, + 0, + ); + expect(sampledTotal).toBe(3); + for (const p of output.patterns) { + expect(p.estimatedCount).toBe(p.sampleCount * 3); + } + }); + + it('reports the effective trend buckets, and omits trends when trendBuckets is 0', async () => { + const withTrend = await mine({ + where: 'ServiceName:svc-a', + trendBuckets: 24, + }); + const { trend } = withTrend.summary; + expect(trend.requestedBuckets).toBe(24); + expect(trend.bucketCount).toBeLessThanOrEqual(24); + expect(typeof trend.bucketInterval).toBe('string'); + expect(new Date(trend.firstBucketStart).getTime()).toBeLessThanOrEqual( + new Date(withTrend.summary.timeRange.start).getTime(), + ); + expect(new Date(trend.lastBucketEnd).getTime()).toBeGreaterThanOrEqual( + new Date(withTrend.summary.timeRange.end).getTime(), + ); + for (const p of withTrend.patterns) { + expect(p.trend).toHaveLength(trend.bucketCount); + } + + const noTrend = await mine({ + where: 'ServiceName:svc-a', + trendBuckets: 0, + }); + expect(noTrend.summary.trend).toBeUndefined(); + for (const p of noTrend.patterns) expect(p.trend).toBeUndefined(); + expect(noTrend.patterns.length).toBeGreaterThan(0); + }); + }); }); // ─── clickstack_sql ──────────────────────────────────────────────────────────── diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index 69c8f4eb17..ea024d118a 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -1458,8 +1458,10 @@ the most log volume", or similar pattern-discovery questions. It samples random events and clusters them with the Drain algorithm, returning: - pattern templates (with <*> wildcards for variable parts) - - estimated counts (scaled from the sample to the full time range) - - time-bucketed trends + - sampleCount (rows in the sample) and estimatedCount (sampleCount × + sampleMultiplier, an extrapolation, not an exact count) + - time-bucketed trends, also extrapolated; summary.trend gives the actual + bucket interval and boundaries - a whereSnippet for each pattern to drill into matching raw events Required parameters: @@ -1469,15 +1471,37 @@ Required parameters: Optional parameters: - where: filter to specific services or severity levels before mining - sampleSize: increase for more accurate patterns (default 10000, max 25000) - - bodyExpression: column to mine (auto-detected: Body for logs, SpanName for traces) - -Example: find top patterns for production services over the last 4 hours: - clickstack_event_patterns({ - sourceId: "", - startTime: "<4 hours ago ISO>", - endTime: "", - where: "service.name:prod-*" - }) + - bodyExpression: what to mine (auto-detected: Body for logs, SpanName for + traces). A column or map reference, or JSONExtractString over one with + string-literal keys, e.g. JSONExtractString(Body, 'error', 'message'). + Other function calls are rejected. + - trendBuckets: maximum trend buckets (0 disables trends) + +Examples: + "What kinds of errors is checkout producing?" + { where: "ServiceName:checkout AND SeverityText:error", topN: 5, sampleSize: 1000 } + "What messages dominate this service?" + { where: "ServiceName:api", trendBuckets: 0 } + Patterns in extracted exception names (existing map attribute): + { bodyExpression: "LogAttributes['exception.type']", + where: "LogAttributes['exception.type'] != ''", whereLanguage: "sql" } + A field inside JSON bodies (exclude rows without it): + { bodyExpression: "JSONExtractString(Body, 'message')", + where: "JSONExtractString(Body, 'message') != ''", whereLanguage: "sql" } + +Drilling into a pattern: call clickstack_search with the same sourceId, +startTime and endTime, where = whereSnippet, whereLanguage = "sql". The snippet +already includes the source table filter and, unless +summary.drilldown.includesFilters is false, your where filter, so do not add +them again. Skip patterns with drilldownUnavailable. It matches events whose +value has the template's literal tokens in order with one token per <*>; this +can include events Drain put in a more specific cluster. + +Sampling limits: a pattern or trend bucket with 0 sampled events can still have +real events, especially when sampleMultiplier is large. For exact counts or +breakdowns by a known field use clickstack_table / clickstack_timeseries; for +patterns that are new or gone versus an earlier window use +clickstack_emerging_signals. == COMMON MISTAKES == diff --git a/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts b/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts new file mode 100644 index 0000000000..0e9e8a5bd3 --- /dev/null +++ b/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts @@ -0,0 +1,177 @@ +import { minePatterns } from '@hyperdx/common-utils/dist/drain'; + +import { + andSql, + buildPatternPredicate, + MAX_PREDICATE_TEMPLATE_LENGTH, + templateToRegex, +} from '@/mcp/tools/query/patternDrilldown'; + +// RE2 spells code points \x{HHHH} and categories \pZ; JS (with the u flag) +// spells them \u{HHHH} and \p{Z}. ClickHouse behaviour is covered by +// integration tests. +function toJsRegex(re2: string): RegExp { + return RegExp( + re2 + .replace(/\\x\{([0-9A-Fa-f]+)\}/g, '\\u{$1}') + .replace(/(? { + it('keeps the variable position between literal tokens', () => { + const t = 'Sending <*> rows'; + expect(matches(t, 'Sending 10 rows')).toBe(true); + expect(matches(t, 'Sending 20 rows')).toBe(true); + + // The old Lucene snippet (Body:"Sending rows") required these two words + // to be adjacent, so it matched none of the real messages above. The new + // predicate must not broaden either: each of these differs in shape. + expect(matches(t, 'Sending rows')).toBe(false); + expect(matches(t, 'Sending 10 more rows')).toBe(false); + expect(matches(t, 'Sending 10 rows now')).toBe(false); + expect(matches(t, 'Resending 10 rows')).toBe(false); + expect(matches(t, 'Sending 10 rowsX')).toBe(false); + expect(matches(t, 'rows 10 Sending')).toBe(false); + }); + + it('matches whole tokens only', () => { + expect(matches('user <*> logged in', 'user bob logged in')).toBe(true); + expect(matches('user <*> logged in', 'superuser bob logged in')).toBe( + false, + ); + expect(matches('user <*> logged in', 'user bob logged inside')).toBe(false); + expect(matches('a <*>', 'ab 1')).toBe(false); + }); + + it('handles repeated variables', () => { + const t = 'copy <*> to <*> from <*>'; + expect(matches(t, 'copy a.txt to /tmp from host-1')).toBe(true); + expect(matches(t, 'copy a.txt to /tmp from')).toBe(false); + expect(matches(t, 'copy a.txt b.txt to /tmp from host-1')).toBe(false); + }); + + it('treats a template made only of variables as a token-count match', () => { + expect(matches('<*> <*>', 'alpha beta')).toBe(true); + expect(matches('<*> <*>', 'alpha')).toBe(false); + expect(matches('<*> <*>', 'alpha beta gamma')).toBe(false); + }); + + it('escapes regex metacharacters, quotes and backslashes in literals', () => { + const literal = String.raw`path=C:\temp "quoted" it's (x)*[y]+ $1 ^a|b {n} .?`; + const t = `${literal} <*>`; + expect(matches(t, `${literal} 42`)).toBe(true); + // `.` must be literal, not "any character". + expect(matches(t, `${literal.replace('.?', 'x?')} 42`)).toBe(false); + // `*` and `+` must be literal, not quantifiers. + expect(matches(t, `${literal.replace('(x)*', '(x)(x)')} 42`)).toBe(false); + expect(matches(t, `${literal.replace('[y]+', '[y]]')} 42`)).toBe(false); + }); + + it('matches Unicode tokens separated by spaces, tabs and newlines', () => { + const t = 'café <*> 日本'; + expect(matches(t, 'café x 日本')).toBe(true); + expect(matches(t, ' café\n\tx 日本 \n')).toBe(true); + expect(matches(t, 'cafe x 日本')).toBe(false); + // U+200B (zero-width space) is not whitespace to Drain, so it is part of + // the token and the value has only two tokens. + expect(matches(t, 'café\u200Bx 日本')).toBe(false); + }); + + it('treats the Unicode whitespace Drain splits on as separators', () => { + expect(matches('Connection <*> reset', '\uFEFFConnection 42 reset')).toBe( + true, + ); + expect(matches('<*> <*> EUR', '1\u202F234 EUR')).toBe(true); + expect(matches('a <*> b', 'a\u00A0x\u3000b')).toBe(true); + expect(matches('a <*> b', 'a x\u00A0y b')).toBe(false); + }); + + it('a variable spans one token, even a multi-byte one', () => { + expect(matches('id <*> ok', 'id 日本語 ok')).toBe(true); + expect(matches('id <*> ok', 'id 日本 語 ok')).toBe(false); + }); + + it('turns an empty template into a whitespace-only match', () => { + expect(matches('', '')).toBe(true); + expect(matches('', ' \n')).toBe(true); + expect(matches('', 'x')).toBe(false); + }); + + it('matches every sampled member of every Drain cluster, and nothing from other shapes', () => { + const messages = [ + ...[10, 20, 30, 40].map(n => `Sending ${n} rows`), + ...[1, 2, 3].map(n => `Sending ${n} more rows`), + ...['alice', 'bob', 'carol'].map( + u => `User ${u} logged in from 10.0.0.${u.length}`, + ), + ...['a', 'b'].map(k => `cache miss for key "${k}\\x" (retry)`), + 'café ünïcode 日本 message', + ]; + const { patterns } = minePatterns( + messages.map(body => ({ body })), + { + totalCount: messages.length, + startDate: new Date(0), + endDate: new Date(1000), + trendBuckets: 0, + maxSamples: messages.length, + getBody: r => r.body, + getTimestamp: () => 0, + }, + ); + expect(patterns.length).toBeGreaterThan(1); + + for (const p of patterns) { + const re = toJsRegex(templateToRegex(p.pattern)); + const members = p.samples.map(s => s.body); + expect(members).toHaveLength(p.sampleCount); + for (const m of members) expect(re.test(m)).toBe(true); + // Members of other clusters with a different shape must not match. + const others = patterns + .filter(o => o !== p) + .flatMap(o => o.samples.map(s => s.body)); + for (const o of others) { + if (re.test(o)) { + // Only allowed when that cluster's template is a specialization + // of this one (same shape); none of these fixtures overlap. + throw new Error(`"${p.pattern}" unexpectedly matched "${o}"`); + } + } + } + }); +}); + +describe('buildPatternPredicate', () => { + it('wraps the selected value expression in match() with an escaped literal', () => { + const predicate = buildPatternPredicate( + "LogAttributes['exception.type']", + String.raw`it's C:\x <*>`, + ); + expect(predicate).toMatch(/^match\(LogAttributes\['exception\.type'\], '/); + // Quote doubled for SQL, backslash doubled for SQL on top of the regex escape. + expect(predicate).toContain("it''s"); + expect(predicate).toContain(String.raw`C:\\\\x`); + }); + + it('returns null for templates too long to turn into a predicate', () => { + const long = 'x'.repeat(MAX_PREDICATE_TEMPLATE_LENGTH + 1); + expect(buildPatternPredicate('Body', long)).toBeNull(); + }); +}); + +describe('andSql', () => { + it('skips blanks and parenthesizes when combining', () => { + expect(andSql()).toBe(''); + expect(andSql('', undefined, ' ')).toBe(''); + expect(andSql('a = 1')).toBe('a = 1'); + expect(andSql('a = 1 OR b = 2', '', 'c = 3')).toBe( + '(a = 1 OR b = 2) AND (c = 3)', + ); + }); +}); diff --git a/packages/api/src/mcp/tools/query/eventPatterns.ts b/packages/api/src/mcp/tools/query/eventPatterns.ts index fc27683a1e..6e09c127c5 100644 --- a/packages/api/src/mcp/tools/query/eventPatterns.ts +++ b/packages/api/src/mcp/tools/query/eventPatterns.ts @@ -30,15 +30,20 @@ const eventPatternsSchema = z.object({ .default(10_000) .describe( 'Random rows to sample for pattern mining. ' + - 'Higher values produce more accurate patterns but take longer.', + 'Higher values produce more accurate patterns but take longer. ' + + 'Patterns rarer than about 1/sampleSize of matching rows may not appear at all.', ), bodyExpression: z .string() .optional() .describe( - 'Column expression to mine patterns from. ' + + 'Expression to mine patterns from: a column or map reference, or JSONExtractString ' + + 'over one with string-literal keys to mine a field inside a JSON body. ' + 'Auto-detected from the source if omitted (Body for logs, SpanName for traces). ' + - 'Example: "Body", "SpanName", "SpanAttributes[\'http.url\']"', + 'Examples: "Body", "SpanName", "LogAttributes[\'exception.type\']", ' + + "\"JSONExtractString(Body, 'error', 'message')\". Other function calls are rejected. " + + "Rows where the value is '' (missing key, invalid JSON) are mined as an empty " + + 'pattern; add where " != \'\'" (whereLanguage:"sql") to exclude them.', ), topN: z .number() @@ -54,9 +59,12 @@ const eventPatternsSchema = z.object({ .optional() .default(24) .describe( - 'Number of time buckets for per-pattern trend sparklines. ' + - 'Compare bucket counts at start vs end to spot patterns that started or stopped mid-window. ' + - 'Set to 0 to disable trend computation (smaller response).', + 'Maximum number of time buckets for per-pattern trends. The bucket width is rounded ' + + 'up to a standard interval (15s, 1m, 5m, 15m, 30m, 1h, ...) and aligned to it, so ' + + 'fewer buckets can come back and the first can start before startTime; ' + + 'summary.trend reports the actual interval and boundaries. ' + + 'Trend counts are extrapolated from the sample, so sparse buckets read 0 even when ' + + 'events occurred. Set to 0 to disable trends (smaller response).', ), startTime: startTimeSchema, endTime: endTimeSchema, @@ -76,24 +84,28 @@ export function registerEventPatterns({ title: 'Event Pattern Mining', annotations: READ_ONLY_TOOL, description: - 'Discover the most common log messages and event patterns. ' + - 'Samples random events, clusters them using the Drain algorithm, and returns ' + - 'patterns sorted by frequency with estimated counts and time trends.\n\n' + - 'PREFER THIS TOOL over clickstack_search or clickstack_table when the goal is to ' + - 'understand what kinds of messages, errors, or events exist — e.g. "sample logs", ' + - '"what errors are happening", "show me common messages", "what does this service log". ' + - 'It returns frequency-ranked patterns instead of raw rows, giving a much better overview.\n\n' + - 'Also use when asked about "top patterns", "common logs", "noisy services", ' + - '"recurring messages", or log noise analysis.\n\n' + - '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' + + 'Discover what kinds of messages, errors, or events exist. Samples random events, ' + + 'clusters them into templates (Drain), and returns patterns ranked by frequency with ' + + 'sampled and estimated counts, optional trends, and raw samples. Use for "what errors ' + + 'are happening", common or noisy logs, recurring messages.\n\n' + + 'Examples:\n' + + ' - "What kinds of errors is checkout producing?" → where:"ServiceName:checkout AND ' + + 'SeverityText:error", topN:5, sampleSize:1000 (trends on by default)\n' + + ' - "What messages dominate this service?" → where:"ServiceName:api", trendBuckets:0\n' + + ' - Patterns in a map attribute → bodyExpression:"LogAttributes[\'exception.type\']", ' + + 'where:"LogAttributes[\'exception.type\'] != \'\'", whereLanguage:"sql"\n' + + ' - A field inside JSON bodies → bodyExpression:"JSONExtractString(Body, \'message\')", ' + + 'where:"JSONExtractString(Body, \'message\') != \'\'", whereLanguage:"sql"\n\n' + + 'Use instead: exact counts or breakdowns by a known field → clickstack_table / ' + + 'clickstack_timeseries; raw rows → clickstack_search; patterns new or gone vs an ' + + 'earlier window → clickstack_emerging_signals.\n\n' + + 'Counts come from a sample: estimatedCount is an extrapolation, and a pattern with ' + + '0 sampled events may still exist.\n\n' + + 'Drill down: call clickstack_search with the same sourceId and time range, ' + + 'where = a pattern\'s whereSnippet, whereLanguage = "sql". whereSnippet already ' + + 'includes your where filter unless summary.drilldown.includesFilters is false.\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' + - ' - clickstack_table: aggregated metrics / counts / top-N', + PREFER_BUILDER_OVER_SQL_NUDGE, inputSchema: eventPatternsSchema, }, async input => { diff --git a/packages/api/src/mcp/tools/query/helpers.ts b/packages/api/src/mcp/tools/query/helpers.ts index 028164edbe..aa808414eb 100644 --- a/packages/api/src/mcp/tools/query/helpers.ts +++ b/packages/api/src/mcp/tools/query/helpers.ts @@ -78,7 +78,35 @@ export function resolveBodyExpression( /** Reject bodyExpression values containing SQL-unsafe characters. */ // eslint-disable-next-line no-useless-escape -export const SAFE_BODY_EXPR_CHARS = /^[\w.':\[\]\-]+$/; +const SAFE_BODY_EXPR_CHARS = /^[\w.':\[\]\-]+$/; + +const COLUMN_OR_MAP_REF = /^[A-Za-z_][\w.]*$|^[A-Za-z_][\w.]*\['[^'\\]*'\]$/; +const SQL_STRING_LITERAL = /^'([^'\\]|\\.)*'$/; +const JSON_EXTRACT_STRING_CALL = /^JSONExtractString\((.*)\)$/s; + +/** + * Return a caller-supplied bodyExpression in a form that is safe to embed in + * SQL, or null when it is not allowed. Accepts a column or map reference, or + * JSONExtractString(, '', ...), the form the app + * generates for JSON fields. The JSONExtractString form is rebuilt from its + * validated arguments, so nothing else reaches the SQL. + */ +export function normalizeBodyExpression(expression: string): string | null { + const trimmed = expression.trim(); + if (SAFE_BODY_EXPR_CHARS.test(trimmed)) return trimmed; + const call = JSON_EXTRACT_STRING_CALL.exec(trimmed); + if (!call) return null; + const [column, ...keys] = splitAndTrimWithBracket(call[1]); + if ( + column == null || + keys.length === 0 || + !COLUMN_OR_MAP_REF.test(column) || + !keys.every(k => SQL_STRING_LITERAL.test(k)) + ) { + return null; + } + return `JSONExtractString(${[column, ...keys].join(', ')})`; +} // ─── Safety limits ─────────────────────────────────────────────────────────── diff --git a/packages/api/src/mcp/tools/query/patternDrilldown.ts b/packages/api/src/mcp/tools/query/patternDrilldown.ts new file mode 100644 index 0000000000..49c1a62843 --- /dev/null +++ b/packages/api/src/mcp/tools/query/patternDrilldown.ts @@ -0,0 +1,111 @@ +import type { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; +import { escapeSqlString } from '@hyperdx/common-utils/dist/core/utils'; +import { + CustomSchemaSQLSerializerV2, + SearchQueryBuilder, +} from '@hyperdx/common-utils/dist/queryParser'; +import type { + ChartVariable, + UseTextIndex, +} from '@hyperdx/common-utils/dist/types'; +import { substituteVariables } from '@hyperdx/common-utils/dist/variables'; + +const PATTERN_WILDCARD = '<*>'; + +/** + * Templates longer than this get no predicate: the regex (and the SQL that + * embeds it) would be unwieldy, and very long templates are usually a + * single unparameterized payload rather than a reusable shape. + */ +export const MAX_PREDICATE_TEMPLATE_LENGTH = 4000; + +export const PATTERN_MATCH_SEMANTICS = + 'whereSnippet matches values with the same whitespace-separated token count as the ' + + 'template, the same literal tokens (case-sensitive) in the same positions, and exactly ' + + 'one non-whitespace token at each <*>. Every sampled member of the cluster matches. ' + + 'It can also match unsampled events whose shape fits, including ones Drain would have ' + + 'put in a more specific cluster, so the drill-down is a superset of cluster membership.'; + +// Drain tokenizes with JS `\s`, which includes Unicode spaces and BOM; RE2's +// `\s` is ASCII-only. JS `\s` = RE2 `\s` + \v + \pZ (Zs/Zl/Zp) + BOM. `\pZ` +// keeps the snippet short; spelling out the code points repeats ~100 chars +// between every token. +const JS_WHITESPACE = '\\s\\x{0B}\\pZ\\x{FEFF}'; +const WS = `[${JS_WHITESPACE}]`; +const NON_WS = `[^${JS_WHITESPACE}]`; + +function escapeRe2Literal(token: string): string { + return token.replace(/[\\^$.|?*+()[\]{}]/g, '\\$&'); +} + +/** Anchored RE2 regex that a value matches when it fits the Drain template. */ +export function templateToRegex(template: string): string { + const tokens = template.split(/\s+/).filter(t => t.length > 0); + const body = tokens + .map(t => (t === PATTERN_WILDCARD ? `${NON_WS}+` : escapeRe2Literal(t))) + .join(`${WS}+`); + return `^${WS}*${body}${WS}*$`; +} + +/** + * SQL predicate selecting events whose `valueExpression` fits the template, + * or null when the template is too long to turn into a predicate. + */ +export function buildPatternPredicate( + valueExpression: string, + template: string, +): string | null { + if (template.length > MAX_PREDICATE_TEMPLATE_LENGTH) return null; + return `match(${valueExpression}, '${escapeSqlString(templateToRegex(template))}')`; +} + +/** AND together SQL conditions, skipping blanks. */ +export function andSql(...conditions: Array): string { + const parts = conditions.filter( + (c): c is string => c != null && c.trim() !== '', + ); + if (parts.length <= 1) return parts[0] ?? ''; + return parts.map(c => `(${c})`).join(' AND '); +} + +/** + * Render the caller's filter as SQL so it can be ANDed with a pattern + * predicate in one clickstack_search `where`. Dashboard variables are + * expanded first because the search tool has no variable context. + */ +export async function userWhereAsSql({ + where, + whereLanguage, + variables, + metadata, + databaseName, + tableName, + connectionId, + implicitColumnExpression, + useTextIndexForImplicitColumn, +}: { + where: string; + whereLanguage: 'lucene' | 'sql'; + variables?: ChartVariable[]; + metadata: Metadata; + databaseName: string; + tableName: string; + connectionId: string; + implicitColumnExpression?: string; + useTextIndexForImplicitColumn?: UseTextIndex; +}): Promise { + if (!where.trim()) return ''; + const expanded = variables + ? substituteVariables(where, { variables, inputLanguage: whereLanguage }) + : where; + if (whereLanguage === 'sql') return expanded; + const serializer = new CustomSchemaSQLSerializerV2({ + metadata, + databaseName, + tableName, + connectionId, + implicitColumnExpression, + useTextIndexForImplicitColumn, + }); + return new SearchQueryBuilder(expanded, serializer).build(); +} diff --git a/packages/api/src/mcp/tools/query/runEventPatterns.ts b/packages/api/src/mcp/tools/query/runEventPatterns.ts index b9a2464c4d..97eceea23a 100644 --- a/packages/api/src/mcp/tools/query/runEventPatterns.ts +++ b/packages/api/src/mcp/tools/query/runEventPatterns.ts @@ -1,6 +1,8 @@ import { getMetadata } from '@hyperdx/common-utils/dist/core/metadata'; import { + convertGranularityToSeconds, getFirstTimestampValueExpression, + type Granularity, splitAndTrimWithBracket, } from '@hyperdx/common-utils/dist/core/utils'; import { minePatterns } from '@hyperdx/common-utils/dist/drain'; @@ -8,7 +10,8 @@ import type { ChartConfigWithDateRange, ChartVariable, } from '@hyperdx/common-utils/dist/types'; -import { DisplayType } from '@hyperdx/common-utils/dist/types'; +import { DisplayType, SourceKind } from '@hyperdx/common-utils/dist/types'; +import { trace } from '@opentelemetry/api'; import { ClickhouseClient } from '@/clickhouse'; import { getConnectionById } from '@/controllers/connection'; @@ -19,13 +22,21 @@ import { mcpUserError, } from '@/mcp/utils/errors'; import { MCP_QUERY_MAX_EXECUTION_SEC } from '@/mcp/utils/timeout'; +import logger from '@/utils/logger'; import { trimToolResponse } from '@/utils/trimToolResponse'; import { clickHouseErrorResult, + normalizeBodyExpression, resolveBodyExpression, - SAFE_BODY_EXPR_CHARS, } from './helpers'; +import { + andSql, + buildPatternPredicate, + MAX_PREDICATE_TEMPLATE_LENGTH, + PATTERN_MATCH_SEMANTICS, + userWhereAsSql, +} from './patternDrilldown'; // ─── Reusable window mining ────────────────────────────────────────────────── @@ -50,6 +61,8 @@ export async function mineWindowPatterns( abortSignal?: AbortSignal; /** Dashboard variables and their selected values. Omitted outside a dashboard context. */ variables?: ChartVariable[]; + /** Also render every row filter as one SQL condition for drill-down. */ + includeDrilldownWhere?: boolean; }, ): Promise< | { error: McpErrorResult } @@ -59,6 +72,15 @@ export async function mineWindowPatterns( totalCount: number; sampledCount: number; bodyColumn: string; + trendGranularity?: Granularity; + trendBucketStarts: number[]; + /** + * Row filters as one SQL condition ('' when there are none). Undefined + * when not requested. The source table filter is always included; the + * caller filter only when `includesUserFilter` is true (it could not be + * rendered as SQL otherwise). + */ + drilldown?: { where: string; includesUserFilter: boolean }; } > { const sampleSize = options?.sampleSize ?? 10_000; @@ -85,22 +107,26 @@ export async function mineWindowPatterns( }; } - // Sanitize caller-supplied bodyExpression: must be a single column reference - // matching the documented format (e.g. "Body", "SpanAttributes['http.url']"). - // The allowlist rejects injection attempts like "Body) OR (1=1". + // Sanitize caller-supplied bodyExpression: a single column/map reference or + // JSONExtractString over one. The allowlist rejects injection attempts like + // "Body) OR (1=1". let bodyColumn: string | undefined; if (options?.bodyExpression) { const parts = splitAndTrimWithBracket(options.bodyExpression); - if (parts.length !== 1 || !SAFE_BODY_EXPR_CHARS.test(parts[0])) { + const normalized = + parts.length === 1 ? normalizeBodyExpression(parts[0]) : null; + if (normalized == null) { return { error: mcpUserError( - 'bodyExpression must be a single column expression ' + - '(e.g. "Body", "SpanName", "SpanAttributes[\'http.url\']"). ' + - 'Multiple expressions, function calls, or sub-queries are not allowed.', + 'bodyExpression must be a single column or map reference ' + + '(e.g. "Body", "SpanName", "SpanAttributes[\'http.url\']"), or ' + + 'JSONExtractString over one with string-literal keys ' + + "(e.g. \"JSONExtractString(Body, 'error', 'message')\"). " + + 'Other function calls, multiple expressions, or sub-queries are not allowed.', ), }; } - bodyColumn = parts[0]; + bodyColumn = normalized; } else { bodyColumn = resolveBodyExpression(source); } @@ -114,6 +140,16 @@ export async function mineWindowPatterns( }; } + const tableFilterSql = + source.kind === SourceKind.Log && + 'tableFilterExpression' in source && + source.tableFilterExpression + ? source.tableFilterExpression + : undefined; + const filters = tableFilterSql + ? [{ type: 'sql' as const, condition: tableFilterSql }] + : []; + const clickhouseClient = new ClickhouseClient({ host: connection.host, username: connection.username, @@ -145,6 +181,7 @@ export async function mineWindowPatterns( }, where: options?.where ?? '', whereLanguage: options?.whereLanguage ?? ('lucene' as const), + filters, connection: source.connection.toString(), timestampValueExpression: source.timestampValueExpression, implicitColumnExpression: implicitColumn, @@ -165,6 +202,7 @@ export async function mineWindowPatterns( }, where: options?.where ?? '', whereLanguage: options?.whereLanguage ?? ('lucene' as const), + filters, connection: source.connection.toString(), timestampValueExpression: source.timestampValueExpression, implicitColumnExpression: implicitColumn, @@ -174,14 +212,44 @@ export async function mineWindowPatterns( variables: options?.variables, } satisfies ChartConfigWithDateRange; - // The client is constructed fresh per call (not pooled) and is only used for - // the two queries below — the Drain mining afterward never touches it. Close - // it in a finally so the underlying HTTP agent's sockets are released whether - // the queries succeed or throw, rather than leaking one client per call - // (emerging_signals opens two per invocation). + // Drill-down is optional, so a caller filter that cannot be rendered as SQL + // resolves to null instead of failing the mining. + const userWherePromise: Promise = + options?.includeDrilldownWhere + ? userWhereAsSql({ + where: options.where ?? '', + whereLanguage: options.whereLanguage ?? 'lucene', + variables: options.variables, + metadata, + databaseName: source.from.databaseName, + tableName: source.from.tableName, + connectionId: source.connection.toString(), + implicitColumnExpression: implicitColumn, + useTextIndexForImplicitColumn, + }).catch(err => { + trace.getActiveSpan()?.setAttributes({ + 'mcp.event_patterns.drilldown_where_failed': true, + 'mcp.event_patterns.drilldown_where_error': + err instanceof Error ? err.message : String(err), + }); + logger.warn( + { err, teamId, sourceId }, + 'event patterns: could not render where as SQL for drill-down', + ); + return null; + }) + : Promise.resolve(undefined); + + // The client is constructed fresh per call (not pooled) and serves the two + // queries below plus the metadata lookups of the drill-down rendering — the + // Drain mining afterward never touches it. Close it in a finally so the + // underlying HTTP agent's sockets are released whether the queries succeed + // or throw, rather than leaking one client per call (emerging_signals opens + // two per invocation). let sampleResult, countResult; + let userWhereSql: string | null | undefined; try { - [sampleResult, countResult] = await Promise.all([ + [sampleResult, countResult, userWhereSql] = await Promise.all([ clickhouseClient.queryChartConfig({ config: sampleConfig, metadata, @@ -204,6 +272,7 @@ export async function mineWindowPatterns( abort_signal: options?.abortSignal, }, }), + userWherePromise, ]); } catch (err) { return { error: clickHouseErrorResult(err) }; @@ -213,25 +282,22 @@ export async function mineWindowPatterns( const sampleRows = sampleResult.data ?? []; const totalCount = Number(countResult.data?.[0]?.total ?? 0); - if (sampleRows.length === 0) { - return { - patterns: [], - sampleMultiplier: 1, - totalCount, - sampledCount: 0, - bodyColumn, - }; - } + const drilldown = + userWhereSql === undefined + ? undefined + : { + where: andSql(tableFilterSql, userWhereSql ?? undefined), + includesUserFilter: userWhereSql !== null, + }; // Wrap the synchronous Drain mining in try/catch. Without this a thrown // Drain error escapes the helper — and since callers (emerging_signals) // await it inside Promise.all and only inspect the returned {error} union // AFTER the await, an unwrapped throw becomes an unhandled rejection / 500 // instead of a clean MCP error result. - let patterns: ReturnType['patterns']; - let sampleMultiplier: number; + let mined: ReturnType; try { - ({ patterns, sampleMultiplier } = minePatterns(sampleRows, { + mined = minePatterns(sampleRows, { totalCount, startDate, endDate, @@ -245,18 +311,21 @@ export async function mineWindowPatterns( const tsRaw = row.__hdx_pattern_ts; return tsRaw != null ? new Date(String(tsRaw)).getTime() : null; }, - })); + }); } catch (err) { const message = err instanceof Error ? err.message : String(err); return { error: mcpServerError(`Pattern mining failed: ${message}`) }; } return { - patterns, - sampleMultiplier, totalCount, - sampledCount: sampleRows.length, bodyColumn, + drilldown, + patterns: mined.patterns, + sampleMultiplier: mined.sampleMultiplier, + sampledCount: sampleRows.length, + trendGranularity: mined.trendGranularity, + trendBucketStarts: mined.trendBucketStarts, }; } @@ -318,6 +387,7 @@ export async function runEventPatterns( trendBuckets, abortSignal: options?.abortSignal, variables: options?.variables, + includeDrilldownWhere: true, }); if ('error' in mined) return mined.error; @@ -327,8 +397,16 @@ export async function runEventPatterns( totalCount, sampledCount, bodyColumn, + trendGranularity, + trendBucketStarts, + drilldown, } = mined; + const timeRange = { + start: startDate.toISOString(), + end: endDate.toISOString(), + }; + if (sampledCount === 0) { return { content: [ @@ -343,10 +421,7 @@ export async function runEventPatterns( clusterCount: 0, patternsReturned: 0, bodyColumn, - timeRange: { - start: startDate.toISOString(), - end: endDate.toISOString(), - }, + timeRange, }, patterns: [], hint: 'No data found in the queried time range. Try setting startTime to a wider window (e.g. 24 hours ago) or check that filters match existing data.', @@ -360,23 +435,12 @@ export async function runEventPatterns( } // ── Format response ── - // Convert trend timestamps to ISO strings, extract sample body texts, - // and build a whereSnippet per pattern so the agent can drill into - // matching events via a follow-up clickstack_search query. const slicedPatterns = rawPatterns.slice(0, topN); const patterns = slicedPatterns.map((p, i) => { - // Build a Lucene-compatible where clause from the pattern's literal - // (non-<*>) tokens. This lets agents chain: pattern → search. - // Escape \ and " (the phrase-query metachars) in each token. - const literalTokens = p.pattern - .split(/\s+/) - .filter(t => t !== '<*>' && t.length > 0) - .map(t => t.replace(/[\\"]/g, '\\$&')); + const predicate = buildPatternPredicate(bodyColumn, p.pattern); const whereSnippet = - literalTokens.length > 0 - ? `${bodyColumn}:"${literalTokens.join(' ')}"` - : ''; + predicate == null ? '' : andSql(drilldown?.where, predicate); const shareOfTotal = sampledCount > 0 ? p.sampleCount / sampledCount : 0; @@ -396,6 +460,14 @@ export async function runEventPatterns( sampleCount: p.sampleCount, shareOfTotal: Math.round(shareOfTotal * 10000) / 10000, whereSnippet, + whereLanguage: 'sql' as const, + ...(predicate == null + ? { + drilldownUnavailable: + `Template is longer than ${MAX_PREDICATE_TEMPLATE_LENGTH} characters, so ` + + 'whereSnippet is empty. Do not search with it; it would match every row.', + } + : {}), ...(formattedTrend ? { trend: formattedTrend } : {}), samples: p.samples.map(row => { const raw = row.__hdx_pattern_body; @@ -404,27 +476,70 @@ export async function runEventPatterns( }; }); + const trendSummary = + trendGranularity && trendBucketStarts.length > 0 + ? { + requestedBuckets: trendBuckets, + bucketCount: trendBucketStarts.length, + bucketInterval: trendGranularity, + firstBucketStart: new Date(trendBucketStarts[0]).toISOString(), + lastBucketEnd: new Date( + trendBucketStarts[trendBucketStarts.length - 1] + + convertGranularityToSeconds(trendGranularity) * 1000, + ).toISOString(), + } + : undefined; + const output = { summary: { totalCount, sampledCount, + sampleCoverage: + totalCount > 0 + ? Math.round(Math.min(sampledCount / totalCount, 1) * 10000) / 10000 + : 1, sampleMultiplier: Math.round(sampleMultiplier * 100) / 100, clusterCount: rawPatterns.length, patternsReturned: patterns.length, bodyColumn, - timeRange: { - start: startDate.toISOString(), - end: endDate.toISOString(), + timeRange, + ...(trendSummary ? { trend: trendSummary } : {}), + drilldown: { + tool: 'clickstack_search', + sourceId, + startTime: timeRange.start, + endTime: timeRange.end, + whereLanguage: 'sql', + includesFilters: drilldown?.includesUserFilter ?? false, + ...(drilldown?.includesUserFilter + ? {} + : { + warning: + 'The where filter could not be rendered as SQL, so whereSnippet holds only ' + + 'the pattern predicate and the source table filter. AND your filter onto it ' + + '(in SQL) before searching.', + }), + matchSemantics: PATTERN_MATCH_SEMANTICS, }, }, patterns, usage: - 'shareOfTotal is the fraction of sampled rows matching this pattern. ' + - 'estimatedCount = sampleCount * sampleMultiplier. ' + - (trendBuckets > 0 - ? 'trend.count is similarly extrapolated from sample bucket counts. ' + 'totalCount is exact. sampleCount is the number of sampled rows in the pattern. ' + + 'estimatedCount = sampleCount * sampleMultiplier, an extrapolation, not a count; ' + + 'use clickstack_table for exact counts. shareOfTotal = sampleCount / sampledCount. ' + + (trendSummary + ? 'trend.count = round(sampled rows in the bucket * sampleMultiplier). Buckets are ' + + 'summary.trend.bucketInterval wide and aligned to it, so there can be fewer ' + + 'than requested and the first/last can extend past timeRange (partial buckets). ' + + 'A 0 bucket means no SAMPLED row fell in it, not that none occurred: when ' + + 'sampleMultiplier is N, a bucket with fewer than about N real events often ' + + 'shows 0. Compare trends only for patterns with many sampled rows. ' : '') + - 'Use whereSnippet as the "where" parameter in a clickstack_search call to browse matching raw events.', + 'To browse raw events of a pattern, call clickstack_search with the sourceId, ' + + 'startTime and endTime from summary.drilldown, where = whereSnippet, and ' + + 'whereLanguage = "sql". whereSnippet already includes the source table filter and, ' + + 'unless summary.drilldown.includesFilters is false, your where filter, so do not ' + + 'add them again. Skip patterns that have drilldownUnavailable.', }; const { data: trimmedOutput, isTrimmed } = trimToolResponse(output); diff --git a/packages/common-utils/src/__tests__/mine-patterns.test.ts b/packages/common-utils/src/__tests__/mine-patterns.test.ts index 4553d9244e..f3c543aa45 100644 --- a/packages/common-utils/src/__tests__/mine-patterns.test.ts +++ b/packages/common-utils/src/__tests__/mine-patterns.test.ts @@ -264,6 +264,66 @@ describe('minePatterns', () => { // The bucket containing the event should have count = 1 * 100 = 100 expect(nonZeroBuckets[0].count).toBe(100); }); + + it('rounds the bucket width up, so fewer buckets than requested can come back', () => { + const start = new Date('2024-01-01T00:00:00Z'); + const end = new Date('2024-01-01T01:00:00Z'); + + const result = minePatterns( + [{ body: 'event', ts: start.toISOString() }], + { + totalCount: 1, + startDate: start, + endDate: end, + trendBuckets: 24, + ...defaultCallbacks, + }, + ); + + // 3600s / 24 = 150s, rounded up to the 5 minute interval. + expect(result.trendGranularity).toBe('5 minute'); + expect(result.trendBucketStarts).toHaveLength(12); + expect(result.patterns[0].trend.map(b => b.ts)).toEqual( + result.trendBucketStarts, + ); + }); + + it('aligns the first bucket to the interval, before an unaligned start', () => { + const start = new Date('2024-01-01T00:07:00Z'); + const end = new Date('2024-01-01T01:07:00Z'); + + const result = minePatterns( + [{ body: 'event', ts: start.toISOString() }], + { + totalCount: 1, + startDate: start, + endDate: end, + trendBuckets: 4, + ...defaultCallbacks, + }, + ); + + expect(result.trendGranularity).toBe('15 minute'); + expect(result.trendBucketStarts[0]).toBe( + new Date('2024-01-01T00:00:00Z').getTime(), + ); + expect(result.trendBucketStarts[0]).toBeLessThan(start.getTime()); + }); + + it('skips trend computation when trendBuckets is 0', () => { + const result = minePatterns([makeRow('event')], { + totalCount: 1, + startDate: baseDate, + endDate, + trendBuckets: 0, + ...defaultCallbacks, + }); + + expect(result.trendGranularity).toBeUndefined(); + expect(result.trendBucketStarts).toEqual([]); + expect(result.patterns[0].trend).toEqual([]); + expect(result.patterns[0].sampleCount).toBe(1); + }); }); describe('NaN/null timestamp handling', () => { @@ -390,3 +450,56 @@ describe('minePatterns', () => { }); }); }); + +describe('minePatterns template fidelity', () => { + // Deterministic PRNG so the fuzz input is stable across runs. + function lcg(seed: number) { + let s = seed; + return () => { + s = (s * 1103515245 + 12345) % 2 ** 31; + return s / 2 ** 31; + }; + } + + function fits(template: string, body: string): boolean { + const t = template.split(/\s+/).filter(Boolean); + const b = body.split(/\s+/).filter(Boolean); + return ( + t.length === b.length && + t.every((tok, i) => tok === '<*>' || tok === b[i]) + ); + } + + it("reports each cluster's own template, which every member fits", () => { + const words = ['a', 'b', 'c', 'ok', 'fail', 'x1', 'y2', '42', 'retry']; + for (let seed = 1; seed <= 50; seed++) { + const rand = lcg(seed); + const rows = Array.from({ length: 300 }, () => { + const len = 2 + Math.floor(rand() * 3); + const body = Array.from( + { length: len }, + () => words[Math.floor(rand() * words.length)], + ).join(' '); + return makeRow(body); + }); + const { patterns } = minePatterns(rows, { + totalCount: rows.length, + startDate: baseDate, + endDate, + trendBuckets: 0, + maxSamples: rows.length, + ...defaultCallbacks, + }); + for (const p of patterns) { + for (const s of p.samples) { + const body = String(s.body); + if (!fits(p.pattern, body)) { + throw new Error( + `seed ${seed}: "${body}" does not fit "${p.pattern}"`, + ); + } + } + } + } + }); +}); diff --git a/packages/common-utils/src/drain/drain.ts b/packages/common-utils/src/drain/drain.ts index ead441df6c..661cf89e15 100644 --- a/packages/common-utils/src/drain/drain.ts +++ b/packages/common-utils/src/drain/drain.ts @@ -65,6 +65,11 @@ export class Drain { return total; } + /** Look up a cluster by ID without touching LRU order. */ + getCluster(id: number): LogCluster | undefined { + return this.clusterPeek(id); + } + private clusterPeek(id: number): LogCluster | undefined { if (this.unlimitedStore) return this.unlimitedStore.get(id); return this.limitedStore!.peek(id); diff --git a/packages/common-utils/src/drain/mine-patterns.ts b/packages/common-utils/src/drain/mine-patterns.ts index 83a33397ea..f5f663962d 100644 --- a/packages/common-utils/src/drain/mine-patterns.ts +++ b/packages/common-utils/src/drain/mine-patterns.ts @@ -1,5 +1,6 @@ import { convertDateRangeToGranularityString, + Granularity, timeBucketByGranularity, toStartOfInterval, } from '@/core/utils'; @@ -53,6 +54,18 @@ export interface PatternGroup> { export interface MinePatternResult> { patterns: PatternGroup[]; sampleMultiplier: number; + /** + * Bucket width actually used for `trend`. Rounded up to a standard + * interval, so there can be fewer buckets than requested. Undefined when + * trends are disabled (trendBuckets <= 0). + */ + trendGranularity?: Granularity; + /** + * Start of every trend bucket in ms. Buckets align to the interval, so the + * first one can start before `startDate` and the last can end after + * `endDate`. + */ + trendBucketStarts: number[]; } export interface MinePatternOptions> { @@ -62,7 +75,7 @@ export interface MinePatternOptions> { startDate: Date; /** End of the query time range. */ endDate: Date; - /** Max number of trend time buckets to generate. Default: 24. */ + /** Max number of trend time buckets to generate. Default: 24. 0 disables trends. */ trendBuckets?: number; /** Max number of sample rows to keep per pattern. Default: 5. */ maxSamples?: number; @@ -102,26 +115,29 @@ export function minePatterns>( getTimestamp, } = options; + const granularity = + trendBuckets > 0 + ? convertDateRangeToGranularityString([startDate, endDate], trendBuckets) + : undefined; + const allBuckets = granularity + ? timeBucketByGranularity(startDate, endDate, granularity).map(d => + d.getTime(), + ) + : []; + if (rows.length === 0) { - return { patterns: [], sampleMultiplier: 1 }; + return { + patterns: [], + sampleMultiplier: 1, + trendGranularity: granularity, + trendBucketStarts: allBuckets, + }; } // ── Set up Drain miner ── const drainConfig = new TemplateMinerConfig(); const miner = new TemplateMiner(drainConfig); - // ── Compute time buckets using shared helpers ── - const granularity = convertDateRangeToGranularityString( - [startDate, endDate], - trendBuckets, - ); - const allBucketDates = timeBucketByGranularity( - startDate, - endDate, - granularity, - ); - const allBuckets = allBucketDates.map(d => d.getTime()); - // ── Process each row through Drain ── const clustered: Array<{ clusterId: number; @@ -147,7 +163,9 @@ export function minePatterns>( >(); for (const { clusterId, row, bodyText, tsMs } of clustered) { - const bucket = toStartOfInterval(new Date(tsMs), granularity).getTime(); + const bucket = granularity + ? toStartOfInterval(new Date(tsMs), granularity).getTime() + : 0; const existing = groups.get(clusterId); if (existing) { if (existing.samples.length < maxSamples) { @@ -158,12 +176,15 @@ export function minePatterns>( (existing.bucketCounts.get(bucket) ?? 0) + 1, ); } else { - const match = miner.match(bodyText, 'fallback'); + // Read the template from the row's own cluster: re-matching the body can + // land on a different same-length cluster whose template other members + // of this one do not fit. + const cluster = miner.drain.getCluster(clusterId); const bucketCounts = new Map(); bucketCounts.set(bucket, 1); groups.set(clusterId, { samples: [row], - template: match?.getTemplate() ?? bodyText, + template: cluster?.getTemplate() ?? bodyText, bucketCounts, }); } @@ -200,5 +221,10 @@ export function minePatterns>( patterns.sort((a, b) => b.estimatedCount - a.estimatedCount); - return { patterns, sampleMultiplier }; + return { + patterns, + sampleMultiplier, + trendGranularity: granularity, + trendBucketStarts: allBuckets, + }; } From 359a5d464bc7b72583ec3edca70af5a475b23a22 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 20 Oct 2026 17:08:30 -0600 Subject: [PATCH 2/8] fix(common-utils): keep lint under the warning budget in mine-patterns test --- packages/common-utils/src/__tests__/mine-patterns.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/common-utils/src/__tests__/mine-patterns.test.ts b/packages/common-utils/src/__tests__/mine-patterns.test.ts index f3c543aa45..1aa39020aa 100644 --- a/packages/common-utils/src/__tests__/mine-patterns.test.ts +++ b/packages/common-utils/src/__tests__/mine-patterns.test.ts @@ -466,7 +466,7 @@ describe('minePatterns template fidelity', () => { const b = body.split(/\s+/).filter(Boolean); return ( t.length === b.length && - t.every((tok, i) => tok === '<*>' || tok === b[i]) + t.every((tok, i) => tok === '<*>' || tok === b.at(i)) ); } From 47355dfc6566d25993d1ccb09a582e7507cc22ce Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Tue, 20 Oct 2026 17:12:34 -0600 Subject: [PATCH 3/8] fix(mcp): keep denoise removing noisy clusters after minePatterns template change denoise compared minePatterns templates with the template match() returns, which now differ when match() lands a member on another same-length cluster. Build the noisy set from the clusters match() returns for noisy members instead. Also accept doubled-quote JSON keys, count drill-down filter render failures, and type the event pattern int-test helpers. --- packages/api/src/mcp/__tests__/query.test.ts | 5 ++ .../src/mcp/__tests__/queryTool.int.test.ts | 55 +++++++++++++--- .../mcp/tools/query/__tests__/denoise.test.ts | 66 +++++++++++++++++++ packages/api/src/mcp/tools/query/denoise.ts | 52 +++++++++++---- packages/api/src/mcp/tools/query/helpers.ts | 2 +- .../src/mcp/tools/query/runEventPatterns.ts | 20 ++++-- 6 files changed, 170 insertions(+), 30 deletions(-) create mode 100644 packages/api/src/mcp/tools/query/__tests__/denoise.test.ts diff --git a/packages/api/src/mcp/__tests__/query.test.ts b/packages/api/src/mcp/__tests__/query.test.ts index fca56d7a97..da6f369b58 100644 --- a/packages/api/src/mcp/__tests__/query.test.ts +++ b/packages/api/src/mcp/__tests__/query.test.ts @@ -133,6 +133,10 @@ describe('normalizeBodyExpression', () => { String.raw`JSONExtractString(Body, 'it\'s, ok')`, String.raw`JSONExtractString(Body, 'it\'s, ok')`, ], + [ + "JSONExtractString(Body, 'it''s, ok')", + "JSONExtractString(Body, 'it''s, ok')", + ], ])('accepts %s', (expr, expected) => { expect(normalizeBodyExpression(expr)).toBe(expected); }); @@ -152,6 +156,7 @@ describe('normalizeBodyExpression', () => { "JSONExtractString(Body, 'm', (SELECT 1))", // A stray quote in the column would pair with a key's quotes. "JSONExtractString(Body', ' OR 1=1 --')", + "JSONExtractString(Body, 'm'' OR 1=1)", ])('rejects %s', expr => { expect(normalizeBodyExpression(expr)).toBeNull(); }); diff --git a/packages/api/src/mcp/__tests__/queryTool.int.test.ts b/packages/api/src/mcp/__tests__/queryTool.int.test.ts index d2ea418580..90052fe53b 100644 --- a/packages/api/src/mcp/__tests__/queryTool.int.test.ts +++ b/packages/api/src/mcp/__tests__/queryTool.int.test.ts @@ -788,7 +788,45 @@ describe('MCP Query Tools', () => { await bulkInsertLogs(logs); }); - const mine = async (args: Record) => { + type MinedPattern = { + pattern: string; + sampleCount: number; + estimatedCount: number; + whereSnippet: string; + whereLanguage: string; + samples: string[]; + trend?: { time: string; count: number }[]; + }; + type MinedOutput = { + summary: { + totalCount: number; + sampledCount: number; + sampleCoverage: number; + sampleMultiplier: number; + bodyColumn: string; + timeRange: { start: string; end: string }; + trend?: { + requestedBuckets: number; + bucketCount: number; + bucketInterval: string; + firstBucketStart: string; + lastBucketEnd: string; + }; + drilldown: { + tool: string; + sourceId: string; + startTime: string; + endTime: string; + includesFilters: boolean; + warning?: string; + }; + }; + patterns: MinedPattern[]; + }; + + const mine = async ( + args: Record, + ): Promise => { const result = await callTool(client, 'clickstack_event_patterns', { sourceId: logSource._id.toString(), ...window, @@ -798,7 +836,7 @@ describe('MCP Query Tools', () => { return JSON.parse(getFirstText(result)); }; - const drill = async (output: any, pattern: any) => { + const drill = async (output: MinedOutput, pattern: MinedPattern) => { expect(pattern.whereLanguage).toBe('sql'); expect(pattern.whereSnippet).not.toBe(''); const { drilldown } = output.summary; @@ -819,9 +857,9 @@ describe('MCP Query Tools', () => { return parsed.result.data; }; - const findPattern = (output: any, template: string) => { - const p = output.patterns.find((x: any) => x.pattern === template); - expect(p).toBeDefined(); + const findPattern = (output: MinedOutput, template: string) => { + const p = output.patterns.find(x => x.pattern === template); + if (!p) throw new Error(`pattern not found: ${template}`); return p; }; @@ -909,9 +947,7 @@ describe('MCP Query Tools', () => { // are filtered out. How numbers and objects extract varies by // ClickHouse version, so only the payment strings are asserted. expect(output.summary.totalCount).toBeGreaterThanOrEqual(3); - const allSamples: string[] = output.patterns.flatMap( - (x: any) => x.samples, - ); + const allSamples = output.patterns.flatMap(x => x.samples); expect(allSamples).not.toContain(''); const p = findPattern(output, 'Payment <*> declined by issuer'); @@ -990,7 +1026,7 @@ describe('MCP Query Tools', () => { expect(summary.sampleCoverage).toBeCloseTo(3 / 9, 3); expect(summary.sampleMultiplier).toBe(3); const sampledTotal = output.patterns.reduce( - (n: number, p: any) => n + p.sampleCount, + (n, p) => n + p.sampleCount, 0, ); expect(sampledTotal).toBe(3); @@ -1005,6 +1041,7 @@ describe('MCP Query Tools', () => { trendBuckets: 24, }); const { trend } = withTrend.summary; + if (!trend) throw new Error('summary.trend missing'); expect(trend.requestedBuckets).toBe(24); expect(trend.bucketCount).toBeLessThanOrEqual(24); expect(typeof trend.bucketInterval).toBe('string'); diff --git a/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts new file mode 100644 index 0000000000..f59590b876 --- /dev/null +++ b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts @@ -0,0 +1,66 @@ +import { minePatterns } from '@hyperdx/common-utils/dist/drain'; + +import { buildNoisyBodyMatcher } from '@/mcp/tools/query/denoise'; + +function lcg(seed: number) { + let s = seed; + return () => { + s = (s * 1103515245 + 12345) % 2 ** 31; + return s / 2 ** 31; + }; +} + +describe('buildNoisyBodyMatcher', () => { + const startDate = new Date('2026-01-01T00:00:00Z'); + const endDate = new Date('2026-01-01T01:00:00Z'); + + it('matches every sampled member of a noisy cluster', () => { + // Short random bodies over a small vocabulary produce same-length + // clusters that overlap, where `match` sends a member elsewhere. + const words = ['a', 'b', 'c', 'ok', 'fail', 'x1', 'y2', '42', 'retry']; + for (let seed = 1; seed <= 50; seed++) { + const rand = lcg(seed); + const bodies = Array.from({ length: 300 }, () => + Array.from( + { length: 2 + Math.floor(rand() * 3) }, + () => words[Math.floor(rand() * words.length)], + ).join(' '), + ); + const { patterns } = minePatterns( + bodies.map(body => ({ body })), + { + totalCount: bodies.length, + startDate, + endDate, + trendBuckets: 0, + maxSamples: bodies.length, + getBody: row => row.body, + getTimestamp: () => startDate.getTime(), + }, + ); + for (const noisy of patterns) { + const isNoisy = buildNoisyBodyMatcher(bodies, new Set([noisy.pattern])); + for (const { body } of noisy.samples) { + if (!isNoisy(body)) { + throw new Error( + `seed ${seed}: "${body}" in noisy "${noisy.pattern}" was kept`, + ); + } + } + } + } + }); + + it('keeps bodies that only match non-noisy clusters', () => { + const bodies = [ + ...Array.from({ length: 20 }, (_, i) => `heartbeat ok ${i}`), + ...Array.from({ length: 5 }, (_, i) => `user ${i} logged in`), + ]; + const isNoisy = buildNoisyBodyMatcher( + bodies, + new Set(['heartbeat ok <*>']), + ); + expect(isNoisy('heartbeat ok 999')).toBe(true); + expect(isNoisy('user 7 logged in')).toBe(false); + }); +}); diff --git a/packages/api/src/mcp/tools/query/denoise.ts b/packages/api/src/mcp/tools/query/denoise.ts index 182aa54487..9b4cbb3a47 100644 --- a/packages/api/src/mcp/tools/query/denoise.ts +++ b/packages/api/src/mcp/tools/query/denoise.ts @@ -226,16 +226,6 @@ export async function denoiseSearchResults( return { rows, removedPatterns: [] }; } - // ── Build a miner trained on the same sample for row matching ── - const drainConfig = new TemplateMinerConfig(); - const miner = new TemplateMiner(drainConfig); - for (const row of sampleRows) { - const raw = row.__hdx_pattern_body; - const bodyText = flattenBody(raw != null ? String(raw) : ''); - miner.addLogMessage(bodyText); - } - - // ── Match each result row and filter out noisy ones ── const bodyColumnKey = findBodyColumnKey(rows[0], bodyColumn); if (!bodyColumnKey) { return { @@ -245,13 +235,17 @@ export async function denoiseSearchResults( }; } + const isNoisy = buildNoisyBodyMatcher( + sampleRows.map(row => { + const raw = row.__hdx_pattern_body; + return raw != null ? String(raw) : ''; + }), + noisyTemplates, + ); const filteredRows = rows.filter(row => { const bodyValue = row[bodyColumnKey]; if (bodyValue == null) return true; // Keep rows with no body - const bodyText = flattenBody(String(bodyValue)); - const match = miner.match(bodyText, 'fallback'); - if (!match) return true; // No pattern match — keep the row - return !noisyTemplates.has(match.getTemplate()); + return !isNoisy(String(bodyValue)); }); return { @@ -262,6 +256,36 @@ export async function denoiseSearchResults( // ─── Helpers ───────────────────────────────────────────────────────────────── +/** + * Train a miner on the sample the same way minePatterns does and return a + * predicate that is true when a body matches a noisy cluster. minePatterns + * reports each cluster's own template, but `match` can land a member on a + * different same-length cluster. So the noisy set is the clusters that + * `match` returns for members of noisy clusters, which removes every sampled + * noisy row. + */ +export function buildNoisyBodyMatcher( + sampleBodies: string[], + noisyTemplates: Set, +): (body: string) => boolean { + const miner = new TemplateMiner(new TemplateMinerConfig()); + const trained = sampleBodies.map(body => { + const text = flattenBody(body); + return { text, clusterId: miner.addLogMessage(text).clusterId }; + }); + const noisyClusterIds = new Set(); + for (const { text, clusterId } of trained) { + const template = miner.drain.getCluster(clusterId)?.getTemplate(); + if (template == null || !noisyTemplates.has(template)) continue; + const match = miner.match(text, 'fallback'); + noisyClusterIds.add(match?.clusterId ?? clusterId); + } + return body => { + const match = miner.match(flattenBody(body), 'fallback'); + return match != null && noisyClusterIds.has(match.clusterId); + }; +} + /** * Find the key in a result row that corresponds to the body column expression. * ClickHouse may return the column under its expression name or a simplified alias. diff --git a/packages/api/src/mcp/tools/query/helpers.ts b/packages/api/src/mcp/tools/query/helpers.ts index aa808414eb..94964efec8 100644 --- a/packages/api/src/mcp/tools/query/helpers.ts +++ b/packages/api/src/mcp/tools/query/helpers.ts @@ -81,7 +81,7 @@ export function resolveBodyExpression( const SAFE_BODY_EXPR_CHARS = /^[\w.':\[\]\-]+$/; const COLUMN_OR_MAP_REF = /^[A-Za-z_][\w.]*$|^[A-Za-z_][\w.]*\['[^'\\]*'\]$/; -const SQL_STRING_LITERAL = /^'([^'\\]|\\.)*'$/; +const SQL_STRING_LITERAL = /^'([^'\\]|\\.|'')*'$/; const JSON_EXTRACT_STRING_CALL = /^JSONExtractString\((.*)\)$/s; /** diff --git a/packages/api/src/mcp/tools/query/runEventPatterns.ts b/packages/api/src/mcp/tools/query/runEventPatterns.ts index 97eceea23a..e60513a7bc 100644 --- a/packages/api/src/mcp/tools/query/runEventPatterns.ts +++ b/packages/api/src/mcp/tools/query/runEventPatterns.ts @@ -10,7 +10,7 @@ import type { ChartConfigWithDateRange, ChartVariable, } from '@hyperdx/common-utils/dist/types'; -import { DisplayType, SourceKind } from '@hyperdx/common-utils/dist/types'; +import { DisplayType, isLogSource } from '@hyperdx/common-utils/dist/types'; import { trace } from '@opentelemetry/api'; import { ClickhouseClient } from '@/clickhouse'; @@ -22,6 +22,7 @@ import { mcpUserError, } from '@/mcp/utils/errors'; import { MCP_QUERY_MAX_EXECUTION_SEC } from '@/mcp/utils/timeout'; +import { getCounter } from '@/utils/instrumentation'; import logger from '@/utils/logger'; import { trimToolResponse } from '@/utils/trimToolResponse'; @@ -38,6 +39,14 @@ import { userWhereAsSql, } from './patternDrilldown'; +const drilldownWhereFailedCounter = getCounter( + 'hyperdx.mcp.event_patterns.drilldown_where_failed', + { + description: + 'Count of event pattern requests whose where filter could not be rendered as SQL for the drill-down.', + }, +); + // ─── Reusable window mining ────────────────────────────────────────────────── /** @@ -141,11 +150,7 @@ export async function mineWindowPatterns( } const tableFilterSql = - source.kind === SourceKind.Log && - 'tableFilterExpression' in source && - source.tableFilterExpression - ? source.tableFilterExpression - : undefined; + (isLogSource(source) && source.tableFilterExpression) || undefined; const filters = tableFilterSql ? [{ type: 'sql' as const, condition: tableFilterSql }] : []; @@ -232,6 +237,9 @@ export async function mineWindowPatterns( 'mcp.event_patterns.drilldown_where_error': err instanceof Error ? err.message : String(err), }); + drilldownWhereFailedCounter.add(1, { + where_language: options.whereLanguage ?? 'lucene', + }); logger.warn( { err, teamId, sourceId }, 'event patterns: could not render where as SQL for drill-down', From 62f820096a896ffea0fc85eb884ddf4f56fc3ed9 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 21 Oct 2026 11:22:28 -0600 Subject: [PATCH 4/8] fix(mcp): denoise by fitting noisy templates, skip drill-down failure reporting after query errors Matching rows against the clusters that noisy members re-match removed whole rare clusters (greptile P1). A row is now noisy when its body fits a noisy template token by token. Also stop counting drill-down render failures caused by the client closing after the queries failed. --- .../mcp/tools/query/__tests__/denoise.test.ts | 16 +++--- packages/api/src/mcp/tools/query/denoise.ts | 50 ++++++++----------- .../src/mcp/tools/query/runEventPatterns.ts | 6 ++- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts index f59590b876..e35b36b7ef 100644 --- a/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts +++ b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts @@ -39,7 +39,7 @@ describe('buildNoisyBodyMatcher', () => { }, ); for (const noisy of patterns) { - const isNoisy = buildNoisyBodyMatcher(bodies, new Set([noisy.pattern])); + const isNoisy = buildNoisyBodyMatcher(new Set([noisy.pattern])); for (const { body } of noisy.samples) { if (!isNoisy(body)) { throw new Error( @@ -51,16 +51,16 @@ describe('buildNoisyBodyMatcher', () => { } }); - it('keeps bodies that only match non-noisy clusters', () => { - const bodies = [ - ...Array.from({ length: 20 }, (_, i) => `heartbeat ok ${i}`), - ...Array.from({ length: 5 }, (_, i) => `user ${i} logged in`), - ]; + it('keeps bodies that do not fit a noisy template', () => { const isNoisy = buildNoisyBodyMatcher( - bodies, - new Set(['heartbeat ok <*>']), + new Set(['heartbeat ok <*>', 'GET <*> <*> ok 200']), ); expect(isNoisy('heartbeat ok 999')).toBe(true); + expect(isNoisy(' GET api a ok 200 ')).toBe(true); expect(isNoisy('user 7 logged in')).toBe(false); + expect(isNoisy('heartbeat ok')).toBe(false); + // Drain can match this to a cluster that a noisy member also matches; + // it does not fit the noisy template, so it stays. + expect(isNoisy('GET user p q r')).toBe(false); }); }); diff --git a/packages/api/src/mcp/tools/query/denoise.ts b/packages/api/src/mcp/tools/query/denoise.ts index 9b4cbb3a47..9006f7976d 100644 --- a/packages/api/src/mcp/tools/query/denoise.ts +++ b/packages/api/src/mcp/tools/query/denoise.ts @@ -5,8 +5,6 @@ import { DENOISE_SAMPLE_SIZE, flattenBody, minePatterns, - TemplateMiner, - TemplateMinerConfig, } from '@hyperdx/common-utils/dist/drain'; import type { ChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; import { DisplayType } from '@hyperdx/common-utils/dist/types'; @@ -18,6 +16,8 @@ import { MCP_QUERY_MAX_EXECUTION_SEC } from '@/mcp/utils/timeout'; import { resolveBodyExpression } from './helpers'; +const PATTERN_WILDCARD = '<*>'; + // ─── Types ─────────────────────────────────────────────────────────────────── export interface DenoiseResult { @@ -235,13 +235,7 @@ export async function denoiseSearchResults( }; } - const isNoisy = buildNoisyBodyMatcher( - sampleRows.map(row => { - const raw = row.__hdx_pattern_body; - return raw != null ? String(raw) : ''; - }), - noisyTemplates, - ); + const isNoisy = buildNoisyBodyMatcher(noisyTemplates); const filteredRows = rows.filter(row => { const bodyValue = row[bodyColumnKey]; if (bodyValue == null) return true; // Keep rows with no body @@ -257,35 +251,31 @@ export async function denoiseSearchResults( // ─── Helpers ───────────────────────────────────────────────────────────────── /** - * Train a miner on the sample the same way minePatterns does and return a - * predicate that is true when a body matches a noisy cluster. minePatterns - * reports each cluster's own template, but `match` can land a member on a - * different same-length cluster. So the noisy set is the clusters that - * `match` returns for members of noisy clusters, which removes every sampled - * noisy row. + * Return a predicate that is true when a body fits one of the noisy + * templates: same token count, same literal tokens, any token at each <*>. + * Every member of a cluster fits its own template. Re-running Drain's + * `match` instead can land a member on a different same-length cluster. */ export function buildNoisyBodyMatcher( - sampleBodies: string[], noisyTemplates: Set, ): (body: string) => boolean { - const miner = new TemplateMiner(new TemplateMinerConfig()); - const trained = sampleBodies.map(body => { - const text = flattenBody(body); - return { text, clusterId: miner.addLogMessage(text).clusterId }; - }); - const noisyClusterIds = new Set(); - for (const { text, clusterId } of trained) { - const template = miner.drain.getCluster(clusterId)?.getTemplate(); - if (template == null || !noisyTemplates.has(template)) continue; - const match = miner.match(text, 'fallback'); - noisyClusterIds.add(match?.clusterId ?? clusterId); - } + const templates = [...noisyTemplates].map(tokenize); return body => { - const match = miner.match(flattenBody(body), 'fallback'); - return match != null && noisyClusterIds.has(match.clusterId); + const tokens = tokenize(flattenBody(body)); + return templates.some( + t => + t.length === tokens.length && + t.every((tok, i) => tok === PATTERN_WILDCARD || tok === tokens.at(i)), + ); }; } +// Drain tokenizes with trim + split(/\s+/). +function tokenize(text: string): string[] { + const trimmed = text.trim(); + return trimmed === '' ? [] : trimmed.split(/\s+/); +} + /** * Find the key in a result row that corresponds to the body column expression. * ClickHouse may return the column under its expression name or a simplified alias. diff --git a/packages/api/src/mcp/tools/query/runEventPatterns.ts b/packages/api/src/mcp/tools/query/runEventPatterns.ts index e60513a7bc..d5b9534749 100644 --- a/packages/api/src/mcp/tools/query/runEventPatterns.ts +++ b/packages/api/src/mcp/tools/query/runEventPatterns.ts @@ -218,7 +218,9 @@ export async function mineWindowPatterns( } satisfies ChartConfigWithDateRange; // Drill-down is optional, so a caller filter that cannot be rendered as SQL - // resolves to null instead of failing the mining. + // resolves to null instead of failing the mining. Once the queries fail the + // client is closed under the rendering, so its failure is not reported. + let queriesFailed = false; const userWherePromise: Promise = options?.includeDrilldownWhere ? userWhereAsSql({ @@ -232,6 +234,7 @@ export async function mineWindowPatterns( implicitColumnExpression: implicitColumn, useTextIndexForImplicitColumn, }).catch(err => { + if (queriesFailed || options.abortSignal?.aborted) return null; trace.getActiveSpan()?.setAttributes({ 'mcp.event_patterns.drilldown_where_failed': true, 'mcp.event_patterns.drilldown_where_error': @@ -283,6 +286,7 @@ export async function mineWindowPatterns( userWherePromise, ]); } catch (err) { + queriesFailed = true; return { error: clickHouseErrorResult(err) }; } finally { await clickhouseClient.close().catch(() => {}); From 00f3439a1ce5fb822d832c0a3d48565935fbef3e Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 21 Oct 2026 11:40:44 -0600 Subject: [PATCH 5/8] fix(mcp): denoise keeps rows whose most specific template is not noisy A broad noisy template like '<*> <*>' removed every same-length row, including rows of rare specific clusters. Pick the fitting template with the fewest wildcards and drop the row only if it is noisy. Also test the drilldownUnavailable branch and the predicate length boundary. --- .../src/mcp/__tests__/queryTool.int.test.ts | 17 ++++++ .../mcp/tools/query/__tests__/denoise.test.ts | 54 ++++++++++++++++--- .../query/__tests__/patternDrilldown.test.ts | 2 + packages/api/src/mcp/tools/query/denoise.ts | 50 ++++++++++++----- 4 files changed, 103 insertions(+), 20 deletions(-) diff --git a/packages/api/src/mcp/__tests__/queryTool.int.test.ts b/packages/api/src/mcp/__tests__/queryTool.int.test.ts index 90052fe53b..9f535a5318 100644 --- a/packages/api/src/mcp/__tests__/queryTool.int.test.ts +++ b/packages/api/src/mcp/__tests__/queryTool.int.test.ts @@ -794,6 +794,7 @@ describe('MCP Query Tools', () => { estimatedCount: number; whereSnippet: string; whereLanguage: string; + drilldownUnavailable?: string; samples: string[]; trend?: { time: string; count: number }[]; }; @@ -1015,6 +1016,22 @@ describe('MCP Query Tools', () => { } }); + it('marks templates too long for a predicate as drilldownUnavailable', async () => { + await bulkInsertLogs([ + { + Body: 'x'.repeat(4001), + ServiceName: 'long', + SeverityText: 'INFO', + Timestamp: base, + }, + ]); + const output = await mine({ where: 'ServiceName:long' }); + expect(output.patterns).toHaveLength(1); + const [p] = output.patterns; + expect(p.whereSnippet).toBe(''); + expect(p.drilldownUnavailable).toContain('longer than 4000'); + }); + it('separates sampled counts from estimates', async () => { const output = await mine({ where: 'ServiceName:svc-a', diff --git a/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts index e35b36b7ef..20393f6d4a 100644 --- a/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts +++ b/packages/api/src/mcp/tools/query/__tests__/denoise.test.ts @@ -10,11 +10,14 @@ function lcg(seed: number) { }; } +const wildcards = (template: string) => + template.split(/\s+/).filter(t => t === '<*>').length; + describe('buildNoisyBodyMatcher', () => { const startDate = new Date('2026-01-01T00:00:00Z'); const endDate = new Date('2026-01-01T01:00:00Z'); - it('matches every sampled member of a noisy cluster', () => { + it('removes members of a noisy cluster unless a non-noisy template fits them more specifically', () => { // Short random bodies over a small vocabulary produce same-length // clusters that overlap, where `match` sends a member elsewhere. const words = ['a', 'b', 'c', 'ok', 'fail', 'x1', 'y2', '42', 'retry']; @@ -38,12 +41,28 @@ describe('buildNoisyBodyMatcher', () => { getTimestamp: () => startDate.getTime(), }, ); + const templates = patterns.map(p => p.pattern); for (const noisy of patterns) { - const isNoisy = buildNoisyBodyMatcher(new Set([noisy.pattern])); + const isNoisy = buildNoisyBodyMatcher( + templates, + new Set([noisy.pattern]), + ); + const own = wildcards(noisy.pattern); for (const { body } of noisy.samples) { - if (!isNoisy(body)) { + const tokens = body.split(' '); + const moreSpecific = templates.some(t => { + const tt = t.split(' '); + return ( + t !== noisy.pattern && + wildcards(t) < own && + tt.length === tokens.length && + tt.every((tok, i) => tok === '<*>' || tok === tokens.at(i)) + ); + }); + if (isNoisy(body) === moreSpecific) { throw new Error( - `seed ${seed}: "${body}" in noisy "${noisy.pattern}" was kept`, + `seed ${seed}: "${body}" in noisy "${noisy.pattern}" ` + + `removed=${isNoisy(body)}`, ); } } @@ -51,16 +70,39 @@ describe('buildNoisyBodyMatcher', () => { } }); + it('keeps rows of a specific cluster when a broad template is noisy', () => { + const isNoisy = buildNoisyBodyMatcher( + ['<*> <*>', 'user login'], + new Set(['<*> <*>']), + ); + expect(isNoisy('foo bar')).toBe(true); + expect(isNoisy('user login')).toBe(false); + }); + it('keeps bodies that do not fit a noisy template', () => { + const templates = [ + 'heartbeat ok <*>', + 'GET <*> <*> ok 200', + 'GET user <*> <*> <*>', + 'user <*> logged in', + ]; const isNoisy = buildNoisyBodyMatcher( + templates, new Set(['heartbeat ok <*>', 'GET <*> <*> ok 200']), ); expect(isNoisy('heartbeat ok 999')).toBe(true); expect(isNoisy(' GET api a ok 200 ')).toBe(true); expect(isNoisy('user 7 logged in')).toBe(false); expect(isNoisy('heartbeat ok')).toBe(false); - // Drain can match this to a cluster that a noisy member also matches; - // it does not fit the noisy template, so it stays. expect(isNoisy('GET user p q r')).toBe(false); + expect(isNoisy('something unseen')).toBe(false); + }); + + it('prefers the noisy template on a tie', () => { + const isNoisy = buildNoisyBodyMatcher( + ['a <*> c', 'a b <*>'], + new Set(['a b <*>']), + ); + expect(isNoisy('a b c')).toBe(true); }); }); diff --git a/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts b/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts index 0e9e8a5bd3..ce69e41d69 100644 --- a/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts +++ b/packages/api/src/mcp/tools/query/__tests__/patternDrilldown.test.ts @@ -162,6 +162,8 @@ describe('buildPatternPredicate', () => { it('returns null for templates too long to turn into a predicate', () => { const long = 'x'.repeat(MAX_PREDICATE_TEMPLATE_LENGTH + 1); expect(buildPatternPredicate('Body', long)).toBeNull(); + const atLimit = 'x'.repeat(MAX_PREDICATE_TEMPLATE_LENGTH); + expect(buildPatternPredicate('Body', atLimit)).not.toBeNull(); }); }); diff --git a/packages/api/src/mcp/tools/query/denoise.ts b/packages/api/src/mcp/tools/query/denoise.ts index 9006f7976d..231cad2401 100644 --- a/packages/api/src/mcp/tools/query/denoise.ts +++ b/packages/api/src/mcp/tools/query/denoise.ts @@ -204,9 +204,6 @@ export async function denoiseSearchResults( } // ── Identify noisy patterns (>10% of sampled events) ── - // Key by template string rather than cluster ID so we are not coupled to - // the auto-incrementing IDs generated inside minePatterns(). The matching - // miner below produces its own IDs; comparing template strings is stable. const sampledRowCount = sampleRows.length; const noisyTemplates = new Set(); const removedPatterns: DenoiseResult['removedPatterns'] = []; @@ -235,7 +232,10 @@ export async function denoiseSearchResults( }; } - const isNoisy = buildNoisyBodyMatcher(noisyTemplates); + const isNoisy = buildNoisyBodyMatcher( + patterns.map(p => p.pattern), + noisyTemplates, + ); const filteredRows = rows.filter(row => { const bodyValue = row[bodyColumnKey]; if (bodyValue == null) return true; // Keep rows with no body @@ -251,22 +251,44 @@ export async function denoiseSearchResults( // ─── Helpers ───────────────────────────────────────────────────────────────── /** - * Return a predicate that is true when a body fits one of the noisy - * templates: same token count, same literal tokens, any token at each <*>. - * Every member of a cluster fits its own template. Re-running Drain's - * `match` instead can land a member on a different same-length cluster. + * Return a predicate that is true when the most specific template a body + * fits (fewest <*>) is noisy. Fitting means same token count, same literal + * tokens, any token at each <*>. Taking the most specific fit keeps rows of + * a rare `user login` cluster when a broad `<*> <*>` cluster is noisy; ties + * go to the noisy template. Re-running Drain's `match` instead can land a + * member on a different same-length cluster than the one it was counted in. */ export function buildNoisyBodyMatcher( + templates: string[], noisyTemplates: Set, ): (body: string) => boolean { - const templates = [...noisyTemplates].map(tokenize); + const candidates = templates.map(template => { + const tokens = tokenize(template); + return { + tokens, + wildcards: tokens.filter(t => t === PATTERN_WILDCARD).length, + noisy: noisyTemplates.has(template), + }; + }); return body => { const tokens = tokenize(flattenBody(body)); - return templates.some( - t => - t.length === tokens.length && - t.every((tok, i) => tok === PATTERN_WILDCARD || tok === tokens.at(i)), - ); + let best: (typeof candidates)[number] | undefined; + for (const c of candidates) { + const fits = + c.tokens.length === tokens.length && + c.tokens.every( + (tok, i) => tok === PATTERN_WILDCARD || tok === tokens.at(i), + ); + if (!fits) continue; + if ( + best == null || + c.wildcards < best.wildcards || + (c.wildcards === best.wildcards && c.noisy) + ) { + best = c; + } + } + return best?.noisy ?? false; }; } From 15591b43d17309cf1cb76839cc278532969f6d47 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 21 Oct 2026 14:50:31 -0600 Subject: [PATCH 6/8] docs(mcp): point query guide at clickstack_event_patterns instead of repeating its description --- .../api/src/mcp/prompts/dashboards/content.ts | 55 ++----------------- 1 file changed, 4 insertions(+), 51 deletions(-) diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index ea024d118a..e147117245 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -1451,57 +1451,10 @@ containers[i].tabs[j].id). Read the path to know which input to fix. == EVENT PATTERN MINING == -Use clickstack_event_patterns when asked to find common log messages, recurring -patterns, noisy services, or top event types. This is the right choice whenever -the question is about "most common logs", "top patterns", "what is generating -the most log volume", or similar pattern-discovery questions. - -It samples random events and clusters them with the Drain algorithm, returning: - - pattern templates (with <*> wildcards for variable parts) - - sampleCount (rows in the sample) and estimatedCount (sampleCount × - sampleMultiplier, an extrapolation, not an exact count) - - time-bucketed trends, also extrapolated; summary.trend gives the actual - bucket interval and boundaries - - a whereSnippet for each pattern to drill into matching raw events - -Required parameters: - - sourceId (call clickstack_list_sources first) - - startTime / endTime for the time window - -Optional parameters: - - where: filter to specific services or severity levels before mining - - sampleSize: increase for more accurate patterns (default 10000, max 25000) - - bodyExpression: what to mine (auto-detected: Body for logs, SpanName for - traces). A column or map reference, or JSONExtractString over one with - string-literal keys, e.g. JSONExtractString(Body, 'error', 'message'). - Other function calls are rejected. - - trendBuckets: maximum trend buckets (0 disables trends) - -Examples: - "What kinds of errors is checkout producing?" - { where: "ServiceName:checkout AND SeverityText:error", topN: 5, sampleSize: 1000 } - "What messages dominate this service?" - { where: "ServiceName:api", trendBuckets: 0 } - Patterns in extracted exception names (existing map attribute): - { bodyExpression: "LogAttributes['exception.type']", - where: "LogAttributes['exception.type'] != ''", whereLanguage: "sql" } - A field inside JSON bodies (exclude rows without it): - { bodyExpression: "JSONExtractString(Body, 'message')", - where: "JSONExtractString(Body, 'message') != ''", whereLanguage: "sql" } - -Drilling into a pattern: call clickstack_search with the same sourceId, -startTime and endTime, where = whereSnippet, whereLanguage = "sql". The snippet -already includes the source table filter and, unless -summary.drilldown.includesFilters is false, your where filter, so do not add -them again. Skip patterns with drilldownUnavailable. It matches events whose -value has the template's literal tokens in order with one token per <*>; this -can include events Drain put in a more specific cluster. - -Sampling limits: a pattern or trend bucket with 0 sampled events can still have -real events, especially when sampleMultiplier is large. For exact counts or -breakdowns by a known field use clickstack_table / clickstack_timeseries; for -patterns that are new or gone versus an earlier window use -clickstack_emerging_signals. +Use clickstack_event_patterns for pattern-discovery questions: most common +logs, top patterns, noisy services, what generates the most log volume. Its +tool description covers parameters, examples, sampling limits, and how to +drill into a pattern with clickstack_search. == COMMON MISTAKES == From 510bc7777f42b13307cc8845d5f3a51bf65e5c8e Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 21 Oct 2026 14:52:30 -0600 Subject: [PATCH 7/8] docs(mcp): explain the bodyExpression validation regexes --- packages/api/src/mcp/tools/query/helpers.ts | 32 ++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/api/src/mcp/tools/query/helpers.ts b/packages/api/src/mcp/tools/query/helpers.ts index 94964efec8..cc53623e94 100644 --- a/packages/api/src/mcp/tools/query/helpers.ts +++ b/packages/api/src/mcp/tools/query/helpers.ts @@ -76,12 +76,42 @@ export function resolveBodyExpression( return multiExpr.length === 1 ? expression : multiExpr[0]; } -/** Reject bodyExpression values containing SQL-unsafe characters. */ +/** + * A bodyExpression made only of word characters, `.`, `'`, `:`, `[`, `]` and + * `-`. No spaces, parentheses, commas or operators, so it cannot carry a + * function call or a second condition. + * + * Accepts: `Body`, `SpanAttributes['http.url']` + * Rejects: `Body) OR (1=1`, `lower(Body)` + */ // eslint-disable-next-line no-useless-escape const SAFE_BODY_EXPR_CHARS = /^[\w.':\[\]\-]+$/; +/** + * The column argument of JSONExtractString: an identifier (letters, digits, + * `_`, `.`; not starting with a digit), optionally followed by one map key in + * single quotes. The key may not contain `'` or `\`. + * + * Accepts: `Body`, `ResourceAttributes.service`, `LogAttributes['object']` + * Rejects: `lower(Body)`, `Body'`, `LogAttributes['a'']` + */ const COLUMN_OR_MAP_REF = /^[A-Za-z_][\w.]*$|^[A-Za-z_][\w.]*\['[^'\\]*'\]$/; + +/** + * One complete single-quoted SQL string. Inside the quotes, any character + * other than `'` and `\`, a backslash escape (`\'`, `\\`), or a doubled + * quote (`''`). + * + * Accepts: `'message'`, `'it\'s'`, `'it''s'` + * Rejects: `message`, `'a' || 'b'`, `'m'' OR 1=1` (unterminated) + */ const SQL_STRING_LITERAL = /^'([^'\\]|\\.|'')*'$/; + +/** + * A whole expression of the form `JSONExtractString(...)`. Captures the raw + * argument list (which may span lines, hence the `s` flag) for + * `normalizeBodyExpression` to split and validate argument by argument. + */ const JSON_EXTRACT_STRING_CALL = /^JSONExtractString\((.*)\)$/s; /** From 735fa6cbca35d1b17e5fc42f0934d7bf481d843d Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 4 Nov 2026 13:04:11 -0600 Subject: [PATCH 8/8] refactor(mcp): share where rendering and table filter with chart rendering userWhereAsSql now calls renderWhereExpressionStr, the same Lucene-to-SQL step the mining query uses, instead of building its own serializer. The source table filter comes from a new getSourceTableFilters helper that buildSearchChartConfig also uses. --- .../src/mcp/tools/query/patternDrilldown.ts | 19 ++++++++----------- .../src/mcp/tools/query/runEventPatterns.ts | 12 ++++++------ .../src/core/renderChartConfig.ts | 8 +++++++- .../src/core/searchChartConfig.ts | 17 +++++++++++++---- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/api/src/mcp/tools/query/patternDrilldown.ts b/packages/api/src/mcp/tools/query/patternDrilldown.ts index 49c1a62843..5e85cb6c00 100644 --- a/packages/api/src/mcp/tools/query/patternDrilldown.ts +++ b/packages/api/src/mcp/tools/query/patternDrilldown.ts @@ -1,9 +1,6 @@ import type { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; +import { renderWhereExpressionStr } from '@hyperdx/common-utils/dist/core/renderChartConfig'; import { escapeSqlString } from '@hyperdx/common-utils/dist/core/utils'; -import { - CustomSchemaSQLSerializerV2, - SearchQueryBuilder, -} from '@hyperdx/common-utils/dist/queryParser'; import type { ChartVariable, UseTextIndex, @@ -70,8 +67,9 @@ export function andSql(...conditions: Array): string { /** * Render the caller's filter as SQL so it can be ANDed with a pattern - * predicate in one clickstack_search `where`. Dashboard variables are - * expanded first because the search tool has no variable context. + * predicate in one clickstack_search `where`. Uses the same rendering as the + * mining query. Dashboard variables are expanded first because the search + * tool has no variable context. */ export async function userWhereAsSql({ where, @@ -98,14 +96,13 @@ export async function userWhereAsSql({ const expanded = variables ? substituteVariables(where, { variables, inputLanguage: whereLanguage }) : where; - if (whereLanguage === 'sql') return expanded; - const serializer = new CustomSchemaSQLSerializerV2({ + return renderWhereExpressionStr({ + condition: expanded, + language: whereLanguage, metadata, - databaseName, - tableName, + from: { databaseName, tableName }, connectionId, implicitColumnExpression, useTextIndexForImplicitColumn, }); - return new SearchQueryBuilder(expanded, serializer).build(); } diff --git a/packages/api/src/mcp/tools/query/runEventPatterns.ts b/packages/api/src/mcp/tools/query/runEventPatterns.ts index d5b9534749..cd2ad6f304 100644 --- a/packages/api/src/mcp/tools/query/runEventPatterns.ts +++ b/packages/api/src/mcp/tools/query/runEventPatterns.ts @@ -1,4 +1,5 @@ import { getMetadata } from '@hyperdx/common-utils/dist/core/metadata'; +import { getSourceTableFilters } from '@hyperdx/common-utils/dist/core/searchChartConfig'; import { convertGranularityToSeconds, getFirstTimestampValueExpression, @@ -10,7 +11,7 @@ import type { ChartConfigWithDateRange, ChartVariable, } from '@hyperdx/common-utils/dist/types'; -import { DisplayType, isLogSource } from '@hyperdx/common-utils/dist/types'; +import { DisplayType } from '@hyperdx/common-utils/dist/types'; import { trace } from '@opentelemetry/api'; import { ClickhouseClient } from '@/clickhouse'; @@ -149,11 +150,10 @@ export async function mineWindowPatterns( }; } - const tableFilterSql = - (isLogSource(source) && source.tableFilterExpression) || undefined; - const filters = tableFilterSql - ? [{ type: 'sql' as const, condition: tableFilterSql }] - : []; + const filters = getSourceTableFilters(source); + const tableFilterSql = andSql( + ...filters.map(f => ('condition' in f ? f.condition : undefined)), + ); const clickhouseClient = new ClickhouseClient({ host: connection.host, diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 8de7551337..6f7d5c7ddd 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1274,7 +1274,13 @@ function renderFrom({ ); } -async function renderWhereExpressionStr({ +/** + * Render a where condition as a SQL string: Lucene is serialized against the + * table's schema, and the result is rewritten to use materialized columns + * where the table has them. Used by chart rendering and by any caller that + * needs the same SQL outside a chart config. + */ +export async function renderWhereExpressionStr({ condition, language, metadata, diff --git a/packages/common-utils/src/core/searchChartConfig.ts b/packages/common-utils/src/core/searchChartConfig.ts index 37144db8ab..066c0f9760 100644 --- a/packages/common-utils/src/core/searchChartConfig.ts +++ b/packages/common-utils/src/core/searchChartConfig.ts @@ -14,6 +14,18 @@ import { TSource, } from '@/types'; +/** + * The source's `tableFilterExpression` as a SQL filter, or `[]` when it has + * none. Log sources are the only kind that carries the field. Every path + * that queries a source's rows should apply this so they all see the same + * row set (see `buildSearchChartConfig`). + */ +export function getSourceTableFilters(source: TSource): Filter[] { + return isLogSource(source) && source.tableFilterExpression + ? [{ type: 'sql', condition: source.tableFilterExpression }] + : []; +} + /** * Return type for `buildSearchChartConfig`. * @@ -131,10 +143,7 @@ export function buildSearchChartConfig( // For hard isolation, configure a ClickHouse ROW POLICY at the DB level // instead. Existing values are still honored here for backward // compatibility; new sources should not set the field. - const tableFilter: Filter[] = - isLogSource(source) && source.tableFilterExpression != null - ? [{ type: 'sql', condition: source.tableFilterExpression }] - : []; + const tableFilter = getSourceTableFilters(source); const userFilters: Filter[] = input.filters ?? []; const mergedFilters: Filter[] = [...tableFilter, ...userFilters];