-
Notifications
You must be signed in to change notification settings - Fork 2
DGAI: Create embedding api route - ENG-291 #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DGAI: Create embedding api route - ENG-291 #146
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughA new API route handler was implemented to process POST requests for generating text embeddings using OpenAI's API, with input validation, error handling, and CORS support. The OpenAI Node SDK was added as a dependency to the project. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API Route Handler
participant OpenAI API
Client->>API Route Handler: POST /api/embeddings/openai/small (input, model, etc.)
API Route Handler->>API Route Handler: Validate input & env vars
API Route Handler->>OpenAI API: Request embeddings with parameters
OpenAI API-->>API Route Handler: Embedding response
API Route Handler-->>Client: JSON response with embeddings
Client->>API Route Handler: OPTIONS /api/embeddings/openai/small
API Route Handler-->>Client: 204 No Content (CORS preflight)
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/website/app/api/embeddings/openai/small/route.ts (2)
19-38
: Ensure error response uses consistent formatting.The error handling for empty input uses a different structure than the main error handler at the end of the function. Consider standardizing error response formats across the API.
if (!input || (Array.isArray(input) && input.length === 0)) { response = NextResponse.json( - { error: "Input text cannot be empty." }, + { + error: "Input text cannot be empty.", + details: "The input parameter must contain non-empty text." + }, { status: 400 }, ); return cors(req, response) as NextResponse; }
53-64
: Improve error handling with specific error types.The current error handling doesn't differentiate between different types of errors (e.g., API errors, network errors, authentication errors).
try { // ...existing code } catch (error: unknown) { console.error("Error calling OpenAI Embeddings API:", error); + + let status = 500; + let errorMessage = "Unknown error"; + + if (error instanceof Error) { + errorMessage = error.message; + + // Check for specific OpenAI error types + if ('status' in error && typeof error.status === 'number') { + status = error.status; + } + } + response = NextResponse.json( { error: "Failed to generate embeddings.", - details: errorMessage, + details: errorMessage, }, - { status: 500 }, + { status }, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/website/app/api/embeddings/openai/small/route.ts
(1 hunks)package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/website/app/api/embeddings/openai/small/route.ts (2)
apps/website/app/types/llm.ts (1)
RequestBody
(16-20)apps/website/app/utils/llm/cors.ts (1)
cors
(12-39)
🔇 Additional comments (4)
package.json (1)
32-32
:✅ Verification successful
Added OpenAI SDK as dependency.
The OpenAI SDK has been added as a dependency with version constraint
^4.97.0
. This addition supports the new embeddings API route.
🏁 Script executed:
#!/bin/bash # Verify the latest OpenAI SDK version and check for any significant changes since 4.97.0 npm view openai version npm view openai versions --json | jq '.[] | select(startswith("4.") and . > "4.97.0")' | sort -V | tail -5Length of output: 141
Approve OpenAI SDK v4.97.0 Addition
The OpenAI SDK dependency has been added as
"openai": "^4.97.0"
, which is the latest 4.x release. Verified there are no newer 4.x versions available. This change correctly supports the new embeddings API route.apps/website/app/api/embeddings/openai/small/route.ts (3)
9-14
: Type definition looks good.The RequestBody type accurately defines the expected parameters for the OpenAI embeddings API with appropriate optionals.
40-48
: Good conditional handling of dimensions parameter.The code correctly includes the dimensions parameter only for text-embedding-3 models, which aligns with OpenAI's API specifications.
69-71
: Properly handling OPTIONS requests.The route correctly implements an OPTIONS handler for CORS preflight requests.
@coderabbitai review |
✅ Actions performedReview triggered.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/website/app/api/embeddings/openai/small/route.ts (1)
98-99
: 🛠️ Refactor suggestionConsider implementing rate limiting.
The current implementation doesn't include any rate limiting which could lead to excessive costs or API quota exhaustion if the endpoint is called frequently.
🧹 Nitpick comments (2)
apps/website/app/api/embeddings/openai/small/route.ts (2)
26-43
: Double checking of OPTIONS method is unnecessary.The function already handles OPTIONS requests in the OPTIONS export at the bottom of the file, but also checks for it again inside the POST handler which is redundant.
export async function POST(req: NextRequest): Promise<NextResponse> { let response: NextResponse; if (!apiKey) { response = NextResponse.json( { error: "Server configuration error.", details: "Embeddings service is not configured.", }, { status: 500 }, ); return cors(req, response) as NextResponse; } try { - if (req.method === "OPTIONS") { - return cors(req, new NextResponse(null, { status: 204 })) as NextResponse; - } - const body: RequestBody = await req.json();
29-38
: Consider returning a 503 status code for service unavailability.When the API key is missing, a 500 status code is returned, but 503 (Service Unavailable) would be more appropriate for a configuration issue.
if (!apiKey) { response = NextResponse.json( { error: "Server configuration error.", details: "Embeddings service is not configured.", }, - { status: 500 }, + { status: 503 }, ); return cors(req, response) as NextResponse; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/website/app/api/embeddings/openai/small/route.ts
(1 hunks)package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/website/app/api/embeddings/openai/small/route.ts (2)
apps/website/app/types/llm.ts (1)
RequestBody
(16-20)apps/website/app/utils/llm/cors.ts (1)
cors
(12-39)
🔇 Additional comments (6)
apps/website/app/api/embeddings/openai/small/route.ts (6)
17-22
: LGTM! The RequestBody type is well-defined.The type definition clearly specifies the expected request parameters with proper typing for OpenAI embeddings API.
53-59
: Good validation for empty input.The code properly validates that the input is not empty, which is important since the OpenAI API would reject such requests anyway.
61-69
: Proper conditional application of dimensions parameter.The code correctly applies the dimensions parameter only when the model supports it (text-embedding-3 models), which prevents errors with models that don't support this parameter.
71-82
: Well-implemented timeout handling.The implementation of request timeout using Promise.race() is an excellent practice to prevent hanging requests.
85-96
: Good error handling with specific error messages.The error handling captures both specific Error instances and unknown errors, providing appropriate feedback in the response.
101-103
: OPTIONS method implementation correctly handles CORS preflight requests.Using the cors utility ensures consistent CORS handling across the application.
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/website/app/api/embeddings/openai/small/route.ts (1)
24-97
: 🛠️ Refactor suggestionConsider implementing rate limiting for the API endpoint.
The current implementation doesn't include any rate limiting which could lead to excessive costs or API quota exhaustion if the endpoint is called frequently.
Consider adding a rate limiting middleware to protect this endpoint. You could use a package like
@vercel/rate-limit
or implement a simple in-memory rate limiter.import { RateLimit } from '@vercel/rate-limit'; // Create a rate limiter that allows 10 requests per minute const limiter = RateLimit({ uniqueTokenPerInterval: 500, // Max number of unique tokens per interval interval: 60 * 1000, // 1 minute in milliseconds limit: 10, // Max requests per interval }); export async function POST(req: NextRequest): Promise<NextResponse> { try { // Apply rate limiting const ratelimitResult = await limiter.check( // Use an identifier like IP or session token req.ip ?? '127.0.0.1', 10, // Number of requests 'embeddings_api' // Optional prefix for the rate limit key ); if (!ratelimitResult.success) { return cors( req, NextResponse.json( { error: 'Too many requests', details: 'Rate limit exceeded' }, { status: 429, headers: { 'Retry-After': ratelimitResult.retryAfter.toString() } } ) ) as NextResponse; } // Rest of your code... } catch (error) { // ... } }
🧹 Nitpick comments (3)
apps/website/app/api/embeddings/openai/small/route.ts (3)
38-41
: Remove redundant OPTIONS handling in POST method.The OPTIONS request handling is redundant here since you've already implemented a separate OPTIONS handler function at lines 99-101.
try { - if (req.method === "OPTIONS") { - return cors(req, new NextResponse(null, { status: 204 })) as NextResponse; - } const body: RequestBody = await req.json();
51-57
: Consider adding input size validation.While you validate that the input is not empty, there's no validation for input size, which could lead to excessive token usage and costs.
if (!input || (Array.isArray(input) && input.length === 0)) { response = NextResponse.json( { error: "Input text cannot be empty." }, { status: 400 }, ); return cors(req, response) as NextResponse; } + + // Check if input size exceeds limits + const inputSize = Array.isArray(input) + ? input.reduce((total, text) => total + text.length, 0) + : input.length; + + const MAX_INPUT_SIZE = 100000; // Define appropriate limit + if (inputSize > MAX_INPUT_SIZE) { + response = NextResponse.json( + { error: "Input text exceeds maximum allowed size." }, + { status: 400 }, + ); + return cors(req, response) as NextResponse; + }
24-97
: Consider implementing caching for repeated requests.For efficiency and cost reduction, consider caching embeddings results for repeated inputs, as embedding the same text multiple times produces identical results.
You could implement a simple in-memory cache or use a more robust solution like Redis:
// Simple in-memory cache example const cache = new Map<string, any>(); export async function POST(req: NextRequest): Promise<NextResponse> { // ... existing code ... try { // ... parsing body ... // Generate a cache key based on request parameters const cacheKey = JSON.stringify({ input, model, dimensions, encoding_format }); // Check cache before making API call if (cache.has(cacheKey)) { response = NextResponse.json(cache.get(cacheKey), { status: 200 }); return cors(req, response) as NextResponse; } // ... existing API call code ... // Cache the response cache.set(cacheKey, openAIResponse); response = NextResponse.json(openAIResponse, { status: 200 }); } catch (error) { // ... existing error handling ... } return cors(req, response) as NextResponse; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/website/app/api/embeddings/openai/small/route.ts
(1 hunks)apps/website/package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/website/app/api/embeddings/openai/small/route.ts (2)
apps/website/app/types/llm.ts (1)
RequestBody
(16-20)apps/website/app/utils/llm/cors.ts (1)
cors
(12-39)
🔇 Additional comments (10)
apps/website/package.json (1)
18-18
: OpenAI SDK dependency added correctly.The OpenAI Node.js SDK has been added with version
^4.97.0
, which is required for integrating with OpenAI's embeddings API.apps/website/app/api/embeddings/openai/small/route.ts (9)
5-13
: Good implementation of API key validation.The code properly checks for the existence of the OpenAI API key and only initializes the client if it's available. This implementation addresses the previous review comment about API key validation.
15-20
: Type definition is clear and comprehensive.The RequestBody type definition correctly specifies all required parameters for the OpenAI embeddings API with appropriate types.
22-22
: Timeout configuration is appropriate.Setting a 30-second timeout for OpenAI API requests prevents hanging requests, addressing the previous review comment.
27-36
: Error handling for missing API key is well-implemented.The API properly returns a clear error message when the API key is not configured, preventing confusing errors from reaching the client.
51-57
: Input validation is properly implemented.The code correctly validates that the input is not empty before proceeding, which prevents unnecessary API calls and provides a clear error message.
59-67
: Proper handling of optional dimensions parameter.The code correctly applies the dimensions parameter only when the model supports it (text-embedding-3 models), which aligns with OpenAI's API requirements.
69-80
: Timeout implementation is well done.The Promise.race approach for timeout handling is an excellent pattern, addressing the previous review comment about request timeout configuration.
83-94
: Error handling is comprehensive.The code properly catches and logs errors, extracts the error message when available, and returns a structured error response with appropriate status code.
99-101
: OPTIONS handler is properly implemented.The OPTIONS handler correctly implements the CORS preflight response, which is essential for cross-origin requests.
@coderabbitai full review |
✅ Actions performedFull review triggered. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/website/app/api/embeddings/openai/small/route.ts (2)
1-87
: Consider implementing rate limiting for the API endpoint.The current implementation doesn't include any rate limiting which could lead to excessive costs or API quota exhaustion if the endpoint is called frequently.
Consider adding a rate limiting middleware to protect this endpoint. You could use a package like
@vercel/rate-limit
or implement a simple in-memory rate limiter.import { RateLimit } from '@vercel/rate-limit'; // Create a rate limiter that allows 10 requests per minute const limiter = RateLimit({ uniqueTokenPerInterval: 500, // Max number of unique tokens per interval interval: 60 * 1000, // 1 minute in milliseconds limit: 10, // Max requests per interval }); export async function POST(req: NextRequest): Promise<NextResponse> { try { // Apply rate limiting const ratelimitResult = await limiter.check( // Use an identifier like IP or session token req.ip ?? '127.0.0.1', 10, // Number of requests 'embeddings_api' // Optional prefix for the rate limit key ); if (!ratelimitResult.success) { return cors( req, NextResponse.json( { error: 'Too many requests', details: 'Rate limit exceeded' }, { status: 429, headers: { 'Retry-After': ratelimitResult.retryAfter.toString() } } ) ) as NextResponse; } // Rest of your code... } catch (error) { // ... } }
47-52
: Validate the dimensions parameter against model-specific bounds.When using a fixed dimensions value, it's good to ensure it's within the supported range for the model.
According to OpenAI's documentation:
- text-embedding-3-small supports 512 – 1536 dimensions
- text-embedding-3-large supports 256 – 3072 dimensions
If you make the dimensions configurable in the future, please add validation:
+ // If dimensions becomes configurable: + if (dimensions != null) { + // Determine supported bounds based on the model + const isSmall = model === "text-embedding-3-small"; + const minDim = isSmall ? 512 : 256; + const maxDim = isSmall ? 1536 : 3072; + + if (dimensions < minDim || dimensions > maxDim) { + return NextResponse.json( + { + error: "Invalid dimensions parameter.", + details: `Dimensions must be between ${minDim} and ${maxDim} for model ${model}.` + }, + { status: 400 } + ); + } + }What are the valid dimensions ranges for OpenAI's text-embedding-3-small model?
🧹 Nitpick comments (2)
apps/website/app/api/embeddings/openai/small/route.ts (2)
47-52
: Consider making the model configurable.While hardcoding "text-embedding-3-small" works, consider allowing the client to specify the model as an optional parameter with this as the default.
const body: RequestBody = await req.json(); -const { input } = body; +const { input, model = "text-embedding-3-small" } = body as RequestBody & { model?: string }; if (!input || (Array.isArray(input) && input.length === 0)) { response = NextResponse.json( { error: "Input text cannot be empty." }, { status: 400 }, ); return cors(req, response) as NextResponse; } +// Validate model name +const validModels = ["text-embedding-3-small", "text-embedding-3-large"]; +if (!validModels.includes(model)) { + response = NextResponse.json( + { + error: "Invalid model parameter.", + details: `Model must be one of: ${validModels.join(", ")}` + }, + { status: 400 }, + ); + return cors(req, response) as NextResponse; +} const options: OpenAI.EmbeddingCreateParams = { - model: "text-embedding-3-small", + model, input: input, encoding_format: "float", dimensions: 1536, };
1-13
: Consider extracting OpenAI client initialization to a shared utility.If you plan to add more OpenAI-related API routes in the future, consider extracting the API key check and client initialization logic to a shared utility file.
// utils/llm/openai.ts import OpenAI from "openai"; export function initializeOpenAIClient() { const apiKey = process.env.OPENAI_API_KEY; if (!apiKey) { console.error( "Missing OPENAI_API_KEY environment variable. OpenAI services will not function.", ); return { client: null, apiKey: null }; } return { client: new OpenAI({ apiKey }), apiKey }; }Then in your route files:
import { initializeOpenAIClient } from "~/utils/llm/openai"; const { client: openai, apiKey } = initializeOpenAIClient();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/website/app/api/embeddings/openai/small/route.ts
(1 hunks)apps/website/package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/website/app/api/embeddings/openai/small/route.ts (2)
apps/website/app/types/llm.ts (1)
RequestBody
(16-20)apps/website/app/utils/llm/cors.ts (1)
cors
(12-39)
🔇 Additional comments (8)
apps/website/package.json (1)
18-18
: OpenAI dependency added correctly.The addition of the OpenAI SDK dependency at version ^4.97.0 aligns perfectly with the new embeddings API route implementation.
apps/website/app/api/embeddings/openai/small/route.ts (7)
5-13
: API key initialization is handled properly.Good implementation of OpenAI client initialization with proper error handling:
- The code checks for the API key's presence
- Logs a clear error message if missing
- Only initializes the client when the API key is available
This addresses previous feedback about API key validation.
15-17
: Type definition is appropriate.The
RequestBody
type correctly defines the expected input format, supporting both single strings and string arrays as required by OpenAI's embeddings API.
19-19
: Good practice: Timeout configuration.Setting a 30-second timeout constant is a good practice to prevent hanging requests.
21-45
: Input validation looks good.The implementation properly:
- Checks for API key availability first
- Returns appropriate error responses with status codes
- Validates that input is not empty
- Applies CORS headers to all responses
These are all good practices for a robust API endpoint.
54-65
: Timeout implementation is well done.The implementation of the timeout mechanism using Promise.race is a good approach to prevent hanging requests. This addresses previous feedback about request timeout configuration.
68-79
: Error handling is comprehensive.Good error handling with:
- Error logging
- Type checking with instanceof
- Appropriate error response with details
- Consistent status code (500)
84-86
: CORS preflight handling is correctly implemented.The OPTIONS handler correctly implements CORS preflight response with a 204 status code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
Just one non-blocking comment.
Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Summary by CodeRabbit