feat(mcp): add denoise option to hyperdx_search tool#2371
Merged
Conversation
Add a `denoise` boolean parameter to the MCP `hyperdx_search` tool that automatically filters out high-frequency repetitive event patterns from search results, mirroring the web app's "Denoise Results" feature. When `denoise=true`: - Samples 10k random events from the same source/time range - Mines patterns using the shared Drain algorithm (common-utils) - Identifies "noisy" patterns (>10% of sampled events) - Matches each search result row against learned patterns - Filters out rows matching noisy patterns - Returns filtered rows plus metadata listing removed patterns Also extracts `resolveBodyExpression()` and `SAFE_BODY_EXPR_CHARS` from runEventPatterns.ts into helpers.ts for reuse.
🦋 Changeset detectedLatest commit: 27f0542 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Resolve conflicts in helpers.ts and runEventPatterns.ts:
- helpers.ts: keep both our resolveBodyExpression/SAFE_BODY_EXPR_CHARS
exports and main's mergeWhereIntoSelectItems/clickHouseErrorResult
- runEventPatterns.ts: import resolveBodyExpression, SAFE_BODY_EXPR_CHARS,
and clickHouseErrorResult from helpers
- search.ts: update trimToolResponse usage to new { data, isTrimmed } API
Add changeset for the denoise feature (patch).
Contributor
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Contributor
Deep Review✅ No critical issues found. No P0/P1: there is no data loss, auth bypass, or injection introduced here (the SQL-interpolated body column is operator-configured, not end-user input), and the graceful-degradation contract holds on every path — a denoise failure is caught and the original search result is always preserved. The findings below are recommended improvements and nits. 🟡 P2 — recommended
🔵 P3 nitpicks (9)
Reviewers (12): correctness, security, adversarial, testing, maintainability, performance, api-contract, reliability, kieran-typescript, project-standards, agent-native, learnings-researcher. Testing gaps:
|
Contributor
E2E Test Results✅ All tests passed • 198 passed • 3 skipped • 1320s
Tests ran across 4 shards in parallel. |
- Key noisy patterns by template string (p.pattern / match.getTemplate()) instead of per-instance cluster ID, eliminating fragile coupling to minePatterns() internal auto-increment ordering - Always emit a 'denoised' metadata block when denoise=true, including a 'skipped' field with the reason when denoising cannot proceed (source_not_found, no_body_column, body_column_not_in_results, connection_not_found, sampling_failed, no_sample_data, no_rows) - Rename originalRowCount to returnedRowCountBeforeDenoise to make the post-trim semantics explicit - Fix misleading maxSamples:0 comment (minePatterns always keeps at least one sample per cluster); use maxSamples:1 instead - Add integration tests for denoise=true: schema exposure, empty results handling, noisy pattern filtering with seeded data, denoised metadata shape assertions, and denoise=false control case
brandon-pereira
requested review from
a team and
teeohhem
and removed request for
a team
teeohhem
reviewed
Jun 2, 2026
teeohhem
previously approved these changes
Jun 2, 2026
- Resolve conflict in search.ts: keep clickstack_ prefix + denoise description - Rename hyperdx_ to clickstack_ in denoise tests - Extract DENOISE_SAMPLE_SIZE and DENOISE_NOISE_THRESHOLD to common-utils/drain (shared between FE DBRowTable and BE MCP denoise)
Wrap the denoiseSearchResults call in a try/catch so that a failure in the denoise post-processing step (getSource, getConnectionById, pattern mining, etc.) does not discard an already-successful search result. On catch, return the original rows with a denoised metadata block containing skipped: 'denoise_failed', following the same graceful- degradation convention used by other failure paths in denoise.ts.
# Conflicts: # packages/api/src/mcp/tools/query/helpers.ts
teeohhem
approved these changes
Jun 9, 2026
Greptile SummaryThis PR adds a
Confidence Score: 4/5The change is safe to merge — denoising is opt-in, all failure paths fall back to returning the original search results, and the integration tests cover the key scenarios. The core logic is sound and graceful degradation is thorough. The two issues in denoise.ts (missing HTTP timeout on the ClickhouseClient and double Drain training on the same 10k rows) are real inefficiencies but neither causes incorrect results or data loss. The comment/behaviour inconsistency around the empty-rows early return is cosmetic. No correctness bugs were found. packages/api/src/mcp/tools/query/denoise.ts — missing requestTimeout and double Drain training Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as LLM Agent
participant Search as clickstack_search
participant CH1 as ClickHouse (search)
participant Denoise as denoiseSearchResults
participant CH2 as ClickHouse (denoise)
participant Drain as Drain/minePatterns
Agent->>Search: "{sourceId, where, denoise:true, maxResults}"
Search->>CH1: "queryChartConfig (search, limit=maxResults)"
CH1-->>Search: rows[0..N]
alt "denoise=false OR rows empty OR error"
Search-->>Agent: "{result: {...}}"
else "denoise=true AND rows non-empty"
Search->>Denoise: rows, sourceId, dateRange, where
par parallel sampling
Denoise->>CH2: SAMPLE rand() LIMIT 10k
CH2-->>Denoise: sampleRows
and count query
Denoise->>CH2: count() total
CH2-->>Denoise: totalCount
end
Denoise->>Drain: minePatterns(sampleRows) → patterns
Denoise->>Drain: new TemplateMiner().train(sampleRows)
Denoise->>Denoise: "identify noisyTemplates (>10% threshold)"
Denoise->>Denoise: filter rows by miner.match()
Denoise-->>Search: "{rows: filteredRows, removedPatterns}"
Search-->>Agent: "{result: {data: filteredRows}, denoised: {removedPatterns, ...}}"
end
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+84
to
+88
| const clickhouseClient = new ClickhouseClient({ | ||
| host: connection.host, | ||
| username: connection.username, | ||
| password: connection.password, | ||
| }); |
There was a problem hiding this comment.
requestTimeout on the ClickhouseClient created for denoise queries. helpers.ts establishes a MCP_REQUEST_TIMEOUT (32 s) specifically as a belt-and-suspenders above the server-side max_execution_time: 30 — without it the HTTP connection can hang indefinitely if ClickHouse is slow to acknowledge the server-side cancellation.
Suggested change
| const clickhouseClient = new ClickhouseClient({ | |
| host: connection.host, | |
| username: connection.username, | |
| password: connection.password, | |
| }); | |
| const clickhouseClient = new ClickhouseClient({ | |
| host: connection.host, | |
| username: connection.username, | |
| password: connection.password, | |
| requestTimeout: 32_000, // 30s query limit + 2s buffer (mirrors MCP_REQUEST_TIMEOUT) | |
| }); |
Comment on lines
+178
to
+227
| const { patterns } = minePatterns(sampleRows, { | ||
| totalCount, | ||
| startDate, | ||
| endDate, | ||
| maxSamples: 1, | ||
| getBody: row => { | ||
| const raw = row.__hdx_pattern_body; | ||
| return raw != null ? String(raw) : ''; | ||
| }, | ||
| getTimestamp: row => { | ||
| const tsRaw = row.__hdx_pattern_ts; | ||
| return tsRaw != null ? new Date(String(tsRaw)).getTime() : null; | ||
| }, | ||
| }); | ||
|
|
||
| if (patterns.length === 0) { | ||
| return { rows, removedPatterns: [] }; | ||
| } | ||
|
|
||
| // ── 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<string>(); | ||
| const removedPatterns: DenoiseResult['removedPatterns'] = []; | ||
|
|
||
| for (const p of patterns) { | ||
| if (p.sampleCount / sampledRowCount > DENOISE_NOISE_THRESHOLD) { | ||
| noisyTemplates.add(p.pattern); | ||
| removedPatterns.push({ | ||
| pattern: p.pattern, | ||
| estimatedCount: p.estimatedCount, | ||
| sampleCount: p.sampleCount, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (noisyTemplates.size === 0) { | ||
| 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); | ||
| } |
There was a problem hiding this comment.
minePatterns() trains a TemplateMiner internally (and discards it), then immediately the function builds a second TemplateMiner from scratch by re-iterating the same sampleRows. For the default 10 k sample this doubles memory allocation and Drain processing time. The workaround is necessary because minePatterns doesn't expose the trained miner. Consider either (a) adding a getMiner() return value to MinePatternResult, or (b) extracting a lower-level helper that accepts an already-trained miner so the second pass is just a match loop without re-training.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+128
to
+130
| if (!rows || !Array.isArray(rows) || rows.length === 0) { | ||
| return result; | ||
| } |
There was a problem hiding this comment.
denoised block when denoise=true" but this early return sends back the plain result object — no denoised key — when the search succeeds but returns zero rows. An LLM agent that always expects the denoised key when it requested denoise=true will see an inconsistent response shape. Either update the comment to document the exception, or emit an empty denoised block here.
Suggested change
| if (!rows || !Array.isArray(rows) || rows.length === 0) { | |
| return result; | |
| } | |
| if (!rows || !Array.isArray(rows) || rows.length === 0) { | |
| // No rows to denoise — return result as-is (no denoised block). | |
| // NOTE: this is intentional; the "always emit" comment below applies | |
| // only to the non-empty-rows path. | |
| return result; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
denoiseboolean parameter to the MCPhyperdx_searchtool that automatically filters out high-frequency repetitive event patterns from search results, mirroring the web app's "Denoise Results" checkbox.Why
When investigating issues via MCP, LLM agents get back raw search results that are often dominated by repetitive log noise. This forces agents to either sift through noisy results or make a separate
hyperdx_event_patternscall and then manually filter. Thedenoiseoption makes this a single-call workflow.How it works
When
denoise=true:common-utils)TemplateMiner, matches each search result row against learned patternsdenoisedmetadata block listing removed patterns, original row count, and filtered row countGraceful degradation: if source/connection/body column can't be resolved, or if pattern sampling fails, the original results are returned unmodified.
Changes
packages/api/src/mcp/tools/query/denoise.tsdenoiseSearchResults()functionpackages/api/src/mcp/tools/query/search.tsdenoiseschema param + post-processing logicpackages/api/src/mcp/tools/query/helpers.tsresolveBodyExpression()+SAFE_BODY_EXPR_CHARSpackages/api/src/mcp/tools/query/runEventPatterns.tsExample response (with denoise=true)
{ "result": { "data": [...filtered rows...] }, "denoised": { "removedPatterns": [ { "pattern": "GET /health <*> <*>", "estimatedCount": 45000, "sampleCount": 4500 } ], "originalRowCount": 50, "filteredRowCount": 12 } }Performance
Adds ~1-2s latency when
denoise=truedue to the pattern sampling queries. No impact whendenoise=false(the default).Closes HDX-4346