Skip to content

Batch click stats updates through a Redis stream cron - #4231

Merged
steven-tey merged 11 commits into
mainfrom
click-stream-updates
Jul 28, 2026
Merged

Batch click stats updates through a Redis stream cron#4231
steven-tey merged 11 commits into
mainfrom
click-stream-updates

Conversation

@devkiran

@devkiran devkiran commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a scheduled background job (runs every minute) to drain click events and update aggregated click totals.
  • Improvements
    • Click tracking now publishes click events to a Redis stream before updating totals.
    • Workspace usage during sale tracking is updated directly.
    • Partner stats synchronization now supports only lead and sale event types.
  • Bug Fixes
    • Added safeguards to prevent double-processing when multiple runs overlap.
    • Improved batch processing resilience by continuing through individual failures without stopping the job.

@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dub Ready Ready Preview Jul 28, 2026 7:39pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Click recording publishes click events to Redis instead of directly incrementing counters. A locked cron route aggregates stream entries into SQL updates with batching, fallback handling, backlog alerts, and QStash continuation. Workspace and partner accounting contracts are also updated.

Changes

Click statistics pipeline

Layer / File(s) Summary
Click event stream publisher
apps/web/lib/upstash/redis-streams/link-click-events.ts
Defines the click event contract, writes events to Redis, and applies SQL fallback updates when publishing fails.
Click recording integration
apps/web/lib/tinybird/record-click.ts, apps/web/app/(ee)/api/track/application/route.ts
Replaces direct click counter updates with click-event publication while retaining caching and error reporting.
Scheduled aggregation and database updates
apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts, apps/web/vercel.json
Adds a per-minute locked cron route that batches stream entries, aggregates counters, updates SQL records, alerts on backlog, and republishes when work remains.
Accounting and partner event contracts
apps/web/lib/api/conversions/track-sale.ts, apps/web/lib/api/partners/sync-partner-links-stats.ts, apps/web/lib/upstash/redis-streams/*, apps/web/scripts/partners/aggregate-stats-seeding.ts
Records sale usage directly in SQL, removes click partner activity types, emits seeded lead events, and updates stream lifecycle documentation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClickTracking
  participant linkClickEventStream
  participant updateClickStatsCron
  participant PlanetScale
  ClickTracking->>linkClickEventStream: publish link click event
  updateClickStatsCron->>linkClickEventStream: consume and delete stream batch
  updateClickStatsCron->>PlanetScale: apply aggregated link, workspace, and enrollment counters
  updateClickStatsCron->>updateClickStatsCron: schedule continuation when backlog remains
Loading

Possibly related PRs

  • dubinc/dub#4033 — Both changes modify track-sale.ts within sale-tracking flows.
  • dubinc/dub#4148 — Both changes modify track-sale.ts within sale-tracking flows.
  • dubinc/dub#4193 — Both changes modify track-sale.ts within sale-tracking flows.

Suggested reviewers: steven-tey, pepeladeira

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: batching click stats updates via a Redis stream cron.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch click-stream-updates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@steven-tey
steven-tey marked this pull request as ready for review July 28, 2026 17:33
@steven-tey

Copy link
Copy Markdown
Collaborator

@coderabbitai full review, is this backwards compatible? any bugs?

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR metadata ==='
gh pr view 4231 --repo dubinc/dub --json number,title,baseRefName,headRefName,body,files \
  --jq '{number,title,baseRefName,headRefName,body,files:[.files[].path]}'

echo '=== Repository guidance ==='
fd -HI -a '^(AGENTS|CONTRIBUTING|CODEOWNERS|\.coderabbit)\.(md|yml|yaml)$|^AGENTS\.md$|^\.coderabbit\.yaml$|^\.coderabbit\.yml$' . \
  -E .git -E node_modules -E dist -E build | sort

echo '=== Changed-file summary ==='
git diff --stat origin/"$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')"..."$(git branch --show-current)"
echo '=== Changed files ==='
git diff --name-status origin/"$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')"..."$(git branch --show-current)"
echo '=== PR diff ==='
git diff --find-renames --find-copies --unified=80 \
  origin/"$(git remote show origin | sed -n '/HEAD branch/s/.*: //p')"..."$(git branch --show-current)"

Length of output: 28203


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Stream client outline ==='
ast-grep outline apps/web/lib/upstash/redis-streams/client.ts --items all
echo '=== Stream client ==='
cat -n apps/web/lib/upstash/redis-streams/client.ts

echo '=== Comparable cron routes ==='
fd -t f 'route\.ts$' 'apps/web/app/(ee)/api/cron/streams' -x sh -c '
  case "$1" in
    *update-workspace-clicks*|*update-workspace-links-usage*|*update-partner-stats*)
      echo "\n===== $1 ====="
      cat -n "$1"
      ;;
  esac
' sh {}

echo '=== All processBatch call sites with context ==='
rg -n -C 12 'processBatch\s*(?:<[^>]+>)?\s*\(' apps/web --glob '*.ts' --glob '*.tsx'

echo '=== Click-stats publisher imports/calls ==='
rg -n -C 8 '\bpublishClickStatsEvent\s*\(' apps/web --glob '*.ts' --glob '*.tsx'

echo '=== Cron and lock helpers ==='
for f in apps/web/lib/cron/with-cron.ts apps/web/lib/cron/index.ts apps/web/app/'(ee)'/api/cron/utils.ts; do
  if [ -f "$f" ]; then
    echo "\n===== $f ====="
    cat -n "$f"
  fi
done

echo '=== Link and project/enrollment definitions and relevant indexes ==='
rg -n -i -C 6 'model (Link|Project|ProgramEnrollment)\b|@@(id|index|unique).*?(programId|partnerId)|\b(lastClicked|totalClicks|usage|clicks)\b' \
  packages apps --glob 'schema.prisma' --glob '*.prisma' 2>/dev/null || true

Length of output: 109986


@steven-tey I found two correctness issues that should be addressed before merging:

  1. Potential permanent undercount of workspace/partner stats — In apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts, entries are added to processedEntryIds as soon as the Link update succeeds. If the subsequent Project or ProgramEnrollment update fails, RedisStream.processBatch still deletes the entry. The link count is correct, but workspace usage/totalClicks or enrollment totalClicks is then permanently missing. Acknowledge/delete an event only after all applicable derived updates succeed, or make each destination independently idempotent and retryable.

  2. QStash backlog chaining can be dropped by the lock — The route calls qstash.publishJSON() before finally releases lock:update-click-stats. If QStash delivers the POST quickly, it sees the active lock and returns a successful no-op (200), so QStash will not retry it. Under backlog, draining can fall back to one 10,000-entry batch per minute instead of chaining immediately. Publish after releasing the lock, or return a retryable status when the lock is held.

Backward compatibility: There are no public API or schema changes, but this is not behaviorally identical: link, workspace, and partner counters are now eventually consistent and may lag by at least one cron interval (or longer during a backlog). Any UI, limit enforcement, or downstream process that relies on an immediate post-click counter increment must tolerate that delay.

I also noted that the new route calls getStreamInfo() every batch; its current implementation does an unbounded XRANGE just to calculate length. A large backlog can make the monitoring path expensive precisely when the stream is under pressure.

Static review is complete; I did not execute repository tests because the review sandbox does not allow running project code.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
apps/web/lib/upstash/redis-streams/click-stats.ts (1)

6-8: 📐 Maintainability & Code Quality | 🔵 Trivial

Tracking the legacy stream cleanup TODO.

Want me to open an issue to track migrating the remaining publishers and removing the legacy streams?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/click-stats.ts` around lines 6 - 8, Open a
tracking issue for migrating the remaining legacy stream publishers to
track-sale → workspace-clicks-usage and lead/sale/commission → partner-activity,
then remove the legacy streams after migration or confirmation that they remain
needed; update the TODO with the issue reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/`(ee)/api/cron/streams/update-click-stats/route.ts:
- Around line 328-332: Replace the full-stream getStreamInfo call in the
click-stats route with an O(1) or bounded backlog check, such as XLEN or a
limited XRANGE using BATCH_SIZE + 1, and use that result for maybeAlertOnBacklog
and hasMore. Avoid downloading all remaining entries solely to determine stream
length.
- Around line 191-217: The update-click-stats handler currently marks entries
processed after only the Link write, allowing aggregate failures to lose clicks
or retries to double-count them. Update the link, workspace, and enrollment
batch handlers to carry their entryIds and report failed entry IDs when writes
fail; then derive processedEntryIds from all input entries by excluding the
union of failures from linkResult, workspaceResult, and enrollmentResult before
RedisStream deletion. Use the existing processInSubBatches flow and the
workspace/program-partner aggregate keys.
- Around line 366-383: Update runWithLock to acquire the lock with a unique
ownership token instead of the constant value, and release it conditionally only
when the stored token still matches before deleting LOCK_KEY. Increase
LOCK_TTL_SECONDS above the 600-second maxDuration, such as 660 seconds, while
preserving the existing skip and executeClickStatsCron flow.
- Around line 198-205: Update the click-stats write in the route’s conn.execute
call to keep lastClicked monotonic using a NULL-safe GREATEST expression, so
older or retried events cannot regress it. Replace the date-fns format
conversion with UTC-based ISO formatting via the update.lastClicked value,
removing the now-unused format dependency while preserving the existing clicks
and linkId parameters.

In `@apps/web/lib/tinybird/record-click.ts`:
- Around line 191-196: Update the publishClickStatsEvent call in the
click-recording flow so workspaceId is included whenever it is available,
independently of url. Preserve url as an optional field handled separately,
ensuring /api/track callers without url still provide workspaceId for workspace
usage accounting.

In `@apps/web/lib/upstash/redis-streams/click-stats.ts`:
- Around line 51-72: Update the Promise.allSettled flow in the click-stats
handler to inspect settled results and propagate an error when any database
write, including the fallback updates, is rejected. Preserve successful
completion and logger.flush behavior, while ensuring record-click.ts can observe
the rejection and log the failure.

---

Nitpick comments:
In `@apps/web/lib/upstash/redis-streams/click-stats.ts`:
- Around line 6-8: Open a tracking issue for migrating the remaining legacy
stream publishers to track-sale → workspace-clicks-usage and
lead/sale/commission → partner-activity, then remove the legacy streams after
migration or confirmation that they remain needed; update the TODO with the
issue reference.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9b5765eb-9d25-49ce-833b-ad840da978d1

📥 Commits

Reviewing files that changed from the base of the PR and between 3083157 and ed2d55f.

📒 Files selected for processing (4)
  • apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
  • apps/web/lib/tinybird/record-click.ts
  • apps/web/lib/upstash/redis-streams/click-stats.ts
  • apps/web/vercel.json

Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts Outdated
Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts Outdated
Comment thread apps/web/lib/tinybird/record-click.ts Outdated
Comment thread apps/web/lib/upstash/redis-streams/link-click-events.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/lib/api/conversions/track-sale.ts`:
- Around line 660-669: Update the sale conversion flow containing
prisma.project.update so the mandatory project.usage increment executes outside
the Promise.allSettled best-effort group. Await it directly and route database
failures through the existing durable retry or alerting mechanism, ensuring the
sale cannot silently ignore an undercount while preserving best-effort handling
for unrelated operations.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e2917ca-b4dd-4514-a540-76cdb1a1a492

📥 Commits

Reviewing files that changed from the base of the PR and between ed2d55f and 52ef123.

📒 Files selected for processing (2)
  • apps/web/lib/api/conversions/track-sale.ts
  • apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts
💤 Files with no reviewable changes (1)
  • apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts

Comment thread apps/web/lib/api/conversions/track-sale.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
apps/web/lib/upstash/redis-streams/link-click-events.ts (2)

51-55: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve the event timestamp in the SQL fallback.

The required timestamp is ignored and lastClicked is set to NOW(). Delayed or replayed clicks can therefore record recovery time instead of click time. Use the supplied timestamp with the same normalization as the stream consumer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/link-click-events.ts` around lines 51 -
55, Update the SQL fallback in the link-click event handler to set lastClicked
from the supplied timestamp rather than NOW(). Apply the same timestamp
normalization used by the stream consumer before binding it in the UPDATE
executed via conn.execute, while leaving the clicks increment and linkId
filtering unchanged.

36-72: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make the publish fallback idempotent to prevent double counting.

If Redis accepts XADD but the client observes a timeout or connection error, this catch performs the SQL increments while the event remains in the stream and is later processed again. Use a stable event/click ID with idempotent deduplication, or route both paths through a durable outbox.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/link-click-events.ts` around lines 36 -
72, Make the fallback in the Redis publish catch block idempotent so an event
accepted by xadd cannot be counted twice when retried or later processed from
the stream. Use a stable event or click identifier tied to the operation and
enforce deduplication across the SQL updates and stream consumer, or route
publishing and counting through the existing durable outbox mechanism; update
the flow around redis.xadd and the fallback Promise.allSettled without changing
normal successful publishing behavior.
apps/web/app/(ee)/api/cron/streams/update-workspace-clicks/route.ts (2)

194-194: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Avoid scanning the entire stream for status reporting.

getStreamInfo() calls XRANGE("-", "+") without a limit, so each cron run can read and materialize the full remaining backlog after processing. Use XLEN plus bounded first/last-entry queries instead; otherwise monitoring becomes an O(backlog) operation and can add significant latency and memory pressure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/`(ee)/api/cron/streams/update-workspace-clicks/route.ts at line
194, Update the status-reporting logic around
workspaceClicksUsageStream.getStreamInfo() to avoid the unbounded stream scan.
Use XLEN for the stream length and bounded first/last-entry queries for
metadata, preserving the existing reported status fields while ensuring
monitoring work remains constant-sized relative to backlog.

109-115: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make stream consumption idempotent before deleting entries.

The SQL increment can commit, then the process can crash before processBatch calls XDEL; the same entries will be replayed and counters will be incremented again. The shared client also swallows XDEL failures, so this is not limited to crashes. Add per-event deduplication/idempotency, or use an atomic persistence-and-acknowledgement design.

Also applies to: 174-177

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/`(ee)/api/cron/streams/update-workspace-clicks/route.ts around
lines 109 - 115, The stream update flow around processBatch must make workspace
click increments idempotent before entries are acknowledged or deleted. Use a
durable per-event deduplication mechanism keyed by each stream entry ID, and
ensure the counter update and deduplication record are committed atomically;
skip already-recorded entries while still marking them processed for
acknowledgement, preserving the existing processedEntryIds/XDEL flow.
apps/web/lib/upstash/redis-streams/workspace-links-usage.ts (1)

24-40: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Do not fall back to a non-idempotent SQL increment after an ambiguous XADD failure.

If Redis accepts the event but the client times out, this branch increments linksUsage and totalLinks; the stream entry may still be processed later and increment both counters again. Remove this fallback or add an idempotency key/deduplication record so an event can be applied only once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/workspace-links-usage.ts` around lines 24
- 40, The error handler in the workspace links usage publisher must not perform
the non-idempotent SQL increment after an ambiguous redis.xadd failure. Remove
the direct conn.execute fallback from the catch block, or replace it with an
idempotency-key and deduplication mechanism that guarantees the usage event is
applied only once.
apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts (1)

16-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Do not describe XADD as a transaction.

This publishes one stream entry with a single Redis command; it does not provide transactional or atomic coordination with other writes. Use “single stream entry” or “single Redis operation” to avoid implying stronger delivery guarantees.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts` at line 16,
Update the comment above the click-event publishing logic to describe it as
publishing a single stream entry or using a single Redis operation, and remove
the claim that it is a transaction.
🧹 Nitpick comments (1)
apps/web/lib/upstash/redis-streams/link-click-events.ts (1)

10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse this exported event type in the cron consumer.

update-click-stats/route.ts currently redeclares the same LinkClickEvent shape. Import this type there so publisher and consumer cannot silently diverge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/upstash/redis-streams/link-click-events.ts` around lines 10 -
16, Remove the duplicate LinkClickEvent declaration from the update-click-stats
cron consumer and import the exported LinkClickEvent type from
link-click-events.ts, using it for the consumer’s event data so the publisher
and consumer share one contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/lib/upstash/redis-streams/link-click-events.ts`:
- Around line 22-28: Update the call site in record-click.ts so workspaceId is
included whenever it is present, independent of the optional url value. Remove
the url condition from the object spread passed to publishLinkClickEvent,
preserving the existing behavior for clicks without a workspaceId.

---

Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/streams/update-workspace-clicks/route.ts:
- Line 194: Update the status-reporting logic around
workspaceClicksUsageStream.getStreamInfo() to avoid the unbounded stream scan.
Use XLEN for the stream length and bounded first/last-entry queries for
metadata, preserving the existing reported status fields while ensuring
monitoring work remains constant-sized relative to backlog.
- Around line 109-115: The stream update flow around processBatch must make
workspace click increments idempotent before entries are acknowledged or
deleted. Use a durable per-event deduplication mechanism keyed by each stream
entry ID, and ensure the counter update and deduplication record are committed
atomically; skip already-recorded entries while still marking them processed for
acknowledgement, preserving the existing processedEntryIds/XDEL flow.

In `@apps/web/lib/upstash/redis-streams/link-click-events.ts`:
- Around line 51-55: Update the SQL fallback in the link-click event handler to
set lastClicked from the supplied timestamp rather than NOW(). Apply the same
timestamp normalization used by the stream consumer before binding it in the
UPDATE executed via conn.execute, while leaving the clicks increment and linkId
filtering unchanged.
- Around line 36-72: Make the fallback in the Redis publish catch block
idempotent so an event accepted by xadd cannot be counted twice when retried or
later processed from the stream. Use a stable event or click identifier tied to
the operation and enforce deduplication across the SQL updates and stream
consumer, or route publishing and counting through the existing durable outbox
mechanism; update the flow around redis.xadd and the fallback Promise.allSettled
without changing normal successful publishing behavior.

In `@apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts`:
- Line 16: Update the comment above the click-event publishing logic to describe
it as publishing a single stream entry or using a single Redis operation, and
remove the claim that it is a transaction.

In `@apps/web/lib/upstash/redis-streams/workspace-links-usage.ts`:
- Around line 24-40: The error handler in the workspace links usage publisher
must not perform the non-idempotent SQL increment after an ambiguous redis.xadd
failure. Remove the direct conn.execute fallback from the catch block, or
replace it with an idempotency-key and deduplication mechanism that guarantees
the usage event is applied only once.

---

Nitpick comments:
In `@apps/web/lib/upstash/redis-streams/link-click-events.ts`:
- Around line 10-16: Remove the duplicate LinkClickEvent declaration from the
update-click-stats cron consumer and import the exported LinkClickEvent type
from link-click-events.ts, using it for the consumer’s event data so the
publisher and consumer share one contract.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 00f2a5e2-5f98-4c3d-b4aa-a03355f37495

📥 Commits

Reviewing files that changed from the base of the PR and between 3e6a96e and 3126ea6.

📒 Files selected for processing (8)
  • apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
  • apps/web/app/(ee)/api/cron/streams/update-workspace-clicks/route.ts
  • apps/web/lib/tinybird/record-click.ts
  • apps/web/lib/upstash/redis-streams/link-click-events.ts
  • apps/web/lib/upstash/redis-streams/partner-activity.ts
  • apps/web/lib/upstash/redis-streams/workspace-click-events.ts
  • apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts
  • apps/web/lib/upstash/redis-streams/workspace-links-usage.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/web/lib/upstash/redis-streams/partner-activity.ts
  • apps/web/lib/tinybird/record-click.ts
  • apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts

Comment thread apps/web/lib/upstash/redis-streams/link-click-events.ts
@steven-tey

Copy link
Copy Markdown
Collaborator

@coderabbitai final review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Current PR metadata ==='
gh pr view 4231 --repo dubinc/dub --json number,title,headRefOid,baseRefOid,updatedAt,files \
  --jq '{number,title,headRefOid,baseRefOid,updatedAt,files:[.files[].path]}'

echo '=== Current merge-base diff ==='
base="$(gh pr view 4231 --repo dubinc/dub --json baseRefName --jq .baseRefName)"
git fetch --quiet origin "$base"
git diff --find-renames --unified=50 "origin/$base...HEAD" -- \
  'apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts' \
  'apps/web/lib/upstash/redis-streams/link-click-events.ts' \
  'apps/web/lib/upstash/redis-streams/client.ts' \
  'apps/web/lib/tinybird/record-click.ts' \
  'apps/web/app/(ee)/api/track/application/route.ts' \
  'apps/web/lib/api/conversions/track-sale.ts' \
  'apps/web/vercel.json'

echo '=== Outlines ==='
ast-grep outline 'apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts' --items all
ast-grep outline 'apps/web/lib/upstash/redis-streams/client.ts' --items all
ast-grep outline 'apps/web/lib/upstash/redis-streams/link-click-events.ts' --items all

echo '=== Stream processing and lock/QStash contexts ==='
rg -n -C 18 -e '\b(processBatch|getStreamInfo|processedEntryIds|qstash\.publishJSON|lock:|redis\.set|redis\.del|xrange)\b' \
  'apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts' \
  'apps/web/lib/upstash/redis-streams/client.ts'

echo '=== All link click publishers and event consumers ==='
rg -n -C 8 -e '\b(publishLinkClickEvent|linkClickEventStream)\b' apps/web \
  --glob '*.ts' --glob '*.tsx'

Length of output: 86073


@steven-tey Final review complete — I would not merge yet. The prior blocking issues are still present in the current head:

  1. Events can be acknowledged before all counters are updated. In apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts, Line 206 adds all event IDs for a link to processedEntryIds immediately after the Link update succeeds. The workspace and enrollment updates happen afterward. If either later update fails, processBatch() deletes the event, permanently undercounting that workspace or partner.

    The inverse failure mode also exists: if the Link update fails but workspace/enrollment updates succeed, the event remains for retry and those successful += updates are applied again, overcounting them. Acknowledgement/retry must be atomic across all derived counters, or each destination update needs an idempotency mechanism keyed by stream entry.

  2. QStash backlog chaining can still be lost. Lines 334–340 enqueue the next POST while the Redis lock remains held until Line 381. A quickly delivered QStash request receives the successful “another run is in progress” no-op response, so it is not retried. Backlog draining can therefore degrade to the once-per-minute Vercel cron cadence.

  3. Backlog inspection still performs an unbounded stream read. apps/web/lib/upstash/redis-streams/client.ts Line 100 calls XRANGE without a count merely to determine stream length. Since this runs after every batch, its cost grows with the backlog precisely when the consumer is under pressure. Use XLEN (plus bounded first/last entry reads) instead.

No public API or schema compatibility issues were introduced, but the counter updates are now eventually consistent. Immediate consumers of link, workspace, or partner click totals must tolerate the delay.

I did not execute repository tests because the environment does not allow running project code.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/`(ee)/api/cron/streams/update-click-stats/route.ts:
- Around line 331-340: Move the continuation scheduling out of the lock-held
section: have executeClickStatsCron return or otherwise expose the hasMore
result, release LOCK_KEY via the existing finally/runWithLock flow, then call
qstash.publishJSON only after the lock is released. Preserve the current
continuation condition and payload.
- Around line 52-57: Update aggregateClickStats to collect the IDs of entries
skipped for missing linkId in a skippedEntryIds result, return them alongside
the aggregates, and initialize processedEntryIds with those IDs before adding
update.entryIds. Ensure skipped entries are included in xdel acknowledgment so
malformed entries cannot keep the stream and QStash chain active.

In `@apps/web/lib/api/conversions/track-sale.ts`:
- Around line 660-669: Remove the mandatory prisma.project.update usage
increment from the Promise.allSettled batch in the track-sale flow, and await it
through an error-propagating path before caching the invoice response. Preserve
Promise.allSettled only for non-critical side effects so project.usage failures
reject the request and cannot be silently undercounted.

In `@apps/web/vercel.json`:
- Around line 27-30: Before enabling the per-minute schedule in the Vercel
configuration, update the `/api/cron/streams/update-click-stats` handler so
QStash POST requests encountering the shared lock return a retryable failure
instead of a successful no-op. Preserve the existing GET behavior, and ensure
continuation ownership prevents acknowledgments when processing was skipped.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 36297b64-3336-4b10-b5bf-67e5f6688746

📥 Commits

Reviewing files that changed from the base of the PR and between 3083157 and 914237c.

📒 Files selected for processing (13)
  • apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
  • apps/web/app/(ee)/api/cron/streams/update-workspace-clicks/route.ts
  • apps/web/app/(ee)/api/track/application/route.ts
  • apps/web/lib/api/conversions/track-sale.ts
  • apps/web/lib/api/partners/sync-partner-links-stats.ts
  • apps/web/lib/tinybird/record-click.ts
  • apps/web/lib/upstash/redis-streams/link-click-events.ts
  • apps/web/lib/upstash/redis-streams/partner-activity.ts
  • apps/web/lib/upstash/redis-streams/workspace-click-events.ts
  • apps/web/lib/upstash/redis-streams/workspace-clicks-usage.ts
  • apps/web/lib/upstash/redis-streams/workspace-links-usage.ts
  • apps/web/scripts/partners/aggregate-stats-seeding.ts
  • apps/web/vercel.json

Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts
Comment thread apps/web/app/(ee)/api/cron/streams/update-click-stats/route.ts Outdated
Comment thread apps/web/lib/api/conversions/track-sale.ts
Comment thread apps/web/vercel.json
@steven-tey steven-tey changed the title Batch click stats updates through a Redis stream cron. Batch click stats updates through a Redis stream cron Jul 28, 2026
@steven-tey
steven-tey merged commit cd8caef into main Jul 28, 2026
11 checks passed
@steven-tey
steven-tey deleted the click-stream-updates branch July 28, 2026 20:21
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