feat(customer-analytics): add custom property filters to the accounts list - #69506
Conversation
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/customer_analytics/frontend/CustomerAnalyticsScene.js |
124.9 KiB | 🔺 +4.3 KiB (+3.5%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.23 MiB · 22 files | 🔺 +42 B (+0.0%) | ███░░░░░░░ 28.6% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.18 MiB · 2,996 files | 🔺 +42 B (+0.0%) | █████████░ 88.4% of 9.25 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.3 KiB | ../node_modules/.pnpm/posthog-js@1.404.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 235.8 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 223.4 KiB | ../node_modules/.pnpm/posthog-js@1.404.1/node_modules/posthog-js/dist/module.js |
| 167.1 KiB | src/queries/validators.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.9 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 93.2 KiB | ../packages/quill/packages/quill/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Dist folder size — 🔺 +32.6 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1349.29 MiB · 🔺 +32.6 KiB (+0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
7e6007b to
31f0825
Compare
4d9de78 to
bd7943d
Compare
There was a problem hiding this comment.
Client-side HogQL predicate compilation touches injection-sensitive territory, but the author is the owning team member with STRONG familiarity (100% of touched lines, 42 merged PRs in this area) which satisfies the independent-assurance bar, the reviewer-flagged concerns (NULL handling on negation, boolean literal ambiguity, multi-value drop, unescaped identifier) were all fixed with pinned tests including an explicit injection-attempt test case, and no unresolved concerns or in-flight reviews remain.
- Author wrote 100% of the modified lines and has 42 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
- Greptile flagged that the NotRegex operator lacks direct test coverage — minor gap, not a blocker.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 382L, 6F substantive, 621L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (621L, 9F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ d13e77d · reviewed head bd7943d |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
0d11d28 to
15a2260
Compare
…p and filter type Generated-By: PostHog Code Task-Id: 3faf9d95-024e-4878-bd8b-57e2e9e46724
…tion Generated-By: PostHog Code Task-Id: d1cd8690-a9f6-4071-96ff-179eb665dee8
… list Generated-By: PostHog Code Task-Id: 3faf9d95-024e-4878-bd8b-57e2e9e46724
Address review findings on the accounts custom property filter compiler:
- Wrap negative operators (is not / not contains / not regex) in
ifNull(..., true) so accounts where the property is unset (the lazy
join yields NULL) stay in the result, matching the canonical
property-filter semantics in posthog/hogql/property.py.
- Compile boolean equality against both string renderings of the stored
bool ('true'/'false' and '1'/'0'), the same hedge the boolean cell
renderer already uses, so the filter matches regardless of how the
federated engine stringifies the column.
- Support multiple values for contains/regex operators ("matches any",
negated "matches none") instead of silently dropping all but the
first value.
- Quote the column key via the shared escapePropertyAsHogQLIdentifier
instead of hand-rolled backticks (the UUID guard stays as the primary
validation).
- Dedupe the definition maps in accountsColumnConfigLogic: derive
aliasToDefinition from customPropertyDefinitionsById and seed
propertyDefinitionsModel from customPropertyTaxonomicOptions so the
display-type mapping is derived once.
All compiled expression shapes verified against the HogQL parser.
Generated-By: PostHog Code
Task-Id: 1f0c5fc4-8c89-4998-b535-e13719c171b3
Generated-By: PostHog Code Task-Id: ed0a382a-932d-4101-b251-7b5474388cab
bd7943d to
8604731
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Client-side HogQL predicate builder is properly guarded (UUID-only keys, shared escapePropertyAsHogQLIdentifier/escapeHogQLString, injection cases covered by tests) and all substantive review findings (NULL/ifNull semantics, boolean representation, multi-value operators, identifier escaping) are visibly fixed in the diff; author is on the owning team, giving independent assurance for this query-building surface, and there's no maintainer hold or in-flight (👀) review.
- Author wrote 63% of the modified lines and has 44 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
- Minor: greptile-apps[bot] noted NotRegex has no dedicated test case (P2, cosmetic gap) — not blocking.
- Minor: a small formatting artifact in accountsLogic.ts's viewQuery selector params (stray leading comma on its own line) suggests lint/format wasn't rerun after the last edit — cosmetic, CI/preflight will catch if it matters.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 383L, 6F substantive, 678L/13F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (678L, 13F, two-areas, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 6ef8ccd · reviewed head 8604731 |
|
Retaining stamphog approval — delta since last review classified as |
… import Generated-By: PostHog Code Task-Id: ed0a382a-932d-4101-b251-7b5474388cab
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Frontend-only feature in the author's own product area with STRONG familiarity; the HogQL predicate-building path (the one security-sensitive part) uses established escaping utilities and validated UUID keys, with dedicated injection/escaping tests, and all substantive self-review findings were fixed in a later commit. Greptile's only outstanding note is a minor missing test case, not a correctness or safety concern.
- Author wrote 98% of the modified lines and has 43 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
- Minor: Greptile flagged that the NotRegex operator lacks explicit test coverage in accountsCustomPropertyFilters.test.ts — worth a follow-up test but not blocking.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 380L, 6F substantive, 619L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (619L, 9F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 6ef8ccd · reviewed head 1dc7a0b |
Generated-By: PostHog Code Task-Id: ed0a382a-932d-4101-b251-7b5474388cab
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Frontend-only feature by an owning-team author with STRONG familiarity; the injection-adjacent HogQL predicate builder uses proper string/identifier escaping and UUID-key validation, and every substantive self-review concern (NULL-handling on negated operators, boolean representation, multi-value operators, identifier escaping) was fixed with dedicated parameterized tests, with no unresolved comments or in-flight reviews.
- Author wrote 91% of the modified lines and has 43 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 417L, 6F substantive, 656L/9F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (656L, 9F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ dfcb7cb · reviewed head d7f8b9f |
Problem
Part 3/3 of custom property filters for the accounts list (split from #69415, stacked on #69504; value suggestions come from #69500). Teams can define custom account properties but can't filter the accounts list by them.
Changes
PropertyFilterscomponent, restricted to the newAccountCustomPropertiesgroup (rendered only when the team has definitions)propertyDefinitionsModel, dropdown narrowed to exactly the operators the expression builder can compilecustom_propertieslazy join (numeric viatoFloatOrNull, dates viaparseDateTimeBestEffort, select equality on the stored label), ANDed into the existingfilterExpressionso list rows and overview tiles stay consistent#view=URL hash, keyed by definition id so they survive renames; filters for deleted definitions compile to nothingcustom_propertycase to the filter-changed analytics event (shape only, no values)How did you test this code?
Unit tests: expression builder (escaping/injection, per-type casts, incomplete filters degrade to no-op), view-state round-trip, and the full Accounts jest suite — 159 passing. The compiled predicate shapes are pinned backend-side by the typed round-trip runner tests in #69500.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
No
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude Code (PostHog Code); split out of #69415 by layer, includes fixes from dev-server testing (boolean/date filters). Skills invoked: /writing-kea-logics, /writing-tests. Notable decision: filters compile client-side onto the pre-existing
filterExpressionseam instead of adding apropertiesfield toAccountsQuery— the lazy join expands from a WHERE-only reference, and tile/deep-link filters already use the same hook.Created with PostHog Code