Skip to content

feat: seed tag-chip feeds from clustered topics - #4046

Draft
capJavert wants to merge 1 commit into
mainfrom
feat/tag-chip-topic-grouping
Draft

feat: seed tag-chip feeds from clustered topics#4046
capJavert wants to merge 1 commit into
mainfrom
feat/tag-chip-topic-grouping

Conversation

@capJavert

Copy link
Copy Markdown
Contributor

Adds a second tag-chip seeding strategy behind a client-controlled feedList(tagChipSeedStrategy:) arg so the two can be A/B tested. V1 is today's behavior (one feed per tag from /api/user_tags) and stays the default; V2 clusters the user's onboarding tags through the feed service's new /api/topics endpoint and seeds one multi-tag feed per topic.

  • FeedClient.getTopics + mock so MOCK_EXTERNAL_SERVICES still works locally
  • cluster_threshold read from remoteConfig, omitted when unset so the service default applies
  • getUserOnboardingTags moved out of channelDigest into common/feed and shared by both strategies
  • strategy recorded on User.flags.tagChipFeedsSeedStrategy so the arms are segmentable after the fact
  • V2 degrades to V1 when the user has no onboarding tags or clustering fails; seeding still never throws into the resolver

Seeding stays one-shot per user, so the experiment buckets users at their first opted-in read and already-seeded users are untouched.

Adds a second tag-chip seeding strategy behind a client-controlled
feedList(tagChipSeedStrategy:) arg so the two can be A/B tested. V1 is
today's behavior (one feed per tag from /api/user_tags) and stays the
default; V2 clusters the user's onboarding tags through the feed service's
new /api/topics endpoint and seeds one multi-tag feed per topic.

- FeedClient.getTopics + mock so MOCK_EXTERNAL_SERVICES still works locally
- cluster_threshold read from remoteConfig, omitted when unset so the
  service default applies
- getUserOnboardingTags moved out of channelDigest into common/feed and
  shared by both strategies
- strategy recorded on User.flags.tagChipFeedsSeedStrategy so the arms are
  segmentable after the fact
- V2 degrades to V1 when the user has no onboarding tags or clustering
  fails; seeding still never throws into the resolver

Seeding stays one-shot per user, so the experiment buckets users at their
first opted-in read and already-seeded users are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@capJavert

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @capJavert's task in 4m 6s —— View job


Review

Solid, well-scoped change — the one-shot reservation, the V2→V1 degradation, and the "seeding never throws into the resolver" contract are all preserved, and test coverage of the new paths is good. A few things worth addressing, roughly in severity order.


1. The recorded strategy is requested, not effective — the V2 arm will be contaminated

reserveSeedSlot writes tagChipFeedsSeedStrategy before any seeding work happens (src/common/seedTagChipFeeds.ts:83-86), but V2 falls back to V1 in three cases: no onboarding tags, getTopics throws, getTopics returns nothing. All three end up with V1-shaped single-tag feeds recorded as V2.

Worse, the fallback is mostly silent. fetchParse never checks res.ok (src/integrations/retry.ts:133-146), so a 500 from /api/topics with a JSON body returns {}result?.data ?? [][]. The try/catch + logger.error at src/common/seedTagChipFeeds.ts:206-211 only fires on network/JSON-parse failures, not on HTTP errors — which is exactly the failure mode the PR body says it degrades from. Your own test asserts this (__tests__/integrations/feed.ts "should degrade to an empty list when the feed service errors" passes because 500 is indistinguishable from success).

Net effect: if /api/topics is unhealthy during rollout, a chunk of the V2 bucket gets V1 feeds, labelled V2, with no log line. Since seeding is one-shot, that's unrecoverable per-user.

Suggestion: record the effective outcome — either write the strategy after seeding, or add a second flag write / a tagChipFeedsSeedTopicCount so V2 users that actually got clustered feeds are distinguishable. At minimum, logger.warn when V2 was requested but no topics came back. Fix this →

2. Service-returned tags are inserted unvalidated → one bad tag permanently kills the whole seed

ContentPreferenceKeyword.keywordId has an FK to Keyword (src/entity/contentPreference/ContentPreferenceKeyword.ts:11-13), and topic.tags goes straight from the /api/topics response into the insert (src/common/seedTagChipFeeds.ts:244-253). If the service returns a normalized/synthesized/renamed tag that isn't a keyword.value row, the FK violation rolls back the whole transaction → the resolver swallows it → the user is left with tagChipFeedsSeededAt set and zero chip feeds, forever.

You already know the valid set: it's the tags array you sent. Intersecting topicTags against it in getSeedTopics removes this class of failure entirely and costs one Set. Fix this →

3. V2 sends an unbounded tag list to /api/topics

