Skip to content

Commit 0262822

Browse files
authored
fix(mcp): shorten instead of replacing when trimming tool responses, ordered reductions, trim notes (#3554)
Refs #3542 Follows the proposal in the maintainer comment on #3542 (the three points, in order). Kept to `trimToolResponse`; the table tool's `having` / `limit` will go to a separate issue and PR as asked. ## What 1. **Shorten instead of replace** (`packages/api/src/utils/trimToolResponse.ts`). A string over its budget is cut and gets a `…(+N chars)` suffix (e.g. `…(+6,900 chars)`), instead of becoming `{"__hdx_trimmed":true,"originalSize":N}`. This applies to every tool through the fallback. In arrays, items within an equal share stay whole and the large items split what's left, so shortened items add up to the array's budget (with replacement markers that didn't matter; with shortened strings it does). 2. **Trim notes.** `TrimResult` gains `notes: string[]`: the size before/after and limit, each reduction that changed something (e.g. `patterns[].samples: kept the first 1 (60 elements removed from 30 items)`), arrays cut by path (`patterns: cut from 30 to 10 items`), and how many values were shortened. Every caller this PR changes that already adds a `note` when trimmed now also returns them as `trimNotes`; `clickstack_describe_table` is not changed here. 3. **Ordered reductions.** `trimToolResponse(data, maxSize, { reductions })` with `{ path, keep }`, `{ path, maxChars }` and `{ path, drop: true }`, using the path syntax from the proposal (`patterns[].samples`, `patterns[].samples[]`). It works as described: - a response that fits is returned unchanged (same object); - otherwise the reductions run in order on a copy, each on every match, and the whole response is measured after each step; it stops as soon as it fits; - only then the existing per-key split and array cuts run. `clickstack_event_patterns` uses the list from the proposal (samples keep 3, keep 1, samples[] maxChars 2000, pattern maxChars 2000, trend drop), plus a last step: `patterns[].whereSnippet` is dropped, never shortened, because a cut drill-down predicate is invalid SQL. Since it's dropped before the fallback runs, the fallback (the only part that shortens strings) never sees one; the `note` says when snippets were removed. `clickstack_emerging_signals` shortens `emerging[].pattern` / `disappeared[].pattern` to 2,000 chars first. 4. **Smaller fixes from review.** `shortenString` never makes a value longer and doesn't split surrogate pairs; array item budgets count the separators; array cuts are noted once per path and size markers are noted; note paths match the response (`result.…`); `search` with `denoise` keeps the first trim's notes; the `clickstack_sql` batch appends its row cut to the per-query notes. ## Effect An `event_patterns`-shaped result with 30 patterns, the top 3 being ~4.5 KB stack traces with 5 samples each (101,450 chars, limit 50,000): | | Patterns returned | Top pattern | Size | |---|---|---|---| | `main` | 10 | `{"__hdx_trimmed":true,"originalSize":4736}` | 3,848 | | Fallback only (this PR, no reductions) | 10 | shortened, with `…(+N chars)` | 8,282 | | With the event_patterns reductions | **30** | complete | 41,082 (stopped after "samples keep 1") | ## Tests `packages/api/src/utils/__tests__/trimToolResponse.test.ts` (12 new): - suffix format; shortened (not replaced) strings in the fallback within the budget; notes for size, shortened values and array cuts; no notes when nothing is trimmed; - each reduction kind on every match, with its note; stop-early (later steps don't run once it fits); unchanged when it fits; input not mutated; fallback after reductions stays within the budget; - the 30-pattern case above: all 30 kept, no `__hdx_trimmed`. - JSON-escape expansion (quotes, control chars, non-ASCII, `…`) stays within the budget; separators-only arrays; no-growth and surrogate cases; grouped notes; `drop` on array elements. `packages/api/src/mcp/__tests__/queryTool.int.test.ts`: the real `event_patterns` tool with 30 large patterns keeps all 30, drops (never cuts) `whereSnippet`, and returns `trimNotes`. `sqlToolBatch.test.ts`: batch cuts replace `trimNotes`. `npx jest src/mcp src/utils --testPathIgnorePatterns '\.int\.test\.ts$'`: 932 passed; `queryTool` + `emergingSignalsTool` int tests: 117 passed. tsc, eslint (no new warnings) and prettier clean on the changed files. ## Not included - `having` / `limit` / default order for `clickstack_table`: separate issue and PR (#3555, #3558), as asked.
1 parent b7b4d95 commit 0262822

14 files changed

Lines changed: 898 additions & 87 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperdx/api': patch
3+
---
4+
5+
fix: MCP tool responses that are over the size budget now shorten long strings with a `…(+N chars)` suffix instead of replacing them, say what was trimmed in `trimNotes`, and `clickstack_event_patterns` / `clickstack_emerging_signals` drop samples and shorten long patterns before cutting whole patterns, so the requested `topN` survives when it can

‎packages/api/src/mcp/__tests__/queryTool.int.test.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
getServer,
1919
seedExponentialHistogramMetric,
2020
} from '@/fixtures';
21+
import * as denoise from '@/mcp/tools/query/denoise';
2122
import * as patternDrilldown from '@/mcp/tools/query/patternDrilldown';
2223
import { McpContext } from '@/mcp/tools/types';
2324
import Connection from '@/models/connection';
@@ -640,11 +641,123 @@ describe('MCP Query Tools', () => {
640641
expect(output).not.toHaveProperty('denoised');
641642
});
642643
});
644+
645+
describe('denoise on a trimmed result', () => {
646+
const now = new Date();
647+
648+
beforeEach(async () => {
649+
const pad = ' payload'.repeat(150);
650+
await bulkInsertLogs(
651+
Array.from({ length: 120 }, (_, i) => ({
652+
Body:
653+
i < 110
654+
? `Health check OK from 10.0.0.${i}${pad}`
655+
: `Rare event ${i} in subsystem${pad}`,
656+
ServiceName: 'loadbalancer',
657+
SeverityText: 'INFO',
658+
Timestamp: new Date(now.getTime() - (i + 1) * 1000),
659+
})),
660+
);
661+
});
662+
663+
const search = () =>
664+
callTool(client, 'clickstack_search', {
665+
sourceId: logSource._id.toString(),
666+
denoise: true,
667+
maxResults: 200,
668+
startTime: new Date(now.getTime() - 10 * 60 * 1000).toISOString(),
669+
endTime: new Date(now.getTime() + 60 * 1000).toISOString(),
670+
});
671+
672+
it("keeps the first trim's row cut but not its stale size line", async () => {
673+
const result = await search();
674+
675+
expect(result.isError).toBeFalsy();
676+
const output = JSON.parse(getFirstText(result));
677+
expect(output).toHaveProperty('denoised');
678+
expect(output.note).toContain('trimmed');
679+
const sizeLines = output.trimNotes.filter((n: string) =>
680+
n.startsWith('Response trimmed from '),
681+
);
682+
expect(sizeLines).toHaveLength(0); // denoise didn't need a second trim
683+
expect(output.trimNotes).toEqual(
684+
expect.arrayContaining([
685+
expect.stringMatching(/^result\.data: cut from 120 to \d+ items$/),
686+
]),
687+
);
688+
});
689+
690+
it('keeps the same notes when denoise fails', async () => {
691+
const spy = jest
692+
.spyOn(denoise, 'denoiseSearchResults')
693+
.mockRejectedValueOnce(new Error('denoise down'));
694+
try {
695+
const result = await search();
696+
expect(result.isError).toBeFalsy();
697+
const output = JSON.parse(getFirstText(result));
698+
expect(output.denoised.skipped).toBe('denoise_failed');
699+
expect(output.trimNotes).toEqual([
700+
`result.data: cut from 120 to ${output.result.data.length} items`,
701+
]);
702+
} finally {
703+
spy.mockRestore();
704+
}
705+
});
706+
});
643707
});
644708

