[alex-fedotyev]

Adds buildCompactPatternPredicate: the same match as buildPatternPredicate in much shorter SQL, for pattern filters that live in the URL (#3665). parsePatternPredicate reads both forms. Part of FR-385.

Stack

Part 3 of 4. Each PR is based on the one before it, so its diff shows only its own step. Review in order:

1. feat(common-utils): share the Event Patterns drill-down predicate #3662 Share the pattern predicate in common-utils
2. refactor(mcp): use the shared pattern predicate for drill-downs #3663 MCP uses the shared predicate
3. feat(common-utils): add a compact form of the pattern predicate #3664 Compact form of the predicate, short enough for a URL (this PR)
4. feat(app): filter for or exclude a pattern from Event Patterns #3665 Filter for or exclude a pattern from Event Patterns

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 is main, so PRs 2 to 4 get them at that point. Until then the test plan lists what I ran locally.

Summary

buildPatternPredicate repeats the separator class between every token. With all ASCII punctuation as delimiters that is about 30 characters per token, before SQL escaping and URL encoding. The compact form turns every separator run in the value into one space, then matches tokens joined by single spaces, so the class appears once. For GET api v1 users <*>:

-- buildPatternPredicate (276 characters)
match(Body, '^[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]*GET[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+api[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+v1[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+users[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+[^\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]*$')

-- buildCompactPatternPredicate (105 characters)
match(replaceRegexpAll(Body, '[\\s\\x{0B}\\pZ\\x{FEFF}[:punct:]]+', ' '), '^ ?GET api v1 users [^ ]+ ?$')

Why a second form

Measured on the OpenTelemetry demo logs, with 8 templates the app mined from 15 minutes of logs:

	buildPatternPredicate	buildCompactPatternPredicate
URL characters for the 8 exclusions	104,074	11,045
Events left after the 8 exclusions	173,588	173,588
Query time for the 8 exclusions	0.45 s	0.95 s

Each template also matched the same number of events with either form (15,198 to 30,056). Node's default header limit is 16 KB, so with the full form two exclusions of long patterns already break the page URL, while the compact form fits about ten. The price is about 2x on the exclusion filters. Keeping the compact form in the URL and querying with the full form would win that back; that is left for a follow-up.

Test plan

• unit: the compact predicate matches exactly what the full one matches for every mined template and a set of edge cases (leading and trailing separators, empty and whitespace-only values, a wildcard-only template, non-ASCII separators, and separators only one whitespace set has), under every option set: none, all punctuation, a partial delimiter set, and Python whitespace. The test evaluates it the way ClickHouse does: replaceRegexpAll, then match
• unit: the compact form is more than 4x shorter for a 40-token template; both forms round-trip through parsePatternPredicate under every option set; a hand-written condition is rejected both by the separator check and by the round trip
• ClickHouse: the counts and timings above
• yarn ci:lint and yarn ci:unit in packages/common-utils (47 suites, 3,160 tests), knip, yarn dupes (no new duplicate code)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 13, 2026 11:44pm UTC
hyperdx-storybook	Ready	Preview	Nov 13, 2026 11:44pm UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: efd34a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Minor
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

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]

🔵 minor — The inverse of escapeSqlString now exists in three places

unescapeSql is new here, a copy of it is pasted into __tests__/pattern-predicate.test.ts:49, and filters.ts:190 (getBooleanOrUnquotedString) already does the same unescape inline. Add an exported unescapeSqlString next to escapeSqlString in core/utils.ts and use it in all three places, so the escape and unescape rules live in one file.

_{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}

[alex-fedotyev]

Hoisting an unescapeSqlString next to escapeSqlString and switching filters.ts over would touch files outside this PR, so that's left for a separate cleanup.

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

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.}

[github-actions]

Deep Review

Scope: packages/common-utils — adds buildCompactPatternPredicate plus parseCompactPredicate, extends parsePatternPredicate to read both predicate forms, and adds tests. Diff base eda26c96c.
Intent: Provide a URL-sized equivalent of buildPatternPredicate by normalizing separator runs to single spaces so the separator class appears once; both forms must match exactly the same values. Part of FR-385 (part 3 of 4; no production caller lands in this diff).
Mode: report-only (read-only, no edits).

✅ No critical issues found. The equivalence logic is sound for all realistically-mined templates, SQL escaping is applied on every template-derived path, and the round-trip guard (compactRegex(template, options) === regex) robustly rejects hand-written predicates. The items below are a correctness-robustness gap behind unusual inputs and some test/maintainability hardening.

🟡 P2 -- recommended

• packages/common-utils/src/drain/pattern-predicate.ts:140 -- the splitsToken guard only rejects tokens holding an extraDelimiter, but under whitespace: 'python' the separator class also covers \x1C-\x1F and \x85 (NEL), which tokensOf's python splitter /[^\S\u{FEFF}]+/u does not split on, so such a char stays inside a literal token; compactRegex embeds it literally while replaceRegexpAll collapses it to a space in the value, making the compact form fail to match where the full form matches — silently, with no null returned.
  • Fix: Return null (or split tokensOf on the full separator class) when a non-wildcard token contains any character in separatorClass(options) that tokensOf does not split on, not just extraDelimiters.
  • _{correctness, adversarial, kieran-typescript, testing}
