refactor(taxonomic-filter): extract cohortTaxonomicGroupsLogic#59818
Merged
Conversation
Member
Author
This was referenced May 24, 2026
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
|
Contributor
|
👋 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 was referenced May 24, 2026
Member
Author
|
Note 🤖 Automated comment by QA Swarm — not written by a human QA Swarm review — PR #59818Verdict: APPROVE Pure refactor, second in the by-data-type extraction stack. FindingsNone across all four perspectives. Scoreboard: 🔴 0 · 🟠 0 · 🟡 0 · 🟢 0 · ⚪ 0. Perspective notes
Things worth scrutiny (per task) — all clean
XP-side observation (non-blocking)The extraction pattern (data-type tab + its sole-consumer prop-mirror selector) is repeatable. Looking at the remaining parent, the Commit reviewed: |
pauldambra
force-pushed
the
posthog-code/extract-cohort-taxonomic-logic
branch
from
698cdd6 to
689e986
Compare
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
0f90824 to
3ea262c
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Purely additive refactor — new kea logic file extracting cohort taxonomic group definitions into a dedicated logic. The .tsx extension is correct (JSX rendered inline). No existing code is modified, no API contracts or data models are touched, and the stable-reference optimization is clearly documented.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 689e986ecc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
frontend/src/lib/components/TaxonomicFilter/cohortTaxonomicGroupsLogic.tsx:19-26
**`COHORTS_WITH_ALL_USERS_OPTIONS` now defined three times**
This constant (together with its 6-line rationale comment) already exists in `utils/buildTaxonomicGroups.tsx` (line 85) and in `taxonomicFilterLogic.tsx` (line 305). With this PR it is now defined a third time. If the sentinel value or its stable-reference reasoning ever needs to change, all three copies must be updated in sync. The constant is currently unexported in `buildTaxonomicGroups.tsx`; exporting it there and importing it here (and in `taxonomicFilterLogic.tsx`) would satisfy OnceAndOnlyOnce without any behaviour change.
### Issue 2 of 2
frontend/src/lib/components/TaxonomicFilter/cohortTaxonomicGroupsLogic.tsx:1-80
**New logic is not connected to any consumer**
`cohortTaxonomicGroupsLogic` is never imported or `connect`-ed from `taxonomicFilterLogic` or `buildTaxonomicGroups`. Both files still define the same cohort groups independently, so the `cohortTaxonomicGroups` selector produced here is currently dead code. If this is a deliberate stepping-stone commit in a larger series, a follow-up that wires it in (and removes the duplicate definitions) would complete the extraction; otherwise the original code and this new file diverge silently over time.
Reviews (1): Last reviewed commit: "refactor(taxonomic-filter): extract coho..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
pauldambra
force-pushed
the
posthog-code/extract-cohort-taxonomic-logic
branch
from
689e986 to
0c103a7
Compare
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
3ea262c to
5831030
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
Contributor
There was a problem hiding this comment.
Purely additive refactor — new kea logic file extracting cohort taxonomic group definitions. No existing code modified, no API contracts or data models touched, .tsx extension is correct for JSX content, and all resolved review comments were adequately explained by the author.
pauldambra
force-pushed
the
posthog-code/extract-group-analytics-taxonomic-logic
branch
from
5831030 to
fa63f64
Compare
pauldambra
force-pushed
the
posthog-code/extract-cohort-taxonomic-logic
branch
from
0c103a7 to
1280bf3
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
Contributor
There was a problem hiding this comment.
Purely additive new kea logic file with no modifications to existing code, no API/data-model changes, and all reviewer comments resolved. The .tsx extension correctly matches JSX content, and the missing type-file import is expected (kea-typegen outputs are gitignored).
Base automatically changed from
posthog-code/extract-group-analytics-taxonomic-logic
to
master
May 25, 2026 10:46
Third slice in the by-data-type split of `taxonomicFilterLogic` (PR 1 was the stale-events feature; PR 2 extracted the group analytics tabs). The two `Cohorts` tabs (one with `CohortsWithAllUsers`, one without) and their only consumer of the `hideBehavioralCohorts` selector were still inline in the parent's `taxonomicGroups` body. This was the smallest self-contained slice that actually frees an input dependency from the parent's giant selector — `hideBehavioralCohorts` was used only by the cohort tabs, so moving it goes with them. New `cohortTaxonomicGroupsLogic.tsx` (note `.tsx` because the cohort tab definitions render JSX in `getIcon` / `footerMessage`): - Owns the `hideBehavioralCohorts` selector (moved from the parent). - Owns a new `cohortTaxonomicGroups` selector returning the array of two cohort `TaxonomicFilterGroup` definitions. - Owns the `COHORTS_WITH_ALL_USERS_OPTIONS` stable-reference constant and its explanatory comment about the React 50-update limit (the reason for that comment hasn't changed; it just moves with the data). Parent `taxonomicFilterLogic`: - Connects to the new logic for `cohortTaxonomicGroups`. - Removes the two inline cohort tab definitions and replaces them with `...cohortTaxonomicGroups` so ordering is preserved. - Removes the local `hideBehavioralCohorts` selector and drops it from the `taxonomicGroups` selector input tuple (slot reused by the new `cohortTaxonomicGroups` connect value, so net input count stays at 16 — still tight, but no regression). - Drops now-unused imports: `IconCohort`, `Link`, `COHORT_BEHAVIORAL_LIMITATIONS_URL`, `CohortType`. - Net change: parent shrinks by 46 lines. No behaviour change. Both cohort tabs render in the same positions with the same props, including the `footerMessage` that warns when behavioral cohorts are filtered out. This PR was written by an agent. Automated tests run: - `hogli test frontend/src/lib/components/TaxonomicFilter/` — 362/362 pass. - `pnpm exec tsc --noEmit` filtered to touched files — clean. - `pnpm format` — clean. - Kea typegen produces `cohortTaxonomicGroupsLogicType.ts` and updates `taxonomicFilterLogicType.ts`. The agent did not manually click through the UI. Written by PostHog Code (Claude Opus 4.7). Strategy is the human's: split by data type, one type per PR, stack them so each is reviewable on its own and the risk per PR stays small. Cohorts picked next because it was the smallest cluster that also retires a parent selector input dependency — useful even though it doesn't push the parent below the 16-input cap on its own; that comes later in the stack once enough types have moved out that the monolithic selector body can collapse into a composition selector. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
pauldambra
force-pushed
the
posthog-code/extract-cohort-taxonomic-logic
branch
from
70755c3 to
65a07f6
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
Contributor
There was a problem hiding this comment.
Clean kea logic extraction — cohort group definitions moved to a dedicated file and properly connected via connect() in the parent logic. No behavioral changes, no API/data model touches, and all bot review concerns were addressed with valid explanations.
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