getSeedTopics calls getUserOnboardingTags({ con, userId }) with no limit (src/common/seedTagChipFeeds.ts:133). Despite the name, that query is all non-blocked keyword preferences on the main feed — which keeps growing as the user follows tags, so it isn't the onboarding set for anyone but a brand-new user. A long-lived user hitting feedList(includeTagChipFeeds: true, tagChipSeedStrategy: V2) for the first time could ship hundreds of tags to the feed service, and the clustering will be dominated by later follows rather than the onboarding picks. headlineFollows.ts has an explicit maxSeededChannels cap for the same reason. Worth a cap (and/or order: { createdAt: 'ASC' }) here.

4. The shared helper quietly loosened the V1 (control) arm

The extracted getUserOnboardingTags (src/common/feed.ts:7-27) filters status: Not(Blocked), whereas the V1 fallback it replaced filtered status: Follow — so Subscribed keywords are now included in the control arm, in the same PR that establishes the A/B baseline. Probably harmless (arguably more correct), but it's an unflagged behavior change in the control. It also switched keywordIdreferenceId (equivalent in practice for this entity, just noting it).

Related: find + take: limit with no order means which tags survive truncation is nondeterministic. Pre-existing, but it now backs more call sites — and __tests__/common/seedTagChipFeeds.ts expect(getTopicsMock).toHaveBeenCalledWith(['javascript', 'nodejs'], 0.4) depends on unordered rows coming back in insert order, which is a latent flake.

5. A third copy of the same fallback block was left behind

src/common/feedTagsList.ts:109-135 still hand-rolls the identical getUserTags → onboarding-follows fallback (plus its own dedupeKeepOrder). Since this PR is doing exactly that extraction, that's the natural third caller. Note the two implementations have already diverged: feedTagsList uses queryReadReplica + status: Follow, the new helper uses the primary connection + Not(Blocked).

On the connection: the new helper reads from primary while its neighbours in seedTagChipFeeds.ts (resolveLabel, countUserOwnedFeeds) both go through queryReadReplica. Per CLAUDE.md's replica preference these reads should probably be on the replica too. Fix this →

6. tagChipSeedStrategy: null persists null instead of V1

The SDL default (src/schema/feeds.ts:1075) only applies when the arg is absent — a client explicitly sending tagChipSeedStrategy: null reaches the resolver as null, and the strategy = TagChipSeedStrategy.V1 default parameter only covers undefined. Result: flags.tagChipFeedsSeedStrategy: null for a user who was seeded with V1. Either make the arg non-nullable (TagChipSeedStrategy! = V1) or strategy: args.tagChipSeedStrategy ?? TagChipSeedStrategy.V1.


Nits

  • Per-tag round-trips inside the transaction (src/common/seedTagChipFeeds.ts:244-254): 2 save() calls per tag. With 5 topics × 5 tags that's 55 statements; save([...]) on arrays would collapse it.
  • FeedTopic.tags is typed required but read defensively (topicTags ?? [] at src/common/seedTagChipFeeds.ts:148). It's an external payload — type it tags?: string[], or validate the response with a zod schema in src/common/schema/ per CLAUDE.md, so the guard and the type agree.
  • Dedupe keys on label, not the resolved name: two distinct labels resolving to the same Keyword.flags.title would produce two identically-named feeds.

Things I liked

  • cluster_threshold omitted rather than sent as undefined when unset, so the service default applies — and both branches are tested.
  • Mock added alongside the client so MOCK_EXTERNAL_SERVICES keeps working, matching the userTags pattern.
  • The docblock rewrite on seedTagChipFeedsIfNeeded accurately describes both arms.
  • No migration needed (JSONB flag), correctly.

I could not run pnpm run build / lint / the test suite here — dependencies aren't installed and there's no database in this environment, so the findings above are from reading the code rather than execution.
branch feat/tag-chip-topic-grouping

@pulumi

pulumi Bot commented Aug 4, 2026

Copy link
Copy Markdown

🍹 The Update (preview) for dailydotdev/api/prod (at d00f62b) was successful.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-post-lifecycle-state-clickhouse-cron            kubernetes:batch/v1:CronJob    update
-   vpc-native-api-db-migration-80d8ded0                       kubernetes:batch/v1:Job        delete
~   vpc-native-personalized-digest-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-daily-quests-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
+   vpc-native-api-clickhouse-migration-f1f1cb6e               kubernetes:batch/v1:Job        create
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-interest-scheduled-run-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-user-world-clickhouse-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-old-notifications-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-yearly-best-post-archives-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
-   vpc-native-api-clickhouse-migration-80d8ded0               kubernetes:batch/v1:Job        delete
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-achievements-cron                kubernetes:batch/v1:CronJob    update
... and 17 other changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant