Repository navigation
feat(ai): render HTML content in AI stream instead of raw JSON - #307
Conversation
- Add RichContentRenderer for displaying page content as rendered HTML/markdown - Add RichDiffRenderer for showing visual content diffs with red/green highlighting - Update ToolCallRenderer and CompactToolCallRenderer to use new renderers - Modify read_page to return rawContent and pageId for navigation - Modify replace_lines to return oldContent and newContent for diff visualization - Page previews are clickable to navigate to the actual page https://claude.ai/code/session_01QemLDWC938Gfr7VSk44yKe
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds two renderers (RichContentRenderer, RichDiffRenderer), updates chat tool-call renderers to use them, and extends tool outputs (read_page, replace_lines) to include pageId, rawContent, oldContent, and newContent for richer rendering and page navigation. Changes
Sequence Diagram(s)sequenceDiagram
participant AI as AI Tool (read_page / replace_lines)
participant UI as Chat UI / Renderer
participant Router as Next.js Router
AI->>UI: return payload (content, rawContent?, oldContent?, newContent?, pageId?, type?)
alt replace_lines with old/new
UI->>UI: compute diff (RichDiffRenderer)
UI-->>UI: render diff HTML (sanitized)
else read_page or fallback
UI->>UI: prepare content (stripLineNumbers, markdownToHtml)
UI-->>UI: render content (RichContentRenderer)
end
UI->>Router: navigate to /p/{pageId} on header click (if pageId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/ai/shared/chat/tool-calls/RichContentRenderer.tsx`:
- Around line 193-197: The element currently uses dangerouslySetInnerHTML and
also renders children conditionally, which React forbids; in RichContentRenderer
replace the single element that mixes dangerouslySetInnerHTML and children with
two separate branches: when hasHtmlContent is true render a container (e.g., a
div) that only uses dangerouslySetInnerHTML={{ __html: processedHtml }} and no
children, otherwise render the <pre> (or the existing non-HTML branch) showing
processedHtml as its child; ensure you remove any children from the element that
uses dangerouslySetInnerHTML and keep references to hasHtmlContent and
processedHtml to locate the change.
🧹 Nitpick comments (6)
apps/web/src/components/ai/shared/chat/tool-calls/RichDiffRenderer.tsx (3)
31-42: Duplicate utility:stripLineNumbersexists in both renderers.This function is duplicated in
RichContentRenderer.tsx. Extract it to a shared utility to maintain DRY principles.♻️ Suggested extraction
Create a shared utility file, e.g.,
apps/web/src/components/ai/shared/chat/tool-calls/utils.ts:/** * Strips line numbers from content formatted as "123→content" */ export function stripLineNumbers(content: string): string { return content .split('\n') .map(line => { const match = line.match(/^\d+→(.*)$/); return match ? match[1] : line; }) .join('\n'); }Then import in both renderer files.
232-234: Dark mode styling uses media query instead of Tailwind's dark mode class.The inline CSS uses
@media (prefers-color-scheme: dark)which respects system preferences but won't respond to Tailwind's class-based dark mode toggle if users can switch themes independently. If the app supports manual theme switching, consider using CSS variables or Tailwind classes instead.
9-12: Consider reusingDiffChangeinterface from@pagespace/lib.The file defines a local
DiffChangeinterface withtypeandvaluefields. An exportedDiffChangeinterface already exists inpackages/lib/src/content/diff-utils.tswith the same structure plus optional position fields (originalStart,originalEnd,newStart,newEnd). Import and reuse the shared interface instead of duplicating it locally.apps/web/src/components/ai/shared/chat/tool-calls/RichContentRenderer.tsx (3)
30-39: Duplicate utility:stripLineNumbersshould be shared.This function is identical to the one in
RichDiffRenderer.tsx. Extract to a shared utility as noted in the other file review.
88-98: Consider using allowlist-based sanitization for stronger security.The current sanitization uses
FORBID_TAGSandFORBID_ATTR(blocklist approach), whileRichDiffRendererusesALLOWED_TAGSandALLOWED_ATTR(allowlist approach). Allowlist is generally more secure as it's resilient to newly discovered attack vectors.🛡️ Suggested improvement
function sanitizeHtml(html: string): string { if (typeof window === 'undefined') { return html; } return DOMPurify.sanitize(html, { - FORBID_TAGS: ['script', 'iframe', 'object', 'embed', 'link', 'meta', 'style'], - FORBID_ATTR: ['onerror', 'onload', 'onclick', 'onmouseover', 'onmouseout', 'onfocus', 'onblur'], - KEEP_CONTENT: true, + ALLOWED_TAGS: ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'br', 'ul', 'ol', 'li', 'strong', 'em', 'code', 'pre', 'a', 'blockquote', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'span', 'div'], + ALLOWED_ATTR: ['href', 'class', 'title'], }); }
45-83: Consider using a battle-tested markdown library.The custom
markdownToHtmlimplementation handles basic cases but has limitations:
- Regex-based parsing can miss edge cases (nested formatting, escaped characters)
- No support for code blocks, horizontal rules, images, etc.
If markdown rendering becomes more important, consider using a lightweight library like
marked(with sanitization) ormarkdown-it.
- Fix React error: separate dangerouslySetInnerHTML and children into branches - Extract shared utilities (stripLineNumbers, escapeHtml, markdownToHtml) to content-utils.ts - Use allowlist-based HTML sanitization for stronger security - Replace CSS media query with Tailwind dark mode classes for proper theme support - Add consistent dark mode styling across both renderers https://claude.ai/code/session_01QemLDWC938Gfr7VSk44yKe
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/ai/shared/chat/tool-calls/content-utils.ts`:
- Around line 85-88: The SSR fallback in sanitizeHtmlAllowlist returns the raw
html when typeof window === 'undefined', which risks emitting unsanitized
content server-side; change the function so that when window is undefined it
returns a safe default (e.g., an empty string or a server-sanitized result)
instead of the raw html, or wire in a server-compatible sanitizer (e.g.,
isomorphic DOMPurify or a jsdom+DOMPurify fallback) so sanitizeHtmlAllowlist
always returns sanitized output whether running in the browser or SSR.
In `@apps/web/src/components/ai/shared/chat/tool-calls/RichDiffRenderer.tsx`:
- Around line 151-158: The SSR fallback currently returns unsanitized diffHtml
when typeof window === 'undefined', which can expose raw HTML; update the
useMemo that produces sanitizedHtml (the variable and hook in
RichDiffRenderer.tsx) to avoid returning raw diffHtml on the server by returning
a safe placeholder (e.g., empty string or sanitized empty markup) when window is
undefined, or ensure server-side content is sanitized via the same DOMPurify
rules before returning — change only the fallback branch inside the useMemo that
references diffHtml so server-rendered output is never raw HTML.
🧹 Nitpick comments (1)
apps/web/src/components/ai/shared/chat/tool-calls/RichDiffRenderer.tsx (1)
36-86: Solid LCS diff implementation with a minor performance consideration.The algorithm is correct. However, for very large documents, the O(m×n) time and space complexity could cause noticeable delays. This is acceptable for typical page content, but consider adding a size threshold with a fallback to a simpler diff or truncation if content exceeds reasonable limits (e.g., >50KB).
- Return empty string instead of raw HTML when window is undefined in sanitizeHtmlAllowlist - Return empty string in RichDiffRenderer sanitizedHtml useMemo during SSR - Content will be properly sanitized client-side after hydration https://claude.ai/code/session_01QemLDWC938Gfr7VSk44yKe
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/ai/shared/chat/tool-calls/RichDiffRenderer.tsx`:
- Around line 164-171: The header <button> in RichDiffRenderer.tsx currently
omits an explicit type, which defaults to "submit" and can accidentally submit
enclosing forms; update the button element used with onClick={handleNavigate} to
include type="button" (the same element using cn(...) and disabled={!pageId}) so
it no longer triggers form submission when clicked.
- Around line 36-86: computeDiff currently always allocates an O(m×n) LCS matrix
(lcs) using m = oldWords.length and n = newWords.length which can freeze the UI
for large inputs; add a guard before allocating lcs to bail out to a cheap
fallback when m×n (or m or n) exceeds a defined cap (e.g., MAX_CELLS or
MAX_WORDS). Implement: define a constant cap, check if m * n > MAX_CELLS (or m >
MAX_WORDS || n > MAX_WORDS) at the start of computeDiff, and if so return a
simple safe fallback (for example a single remove/add pair containing
oldText/newText or a coarse line-based diff) instead of building lcs; keep
variable names computeDiff, m, n, and lcs so the check is adjacent to the
existing allocation and avoid allocating the lcs matrix when the cap is
exceeded.
- Add type="button" to header buttons to prevent accidental form submission - Add MAX_DIFF_WORDS guard (5000) in computeDiff to prevent UI freeze on large inputs - Fall back to simple remove/add diff for inputs exceeding the threshold https://claude.ai/code/session_01QemLDWC938Gfr7VSk44yKe
2 participants
https://claude.ai/code/session_01QemLDWC938Gfr7VSk44yKe
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.