refactor(taxonomic-filter): extract replayTaxonomicGroupsLogic#59825
Conversation
|
🎭 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. |
|
Size Change: 0 B Total Size: 80 MB ℹ️ View Unchanged
|
|
👋 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. |
55777d1 to
cef85a3
Compare
0214205 to
1eaff23
Compare
There was a problem hiding this comment.
Purely additive new file extracting replay-related taxonomic groups into a dedicated kea logic. The file content is valid TypeScript matching its extension, follows established kea patterns correctly, and introduces no behavioral changes or API contract modifications.
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
frontend/src/lib/components/TaxonomicFilter/replayTaxonomicGroupsLogic.ts:18-96
**Extraction is incomplete — logic still duplicated in the parent**
`replayTaxonomicGroupsLogic` is never imported or connected anywhere in the codebase; `taxonomicFilterLogic.tsx` still carries the identical inline definitions at lines 1365–1423 and has not been updated to `connect()` this new logic. The same pattern appears in all nine prior commits in this stack (none of the extracted logics are wired into the parent yet), so this is likely intentional — but as it stands the new file is dead code and violates OnceAndOnlyOnce. If a follow-up assembly PR is planned, that intent is worth noting somewhere so reviewers and CI can track it.
Reviews (1): Last reviewed commit: "refactor(taxonomic-filter): extract repl..." | Re-trigger Greptile |
83d7e73 to
81397eb
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure mechanical extraction with no behavioral change: replay group definitions move verbatim into a new kea logic file following the established sibling pattern, and the parent logic correctly connects and spreads the selector in the same position.
5aec6a2 to
6039f4e
Compare
81397eb to
8895b8c
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure mechanical extraction with no behavioral change: replay group definitions move verbatim into a new kea logic file following the established sibling pattern, and the parent logic correctly connects and spreads the selector in the same position.
6039f4e to
baa9377
Compare
8895b8c to
ecf0fc1
Compare
ecf0fc1 to
c143a19
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure mechanical refactor: replay group definitions extracted verbatim into a new kea logic file following the established sibling pattern, and the parent logic correctly imports, connects, and spreads the selector in the same position. No behavioral changes or showstoppers.
c143a19 to
8bc2be2
Compare
Tenth slice. The `Replay` tab (recording / log-entry property keys) and the `Saved filters` tab (`session_recording_playlists` endpoint rendered via `SavedFiltersTaxonomicGroup`) are both session-replay concerns. Group them. New `replayTaxonomicGroupsLogic.ts`: - Connects to `teamLogic` (for `currentTeam`) and `projectLogic` (for `currentProjectId`). - Owns a new `replayTaxonomicGroups` selector returning the two-element array (Replay + Saved filters). Parent `taxonomicFilterLogic`: - Connects to the new logic for `replayTaxonomicGroups`. - Removes both inline tabs, replaces with the spread. - Drops now-unused `SavedFiltersTaxonomicGroup`, `SessionRecordingPlaylistType`, and `getFilterLabel` imports. No behaviour change. Agent-written. Tests: - `hogli test frontend/src/lib/components/TaxonomicFilter/` — 362/362 pass. - TS check on touched files — clean. Continuing the by-data-type stack. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
8bc2be2 to
533f0a8
Compare
Merge activity
|
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure mechanical refactor: replay group definitions extracted verbatim into a new kea logic file following the established sibling pattern. The parent logic correctly imports, connects, and spreads the selector in the same position. The removed primaryPropertiesForContextEvents was declared but never consumed in the taxonomicGroups body, so its removal is safe.
…16-tuple limit Adding replayTaxonomicGroups would have made taxonomicGroups's input tuple 17 wide, blowing past kea's SelectorTuple type's max of 16. Combine the two groupAnalyticsTaxonomic* selectors into a single groupAnalyticsCombined wrapper that returns the concatenated array — the parent already spreads both back-to-back, so the order is preserved. Also drop eventNamesWithPrimaryProperties (no longer needed after the distinct-primary-properties util landed; the parent only ever used eventNames from it) and the now-unused primaryProperties / getDistinctPrimaryPropertiesForEvents references. Net: -2 inputs, -1 selector, -1 import, restores the type tuple to 16 entries with room for replay. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
533f0a8 to
a148547
Compare
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
There was a problem hiding this comment.
Pure mechanical extraction of replay taxonomic group definitions into a dedicated kea logic file following the established sibling pattern. The parent correctly connects and spreads the new selector in the same position, and the removed primaryPropertiesForContextEvents was unused in the selector body. No behavioral changes or showstoppers.

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