chore(taxonomic-filter): boy scout fixes — three pre-existing issues#59848
Closed
pauldambra wants to merge 5 commits into
Closed
chore(taxonomic-filter): boy scout fixes — three pre-existing issues#59848pauldambra wants to merge 5 commits into
pauldambra wants to merge 5 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Purely mechanical deduplication: extracts duplicate Recent/Pinned group definitions into a shared constant and makes two minor simplifications (removes a redundant wrapper arrow function; removes a dead-code branch in the feature-flag filter). No behavioral changes, no API or data model impact.
Contributor
|
Contributor
|
Size Change: 0 B Total Size: 80.7 MB ℹ️ View Unchanged
|
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. |
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
pauldambra
force-pushed
the
posthog-code/boy-scout-taxonomic-filter-fixes
branch
from
c567785 to
5e7146b
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 mechanical deduplication and simplification with no behavioral changes — shared constant extraction, redundant wrapper removal, tightened import path, and removal of explicitly dead-code filter fallbacks in a predicate that only ever receives FeatureFlagType items.
pauldambra
force-pushed
the
posthog-code/boy-scout-taxonomic-filter-fixes
branch
from
5e7146b to
cae7f07
Compare
pauldambra
force-pushed
the
posthog-code/extract-misc-taxonomic-logic
branch
from
3937648 to
89ed1f8
Compare
This was referenced May 25, 2026
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
pauldambra
force-pushed
the
posthog-code/extract-misc-taxonomic-logic
branch
from
ef683a5 to
7703f8d
Compare
pauldambra
force-pushed
the
posthog-code/boy-scout-taxonomic-filter-fixes
branch
from
dc7eb52 to
3e6a576
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
pauldambra
force-pushed
the
posthog-code/extract-misc-taxonomic-logic
branch
from
7703f8d to
cf3c8b3
Compare
pauldambra
force-pushed
the
posthog-code/boy-scout-taxonomic-filter-fixes
branch
from
3e6a576 to
5a179bc
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 mechanical frontend refactoring with no behavioral changes — deduplicates identical Recent/Pinned group definitions into a shared constant, removes a redundant wrapper arrow function, and simplifies a filter predicate that only ever receives FeatureFlagType items in the feature-flag group context. No production risk.
pauldambra
force-pushed
the
posthog-code/extract-misc-taxonomic-logic
branch
from
cf3c8b3 to
01feaa9
Compare
pauldambra
force-pushed
the
posthog-code/boy-scout-taxonomic-filter-fixes
branch
from
5a179bc to
357522b
Compare
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.
pauldambra
commented
May 26, 2026
Member
Author
pauldambra
left a comment
pauldambra
left a comment
There was a problem hiding this comment.
QA Swarm review complete. See inline comments. Verdict: APPROVE WITH NITS — clean boy-scout cleanup, no blocking issues.
Member
Author
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: qa-team (correctness/reliability), paul-reviewer, xp-reviewer. security-audit skipped (no API surface — pure UI cleanup). Verdict: 💬 APPROVE WITH NITSTextbook boy-scout commit. Three small, behavior-preserving cleanups: kills the worst duplication (Recent/Pinned tab definitions in two places), drops a defensive-but-unreachable type guard in the Feature Flags Key findings🟢 LOW (convergent: qa-team + paul) — ⚪ NIT (convergent: paul + xp) — 🟢 LOW (paul) — The dropped fallback in posthogResources Reviewer summaries
Automated by QA Swarm — not a human review |
github-actions
Bot
dismissed
their stale review
New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.
Contributor
There was a problem hiding this comment.
Pure mechanical refactoring — deduplication into a shared constant, dead-code removal in a FeatureFlag-only search predicate, and a redundant wrapper elimination. No behavioral changes and no production risk. The one unresolved inline comment about readonly is a low-priority style nit, not a blocker.
Contributor
There was a problem hiding this comment.
Pure mechanical frontend refactoring with no behavioral changes: deduplicates static group definitions into a shared immutable constant, removes a now-unnecessary kea logic wrapper, eliminates dead-code filter branches, and eta-reduces a redundant wrapper. All inline review concerns are resolved, including the mutation-safety nit (addressed via as const satisfies readonly).
Greptile correctly identified three pre-existing code quality issues
in files touched by the refactor stack. Fixing them here per the
boy scout rule.
1. **Recent/Pinned tab duplication between kea and headless paths**
(`recentPinnedTaxonomicGroupsLogic.ts` /
`utils/buildTaxonomicGroups.tsx`)
Both the kea-logic path (`taxonomicFilterLogic` → kea hooks) and
the pure-function path (`buildTaxonomicGroups` → `useTaxonomicFilter`
React hook) were defining the Recent and Pinned tab configurations
independently with identical body. The kea PR extracted one copy
into `recentPinnedTaxonomicGroupsLogic.ts`; the other copy in
`buildTaxonomicGroups.tsx` was untouched. Fix: extract the two tab
definition objects as a shared exported constant
`RECENT_PINNED_TAB_DEFINITIONS` and spread it in both places.
2. **Unreachable second branch in Feature Flags `localItemsSearch`**
(`posthogResourcesTaxonomicGroupsLogic.tsx`)
`FeatureFlagType` always has both `key` and `name`, so the first
`if ('key' in item && 'name' in item)` branch always matches,
making the subsequent `if ('name' in item)` dead code. Simplified
to a single `.filter` using the flag cast directly.
3. **Superfluous arrow wrapper on `getRevenueAnalyticsDefinitionIcon`**
(`revenueAnalyticsTaxonomicGroupsLogic.ts`)
`getIcon: (option: PropertyDefinition): JSX.Element =>
getRevenueAnalyticsDefinitionIcon(option)` wraps a function that
already has the same signature. Use the function reference directly.
All 415 affected tests pass (362 TaxonomicFilter + 53 hooks).
Generated-By: PostHog Code
Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
…ea wrapper The kea logic existed only to expose RECENT_PINNED_TAB_DEFINITIONS via a selector with zero inputs that returned the constant unchanged. After the previous boy-scout commit hoisted the array to a module-level const, the wrapper added nothing: no actions, no reducers, no inputs, no reactivity, no key-based variation. Pure ceremony. Drop the kea logic and have taxonomicFilterLogic import the constant directly, the same way buildTaxonomicGroups already does. allMetaTaxonomicGroups now spreads RECENT_PINNED_TAB_DEFINITIONS inline, leaving only the dynamic suggested-filters tab as a selector input. The file keeps its name to minimise import churn; the *Logic suffix is now a misnomer but the rename is a follow-up. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
The constant is shared by two consumers (taxonomicFilterLogic and buildTaxonomicGroups). Without compile-time immutability, a future caller doing `groups[0].name = 'Recently used'` would silently affect both. Switch from `: TaxonomicFilterGroup[]` to `as const satisfies readonly TaxonomicFilterGroup[]` — the array is now readonly at the array level and each element is deeply readonly through literal-type narrowing, while still being type-checked against the TaxonomicFilterGroup interface. Drops the per-element `as TaxonomicFilterGroup` casts which `satisfies` now subsumes. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
…s to recentPinnedTabDefinitions.ts The file no longer contains a kea logic — just exports a frozen constant. Convergent qa-swarm finding (qa-team + paul + xp) on the prior iteration: the *Logic suffix misleads a future reader grepping for kea actions/selectors. Match the filename to the contents. Two import sites updated. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
After dropping the kea wrapper, the explanatory comment over-narrated what the body already shows. Cut to just the why: dodging the 16-dep tuple limit. paul + xp NIT from the follow-up qa-swarm pass. Generated-By: PostHog Code Task-Id: 2649f7ae-c1f7-40ae-8866-be024f3f1285
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Member
Author
|
Closing — the extraction approach hit kea's 16-dep SelectorTuple limit repeatedly and needs a different architectural strategy. The stack is being abandoned and master is being reverted back to the state right after #59816. See the upcoming revert PR for details. |
This was referenced May 28, 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
Greptile correctly identified three pre-existing code quality issues in files touched by the refactor stack. Fixing them here per the boy scout rule.
Changes
1. Recent/Pinned tab duplication between kea and headless paths
Both
taxonomicFilterLogic(kea) andbuildTaxonomicGroups(pure function foruseTaxonomicFilter) were defining the Recent and Pinned tab configurations independently with identical bodies. Fix: extract as a sharedRECENT_PINNED_TAB_DEFINITIONSconstant inrecentPinnedTaxonomicGroupsLogic.ts, spread it in both places.2. Unreachable second branch in Feature Flags
localItemsSearch(posthogResourcesTaxonomicGroupsLogic.tsx)FeatureFlagTypealways has bothkeyandname, so the first branch always matched, making the secondif ('name' in item)dead code. Simplified to a single.filter.3. Superfluous arrow wrapper on
getRevenueAnalyticsDefinitionIcon(revenueAnalyticsTaxonomicGroupsLogic.ts)(option) => getRevenueAnalyticsDefinitionIcon(option)wraps a function with the same signature. Use the reference directly.How did you test this code?
Agent-written. Automated tests:
hogli test frontend/src/lib/components/TaxonomicFilter/— 362/362 passhogli test frontend/src/lib/components/TaxonomicFilter/hooks/— 53/53 passpnpm format— cleanThe agent did not manually click through the UI.
🤖 Agent context
Written by PostHog Code (Claude Sonnet 4.6). All three fixes were pre-existing in the original
taxonomicFilterLogic.tsxand moved verbatim into the extracted child logics. Greptile flagged them in bot review threads on the refactor PRs; the shepherd triage correctly identified the spanmessagekey as out-of-scope (unclear whether intentional) and fixed the other three.Created with PostHog Code