-
Notifications
You must be signed in to change notification settings - Fork 4
add search route, truncated. #182
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
Conversation
|
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 📝 WalkthroughWalkthroughThis change refactors and generalizes the API for text embeddings by removing the provider-specific OpenAI embeddings route and introducing a unified embeddings API supporting multiple providers. It adds new type definitions, utility functions for handling embeddings, and enhances Supabase search functionality using embeddings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as /api/embeddings
participant EmbeddingUtil as genericEmbedding
participant Provider as OpenAI API
Client->>API: POST /api/embeddings (input, settings, provider)
API->>EmbeddingUtil: genericEmbedding(input, settings, provider)
EmbeddingUtil->>Provider: Request embeddings (if provider is openai)
Provider-->>EmbeddingUtil: Embedding response
EmbeddingUtil-->>API: Embedding vectors
API-->>Client: Embedding vectors (JSON)
sequenceDiagram
participant Client
participant API as /api/supabase/rpc/search-content
participant EmbeddingUtil as genericEmbedding
participant Supabase
Client->>API: POST /api/supabase/rpc/search-content (query, platform_ids)
API->>EmbeddingUtil: Generate embedding for query
EmbeddingUtil-->>API: Embedding vector
API->>Supabase: RPC match_embeddings_for_subset_nodes(embedding, platform_ids)
Supabase-->>API: Matching content results
API-->>Client: Search results (JSON)
Possibly related PRs
Suggested reviewers
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 (
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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: 8
🧹 Nitpick comments (4)
apps/website/app/api/embeddings/route.ts (1)
37-37: Update the error log message to reflect the generic nature of this API.The error message still references "OpenAI Embeddings API" but this is now a generic embeddings API that supports multiple providers.
- console.error("Error calling OpenAI Embeddings API:", error); + console.error("Error calling Embeddings API:", error);apps/website/app/utils/llm/embeddings.ts (1)
46-46: Use strict equality comparison.Use
===instead of==for type-safe comparison.- if (provider == "openai") { + if (provider === "openai") {apps/website/app/api/supabase/rpc/search-content/route.ts (1)
61-61: Correct the logging path to match actual route.The log message shows
/api/supabase/rpc/searchbut the actual file path suggests the route is/api/supabase/rpc/search-content.- console.log("[API Route] POST /api/supabase/rpc/search: Request received"); + console.log("[API Route] POST /api/supabase/rpc/search-content: Request received");apps/website/app/utils/supabase/dbUtils.ts (1)
35-35: Consider making the provider parameter required.Defaulting the provider to "openai" when not specified could mask configuration errors. Consider making the provider parameter required or at least logging when the default is used.
- const embeddingName = - KNOWN_EMBEDDINGS[`${provider || "openai"}-${model}-${dimensions}`]; + const actualProvider = provider || "openai"; + if (!provider) { + console.warn(`No provider specified for model ${model}, defaulting to '${actualProvider}'`); + } + const embeddingName = KNOWN_EMBEDDINGS[`${actualProvider}-${model}-${dimensions}`];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/website/app/api/embeddings/openai/small/route.ts(0 hunks)apps/website/app/api/embeddings/route.ts(1 hunks)apps/website/app/api/supabase/rpc/search-content/route.ts(1 hunks)apps/website/app/types/llm.ts(2 hunks)apps/website/app/utils/llm/embeddings.ts(1 hunks)apps/website/app/utils/llm/providers.ts(2 hunks)apps/website/app/utils/supabase/dbUtils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/website/app/api/embeddings/openai/small/route.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/website/app/utils/llm/providers.ts (1)
apps/website/app/types/llm.ts (2)
Provider(1-1)LLMProviderConfig(32-39)
apps/website/app/api/embeddings/route.ts (3)
apps/website/app/types/llm.ts (2)
EmbeddingSettings(24-27)Provider(1-1)apps/website/app/utils/llm/cors.ts (1)
cors(12-39)apps/website/app/utils/llm/embeddings.ts (1)
genericEmbedding(41-59)
apps/website/app/utils/supabase/dbUtils.ts (1)
packages/database/types.gen.ts (1)
Database(9-645)
🔇 Additional comments (12)
apps/website/app/utils/llm/providers.ts (2)
1-1: LGTM!Good addition of the
Providertype to the imports to support the new configuration mapping.
62-66: LGTM!Excellent addition of the centralized provider configuration mapping. This provides type-safe access to provider configurations and supports the new generalized embedding architecture well.
apps/website/app/types/llm.ts (2)
1-1: LGTM!Well-defined
Providertype that clearly represents the supported LLM providers.
24-27: LGTM!Clean and focused
EmbeddingSettingstype. The optionaldimensionsfield is appropriate since not all embedding models support custom dimensions.apps/website/app/api/embeddings/route.ts (4)
6-10: LGTM!Well-defined request body type that clearly specifies the expected parameters with appropriate optional provider defaulting to OpenAI.
19-25: LGTM!Excellent input validation that handles both string and array inputs appropriately.
27-35: LGTM!Good error handling for when embedding generation fails, with appropriate HTTP status codes.
56-58: LGTM!Proper OPTIONS handler for CORS preflight requests.
apps/website/app/utils/llm/embeddings.ts (2)
27-38: LGTM!Excellent timeout handling with
Promise.raceand proper return type handling for both string and array inputs.
49-57: LGTM!Excellent commentary on the implementation tradeoffs between using provider-specific libraries vs direct API calls. This provides valuable context for future development decisions.
apps/website/app/api/supabase/rpc/search-content/route.ts (1)
120-123: LGTM on OPTIONS handler.The OPTIONS handler correctly implements CORS preflight handling with appropriate status code and CORS middleware.
apps/website/app/utils/supabase/dbUtils.ts (1)
8-11: Well-structured type definition.The
EmbeddingTableDatatype provides good structure for embedding table metadata with appropriate constraints using database table keys.
05015a7 to
14ada91
Compare
|
Closed in favor of #185 |
Summary by CodeRabbit
New Features
Refactor
Chores