feat(feeds): A/B clustered tag-chip feed seeding - #6427
Conversation
FeedChipsVariant.V3 opts the user into the API's V2 seed strategy, which groups related tags into a single chip feed per topic instead of one feed per tag. Default stays V2, so merging does not ship the new arm. The chip surfaces needed no changes for grouping itself: buildPersonalizedCategories already maps Feed -> chip via flags.name/slug/id with no single-tag assumption. - useFeedChipsVariant owns the flag -> strategy derivation for its three callers; the isOnboardingComplete gate stays in useFeeds so the chip strips don't pull in a QueryClient dependency - tagChipSeedStrategy is part of the useFeeds query key so the arms never share a cache entry - ClickFeedTagChip now carries the variant and the chip's feed origin, so the arms are comparable on chip engagement Also fixes ClickFeedTagChip on mobile: UnifiedMobileFeedNav declared and read item.tag but never assigned it, so the guard always short-circuited and the event never fired there. Feed chips now set tag (the feed id, matching desktop) alongside origin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
|
Claude finished @capJavert's task in 3m 50s —— View job Review
The refactor is clean and the query-key/default-value handling is right. One blocker though: the V3 arm never renders any chips, so the experiment can't measure what it's designed to measure. 🔴 Blocker — every chip surface is still gated on
|
| File | Effect on V3 |
|---|---|
packages/shared/src/components/MainFeedLayout.tsx:336 |
showExploreChips → false, no desktop chip strip |
packages/shared/src/components/feeds/FeedNav.tsx:83 |
shouldRenderFeedChips → false, no mobile chip strip (except /daily) |
packages/shared/src/components/feeds/FeedSettingsButton.tsx:48 |
reverts to pre-chips behavior: non-Plus users on a custom feed get the "upgrade → else delete the feed" prompt |
packages/shared/src/components/feeds/FeedSettings/useFeedSettingsEdit.tsx:68 |
pre-chips edit flow |
packages/shared/src/components/feeds/FeedSettings/FeedSettingsEditHeader.tsx:103 |
pre-chips header |
packages/webapp/pages/feeds/[slugOrId]/edit.tsx:47 |
pre-chips edit page |
So a V3 user gets clustered tag-chip feeds seeded server-side (useFeeds correctly asks for TagChipSeedStrategy.V2), those feeds show up in CustomFeedSection (sidebar) and AddToCustomFeedModal — but there is no chip strip to click, and ClickFeedTagChip (the metric this PR adds extra.variant for) can never fire. Worse, V3 lands non-Plus users back on the delete-feed prompt path in FeedSettingsButton.
These sites should read hasTagChipFeeds from the new hook — which needs item 2 first. Fix this →
🟠 The hook hardcodes shouldEvaluate: !!user, which blocks that reuse
useFeedChipsVariant.ts:20 fixes shouldEvaluate: !!user, but the six sites above each have a narrower gate — isBelowLaptop (FeedNav), !!user && isLaptop && isChipStripPage (MainFeedLayout), !isPlus (FeedSettingsButton). A straight swap would widen flag evaluation, which CLAUDE.md explicitly warns against. Accept an option instead:
export const useFeedChipsVariant = ({ shouldEvaluate = true } = {}): UseFeedChipsVariant => {
const { user } = useAuthContext();
const { value: variant } = useConditionalFeature({
feature: featureFeedChips,
shouldEvaluate: !!user && shouldEvaluate,
});🟠 Deploy ordering is a hard coupling, not a soft one
FEED_LIST_QUERY now declares $tagChipSeedStrategy: TagChipSeedStrategy (packages/shared/src/graphql/feed.ts:833). GraphQL validates the variable's type name against the server schema, so if this ships before daily-api#4046 lands, FeedList fails validation for every user on every arm — the custom feed list goes empty app-wide, not just for V3. Same exposure in reverse for the extension (already-released bundles can't be rolled back) if the API PR is ever reverted. Please merge/deploy the API first and confirm the extension release trails it.
🟡 tagChipSeedStrategy is sent even when includeTagChipFeeds is false
useFeeds.ts:44,64 always sends the strategy, including for the None control arm and pre-onboarding users. Harmless if the API ignores it, but it also changes the query key for everyone, forcing an extra FeedList refetch on rollout for users whose request is otherwise identical. Passing undefined (and omitting it from the key) unless includeTagChipFeeds keeps the control arm on its existing cache entry.
🟡 Neither new test covers flag → query wiring anymore
useFeedChipsVariant.spec.tsxmocksuseConditionalFeature, so it only asserts the two-line ternary. The one piece with real behavior — theshouldEvaluate: !!usergate — is never asserted, even thoughuseAuthContextis mocked for it.useFeeds.spec.tsxnow mocksuseFeedChipsVariantwholesale, so the previous end-to-end path (real flag throughGrowthBookProvider→ query variables) is gone. A single test that setsfeed_chips: 'v3'via the existingGrowthBookProviderwrapper and assertstagChipSeedStrategy: V2on the wire would cover the chain the experiment depends on.
✅ Verified good
- The mobile
ClickFeedTagChipfix is real —ChipItem.tagwas declared and read atUnifiedMobileFeedNav.tsx:274but never assigned, so the guard always returned early.tag: feed.idnow matches desktop'sbuildPersonalizedCategories. originis inCUSTOM_FEED_FRAGMENT(packages/shared/src/graphql/fragments.ts:576), sofeed.flags?.originis actually populated — the newextra.originisn't dead.tagChipSeedStrategyin theuseFeedsquery key correctly prevents cross-arm cache sharing.- Flag default stays
V2, so merging doesn't ship V3.
Notes for the analysis
UnifiedMobileFeedNav/ExploreChipsBar also render on /daily regardless of the arm (FeedNav.tsx:103 || isDailyPage, and DailyHome.tsx:52 unconditionally on laptop), so expect ClickFeedTagChip events with variant: 'none' from /daily. And since tag is set for all category chips, the event fires for user-created custom feeds too — filter on extra.origin === 'TAG_CHIP' for chip-engagement metrics.
I couldn't run jest or lint here — the sandbox denied the test commands, so the findings above are from reading the code. Nothing in the diff looked likely to fail the changed-file typecheck.
· branch feat/tag-chip-topic-grouping
FeedChipsVariant.V3 opts the user into the API's V2 seed strategy, which groups related tags into a single chip feed per topic instead of one feed per tag. Default stays V2, so merging does not ship the new arm.
The chip surfaces needed no changes for grouping itself: buildPersonalizedCategories already maps Feed -> chip via flags.name/slug/id with no single-tag assumption.
Also fixes ClickFeedTagChip on mobile: UnifiedMobileFeedNav declared and read item.tag but never assigned it, so the guard always short-circuited and the event never fired there. Feed chips now set tag (the feed id, matching desktop) alongside origin.
Changes
Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Preview domain
https://feat-tag-chip-topic-grouping.preview.app.daily.dev