revert(taxonomic-filter): undo extraction stack (#59817-#59826)#60485
Merged
pauldambra merged 10 commits intoMay 28, 2026
Conversation
Contributor
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/taxonomicFilterLogic.tsx:565-583
**Reverted state already at the 16-input ceiling**
The new `taxonomicGroups` selector has exactly 16 inputs — kea's `SelectorTuple` maximum — and still relies on the same "combine two values into one selector" workaround (`eventNamesWithPrimaryProperties` packs `eventNames` + `primaryPropertiesForContextEvents`; `hogQLExpressionComponentProps` packs two props). Adding any new inline tab group immediately re-hits the limit. The PR description calls this pattern a band-aid that doesn't scale, but the reverted state still depends on it to stay under 16.
Reviews (1): Last reviewed commit: "Revert "refactor(taxonomic-filter): extr..." | Re-trigger Greptile |
Contributor
|
Size Change: -19.3 kB (-0.02%) Total Size: 80.6 MB 📦 View Changed
ℹ️ View Unchanged
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is a large revert (1556 lines, 13 files) of a TaxonomicFilter extraction that exceeded the automated size gate. The only review is from a bot (greptile), which the author acknowledged. No human reviewer has approved this change. A revert of this scope touching core filter logic needs a human team review before auto-approval.
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Member
Author
|
Note 🤖 Automated comment by PR Shepherd — not written by a human Stamphog declined to auto-approve citing the size gate (1556 lines, 13 files) and asked for human review of a revert touching core taxonomic-filter logic. Tagging @adamleithp for review. Context: this PR undoes the 10 merged extraction PRs (#59817–#59826). The extraction approach hit kea's 16-tuple |
pauldambra
added a commit
that referenced
this pull request
May 29, 2026
## Problem Three small pre-existing issues in `taxonomicFilterLogic.tsx` (and one consumer) that were surfaced during the now-reverted extraction effort (#60485). Each one stands on its own without the extraction context. ## Changes 1. **`localItemsSearch` for the Feature Flags tab** — dropped a defensive `'key' in item && 'name' in item` type guard and a `return true` fallback. The group only ever receives `FeatureFlagType` items so both branches were unreachable; the simplified filter agrees with `getName` two lines above. 2. **Recent / Pinned tab definitions extracted to a shared const** — `taxonomicFilterLogic.tsx` and `utils/buildTaxonomicGroups.tsx` each had identical ~13-line object literals for the two tabs. Both now spread `RECENT_PINNED_TAB_DEFINITIONS` from the new `recentPinnedTabDefinitions.ts`. Typed `as const satisfies readonly TaxonomicFilterGroup[]` so a future caller can't mutate one consumer's view through the shared reference. 3. **revenueAnalytics `getIcon`** — `getIcon: (option) => getRevenueAnalyticsDefinitionIcon(option)` collapsed to `getIcon: getRevenueAnalyticsDefinitionIcon`. Identical signature, no behaviour change, stable module-level reference instead of a fresh arrow per recompute. No behaviour change in any of the three. ## How did you test this code? I'm an agent (PostHog Code / Claude Opus). Ran `hogli test frontend/src/lib/components/TaxonomicFilter/` against this branch — all 16 test suites / 362 tests pass. Did not click through the picker in a browser. ## Publish to changelog? no ## 🤖 Agent context These three cleanups originated in #59848 (closed) while it sat on top of the abandoned extraction stack. After #60485 reverted the stack, each fix was re-verified to apply to the pre-extraction code on master and lifted to its own PR. No new design decisions — each change is mechanical and behaviour-preserving. --- *Created with [PostHog Code](https://posthog.com/code?ref=pr)*
rorylshanks
pushed a commit
that referenced
this pull request
May 29, 2026
rorylshanks
pushed a commit
that referenced
this pull request
May 29, 2026
## Problem Three small pre-existing issues in `taxonomicFilterLogic.tsx` (and one consumer) that were surfaced during the now-reverted extraction effort (#60485). Each one stands on its own without the extraction context. ## Changes 1. **`localItemsSearch` for the Feature Flags tab** — dropped a defensive `'key' in item && 'name' in item` type guard and a `return true` fallback. The group only ever receives `FeatureFlagType` items so both branches were unreachable; the simplified filter agrees with `getName` two lines above. 2. **Recent / Pinned tab definitions extracted to a shared const** — `taxonomicFilterLogic.tsx` and `utils/buildTaxonomicGroups.tsx` each had identical ~13-line object literals for the two tabs. Both now spread `RECENT_PINNED_TAB_DEFINITIONS` from the new `recentPinnedTabDefinitions.ts`. Typed `as const satisfies readonly TaxonomicFilterGroup[]` so a future caller can't mutate one consumer's view through the shared reference. 3. **revenueAnalytics `getIcon`** — `getIcon: (option) => getRevenueAnalyticsDefinitionIcon(option)` collapsed to `getIcon: getRevenueAnalyticsDefinitionIcon`. Identical signature, no behaviour change, stable module-level reference instead of a fresh arrow per recompute. No behaviour change in any of the three. ## How did you test this code? I'm an agent (PostHog Code / Claude Opus). Ran `hogli test frontend/src/lib/components/TaxonomicFilter/` against this branch — all 16 test suites / 362 tests pass. Did not click through the picker in a browser. ## Publish to changelog? no ## 🤖 Agent context These three cleanups originated in #59848 (closed) while it sat on top of the abandoned extraction stack. After #60485 reverted the stack, each fix was re-verified to apply to the pre-extraction code on master and lifted to its own PR. No new design decisions — each change is mechanical and behaviour-preserving. --- *Created with [PostHog Code](https://posthog.com/code?ref=pr)*
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
2 participants
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
The taxonomic-filter extraction stack (10 PRs, #59817 through #59826) split sub-logics out of
taxonomicFilterLogic.tsxto shrink the file and make the picker easier to maintain. The structural shape of the refactor turned out to be wrong: every new sub-logic adds one input to the parent'staxonomicGroupsselector, and kea'sSelectorTupletype maxes out at 16 entries. We hit that limit repeatedly during the stack, kept patching it with one-shot "combine two adjacent selectors into a wrapper" PRs, and each subsequent extraction immediately re-hit the same wall.The combining workaround does not scale — every future tab extraction will run into it again. The stack needs a different approach (a registry / composition pattern, or moving the orchestration out of kea) before any of these extractions are useful.
Changes
Reverts the 10 merged extraction PRs in reverse-chronological order:
Each revert is its own commit so a future redo can pick the ones that still apply individually. After this lands,
taxonomicFilterLogic.tsxis back to the state right after #59816 (stale-events) merged — all tab definitions are inline intaxonomicGroupsagain. The 10 follow-up PRs (#59827–#59848) have been closed; nothing else is open against this stack.The remaining open work is unrelated:
How did you test this code?
I'm an agent (PostHog Code / Claude Opus). I ran
hogli test frontend/src/lib/components/TaxonomicFilter/against the reverted branch — all 16 test suites / 362 tests pass. I did not click through the picker in a browser.Publish to changelog?
no
🤖 Agent context
The extraction approach was tried over a long session and produced a stack of 19 PRs. The first 10 merged cleanly when reviewed individually, but every PR above #59826 hit kea's 16-tuple type limit on
taxonomicGroups's inputs. The session repeatedly applied a "wrap two adjacent selectors into one combining selector" workaround (`allGroupAnalyticsTaxonomicGroups`, `allMetaTaxonomicGroups`) — each one freed exactly one slot and the next extraction immediately re-hit the limit. The author's call was that combining is a band-aid that does not scale, and that a structural alternative (composition selectors at multiple levels, a registry pattern, or moving the orchestration out of kea entirely) needs to land before any of these extractions are worth keeping.Created with PostHog Code