refactor(taxonomic-filter): extract groupAnalyticsTaxonomicGroupsLogic#59817
Merged
pauldambra merged 2 commits intoMay 25, 2026
Merged
Conversation
Member
Author
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
|
Size Change: 0 B Total Size: 79.9 MB ℹ️ View Unchanged
|
This was referenced May 24, 2026
Member
Author
|
Note 🤖 Automated comment by QA Swarm — not written by a human 🏁 Verdict: ✅ APPROVEPure refactor, clean verbatim move of two selectors into Reviewers
Findings: nonePattern is sound and future-proof for the rest of the stack — subsequent PRs can mirror this shape (props/key/path/connect/selectors, no actions/reducers, single connect block) without further bikeshedding. Ship it 🚢 |
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
0f90824 to
3ea262c
Compare
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
frontend/src/lib/components/TaxonomicFilter/groupAnalyticsTaxonomicGroupsLogic.ts:1-79
**Extracted logic not yet wired in — dead code as committed**
The two selectors defined here (`groupAnalyticsTaxonomicGroupNames` and `groupAnalyticsTaxonomicGroups`) are exact copies of the selectors that still exist at lines 1574-1614 of `taxonomicFilterLogic.tsx`. The new file is not imported anywhere in the codebase, so `taxonomicFilterLogic.tsx` still resolves both selectors from its own inline definitions. Until `taxonomicFilterLogic.tsx` is updated to `connect` to this new logic and remove its own copies, both versions live side-by-side in violation of the OnceAndOnlyOnce principle — and a change to one will silently diverge from the other.
### Issue 2 of 3
frontend/src/lib/components/TaxonomicFilter/groupAnalyticsTaxonomicGroupsLogic.ts:47
Template literals used for plain string literals — backticks add no value here, making the code slightly harder to read at a glance.
```suggestion
getPopoverHeader: () => 'Group Names',
```
### Issue 3 of 3
frontend/src/lib/components/TaxonomicFilter/groupAnalyticsTaxonomicGroupsLogic.ts:73
Same unnecessary template literal for a plain string (`'Property'`). Consistent with the other `getPopoverHeader` fix above.
```suggestion
getPopoverHeader: () => 'Property',
```
Reviews (1): Last reviewed commit: "refactor(taxonomic-filter): extract grou..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
pauldambra
changed the base branch from
posthog-code/taxonomic-filter-exclude-stale-events
to
graphite-base/59817
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
3ea262c to
5831030
Compare
pauldambra
changed the base branch from
graphite-base/59817
to
posthog-code/taxonomic-filter-exclude-stale-events
Base automatically changed from
posthog-code/taxonomic-filter-exclude-stale-events
to
master
May 25, 2026 08:21
Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
5831030 to
fa63f64
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pure additive refactor extracting two kea selectors into a dedicated logic file. No existing files modified, no behavioral changes, no API contract or data model concerns. All inline comments are resolved, and the stacked-PR wiring concern was addressed by the author.
…arent The extraction commit added the new logic but forgot to connect it — the parent kept its own inline copies of the two selectors. Connect the new logic, remove the inline duplicates, and drop the now-unused groupsModel, groupDisplayId, Group, capitalizeFirstLetter, and toParams imports from the parent. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
pauldambra
deleted the
posthog-code/extract-group-analytics-taxonomic-logic
branch
pauldambra
added a commit
that referenced
this pull request
May 28, 2026
rorylshanks
pushed a commit
that referenced
this pull request
May 29, 2026
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
1 participant
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.
Problem
Changes
How did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Publish to changelog?
Docs update
🤖 Agent context