fix(conversations): fix email preview mangling newsletter content#823
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe changes refactor the email preview component's quoted content detection by exporting the previously internal Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/components/ui/data-display/email-preview.tsx`:
- Around line 322-328: splitQuotedContent's reply-header regexes fail on
HTML-wrapped headers; update the patterns used in splitQuotedContent to anchor
to HTML/text/block boundaries by allowing optional HTML tags or block-level
breaks (e.g., <br>, </p>, other tags or end-of-string) after the header
delimiter and by requiring a boundary before the header start (start of string
or block/tag boundary) so the "/On\s+(?=.*\d).*\swrote:\s*$/im" and
"/From:\s[\s\S]*?Subject:/i" matches also when headers are wrapped in markup;
adjust the two regexes to accept trailing HTML tags/line-break elements and
preceding block boundaries and add a regression test that passes an
HTML-formatted reply header like "<div>On ... wrote:<br>" and an HTML-formatted
"From: ... Subject:" block to ensure splitting still occurs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fe5aca7-3a70-4ba6-8754-6b9750b3df77
📒 Files selected for processing (2)
services/platform/app/components/ui/data-display/email-preview.test.tsxservices/platform/app/components/ui/data-display/email-preview.tsx
| // "On <date/time>, <name> wrote:" — standard reply header. | ||
| // Requires a colon after "wrote" and a date-like token (digit) between On and wrote. | ||
| /On\s+(?=.*\d).*\swrote:\s*$/im, | ||
| // "From: ... Subject: ..." — Outlook-style forwarding header | ||
| /From:\s[\s\S]*?Subject:/i, | ||
| // "-----Original Message-----" — Outlook reply separator | ||
| /-----\s*Original Message\s*-----/i, |
There was a problem hiding this comment.
Anchor the reply-header regexes to HTML/text boundaries.
splitQuotedContent operates on raw HTML. Line 324 only matches On … wrote: when the colon is followed by plain-text whitespace/end-of-line, so common client output like <div>On … wrote:<br> or <p>On … wrote:</p> will no longer collapse. Line 326 has the same boundary problem and can still split normal body prose containing From: … Subject:. Please make these matchers aware of HTML/block boundaries and add a regression case for an HTML-formatted reply header.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/email-preview.tsx` around
lines 322 - 328, splitQuotedContent's reply-header regexes fail on HTML-wrapped
headers; update the patterns used in splitQuotedContent to anchor to
HTML/text/block boundaries by allowing optional HTML tags or block-level breaks
(e.g., <br>, </p>, other tags or end-of-string) after the header delimiter and
by requiring a boundary before the header start (start of string or block/tag
boundary) so the "/On\s+(?=.*\d).*\swrote:\s*$/im" and
"/From:\s[\s\S]*?Subject:/i" matches also when headers are wrapped in markup;
adjust the two regexes to accept trailing HTML tags/line-break elements and
preceding block boundaries and add a regression test that passes an
HTML-formatted reply header like "<div>On ... wrote:<br>" and an HTML-formatted
"From: ... Subject:" block to ensure splitting still occurs.
The quoted-content detection regex `On .* wrote` was too greedy, matching normal prose like "Jon Kuperman wrote about..." and splitting the email at the wrong point. The `>` stripping regexes were also corrupting HTML content. - Tighten splitQuotedContent patterns to require a colon after "wrote", a digit (date) between "On" and "wrote", and line-boundary anchoring - Remove aggressive `>` / `>` stripping from sanitizePreviewHtml - Add tests for splitQuotedContent covering false-positives and legitimate reply headers
Address CodeRabbit feedback: the "On ... wrote:" regex now matches headers wrapped in HTML tags (<div>, <p>, <br>) by using [^<] to stay within text content boundaries instead of relying on end-of-line anchors.
13810d2 to
873b89a
Compare
Summary
EmailPreviewthat was incorrectly splitting newsletter emails (e.g. text containing "wrote about" was triggering the reply-header detector)splitQuotedContentregex patterns to require a colon after "wrote", a digit (date) in the "On...wrote:" pattern, and proper line boundaries>/>stripping fromsanitizePreviewHtmlthat was corrupting HTML contentTest plan
splitQuotedContentcovering false-positives (newsletter prose) and legitimate reply headers🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests