Repository navigation
feat(mcp): add denoise option to hyperdx_search tool - #2371
Merged
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
May 29, 2026 17:58
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
Contributor
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, | ||
| }); |
Contributor
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); | ||
| } |
Contributor
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; | ||
| } |
Contributor
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; | |
| } |
kodiakhq Bot
pushed a commit
that referenced
this pull request
Nov 11, 2026
## Summary
"Denoise results" on the search page often left noisy rows in the table. Three bugs caused it:
- **It mined the last table column, not the body.** `patternColumn = columns[columns.length - 1]`. Adding a column from the side panel appends it, so denoise switched to that attribute. An added map column carries an alias (`LogAttributes['k'] AS "k"`), and the alias was then used as a SQL expression in the sample query. Denoise now uses the source's body expression (`getEventBody`), finds its result column by position in the select list, and says so in the panel when the body isn't in the table.
- **ANSI-colored rows never matched.** Sampling strips ANSI codes; matching the visible rows didn't. In a local replay (drain3 wheel the app loads, 10k sampled rows, 200 visible rows), a colored template at ~27% of traffic had 0 of 54 rows removed. With this PR: 54 of 54.
- **Rows were re-matched with drain3's `match()`.** That needs an exact template fit and can put a row in a different cluster from the one it was counted in. The MCP denoise already replaced this with `buildNoisyBodyMatcher`. That matcher moves to `common-utils/src/drain/noisy-matcher.ts`, gains an `extraDelimiters` option so it tokenizes like the web miner, and is now used by both.
The denoise logic moves out of `DBRowTable.tsx` into `hooks/useDenoisedRows.ts` and `components/DenoiseSummary.tsx`. If the pattern query fails, the table now shows all rows instead of an empty table.
**Noise threshold: 10% → 5%.** A pattern counts as noisy when its cluster holds more than the threshold share of the 10k-row sample. Drain splits one logical pattern across clusters by token count and first token, so on the demo logs no cluster passed 10% (the top one was 9.2%) and denoise removed nothing. At 5%, the demo data's 6-hour window hides 6–7 patterns (the sample is random) covering about 45% of it. The threshold lives in `selectNoisyPatterns` (common-utils), shared by the app and the MCP `denoise` option, whose tool description is updated. The MCP integration test still passes.
**Errors and warnings are never hidden.** Frequency isn't importance: during an incident a flood of errors becomes the most common pattern. The app skips any pattern whose sampled lines include an error or warning (`getLogLevelClass`). The MCP denoise doesn't apply this yet, because its sample query doesn't select the level.
**Readable panel.** The raw template list is replaced by a one-line summary ("Hiding 6 common patterns, about 45% of events. Errors and warnings are never hidden.") with a "Show patterns" toggle. Each pattern is one truncated line with its share of events, and runs of wildcards collapse to `…` (`simplifyPattern`). Hover shows the full template.
### Screenshots or video
Demo Logs on the Vercel previews (`main` and this branch), same 6-hour window, "Denoise results" on.
| Before | After |
| :----- | :---- |
|  |  |
| "No noisy patterns found", nothing removed | Summary line; error rows still shown |
| |  |
| | "Show patterns" expanded |
### How to reproduce
**Starting point:** `yarn app:dev:local` connected to the demo server (onboarding → "Connect to Demo Server").
**On `main`:**
1. Open Search with the Logs source and turn on "Denoise results" in the filters panel.
2. Expand any row and add a log attribute as a column from the side panel.
3. Before step 2, the panel already reads "No noisy patterns found" and no rows are removed. After step 2, denoise mines that attribute instead of the body, or the table stays empty if the sample query fails.
**On this branch:**
1. Turn on "Denoise results". The panel reads "Hiding … common patterns, about …% of events…". Click "Show patterns" to see them (e.g. "Browsing product"); those rows are gone from the table, and error rows remain.
2. Repeat step 2 above. The removed patterns are still body patterns.
3. Remove the `Body` column from the select. The panel reads "Add Body to the table columns to denoise results." and all rows are shown.
### How to test on Vercel preview
**Preview routes:** /search
**Steps:**
1. Select the Logs source.
2. Turn on "Denoise results" in the filters panel.
3. Verify the text starting "Hiding" and ending "Errors and warnings are never hidden." appears above the results table.
4. Click "Show patterns" and verify a list of one-line patterns with percentages appears.
5. Change the select to `Timestamp, ServiceName` and run the search.
6. Verify the text "Add Body to the table columns to denoise results." appears and results are shown.
### References
- Linear Issue:
- Related PRs: #2371 (MCP denoise, which introduced `buildNoisyBodyMatcher`)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
This branch was successfully deployed
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
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