• packages/common-utils/src/drain/pattern-predicate.ts:138 -- buildCompactPatternPredicate has its own MAX_PREDICATE_TEMPLATE_LENGTH guard rather than delegating to buildPatternPredicate, and no test exercises it; the only length-limit test covers buildPatternPredicate.
  • Fix: Add assertions that buildCompactPatternPredicate returns non-null at the limit and null one character over it.
  • _testing

🔵 P3 nitpicks (3)

• packages/common-utils/src/__tests__/pattern-predicate.test.ts:36 -- the matchesCompact oracle hand-copies the production parseCompactPredicate structural regex and re-declares unescapeSql, so a change to the emitted SQL shape could leave the equivalence suite passing against a stale decoder.
  • Fix: Drive matchesCompact through a shared/exported decode helper instead of a duplicated regex so the oracle cannot drift from the parser.
• packages/common-utils/src/drain/pattern-predicate.ts:149 -- unescapeSql is defined byte-for-byte in both the module and the test file; its ''-then-\\ ordering matters, so the copies can diverge silently.
  • Fix: Export unescapeSql and import it in the test, or mark the test copy as an intentional independent oracle in a comment.
• packages/common-utils/src/drain/pattern-predicate.ts:158 -- parseCompactPredicate adds a second greedy-(.+) parser with no input-length cap, while the builders bound input at MAX_PREDICATE_TEMPLATE_LENGTH; since the compact form is intended to live in URLs, a future caller parsing untrusted predicate strings could incur O(n²) backtracking on large inputs.
  • Fix: Cap condition length in parsePatternPredicate/parseCompactPredicate before matching, or document that callers must bound untrusted input.

---

Reviewers (9): correctness, adversarial, security, testing, maintainability, kieran-typescript, api-contract, project-standards, previous-comments.

Testing gaps:

• Equivalence is proven only within TypeScript (shared tokensOf + the toJsRegex/replaceRegexpAll shims on both sides); nothing asserts fidelity to real ClickHouse RE2 or Drain3/Python tokenization.
• No template places a python-only separator (\x1C-\x1F/\x85) inside a literal token — these appear only at wildcard positions in test values, so the guard gap at line 140 is uncovered.
• Suppressed below confidence gate: one maintainability note (anchor 50) on the repeated (?:[^'\\]|\\\\|'')* SQL-literal subpattern across three parser regexes. Not re-graded.
• Pre-existing (not introduced here): valueExpression is interpolated into SQL unescaped, identical to buildPatternPredicate's documented column-expression-only contract — no new exposure.

_{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}

[alex-fedotyev]

Review findings outside the inline threads, fixed in e6804d4:

• The rejection test now includes a condition with the real separator class and a hand-written regex, which only the round trip rejects.
• The equivalence and round-trip tests run under every option set (none, all punctuation, a partial delimiter set, Python whitespace), with values that use non-ASCII separators and separators only one whitespace set has.
• compactRegex and parseCompactPredicate share the start, end and wildcard constants.
• The docstring says when the two forms agree (no template token contains a separator, which holds for templates mined with the same options) and that valueExpression goes into the SQL as is.

[github-actions]

🔵 minor — The inverse of escapeSqlString is now written out by hand in three places

unescapeSql here, its copy in pattern-predicate.test.ts:31, and the inline .replace(/\\\\/g, '\\').replace(/''/g, "'") in filters.ts:190 all undo escapeSqlString (core/utils.ts:147). Fix: export one unescapeSqlString next to escapeSqlString in core/utils.ts and import it in all three places. The pattern for a quoted SQL literal, '((?:[^'\\]|\\\\|'')*)', now appears three times in this file (lines 150 twice, 189) and could become one shared constant.

_{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}

[alex-fedotyev]

Same answer as the earlier thread: hoisting the helper means touching core/utils.ts and filters.ts, outside this PR, and building the two parser regexes from a shared string would make them harder to read than the literals.

[alex-fedotyev]

Second-round findings outside the inline threads:

• A token that holds a delimiter (P2): fixed in efd34a5. buildCompactPatternPredicate returns null for it, since the two forms would disagree.
• Escaping in the compact form (P2): fixed in efd34a5, see the inline thread.
• valueExpression from a URL (P2): the app never re-embeds a parsed valueExpression. It keeps the original condition, and a filter in the URL is a SQL condition the search page runs as is today, like any other sql filter.
• The tokenizer regex spelling out COMPACT_WILDCARD, and the test's own unescapeSql: left as is. The round-trip tests fail if the regex and the constant drift, and the test keeps its own copy so it checks the module's output independently.

That's the second round of automated findings, so per AGENTS.md I'm leaving any further ones for review.

[github-actions]

🔵 minor — unescapeSql is a third hand-written inverse of escapeSqlString, and the test file copies it again

Export an unescapeSqlString from packages/common-utils/src/core/utils.ts, next to escapeSqlString. Then use it here, in __tests__/pattern-predicate.test.ts:31, and in filters.ts:190 (getBooleanOrUnquotedString), which already does the same \\→\ and ''→' replacements inline.

_{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}