feat(apify): one consolidated new-posts digest per scrape batch#760
Open
sweetmantech wants to merge 1 commit into
Open
feat(apify): one consolidated new-posts digest per scrape batch#760sweetmantech wants to merge 1 commit into
sweetmantech wants to merge 1 commit into
Conversation
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>
Contributor
|
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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
5 tasks
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 21 files
Confidence score: 3/5
- In
lib/supabase/apify_scraper_runs/selectApifyScraperRunsByBatch.ts, query failures are converted to[], sopostArtistSocialsScrapeHandlercan treat a DB error as “no runs” and skip/suppress the digest path; this is the highest regression risk because failures become invisible—return/throw an explicit error (or status) and handle it upstream before merging. lib/apify/digest/maybeSendScrapeDigest.tscan send the same consolidated digest multiple times when webhooks are replayed after batch completion, which can spam users and create inconsistent notifications—add an idempotent, atomic send gate (e.g., single update/check) before merge.lib/apify/digest/getScrapeDigestRecipients.tsonly reads the first page, so socials with >10,000 links can silently miss recipients and under-notify watchers—paginate or move this to a DB-side joined lookup so recipient selection is complete.- Current tests don’t cover key new wiring in
lib/artist/__tests__/postArtistSocialsScrapeHandler.test.ts(batch registration flow) or thefilterNewPostUrlsintegration inlib/apify/twitter/__tests__/handleTwitterProfileScraperResults.test.ts, so regressions in digest-driving behavior are easier to ship—add focused assertions for those paths before merging.
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/supabase/apify_scraper_runs/selectApifyScraperRunsByBatch.ts">
<violation number="1" location="lib/supabase/apify_scraper_runs/selectApifyScraperRunsByBatch.ts:8">
P1: Silent error swallowing in batch query can suppress the digest. When the Supabase query fails, the function returns an empty array, making it indistinguishable from an empty batch. The caller treats `runs.length === 0` as "batch not ready" and returns null without re-raising or alerting — so a transient DB error on the last sibling's completion webhook silently drops the consolidated digest. Consider either propagating the error (so the caller can retry) or adding a dedicated recovery path (e.g., a queue retry or alert) for query failures.</violation>
</file>
<file name="lib/apify/digest/maybeSendScrapeDigest.ts">
<violation number="1" location="lib/apify/digest/maybeSendScrapeDigest.ts:26">
P2: A completed batch can send the consolidated digest more than once if the webhook for any registered run is delivered or replayed after all siblings are complete. Consider making the send gate idempotent with an atomic persisted marker/claim for the batch before calling `sendScrapeDigestEmail`.</violation>
</file>
<file name="lib/apify/twitter/__tests__/handleTwitterProfileScraperResults.test.ts">
<violation number="1" location="lib/apify/twitter/__tests__/handleTwitterProfileScraperResults.test.ts:5">
P2: The handler now calls `filterNewPostUrls` and includes `newPostUrls` in its return value (drives the digest email), but the tests don't verify that integration. Consider adding an assertion on the `filterNewPostUrls` call args and on `newPostUrls` in the return value so the mock isn't purely ornamental — the digest behavior depends on correct new-post tracking.</violation>
</file>
<file name="lib/apify/digest/getScrapeDigestRecipients.ts">
<violation number="1" location="lib/apify/digest/getScrapeDigestRecipients.ts:16">
P2: Digest recipients can be silently dropped for a social watched by more than 10,000 account links because this query reads only the first page. Consider paginating or moving this lookup into a DB-side joined query rather than encoding a larger fixed cap.
(Based on your team's feedback about DB-side pagination over raised Supabase limits.) [FEEDBACK_USED]</violation>
</file>
<file name="lib/apify/digest/sendScrapeDigestEmail.ts">
<violation number="1" location="lib/apify/digest/sendScrapeDigestEmail.ts:26">
P2: Post URLs and platform names are interpolated into the email HTML body without any escaping. Both the href attribute and visible text content for each link use the raw URL directly, and the platform name goes into an `<h3>` element. A URL containing a `"` would break the href attribute; a URL or platform with `<` or `>` could inject arbitrary HTML into the email. Consider encoding the URL for the href attribute (e.g., wrapping with a helper that escapes `&`, `"`, `<`, `>`) and optionally encoding visible text too, as a defense-in-depth measure.</violation>
</file>
<file name="lib/artist/__tests__/postArtistSocialsScrapeHandler.test.ts">
<violation number="1" location="lib/artist/__tests__/postArtistSocialsScrapeHandler.test.ts:12">
P1: The batch registration logic added to `postArtistSocialsScrapeHandler` (batchId generation, socialByUrl mapping, insertApifyScraperRuns call) has no test coverage in the handler's own test file. The test file was updated with the required mock and matching return shapes, but no `it(...)` block verifies the new behavior. This leaves the digest pipeline's registration step untested, so regressions — wrong account_id, wrong platform derivation, missing social_id, or a broken batchId — would go undetected in CI. Consider adding test cases that assert on the arguments passed to `insertApifyScraperRuns`, verify the response body omits profileUrl, and confirm the call is skipped in early-return paths (auth failure, 403, 402).</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant ScrapeHandler as postArtistSocialsScrapeHandler
participant ScrapeBatch as scrapeProfileUrlBatch
participant DB as Supabase DB
participant Apify as Apify (external)
participant WebhookHandler as apifyWebhookHandler
participant ResultHandler as per‑platform result handler
participant Digest as maybeSendScrapeDigest
participant EmailService as digest email service
rect rgb(240, 240, 240)
Note over Client,ScrapeHandler: Phase 1 — Batch initialization
end
Client->>ScrapeHandler: POST /artist/socials/scrape (artist_id, socials)
ScrapeHandler->>ScrapeBatch: CHANGED: scrapeProfileUrlBatch(socials)
ScrapeBatch->>Apify: start scrape per social
Apify-->>ScrapeBatch: runId, datasetId (per social)
ScrapeBatch-->>ScrapeHandler: CHANGED: results include profileUrl
ScrapeHandler->>DB: NEW: insertApifyScraperRuns(runs with batch_id, social_id, platform)
alt registration succeeds
DB-->>ScrapeHandler: ok
else registration fails
DB-->>ScrapeHandler: error (logged, scrape continues)
end
Note over ScrapeHandler: strips profileUrl from response (contract unchanged)
ScrapeHandler-->>Client: 200 {runId, datasetId, error}[]
rect rgb(245, 245, 245)
Note over Apify,WebhookHandler: Phase 2 — Webhook processing (each run)
end
Apify->>WebhookHandler: POST /api/apify (webhook for run‑X)
alt payload has no runId
Note over WebhookHandler: skip digest bookkeeping (legacy path)
else runId present
WebhookHandler->>ResultHandler: dispatch per actor type
Note over ResultHandler: e.g., handleInstagramProfileScraperResults
alt Instagram with batch_id
ResultHandler->>ResultHandler: NEW: suppress solo email (skip sendApifyWebhookEmail)
end
ResultHandler->>DB: NEW: filterNewPostUrls (pre‑persist diff)
DB-->>ResultHandler: newPostUrls[]
ResultHandler-->>WebhookHandler: CHANGED: {posts, newPostUrls, ...}
WebhookHandler->>DB: NEW: completeApifyScraperRun(runId, newPostUrls)
DB-->>WebhookHandler: updated row (with batch_id)
WebhookHandler->>Digest: NEW: maybeSendScrapeDigest(batch_id)
Digest->>DB: NEW: selectApifyScraperRunsByBatch(batch_id)
DB-->>Digest: all runs in batch
alt any sibling incomplete
Note over Digest: wait for last run
Digest-->>WebhookHandler: null (no email)
else all completed
alt at least one platform has new posts
Digest->>EmailService: NEW: getScrapeDigestRecipients(socialIds)
EmailService->>DB: selectAccountSocials → getAccountArtistIds → selectAccountEmails
DB-->>EmailService: watcher email addresses
EmailService-->>Digest: [watcher@...]
Digest->>EmailService: NEW: sendScrapeDigestEmail(emails, sections)
Note over EmailService: BCC‑only, per‑platform sections
EmailService-->>Digest: email id
Digest-->>WebhookHandler: email id
else no new posts anywhere
Note over Digest: skip (sends nothing)
Digest-->>WebhookHandler: null
end
end
end
Note over WebhookHandler: Always return 200 (digest errors caught & logged)
WebhookHandler-->>Apify: 200 OK
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export async function selectApifyScraperRunsByBatch( | ||
| batchId: string, | ||
| ): Promise<ApifyScraperRunRow[]> { | ||
| const { data, error } = await supabase |
There was a problem hiding this comment.
P1: Silent error swallowing in batch query can suppress the digest. When the Supabase query fails, the function returns an empty array, making it indistinguishable from an empty batch. The caller treats runs.length === 0 as "batch not ready" and returns null without re-raising or alerting — so a transient DB error on the last sibling's completion webhook silently drops the consolidated digest. Consider either propagating the error (so the caller can retry) or adding a dedicated recovery path (e.g., a queue retry or alert) for query failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/apify_scraper_runs/selectApifyScraperRunsByBatch.ts, line 8:
<comment>Silent error swallowing in batch query can suppress the digest. When the Supabase query fails, the function returns an empty array, making it indistinguishable from an empty batch. The caller treats `runs.length === 0` as "batch not ready" and returns null without re-raising or alerting — so a transient DB error on the last sibling's completion webhook silently drops the consolidated digest. Consider either propagating the error (so the caller can retry) or adding a dedicated recovery path (e.g., a queue retry or alert) for query failures.</comment>
<file context>
@@ -0,0 +1,17 @@
+export async function selectApifyScraperRunsByBatch(
+ batchId: string,
+): Promise<ApifyScraperRunRow[]> {
+ const { data, error } = await supabase
+ .from("apify_scraper_runs" as never)
+ .select("*")
</file context>
| import { ensureSocialScrapeCredits } from "@/lib/socials/ensureSocialScrapeCredits"; | ||
| import { deductSocialScrapeCredits } from "@/lib/socials/deductSocialScrapeCredits"; | ||
|
|
||
| vi.mock("@/lib/supabase/apify_scraper_runs/insertApifyScraperRuns", () => ({ |
There was a problem hiding this comment.
P1: The batch registration logic added to postArtistSocialsScrapeHandler (batchId generation, socialByUrl mapping, insertApifyScraperRuns call) has no test coverage in the handler's own test file. The test file was updated with the required mock and matching return shapes, but no it(...) block verifies the new behavior. This leaves the digest pipeline's registration step untested, so regressions — wrong account_id, wrong platform derivation, missing social_id, or a broken batchId — would go undetected in CI. Consider adding test cases that assert on the arguments passed to insertApifyScraperRuns, verify the response body omits profileUrl, and confirm the call is skipped in early-return paths (auth failure, 403, 402).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/artist/__tests__/postArtistSocialsScrapeHandler.test.ts, line 12:
<comment>The batch registration logic added to `postArtistSocialsScrapeHandler` (batchId generation, socialByUrl mapping, insertApifyScraperRuns call) has no test coverage in the handler's own test file. The test file was updated with the required mock and matching return shapes, but no `it(...)` block verifies the new behavior. This leaves the digest pipeline's registration step untested, so regressions — wrong account_id, wrong platform derivation, missing social_id, or a broken batchId — would go undetected in CI. Consider adding test cases that assert on the arguments passed to `insertApifyScraperRuns`, verify the response body omits profileUrl, and confirm the call is skipped in early-return paths (auth failure, 403, 402).</comment>
<file context>
@@ -9,6 +9,9 @@ import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess
import { ensureSocialScrapeCredits } from "@/lib/socials/ensureSocialScrapeCredits";
import { deductSocialScrapeCredits } from "@/lib/socials/deductSocialScrapeCredits";
+vi.mock("@/lib/supabase/apify_scraper_runs/insertApifyScraperRuns", () => ({
+ insertApifyScraperRuns: vi.fn(async () => ({ data: null, error: null })),
+}));
</file context>
| const emails = await getScrapeDigestRecipients( | ||
| runs.map(r => r.social_id).filter((id): id is string => Boolean(id)), | ||
| ); | ||
| return await sendScrapeDigestEmail({ emails, sections }); |
There was a problem hiding this comment.
P2: A completed batch can send the consolidated digest more than once if the webhook for any registered run is delivered or replayed after all siblings are complete. Consider making the send gate idempotent with an atomic persisted marker/claim for the batch before calling sendScrapeDigestEmail.
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 26:
<comment>A completed batch can send the consolidated digest more than once if the webhook for any registered run is delivered or replayed after all siblings are complete. Consider making the send gate idempotent with an atomic persisted marker/claim for the batch before calling `sendScrapeDigestEmail`.</comment>
<file context>
@@ -0,0 +1,27 @@
+ const emails = await getScrapeDigestRecipients(
+ runs.map(r => r.social_id).filter((id): id is string => Boolean(id)),
+ );
+ return await sendScrapeDigestEmail({ emails, sections });
+}
</file context>
| import { handleTwitterProfileScraperResults } from "@/lib/apify/twitter/handleTwitterProfileScraperResults"; | ||
| const listItems = vi.fn(); | ||
| vi.mock("@/lib/socials/filterNewPostUrls", () => ({ | ||
| filterNewPostUrls: vi.fn(async (urls: string[]) => urls), |
There was a problem hiding this comment.
P2: The handler now calls filterNewPostUrls and includes newPostUrls in its return value (drives the digest email), but the tests don't verify that integration. Consider adding an assertion on the filterNewPostUrls call args and on newPostUrls in the return value so the mock isn't purely ornamental — the digest behavior depends on correct new-post tracking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/twitter/__tests__/handleTwitterProfileScraperResults.test.ts, line 5:
<comment>The handler now calls `filterNewPostUrls` and includes `newPostUrls` in its return value (drives the digest email), but the tests don't verify that integration. Consider adding an assertion on the `filterNewPostUrls` call args and on `newPostUrls` in the return value so the mock isn't purely ornamental — the digest behavior depends on correct new-post tracking.</comment>
<file context>
@@ -1,6 +1,9 @@
import { handleTwitterProfileScraperResults } from "@/lib/apify/twitter/handleTwitterProfileScraperResults";
const listItems = vi.fn();
+vi.mock("@/lib/socials/filterNewPostUrls", () => ({
+ filterNewPostUrls: vi.fn(async (urls: string[]) => urls),
+}));
vi.mock("@/lib/apify/client", () => ({ default: { dataset: vi.fn(() => ({ listItems })) } }));
</file context>
|
|
||
| const accountSocials = ( | ||
| await Promise.all( | ||
| uniqueSocialIds.map(socialId => selectAccountSocials({ socialId, limit: 10000 })), |
There was a problem hiding this comment.
P2: Digest recipients can be silently dropped for a social watched by more than 10,000 account links because this query reads only the first page. Consider paginating or moving this lookup into a DB-side joined query rather than encoding a larger fixed cap.
(Based on your team's feedback about DB-side pagination over raised Supabase limits.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/getScrapeDigestRecipients.ts, line 16:
<comment>Digest recipients can be silently dropped for a social watched by more than 10,000 account links because this query reads only the first page. Consider paginating or moving this lookup into a DB-side joined query rather than encoding a larger fixed cap.
(Based on your team's feedback about DB-side pagination over raised Supabase limits.) </comment>
<file context>
@@ -0,0 +1,28 @@
+
+ const accountSocials = (
+ await Promise.all(
+ uniqueSocialIds.map(socialId => selectAccountSocials({ socialId, limit: 10000 })),
+ )
+ ).flat();
</file context>
| .map( | ||
| s => | ||
| `<h3 style="margin:16px 0 4px">${s.platform}</h3><ul>${s.postUrls | ||
| .map(u => `<li><a href="${u.startsWith("http") ? u : `https://${u}`}">${u}</a></li>`) |
There was a problem hiding this comment.
P2: Post URLs and platform names are interpolated into the email HTML body without any escaping. Both the href attribute and visible text content for each link use the raw URL directly, and the platform name goes into an <h3> element. A URL containing a " would break the href attribute; a URL or platform with < or > could inject arbitrary HTML into the email. Consider encoding the URL for the href attribute (e.g., wrapping with a helper that escapes &, ", <, >) and optionally encoding visible text too, as a defense-in-depth measure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/sendScrapeDigestEmail.ts, line 26:
<comment>Post URLs and platform names are interpolated into the email HTML body without any escaping. Both the href attribute and visible text content for each link use the raw URL directly, and the platform name goes into an `<h3>` element. A URL containing a `"` would break the href attribute; a URL or platform with `<` or `>` could inject arbitrary HTML into the email. Consider encoding the URL for the href attribute (e.g., wrapping with a helper that escapes `&`, `"`, `<`, `>`) and optionally encoding visible text too, as a defense-in-depth measure.</comment>
<file context>
@@ -0,0 +1,38 @@
+ .map(
+ s =>
+ `<h3 style="margin:16px 0 4px">${s.platform}</h3><ul>${s.postUrls
+ .map(u => `<li><a href="${u.startsWith("http") ? u : `https://${u}`}">${u}</a></li>`)
+ .join("")}</ul>`,
+ )
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The digest architecture for recoupable/chat#1855 (PR #4 of 6, decision: one email per scrape, all platforms): registers each platform run under a
batch_idat scrape start, records completions + genuinely-new post URLs in the webhook layer, and sends one consolidated digest when the batch's last run completes.Stacked on api#759 (newness diff — provides
filterNewPostUrls); requires database#41 (batch columns) applied before this deploys. Base is #759's branch so the diff shows only this PR's work; GitHub retargets tomainwhen #759 merges.What changed
lib/supabase/apify_scraper_runs/(new):insertApifyScraperRuns(start-time registration),completeApifyScraperRun(completion + new URLs),selectApifyScraperRunsByBatch,selectApifyScraperRun, local row type (generated types don't carry database#41's columns yet).lib/apify/digest/(new):maybeSendScrapeDigest(the brain: all-siblings-complete gate → per-platform sections → skip-if-nothing-new),getScrapeDigestRecipients(same watcher chain the solo alert used),sendScrapeDigestEmail(BCC-only, deterministic minimal body — upgraded to the house template in PR Sweetmantech/myc 3550 apiimagegenerate use credits #5).postArtistSocialsScrapeHandler: mints the batch, registers runs (with platform + social mapping via a new additiveprofileUrlonscrapeProfileUrlBatchresults); response contract unchanged (extra field stripped).apifyWebhookHandler: after each result handler, completes the run and runs the digest check; failures never fail the webhook. Schema now passes through the optionalresource.idApify already sends.newPostUrls(pre-persist diff); Instagram suppresses its solo email for batch-registered runs (legacy runs keep it).Tests (RED→GREEN)
maybeSendScrapeDigest: 4 cases — waits for incomplete siblings; one send with per-platform sections (empty platforms omitted); nothing-new sends nothing; null batch no-ops.newPostUrlsreturned.lib/apify+lib/artist+lib/socials→ 272 tests passed;tsc --noEmitat exact baseline parity (200 pre-existing, 0 new); eslint clean.Merge sequencing
database#41 → api#759 → this. E2E verification (real batch scrape → one digest) planned post-merge on the tracking issue since Apify webhooks call the deployed receiver.
🤖 Generated with Claude Code
Summary by cubic
Send one consolidated new‑posts digest per scrape batch across all platforms. Meets chat#1855 by batching runs, recording genuinely new URLs, and sending a single BCC email when the last run finishes.
New Features
apify_scraper_runs; trackscompleted_atandnew_post_urls.maybeSendScrapeDigest; never fails the webhook on digest errors.getScrapeDigestRecipients,maybeSendScrapeDigest,sendScrapeDigestEmail(per‑platform sections, BCC‑only).newPostUrls(diffed pre‑persist); Instagram suppresses solo email for batch runs (legacy runs unchanged).postArtistSocialsScrapeHandlermints abatch_id, registers runs with platform/social mapping, and returns the same response shape.Migration
apify_scraper_runshas the batch columns (batch_id,completed_at,new_post_urls) before deploying.Written for commit 9a991be. Summary will update on new commits.