645709
// ─── clickstack_event_patterns ─────────────────────────────────────────────────
646710

647711
describe('clickstack_event_patterns', () => {
712+
describe('when the response is over the size limit', () => {
713+
const now = new Date();
714+
715+
beforeEach(async () => {
716+
// 30 unrelated patterns of ~60 tokens, 4 rows each: the samples and
717+
// drill-down predicates push the response well past 50,000 chars.
718+
const logs: Parameters<typeof bulkInsertLogs>[0] = [];
719+
for (let p = 0; p < 30; p++) {
720+
const words = Array.from({ length: 60 }, (_, k) => `f${p}x${k}`);
721+
for (let r = 0; r < 4; r++) {
722+
logs.push({
723+
Body: `Failure${p} ${words.join(' ')} id${p}_${r}`,
724+
ServiceName: 'big-service',
725+
SeverityText: 'ERROR',
726+
Timestamp: new Date(now.getTime() - (p * 4 + r + 1) * 1000),
727+
});
728+
}
729+
}
730+
await bulkInsertLogs(logs);
731+
});
732+
733+
it('keeps all topN patterns and drops, never cuts, whereSnippet', async () => {
734+
const result = await callTool(client, 'clickstack_event_patterns', {
735+
sourceId: logSource._id.toString(),
736+
topN: 30,
737+
startTime: new Date(now.getTime() - 10 * 60 * 1000).toISOString(),
738+
endTime: new Date(now.getTime() + 60 * 1000).toISOString(),
739+
});
740+
741+
expect(result.isError).toBeFalsy();
742+
const text = getFirstText(result);
743+
const output = JSON.parse(text);
744+
expect(output.patterns).toHaveLength(30);
745+
expect(text).not.toContain('__hdx_trimmed');
746+
expect(output.trimNotes).toEqual(
747+
expect.arrayContaining([
748+
expect.stringMatching(/^Response trimmed from /),
749+
expect.stringMatching(/^patterns\[\]\.samples: kept the first/),
750+
expect.stringMatching(/^patterns\[\]\.whereSnippet: removed/),
751+
]),
752+
);
753+
expect(output.note).toContain('whereSnippet was removed');
754+
for (const p of output.patterns) {
755+
expect(p).not.toHaveProperty('whereSnippet');
756+
expect(p.pattern).toMatch(/^Failure\d+ /);
757+
}
758+
});
759+
});
760+
648761
it('should execute an event_patterns query on a log source', async () => {
649762
const result = await callTool(client, 'clickstack_event_patterns', {
650763
sourceId: logSource._id.toString(),

‎packages/api/src/mcp/__tests__/sqlToolBatch.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,14 @@ describe('clickstack_sql batch mode', () => {
320320
content: [
321321
{
322322
type: 'text' as const,
323-
text: JSON.stringify({ result: { data: Array(10).fill(bigRow) } }),
323+
text: JSON.stringify({
324+
result: { data: Array(10).fill(bigRow) },
325+
trimNotes: [
326+
'Response trimmed from 90,000 to 50,000 chars (limit 50,000).',
327+
'result.data: cut from 500 to 10 items',
328+
'10 long value(s) shortened (4,000 chars removed)',
329+
],
330+
}),
324331
},
325332
],
326333
});
@@ -338,6 +345,10 @@ describe('clickstack_sql batch mode', () => {
338345
expect(entry.result.data.length).toBeGreaterThan(0);
339346
expect(entry.result.data.length).toBeLessThan(10);
340347
expect(entry.note).toContain('batch response size budget');
348+
// The batch cut replaces the per-query notes it made stale.
349+
expect(entry.trimNotes).toEqual([
350+
`Rows cut to ${entry.result.data.length} to fit the batch response size budget.`,
351+
]);
341352
}
342353
});
343354

‎packages/api/src/mcp/tools/query/emergingSignals.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { z } from 'zod';
33
import { READ_ONLY_TOOL } from '@/mcp/tools/annotations';
44
import type { ToolRegistrar } from '@/mcp/tools/types';
55
import { mcpUserError } from '@/mcp/utils/errors';
6-
import { trimToolResponse } from '@/utils/trimToolResponse';
6+
import { type Reduction, trimToolResponse } from '@/utils/trimToolResponse';
77

88
import { parseTimeRange } from './helpers';
99
import { mineWindowPatterns, normalizeTemplate } from './runEventPatterns';
@@ -130,6 +130,11 @@ const emergingSignalsSchema = z.object({
130130

131131
// ─── Tool registration ───────────────────────────────────────────────────────
132132

133+
export const EMERGING_SIGNALS_REDUCTIONS: Reduction[] = [
134+
{ path: 'emerging[].pattern', maxChars: 2000 },
135+
{ path: 'disappeared[].pattern', maxChars: 2000 },
136+
];
137+
133138
export function registerEmergingSignals({
134139
context,
135140
registerTool,
@@ -342,7 +347,13 @@ export function registerEmergingSignals({
342347
'the threshold — a valid finding.',
343348
};
344349

345-
const { data, isTrimmed } = trimToolResponse(output);
350+
const {
351+
data,
352+
isTrimmed,
353+
notes: trimNotes,
354+
} = trimToolResponse(output, undefined, {
355+
reductions: EMERGING_SIGNALS_REDUCTIONS,
356+
});
346357
return {
347358
content: [
348359
{
@@ -352,6 +363,7 @@ export function registerEmergingSignals({
352363
? {
353364
...data,
354365
note: 'Trimmed for context size — lower topN or narrow windows.',
366+
trimNotes,
355367
}
356368
: data,
357369
null,

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,17 @@ export function registerEventDeltas({ context, registerTool }: ToolRegistrar) {
463463
'are dropped by default — set includeHidden:true to inspect them.',
464464
};
465465

466-
const { data: trimmedOutput, isTrimmed } = trimToolResponse(output);
466+
const {
467+
data: trimmedOutput,
468+
isTrimmed,
469+
notes: trimNotes,
470+
} = trimToolResponse(output);
467471

468472
const finalOutput = isTrimmed
469473
? {
470474
...trimmedOutput,
471475
note: 'Result was trimmed for context size. Narrow the time range, add filters, or reduce topN/topValuesPerProperty to reduce data.',
476+
trimNotes,
472477
}
473478
: trimmedOutput;
474479

‎packages/api/src/mcp/tools/query/helpers.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,12 @@ function isEmptyResult(result: unknown): boolean {
355355
}
356356

357357
function formatQueryResult(result: unknown, maxResponseChars?: number) {
358-
const { data: trimmedResult, isTrimmed } = trimToolResponse(
359-
result,
360-
maxResponseChars,
361-
);
358+
// Trimmed under its response key, so note paths read `result.data…`.
359+
const {
360+
data: { result: trimmedResult },
361+
isTrimmed,
362+
notes: trimNotes,
363+
} = trimToolResponse({ result }, maxResponseChars);
362364
const empty = isEmptyResult(result);
363365
return {
364366
content: [
@@ -370,6 +372,7 @@ function formatQueryResult(result: unknown, maxResponseChars?: number) {
370372
...(isTrimmed
371373
? {
372374
note: 'Result was trimmed for context size. Narrow the time range or add filters to reduce data.',
375+
trimNotes,
373376
}
374377
: {}),
375378
...(empty

‎packages/api/src/mcp/tools/query/runEventPatterns.ts‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
import { MCP_QUERY_MAX_EXECUTION_SEC } from '@/mcp/utils/timeout';
2626
import { getCounter } from '@/utils/instrumentation';
2727
import logger from '@/utils/logger';
28-
import { trimToolResponse } from '@/utils/trimToolResponse';
28+
import { type Reduction, trimToolResponse } from '@/utils/trimToolResponse';
2929

3030
import {
3131
clickHouseErrorResult,
@@ -362,6 +362,19 @@ export function normalizeTemplate(pattern: string): string {
362362

363363
// ─── Event pattern mining ────────────────────────────────────────────────────
364364

365+
// Shrink samples, then long text, before the generic trim cuts whole patterns,
366+
// so topN patterns survive when they can. whereSnippet is dropped, never
367+
// shortened: a cut snippet is invalid SQL. Dropping it is the last step, so
368+
// the generic fallback (which shortens strings) never sees one.
369+
export const EVENT_PATTERNS_REDUCTIONS: Reduction[] = [
370+
{ path: 'patterns[].samples', keep: 3 },
371+
{ path: 'patterns[].samples', keep: 1 },
372+
{ path: 'patterns[].samples[]', maxChars: 2000 },
373+
{ path: 'patterns[].pattern', maxChars: 2000 },
374+
{ path: 'patterns[].trend', drop: true },
375+
{ path: 'patterns[].whereSnippet', drop: true },
376+
];
377+
365378
export async function runEventPatterns(
366379
teamId: string,
367380
sourceId: string,
@@ -554,12 +567,26 @@ export async function runEventPatterns(
554567
'add them again. Skip patterns that have drilldownUnavailable.',
555568
};
556569

557-
const { data: trimmedOutput, isTrimmed } = trimToolResponse(output);
570+
const {
571+
data: trimmedOutput,
572+
isTrimmed,
573+
notes: trimNotes,
574+
} = trimToolResponse(output, undefined, {
575+
reductions: EVENT_PATTERNS_REDUCTIONS,
576+
});
558577

578+
const snippetsDropped = trimmedOutput.patterns.some(
579+
p => !('whereSnippet' in p),
580+
);
559581
const finalOutput = isTrimmed
560582
? {
561583
...trimmedOutput,
562-
note: 'Result was trimmed for context size. Narrow the time range, add filters, or reduce topN to reduce data.',
584+
note:
585+
'Result was trimmed for context size. Narrow the time range, add filters, or reduce topN to reduce data.' +
586+
(snippetsDropped
587+
? ' whereSnippet was removed from the patterns to fit, so the usage steps for it do not apply; narrow the time range or filters, or lower topN, to get it back.'
588+
: ''),
589+
trimNotes,
563590
}
564591
: trimmedOutput;
565592

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { READ_ONLY_TOOL } from '@/mcp/tools/annotations';
44
import type { ToolRegistrar } from '@/mcp/tools/types';
55
import { mcpUserError } from '@/mcp/utils/errors';
66
import logger from '@/utils/logger';
7-
import { trimToolResponse } from '@/utils/trimToolResponse';
7+
import { SIZE_NOTE_PREFIX, trimToolResponse } from '@/utils/trimToolResponse';
88

99
import { PREFER_BUILDER_OVER_SQL_NUDGE } from './builderCatalog';
1010
import { denoiseSearchResults } from './denoise';
@@ -114,14 +114,22 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
114114
const resultText = result.content?.[0]?.text;
115115
if (!resultText) return result;
116116

117-
let parsed: { result?: { data?: Record<string, unknown>[] } };
117+
let parsed: {
118+
result?: { data?: Record<string, unknown>[] };
119+
trimNotes?: string[];
120+
};
118121
try {
119122
parsed = JSON.parse(resultText);
120123
} catch {
121124
return result;
122125
}
123126

124127
const resultData = parsed.result;
128+
// runConfigTile may already have trimmed the rows; keep its notes, except
129+
// the size line, which denoise makes stale.
130+
const firstPassNotes = (parsed.trimNotes ?? []).filter(
131+
n => !n.startsWith(SIZE_NOTE_PREFIX),
132+
);
125133
const rows = (resultData as Record<string, unknown> | undefined)?.data as
126134
| Record<string, unknown>[]
127135
| undefined;
@@ -150,7 +158,11 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
150158
'denoiseSearchResults failed; returning raw results',
151159
);
152160

153-
const { data: trimmedResult, isTrimmed } = trimToolResponse(resultData);
161+
const {
162+
data: { result: trimmedResult },
163+
notes,
164+
} = trimToolResponse({ result: resultData });
165+
const trimNotes = [...firstPassNotes, ...notes];
154166
return {
155167
content: [
156168
{
@@ -164,9 +176,10 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
164176
filteredRowCount: rows.length,
165177
skipped: 'denoise_failed',
166178
},
167-
...(isTrimmed
179+
...(trimNotes.length > 0
168180
? {
169181
note: 'Result was trimmed for context size. Narrow the time range or add filters to reduce data.',
182+
trimNotes,
170183
}
171184
: {}),
172185
},
@@ -185,8 +198,11 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
185198
...resultData,
186199
data: denoised.rows,
187200
};
188-
const { data: trimmedResult, isTrimmed } =
189-
trimToolResponse(denoisedResult);
201+
const {
202+
data: { result: trimmedResult },
203+
notes,
204+
} = trimToolResponse({ result: denoisedResult });
205+
const trimNotes = [...firstPassNotes, ...notes];
190206

191207
return {
192208
content: [
@@ -203,9 +219,10 @@ export function registerSearch({ context, registerTool }: ToolRegistrar) {
203219
filteredRowCount: denoised.rows.length,
204220
...(denoised.skipped ? { skipped: denoised.skipped } : {}),
205221
},
206-
...(isTrimmed
222+
...(trimNotes.length > 0
207223
? {
208224
note: 'Result was trimmed for context size. Narrow the time range or add filters to reduce data.',
225+
trimNotes,
209226
}
210227
: {}),
211228
...(denoised.rows.length === 0 && !denoised.skipped

0 commit comments

Comments
 (0)