feat(apify): deterministic digest template — direct links to each new post, no LLM body#761
Open
sweetmantech wants to merge 1 commit into
Open
Conversation
…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>
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.
5 issues found across 7 files
Confidence score: 3/5
- In
lib/apify/digest/renderScrapeDigestHtml.ts,postUrls,platform, andartistNameare interpolated into HTML (includinghref) without escaping, so crafted values could inject markup or break email rendering for recipients — HTML-escape dynamic content and strictly validate/sanitize URL attributes before merging. lib/apify/digest/sendScrapeDigestEmail.tsstill builds its own inline subject/HTML and does not callrenderScrapeDigestHtml, so the new deterministic renderer is effectively bypassed and behavior can diverge across digest paths — wire this sender to the shared renderer (or remove the unused import and land that refactor separately).- Test coverage gaps in
lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts(singular pluralization and null/undefinedartistName) and weak typing inlib/apify/__tests__/sendApifyWebhookEmail.test.ts(as never) increase regression risk around edge-case copy and fixture correctness — add focused cases and replaceas neverwith a typed fixture before merge.
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/__tests__/sendApifyWebhookEmail.test.ts">
<violation number="1" location="lib/apify/__tests__/sendApifyWebhookEmail.test.ts:22">
P3: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
The PROFILE test fixture uses `as never` which completely erases type checking on this object. Since `ApifyInstagramProfileResult` has all-optional fields, the literal already satisfies the interface — no assertion is needed. Removing `as never` (or switching to `as ApifyInstagramProfileResult` if you want explicit annotation) would let TypeScript verify the fixture stays in sync with the type definition.</violation>
</file>
<file name="lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts">
<violation number="1" location="lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts:14">
P3: No test for null/undefined artistName. The fallback `"Your artist"` is used in the subject and heading when artistName is omitted, falsy, or null — consider adding a case with no artistName to pin that default behavior.</violation>
<violation number="2" location="lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts:35">
P2: Singular subject form is untested: no case covers 1 post or 1 platform, where the pluralization code (`total === 1 ? "" : "s"`, `sections.length === 1 ? "" : "s"`) produces different output. Consider adding a case like `sections: [{platform: "instagram", postUrls: ["https://instagram.com/p/abc"]}]` and asserting subject `"Ashnikko: 1 new post across 1 platform"`.</violation>
</file>
<file name="lib/apify/digest/renderScrapeDigestHtml.ts">
<violation number="1" location="lib/apify/digest/renderScrapeDigestHtml.ts:37">
P1: The new renderer inserts `postUrls`, `platform`, and `artistName` into HTML without escaping, including inside the `href` attribute. A crafted value containing quotes or tags can break the email markup and inject unexpected links/content. Escaping dynamic HTML content (and safely encoding attribute values) before interpolation would avoid this injection path.</violation>
</file>
<file name="lib/apify/digest/sendScrapeDigestEmail.ts">
<violation number="1" location="lib/apify/digest/sendScrapeDigestEmail.ts:3">
P2: The batch digest sender still composes its own inline HTML/subject and never calls `renderScrapeDigestHtml`, so this change adds an unused import but does not actually switch this path to the deterministic renderer. That leaves two divergent templates to maintain and means future fixes in the shared renderer won’t apply to batch sends.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Webhook as Apify Webhook
participant Handler as Profile Scraper Handler
participant Upsert as Upsert Posts
participant Solo as Solo Alert (sendApifyWebhookEmail)
participant Batch as Batch Digest (sendScrapeDigestEmail)
participant Renderer as Deterministic Renderer
participant Email as Email Service (Resend)
Webhook->>Handler: Receive scrape results
Handler->>Upsert: upsertPosts(parsed)
Upsert-->>Handler: newPostUrls, profile
alt Solo scrape run
Handler->>Solo: sendApifyWebhookEmail(profile, emails, newPostUrls)
alt No recipients or no new posts
Solo-->>Handler: Return null (short circuit)
else Has recipients and new posts
Solo->>Renderer: renderScrapeDigestHtml({sections: [{platform:"instagram", postUrls: newPostUrls}], artistName})
Renderer-->>Solo: {subject, html}
Note over Solo,Email: BCC-only invariant: all recipients in bcc, to=RECOUP_FROM_EMAIL
Solo->>Email: sendEmailWithResend({to: [RECOUP_FROM_EMAIL], bcc: emails, subject, html})
Email-->>Solo: {id: "email-id"}
Solo-->>Handler: result
end
else Batch digest run
Handler->>Batch: sendScrapeDigestEmail(input)
Batch->>Renderer: renderScrapeDigestHtml({sections, artistName})
Renderer-->>Batch: {subject, html}
Batch->>Email: sendEmailWithResend({...}) (BCC)
Email-->>Batch: result
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const items = section.postUrls | ||
| .map(url => { | ||
| const href = url.startsWith("http") ? url : `https://${url}`; | ||
| return `<li style="margin:4px 0"><a href="${href}" style="color:#111;text-decoration:underline">${href}</a></li>`; |
There was a problem hiding this comment.
P1: The new renderer inserts postUrls, platform, and artistName into HTML without escaping, including inside the href attribute. A crafted value containing quotes or tags can break the email markup and inject unexpected links/content. Escaping dynamic HTML content (and safely encoding attribute values) before interpolation would avoid this injection path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/renderScrapeDigestHtml.ts, line 37:
<comment>The new renderer inserts `postUrls`, `platform`, and `artistName` into HTML without escaping, including inside the `href` attribute. A crafted value containing quotes or tags can break the email markup and inject unexpected links/content. Escaping dynamic HTML content (and safely encoding attribute values) before interpolation would avoid this injection path.</comment>
<file context>
@@ -0,0 +1,53 @@
+ const items = section.postUrls
+ .map(url => {
+ const href = url.startsWith("http") ? url : `https://${url}`;
+ return `<li style="margin:4px 0"><a href="${href}" style="color:#111;text-decoration:underline">${href}</a></li>`;
+ })
+ .join("");
</file context>
| expect((html + subject).toLowerCase()).not.toContain("dataset"); | ||
| }); | ||
|
|
||
| it("counts posts and platforms in the subject", () => { |
There was a problem hiding this comment.
P2: Singular subject form is untested: no case covers 1 post or 1 platform, where the pluralization code (total === 1 ? "" : "s", sections.length === 1 ? "" : "s") produces different output. Consider adding a case like sections: [{platform: "instagram", postUrls: ["https://instagram.com/p/abc"]}] and asserting subject "Ashnikko: 1 new post across 1 platform".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts, line 35:
<comment>Singular subject form is untested: no case covers 1 post or 1 platform, where the pluralization code (`total === 1 ? "" : "s"`, `sections.length === 1 ? "" : "s"`) produces different output. Consider adding a case like `sections: [{platform: "instagram", postUrls: ["https://instagram.com/p/abc"]}]` and asserting subject `"Ashnikko: 1 new post across 1 platform"`.</comment>
<file context>
@@ -0,0 +1,39 @@
+ expect((html + subject).toLowerCase()).not.toContain("dataset");
+ });
+
+ it("counts posts and platforms in the subject", () => {
+ const { subject } = renderScrapeDigestHtml({ sections: SECTIONS, artistName: "Ashnikko" });
+ expect(subject).toBe("Ashnikko: 3 new posts across 2 platforms");
</file context>
| @@ -1,5 +1,6 @@ | |||
| import { sendEmailWithResend } from "@/lib/emails/sendEmail"; | |||
| import { RECOUP_FROM_EMAIL } from "@/lib/const"; | |||
| import { renderScrapeDigestHtml } from "@/lib/apify/digest/renderScrapeDigestHtml"; | |||
There was a problem hiding this comment.
P2: The batch digest sender still composes its own inline HTML/subject and never calls renderScrapeDigestHtml, so this change adds an unused import but does not actually switch this path to the deterministic renderer. That leaves two divergent templates to maintain and means future fixes in the shared renderer won’t apply to batch sends.
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 3:
<comment>The batch digest sender still composes its own inline HTML/subject and never calls `renderScrapeDigestHtml`, so this change adds an unused import but does not actually switch this path to the deterministic renderer. That leaves two divergent templates to maintain and means future fixes in the shared renderer won’t apply to batch sends.</comment>
<file context>
@@ -1,5 +1,6 @@
import { sendEmailWithResend } from "@/lib/emails/sendEmail";
import { RECOUP_FROM_EMAIL } from "@/lib/const";
+import { renderScrapeDigestHtml } from "@/lib/apify/digest/renderScrapeDigestHtml";
export type ScrapeDigestSection = { platform: string; postUrls: string[] };
</file context>
| followersCount: 100, | ||
| followsCount: 10, | ||
| latestPosts: [], | ||
| } as never; |
There was a problem hiding this comment.
P3: Custom agent: Enforce Clear Code Style and Maintainability Practices
The PROFILE test fixture uses as never which completely erases type checking on this object. Since ApifyInstagramProfileResult has all-optional fields, the literal already satisfies the interface — no assertion is needed. Removing as never (or switching to as ApifyInstagramProfileResult if you want explicit annotation) would let TypeScript verify the fixture stays in sync with the type definition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/__tests__/sendApifyWebhookEmail.test.ts, line 22:
<comment>The PROFILE test fixture uses `as never` which completely erases type checking on this object. Since `ApifyInstagramProfileResult` has all-optional fields, the literal already satisfies the interface — no assertion is needed. Removing `as never` (or switching to `as ApifyInstagramProfileResult` if you want explicit annotation) would let TypeScript verify the fixture stays in sync with the type definition.</comment>
<file context>
@@ -0,0 +1,62 @@
+ followersCount: 100,
+ followsCount: 10,
+ latestPosts: [],
+} as never;
+const NEW_URLS = ["https://instagram.com/p/new1"];
+
</file context>
|
|
||
| describe("renderScrapeDigestHtml", () => { | ||
| it("links every new post, grouped under its platform", () => { | ||
| const { html } = renderScrapeDigestHtml({ sections: SECTIONS, artistName: "Ashnikko" }); |
There was a problem hiding this comment.
P3: No test for null/undefined artistName. The fallback "Your artist" is used in the subject and heading when artistName is omitted, falsy, or null — consider adding a case with no artistName to pin that default behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/apify/digest/__tests__/renderScrapeDigestHtml.test.ts, line 14:
<comment>No test for null/undefined artistName. The fallback `"Your artist"` is used in the subject and heading when artistName is omitted, falsy, or null — consider adding a case with no artistName to pin that default behavior.</comment>
<file context>
@@ -0,0 +1,39 @@
+
+describe("renderScrapeDigestHtml", () => {
+ it("links every new post, grouped under its platform", () => {
+ const { html } = renderScrapeDigestHtml({ sections: SECTIONS, artistName: "Ashnikko" });
+ for (const s of SECTIONS) for (const u of s.postUrls) expect(html).toContain(`href="${u}"`);
+ expect(html.toLowerCase()).toContain("instagram");
</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 content half of the revamp on recoupable/chat#1855 (PR #5 of 6, stacked on api#760): a shared deterministic renderer replaces the per-send LLM body. Evidence on the issue showed the LLM body produced different branding on every send, leaked vendor jargon ("Apify Dataset Notification") to customers, and didn't reliably link the posts it claimed were new.
What changed
lib/apify/digest/renderScrapeDigestHtml.ts(new): deterministic subject + HTML — per-platform sections, a direct link to every genuinely-new post, friendly platform labels, chat CTA as secondary link. No LLM call.sendScrapeDigestEmail(batch digest) andsendApifyWebhookEmail(legacy solo alert) both use it. The solo alert now takesnewPostUrlsand sends nothing without them; its LLM path (generateText) is deleted.sendApifyWebhookEmail.tswholesale and embodies the same BCC invariant (tests recreated here) — merging fix(emails): BCC scrape-alert recipients — never a shared cross-tenant To: line #758 first then this stack resolves in favor of this version with no behavior change.Tests (RED→GREEN)
vitest run lib/apifyall green; tsc at baseline parity; eslint clean.🤖 Generated with Claude Code
Summary by cubic
Replaced the LLM-generated email body with a deterministic digest template that links directly to every new post and uses consistent, customer-safe branding. Applied to both the batch digest and the legacy Instagram alert, with BCC-only delivery.
sendApifyWebhookEmail(profile, emails, newPostUrls);newPostUrlsis now required.emailsis empty ornewPostUrlsis empty.tois set toRECOUP_FROM_EMAIL.Written for commit 7960d93. Summary will update on new commits.