Conversation
|
@asm0dey is attempting to deploy a commit to the Umami Software Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds glob-style wildcard operators across filters, property filters, funnels, and goals, while preserving arbitrary combobox input that is absent from lookup results.
Confidence Score: 4/5The wildcard implementation is functionally coherent, but the missing non-English catalog entries should be fixed before merging because localized users will see broken operator labels. The new labels are resolved through whichever locale catalog is active, while only the English catalog defines them, causing visible missing-key fallback text in every other supported locale. Files Needing Attention: src/components/messages.ts and public/intl/messages/*.json
|
| Filename | Overview |
|---|---|
| src/lib/wildcard.ts | Adds the shared glob-to-LIKE conversion and escapes user-supplied LIKE metacharacters before translating glob characters. |
| src/lib/prisma.ts | Adds wildcard parameter conversion and case-insensitive matching throughout PostgreSQL column and property-filter builders. |
| src/lib/clickhouse.ts | Mirrors wildcard support across ClickHouse column and property-filter builders using bound query parameters. |
| src/queries/sql/reports/getFunnel.ts | Extends both database branches to recognize wildcards anywhere in step values and adds wildcard step-filter operators. |
| src/queries/sql/reports/getGoal.ts | Replaces endpoint-only asterisk handling with shared wildcard detection and conversion in both database branches. |
| src/components/input/LookupField.tsx | Synchronizes the combobox selected value with arbitrary typed input so unmatched values survive popup closure. |
| src/components/messages.ts | Adds wildcard label keys, but matching entries were added only to en-US.json and are missing from every other locale catalog. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
UI[Filter or funnel UI] --> Params[wc / nwc serialized parameters]
Params --> Schema[Schema and parameter parsing]
Schema --> Glob[wildcardToLikePattern]
Glob --> Router{Configured event backend}
Router --> PG[PostgreSQL ILIKE / LIKE]
Router --> CH[ClickHouse ILIKE / LIKE]
PG --> Results[Filtered analytics results]
CH --> Results
Reviews (1): Last reviewed commit: "test(filters): cover live operators arra..." | Re-trigger Greptile
| matches: 'label.matches', | ||
| doesNotMatch: 'label.does-not-match', |
There was a problem hiding this comment.
Wildcard labels missing translations
When a user selects either wildcard operator in any locale other than en-US, the active locale catalog lacks the new message keys, causing missing-key fallback text to appear instead of translated operator labels.
Knowledge Base Used: Frontend App Shell
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!
|
On the flagged i18n item — that was deliberate, but I'm happy to go either way. The two new keys ( The mechanism Greptile describes is real, though — I'd rather not machine-translate "Matches wildcard" / "Does not match wildcard" into 51 languages and put them in front of your translators as a fait accompli — "wildcard" is a term of art and I'd be guessing at idiomatic phrasing for a good number of those locales. But I'm glad to do whichever you prefer:
Just say which and I'll turn it around. |
|
A reviewer question came up about whether wildcard filters need an index at long date ranges, so I measured it instead of guessing. No change to this PR is proposed — posting in case the numbers are useful. Methodology
1. Wildcards don't add a new cost classAt a 365-day range every shape sequentially scans, including plain equality:
The new anchored wildcard is ~2.4× cheaper than the 2. There may be a worthwhile index — but for existing queries, not this PR
This helps existing funnel and goal queries too — the shape is "specific page, long or fragmented time range." 3. What it costs on writesIndex size: 201 MB for 12.4M rows (~17 bytes/row) at 2.4M distinct paths. Insert throughput, 300k-row batches, logged table,
≈ +52% insert time (per-run range +31% to +72%, high variance). On your hottest write path that is a real trade, and it is your call whether the read win justifies it. Caveats
Happy to open a separate migration PR for the index, or to leave it entirely — you have production numbers I don't. |
26e236c fixed this for LookupField, which covers the funnel and goal step value fields. The funnel step-filter editor uses ComboBox directly and had the same defect: the field value was passed only as inputValue, never as the selection, so Base UI reset the input to '' on popup close whenever the typed text matched no item. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds "matches" / "does not match" (wc / nwc) operators to the filter bar, property filters and the funnel step-filter editor, so a wildcard value such as /blog/* can be used wherever a filter is accepted. A leading and/or trailing * maps to a LIKE %, matching the placement funnels and goals already supported. Both now share one helper in src/lib/wildcard.ts, which additionally escapes LIKE metacharacters in the rest of the value. That escaping is what the previous inline conversions were missing: the tracker stores url_path and url_query percent-encoded, so a stored /blog/hello%20world was matched by the unrelated /blog/hellozzz20world, and _ -- a plain URL character -- stood in for any single character. Values reach SQL only through bound parameters; the helper transforms the value, never the SQL text. Backslash is the default LIKE escape character in both PostgreSQL and ClickHouse, so no ESCAPE clause is needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e492b41 to
f9b2d59
Compare
Support wildcards in filters and funnel/goal steps
Fixes #4500
Problem
Two separate defects, both surfaced by the same use case in #4500 — building a funnel from
/blog/*to afile-downloadevent.1. Typed values that match no lookup item are silently discarded — still true in the funnel step-filter editor.
Base UI's
Combobox.Rootresyncs the input to the selected item's label when the popup closes in single-select mode with the input outside the popup:Nothing is ever selected when the typed text matches no item, so
stringValis''and the typed value is wiped before submit.26e236c solved this for
LookupFieldby prepending the typed value toitemsbehindallowCustomValue, which covers the funnel and goal step value fields.ValueSelectinEventDataFilterRow.tsx— the funnel step-filter editor — usesComboBoxdirectly and still has the defect, so a step filter value absent from the lookup data cannot be entered. This PR passesvaluealongsideinputValuethere, giving Base UI the same string to write back on close.Which combobox is fixed where:
FunnelEditForm / GoalEditForm -- step VALUE <LookupField allowCustomValue> -- fixed on dev by 26e236c9 <ComboBox items={[value, ...items]}> EventDataFilterRow -> ValueSelect -- step FILTER value - <ComboBox items={values} inputValue={value}> -- typed value wiped on close + <ComboBox items={values} value={value} inputValue={value}> CohortEditForm -- not covered by either; see Notes <LookupField>2. Wildcards escaped nothing, and were unavailable outside funnels and goals.
getFunnel.tsandgetGoal.tsrecognised a leading/trailing*viastartsWith('*') || endsWith('*')and mapped it to%withvalue.replace(/^\*|\*$/g, '%')— but neither escaped SQLLIKEmetacharacters. The tracker storesnew URL(raw, location.href).toString(), sourl_pathandurl_queryroutinely hold percent-encoded sequences:/blog/hello%20world,?q=caf%C3%A9. Unescaped,%20means "any run of characters, then20", so/blog/hello%20worldmatched/blog/hellozzz20world._— a plain legal URL character, common in slugs — matched any single character. Wildcards were also unavailable entirely in the filter bar, segments, cohorts, and property filters.What changed
Combobox fix (
ValueSelectonly)ValueSelectnow passesvaluealongsideinputValue, so Base UI's close-sync writes back the same string instead of''.New
matches/doesNotMatchoperators (wc/nwc)src/lib/wildcard.ts— one shared helper converting a value to aLIKEpattern, keeping the existing leading/trailing-*semantics: a*at the start and/or end becomes%, and everything between is escaped (\,%,_) so it matches literally. A*in the middle of a value stays a literal*./blog/*like/blog/%/blog/post-1*/thankslike%/thanks/checkout/thanks/blog/*/comments=/blog/*/comments*/sale/100%*like/sale/100\%%/sale/100%-off, not/sale/100zzz/a_b=/a_b/a_b, not/axbThe last two rows are the escaping fix: on
devthose%and_acted asLIKEwildcards.Wired through
OPERATORS, the zod schemas,isSearchOperator, and both query builders (prisma.ts,clickhouse.ts), including all three property-filter builders in each.Exposed in the filter bar, property filters, and the funnel step-filter editor.
Funnels and goals
LIKEmetacharacters in the value are escaped instead of acting as wildcards. Wildcard placement is unchanged: leading and/or trailing*only.Values reach SQL only through bound parameters (
{{param}}/{param:Type}); the helper transforms the value, never the SQL text. Backslash is the defaultLIKEescape character in both PostgreSQL and ClickHouse, so noESCAPEclause is needed.Case sensitivity follows each site's existing
containsbehaviour rather than introducing a new convention: filter and property-filter sites useilike/ILIKE; funnel and goal step values keep case-sensitivelikeon both engines, so existing funnels' numbers do not move.How to test
The
/blog/*step value from #4500 now works ondevalone, via 26e236c. What this PR adds on top:Step filters (the remaining combobox gap)
Triggered event, valuefile-download. Add a step filter: propertyfile, operatorMatches wildcard, valueliberica-*.liberica-*— ondevit goes blank.Wildcard operators in the filter bar
/blog/*; the URL becomes?path=wc.%2Fblog%2F*and survives a reload.*/thanks(leading wildcard), and/blog/*/comments— the middle*is a literal, so this matches only a path that really contains*./blog/hello%20worldunder the filter/blog/*— the%20is escaped and matches literally. Ondevthe funnel/goal equivalent also matched/blog/hellozzz20world.Automated:
pnpm exec vitest runBaseline on
dev(336fbcb3c) is 759 passing with 4 pre-existing collection failures (boards,Empty,SharePage,SessionProfile— avite:cssplugin error onChartAnnotationMarkers.module.css, present without this branch). With this branch: 796 passing, the same 4 pre-existing failures, no regressions.biome lintreports an identical 6 errors / 13 warnings / 11 infos before and after, andtsc --noEmitis clean on both.An earlier revision of this branch was additionally verified against a live instance (Postgres via
docker compose up -d db,pnpm dev, three seeded sessions hitting/blog/post-1..3thenfile-download, plus one/pricingsession as a control): the saved funnel reported 3 visitors → 3 visitors, 100%, with the/pricingsession correctly excluded. That run exercised theLookupFieldpath now owned by 26e236c; the query-layer behaviour it confirmed is unchanged here.Notes for reviewers
%or_— including any percent-encoded path — is now escaped instead of behaving as aLIKEwildcard. That is the intended semantics, but it will move numbers on affected definitions (generally by removing spurious matches).dev: leading and/or trailing*only. No mid-string globbing and no?single-character wildcard were added; the existing regex operator covers those cases.*at the very start or end of a step value still cannot be expressed — also unchanged fromdev.getGoal.tsis included because leaving it would make a goal and a funnel step disagree about what the same value means — they share the same input component.LookupFieldand were not givenallowCustomValuein 26e236c, so a custom cohort value is still discarded there. Out of scope for wildcards in filters #4500 and untouched here — flagging it in case you want it as a follow-up.LIKE-escaping behaviour is covered by unit tests asserting the generated SQL and bound parameters rather than by round-tripping against PostgreSQL or ClickHouse.AI Disclosure
This pull request was prepared with the assistance of Claude Code (Claude Opus 5). The root-cause analysis, implementation, and tests were AI-drafted across a task-by-task plan with independent review passes; I directed the work and reviewed the change before submitting it. The test, lint, and typecheck results reported above were produced by running those commands on this branch and on a pristine
devfor comparison.One coverage gap stated plainly: jsdom cannot drive Base UI's popup lifecycle, so the
ValueSelectchange is not covered by a unit test asserting the close-and-reset behaviour itself. This repo has Playwright e2e infrastructure undertests/e2e/; happy to add a spec there before merging if you'd prefer the check to run in CI.