Skip to content

feat: post hero#3925

Open
capJavert wants to merge 2 commits into
mainfrom
post-hero
Open

feat: post hero#3925
capJavert wants to merge 2 commits into
mainfrom
post-hero

Conversation

@capJavert
Copy link
Copy Markdown
Contributor

No description provided.

@capJavert capJavert self-assigned this Jun 3, 2026
@capJavert
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented Jun 3, 2026

Claude finished @capJavert's task in 2m 56s —— View job


Review: feat: post hero

  • Read changed files & gather context
  • Analyze entities, migration, cron, schema, tests
  • Post review feedback

Nicely structured change — the unified post_hero MV (editorial highlights ∪ algorithmic lifecycle states) with canonical-wins precedence via DISTINCT ON (postId) ORDER BY postId, src is clean, and the cron is idempotent (upsert + delete in a tx, lastRunAt written last so failures safely replay the window). A few things worth a look:


1. Incremental sync + 12h MV window can drop a still-breakout post — confirm intended

The Clickhouse query only pulls rows with updated_at >= lastRunAt (~15 min window), and the persisted updatedAt is set to cron run time (currentRunAt), not the Clickhouse last_updated_at. The MV then filters "updatedAt" > now() - interval '12 hours'.

Net effect: a post that became breakout and then stays breakout (no further state changes) stops appearing in the incremental window, so its updatedAt is never re-bumped, and it falls out of the hero MV ~12h after its last state change — even if it's still breakout in Clickhouse. If product intent is "show while breakout", this silently expires it. If hero cards are meant to be ephemeral (~12h from the event), this is fine — just want to confirm.

Relatedly, last_updated_at is parsed in the schema (z.coerce.date) but never used — highlightedAt ends up reflecting sync time rather than the actual state-change time. Consider persisting last_updated_at into updatedAt instead, which would also make the freshness window track the real event time.

2. The 12h freshness window is duplicated and can diverge from the highlight TTL

The MV hardcodes interval '12 hours' (in two places), while the hero GraphORM relation filters on getPostHighlightTtlSeconds() (src/graphorm/index.ts:768), which defaults to 12h but is remote-config driven (postHighlightTtlSeconds). If that config is ever changed, the GraphORM filter and the MV's baked-in 12h diverge — a row can exist in the MV but be filtered out by the resolver (or vice-versa). Worth a comment noting the MV window must be kept in sync, or documenting that the MV is the authoritative gate and the resolver TTL is just a safety net.

3. Unrelated change: orderBy removed from the existing postHighlight relation

src/graphorm/index.ts:756-757 — the diff drops .orderBy(significance ASC).addOrderBy(highlightedAt DESC) from postHighlight. This is functionally harmless because highlights_canonical has a unique index on postId (so limit(1) only ever has one candidate), but it's unrelated to the hero feature and the removal isn't obvious-by-inspection safe to a future reader. Either keep it or leave a note — otherwise it reads like an accidental edit.

4. significance: String! is non-null but the MV CASE has no ELSE

In the canonical branch the CASE significance WHEN 1..4 has no ELSE, so any significance value outside 1–4 (with significance != 0 already excluded) yields NULL, which would violate the non-null significance: String! GraphQL field. Currently safe given the PostHighlightSignificance enum only spans 0–4, but an ELSE fallback (or filtering to the known set) would harden against a future enum addition. Fix this →


Minor / nits

  • REFRESH MATERIALIZED VIEW CONCURRENTLY (cron) / raw con.query — correct: CONCURRENTLY is outside the transaction (it can't run inside one) and the required unique index UQ_post_hero_post exists; the MV is populated at CREATE time so the first concurrent refresh won't fail. Raw SQL here is an acceptable exception since REFRESH has no repository equivalent.
  • Hardcoded headlines ('Breaking out', 'Evergreen') live in SQL inside the migration and are re-asserted in tests — not localizable and duplicated. Fine if these are internal labels, but if they're user-facing they probably shouldn't be baked into a migration.
  • hero relation sets both childColumn/parentColumn and a customRelation with an explicit WHERE (src/graphorm/index.ts:762-766); postHighlight achieves the same with just customRelation. The columns look redundant here — harmless, just inconsistent with the sibling relation.
  • Migration down correctly relies on dropping the MV to cascade its indexes and explicitly drops the lifecycle table/index. 👍

Test coverage is solid — cron upsert/update/delete/empty paths, MV-refresh assertion, and the canonical-vs-lifecycle precedence cases in __tests__/posts.ts all look good.
· post-hero

@pulumi
Copy link
Copy Markdown

pulumi Bot commented Jun 3, 2026

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

✨ Neo Explanation

Routine feature deployment adding post lifecycle state tracking (breakout/evergreen) synced from ClickHouse, with a new `post_hero` materialized view and GraphQL `hero` field. DB and ClickHouse migration jobs will run as part of this deploy. ✅ Low Risk.

This PR introduces a new post-lifecycle-state-clickhouse feature: a post_lifecycle_state Postgres table, a post_hero materialized view that unions editorial highlights with algorithmic lifecycle states (breakout/evergreen), a new hero GraphQL field on posts, and a cron job that syncs lifecycle state from ClickHouse every 15 minutes and refreshes the MV concurrently after each run.

The deployment rolls a new image (84e6eab1) to all services, runs both a DB migration job (creating post_lifecycle_state table + post_hero MV) and a ClickHouse migration job, and creates the new cron.

🔵 Info — The DB migration creates the post_lifecycle_state table and post_hero materialized view. The MV uses DISTINCT ON ("postId") ORDER BY postId, src to give canonical highlights priority over lifecycle states — worth confirming the src ordering (0 = canonical, 1 = lifecycle) behaves as expected at query time, since the MV is built once and refreshed concurrently by the cron.

🔵 Info — The post_hero MV has a hardcoded 12-hour TTL window in its definition. Unlike a view with runtime evaluation, this TTL is baked in at REFRESH time, so stale rows won't age out between cron runs — the MV must be refreshed for the window to advance. The cron runs every 15 minutes, which is fine for normal operation, but any cron downtime leaves the MV frozen.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-update-tags-str-cron                            kubernetes:batch/v1:CronJob    update
+   vpc-native-post-lifecycle-state-clickhouse-cron            kubernetes:batch/v1:CronJob    create
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
-   vpc-native-api-db-migration-2575d480                       kubernetes:batch/v1:Job        delete
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-highlights-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-channel-highlights-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-old-notifications-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  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-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
+   vpc-native-api-clickhouse-migration-84e6eab1               kubernetes:batch/v1:Job        create
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-daily-quests-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-channel-digests-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
-   vpc-native-api-clickhouse-migration-2575d480               kubernetes:batch/v1:Job        delete
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                                   kubernetes:apps/v1:Deployment  update
+   vpc-native-api-db-migration-84e6eab1                       kubernetes:batch/v1:Job        create
... and 13 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