Use Neynar metadata for cast embeds #1638
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@willyogo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 9 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds client-side embed previewing with per-URL metadata fetching/caching, Farcaster embed normalization and sanitization utilities, SharedDataProvider support for embed metadata, server endpoints for metadata proxying and publish-time embed validation, tests for embed logic, and a small modal layout height/scroll change. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as CreateCast (client)
participant Shared as SharedDataProvider
participant API as Next API /embedMetadata
participant Neynar as Neynar Service
participant PublishAPI as Next API /publishMessage
User->>Editor: Type or paste text containing URL(s)
Editor->>Editor: Debounce, extract URL(s)
alt cache miss
Editor->>API: GET /api/farcaster/neynar/embedMetadata?url=...
API->>Neynar: fetchEmbeddedUrlMetadata(url)
Neynar-->>API: metadata
API-->>Editor: metadata
Editor->>Shared: addRecentEmbed(url, embed, metadata)
else cache hit
Editor->>Shared: getRecentEmbed(url)
Shared-->>Editor: {embed, metadata?}
end
Editor->>Editor: normalize/sanitize embeds, build previews, render embed cards
User->>Editor: Click publish
Editor->>PublishAPI: POST /api/farcaster/neynar/publishMessage (sanitized embeds)
PublishAPI->>PublishAPI: validateFarcasterEmbeds -> sanitizeFarcasterEmbeds
alt invalid or overLimit
PublishAPI-->>Editor: 400 { message: "Invalid embeds", invalid: N, overLimit: true|false }
else valid
PublishAPI->>Neynar: publishMessageToFarcaster
Neynar-->>PublishAPI: success
PublishAPI-->>Editor: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/api/farcaster/neynar/publishMessage.ts (1)
43-44: Incorrect status extraction from AxiosError.
AxiosErrordoesn't have astatusproperty directly; it's onresponse. This could cause valid upstream error statuses to be lost.Proposed fix
if (isAxiosError(e)) { - res.status(e.status || 500).json(e.response?.data); + res.status(e.response?.status || 500).json(e.response?.data); } else {
🧹 Nitpick comments (4)
src/pages/api/farcaster/neynar/embedMetadata.ts (1)
6-9: Redundant method check.The
requestHandlerwrapper already restricts this handler to GET requests (line 30-31) and returns a 405 for unsupported methods. The manual check on lines 7-9 is unreachable.Proposed fix
const embedMetadata = async (req: NextApiRequest, res: NextApiResponse) => { - if (req.method !== "GET") { - return res.status(405).json({ message: "Method not allowed" }); - } - const url = typeof req.query.url === "string" ? req.query.url : "";src/pages/api/farcaster/neynar/publishMessage.ts (1)
18-34: Minor inefficiency: double validation.
sanitizeFarcasterEmbeds(line 33) internally callsvalidateFarcasterEmbedsagain. Since validation already passed, you could use thenormalizedarray directly:Proposed optimization
if (Array.isArray(message.embeds)) { const { normalized, invalid, overLimit } = validateFarcasterEmbeds(message.embeds); if (invalid.length || overLimit) { console.warn("Rejecting invalid Farcaster embeds", { invalidEmbeds: invalid, overLimit, }); return res.status(400).json({ message: "Invalid embeds", invalid: invalid.length, overLimit, limit: FARCASTER_EMBED_LIMIT, }); } - message.embeds = sanitizeFarcasterEmbeds(message.embeds); + message.embeds = normalized.slice(0, FARCASTER_EMBED_LIMIT); }Note: This loses deduplication. If deduplication is needed server-side, keep the current approach or extract a helper that accepts pre-normalized embeds.
src/common/providers/SharedDataProvider.tsx (1)
45-67: Consider adding cache size limits.Unlike
recentChannelswhich limits to 20 entries (line 54),recentEmbedsgrows unbounded. For long-lived sessions with many URLs, this could cause memory growth.Proposed fix
+const MAX_RECENT_EMBEDS = 50; + const addRecentEmbed = useCallback( (url: string, embed: FarcasterEmbed, metadata?: EmbedUrlMetadata) => { - setRecentEmbeds((prev) => ({ - ...prev, - [url]: { embed, metadata }, - })); + setRecentEmbeds((prev) => { + const entries = Object.entries(prev); + const updated = { ...prev, [url]: { embed, metadata } }; + if (entries.length >= MAX_RECENT_EMBEDS) { + // Remove oldest entry (first key) + const [oldestKey] = entries[0]; + delete updated[oldestKey]; + } + return updated; + }); }, [], );src/fidgets/farcaster/components/CreateCast.tsx (1)
677-769: Potential stale closure issue withloadingEmbedsdependency.The effect depends on
loadingEmbeds, but inside thesetTimeoutcallback you access the potentially staleloadingEmbedsvalue. While the current logic works because the check happens before the async operation starts, the dependency onloadingEmbedscan cause unnecessary effect re-runs.Consider using a ref to track loading URLs to avoid effect re-triggering:
Alternative approach
+const loadingEmbedsRef = useRef<Set<string>>(new Set()); const [loadingEmbeds, setLoadingEmbeds] = useState<Set<string>>(new Set()); + +// Sync ref with state +useEffect(() => { + loadingEmbedsRef.current = loadingEmbeds; +}, [loadingEmbeds]); // In the effect, use loadingEmbedsRef.current for checks -if (loadingEmbeds.has(normalizedUrl)) { +if (loadingEmbedsRef.current.has(normalizedUrl)) { return; } // Remove loadingEmbeds from dependency array -}, [text, embedPreviews, addRecentEmbed, getRecentEmbed, removedEmbeds, loadingEmbeds]); +}, [text, embedPreviews, addRecentEmbed, getRecentEmbed, removedEmbeds]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/common/providers/SharedDataProvider.tsxsrc/fidgets/farcaster/components/CreateCast.tsxsrc/fidgets/farcaster/utils.tssrc/fidgets/farcaster/utils/embedNormalization.tssrc/fidgets/farcaster/utils/embedTypes.tssrc/pages/api/farcaster/neynar/embedMetadata.tssrc/pages/api/farcaster/neynar/publishMessage.tstests/farcasterEmbeds.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:25:25.957Z
Learnt from: j-paterson
Repo: blankdotspace/space-system PR: 1625
File: src/common/lib/utils/tokenGates.ts:12-20
Timestamp: 2025-12-29T18:25:25.957Z
Learning: Gate tokens in src/common/lib/utils/tokenGates.ts are currently fetched redundantly from 3 client components (CreateCast.tsx, BackgroundGenerator.tsx, NogsGateButton.tsx) with no caching, and should be refactored to use a cached API route with a shared client hook to reduce redundant server calls.
Applied to files:
src/fidgets/farcaster/components/CreateCast.tsx
🧬 Code graph analysis (5)
src/pages/api/farcaster/neynar/publishMessage.ts (1)
src/fidgets/farcaster/utils/embedNormalization.ts (3)
validateFarcasterEmbeds(87-105)FARCASTER_EMBED_LIMIT(8-8)sanitizeFarcasterEmbeds(107-132)
src/pages/api/farcaster/neynar/embedMetadata.ts (1)
src/common/data/api/requestHandler.ts (1)
requestHandler(31-51)
src/common/providers/SharedDataProvider.tsx (1)
src/fidgets/farcaster/utils/embedTypes.ts (1)
FarcasterEmbed(12-12)
tests/farcasterEmbeds.test.ts (1)
src/fidgets/farcaster/utils/embedNormalization.ts (3)
sanitizeFarcasterEmbeds(107-132)FARCASTER_EMBED_LIMIT(8-8)validateFarcasterEmbeds(87-105)
src/fidgets/farcaster/utils/embedNormalization.ts (2)
src/fidgets/farcaster/utils/embedTypes.ts (2)
FarcasterEmbed(12-12)isFarcasterUrlEmbed(14-18)src/common/lib/utils/urls.ts (1)
isWebUrl(22-28)
🔇 Additional comments (19)
src/fidgets/farcaster/utils/embedNormalization.ts (6)
10-42: LGTM!The
normalizeCastIdfunction correctly validates the cast ID structure, handling bothUint8Arrayand hex string hash formats with proper length validation (20 bytes for Farcaster cast hashes).
44-67: LGTM!The
normalizeEmbedfunction handles multiple input variations (castId/cast_id,url/uri) and includes defensive URL normalization with a safe fallback.
69-85: LGTM!The key generation functions create deterministic, unique keys for deduplication purposes.
87-105: LGTM!The validation function properly categorizes embeds and provides structured feedback for error handling.
107-132: LGTM!The sanitization function correctly combines validation, deduplication, removal filtering, and limit enforcement in a single pass.
134-187: LGTM!The metadata resolution and mapping functions properly handle optional fields with appropriate fallbacks and type safety.
tests/farcasterEmbeds.test.ts (3)
12-25: LGTM!Good test coverage for deduplication and limit enforcement. The test correctly verifies that the first occurrence is kept and subsequent duplicates are removed.
27-35: LGTM!Good edge case test verifying that invalid hash lengths are correctly identified.
38-64: LGTM!Good integration test for the validation rejection flow.
Consider adding tests for: embeds exceeding the limit, and valid embeds passing through successfully.
src/fidgets/farcaster/utils/embedTypes.ts (1)
1-18: LGTM!Clean type definitions with a proper discriminated union and type guard. The hash as
Uint8Arrayaligns with Farcaster core types.src/fidgets/farcaster/utils.ts (1)
30-30: LGTM!Clean refactor extracting embed types to a dedicated module for better reusability.
src/common/providers/SharedDataProvider.tsx (1)
23-25: LGTM!The updated context types correctly reflect the new cache structure storing both embed and metadata.
src/fidgets/farcaster/components/CreateCast.tsx (7)
150-162: LGTM!Simple and effective URL extraction utilities with appropriate defensive coding.
193-197: LGTM!Well-structured state management for the embed preview lifecycle (loading, success, error, removal).
475-486: LGTM!Clean integration with the editor's metadata fetching hook.
585-591: LGTM!The embed prioritization order (previews → editor → initial) ensures newly added embeds take precedence, with proper deduplication via
sanitizeFarcasterEmbeds.
663-675: LGTM!Clean removal handler with proper immutable state updates.
1259-1325: LGTM!Well-structured preview UI with appropriate loading states, error handling, and dismissal functionality.
635-644: The application already has Next.js image configuration that permits arbitrary external domains. Thenext.config.jsfile includesremotePatterns: [{ protocol: "https", hostname: "**" }], which allows any HTTPS image URL, including those from og:image metadata. No configuration changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/fidgets/farcaster/components/CreateCast.tsx (4)
476-487: Consider enhancing error details for debugging.The generic error message
"Failed to fetch embed metadata"could include the HTTP status and URL to aid troubleshooting.🔎 Enhanced error message
const fetchMetadataForEditor = useCallback(async (url: string) => { const response = await fetch( `/api/farcaster/neynar/embedMetadata?url=${encodeURIComponent(url)}`, ); if (!response.ok) { - throw new Error("Failed to fetch embed metadata"); + throw new Error(`Failed to fetch embed metadata for ${url}: ${response.status} ${response.statusText}`); } const payload = await response.json(); return mapEmbedMetadataToUrlMetadata(payload?.metadata); }, []);
682-774: Embed fetching logic is functional but has minor race condition risk.The effect correctly extracts URLs, checks multiple conditions before fetching, and uses a 500ms debounce for UX. However, if the component unmounts or text changes during an in-flight fetch, the subsequent
setEmbedPreviewsorsetEmbedErrorscalls will attempt to update unmounted state.Consider using an AbortController and cleanup to cancel in-flight fetches:
🔎 Add fetch cancellation
useEffect(() => { const urlsInText = extractUrlsFromText(text || ""); const timers: Array<ReturnType<typeof setTimeout>> = []; + const controllers: AbortController[] = []; urlsInText.forEach((rawUrl) => { // ... existing checks ... const timer = setTimeout(() => { + const controller = new AbortController(); + controllers.push(controller); + setLoadingEmbeds((prev) => new Set(prev).add(normalizedUrl)); setEmbedErrors((prev) => { const next = { ...prev }; delete next[normalizedUrl]; return next; }); - fetch(`/api/farcaster/neynar/embedMetadata?url=${encodeURIComponent(normalizedUrl)}`) + fetch(`/api/farcaster/neynar/embedMetadata?url=${encodeURIComponent(normalizedUrl)}`, { + signal: controller.signal + }) .then(async (res) => { // ... existing logic ... }) .catch((error) => { + if (error.name === 'AbortError') return; setEmbedErrors((prev) => ({ ...prev, [normalizedUrl]: error.message })); }) .finally(() => { setLoadingEmbeds((prev) => { const next = new Set(prev); next.delete(normalizedUrl); return next; }); }); }, 500); timers.push(timer); }); return () => { timers.forEach((timer) => clearTimeout(timer)); + controllers.forEach((controller) => controller.abort()); }; }, [text, embedPreviews, addRecentEmbed, getRecentEmbed, removedEmbeds, loadingEmbeds]);Additionally, Line 690-692: URL normalization silently keeps the raw value on failure. Consider whether invalid URLs should be skipped entirely rather than processed.
803-803: Cache check logic is semantically unclear.The condition
!mentionFidCache.has(username) || !mentionFidCache.get(username)checks both key existence and value truthiness. If a FID could legitimately be0(or any falsy value), it would be incorrectly treated as uncached. While Farcaster FIDs start from 1, making this safe in practice, the intent would be clearer with just!mentionFidCache.has(username).🔎 Clearer cache check
const uncachedUsernames = uniqueUsernames.filter( - (username) => !mentionFidCache.has(username) || !mentionFidCache.get(username), + (username) => !mentionFidCache.has(username), );
1264-1331: Embed preview UI is functional but missing accessibility labels.The UI correctly shows loading states, dismissible errors, and embed previews with remove controls. However, the remove buttons (lines 1305-1311 and 1319-1325) should have
aria-labelattributes for screen reader users, as the "✕" symbol alone doesn't convey the action.🔎 Add accessibility labels
<button type="button" className="absolute right-3 top-3 z-10 rounded-full bg-white/90 px-2 py-1 text-xs font-semibold text-gray-700 shadow hover:bg-white" onClick={() => handleRemoveEmbed(item.embed)} + aria-label="Remove embed" > ✕ </button>Apply the same change to the button at lines 1319-1325.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/fidgets/farcaster/components/CreateCast.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:25:25.957Z
Learnt from: j-paterson
Repo: blankdotspace/space-system PR: 1625
File: src/common/lib/utils/tokenGates.ts:12-20
Timestamp: 2025-12-29T18:25:25.957Z
Learning: Gate tokens in src/common/lib/utils/tokenGates.ts are currently fetched redundantly from 3 client components (CreateCast.tsx, BackgroundGenerator.tsx, NogsGateButton.tsx) with no caching, and should be refactored to use a cached API route with a shared client hook to reduce redundant server calls.
Applied to files:
src/fidgets/farcaster/components/CreateCast.tsx
🧬 Code graph analysis (1)
src/fidgets/farcaster/components/CreateCast.tsx (4)
src/fidgets/farcaster/utils/embedTypes.ts (3)
FarcasterEmbed(12-12)FarcasterCastIdEmbed(5-10)isFarcasterUrlEmbed(14-18)src/common/providers/SharedDataProvider.tsx (1)
useSharedData(115-123)src/fidgets/farcaster/utils/embedNormalization.ts (3)
mapEmbedMetadataToUrlMetadata(174-187)sanitizeFarcasterEmbeds(107-132)createEmbedKey(74-85)src/common/lib/utils/urls.ts (2)
isImageUrl(1-20)isWebUrl(22-28)
🔇 Additional comments (6)
src/fidgets/farcaster/components/CreateCast.tsx (6)
194-198: Well-structured embed state management.The state design effectively tracks embed lifecycle with appropriate data structures: Records for keyed access to previews/errors, and Sets for efficient membership checks on loading/removed states. The integration with SharedDataProvider enables cross-component caching.
558-606: Solid embed normalization and sanitization flow.The multi-step process correctly:
- Normalizes Mod Protocol embeds to Farcaster schema
- Combines embeds from previews, editor, and initial state
- Applies deduplication, removal filtering, and limit enforcement via
sanitizeFarcasterEmbeds- Enriches with metadata for rendering
The memoization dependencies are accurate and the prioritization (previews + editor embeds before initialEmbeds) is logical.
608-666: Comprehensive embed rendering with proper fallbacks.The function correctly handles cast embeds, images, videos, and link previews with appropriate metadata extraction. The Next.js Image component is properly configured with
fillandsizesattributes.Consider adding error boundaries or fallback handling if image loading fails or metadata is malformed, but this is not critical for the current implementation.
668-680: LGTM: Removal logic is correct.The function properly tracks removed embeds and clears preview state. Note that once removed, the same URL won't auto-preview again unless removedEmbeds is cleared—this appears to be intentional UX to prevent annoying re-previews.
868-868: Correct usage of sanitizedEmbeds.The mentions effect now uses
sanitizedEmbedswhich properly combines and deduplicates embeds from all sources (previews, editor, initial state) with user removals applied. This aligns with the PR's goal of normalizing embeds to the Farcaster schema.
900-900: LGTM: Correct embed prioritization for publish.The logic
sanitizedEmbeds.length ? sanitizedEmbeds : draft.embeds || []appropriately prioritizes the normalized/sanitized embeds from the new preview system while falling back to draft embeds for compatibility.
| const extractUrlsFromText = (text: string): string[] => { | ||
| const urlRegex = /(https?:\/\/[^\s]+)/gi; | ||
| return Array.from(new Set(text.match(urlRegex) || [])); | ||
| }; |
There was a problem hiding this comment.
URL regex captures trailing punctuation.
The pattern /(https?:\/\/[^\s]+)/gi will include trailing punctuation like ., ,, ), ! that are often not part of the URL. For example, "Check out https://example.com." will capture "https://example.com." with the period.
🔎 Improved URL extraction that handles common punctuation
const extractUrlsFromText = (text: string): string[] => {
- const urlRegex = /(https?:\/\/[^\s]+)/gi;
+ const urlRegex = /(https?:\/\/[^\s]+?)(?=[.,;:!?)}\]]*(?:\s|$))/gi;
return Array.from(new Set(text.match(urlRegex) || []));
};This pattern uses a positive lookahead to stop before trailing punctuation that's followed by whitespace or end of string.
📝 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.
| const extractUrlsFromText = (text: string): string[] => { | |
| const urlRegex = /(https?:\/\/[^\s]+)/gi; | |
| return Array.from(new Set(text.match(urlRegex) || [])); | |
| }; | |
| const extractUrlsFromText = (text: string): string[] => { | |
| const urlRegex = /(https?:\/\/[^\s]+?)(?=[.,;:!?)}\]]*(?:\s|$))/gi; | |
| return Array.from(new Set(text.match(urlRegex) || [])); | |
| }; |
🤖 Prompt for AI Agents
In src/fidgets/farcaster/components/CreateCast.tsx around lines 151 to 154, the
current URL extractor regex captures trailing punctuation (e.g. periods, commas,
closing parens) so URLs like "https://example.com." include the dot; replace the
regex with one that excludes common trailing punctuation in the character class
and uses a positive lookahead to assert the URL ends before whitespace or
end-of-string (i.e. stop before .,;:!?)]}<> or quotes), then return unique
matches as before.
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/fidgets/farcaster/components/CreateCast.tsx (1)
151-154: URL regex captures trailing punctuation.This is a known issue from a previous review. The pattern
/(https?:\/\/[^\s]+)/giwill include trailing punctuation like.,,,),!that are often not part of the URL. For example, "Check out https://example.com." will capture "https://example.com." with the period.
🧹 Nitpick comments (4)
src/fidgets/farcaster/components/CreateCast.tsx (3)
687-779: Consider cancellation for in-flight requests when component unmounts.The effect clears pending timers on cleanup, but doesn't abort fetch requests that have already started. If the component unmounts while a fetch is in progress, state updates will be attempted on an unmounted component.
🔎 Add AbortController for fetch cancellation
useEffect(() => { const urlsInText = extractUrlsFromText(text || ""); const timers: Array<ReturnType<typeof setTimeout>> = []; + const abortController = new AbortController(); urlsInText.forEach((rawUrl) => { // ... existing validation logic ... const timer = setTimeout(() => { setLoadingEmbeds((prev) => new Set(prev).add(normalizedUrl)); setEmbedErrors((prev) => { const next = { ...prev }; delete next[normalizedUrl]; return next; }); - fetch(`/api/farcaster/neynar/embedMetadata?url=${encodeURIComponent(normalizedUrl)}`) + fetch(`/api/farcaster/neynar/embedMetadata?url=${encodeURIComponent(normalizedUrl)}`, { + signal: abortController.signal, + }) .then(async (res) => { // ... existing success logic ... }) .catch((error) => { + if (error.name === 'AbortError') return; setEmbedErrors((prev) => ({ ...prev, [normalizedUrl]: error.message })); }) .finally(() => { + if (abortController.signal.aborted) return; setLoadingEmbeds((prev) => { const next = new Set(prev); next.delete(normalizedUrl); return next; }); }); }, 500); timers.push(timer); }); return () => { timers.forEach((timer) => clearTimeout(timer)); + abortController.abort(); }; }, [text, embedPreviews, addRecentEmbed, getRecentEmbed, removedEmbeds]);
733-771: 500ms fixed delay per URL may cause user-perceived lag for fast typing.Each URL gets a 500ms delay before fetching. While this provides debouncing, it adds noticeable latency. Consider if a shorter delay (e.g., 300ms) or a single debounced effect might improve UX without overwhelming the API.
375-395: Ref.currentin dependency array won't trigger effect re-runs.
editorContentRef.currentin the dependency array won't cause the effect to re-run when the ref changes because mutating.currentdoesn't trigger a re-render. This effect only runs once on mount, which may be the intended behavior, but the dependency is misleading.🔎 Fix the dependency array
return () => { el.removeEventListener("paste", handler as any); }; - }, [editorContentRef.current]); + }, []); // Effect should only run once on mountsrc/common/providers/SharedDataProvider.tsx (1)
61-76: Cache eviction assumes insertion order is preserved in object keys.The eviction logic uses
Object.keys(updated)[0]to find the "oldest" entry. While modern JavaScript engines preserve insertion order for string keys, this is a subtle assumption. Also, the spread{ ...prev, [url]: ... }moves an existing key to the end only in some cases.Consider using a
Mapinstead of a plain object if strict LRU behavior is desired, asMapguarantees insertion order and makes eviction semantics clearer. For the current use case where approximate eviction is acceptable, the implementation works fine.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/common/providers/SharedDataProvider.tsxsrc/fidgets/farcaster/components/CreateCast.tsxsrc/fidgets/farcaster/utils/embedNormalization.tssrc/pages/api/farcaster/neynar/embedMetadata.tssrc/pages/api/farcaster/neynar/publishMessage.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/api/farcaster/neynar/publishMessage.ts
- src/pages/api/farcaster/neynar/embedMetadata.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T18:25:25.957Z
Learnt from: j-paterson
Repo: blankdotspace/space-system PR: 1625
File: src/common/lib/utils/tokenGates.ts:12-20
Timestamp: 2025-12-29T18:25:25.957Z
Learning: Gate tokens in src/common/lib/utils/tokenGates.ts are currently fetched redundantly from 3 client components (CreateCast.tsx, BackgroundGenerator.tsx, NogsGateButton.tsx) with no caching, and should be refactored to use a cached API route with a shared client hook to reduce redundant server calls.
Applied to files:
src/fidgets/farcaster/components/CreateCast.tsx
🧬 Code graph analysis (3)
src/fidgets/farcaster/components/CreateCast.tsx (5)
src/fidgets/farcaster/utils/embedTypes.ts (3)
FarcasterEmbed(12-12)FarcasterCastIdEmbed(5-10)isFarcasterUrlEmbed(14-18)src/common/providers/SharedDataProvider.tsx (1)
useSharedData(124-132)src/fidgets/farcaster/utils/embedNormalization.ts (3)
mapEmbedMetadataToUrlMetadata(180-193)sanitizeFarcasterEmbeds(113-138)createEmbedKey(74-85)src/common/lib/utils/urls.ts (2)
isImageUrl(1-20)isWebUrl(22-28)src/common/components/atoms/spinner.tsx (1)
Spinner(12-27)
src/common/providers/SharedDataProvider.tsx (1)
src/fidgets/farcaster/utils/embedTypes.ts (1)
FarcasterEmbed(12-12)
src/fidgets/farcaster/utils/embedNormalization.ts (2)
src/fidgets/farcaster/utils/embedTypes.ts (2)
FarcasterEmbed(12-12)isFarcasterUrlEmbed(14-18)src/common/lib/utils/urls.ts (1)
isWebUrl(22-28)
🔇 Additional comments (16)
src/fidgets/farcaster/utils/embedNormalization.ts (7)
10-42: LGTM! Robust cast ID normalization with proper validation.The function handles multiple input formats (Uint8Array and hex strings with/without
0xprefix), validates fid is a finite non-negative number, and correctly validates the hash length to 20 bytes as required by Farcaster.
44-67: LGTM! Flexible embed normalization supporting multiple property names.The function correctly handles both
castId/cast_idandurl/uriproperty variants, usesisWebUrlfor validation, and safely normalizes URLs through the URL constructor with fallback.
69-85: LGTM! Clean implementation of embed key generation.The
toHexStringhelper correctly converts bytes to hex, andcreateEmbedKeyproduces unique, deterministic keys for deduplication. URL normalization via URL constructor ensures consistent keys.
87-105: LGTM! Validation function with good separation of valid and invalid embeds.Returns both normalized embeds and invalid entries with their indices, which is useful for error reporting. The
overLimitflag enables callers to detect when the embed count exceeds the limit.
113-138: LGTM! Sanitization correctly handles deduplication and limit enforcement.The function efficiently deduplicates embeds using
Set, respects the removed keys, and enforces the limit. The early break optimization prevents unnecessary iterations.
140-164: LGTM! Sensible priority order for URL resolution.Prioritizes frame URL, then Open Graph URL, then oEmbed URL, and finally falls back to the source URL. Each candidate is validated with
isWebUrlbefore acceptance.
166-193: LGTM! Clean metadata mapping with safe number coercion.The
pickOgImagehelper safely handles missing values and usesNumber.isFiniteto validate dimensions. ThemapEmbedMetadataToUrlMetadatafunction correctly maps Neynar metadata to the expected schema.src/fidgets/farcaster/components/CreateCast.tsx (7)
194-203: LGTM! Well-structured state management for embed previews.The use of
loadingEmbedsRefalongsideloadingEmbedsstate is a good pattern to avoid stale closures in async callbacks. The separation of concerns (previews, removed, loading, errors) is clean.
481-492: LGTM! Clean metadata fetching for the editor.The function properly encodes the URL and handles errors appropriately. The mapping through
mapEmbedMetadataToUrlMetadataensures consistent metadata structure.
563-597: LGTM! Correct embed normalization and sanitization flow.The
normalizeEditorEmbedscorrectly converts ModEmbed to FarcasterEmbed. ThesanitizedEmbedsmemo properly prioritizes preview embeds over editor embeds over initial embeds, and applies the removal filter.
613-671: LGTM! Comprehensive preview rendering with proper type handling.The
renderPreviewForEmbedfunction correctly handles cast embeds, images, videos, and link previews. The fallback to URL for title when metadata is missing is sensible.
873-873: VerifysanitizedEmbedsis correctly used in draft state.The draft is updated with
sanitizedEmbedswhich is derived frompreviewEmbeds,normalizedEditorEmbeds, andinitialEmbeds. This should work correctly, but ensure that the order of precedence (preview → editor → initial) matches the intended behavior.
905-910: LGTM! Correct fallback for embeds in publish flow.The logic correctly uses
sanitizedEmbedswhen available, falling back todraft.embedsotherwise. This ensures embeds are always present for submission.
1269-1335: LGTM! Good UX with loading states, error handling, and remove functionality.The embed preview section properly shows loading indicators per URL, dismissible error messages, and remove buttons for each embed. The conditional rendering based on frame metadata is correct.
src/common/providers/SharedDataProvider.tsx (2)
10-16: LGTM! Clean type-only imports and updated type sources.The imports correctly use
typekeyword for type-only imports, and theFarcasterEmbedtype is now sourced from the newembedTypesmodule.
47-49: LGTM! Metadata-aware caching with bounded size.The updated
recentEmbedsstructure correctly stores both embed and optional metadata. TheMAX_RECENT_EMBEDSlimit prevents unbounded memory growth.Also applies to: 61-76
its trying to convert urls without https:// to http websites |
taggin failed with multiple embeds I also published the same caste twice because I double clicked the publish button |
Previously, the app used Mod Protocol and some custom logic to handle embedding content in casts. As a result, mini apps couldn't be embedded in casts and URLs weren't embedded properly in casts with opengraph metadata.
This PR leverages neynar's fetchEmbeddedUrlMetadata method to fetch URLs metadata (including mini app manifest) and properly structures the embed content according to @farcaster/core's publishMessageToFarcaster standard.
Last but not least, it makes the cast modal scrollable.
Summary
Testing
Mini app previews in casts:
URL preview in casts:
mini app embedded in cast on farcaster:
Codex Task
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.