Documents support in chat#231
Conversation
WalkthroughAdds end-to-end document attachment support (types, file utilities, UI, ChatService APIs, runtime/composer/thread integration) and a client-side GitHub OAuth utility for linking/reauth with Firebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MessageComposer
participant ChatService
participant ValidationModal
participant Thread
User->>MessageComposer: Drop/paste/select file
MessageComposer->>ChatService: validateDocument(conversationId, file)
alt can_upload == false
ChatService-->>MessageComposer: ValidationResponse(can_upload=false)
MessageComposer->>ValidationModal: open(validation)
ValidationModal->>User: show cannot-upload details
else can_upload == true
ChatService-->>MessageComposer: ValidationResponse(can_upload=true)
MessageComposer->>ChatService: uploadAttachment(file)
ChatService-->>MessageComposer: AttachmentUploadResponse(id)
MessageComposer->>MessageComposer: add to documents state
User->>MessageComposer: Send message
MessageComposer->>ChatService: streamMessage(..., attachmentIds=[id])
ChatService->>Thread: stream message data (includes attachment_ids)
Thread->>ChatService: getAttachmentInfo(id)
ChatService-->>Thread: AttachmentInfo / metadata
Thread->>User: render message with attachment block
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 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 |
|
|
Cloud Run service deployed: https://pr-doc-cj6r7x3fpa-uc.a.run.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
components/chat/DocumentAttachmentCard.tsx (2)
69-86: Add aria-labels for better accessibility.The buttons use
titleattributes for tooltips, but these are not read by screen readers. Consider addingaria-labelattributes to improve accessibility.Apply this diff to add aria-labels:
{onDownload && ( <button type="button" onClick={onDownload} className="p-1.5 hover:bg-gray-200 rounded transition-colors" title="Download" + aria-label={`Download ${attachment.file_name}`} > <Download className="w-4 h-4" /> </button> )} <button type="button" onClick={onRemove} className="p-1.5 hover:bg-red-100 text-red-600 rounded transition-colors" title="Remove" + aria-label={`Remove ${attachment.file_name}`} > <X className="w-4 h-4" /> </button>
22-23: Consider using the FileText icon component as fallback.Line 2 imports
FileTextfrom lucide-react, but it's not used. Consider using<FileText />as the fallback icon instead of the emoji '📎' for visual consistency with other lucide-react icons in the codebase.- const icon = fileTypeInfo?.icon || '📎'; + const iconEmoji = fileTypeInfo?.icon;Then in the JSX:
- <div className="text-2xl flex-shrink-0">{icon}</div> + <div className="text-2xl flex-shrink-0"> + {iconEmoji || <FileText className="w-6 h-6 text-gray-400" />} + </div>lib/utils/fileTypes.ts (1)
56-60: Consider handling edge case for 0 bytes.While unlikely in practice, if
bytesis 0, the function returns "0 B" which is correct. However, consider if negative values should be handled or if the function should acceptbytes: numberorbytes: number & { __brand: "positive" }for type safety.export function formatFileSize(bytes: number): string { + if (bytes < 0) return 'Invalid size'; if (bytes < 1024) return `${bytes} B`;app/(main)/chat/[chatId]/runtime.ts (1)
269-270: Type assertion could be unsafe.The cast to
any[]doesn't validate that the objects have the expected structure. IfrunConfig.custom.documentscontains unexpected data, this could lead to runtime errors in the UI when rendering attachments.Consider adding runtime validation or using a type guard:
// Define expected structure interface DocumentAttachmentData { id: string; file_name: string; mime_type: string; file_size: number; token_count?: number; metadata?: any; } // Add type guard function isDocumentAttachment(obj: any): obj is DocumentAttachmentData { return obj && typeof obj.id === 'string' && typeof obj.file_name === 'string' && typeof obj.mime_type === 'string' && typeof obj.file_size === 'number'; } // Use in extraction const documentAttachments = ( (message.runConfig?.custom?.documents as any[]) || [] ).filter(isDocumentAttachment);Alternatively, verify that the upstream code setting
runConfig.custom.documentsensures proper typing.app/(main)/chat/[chatId]/components/Thread.tsx (1)
360-395: Reuse the shared attachment card for consistencyNow that
DocumentAttachmentCardexists, consider rendering it here instead of hand-rolling the emoji UI. That keeps formatting/icon logic single-sourced and avoids the two code paths drifting over time.app/(main)/chat/[chatId]/components/MessageComposer.tsx (1)
300-302: Drop the debug logging before mergeThe new
console.logstatements are handy during development but will spam the console in production. Let’s remove them before we ship.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/(auth)/utils/github-oauth.ts(1 hunks)app/(main)/chat/[chatId]/components/MessageComposer.tsx(9 hunks)app/(main)/chat/[chatId]/components/Thread.tsx(3 hunks)app/(main)/chat/[chatId]/runtime.ts(3 hunks)app/(main)/newchat/components/step1.tsx(1 hunks)components/agent-creation/ChatPanel.tsx(1 hunks)components/chat/ContextUsageIndicator.tsx(1 hunks)components/chat/DocumentAttachmentCard.tsx(1 hunks)components/chat/ValidationErrorModal.tsx(1 hunks)lib/types/attachment.ts(1 hunks)lib/utils/fileTypes.ts(1 hunks)services/ChatService.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use PascalCase for component names and camelCase for utility names
Order imports: React/Next imports first, then components, then utilities
Use Next.js error boundaries, optional chaining, and nullish coalescing for error handling
Files:
components/chat/DocumentAttachmentCard.tsxcomponents/agent-creation/ChatPanel.tsxlib/utils/fileTypes.tscomponents/chat/ContextUsageIndicator.tsxservices/ChatService.tsapp/(main)/chat/[chatId]/components/MessageComposer.tsxcomponents/chat/ValidationErrorModal.tsxapp/(main)/chat/[chatId]/components/Thread.tsxlib/types/attachment.tsapp/(main)/newchat/components/step1.tsxapp/(main)/chat/[chatId]/runtime.tsapp/(auth)/utils/github-oauth.ts
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: Use Tailwind CSS for styling and thecnutility for conditional classes
Use functional components with TypeScript FC types
Prefer destructuring props at the component level
Use the "use client" directive for client components
Files:
components/chat/DocumentAttachmentCard.tsxcomponents/agent-creation/ChatPanel.tsxcomponents/chat/ContextUsageIndicator.tsxapp/(main)/chat/[chatId]/components/MessageComposer.tsxcomponents/chat/ValidationErrorModal.tsxapp/(main)/chat/[chatId]/components/Thread.tsxapp/(main)/newchat/components/step1.tsx
🧬 Code graph analysis (6)
components/chat/DocumentAttachmentCard.tsx (3)
lib/types/attachment.ts (1)
DocumentAttachment(79-83)lib/utils/fileTypes.ts (2)
getFileTypeInfo(46-54)formatFileSize(56-60)lib/utils.ts (1)
cn(4-6)
components/chat/ContextUsageIndicator.tsx (3)
lib/types/attachment.ts (1)
ContextUsageResponse(62-76)services/ChatService.ts (1)
ChatService(8-833)lib/utils.ts (1)
cn(4-6)
services/ChatService.ts (1)
lib/types/attachment.ts (4)
ValidationResponse(4-18)AttachmentUploadResponse(20-26)AttachmentInfo(51-60)ContextUsageResponse(62-76)
app/(main)/chat/[chatId]/components/MessageComposer.tsx (6)
lib/types/attachment.ts (2)
DocumentAttachment(79-83)ValidationResponse(4-18)lib/utils/fileTypes.ts (4)
MAX_FILE_SIZE(36-36)isDocumentType(38-40)isImageType(42-44)getSupportedFileExtensions(78-82)services/ChatService.ts (1)
ChatService(8-833)components/chat/ValidationErrorModal.tsx (1)
ValidationErrorModal(24-134)components/chat/ContextUsageIndicator.tsx (1)
ContextUsageIndicator(19-135)components/chat/DocumentAttachmentCard.tsx (1)
DocumentAttachmentCard(15-90)
components/chat/ValidationErrorModal.tsx (4)
lib/types/attachment.ts (1)
ValidationResponse(4-18)components/ui/dialog.tsx (5)
Dialog(113-113)DialogContent(118-118)DialogHeader(119-119)DialogTitle(121-121)DialogDescription(122-122)lib/utils/fileTypes.ts (1)
formatFileSize(56-60)components/ui/button.tsx (1)
Button(56-56)
app/(main)/chat/[chatId]/runtime.ts (1)
services/ChatService.ts (1)
ChatService(8-833)
🔇 Additional comments (6)
app/(main)/newchat/components/step1.tsx (1)
490-500: LGTM - Consistent repository linking across environments.Making the "Link new repository" option always available (regardless of environment) is a good change. This allows users to link repositories via the GitHub app in all environments, while environment-specific options (public repos for production, local repos for localhost) remain appropriately gated.
app/(auth)/utils/github-oauth.ts (1)
28-48: Well-structured OAuth flow with proper early returns.The function correctly handles the already-linked scenario with an early return and provides a
forceReauthoption for token refresh. The scope configuration is appropriate for GitHub integration.lib/utils/fileTypes.ts (1)
1-44: Well-organized file type definitions and predicates.The type definitions for documents and images are comprehensive and well-structured with appropriate metadata (extensions, icons, labels). The predicate functions provide clean type checking.
app/(main)/chat/[chatId]/runtime.ts (2)
297-306: Good debug logging for document attachment flow.The debug logs will help troubleshoot issues with document attachments. The extraction and passing of documentIds to ChatService.streamMessage correctly integrates document support into the messaging pipeline.
234-234: Empty attachment array is appropriate for onMessage.The
onMessagefunction doesn't handle attachments yet, so passing an empty array maintains API compatibility while keeping the function focused on simple text messages.components/agent-creation/ChatPanel.tsx (1)
187-208: Attachment parameter wiring LGTMAdding the empty attachment array keeps this call site in sync with the expanded
ChatService.streamMessagesignature—no issues spotted.
| const credential = GithubAuthProvider.credentialFromResult(result); | ||
| const accessToken = | ||
| credential?.accessToken ?? | ||
| (result as any)?._tokenResponse?.oauthAccessToken ?? | ||
| null; | ||
|
|
||
| const providerUsername = (result as any)?._tokenResponse?.screenName; |
There was a problem hiding this comment.
🧩 Analysis chain
Accessing undocumented Firebase internal APIs.
Lines 58 and 61 access internal Firebase response fields via (result as any)?._tokenResponse, which are not part of the documented Firebase API. These fields could change or be removed in future Firebase SDK updates, potentially breaking this functionality.
Consider these alternatives:
- Use only the documented
credential?.accessToken(line 57) and remove the fallback to_tokenResponse.oauthAccessToken - If the fallback is necessary, add a comment explaining why and add defensive error handling
- For
screenName, consider fetching the GitHub username via the GitHub API using the access token instead of relying on internal Firebase fields
If the undocumented fields are required, add a TODO comment and error handling:
const accessToken =
credential?.accessToken ??
+ // TODO: Remove fallback to internal API when Firebase SDK provides documented alternative
(result as any)?._tokenResponse?.oauthAccessToken ??
null;
- const providerUsername = (result as any)?._tokenResponse?.screenName;
+ // TODO: Fetch username via GitHub API instead of internal Firebase field
+ const providerUsername = (result as any)?._tokenResponse?.screenName ?? null;I'll verify this review comment by inspecting the actual code and researching Firebase SDK documentation.
🏁 Script executed:
#!/bin/bash
# Verify the exact code at the specified lines
if [ -f "app/(auth)/utils/github-oauth.ts" ]; then
echo "=== Lines 55-61 of app/(auth)/utils/github-oauth.ts ==="
sed -n '55,61p' "app/(auth)/utils/github-oauth.ts"
echo -e "\n=== Full file context (lines 50-70) ==="
sed -n '50,70p' "app/(auth)/utils/github-oauth.ts"
fiLength of output: 1025
🏁 Script executed:
#!/bin/bash
# Search for all usages of _tokenResponse in the codebase
echo "=== Usages of _tokenResponse in codebase ==="
rg -n "_tokenResponse" --type=ts --type=tsx -C2Length of output: 72
🏁 Script executed:
#!/bin/bash
# Check the Firebase/authentication imports and version hints
echo "=== package.json dependencies (firebase) ==="
grep -A 5 -B 5 "firebase" package.json 2>/dev/null || echo "Not found in root package.json"Length of output: 715
🏁 Script executed:
#!/bin/bash
# Search for GitHub OAuth configuration and comments
echo "=== GitHub OAuth context in file ==="
if [ -f "app/(auth)/utils/github-oauth.ts" ]; then
cat "app/(auth)/utils/github-oauth.ts"
fiLength of output: 2050
🌐 Web query:
Firebase SDK GithubAuthProvider credentialFromResult _tokenResponse documented API
💡 Result:
Short answer: _tokenResponse is an internal SDK implementation detail — it is not part of the documented, supported public API. The official docs show GithubAuthProvider.credentialFromResult(userCredential) returns an OAuthCredential (with .accessToken) and do not document or promise any _tokenResponse property. [1][2]
References:
- GithubAuthProvider API (credentialFromResult → OAuthCredential / .accessToken). [1]
- General OAuthProvider / provider credentialFromResult docs (returns OAuthCredential; no _tokenResponse documented). [2]
Sources:
- Firebase JS docs — GithubAuthProvider (credentialFromResult example showing credential.accessToken). [1]
- Firebase JS docs — OAuthProvider / auth API reference (credentialFromResult returns OAuthCredential). [2]
🌐 Web query:
Firebase GitHub OAuth provider screenName username authentication 2024 2025
💡 Result:
Short answer: Firebase's GitHub provider does NOT return the GitHub "username"/"login" (screen name) in user.providerData. It returns displayName, email, photoURL and a numeric GitHub uid. To get the GitHub login you must obtain the provider OAuth access token from the sign-in result and call the GitHub API (GET https://api.github.com/user) to read the "login" field.
How (brief):
- After sign-in get the OAuth credential and access token (e.g. GithubAuthProvider.credentialFromResult(result).accessToken or result.credential.accessToken). [1][2]
- Call GitHub API /user with Authorization: Bearer and read the "login" property. [2]
- Note: providerData only exposes providerId, uid (numeric), displayName, email, photoURL — not the GitHub login. [3]
Sources: Firebase docs on GitHub sign-in and credentials / providerData and a community explanation. [1] [2] [3]
References
[1] Firebase — GitHub sign-in docs (access token / credential examples).
[2] Firebase JS API reference (GithubAuthProvider / OAuthCredential examples).
[3] StackOverflow — explanation that providerData lacks GitHub username and you must call GitHub API to resolve it.
Remove reliance on undocumented Firebase internal APIs _tokenResponse.
The _tokenResponse object is an internal SDK implementation detail, not part of the documented public API. Line 58 (fallback to _tokenResponse?.oauthAccessToken) and line 61 (_tokenResponse?.screenName) create maintenance risk and should be replaced with documented alternatives:
- Line 58: The documented
credential?.accessTokenfromOAuthCredentialis the supported public API; remove the fallback to_tokenResponse.oauthAccessToken. - Line 61: Firebase's GitHub provider does not return the GitHub username (
screenName) in documented fields. Fetch the GitHub login via the GitHub API using the access token (GET https://api.github.com/user), or removeproviderUsernameif not required.
Replace with:
const credential = GithubAuthProvider.credentialFromResult(result);
- const accessToken =
- credential?.accessToken ??
- (result as any)?._tokenResponse?.oauthAccessToken ??
- null;
+ const accessToken = credential?.accessToken ?? null;
- const providerUsername = (result as any)?._tokenResponse?.screenName;
+ // Fetch GitHub username via GitHub API if required; Firebase does not expose login/screenName
+ const providerUsername = undefined;📝 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 credential = GithubAuthProvider.credentialFromResult(result); | |
| const accessToken = | |
| credential?.accessToken ?? | |
| (result as any)?._tokenResponse?.oauthAccessToken ?? | |
| null; | |
| const providerUsername = (result as any)?._tokenResponse?.screenName; | |
| const credential = GithubAuthProvider.credentialFromResult(result); | |
| const accessToken = credential?.accessToken ?? null; | |
| // Fetch GitHub username via GitHub API if required; Firebase does not expose login/screenName | |
| const providerUsername = undefined; |
🤖 Prompt for AI Agents
In app/(auth)/utils/github-oauth.ts around lines 55 to 61, remove reliance on
the undocumented internal _tokenResponse: use only the documented
OAuthCredential public API (i.e. credential?.accessToken) and drop the fallback
to (result as any)?._tokenResponse?.oauthAccessToken; likewise remove reading
providerUsername from _tokenResponse; if you actually need the GitHub username,
perform an authenticated GET to https://api.github.com/user using the access
token (handle missing/null token and errors, parse the "login" field) instead of
accessing internal SDK fields, otherwise delete providerUsername entirely and
propagate null/undefined appropriately.
There was a problem hiding this comment.
does oauth changes required for documents support?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any usage of ensureGithubOAuth in the codebase
echo "=== Searching for usages of ensureGithubOAuth ==="
rg -n "ensureGithubOAuth" --type=ts --type=tsx -C3
echo -e "\n=== Searching for imports from github-oauth.ts ==="
rg -n "from.*github-oauth" --type=ts --type=tsx -C2
echo -e "\n=== Check if there are any connections between OAuth and document features ==="
rg -n "github.*oauth.*(document|attachment|upload)" -i --type=ts --type=tsx
echo -e "\n=== Check what other files were modified in this PR ==="
git diff --name-only origin/main...HEAD 2>/dev/null | head -20
Length of output: 949
🏁 Script executed:
#!/bin/bash
# Search without file type restriction
echo "=== Searching for usages of ensureGithubOAuth ==="
rg -n "ensureGithubOAuth" -C3
echo -e "\n=== Searching for imports from github-oauth ==="
rg -n "from.*github-oauth" -C2
echo -e "\n=== Check link-github and onboarding pages for OAuth usage ==="
echo "--- link-github/page.tsx ---"
cat "app/(auth)/link-github/page.tsx" 2>/dev/null | head -80
echo -e "\n--- onboarding/page.tsx ---"
cat "app/(auth)/onboarding/page.tsx" 2>/dev/null | head -80
Length of output: 6183
@yashkrishan No, the OAuth changes are not required for document support.
Based on my search of the codebase:
- The new
ensureGithubOAuthutility is not used anywhere - there are no imports or usages of this function in the codebase - Document features are completely independent - they rely on file upload APIs, validation, and attachment handling through
ChatService(validateDocument, uploadAttachment, etc.) - Existing OAuth flows don't use the new utility - the
link-githubandonboardingpages still use Firebase'slinkWithPopupdirectly
It appears that the OAuth refactoring (github-oauth.ts creation and related page updates) was bundled into this PR alongside the document support feature, but they are separate, unrelated changes. The document support functionality works entirely through the file upload and attachment APIs and doesn't require any GitHub OAuth integration.
| const handleSend = () => { | ||
| // For now, we'll handle images through the runtime custom config | ||
| // The actual image handling will be done in the runtime when onNew is called | ||
|
|
||
| // Ensure config is set with current documents before sending | ||
| const documentIds = documents.map(doc => doc.id); | ||
| console.log('[MessageComposer] handleSend - documentIds:', documentIds); | ||
| console.log('[MessageComposer] handleSend - documents:', documents); | ||
|
|
||
| composer.setRunConfig({ | ||
| custom: { | ||
| selectedNodes: selectedNodes, | ||
| images: images, | ||
| documentIds: documentIds, | ||
| documents: documents // Pass full document objects for display | ||
| } | ||
| }); | ||
|
|
||
| composer.send(); | ||
| setMessage(""); | ||
| setSelectedNodes([]); | ||
| setImages([]); | ||
| setImagePreviews([]); | ||
| setDocuments([]); | ||
| }; |
There was a problem hiding this comment.
Keep send disabled while uploads are in flight
Right now handleSend can fire while validating or uploadingDocument are still true. If the user clicks “Send” during that window, the message goes out without the document because the upload hasn’t finished populating documents. Please gate the send path (and button) on those flags so attachments always accompany the message.
🤖 Prompt for AI Agents
In app/(main)/chat/[chatId]/components/MessageComposer.tsx around lines 297 to
318, the handleSend path can run while validating or uploadingDocument are true
which results in messages being sent without attachments; guard the send path by
checking those flags before calling composer.send: if validating ||
uploadingDocument then return early (and ensure the Send button is disabled when
either flag is true); also avoid clearing message/images/documents state until
after a successful send (or composer confirms send) so uploads complete and
documents are included.
| import React, { FC, useEffect, useState } from 'react'; | ||
| import { ContextUsageResponse } from '@/lib/types/attachment'; | ||
| import ChatService from '@/services/ChatService'; | ||
| import { cn } from '@/lib/utils'; | ||
| import { AlertTriangle } from 'lucide-react'; | ||
| import { | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipProvider, | ||
| TooltipTrigger, | ||
| } from '@/components/ui/tooltip'; | ||
|
|
||
| interface ContextUsageIndicatorProps { | ||
| conversationId: string; | ||
| className?: string; | ||
| refreshInterval?: number; // ms | ||
| } | ||
|
|
||
| export const ContextUsageIndicator: FC<ContextUsageIndicatorProps> = ({ | ||
| conversationId, | ||
| className, | ||
| refreshInterval = 30000, // 30 seconds default | ||
| }) => { | ||
| const [usage, setUsage] = useState<ContextUsageResponse | null>(null); | ||
| const [loading, setLoading] = useState(true); | ||
|
|
||
| useEffect(() => { | ||
| const fetchUsage = async () => { | ||
| try { | ||
| const data = await ChatService.getContextUsage(conversationId); | ||
| setUsage(data); | ||
| } catch (error) { | ||
| console.error('Error fetching context usage:', error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchUsage(); | ||
| const interval = setInterval(fetchUsage, refreshInterval); | ||
| return () => clearInterval(interval); | ||
| }, [conversationId, refreshInterval]); | ||
|
|
||
| if (loading || !usage) return null; | ||
|
|
||
| const getBarColor = () => { | ||
| if (usage.warning_level === 'critical') return 'bg-red-500'; | ||
| if (usage.warning_level === 'approaching') return 'bg-orange-500'; | ||
| return 'bg-green-500'; | ||
| }; | ||
|
|
||
| const getTextColor = () => { | ||
| if (usage.warning_level === 'critical') return 'text-red-700'; | ||
| if (usage.warning_level === 'approaching') return 'text-orange-700'; | ||
| return 'text-gray-700'; | ||
| }; | ||
|
|
||
| return ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> |
There was a problem hiding this comment.
Add the 'use client' directive
This module uses React hooks and runs in the browser. Without 'use client' at the top, Next.js will treat it as a Server Component and the build will fail once imported from a Client Component (per our coding guidelines). Please add the directive.
Apply this diff:
+'use client';
+
import React, { FC, useEffect, useState } from 'react';📝 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.
| import React, { FC, useEffect, useState } from 'react'; | |
| import { ContextUsageResponse } from '@/lib/types/attachment'; | |
| import ChatService from '@/services/ChatService'; | |
| import { cn } from '@/lib/utils'; | |
| import { AlertTriangle } from 'lucide-react'; | |
| import { | |
| Tooltip, | |
| TooltipContent, | |
| TooltipProvider, | |
| TooltipTrigger, | |
| } from '@/components/ui/tooltip'; | |
| interface ContextUsageIndicatorProps { | |
| conversationId: string; | |
| className?: string; | |
| refreshInterval?: number; // ms | |
| } | |
| export const ContextUsageIndicator: FC<ContextUsageIndicatorProps> = ({ | |
| conversationId, | |
| className, | |
| refreshInterval = 30000, // 30 seconds default | |
| }) => { | |
| const [usage, setUsage] = useState<ContextUsageResponse | null>(null); | |
| const [loading, setLoading] = useState(true); | |
| useEffect(() => { | |
| const fetchUsage = async () => { | |
| try { | |
| const data = await ChatService.getContextUsage(conversationId); | |
| setUsage(data); | |
| } catch (error) { | |
| console.error('Error fetching context usage:', error); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| fetchUsage(); | |
| const interval = setInterval(fetchUsage, refreshInterval); | |
| return () => clearInterval(interval); | |
| }, [conversationId, refreshInterval]); | |
| if (loading || !usage) return null; | |
| const getBarColor = () => { | |
| if (usage.warning_level === 'critical') return 'bg-red-500'; | |
| if (usage.warning_level === 'approaching') return 'bg-orange-500'; | |
| return 'bg-green-500'; | |
| }; | |
| const getTextColor = () => { | |
| if (usage.warning_level === 'critical') return 'text-red-700'; | |
| if (usage.warning_level === 'approaching') return 'text-orange-700'; | |
| return 'text-gray-700'; | |
| }; | |
| return ( | |
| <TooltipProvider> | |
| <Tooltip> | |
| <TooltipTrigger asChild> | |
| 'use client'; | |
| import React, { FC, useEffect, useState } from 'react'; | |
| import { ContextUsageResponse } from '@/lib/types/attachment'; | |
| import ChatService from '@/services/ChatService'; | |
| import { cn } from '@/lib/utils'; | |
| import { AlertTriangle } from 'lucide-react'; | |
| import { | |
| Tooltip, | |
| TooltipContent, | |
| TooltipProvider, | |
| TooltipTrigger, | |
| } from '@/components/ui/tooltip'; | |
| interface ContextUsageIndicatorProps { | |
| conversationId: string; | |
| className?: string; | |
| refreshInterval?: number; // ms | |
| } | |
| export const ContextUsageIndicator: FC<ContextUsageIndicatorProps> = ({ | |
| conversationId, | |
| className, | |
| refreshInterval = 30000, // 30 seconds default | |
| }) => { | |
| const [usage, setUsage] = useState<ContextUsageResponse | null>(null); | |
| const [loading, setLoading] = useState(true); | |
| useEffect(() => { | |
| const fetchUsage = async () => { | |
| try { | |
| const data = await ChatService.getContextUsage(conversationId); | |
| setUsage(data); | |
| } catch (error) { | |
| console.error('Error fetching context usage:', error); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| fetchUsage(); | |
| const interval = setInterval(fetchUsage, refreshInterval); | |
| return () => clearInterval(interval); | |
| }, [conversationId, refreshInterval]); | |
| if (loading || !usage) return null; | |
| const getBarColor = () => { | |
| if (usage.warning_level === 'critical') return 'bg-red-500'; | |
| if (usage.warning_level === 'approaching') return 'bg-orange-500'; | |
| return 'bg-green-500'; | |
| }; | |
| const getTextColor = () => { | |
| if (usage.warning_level === 'critical') return 'text-red-700'; | |
| if (usage.warning_level === 'approaching') return 'text-orange-700'; | |
| return 'text-gray-700'; | |
| }; | |
| return ( | |
| <TooltipProvider> | |
| <Tooltip> | |
| <TooltipTrigger asChild> |
🤖 Prompt for AI Agents
In components/chat/ContextUsageIndicator.tsx around lines 1 to 61, the file uses
React hooks and must be a client component but is missing the 'use client'
directive; add a single line containing "use client" (with quotes) at the very
top of the file (before any imports) so Next.js treats this as a client
component and the build will succeed.
| useEffect(() => { | ||
| const fetchUsage = async () => { | ||
| try { | ||
| const data = await ChatService.getContextUsage(conversationId); | ||
| setUsage(data); | ||
| } catch (error) { | ||
| console.error('Error fetching context usage:', error); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchUsage(); | ||
| const interval = setInterval(fetchUsage, refreshInterval); | ||
| return () => clearInterval(interval); | ||
| }, [conversationId, refreshInterval]); | ||
|
|
||
| if (loading || !usage) return null; | ||
|
|
There was a problem hiding this comment.
Reset loading state when the conversation changes
When conversationId flips, we keep showing the previous chat’s usage until the next fetch resolves. That briefly surfaces stale context data and can mislead the user. Reset loading to true (and optionally clear usage) before kicking off the new fetch so the indicator disappears until fresh data arrives.
🤖 Prompt for AI Agents
In components/chat/ContextUsageIndicator.tsx around lines 27 to 45, when
conversationId changes we currently keep showing the previous usage until the
new fetch completes; update the useEffect to immediately setLoading(true) (and
optionally setUsage(null)) before calling fetchUsage so the indicator is cleared
while the new data loads; keep the existing fetchUsage, interval setup/teardown,
and error/finally handling but ensure the initial state reset happens at the top
of the effect whenever conversationId or refreshInterval changes.
| import React, { FC } from 'react'; | ||
| import { | ||
| Dialog, | ||
| DialogContent, | ||
| DialogDescription, | ||
| DialogHeader, | ||
| DialogTitle, | ||
| } from '@/components/ui/dialog'; | ||
| import { Button } from '@/components/ui/button'; | ||
| import { AlertTriangle } from 'lucide-react'; | ||
| import { ValidationResponse } from '@/lib/types/attachment'; | ||
| import { formatFileSize } from '@/lib/utils/fileTypes'; | ||
|
|
||
| interface ValidationErrorModalProps { | ||
| open: boolean; | ||
| onClose: () => void; | ||
| validation: ValidationResponse | null; | ||
| fileName: string; | ||
| fileSize: number; | ||
| onViewAttachments?: () => void; | ||
| onChangeModel?: () => void; | ||
| } | ||
|
|
||
| export const ValidationErrorModal: FC<ValidationErrorModalProps> = ({ | ||
| open, | ||
| onClose, | ||
| validation, | ||
| fileName, | ||
| fileSize, | ||
| onViewAttachments, | ||
| onChangeModel, | ||
| }) => { | ||
| if (!validation || validation.can_upload) return null; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onClose}> | ||
| <DialogContent className="max-w-md"> |
There was a problem hiding this comment.
Mark the modal as a Client Component
This modal relies on event handlers and UI components meant for the browser. Add 'use client' at the top so Next.js treats it as a Client Component; otherwise the build will flag it when imported.
Apply this diff:
+'use client';
+
import React, { FC } from 'react';🤖 Prompt for AI Agents
In components/chat/ValidationErrorModal.tsx lines 1 to 37, the file uses event
handlers and browser-only UI components but is not marked as a Next.js Client
Component; add the directive 'use client' as the very first line of the file so
Next treats it as a client component and the build error is resolved.
| export function estimateTokens(file: File): number { | ||
| const mimeType = file.type; | ||
| const sizeBytes = file.size; | ||
|
|
||
| // Server-side heuristics | ||
| if (mimeType.includes('pdf')) { | ||
| return Math.floor(sizeBytes / 5); | ||
| } else if (mimeType.includes('word') || mimeType.includes('document')) { | ||
| return Math.floor(sizeBytes / 5); | ||
| } else if (mimeType.includes('csv') || mimeType.includes('spreadsheet')) { | ||
| return Math.floor(sizeBytes / 6); | ||
| } else { | ||
| return Math.floor(sizeBytes / 4); // Text/code | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Document the token estimation as a rough approximation.
The estimateTokens function uses rough heuristics (dividing file size by 4-6 depending on type) that may not accurately reflect actual token counts. This should be documented to set proper expectations.
Add JSDoc documentation to clarify this is an approximation:
+/**
+ * Estimates token count for a file using rough heuristics based on file size.
+ * NOTE: This is a rough client-side approximation. Actual token counts
+ * will be calculated server-side and may differ significantly.
+ *
+ * Heuristics:
+ * - PDF/Word: ~1 token per 5 bytes
+ * - CSV/Spreadsheet: ~1 token per 6 bytes
+ * - Text/Code: ~1 token per 4 bytes
+ */
export function estimateTokens(file: File): number {🤖 Prompt for AI Agents
In lib/utils/fileTypes.ts around lines 62 to 76, the estimateTokens function
uses simple heuristics (dividing file size by 4–6) and lacks documentation
clarifying these are rough approximations; add a JSDoc comment above the
function that states this is an approximate token estimate, explains the
heuristic per MIME type, warns it may diverge from real tokenizer counts, and
suggests using a real tokenizer for accurate counts; keep the comment concise
and include parameter and return descriptions.
| static async validateDocument( | ||
| conversationId: string, | ||
| file: File | ||
| ): Promise<ValidationResponse> { | ||
| const headers = await getHeaders(); | ||
| const formData = new FormData(); | ||
|
|
||
| formData.append('conversation_id', conversationId); | ||
| formData.append('file_size', file.size.toString()); | ||
| formData.append('file_name', file.name); | ||
| formData.append('mime_type', file.type); | ||
|
|
||
| try { | ||
| const response = await axios.post( | ||
| `${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/validate-document`, | ||
| formData, | ||
| { headers } | ||
| ); | ||
| return response.data; | ||
| } catch (error: any) { | ||
| console.error('Document validation error:', error); | ||
| throw new Error( | ||
| error.response?.data?.detail || 'Failed to validate document' | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix multipart headers in validateDocument
validateDocument builds FormData, but we’re still forwarding the default headers from getHeaders(), which include Content-Type: application/json. With that header in place the browser won’t set the multipart boundary, so the backend will see the payload as malformed and the validation call fails. In uploadAttachment you already strip the content type for this reason; we need the same treatment here.
static async validateDocument(
conversationId: string,
file: File
): Promise<ValidationResponse> {
- const headers = await getHeaders();
+ const headers = await getHeaders();
+ const formHeaders = { ...headers };
+ delete (formHeaders as Record<string, string>)["Content-Type"];
@@
const response = await axios.post(
`${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/validate-document`,
formData,
- { headers }
+ { headers: formHeaders }
);📝 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.
| static async validateDocument( | |
| conversationId: string, | |
| file: File | |
| ): Promise<ValidationResponse> { | |
| const headers = await getHeaders(); | |
| const formData = new FormData(); | |
| formData.append('conversation_id', conversationId); | |
| formData.append('file_size', file.size.toString()); | |
| formData.append('file_name', file.name); | |
| formData.append('mime_type', file.type); | |
| try { | |
| const response = await axios.post( | |
| `${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/validate-document`, | |
| formData, | |
| { headers } | |
| ); | |
| return response.data; | |
| } catch (error: any) { | |
| console.error('Document validation error:', error); | |
| throw new Error( | |
| error.response?.data?.detail || 'Failed to validate document' | |
| ); | |
| } | |
| } | |
| static async validateDocument( | |
| conversationId: string, | |
| file: File | |
| ): Promise<ValidationResponse> { | |
| const headers = await getHeaders(); | |
| const formHeaders = { ...headers }; | |
| delete (formHeaders as Record<string, string>)["Content-Type"]; | |
| const formData = new FormData(); | |
| formData.append('conversation_id', conversationId); | |
| formData.append('file_size', file.size.toString()); | |
| formData.append('file_name', file.name); | |
| formData.append('mime_type', file.type); | |
| try { | |
| const response = await axios.post( | |
| `${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/validate-document`, | |
| formData, | |
| { headers: formHeaders } | |
| ); | |
| return response.data; | |
| } catch (error: any) { | |
| console.error('Document validation error:', error); | |
| throw new Error( | |
| error.response?.data?.detail || 'Failed to validate document' | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In services/ChatService.ts around lines 702 to 727, the validateDocument call
builds FormData but passes headers from getHeaders() that include Content-Type:
application/json which prevents the browser/axios from setting the multipart
boundary; remove the Content-Type header before making the POST (e.g., delete
headers['Content-Type'] and delete headers['content-type'] to be safe) and then
call axios.post with the cleaned headers so axios can set the correct multipart
Content-Type with boundary.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
lib/utils/fileTypes.ts (1)
123-137: UsegetEffectiveMimeTypefor consistency.
estimateTokensusesfile.typedirectly, but other functions in this file usegetEffectiveMimeTypeto handle browsers that report empty or generic MIME types. This inconsistency could lead to incorrect token estimates for files where the browser doesn't provide a specific MIME type.export function estimateTokens(file: File): number { - const mimeType = file.type; + const mimeType = getEffectiveMimeType(file); const sizeBytes = file.size;Note: The past review comment about documenting this as a rough approximation is still valid.
components/chat/ContextUsageIndicator.tsx (2)
1-11: Add the'use client'directive.This component uses React hooks (
useState,useEffect) and must be marked as a client component. Without this directive, Next.js will treat it as a Server Component and the build will fail. As per coding guidelines, use the "use client" directive for client components.+'use client'; + import React, { FC, useEffect, useState } from 'react';
40-55: Reset state whenconversationIdchanges.When
conversationIdchanges, the component continues showing stale data until the new fetch completes. Resetloadingtotrueand clearusageat the start of the effect.useEffect(() => { + setLoading(true); + setUsage(null); + const fetchUsage = async () => {services/ChatService.ts (1)
702-727: Fix multipart headers invalidateDocumentso FormData works
validateDocumentbuilds aFormDatabody but passes the rawheadersfromgetHeaders(), which almost certainly includeContent-Type: application/json. With that header set, axios won’t add themultipart/form-databoundary, and the backend will see a malformed payload. This is the same issue called out in the earlier review and still needs fixing.You already handle this correctly in
uploadAttachment. Mirror that here by cloning and cleaning the headers, deleting bothContent-Typeandcontent-typebefore callingaxios.post:static async validateDocument( conversationId: string, file: File ): Promise<ValidationResponse> { - const headers = await getHeaders(); + const headers = await getHeaders(); + const formHeaders: Record<string, string> = { ...(headers as Record<string, string>) }; + delete formHeaders["Content-Type"]; + delete formHeaders["content-type"]; @@ - const response = await axios.post( + const response = await axios.post( `${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/validate-document`, formData, - { headers } + { headers: formHeaders } );app/(main)/chat/[chatId]/components/MessageComposer.tsx (1)
119-129: Still need to block send while document validation/upload is in flightYou’ve added
documents,validating,uploadingDocument,uploadLocked, etc., buthandleSendand the Send button can still fire while a document is being validated or uploaded. In that window, the message is sent without the just-selected document becausedocumentshasn’t been updated with the new attachment yet. This is the same race condition flagged in the previous review.Consider:
- Introducing a single flag (e.g.
const sendDisabled = isDisabled || validating || uploadingDocument || uploadLocked;) after the relevantuseStatedeclarations.- Using it for:
The send button and
ThreadPrimitive.If running={false}:<ThreadPrimitive.If running={false}>
<ThreadPrimitive.If running={false}> <button type="button"
disabled={isDisabled}
disabled={sendDisabled}@@
onClick={handleSend}
onClick={handleSend} >- The composer input’s `submitOnEnter`: ```diff
<ComposerPrimitive.InputsubmitOnEnter={!isDisabled}
<ComposerPrimitive.InputsubmitOnEnter={!sendDisabled}- An early guard in `handleSend`: ```diff const handleSend = () => {
- // Ensure config is set with current documents before sending
- if (sendDisabled) {
toast.info("Please wait", {description: "Document validation/upload is still in progress.",});return;- }
- // Ensure config is set with current documents before sending
const documentIds = documents.map(doc => doc.id);
```This keeps the UX consistent: users can’t send until validation/upload has finished, and you avoid messages going out without their intended documents.
Also applies to: 324-345, 621-650, 861-906
🧹 Nitpick comments (7)
lib/utils.ts (1)
153-161: Consider moving imports to the top of the file.The
dayjsimports are placed mid-file after function definitions. Per convention, imports should be grouped at the top of the file for better readability and maintainability. As per coding guidelines, order imports: React/Next imports first, then components, then utilities.import { type ClassValue, clsx } from "clsx"; import { twMerge } from "tailwind-merge"; +import dayjs from "dayjs"; +import utc from "dayjs/plugin/utc"; +import timezone from "dayjs/plugin/timezone"; +import relativeTime from "dayjs/plugin/relativeTime"; + +dayjs.extend(utc); +dayjs.extend(timezone); +dayjs.extend(relativeTime); export function cn(...inputs: ClassValue[]) {Then remove the duplicate imports from lines 153-161.
components/chat/ContextUsageIndicator.tsx (1)
140-154: Extract nested ternaries for readability.The nested ternary operations flagged by SonarCloud reduce readability. Consider extracting to a helper function.
+const getHeaderBgClass = (warningLevel: string) => { + if (warningLevel === 'critical') return 'bg-red-50'; + if (warningLevel === 'approaching') return 'bg-amber-50'; + return 'bg-gray-50'; +}; + +const getWarningTextClass = (warningLevel: string) => { + if (warningLevel === 'critical') return 'text-red-600'; + if (warningLevel === 'approaching') return 'text-amber-600'; + return 'text-gray-600'; +};Then use
getHeaderBgClass(usage.warning_level)at line 142-144 andgetWarningTextClass(usage.warning_level)at lines 149-150.app/(main)/chat/[chatId]/page.tsx (1)
130-134: Consider inverting the condition for clarity.Per SonarCloud, negated conditions can reduce readability. Consider swapping the branches.
- if (!list_system_agents.includes(agentId)) { - setChatAccess(info.access_type); - } else { + if (list_system_agents.includes(agentId)) { setChatAccess(info.is_creator ? "write" : info.access_type); + } else { + setChatAccess(info.access_type); }services/ChatService.ts (3)
169-205: AttachmentIds default param ordering and noisy logging instreamMessage
- Sonar is right that having defaulted params (
images = [],attachmentIds: string[] = []) before a required callback andsessionIdmakes the signature harder to read and easier to misuse. Consider either moving these defaulted params aftersessionIdor making them optional without defaults to align with the “default parameters last” rule.- The
console.log/console.warncalls aroundattachmentIdswill generate a lot of noise in production; it’s worth either gating them behind a debug flag or removing them once you’re confident in the flow.
729-790: Harden upload headers and handle metadata failures more gracefullyTwo minor but worthwhile robustness points here:
- For
uploadAttachment, you only delete'Content-Type'fromuploadHeaders. IfgetHeaders()ever returns lowercase'content-type', the stale header will remain and can break multipart uploads. It’s safer to delete both keys:const uploadHeaders = { ...headers }; - delete (uploadHeaders as any)['Content-Type']; + delete (uploadHeaders as any)['Content-Type']; + delete (uploadHeaders as any)['content-type'];
- If
getAttachmentInfofails after a successful upload, you currently treat it as “Upload failed” and drop the attachment entirely. That can create “ghost” uploads on the backend and forces the user to repeat the upload. Consider:
- catching metadata failures separately,
- still adding an attachment entry based on
uploadResult(withtoken_count/metadataundefined or defaulted), and- surfacing a softer toast like “Uploaded, but token details are unavailable right now.”
This would give a better UX and avoid wasted uploads while still signaling that enrichment failed.
813-843: MakedownloadAttachmentbrowser-safe and address Sonar’s global/window warnings
downloadAttachmentassumes a browser environment (window.URL,document.body.removeChild), and Sonar is flagging bothwindowusage andremoveChild. You can both guard this as browser-only and satisfy the linter:static async downloadAttachment( attachmentId: string, fileName: string ): Promise<void> { - const headers = await getHeaders(); - - try { - const response = await axios.get( + const headers = await getHeaders(); + + // Guard against accidental server-side usage + if (typeof window === "undefined" || typeof document === "undefined") { + throw new Error("downloadAttachment can only be used in a browser environment"); + } + + try { + const response = await axios.get( `${process.env.NEXT_PUBLIC_CONVERSATION_BASE_URL}/api/v1/media/${attachmentId}/download`, { headers, responseType: 'blob' } ); - // Create download link - const url = window.URL.createObjectURL(response.data); + // Create download link + const url = globalThis.URL.createObjectURL(response.data as Blob); const a = document.createElement('a'); a.href = url; a.download = fileName; document.body.appendChild(a); a.click(); - window.URL.revokeObjectURL(url); - document.body.removeChild(a); + globalThis.URL.revokeObjectURL(url); + a.remove();That keeps this helper clearly client-only and aligns with the static analysis recommendations.
app/(main)/chat/[chatId]/components/MessageComposer.tsx (1)
49-58: Remove unused file-type utilities to keep imports clean
isDocumentType,isImageType, anddetectFileTypeByExtensionare imported from@/lib/utils/fileTypesbut never used in this component. Sonar is already flagging these; dropping them will clean up the file and satisfy the analyzer:-import { - isDocumentType, - isImageType, - isDocumentTypeByFile, - isImageTypeByFile, - detectFileTypeByExtension, - MAX_FILE_SIZE, - getSupportedFileExtensions -} from "@/lib/utils/fileTypes"; +import { + isDocumentTypeByFile, + isImageTypeByFile, + MAX_FILE_SIZE, + getSupportedFileExtensions +} from "@/lib/utils/fileTypes";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/(main)/chat/[chatId]/components/MessageComposer.tsx(11 hunks)app/(main)/chat/[chatId]/page.tsx(2 hunks)components/chat/ContextUsageIndicator.tsx(1 hunks)components/chat/DocumentAttachmentCard.tsx(1 hunks)lib/utils.ts(2 hunks)lib/utils/fileTypes.ts(1 hunks)services/ChatService.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use PascalCase for component names and camelCase for utility names
Order imports: React/Next imports first, then components, then utilities
Use Next.js error boundaries, optional chaining, and nullish coalescing for error handling
Files:
lib/utils.tsservices/ChatService.tscomponents/chat/ContextUsageIndicator.tsxapp/(main)/chat/[chatId]/page.tsxlib/utils/fileTypes.tsapp/(main)/chat/[chatId]/components/MessageComposer.tsxcomponents/chat/DocumentAttachmentCard.tsx
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.tsx: Use Tailwind CSS for styling and thecnutility for conditional classes
Use functional components with TypeScript FC types
Prefer destructuring props at the component level
Use the "use client" directive for client components
Files:
components/chat/ContextUsageIndicator.tsxapp/(main)/chat/[chatId]/page.tsxapp/(main)/chat/[chatId]/components/MessageComposer.tsxcomponents/chat/DocumentAttachmentCard.tsx
🧠 Learnings (2)
📚 Learning: 2025-07-22T09:20:21.427Z
Learnt from: CR
Repo: potpie-ai/potpie-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T09:20:21.427Z
Learning: Applies to **/*.tsx : Use the "use client" directive for client components
Applied to files:
components/chat/ContextUsageIndicator.tsx
📚 Learning: 2025-07-22T09:20:21.427Z
Learnt from: CR
Repo: potpie-ai/potpie-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T09:20:21.427Z
Learning: Applies to **/*.{ts,tsx} : Use Next.js error boundaries, optional chaining, and nullish coalescing for error handling
Applied to files:
components/chat/ContextUsageIndicator.tsx
🧬 Code graph analysis (2)
services/ChatService.ts (1)
lib/types/attachment.ts (4)
ValidationResponse(4-18)AttachmentUploadResponse(20-26)AttachmentInfo(51-60)ContextUsageResponse(62-76)
components/chat/DocumentAttachmentCard.tsx (3)
lib/types/attachment.ts (1)
DocumentAttachment(79-83)lib/utils/fileTypes.ts (2)
getFileTypeInfo(107-115)formatFileSize(117-121)lib/utils.ts (1)
cn(4-6)
🪛 GitHub Check: SonarCloud Code Analysis
services/ChatService.ts
[warning] 169-169: Default parameters should be last.
[warning] 829-829: Prefer globalThis over window.
[warning] 835-835: Prefer globalThis over window.
[warning] 836-836: Prefer childNode.remove() over parentNode.removeChild(childNode).
components/chat/ContextUsageIndicator.tsx
[warning] 150-150: Extract this nested ternary operation into an independent statement.
[warning] 143-143: Extract this nested ternary operation into an independent statement.
app/(main)/chat/[chatId]/page.tsx
[warning] 130-130: Unexpected negated condition.
app/(main)/chat/[chatId]/components/MessageComposer.tsx
[warning] 51-51: Remove this unused import of 'isDocumentType'.
[warning] 55-55: Remove this unused import of 'detectFileTypeByExtension'.
[warning] 52-52: Remove this unused import of 'isImageType'.
components/chat/DocumentAttachmentCard.tsx
[warning] 2-2: Remove this unused import of 'FileText'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build, Deploy, and Comment
🔇 Additional comments (5)
lib/utils.ts (1)
95-149: LGTM!The expanded mock user object correctly includes the Firebase internal methods (
_stopProactiveRefresh,_startProactiveRefresh,stsTokenManager,proactiveRefresh) needed for auth state management compatibility.lib/utils/fileTypes.ts (1)
46-81: Several extensions map to MIME types not inSUPPORTED_DOCUMENT_TYPES.Extensions like
.rb,.php,.c,.cpp,.h,.cs,.swift,.kt,.scala,.sh,.sql,.html,.css,.scss,.less,.vue,.sveltemap to MIME types that aren't inSUPPORTED_DOCUMENT_TYPES. This meansisDocumentTypeByFilewill returnfalsefor these files even though extension detection works. Either add these MIME types toSUPPORTED_DOCUMENT_TYPESor document this as intentional behavior.components/chat/DocumentAttachmentCard.tsx (1)
15-91: LGTM!Well-structured component with:
- Proper TypeScript FC typing and prop destructuring
- Good accessibility with
aria-labelattributes on interactive elements- Tailwind CSS with
cnutility for conditional classes- Clean handling of optional metadata (token_count, page_count, row_count)
app/(main)/chat/[chatId]/page.tsx (2)
115-128: LGTM!Good defensive programming with optional chaining and early return. The error handling provides clear user feedback via toast and sets a structured error state for the UI.
150-154: Good defensive guard forcreator_id.Adding the
info.creator_idcheck prevents unnecessary API calls when the creator ID is missing.
| import React, { FC } from 'react'; | ||
| import { X, Download, FileText } from 'lucide-react'; | ||
| import { DocumentAttachment } from '@/lib/types/attachment'; | ||
| import { formatFileSize, getFileTypeInfo } from '@/lib/utils/fileTypes'; | ||
| import { cn } from '@/lib/utils'; |
There was a problem hiding this comment.
Add 'use client' directive and remove unused import.
This component should be marked as a client component per coding guidelines. Also, FileText is imported but never used (per SonarCloud).
+'use client';
+
import React, { FC } from 'react';
-import { X, Download, FileText } from 'lucide-react';
+import { X, Download } from 'lucide-react';
import { DocumentAttachment } from '@/lib/types/attachment';📝 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.
| import React, { FC } from 'react'; | |
| import { X, Download, FileText } from 'lucide-react'; | |
| import { DocumentAttachment } from '@/lib/types/attachment'; | |
| import { formatFileSize, getFileTypeInfo } from '@/lib/utils/fileTypes'; | |
| import { cn } from '@/lib/utils'; | |
| 'use client'; | |
| import React, { FC } from 'react'; | |
| import { X, Download } from 'lucide-react'; | |
| import { DocumentAttachment } from '@/lib/types/attachment'; | |
| import { formatFileSize, getFileTypeInfo } from '@/lib/utils/fileTypes'; | |
| import { cn } from '@/lib/utils'; |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 2-2: Remove this unused import of 'FileText'.
🤖 Prompt for AI Agents
In components/chat/DocumentAttachmentCard.tsx lines 1-5, add the React client
directive and remove the unused FileText import: put a single line "'use
client'" at the very top of the file (before any imports) and update the
lucide-react import to drop FileText (leave X and Download), ensuring no unused
imports remain.
| export function detectFileTypeByExtension(fileName: string): string | null { | ||
| const ext = '.' + fileName.split('.').pop()?.toLowerCase(); | ||
| return EXTENSION_TO_MIME[ext] || null; | ||
| } |
There was a problem hiding this comment.
Handle filenames without extensions.
If the filename has no dot (e.g., "README"), split('.').pop() returns "README", and prepending '.' creates ".README", which won't match any extension in the map. Consider adding a guard.
export function detectFileTypeByExtension(fileName: string): string | null {
- const ext = '.' + fileName.split('.').pop()?.toLowerCase();
+ const parts = fileName.split('.');
+ if (parts.length < 2) return null;
+ const ext = '.' + parts.pop()?.toLowerCase();
return EXTENSION_TO_MIME[ext] || null;
}📝 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.
| export function detectFileTypeByExtension(fileName: string): string | null { | |
| const ext = '.' + fileName.split('.').pop()?.toLowerCase(); | |
| return EXTENSION_TO_MIME[ext] || null; | |
| } | |
| export function detectFileTypeByExtension(fileName: string): string | null { | |
| const parts = fileName.split('.'); | |
| if (parts.length < 2) return null; | |
| const ext = '.' + parts.pop()?.toLowerCase(); | |
| return EXTENSION_TO_MIME[ext] || null; | |
| } |
🤖 Prompt for AI Agents
In lib/utils/fileTypes.ts around lines 83 to 86, the current logic treats names
without dots (e.g., "README") as if their entire name were an extension; change
it to first check for a real extension by finding the last '.' and ensuring it's
not the leading dot of a hidden file (require lastDot > 0). If no valid dot is
found return null; otherwise compute ext = fileName.slice(lastDot).toLowerCase()
and return EXTENSION_TO_MIME[ext] || null.
|
|
Cloud Run service deployed: https://pr-doc-cj6r7x3fpa-uc.a.run.app |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
lib/utils/emailValidation.ts (1)
268-275: Consider masking PII in error logs.Logging
userIdand🛡️ Suggested approach to mask PII
const errorDetails = { message: 'Failed to delete Firebase user account', error: error?.message || String(error), errorCode: error?.code, - userId: user?.uid, - email: user?.email, + userId: user?.uid ? `${user.uid.substring(0, 4)}...` : undefined, + email: user?.email ? `${user.email.split('@')[0]?.substring(0, 2)}...@${user.email.split('@')[1]}` : undefined, stack: error?.stack, };app/(auth)/link-github/page.tsx (3)
32-32: Analytics event fires on every render, not on user action.
posthog.capture("github login clicked")is called directly in the component body, so it executes on mount and every re-render—not when the user actually clicks. This will produce inflated/misleading analytics.Move it inside
onGithub()to capture the actual click, or wrap inuseEffectwith an empty dependency array if you want to track page views.🐛 Proposed fix
If tracking the click action:
const onGithub = async () => { try { + posthog.capture("github login clicked"); // Capture the currently signed-in user...And remove line 32.
Or if tracking page views:
+ import React, { useRef, useEffect } from "react"; ... const router = useRouter(); const popupRef = useRef<Window | null>(null); - posthog.capture("github login clicked"); + useEffect(() => { + posthog.capture("link github page viewed"); + }, [posthog]);
69-111: Success toast fires before the API call completes.
userSignupis a promise that's never awaited—toast.successon line 109 executes immediately after initiating the request, not after it succeeds. Users see "Account created successfully" even if the request subsequently fails.🐛 Proposed fix
Move the success toast inside the
.then()block:.then((res: { data: any }) => { + toast.success( + "Account created successfully as " + result.user.displayName + ); openPopup(); router.push(`/newchat`); return res.data; }) .catch((e: any) => { // Check for 409 Conflict (GitHub already linked) if (e?.response?.status === 409 && e?.response?.data?.error) { toast.error(e.response.data.error); } else if (e?.response?.data?.error) { toast.error(e.response.data.error); } else { - toast.error("Signup call unsuccessful"); + toast.error("Signup call unsuccessful"); } }); - toast.success( - "Account created successfully as " + result.user.displayName - );
89-90: Relying on internal Firebase_tokenResponseproperty.Accessing
_tokenResponse(underscore-prefixed internal property) viaas anybypasses type safety and may break on Firebase updates without warning. Use the official API instead:const result = await linkWithPopup(originalUser, provider); + const credential = GithubAuthProvider.credentialFromResult(result); + const accessToken = credential?.accessToken; ... - accessToken: (result as any)._tokenResponse.oauthAccessToken, - providerUsername: (result as any)._tokenResponse.screenName, + accessToken: accessToken, + providerUsername: result.user.providerData.find(p => p.providerId === "github.com")?.displayName || "",Note: If the
screenNamevalue is essential and not available fromproviderData, consider fetching it via GitHub's API (GET /user) using the access token obtained above.
🤖 Fix all issues with AI agents
In `@app/`(main)/chat/[chatId]/components/MessageComposer.tsx:
- Around line 576-592: handleEnhancePrompt currently overwrites the user's
message with "Enhancing..." before calling ChatService.enhancePrompt, so if the
API fails the original text is lost; fix it by capturing the current message
into a local variable (e.g., originalMessage) before calling
setMessage/composer.setText, set the UI to the enhancing state using that local
save, and in the catch block restore the original text via
setMessage(originalMessage) and composer.setText(originalMessage) (keep
setIsEnhancing(false) in finally). Reference: handleEnhancePrompt, setMessage,
composer.setText, ChatService.enhancePrompt, setIsEnhancing.
In `@app/`(main)/chat/[chatId]/components/Thread.tsx:
- Around line 386-419: In the documentAttachments map inside the Thread
component (the JSX block mapping documentAttachments in Thread.tsx), replace the
React key currently using the array index with a stable unique identifier from
the attachment object (use attachment.id or another unique id field) so each
mapped <div> uses key={attachment.id} instead of key={index}; ensure you handle
the case where id might be missing by falling back to a stable unique value if
necessary.
🧹 Nitpick comments (10)
components/Layouts/minors/nav-user.tsx (1)
123-130: Consider adding accessibility attributes for screen reader support.The
titleattribute provides a native tooltip, but screen readers may not consistently announce it. For improved accessibility, consider addingaria-labelandrole="img"to the wrapper spans.♿ Optional accessibility enhancement
{user.emailVerified ? ( - <span title="Email verified"> + <span title="Email verified" role="img" aria-label="Email verified"> <CheckCircle2 className="h-3 w-3 text-green-500 flex-shrink-0" /> </span> ) : ( - <span title="Email not verified"> + <span title="Email not verified" role="img" aria-label="Email not verified"> <AlertCircle className="h-3 w-3 text-amber-500 flex-shrink-0" /> </span> )}services/ChatService.ts (2)
328-335: Remove debug console statements before merging.These
console.logandconsole.warncalls appear to be debug artifacts. The warning on line 334 is particularly noisy since most messages won't have attachments.🧹 Proposed fix to remove debug logs
// Add attachment IDs if present - console.log('[ChatService] streamMessage - attachmentIds:', attachmentIds); if (attachmentIds.length > 0) { - console.log('[ChatService] streamMessage - Adding attachment_ids to FormData:', JSON.stringify(attachmentIds)); formData.append('attachment_ids', JSON.stringify(attachmentIds)); - } else { - console.warn('[ChatService] streamMessage - No attachment IDs to send'); }
1068-1076: Consider using try-finally for cleanup robustness.If an error occurs after
createObjectURLbut before cleanup, the blob URL won't be revoked. A try-finally ensures cleanup runs regardless of exceptions.♻️ Proposed improvement for cleanup
// Create download link const url = window.URL.createObjectURL(response.data); const a = document.createElement('a'); - a.href = url; - a.download = fileName; - document.body.appendChild(a); - a.click(); - window.URL.revokeObjectURL(url); - document.body.removeChild(a); + try { + a.href = url; + a.download = fileName; + document.body.appendChild(a); + a.click(); + } finally { + window.URL.revokeObjectURL(url); + if (a.parentNode) { + document.body.removeChild(a); + } + }app/(main)/newchat/components/step1.tsx (1)
213-231: Side effect in queryFn may cause unexpected behavior on refetches.Dispatching Redux actions inside
queryFnis discouraged because React Query may refetch this query (on window focus, stale time, manual refetch, etc.). If the user manually selects a different repository after the initial load, a background refetch could reset their selection back todefaultRepo.Consider moving this logic to a
useEffectthat watchesUserRepositorysand only runs once on initial data load:♻️ Suggested refactor using useEffect
const { data: UserRepositorys, isLoading: UserRepositorysLoading } = useQuery( { queryKey: ["user-repository"], - queryFn: async () => { - const repos = - await BranchAndRepositoryService.getUserRepositories().then((data) => { - if (defaultRepo && data.length > 0) { - const decodedDefaultRepo = - decodeURIComponent(defaultRepo).toLowerCase(); - const matchingRepo = data.find((repo: RepoIdentifier) => { - const repoIdentifier = getRepoIdentifier(repo); - return ( - repoIdentifier && - repoIdentifier.toLowerCase() === decodedDefaultRepo - ); - }); - dispatch( - setRepoName(matchingRepo ? decodeURIComponent(defaultRepo) : "") - ); - } - return data; - }); - return repos; - }, + queryFn: () => BranchAndRepositoryService.getUserRepositories(), } );Then add a separate effect:
// Set default repo from URL params on initial data load useEffect(() => { if (UserRepositorys && defaultRepo && !repoName) { const decodedDefaultRepo = decodeURIComponent(defaultRepo).toLowerCase(); const matchingRepo = UserRepositorys.find((repo: RepoIdentifier) => { const repoIdentifier = getRepoIdentifier(repo); return repoIdentifier && repoIdentifier.toLowerCase() === decodedDefaultRepo; }); if (matchingRepo) { dispatch(setRepoName(decodeURIComponent(defaultRepo))); } } }, [UserRepositorys, defaultRepo]);lib/utils/emailValidation.ts (1)
8-11: Use Firebase'sgetAdditionalUserInfo()helper instead of type casting.Firebase provides an official
getAdditionalUserInfo(userCredential)function that returns properly typedAdditionalUserInfo | null, eliminating the need for custom type definitions and unsafe casting.♻️ Suggested refactor using Firebase's built-in helper
-// Firebase modular UserCredential type omits additionalUserInfo; it exists at runtime from signInWithPopup -type UserCredentialWithAdditionalInfo = UserCredential & { - additionalUserInfo?: { isNewUser?: boolean } | null; -};Add import:
+import { getAdditionalUserInfo } from 'firebase/auth';Then in
isNewUser(lines 198-202):- const cred = result as UserCredentialWithAdditionalInfo; // Method 1: additionalUserInfo.isNewUser // Note: This only returns true if the current SDK call created the account. // It will be false for accounts created via Admin SDK, console, or backend. - if (cred.additionalUserInfo?.isNewUser === true) { + const additionalInfo = getAdditionalUserInfo(result); + if (additionalInfo?.isNewUser === true) { return true; }app/(auth)/link-github/page.tsx (2)
5-17: Unused imports can be removed.Several imports are not used in this component:
- From
@/public:arrowcon,chat,cloud,cross,logo60,sendBlue,setting- From
lucide-react:LinkRemoving these will reduce bundle size and improve code clarity.
♻️ Proposed cleanup
-import { - arrowcon, - chat, - cloud, - cross, - logo60, - logoWithText, - sendBlue, - setting, -} from "@/public"; +import { logoWithText } from "@/public"; ... -import { Link, LucideCheck, LucideGithub } from "lucide-react"; +import { LucideCheck, LucideGithub } from "lucide-react";
1-22: Import order doesn't follow guidelines.Per coding guidelines, imports should be ordered: React/Next imports first, then components, then utilities. Currently React is at line 21 and Next imports are at lines 18-19.
♻️ Suggested import order
"use client"; import React, { useRef } from "react"; import Image from "next/image"; import { useRouter } from "next/navigation"; import { GithubAuthProvider, linkWithPopup } from "firebase/auth"; import axios from "axios"; import { usePostHog } from "posthog-js/react"; import { LucideCheck, LucideGithub } from "lucide-react"; import { toast } from "sonner"; import { Button } from "@/components/ui/button"; import { auth } from "@/configs/Firebase-config"; import { logoWithText } from "@/public"; import getHeaders from "@/app/utils/headers.util";As per coding guidelines: "Order imports: React/Next imports first, then components, then utilities"
app/(main)/chat/[chatId]/components/MessageComposer.tsx (1)
469-478: Consider specific error type checking instead ofanyThe error handling relies on
error.namechecks with ananytype. Consider using a type guard or checking for axios-specific cancellation to improve type safety.♻️ Suggested improvement
- } catch (error: any) { - if (error.name === 'AbortError' || error.name === 'CanceledError') { + } catch (error: unknown) { + if ( + error instanceof Error && + (error.name === 'AbortError' || error.name === 'CanceledError') + ) { // User cancelled, already handled return; } console.error("Document upload error:", error); toast.error("Upload failed", { description: - error.message || "Failed to upload document. Please try again.", + error instanceof Error + ? error.message + : "Failed to upload document. Please try again.", });app/(main)/chat/[chatId]/components/Thread.tsx (2)
345-346: Consider defining a typed interface for message metadataThe
anycast onmessage.metadatabypasses type checking. Consider defining a proper interface for the custom metadata structure.♻️ Suggested improvement
// At the top of the file or in a shared types file interface MessageMetadata { custom?: { documentAttachments?: DocumentAttachment[]; selectedNodes?: unknown[]; documentIds?: string[]; }; } // In the component const metadataDocumentAttachments = (message.metadata as MessageMetadata | undefined)?.custom?.documentAttachments ?? [];
396-401: Add fallback for unknown attachment typesThe icon display only handles specific
attachment_typevalues. Unknown types will render an empty span. Consider adding a default fallback icon.♻️ Suggested improvement
<span className="text-lg"> {attachment.attachment_type === "pdf" && "📄"} {attachment.attachment_type === "spreadsheet" && "📊"} {attachment.attachment_type === "code" && "💻"} {attachment.attachment_type === "document" && "📝"} + {!["pdf", "spreadsheet", "code", "document"].includes( + attachment.attachment_type + ) && "📎"} </span>
| const handleEnhancePrompt = async () => { | ||
| try { | ||
| setIsEnhancing(true); | ||
| setMessage("Enhancing..."); | ||
| composer.setText("Enhancing..."); | ||
| const enhancedMessage = await ChatService.enhancePrompt( | ||
| conversation_id, | ||
| message | ||
| ); | ||
| setMessage(enhancedMessage); | ||
| composer.setText(enhancedMessage); | ||
| } catch (error) { | ||
| console.error("Error enhancing prompt:", error); | ||
| } finally { | ||
| setIsEnhancing(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Original message is lost if prompt enhancement fails
When handleEnhancePrompt is called, the message is immediately overwritten with "Enhancing...". If the API call fails, the original message is not restored, causing user data loss.
🐛 Proposed fix to preserve original message
const handleEnhancePrompt = async () => {
+ const originalMessage = message;
try {
setIsEnhancing(true);
setMessage("Enhancing...");
composer.setText("Enhancing...");
const enhancedMessage = await ChatService.enhancePrompt(
conversation_id,
- message
+ originalMessage
);
setMessage(enhancedMessage);
composer.setText(enhancedMessage);
} catch (error) {
console.error("Error enhancing prompt:", error);
+ // Restore original message on failure
+ setMessage(originalMessage);
+ composer.setText(originalMessage);
+ toast.error("Failed to enhance prompt");
} finally {
setIsEnhancing(false);
}
};🤖 Prompt for AI Agents
In `@app/`(main)/chat/[chatId]/components/MessageComposer.tsx around lines 576 -
592, handleEnhancePrompt currently overwrites the user's message with
"Enhancing..." before calling ChatService.enhancePrompt, so if the API fails the
original text is lost; fix it by capturing the current message into a local
variable (e.g., originalMessage) before calling setMessage/composer.setText, set
the UI to the enhancing state using that local save, and in the catch block
restore the original text via setMessage(originalMessage) and
composer.setText(originalMessage) (keep setIsEnhancing(false) in finally).
Reference: handleEnhancePrompt, setMessage, composer.setText,
ChatService.enhancePrompt, setIsEnhancing.
| {documentAttachments.map((attachment: any, index: number) => { | ||
| const tokenCount = | ||
| attachment.token_count || attachment.file_metadata?.token_count; | ||
| const metadata = attachment.metadata || attachment.file_metadata; | ||
|
|
||
| return ( | ||
| <div | ||
| key={index} | ||
| className="flex items-center gap-2 text-xs bg-white rounded-lg p-2 border" | ||
| > | ||
| <span className="text-lg"> | ||
| {attachment.attachment_type === "pdf" && "📄"} | ||
| {attachment.attachment_type === "spreadsheet" && "📊"} | ||
| {attachment.attachment_type === "code" && "💻"} | ||
| {attachment.attachment_type === "document" && "📝"} | ||
| </span> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-medium truncate"> | ||
| {attachment.file_name} | ||
| </div> | ||
| {tokenCount && ( | ||
| <div className="text-gray-500"> | ||
| {tokenCount.toLocaleString()} tokens | ||
| </div> | ||
| )} | ||
| {metadata?.page_count && ( | ||
| <div className="text-gray-400 text-xs"> | ||
| {metadata.page_count} pages | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
Use unique id instead of array index for document attachment keys
Using array index as the React key can cause rendering issues if documents are removed or reordered. Document attachments have unique id properties that should be used instead.
🐛 Proposed fix
{documentAttachments.map((attachment: any, index: number) => {
const tokenCount =
attachment.token_count || attachment.file_metadata?.token_count;
const metadata = attachment.metadata || attachment.file_metadata;
return (
<div
- key={index}
+ key={attachment.id || index}
className="flex items-center gap-2 text-xs bg-white rounded-lg p-2 border"
>🤖 Prompt for AI Agents
In `@app/`(main)/chat/[chatId]/components/Thread.tsx around lines 386 - 419, In
the documentAttachments map inside the Thread component (the JSX block mapping
documentAttachments in Thread.tsx), replace the React key currently using the
array index with a stable unique identifier from the attachment object (use
attachment.id or another unique id field) so each mapped <div> uses
key={attachment.id} instead of key={index}; ensure you handle the case where id
might be missing by falling back to a stable unique value if necessary.
| const credential = GithubAuthProvider.credentialFromResult(result); | ||
| const accessToken = | ||
| credential?.accessToken ?? | ||
| (result as any)?._tokenResponse?.oauthAccessToken ?? | ||
| null; | ||
|
|
||
| const providerUsername = (result as any)?._tokenResponse?.screenName; |
There was a problem hiding this comment.
does oauth changes required for documents support?
| <CommandSeparator className="my-1" /> | ||
| </> | ||
| )} | ||
| {process.env.NEXT_PUBLIC_BASE_URL?.includes("localhost") && ( |
There was a problem hiding this comment.
cannot add public depo in localhost?
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.