Feature/gateway unified analysis #8
Conversation
✅ Deploy Preview for crowncode-by-rthur ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a unified analysis gateway endpoint infrastructure using Netlify Functions, refactors the analysis logic for better maintainability, and standardizes preview indicators across the codebase.
Key changes:
- Added a Netlify Function as a mock analysis gateway endpoint that returns preview responses, setting up infrastructure for future real analysis provider integration
- Introduced a new
analyzeSourcefunction inanalysisGateway.tsto handle analysis requests with improved error mapping and support for multiple source types - Standardized preview indicator messages by creating a
previewIndicators()utility function, eliminating code duplication in file and YouTube analysis hooks
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
netlify/functions/analyze.js |
New Netlify Function providing a mock analysis gateway endpoint that returns preview responses |
netlify.toml |
Configuration update specifying the functions directory for Netlify Functions |
platform/hooks/analysisGateway.ts |
New gateway module for handling analysis requests with FormData support and error mapping |
platform/hooks/analysisTypes.ts |
Extended error codes to include backend-related errors (not_configured, unreachable, unexpected_response) |
platform/hooks/analysisUtils.ts |
Added previewIndicators() utility to standardize artificial indicator messages |
platform/hooks/useFileAnalysis.ts |
Updated to use the new previewIndicators() utility instead of hardcoded messages |
platform/hooks/useYouTubeAnalysis.ts |
Updated to use the new previewIndicators() utility instead of hardcoded messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,49 @@ | |||
| // Unified analysis gateway (mock/preview). Real integrations (yt-dlp/model) should be wired to the providers. | |||
| const { promises: fs } = require('fs') | |||
There was a problem hiding this comment.
The imported fs module is never used in this function. Consider removing this unused import to keep the code clean.
| const { promises: fs } = require('fs') |
| artificialIndicators: [ | ||
| 'Preview-only decision based on fingerprint.', | ||
| 'No model inference was available at request time.' | ||
| ] |
There was a problem hiding this comment.
The hardcoded preview indicator messages in the Netlify function are inconsistent with the previewIndicators() utility function used elsewhere in the codebase. The Netlify function uses "Preview-only decision based on fingerprint." while the utility in analysisUtils.ts returns the same message. However, this creates maintenance burden - if the preview indicator messages need to change, they must be updated in two places. Consider importing and using the previewIndicators() utility function, or if that's not possible in this Node.js context, document this duplication and the reason for it.
| errors?: string[] | ||
| } | ||
|
|
||
| const MAX_BYTES = 30 * 1024 * 1024 // 30MB |
There was a problem hiding this comment.
The MAX_BYTES constant is set to 30MB here, but in useFileAnalysis.ts, the MAX_FILE_SIZE_BYTES constant is set to 100MB. This inconsistency means that files between 30MB and 100MB would be rejected by the gateway but accepted by the frontend validation. These limits should be aligned to prevent confusing user experiences where a file passes frontend validation but fails at the backend.
| const MAX_BYTES = 30 * 1024 * 1024 // 30MB | |
| const MAX_BYTES = 100 * 1024 * 1024 // 100MB |
| const response = await fetch(`${apiBaseUrl}/api/analyze`, { | ||
| method: 'POST', | ||
| body: formData | ||
| }) | ||
|
|
||
| if (!response.ok) { | ||
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } |
There was a problem hiding this comment.
The error handling for non-OK HTTP responses is too broad. A non-OK response could be a 400 (bad request), 500 (server error), or network timeout, but all are mapped to 'backend_unreachable'. Consider checking response.status to provide more specific error codes. For example, 4xx errors could map to different error codes than 5xx errors, improving the user experience with more accurate error messages.
| const response = await fetch(`${apiBaseUrl}/api/analyze`, { | |
| method: 'POST', | |
| body: formData | |
| }) | |
| if (!response.ok) { | |
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } | |
| let response: Response | |
| try { | |
| response = await fetch(`${apiBaseUrl}/api/analyze`, { | |
| method: 'POST', | |
| body: formData | |
| }) | |
| } catch { | |
| // Network-level error (timeout, DNS failure, etc.) | |
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } | |
| } | |
| if (!response.ok) { | |
| // Distinguish between client and server errors for better error reporting | |
| if (response.status >= 400 && response.status < 500) { | |
| return { result: null, error: 'backend_unexpected_response' as AnalysisErrorCode } | |
| } | |
| if (response.status >= 500 && response.status < 600) { | |
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } | |
| } | |
| return { result: null, error: 'backend_unexpected_response' as AnalysisErrorCode } |
| const response = await fetch(`${apiBaseUrl}/api/analyze`, { | ||
| method: 'POST', | ||
| body: formData | ||
| }) |
There was a problem hiding this comment.
The fetch call does not handle network errors or exceptions. If the fetch fails due to network issues (e.g., no internet connection, DNS failure), this will throw an unhandled exception. Consider wrapping the fetch in a try-catch block to properly handle these errors and return an appropriate error code like 'backend_unreachable'.
| const response = await fetch(`${apiBaseUrl}/api/analyze`, { | |
| method: 'POST', | |
| body: formData | |
| }) | |
| let response: Response | |
| try { | |
| response = await fetch(`${apiBaseUrl}/api/analyze`, { | |
| method: 'POST', | |
| body: formData | |
| }) | |
| } catch { | |
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } | |
| } |
| return { result: null, error: 'backend_unreachable' as AnalysisErrorCode } | ||
| } | ||
|
|
||
| const data = await response.json() as AnalyzeResponse |
There was a problem hiding this comment.
The JSON parsing on line 50 could throw an exception if the response body is not valid JSON. Consider wrapping this in a try-catch block or handling the case where the response is malformed, returning 'backend_unexpected_response' error in such cases.
| const data = await response.json() as AnalyzeResponse | |
| let data: AnalyzeResponse | |
| try { | |
| data = (await response.json()) as AnalyzeResponse | |
| } catch { | |
| return { result: null, error: 'backend_unexpected_response' as AnalysisErrorCode } | |
| } |
This pull request introduces a new unified analysis gateway endpoint using Netlify Functions and refactors the analysis logic to support this integration. It also improves error handling, standardizes preview indicators, and adds utility functions for analysis. The most important changes are summarized below:
Backend/Infrastructure:
netlify/functions/analyze.js) that acts as a mock analysis gateway, returning a preview response for POST requests. This sets up the structure for integrating real analysis providers in the future.netlify.tomlto specify the new functions directory for Netlify Functions.Analysis Logic & API Integration:
platform/hooks/analysisGateway.tswith theanalyzeSourcefunction to handle analysis requests via the new gateway, supporting both file and URL sources, and improved error mapping for backend responses.AnalysisErrorCodetype inplatform/hooks/analysisTypes.tsto include new backend-related error codes for more robust error handling.Preview/Mock Logic Standardization:
previewIndicatorsutility inplatform/hooks/analysisUtils.tsto standardize artificial indicator messages across preview/mock results. Updated both file and YouTube analysis hooks to use this utility. [1] [2] [3] [4] [5]