Repository navigation
fix(mcp): shorten instead of replacing when trimming tool responses, ordered reductions, trim notes - #3554
fix(mcp): shorten instead of replacing when trimming tool responses, ordered reductions, trim notes#3554bcanvural wants to merge 10 commits into
Conversation
…add ordered reductions and trim notes
trimToolResponse replaced any value over its budget with {"__hdx_trimmed":true},
so the longest values (often the top result, such as a stack-trace pattern)
disappeared, and event_patterns lost most of its topN when the response was a
little over budget. Strings are now shortened with a "…(+N chars)" suffix, the
result lists what was trimmed (trimNotes), and an ordered reductions option
(keep / maxChars / drop per path) is applied to every match, measuring the whole
response after each step, before the per-key fallback. event_patterns and
emerging_signals use it. Refs ClickHouse#3542.
|
@bcanvural is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: f33b35f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| data: trimmedOutput, | ||
| isTrimmed, | ||
| notes: trimNotes, | ||
| } = trimToolResponse(output, undefined, { |
There was a problem hiding this comment.
🟠 major — whereSnippet is now cut into an invalid Lucene query that the agent is told to paste into clickstack_search
whereSnippet (built at runEventPatterns.ts:376-379 as Body:"<all literal tokens>") is about as long as pattern. The reductions shorten pattern to 2,000 chars but leave whereSnippet alone. If the response still doesn't fit, the fallback's new shortenToFit now cuts it to something like Body:"java.lang… …(+4,300 chars). That is an unclosed phrase with a junk suffix, and the response tells the agent to use it as where as-is. Before this PR the value became an obvious marker object. Fix: add a reduction for patterns[].whereSnippet (drop it, or rebuild it from the shortened pattern) that runs before the fallback, or have the fallback never cut strings meant to be copied. The 30-pattern test hides this because its whereSnippet is capped at 200 chars (text.slice(0, 200)). Build it the way production does so the test covers this.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
| // If we're still over budget (e.g. a single item exceeds maxSize), truncate | ||
| // individual oversized items so the array itself stays within the limit. | ||
| if (result.length < arr.length) { | ||
| stats.arrays.push({ path, from: arr.length, to: result.length }); |
There was a problem hiding this comment.
🔵 minor — Array-cut notes are recorded once per parent item, so the same path repeats up to 10 times (or more when arrays nest)
A large array item is trimmed with path ${path}[] (line 248-249), and every nested array that trimObject cuts pushes its own stats.arrays entry. A patterns array with 10 oversized items therefore produces 10 separate patterns[].samples: cut from N to M items notes. These notes are added to the response after the size budget was applied. Fix: group entries by path, like applyReduction does, into one line with the item count and total removed.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| ...(isTrimmed | ||
| ? { | ||
| note: 'Result was trimmed for context size. Narrow the time range or add filters to reduce data.', | ||
| trimNotes, |
There was a problem hiding this comment.
🔵 minor — trimNotes become wrong when clickstack_sql batch mode cuts more rows afterwards
formatQueryResult adds trimNotes (for example data: cut from 500 to 10 items and Response trimmed from A to B chars). Then fitToBudget in sqlBatch.ts:133-153 keeps halving result.data without updating those notes, so the agent sees row counts and sizes that are no longer true. Fix: have fitToBudget add a note of its own when it cuts rows (or replace trimNotes), so the notes match the payload the agent actually gets.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
PR Review
1 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 1 minor 1 posted as inline comment(s) on the changed lines. Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep Review✅ No critical issues found. This PR has been through several review rounds; the previously flagged ship-blockers (scalar values growing into 🟡 P2 — recommended
🔵 P3 nitpicks (4)
Reviewers (12): correctness, adversarial, testing, maintainability, kieran-typescript, performance, api-contract, reliability, previous-comments, project-standards, agent-native, learnings. Testing gaps:
No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
…m notes accurate - event_patterns drops whereSnippet as the last reduction instead of letting the fallback shorten it (a cut snippet is invalid SQL); the note says so. - shortenString never makes a value longer and doesn't split a surrogate pair; removed-char counts include the suffix. - trimArray counts separators in the item budget and returns the array as-is when no single item is large. - Notes: array cuts grouped by path, size markers noted, reduction notes say 'cut to the first N chars'; skip re-measuring after a no-op step. - Note paths match the response (trimmed under 'result' in formatQueryResult and search); search denoise keeps the first pass's notes; clickstack_sql batch appends its row cut to the per-query notes. - Remove section banner comments.
|
Thanks for the reviews. Round 1 is in the latest commit (after merging Fixed
Not changed, to keep the scope
|
| result[key] = trimObject(value, perKeyBudget); // eslint-disable-line security/detect-object-injection | ||
| result[key] = trimObject(value, perKeyBudget, keyPath, stats); // eslint-disable-line security/detect-object-injection | ||
| } else if (typeof value === 'string') { | ||
| result[key] = shortenToFit(value, perKeyBudget, stats); // eslint-disable-line security/detect-object-injection |
There was a problem hiding this comment.
🟠 major — Fallback output now goes over maxSize: trimObject gives each string its full per-key share but doesn't count key names, separators or the added __hdx_trimmed flag
Before this PR, an oversized string became a ~40-char marker, so the per-key overhead never mattered. Now shortenToFit(value, perKeyBudget) fills the whole share. Example: trimToolResponse({ aaaaaaaaaa: 'x'.repeat(5000), bbbbbbbbbb: 'y'.repeat(5000) }, 2000) returns about 2,050 chars (2×1000 + 2×13 key chars + braces/comma + 21 for ,"__hdx_trimmed":true). This also happens to every row object trimArray passes to trimObject with itemBudget, so wide ClickHouse rows go over maxResponseChars by roughly (key length + 4) per column per row. Fix: subtract the overhead before splitting, e.g. perKeyBudget = Math.floor((maxSize - 2 - 21 - Σ(JSON.stringify(key).length + 2)) / entries.length). The new 'stays within the budget' test misses this because its third key c: 'ok' leaves enough room to absorb the overhead. Add a case with two or more large values and no small key.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
trimObject split the whole budget across values, but braces, quoted keys, colons, commas and the __hdx_trimmed flag also count, so a trimmed object could end over maxSize. Subtract them before splitting. search.ts: trimNotes is never empty when isTrimmed is true, so the extra check is redundant.
|
Fixed in the latest commit:
One case is still over the limit, and it predates this PR: @brandon-pereira, ready for a look when you have time. |
| data: { result: trimmedResult }, | ||
| notes, | ||
| } = trimToolResponse({ result: denoisedResult }); | ||
| const trimNotes = [...firstPassNotes, ...notes]; |
There was a problem hiding this comment.
🔵 minor — With denoise on, trimNotes can hold two conflicting 'Response trimmed from X to Y' lines, and this merge has no test
firstPassNotes (from runConfigTile/formatQueryResult) and the second trimToolResponse each start with their own sizeNote. The agent then sees two different before/after sizes for one response. Drop the stale size line from firstPassNotes before merging (or keep only the latest one), and add a test for the denoise path that checks the merged trimNotes.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| // replaces this line rather than adding another. | ||
| const earlier = Array.isArray(entry.trimNotes) ? entry.trimNotes : []; | ||
| entry.trimNotes = [ | ||
| ...earlier.filter(n => !String(n).startsWith(BATCH_CUT_PREFIX)), |
There was a problem hiding this comment.
🔵 minor — After a batch halving, the per-query size note left in trimNotes is false
The comment says the earlier notes 'are still true'. But formatQueryResult's 'Response trimmed from A to B chars' and 'result.data: cut from X to Y items' no longer match once fitToBudget halves the rows. Filter out the earlier Response trimmed from and result.data: cut lines here along with the BATCH_CUT_PREFIX line. The fixture in sqlToolBatch.test.ts also uses 'data: cut from…', which is not the result.data… path that formatQueryResult actually emits, so the test doesn't reflect real input.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in 022a787. When the batch halves a result's rows, its per-query notes are replaced by the batch line, since their sizes and counts no longer match the rows left. Shortened values still carry their own …(+N chars). The test fixture now uses the notes formatQueryResult actually emits.
| reductions: [ | ||
| { path: 'emerging[].pattern', maxChars: 2000 }, | ||
| { path: 'disappeared[].pattern', maxChars: 2000 }, | ||
| ], |
There was a problem hiding this comment.
🔵 minor — The new emerging[].pattern / disappeared[].pattern reductions have no test
Only event_patterns gets an integration test for its reduction list. Add an emergingSignals test with patterns over 2,000 chars and an over-limit response. It should assert that patterns are shortened with the …(+N chars) suffix, that topN entries survive, and that trimNotes contains the reduction line.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| (sum, [key]) => sum + JSON.stringify(key).length + 2, | ||
| 2 + TRIMMED_FLAG_LENGTH, | ||
| ); | ||
| const perKeyBudget = Math.max( |
There was a problem hiding this comment.
When an object's budget is smaller than its key overhead, perKeyBudget clamps to 0. Every value then counts as over budget, so even a 1-char number becomes a 39-char marker and the output grows. Main keeps these values whole.
trimToolResponse({ ts: '2026-01-01T00:00:00.000Z', count: 1 }, 20).data
// main: {"ts":{"__hdx_trimmed":true,"originalSize":26},"count":1,"__hdx_trimmed":true} (78 chars)
// PR: {"ts":"…(+24 chars)","count":{"__hdx_trimmed":true,"originalSize":1},"__hdx_trimmed":true} (90 chars)
const rows = Array.from({ length: 12 }, () => ({ a: 1, b: 2, c: 3, d: 4, e: 5, f: 6 }));
trimToolResponse(rows, 100) // main: 381 chars, PR: 2,871 charsThis happens in practice with trend items when an oversized pattern-like result goes through the fallback, which every tool without reductions does. Suggested fix: never replace a value with a marker longer than the value itself (and skip shortening a string when the suffix saves almost nothing).
There was a problem hiding this comment.
Fixed in 022a787. A number, boolean or null now always stays whole, since any marker is longer, so the marker path is gone. A string is shortened to the longest cut that fits, and only when that saves at least twice the …(+N chars) suffix. Your examples, both now tests: { ts, count: 1 } at 20 comes back whole (43 chars; main 78), and the 12 rows at 100 come back at 381 chars, the same as main.
| { path: 'patterns[].samples[]', maxChars: 2000 }, | ||
| { path: 'patterns[].pattern', maxChars: 2000 }, | ||
| { path: 'patterns[].trend', drop: true }, | ||
| { path: 'patterns[].whereSnippet', drop: true }, |
There was a problem hiding this comment.
When all the reductions run and the response is still over budget, the fallback leaves most of the budget unused. Test case: 30 patterns, each a ~10 KB stack trace with 5 samples (~2 MB input). After every reduction, including dropping whereSnippet from all 30, the per-key split still cuts patterns to 10 and shortens each pattern to ~250 chars. The final response is 7,356 chars of the 50,000 budget; main returns 19,160 for the same input. The agent loses drill-down for every pattern, and 85% of the budget goes unused.
The per-key split predates this PR, but the result here is worse than main's. One option: after the reductions, give patterns whatever budget the other keys don't use, instead of an equal third.
There was a problem hiding this comment.
Fixed in 022a787, in trimObject: values within an equal share stay whole and the large ones split the rest, as trimArray already does for items. For your input (30 × ~10 KB stack traces with 5 samples, ~1.8 MB), the response is now 45,132 chars with 10 patterns of 2,000 chars; main returns 19,712. Test added.
On drill-down: templates that long are over MAX_PREDICATE_TEMPLATE_LENGTH (4,000 chars), so their whereSnippet is already empty (drilldownUnavailable). For shorter templates the order stays: the reductions drop whereSnippet to keep all topN patterns. If you'd rather keep drill-down on fewer patterns, I can add a patterns keep step before that drop.
…ep trim notes true - trimObject: values within an equal share stay whole and the large ones split the rest, as trimArray does for items, so one large value (such as event_patterns' patterns) gets the room the small ones don't use. - A number, boolean or null is never replaced by a size marker, which is always longer. A string is shortened to the longest cut that fits, and only when that saves at least twice the "…(+N chars)" suffix. - clickstack_sql batch: halving a result's rows replaces its per-query notes, whose sizes and counts no longer hold. search with denoise drops the first trim's size line. - The event_patterns and emerging_signals reduction lists are exported and the tests use them.
|
Also in 022a787, for the open review findings:
Not changed: |
| } | ||
| } | ||
| result[key] = out; // eslint-disable-line security/detect-object-injection | ||
| if (out !== value) trimmed = true; |
There was a problem hiding this comment.
🔵 minor — __hdx_trimmed gets set on objects where nothing was trimmed
trimArray always returns a new array ([...arr]) and trimObject always returns a new result, so the check out !== value is true even when nothing changed. Two cases: an over-budget array that trimArray returns early at the largeCount === 0 branch, and a nested object whose scalars are all kept whole. In both, the parent is still marked __hdx_trimmed: true and gets 20 extra chars. Fix: have trimArray/trimObject return the original reference when nothing changed (or report a changed flag), and set the flag only on a real change.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
This was already on main, which flags a parent whenever a value is over its share, changed or not (this PR only stops that for scalars), so I'm leaving it out of this PR.
|
d320f6c adds a test for a failed denoise on a trimmed result: Not changed: the |
|
f33b35f adds tests for a top-level string over the limit, and for
|
| const resultData = parsed.result; | ||
| // runConfigTile may already have trimmed the rows; keep its notes, except | ||
| // the size line, which denoise makes stale. | ||
| const firstPassNotes = (parsed.trimNotes ?? []).filter( |
There was a problem hiding this comment.
🔵 minor — Denoise keeps first-pass trim counts that no longer match the rows it returns
Only the size line is filtered out, but denoise then removes rows. So a first-pass note like N long value(s) shortened (… chars removed) still counts values in rows that are no longer in result.data, and result.data: cut from 120 to 80 items sits next to a data array of 20 rows. Either drop long value(s) shortened along with SIZE_NOTE_PREFIX, or reword the kept notes so they say they describe the rows before denoise (they match returnedRowCountBeforeDenoise).
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Not changing. The kept notes describe the first pass, and denoised.returnedRowCountBeforeDenoise is that pass's row count, so they read in order: 120 → 80 trimmed, then 80 → 20 denoised. Values shortened in the 20 rows left still carry their …(+N chars).
| ); | ||
| const largeCount = sizes.filter(s => s > share).length; | ||
| // Over budget only through separators: no single item to shorten. | ||
| if (largeCount === 0) return result; |
There was a problem hiding this comment.
🔵 minor — An array over budget only because of separators gets marked __hdx_trimmed and stays over its budget
When largeCount === 0, trimArray returns result, which is a fresh copy ([...arr]) with nothing removed. In trimObject, out !== value is then true, so the parent gets __hdx_trimmed: true and the top level reports isTrimmed with only a size note, even though no data changed and the value is still over perKeyBudget. Return arr itself when nothing was cut or shortened, or set trimmed from the stats instead of reference identity. The existing separators test only covers a top-level array, so it doesn't catch this.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Not changing here: main does the same (it returns this array as is and flags the parent whenever a value is over its share), as in the thread on line 337. The overshoot is at most 11 characters: an array only ends up here once it is down to 10 items or fewer, each within its share, so only the commas and brackets are over.
|
On deep-review's P2s for f33b35f, not changing:
|
|
@bcanvural code looks good but linting is failing |
Main's ratchet fails when a package gains eslint-disable comments. Reach paths through Reflect, pair values with their sizes, and build objects with Object.fromEntries, so the trimming code needs none.
|
Fixed in f9789fc. The lint job's ratchet fails when a package gains |
| return { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion | ||
| data: trimmed as T, | ||
| isTrimmed: true, |
There was a problem hiding this comment.
🔵 minor — Fallback reports isTrimmed:true and a size note even when it changed nothing
The fallback now keeps numbers, booleans, null and short strings whole (shortenString won't cut them), and trimArray returns early when largeCount === 0. So an over-budget value can come back identical to the input, yet the result is still isTrimmed: true with the note Response trimmed from 111 to 111 chars (limit 110). One example is a 10-item array that's over the limit only because of its separators; another is a describeTable sample row of numeric columns. Callers then add 'Result was trimmed… values may be omitted' when nothing was omitted, which goes against the PR's goal of keeping trim notes true. Fix: set isTrimmed to notes.length > 0 || trimmed !== current (or compare finalSize to originalSize), and skip the size note when nothing changed.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Not changing. It needs a response over the limit with nothing the fallback can shorten (only numbers, booleans, null or short strings, or an array over only by its commas). Tool responses at 50,000 chars don't take that shape.
|
|
||
| // Calls `onTarget(container, key)` for every value the path points at. | ||
| function visit( | ||
| node: any, |
There was a problem hiding this comment.
🔵 minor — New visit/applyReduction helpers are typed with any
The repo conventions say 'Avoid any - use proper typing'. visit(node: any, …, onTarget: (container: any, …)) and applyReduction(root: any, …) are new code. Type them as unknown / object (and unknown[] | Record<string, unknown> for the container). The existing typeof node === 'object' and Array.isArray checks already narrow them, so the Reflect.get/Reflect.set calls need no casts.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Not changing: trimArray and trimObject in this file take any too, and the walker reads arbitrary JSON.
| } else if (typeof current === 'string') { | ||
| trimmed = shortenToFit(current, maxSize, stats); | ||
| } else { | ||
| return { data: current, isTrimmed: notes.length > 0, notes }; |
There was a problem hiding this comment.
🔵 minor — isTrimmed: notes.length > 0 in the non-object branch is always false
This branch only runs when the root value is not an object, array or string. visit returns right away for non-objects (line 170), so no reduction can match the root and notes is always empty here. Return { data: current, isTrimmed: false, notes: [] } directly so it doesn't look like reductions could have applied.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Only a number, boolean or null root reaches this line, and notes is always empty there, so it already returns isTrimmed: false. Not worth a push.
| * Applied in order, each to every match, before the generic fallback. The | ||
| * whole response is measured after each step, and trimming stops as soon as | ||
| * it fits. | ||
| */ |
There was a problem hiding this comment.
🔵 minor — Reduction union accepts mixed kinds such as { path, keep: 3, drop: true }, and applyReduction silently treats it as a drop
Add a discriminant to the union, e.g. { kind: 'keep', … } | { kind: 'maxChars', … } | { kind: 'drop', … }, so a mixed entry fails to compile. Then switch on kind in applyReduction instead of the 'drop' in / 'keep' in checks, which is order-dependent. TypeScript's excess-property check on a union allows keys from any member, so a typo'd entry in EVENT_PATTERNS_REDUCTIONS compiles today and runs as a different reduction.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Refs #3542
Follows the proposal in the maintainer comment on #3542 (the three points, in order). Kept to
trimToolResponse; the table tool'shaving/limitwill go to a separate issue and PR as asked.What
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).Trim notes.
TrimResultgainsnotes: 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 anotewhen trimmed now also returns them astrimNotes;clickstack_describe_tableis not changed here.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:clickstack_event_patternsuses the list from the proposal (samples keep 3, keep 1, samples[] maxChars 2000, pattern maxChars 2000, trend drop), plus a last step:patterns[].whereSnippetis 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; thenotesays when snippets were removed.clickstack_emerging_signalsshortensemerging[].pattern/disappeared[].patternto 2,000 chars first.Smaller fixes from review.
shortenStringnever 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.…);searchwithdenoisekeeps the first trim's notes; theclickstack_sqlbatch 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):main{"__hdx_trimmed":true,"originalSize":4736}…(+N chars)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;dropon array elements.packages/api/src/mcp/__tests__/queryTool.int.test.ts: the realevent_patternstool with 30 large patterns keeps all 30, drops (never cuts)whereSnippet, and returnstrimNotes.sqlToolBatch.test.ts: batch cuts replacetrimNotes.npx jest src/mcp src/utils --testPathIgnorePatterns '\.int\.test\.ts$': 932 passed;queryTool+emergingSignalsToolint tests: 117 passed. tsc, eslint (no new warnings) and prettier clean on the changed files.Not included
having/limit/ default order forclickstack_table: separate issue and PR (clickstack_table: nolimit,havingor default order, so large grouped results come back trimmed in arbitrary order #3555, feat(mcp): limit, having and a default order for clickstack_table #3558), as asked.