Repository navigation
feat(app): filter for or exclude a pattern from Event Patterns - #3665
alex-fedotyev wants to merge 5 commits into
Conversation
Event Patterns groups a 10,000-row sample into patterns, but a pattern could not be acted on: there was no way to see every event that fits it, or to hide it and look at what is left. Hovering a pattern row now shows two buttons. "Filter for this pattern" adds a filter for the events that fit the pattern and switches to the results table. "Exclude this pattern" adds the negated filter and stays on Event Patterns, which mines again without it. The filter is a SQL condition built with the shared compact pattern predicate, so it runs in ClickHouse over the whole time range, and the histogram and counts follow it. Pattern filters show as pills next to the other filters and are kept in the URL and in saved searches like any other filter. A sidebar facet change rebuilds the filter list from facet state, which would drop them, so useSearchPageFilterState takes an isPreservedFilter option that keeps matching filters through facet changes. Only the search page passes it; other callers behave as before, and a test pins that. "Clear all" still removes them. Two guards keep the workflow out of broken states. A new include replaces the previous one, because two includes AND together and match nothing. A source change drops pattern filters through the existing "filters didn't apply" notice, because the condition names the old source's body expression. Part of FR-385. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 26c9359 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 |
PR Review
3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor 2 posted as inline comment(s) on the changed lines. 1 listed below. Findings outside the changed lines1 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep ReviewReport-only multi-agent review of the pattern filter row-actions feature. 12 reviewers ran; findings below are merged, deduplicated, and re-graded against a ship-blocker bar. The change is clean for its surface area — all seven prior-review items are confirmed addressed, and security, performance, type-safety, and project-standards passes found nothing blocking. ✅ No critical issues found. No P0/P1: the state-timing concerns raised do not have a human-reachable reproduction (three reviewers independently confirmed React's passive-effect ordering protects normal click-to-click interactions), and no SQL-injection, crash, or data-loss path was constructable. 🟡 P2 — recommended
🔵 P3 nitpicks (5)
Pre-existing (not counted toward verdict): Reviewers (12): correctness, testing, maintainability, project-standards, agent-native, learnings, kieran-typescript, julik-frontend-races, security, performance, adversarial, previous-comments. 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 |
…-pattern-filter-actions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Pattern filters strip ANSI CSI sequences (colors) before matching, since the miner strips ANSI codes before mining. Colored logs now match their pattern. - Match Python Drain3's whitespace, which the miner splits on. - Pattern pills count toward the eight visible pills, after the facet pills. - Read the template from the row instead of a lookup map; a pattern without a template gets no buttons. - Build the delimiter list from the shared ASCII_PUNCTUATION. - Assert the full filter for both row buttons. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review findings outside the inline threads:
|
| isPreservedFilter: isPatternFilter, | ||
| }); | ||
|
|
||
| const patternFilters = useMemo( |
There was a problem hiding this comment.
🔵 minor — Two separate derivations of the pattern-filter set
useSearchPageFilterState already splits pattern filters out of filters through isPreservedFilter (searchFilters.tsx:129). DBSearchPage then parses filters again for the pills and adds its own remove/add handlers inline. Have the hook return the preserved filters and a remover, or move this into a usePatternFilters hook in patternFilter.ts, so the pills and the facet-preservation logic use one source.
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.
searchFilters.tsx stays pattern-agnostic on purpose: it only knows isPreservedFilter, and the page derives the pills from the same filters the hook reads, so the two always agree.
…-pattern-filter-actions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A value expression with quotes reads back. - A template with punctuation inside a token gets no filter; the miner's templates never have one. - Flips both ways, and one include across columns. - Say which filters isPreservedFilter may match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Second-round findings outside the inline threads:
That's the second round of automated findings, so per AGENTS.md I'm leaving any further ones for review. |
| valueExpression: string; | ||
| onFilter: (filter: ConditionFilter, action: PatternFilterAction) => void; | ||
| }) { | ||
| const filters = useMemo( |
There was a problem hiding this comment.
🔵 minor — Each row builds the same compact predicate twice, and the exclude == null check can never fail on its own
buildPatternFilter returns null only when buildCompactPatternPredicate returns null, and include and exclude are built from that same predicate. Build the include filter once and derive the exclude from it, for example with an invertPatternFilter helper in patternFilter.ts that adds the NOT prefix. Then drop the separate exclude == null check. This halves the regex work on every pattern row mount.
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
| isPreservedFilter: isPatternFilter, | ||
| }); | ||
|
|
||
| const patternFilters = useMemo( |
There was a problem hiding this comment.
🔵 minor — Pattern-filter state (parse list, add, remove) is hand-written inside DBSearchPage, which also tracks the same filters a second way
useSearchPageFilterState already collects the same set through isPreservedFilter into preservedFiltersRef, so these filters now have two copies that must stay in sync. Move patternFilters, addPatternFilter and removePatternFilter into a usePatternFilters(filters, handleSetFilters) hook in components/Patterns/patternFilter.ts, or expose the preserved filters from useSearchPageFilterState. That keeps pattern-filter handling in one module instead of the multi-thousand-line page.
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
Adds Filter for this pattern and Exclude this pattern to each row of Event Patterns. Each adds a search filter that ClickHouse evaluates over the whole time range, not only the 10,000 sampled rows, so the results table, histogram and counts follow it. Part of FR-385.
Stack
Part 4 of 4. Each PR is based on the one before it, so its diff shows only its own step. Review in order:
Please merge in order: once a PR merges, the next one is retargeted to
main. CI and the tier label only run on PRs whose base ismain, so PRs 2 to 4 get them at that point. Until then the test plan lists what I ran locally.Summary
IconFilter,IconFilterX). Filter for this pattern switches to the results table. Exclude this pattern stays on Event Patterns, which mines again without it.{ type: 'sql' }condition built with the compact predicate from feat(common-utils): add a compact form of the pattern predicate #3664. It strips ANSI color codes first and splits on the same delimiters and whitespace as the browser miner (Python Drain3), so it selects the events the pattern describes, colored logs included.=to include,!=to exclude), within the same eight visible pills. A pill's ✕ removes it, and Clear all removes them with the rest.Filters are extended, not changed
A sidebar facet change rebuilds the filter list from facet state, which drops every SQL filter the facets can't represent.
useSearchPageFilterStatetakes a new optionalisPreservedFilter, and filters it matches are appended back after a facet update. Only the search page passes it, so other callers behave as before; a test pins that without the option a facet update still drops other SQL filters. With no pattern filters, every filter path emits what it does onmain.Guards
retainFiltersByColumns, so the existing "N filters didn't apply to this source" notice counts them.Known limitations
2026 04 17T16in the screenshots), so the filter misses events of the same shape from other hours.<*> <*>match more events than the row suggests.Test plan
searchFilters.test.ts(preserved filters survive a facet update; clear all and a source change drop them; without the option a facet update still drops other SQL filters),ActiveFilterPills.test.tsx(a pattern pill renders and its ✕ calls the handler; pattern pills count toward the eight visible pills),patternFilter.test.ts(build and parse, including a quoted value expression; run on bodies the way ClickHouse does, including color codes and Python-only whitespace; only filters it built read back; no filter for a template with punctuation inside a token; flips both ways, one include across columns, excludes stack),PatternRowActions.test.tsx(both buttons build the full filter; none for a template too long to filter on)event-patterns.spec.ts"pattern filters", 3 tests: an exclude from a row survives a facet change; Filter for this pattern opens the results table with aPattern =pill; a source change drops the filter with the notice. CI runs them once this PR targetsmain. I replayed the three flows with the same selectors on a dev stack against the OpenTelemetry demo logs:SeverityText = infokeeps the exclusion, and a second exclusion stacks (244,332)ServiceNamevalues and two exclusions show eight pills and "+1 more", which expands to both pattern pills. In ClickHouse, a colored body fits its template only with the ANSI step, and 8 real exclusions leave the same 177,130 events in about the same time with or without ittsc --noEmit; eslint with no warnings on added lines; the 74 app suites related to the changed files: 71 pass (1,390 tests), and the other 3 (SearchWhereInput,DBSearchPageAlertModal,DBEditTimeChartForm) fail the same 43 tests onmainin my environment (TypeErrorreadingaddEventListenerwhile rendering Mantine notifications); knip;yarn dupesPatternRowActionshas no story: it is two existingDBRowTableIconButtons inside the table's hover strip, covered by a unit test and the e2e tests above.Screenshots
Row buttons on hover, light and dark:
Two exclusions next to a facet filter, with the list mined again without them, light and dark:
After Filter for this pattern on top of those, light: