feat(apify): digest rate cap (1 per artist per 24h) + email_send_log audit trail#762
feat(apify): digest rate cap (1 per artist per 24h) + email_send_log audit trail#762sweetmantech wants to merge 5 commits into
Conversation
… posts first The Instagram scrape alert fired whenever a scrape returned posts, with no comparison against posts already stored — every scrape re-announced the profile's recent feed as new (observed: 6 of 7 alerts in one day announced posts up to 10 days old). New reusable filterNewPostUrls diffs candidate URLs against posts BEFORE upsert; the alert is gated on a non-empty result. Persistence unchanged (recoupable/chat#1855, PR #3 of 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A roster scrape starts one Apify run per platform, each completing independently — extending per-platform alerts would mean 4+ emails per scrape. This registers every run under a batch_id at scrape start (apify_scraper_runs, columns from recoupable/database#41), records each webhook completion with its genuinely-new post URLs, and when the batch's last run completes sends ONE digest (per-platform sections, BCC-only) via the new digest module. Platforms with nothing new are omitted; a batch with nothing new sends nothing. Instagram's solo alert is suppressed for batch runs; legacy/non-batch runs keep today's behavior (recoupable/chat#1855, PR #4 of 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…new post Replaces the per-send LLM email body (nondeterministic branding, vendor jargon reaching customers, no reliable post links) with a shared deterministic renderer: stable subject/branding, one section per platform, a direct link to every genuinely-new post, chat CTA secondary. Used by both the batch digest and the legacy solo Instagram alert, which now also requires new-post URLs and is BCC-only (recoupable/chat#1855, PR #5 of 6; supersedes the interim body from PR #4 and, for this file, the standalone BCC fix in api#758 — same invariant, tests included). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Caps the digest at one per watched artist entity per 24h and records every digest send in email_send_log (the solo alert path bypassed the log entirely — the 7-email burst on chat#1855 was only discoverable via the Resend admin endpoint). Recipients resolver now also returns the watched artist entity ids for the cap key (recoupable/chat#1855, PR #6 of 6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe scrape digest pipeline now returns recipient emails with artist IDs, prevents duplicate and overly frequent sends through email-log checks, records successful attempts, and makes scraper completion updates idempotent. ChangesScrape digest delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant maybeSendScrapeDigest
participant selectScrapeDigestLogs
participant sendScrapeDigestEmail
participant logEmailAttempt
maybeSendScrapeDigest->>selectScrapeDigestLogs: Check batch and recent artist logs
selectScrapeDigestLogs-->>maybeSendScrapeDigest: Return matching records
maybeSendScrapeDigest->>sendScrapeDigestEmail: Send eligible digest
sendScrapeDigestEmail-->>maybeSendScrapeDigest: Return send result
maybeSendScrapeDigest->>logEmailAttempt: Record each artist send
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 2/5
- In
lib/apify/digest/maybeSendScrapeDigest.ts,sendScrapeDigestEmailcan record failed Resend attempts as successfully sent becausesendEmailWithResendreturns a truthy errorNextResponsewithout anid; merging as-is risks inaccurate audit logs and missed retries/triage for real delivery failures — update the success check to require a valid messageid(or explicit success flag) before writing a "sent" audit entry.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/apify/digest/maybeSendScrapeDigest.ts">
<violation number="1" location="lib/apify/digest/maybeSendScrapeDigest.ts:39">
P1: Failed email sends are logged as "sent" in the audit trail. `sendScrapeDigestEmail` calls `sendEmailWithResend`, which returns a `NextResponse` error object (truthy, but no `id`) when Resend returns an error. The `if (sent)` guard incorrectly treats this as a successful delivery, so `logEmailAttempt` records `status: "sent"` even though the email was never sent. The `resendId` is `undefined` in this case, so the audit trail has rows claiming a successful send with a null Resend ID — no way to distinguish real deliveries from API failures. Fix: gate on `resendId` instead of `sent`, since a successful Resend response always has an `id`.</violation>
</file>
Architecture diagram
sequenceDiagram
participant M as maybeSendScrapeDigest
participant R as getScrapeDigestRecipients
participant C as selectRecentScrapeDigestLogs
participant E as sendScrapeDigestEmail
participant L as logEmailAttempt
participant DB as Supabase (email_send_log)
Note over M,DB: Scrape digest orchestration with rate cap & audit trail
M->>M: Fetch batch runs & extract sections of new posts
alt No new posts in batch
M->>M: Return null immediately
else New posts found
M->>R: getScrapeDigestRecipients(socialIds)
R-->>M: { emails, artistIds }
Note over M,C: Rate cap: check if any artist was already alerted in last 24h
M->>C: selectRecentScrapeDigestLogs(artistIds, since)
C->>DB: SELECT account_id, created_at FROM email_send_log WHERE account_id IN artistIds AND raw_body LIKE 'scrape-digest:%' AND created_at >= since
DB-->>C: matching rows (or empty)
C-->>M: recent log rows
alt Recent log exists for any artist
M->>M: Return null (skip send)
else No recent log
M->>E: sendScrapeDigestEmail({ emails, sections })
E-->>M: Resend response (id)
Note over M,L: Audit trail: record send per watched artist entity
loop For each artistId
M->>L: logEmailAttempt({ rawBody: scrape-digest:{batchId}, status: sent, accountId: artistId, resendId})
L->>DB: INSERT into email_send_log
DB-->>L: ok
end
M->>M: Return sent response
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // Audit trail: record the send per artist entity so alerts are debuggable | ||
| // and the cap has something to read (the solo path never logged at all). | ||
| const resendId = (sent as { id?: string } | null)?.id; | ||
| if (sent) { |
There was a problem hiding this comment.
P1: Failed email sends are logged as "sent" in the audit trail. sendScrapeDigestEmail calls sendEmailWithResend, which returns a NextResponse error object (truthy, but no id) when Resend returns an error. The if (sent) guard incorrectly treats this as a successful delivery, so logEmailAttempt records status: "sent" even though the email was never sent. The resendId is undefined in this case, so the audit trail has rows claiming a successful send with a null Resend ID — no way to distinguish real deliveries from API failures. Fix: gate on resendId instead of sent, since a successful Resend response always has an id.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/maybeSendScrapeDigest.ts, line 39:
<comment>Failed email sends are logged as "sent" in the audit trail. `sendScrapeDigestEmail` calls `sendEmailWithResend`, which returns a `NextResponse` error object (truthy, but no `id`) when Resend returns an error. The `if (sent)` guard incorrectly treats this as a successful delivery, so `logEmailAttempt` records `status: "sent"` even though the email was never sent. The `resendId` is `undefined` in this case, so the audit trail has rows claiming a successful send with a null Resend ID — no way to distinguish real deliveries from API failures. Fix: gate on `resendId` instead of `sent`, since a successful Resend response always has an `id`.</comment>
<file context>
@@ -20,8 +22,31 @@ export async function maybeSendScrapeDigest(batchId: string | null | undefined)
+ // Audit trail: record the send per artist entity so alerts are debuggable
+ // and the cap has something to read (the solo path never logged at all).
+ const resendId = (sent as { id?: string } | null)?.id;
+ if (sent) {
+ await Promise.all(
+ artistIds.map(artistId =>
</file context>
| if (sent) { | |
| if (resendId) { |
…dit onto the shipped template, add exact non-duplication Review question (sweetman): why a rate cap instead of properly enforcing non-duplication? Both, now, as separate concerns: - Completion claim: updateApifyScraperRun updates WHERE completed_at IS NULL — a webhook retry can't re-claim a completed run, re-trigger digest assembly, or overwrite the recorded diff (the 3->0 corruption observed live during #760 testing). - Batch idempotency: maybeSendScrapeDigest checks email_send_log for an existing scrape-digest:<batchId> row before sending — one digest per batch, ever, no time-window heuristics. - The 24h per-artist cap remains as the product frequency rule from the issue spec, reading the same audit rows. Every send is logged per watched artist (rawBody scrape-digest:<batchId>). Also deletes 4 orphaned pre-rename supabase files that rode in again via the merge (same both-added mechanism as #761) and supersedes the branch's selectRecentScrapeDigestLogs with filter-object selectScrapeDigestLogs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Ported onto
Every send is logged per watched artist entity (this also closes the 'alert sends bypass the audit log' evidence item). Merge also removed 4 orphaned pre-rename supabase files that rode in again (same both-added mechanism as #761). TDD: 6 assembler cases (incl. batch-dedup and cap suppression) + 3 claim-guard tests; 470 tests green across the affected domains; tsc + lint clean. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@lib/apify/digest/maybeSendScrapeDigest.ts`:
- Around line 34-45: Make the idempotency check and audit-log reservation atomic
in maybeSendScrapeDigest: reserve the batch before sendScrapeDigestEmail using a
pending audit status, or acquire a database-level batch lock/constraint so
concurrent runs cannot both proceed. Update the reservation to sent after
successful delivery and handle failed sends appropriately, using
selectScrapeDigestLogs and logEmailAttempt as the integration points.
- Around line 47-48: Update the audit-trail comment near the batch deduplication
logic to replace “watched artist entity” with the appropriate account-specific
terminology, such as “watched artist account,” while preserving the comment’s
meaning.
In `@lib/supabase/email_send_log/selectScrapeDigestLogs.ts`:
- Line 7: Update the JSDoc for the artistIds parameter to replace “watched
artist entities” with “watched artists” or “watched artist accounts,” following
the project’s account terminology guidelines.
- Around line 10-18: Add an explicit return type to selectScrapeDigestLogs using
the email send log table’s Tables type, with Pick limited to account_id and
created_at, and ensure the implementation returns that typed result.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c991acdc-36c1-4039-bfc7-443148ad9cf6
⛔ Files ignored due to path filters (2)
lib/apify/digest/__tests__/maybeSendScrapeDigest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/apify_scraper_runs/__tests__/updateApifyScraperRun.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
lib/apify/digest/getScrapeDigestRecipients.tslib/apify/digest/maybeSendScrapeDigest.tslib/supabase/apify_scraper_runs/updateApifyScraperRun.tslib/supabase/email_send_log/selectScrapeDigestLogs.ts
| // Exact non-duplication: one digest per batch, ever. | ||
| const already = await selectScrapeDigestLogs({ batchId }); | ||
| if (already.length > 0) return null; | ||
|
|
||
| // Product frequency cap: at most one digest per watched artist per 24h. | ||
| const since = new Date(Date.now() - 24 * 60 * 60 * 1000).toISOString(); | ||
| const recent = await selectScrapeDigestLogs({ artistIds, since }); | ||
| if (recent.length > 0) return null; | ||
|
|
||
| const artistName = sections.map(s => s.artistName).find(Boolean) ?? null; | ||
| return await sendScrapeDigestEmail({ emails, sections, artistName }); | ||
| const sent = await sendScrapeDigestEmail({ emails, sections, artistName }); | ||
| if (!sent) return null; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
TOCTOU race between idempotency check and audit log write can cause duplicate sends.
The batch idempotency check (selectScrapeDigestLogs({ batchId }) on line 35) and the audit log write (logEmailAttempt on lines 50-58) are not atomic. While updateApifyScraperRun prevents the same run from being claimed twice, two different runs in the same batch completing concurrently can both pass the idempotency check before either writes audit entries. The race window includes the email send (a network call to Resend), widening the vulnerability window.
Consider writing audit entries before sending (e.g., with a pending status, updated to sent after successful delivery), or using a database-level constraint/advisory lock on the batch to ensure atomicity.
🤖 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 `@lib/apify/digest/maybeSendScrapeDigest.ts` around lines 34 - 45, Make the
idempotency check and audit-log reservation atomic in maybeSendScrapeDigest:
reserve the batch before sendScrapeDigestEmail using a pending audit status, or
acquire a database-level batch lock/constraint so concurrent runs cannot both
proceed. Update the reservation to sent after successful delivery and handle
failed sends appropriately, using selectScrapeDigestLogs and logEmailAttempt as
the integration points.
| // Audit trail: record the send per watched artist entity — what the batch | ||
| // dedup and the 24h cap read (the legacy solo path never logged at all). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace "entity" terminology per coding guidelines.
The comment uses "watched artist entity" which violates the guideline to never use entity when referring to system actors.
As per coding guidelines: "Use account terminology, never entity or user, when naming IDs or referring to system actors; use specific terms like artist, workspace, or organization when appropriate."
✏️ Proposed fix
- // Audit trail: record the send per watched artist entity — what the batch
+ // Audit trail: record the send per watched artist account — what the batch📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Audit trail: record the send per watched artist entity — what the batch | |
| // dedup and the 24h cap read (the legacy solo path never logged at all). | |
| // Audit trail: record the send per watched artist account — what the batch |
🤖 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 `@lib/apify/digest/maybeSendScrapeDigest.ts` around lines 47 - 48, Update the
audit-trail comment near the batch deduplication logic to replace “watched
artist entity” with the appropriate account-specific terminology, such as
“watched artist account,” while preserving the comment’s meaning.
Source: Coding guidelines
| * Select scrape-digest send-log rows with optional filters. | ||
| * | ||
| * @param batchId - Rows recording exactly this batch's send (idempotency check). | ||
| * @param artistIds - Rows for any of these watched artist entities. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace "entity" terminology per coding guidelines.
The JSDoc uses "watched artist entities" which violates the guideline to never use entity when referring to system actors. Use "artists" or "artist accounts" instead.
As per coding guidelines: "Use account terminology, never entity or user, when naming IDs or referring to system actors; use specific terms like artist, workspace, or organization when appropriate."
✏️ Proposed fix
- * `@param` artistIds - Rows for any of these watched artist entities.
+ * `@param` artistIds - Rows for any of these watched artist accounts.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @param artistIds - Rows for any of these watched artist entities. | |
| * `@param` artistIds - Rows for any of these watched artist accounts. |
🤖 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 `@lib/supabase/email_send_log/selectScrapeDigestLogs.ts` at line 7, Update the
JSDoc for the artistIds parameter to replace “watched artist entities” with
“watched artists” or “watched artist accounts,” following the project’s account
terminology guidelines.
Source: Coding guidelines
| export async function selectScrapeDigestLogs({ | ||
| batchId, | ||
| artistIds, | ||
| since, | ||
| }: { | ||
| batchId?: string; | ||
| artistIds?: string[]; | ||
| since?: string; | ||
| } = {}) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an explicit return type annotation.
The function lacks a return type annotation. Path instructions require "Return typed results using Tables<"table_name">". Since only account_id and created_at are selected, use Pick for an accurate typed result.
As per path instructions: "Return typed results using Tables<"table_name">".
✏️ Proposed fix
-export async function selectScrapeDigestLogs({
+export async function selectScrapeDigestLogs({
batchId,
artistIds,
since,
}: {
batchId?: string;
artistIds?: string[];
since?: string;
} = {}) {
+): Promise<Pick<Tables<"email_send_log">, "account_id" | "created_at">[]> {🤖 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 `@lib/supabase/email_send_log/selectScrapeDigestLogs.ts` around lines 10 - 18,
Add an explicit return type to selectScrapeDigestLogs using the email send log
table’s Tables type, with Pick limited to account_id and created_at, and ensure
the implementation returns that typed result.
Source: Path instructions
Preview verification — 2026-07-09Preview:
This closes the last two evidence items from chat#1855: the 7-emails/day class of failure (cap + exact dedup) and "alert sends bypass the email audit log" (every digest send now writes Fixture fully cleaned, including the test's own audit-log rows (0 posts/socials/runs/logs/artist rows remaining). Verdict: non-duplication is enforced by identity (completion claim + batch idempotency) with the 24h cap as the separate product frequency rule — ready to merge, closing the chat#1855 chain. |
1 participant
Summary
The last slice of recoupable/chat#1855 (PR #6 of 6, stacked on api#761): even with the newness diff, repeated scrapes of an artist in one day (agent runs, verification, retries) could digest repeatedly — evidence on the issue: 7 alerts to one customer in a day. This caps digests at one per watched artist entity per 24h and gives them an audit trail: the old alert path called Resend directly and left zero rows in
email_send_log, so the platform had no record of who was alerted when.What changed
lib/supabase/email_send_log/selectRecentScrapeDigestLogs.ts(new): digest-log lookback by artist ids (raw_bodyprefixscrape-digest:).getScrapeDigestRecipientsnow returns{ emails, artistIds }— the artist entity ids are the cap key.maybeSendScrapeDigest: skips when any watched artist digested within 24h; on send, records oneemail_send_logrow per artist entity (scrape-digest:<batchId>, resend id) via the existing best-effortlogEmailAttempt.Tests (RED→GREEN)
logEmailAttemptwith artist id +sent.lib/apify+lib/socials+lib/artist→ 284 tests passed; tsc at exact baseline parity (200); eslint clean.Stack
database#41 → api#759 → api#760 → api#761 → this. Done-when for the chain (from the issue): a full artist scrape produces exactly one digest listing each platform's new posts; re-running back-to-back produces none the second time.
🤖 Generated with Claude Code
Summary by cubic
Cap scrape digests to one per watched artist per 24h, log every send to
email_send_log, and enforce exact non-duplication per batch. This prevents multi-send bursts (chat#1855) and makes sends traceable.maybeSendScrapeDigest: skips if ascrape-digest:<batchId>row exists viaselectScrapeDigestLogs({ batchId }).selectScrapeDigestLogs({ artistIds, since }), keyed byartistIdsfromgetScrapeDigestRecipients.email_send_logrow per artist withraw_bodyscrape-digest:<batchId>vialogEmailAttempt.updateApifyScraperRunupdates only wherecompleted_at IS NULLso webhook retries can’t re-trigger a digest.Written for commit c10c383. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes