Skip to content

feat: digest cs experiment#3812

Merged
capJavert merged 4 commits intomainfrom
digest-cs-experiment
Apr 23, 2026
Merged

feat: digest cs experiment#3812
capJavert merged 4 commits intomainfrom
digest-cs-experiment

Conversation

@capJavert
Copy link
Copy Markdown
Contributor

Adds a digest_cs_v1 experiment that switches the digest feed_config_name based on Snotra's personalisation signal.

  • New getUserProfile on SnotraClientPOST /api/v1/user/profile
  • New personalizedDigestSnotraClient (dedicated Garmr + metrics, matches the digest feed client)
  • New FeedConfigName.DigestV2 / DigestCsV1
  • In getPersonalizedDigestEmailPayload: when feature.feedConfig === 'digest_cs_v1', call Snotra and map non_personalised → digest_cs_v1, anything else (or error) → digest_v2. Otherwise passthrough.

@capJavert capJavert requested a review from pnvasanth April 22, 2026 14:26
@capJavert capJavert self-assigned this Apr 22, 2026
@capJavert
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 22, 2026

Claude finished @capJavert's task in 1m 59s —— View job


Code Review

  • Gather context and read changed files
  • Review implementation
  • Post findings

Overall the approach is clean and the experiment wiring is straightforward. A few issues worth addressing, one of which is a potential silent-failure bug.

Issues

1. fetchParse does not throw on non-2xx responses

fetchParse in src/integrations/retry.ts:133 calls res.json() without checking res.ok. Unlike retryFetchParseretryFetch, which throws an HttpError for 4xx/5xx, fetchParse silently parses the error body.

For getUserProfile, this means a Snotra 5xx returning {"error":"..."} will be parsed and returned without throwing. The .personalise.state access will be undefined, which happens to compare false to NonPersonalised, so DigestV2 is used — the right fallback, but the catch block is never reached and the error is silently swallowed without any logging.

The same issue now applies to getProfile (which was also switched to fetchParse in this PR). A 4xx/5xx response there would return a mangled object with no .profile_text / .update_at into callers in graphorm/index.ts and schema/opportunity.ts, which could cause subtle data corruption.

Consider adding a status check in fetchParse, or using retryFetchParse for calls that need error propagation. Fix this →

2. getProfile loses application-level retries — potential regression

The PR replaces retryFetchParse with fetchParse in getProfile. The existing snotraClient (used by graphorm and opportunity resolvers) is constructed with a GarmrService that has no retryOpts:

// src/integrations/snotra/clients.ts:75-82
const garmrSnotraService = new GarmrService({
  service: SnotraClient.name,
  breakerOpts: { halfOpenAfter: 5 * 1000, threshold: 0.1, duration: 10 * 1000 },
});

Previously retryFetchParse provided application-level retries for transient network errors. Now getProfile via snotraClient has neither Garmr retries nor application retries. If this regression is intentional, it should be called out explicitly. Fix this →

3. getUserProfile lacks a mock path

getProfile has an isMockEnabled() guard that returns mockSnotraEngagementProfile in mock/dev environments (clients.ts:39-41). getUserProfile has no equivalent guard, so in mock environments it will attempt a real HTTP call to SNOTRA_ORIGIN. This could cause failures in local development when SNOTRA_ORIGIN is not set or reachable. Fix this →

Minor Observations

  • ISnotraClient still uses interface — CLAUDE.md prefers type over interface. The ProfileRequest and ProfileResponse types were correctly converted in this PR, but ISnotraClient (line 29) was not. Low priority.

  • Second test case missing spy assertion — The should use digest_cs_v1 when snotra reports user is non_personalised test does not assert that getUserProfile was called with the expected arguments. Adding expect(spy).toHaveBeenCalledWith(...) (matching the first test case) would tighten the coverage.

  • Garmr config duplication — The personalizedDigestSnotraClient and personalizedDigestFeedClient share near-identical Garmr + counter config. Not a blocker, but worth noting for future maintainability.

@pulumi
Copy link
Copy Markdown

pulumi Bot commented Apr 22, 2026

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

✨ Neo Explanation

Routine deployment bumping all services to a new image tag (`a4643ed6`), with fresh DB and ClickHouse migration jobs created for this commit. ✅ Low Risk

This is a standard application deployment rolling out commit a4643ed6 (from 6e7fcba1) across all workloads. Every deployment and cron job receives only an updated container image tag and version label — no configuration, resource limits, or scheduling changes.

The two migration Jobs (Postgres via TypeORM and ClickHouse) are replaced by design: they are named with the commit SHA suffix, so each deploy creates new Job resources for the new commit and deletes the old ones. The new migration jobs will run against the databases before (or alongside) the rolling deployment of the application pods — confirm the migration jobs complete successfully before the new pods serve traffic, particularly if this release includes schema changes.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob    update
~   vpc-native-update-views-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
-   vpc-native-api-clickhouse-migration-6e7fcba1               kubernetes:batch/v1:Job        delete
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-channel-highlights-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-daily-quests-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-yearly-best-post-archives-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
+   vpc-native-api-clickhouse-migration-a4643ed6               kubernetes:batch/v1:Job        create
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
+   vpc-native-api-db-migration-a4643ed6                       kubernetes:batch/v1:Job        create
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-deployment                                      kubernetes:apps/v1:Deployment  update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob    update
... and 11 other changes

@capJavert capJavert merged commit 144d0ff into main Apr 23, 2026
11 checks passed
@capJavert capJavert deleted the digest-cs-experiment branch April 23, 2026 07:47
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.

2 participants