DGAI: Create API routes for inserting data to various Supabase tables needed to creating an embedding data - ENG-294 #151
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 📝 WalkthroughWalkthroughThis update introduces a comprehensive set of new API routes and utility modules for handling insertion and retrieval of various entities in a Supabase-backed application, including content, embeddings, agents, accounts, documents, discourse platforms and spaces, and persons. The changes provide robust input validation, error handling, batch processing, and standardized API responses, along with enhancements to dependency management and Supabase SSR client setup. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant SupabaseDB
participant Utils
Client->>API_Route: POST /api/supabase/insert/{entity}
API_Route->>Utils: Validate input (Zod)
alt Validation fails
API_Route-->>Client: 400 Bad Request (validation errors)
else Validation succeeds
API_Route->>SupabaseDB: getOrCreateEntity / processAndInsertBatch
SupabaseDB-->>API_Route: Entity data or error
API_Route->>Utils: Format API response
API_Route-->>Client: JSON response (data, error, status)
end
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 (
|
sid597
changed the title
|
TODO: update the required insert routes |
sid597
changed the base branch from
discourse-node-suggestion-component
to
dg-suggestion-component
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
apps/website/package.json (1)
16-29: 🛠️ Refactor suggestionConsider pinning critical infra-dependencies to exact versions.
@supabase/ssrandzodare both infrastructure libraries that can introduce breaking changes even in minor bumps. Using the caret (^) range means everynpm installmay pick up a newer minor/patch version, potentially breaking production builds unexpectedly.- "@supabase/ssr": "^0.6.1", - "zod": "^3.24.4" + "@supabase/ssr": "0.6.1", + "zod": "3.24.4"If you do keep the caret, make sure CI locks the versions via a lock-file diff check or Renovate bot.
🧹 Nitpick comments (15)
apps/website/app/utils/supabase/server.ts (1)
11-40: Minor optimisation: re-use Supabase client across awaits.Every
await supabasePromisedownstream will resolve the same client, but creating the client itself is cheap only once. Suggest explicitly returning the client (not the Promise) and passing it around, simplifying call-sites:-export async function createClient() { ... } // returns Promise<Client> +export function createClient() { ... } // returns ClientYou would then drop the
awaitinside helpers and make type-signatures cleaner.apps/website/app/api/supabase/insert/person/route.ts (3)
97-116: Constraint-name string matching is brittle.Relying on substring checks like
"Account_platform_id_fkey"ties you to specific PG constraint names. If the DB is migrated or renamed, this mapping breaks.Consider inspecting
insertError.column(Supabase v2 exposes it) orinsertError.hint, or map errorcode === '23503'plus examininginsertError.messagewith regex patterns.At minimum, encapsulate the mapping in a small util so future routes share a single source of truth.
148-154: Enumerateperson_typeto avoid free-form data drift.
person_typeis free-text; over time different callers may send “Person”, “person”, “PERSON”, etc.
Define an enum in Zod:const PersonTypeEnum = z.enum(["Person", "Author", "Editor"]); ... person_type: PersonTypeEnum.default("Person"),This keeps the column clean and enables stricter analytics later.
187-193: Status-code derivation can be simplified.
overallStatusis already computed; passing it tocreateApiResponsethen forcescreateApiResponseto massage it again (created ? 201 ...). Since you control both, consider:- return createApiResponse(request, { - data: responsePayload, - status: overallStatus, - }); + return createApiResponse(request, { + data: responsePayload, + status: overallStatus, + created: overallStatus === 201, + });avoiding hidden logic in the util and making intent explicit.
apps/website/app/api/supabase/insert/agents/route.ts (1)
41-71: Good error handling, but there's an opportunity to improve the consistency of error message formatting.The error handling logic is robust, but I noticed a slight inconsistency in the format of error messages compared to other routes. For example, in this file you use:
"${e.path.join(".")} - ${e.message}"while in some other files like discourse-platform and discourse-space routes you use:
"${e.path.join(".")}: ${e.message}"Consider standardizing the error message format across all routes for better consistency:
- .map((e) => `${e.path.join(".")} - ${e.message}`) + .map((e) => `${e.path.join(".")}: ${e.message}`)apps/website/app/api/supabase/insert/discourse-platform/route.ts (1)
53-60: Redundant null check after definite assignment.The check for
!platformName || !platformUrlis redundant given the current logic flow, as these variables will always have values or the function would have already returned. While harmless, it adds unnecessary complexity.- if (!platformName || !platformUrl) { - return { - error: "Platform name or URL could not be derived even from a valid URL.", - entity: null, - created: false, - status: 400, - }; - }This check can be removed as it's unreachable with the current implementation. If you're keeping it as a safeguard for future extensions, consider adding a comment explaining its purpose.
apps/website/app/api/supabase/insert/account/route.ts (1)
37-73: Excellent handling of foreign key constraint violations.The function provides specific error messages for different foreign key constraint violations, which is very helpful for API consumers. This detailed error handling improves the developer experience.
However, there's an opportunity to make the error detection more robust:
Consider using a more reliable method to detect the specific constraint violation:
if ( result.error && result.details && result.status === 400 && result.details.includes("violates foreign key constraint") ) { - if (result.details.includes("Account_person_id_fkey")) { + if (result.details.toLowerCase().includes("account_person_id_fkey")) { return { ...result, error: `Invalid person_id: No Person record found for ID ${person_id}.`, }; - } else if (result.details.includes("Account_platform_id_fkey")) { + } else if (result.details.toLowerCase().includes("account_platform_id_fkey")) { return { ...result, error: `Invalid platform_id: No DiscoursePlatform record found for ID ${platform_id}.`, }; } }This change ensures that the constraint detection works regardless of case differences in error messages, which could vary between database versions or environments.
apps/website/app/api/supabase/insert/discourse-space/route.ts (1)
36-80: Good URL normalization but potential redundancy in function naming.The URL normalization by removing trailing slashes is a good practice. However, the function name
processAndGetOrCreateDiscourseSpaceimplies additional processing beyond whatgetOrCreateEntitydoes, but the only processing is URL normalization and name trimming.Consider simplifying the function name to better reflect its purpose:
-const processAndGetOrCreateDiscourseSpace = async ( +const getOrCreateDiscourseSpace = async (Also, the
trimmedNamevariable is redundant sincenameis already trimmed by Zod:const normalizedUrl = url.replace(/\/$/, ""); - const trimmedName = name;And update the reference in the object:
{ - name: trimmedName, + name, url: normalizedUrl, discourse_platform_id: discourse_platform_id, },apps/website/app/api/supabase/insert/content/route.ts (1)
88-88: Avoid usinganytype.The use of
as anytype assertion for metadata bypasses TypeScript's type checking system. This could lead to potential runtime issues if the data doesn't match the expected structure.- metadata: processedMetadata as any, + metadata: processedMetadata as string | null,apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/batch/route.ts (1)
65-66: Consider extracting table name to a shared constants file.The embedding table name is quite specific and might be referenced in multiple files. Consider moving this to a shared constants file to maintain consistency across the codebase.
apps/website/app/api/supabase/insert/document/route.ts (2)
74-74: Avoid usinganytype.Similar to the content route, using
as anyfor metadata bypasses TypeScript's type checking.- metadata: processedMetadata as any, + metadata: processedMetadata as string | null,
97-102: Error message inconsistency.The error message refers to "Space" but the error check is looking for "space_id_fkey". This might indicate an inconsistency in table naming in the error messages compared to actual database constraints.
- error: `Invalid space_id: No Space record found for ID ${space_id}.`, + error: `Invalid space_id: No DiscourseSpace record found for ID ${space_id}.`,apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/routes.ts (1)
90-98:supabasePromisevariable name is now misleadingAfter removing the unnecessary
await, rename for clarity:- const supabasePromise = createClient(); + const supabase = createClient();Minor, but prevents future confusion.
apps/website/app/utils/supabase/apiUtils.ts (1)
35-45: Avoid masking “no data / no error” cases with a 500The catch-all 500 path is useful but fires on legitimate 204/202 style responses.
Consider allowing callers to explicitly request an empty success by passingstatusin the 2xx range anddata=== null.You could expose
allowEmptySuccess?: booleanin the payload to make intent explicit.apps/website/app/utils/supabase/dbUtils.ts (1)
219-233: Large batch inserts may return huge payloads – considerreturning: "minimal"
.insert().select(selectQuery)returns every column for every row, which for embedding vectors yields megabytes of JSON.
If the caller does not need the vectors back, switch to:const { error } = await supabase .from(tableName) .insert(processedForDb as any, { returning: "minimal" });You’ll cut response size and bandwidth substantially.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
apps/roam/src/components/DiscourseContextOverlay.tsx(2 hunks)apps/website/app/api/supabase/insert/account/route.ts(1 hunks)apps/website/app/api/supabase/insert/agents/route.ts(1 hunks)apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/batch/route.ts(1 hunks)apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/routes.ts(1 hunks)apps/website/app/api/supabase/insert/content/batch/route.ts(1 hunks)apps/website/app/api/supabase/insert/content/route.ts(1 hunks)apps/website/app/api/supabase/insert/discourse-platform/route.ts(1 hunks)apps/website/app/api/supabase/insert/discourse-space/route.ts(1 hunks)apps/website/app/api/supabase/insert/document/route.ts(1 hunks)apps/website/app/api/supabase/insert/person/route.ts(1 hunks)apps/website/app/utils/supabase/apiUtils.ts(1 hunks)apps/website/app/utils/supabase/dbUtils.ts(1 hunks)apps/website/app/utils/supabase/server.ts(1 hunks)apps/website/package.json(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/website/app/api/supabase/insert/person/route.ts (3)
apps/website/app/utils/supabase/server.ts (1)
createClient(4-43)apps/website/app/utils/supabase/dbUtils.ts (2)
GetOrCreateEntityResult(3-9)getOrCreateEntity(23-153)apps/website/app/utils/supabase/apiUtils.ts (3)
createApiResponse(13-47)handleRouteError(53-76)defaultOptionsHandler(81-86)
apps/website/app/utils/supabase/apiUtils.ts (1)
apps/website/app/utils/llm/cors.ts (1)
cors(12-39)
🔇 Additional comments (20)
apps/roam/src/components/DiscourseContextOverlay.tsx (1)
422-423: UI copy change looks good, but check i18n / tests.Renaming the heading to “Ranked by HyDE” clarifies the algorithm.
Just ensure any snapshot / visual tests and i18n keys (if used) are updated accordingly.apps/website/app/api/supabase/insert/agents/route.ts (4)
1-12: Well-structured imports that follow a logical grouping pattern.The imports are organized by source - first the Supabase client, then Next.js essentials, Zod for validation, and finally the custom utilities. This logical separation improves readability.
14-16: Appropriate schema validation for agent type.The Zod schema correctly validates that the agent type is a non-empty string after trimming. This helps prevent invalid data from being stored in the database.
23-39: Well-implemented helper function with proper type safety.The function correctly handles the Supabase client promise, properly trims the input, and uses the generic utility for creating or getting an entity. The return type is appropriately specified as
GetOrCreateEntityResult<AgentRecord>.
76-76: Good CORS support via OPTIONS handler.Using the shared
defaultOptionsHandlerfor CORS preflight requests is a good practice for maintainability and consistency.apps/website/app/api/supabase/insert/discourse-platform/route.ts (3)
14-18: Comprehensive type definition.The type definition for
DiscoursePlatformRecordcovers all necessary fields with proper types, ensuring type safety throughout the application.
20-29: Thorough input validation with helpful error messages.The schema provides clear error messages for each validation case, which is excellent for debugging and API usability. The URL validation ensures that invalid URLs are rejected early in the process.
73-113: POST handler with robust validation and error handling.The POST handler follows a good pattern of validation, processing, and error handling. It correctly uses the schema to validate input, processes the data with the helper function, and returns appropriate responses.
apps/website/app/api/supabase/insert/account/route.ts (2)
14-25: Well-structured validation schema with sensible defaults.The schema correctly enforces positive integers for IDs and provides meaningful error messages. The optional boolean fields with defaults are a good choice for simplifying client usage.
75-111: Well-implemented POST handler with proper error handling.The handler follows the established pattern of validation, processing, and error handling. It correctly uses the schema to validate input and returns appropriate responses.
apps/website/app/api/supabase/insert/discourse-space/route.ts (3)
14-25: Complete validation schema with proper constraints.The schema correctly validates all required fields with appropriate constraints and helpful error messages.
60-77: Robust foreign key error handling with case insensitivity.The error detection for foreign key violations correctly uses case-insensitive comparison, which is more robust than case-sensitive checks. This is a good practice that should be applied consistently across all routes.
82-118: Well-implemented POST handler following established patterns.The handler consistently follows the pattern used across other routes, with proper validation, error handling, and response formatting.
apps/website/app/api/supabase/insert/content/route.ts (3)
95-105: Good implementation of match criteria.The code correctly checks for both the existence of source_local_id and space_id before setting up match criteria. This ensures that the content entry will only be matched with existing records when appropriate identifiers are provided.
116-161: Well-handled foreign key constraints.The detailed error handling for foreign key constraint violations provides clear, user-friendly error messages that specify exactly which reference is invalid (space, author, document, or parent content). This makes debugging and client error handling much more straightforward.
173-182: Good validation error formatting.The code effectively collects and formats all validation errors from Zod, making them readable and actionable for API consumers.
apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/batch/route.ts (2)
18-35: Well-implemented vector field validation.The union type with refinement for the vector field properly handles both array input and JSON string input, with appropriate validation to ensure the data is an array of numbers in either case.
136-138: Good handling of partial batch errors.The response properly includes partial errors in the metadata when some items in the batch fail, allowing clients to identify which specific items had issues without failing the entire batch.
apps/website/app/api/supabase/insert/content/batch/route.ts (1)
55-79: Well-structured validation and processing logic.The
validateAndProcessContentItemfunction handles different metadata formats correctly and properly sets defaults for optional fields, ensuring consistent data structure before insertion.apps/website/app/api/supabase/insert/document/route.ts (1)
82-89: Query always creates new records instead of updating existing ones.The match criteria
{ id: -1 }will never match an existing record since no document will have ID -1. This means the function will always insert new documents rather than updating existing ones.If this is intentional, that's fine, but if you want to update records based on unique identifiers (like with content where you match on space_id and source_local_id), you should modify the match criteria.
Is this intentional behavior for documents? If not, consider implementing a similar matching approach as used in the content route.
| await createBlock({ | ||
| parentUid: blockUid, | ||
| node: { text: `[[${node.text}]]` }, | ||
| node: { text: `[[${nodeText}]]` }, | ||
| }); | ||
| setHydeFilteredNodes(hydeFilteredNodes.filter((n) => n.uid !== node.uid)); | ||
| }; |
There was a problem hiding this comment.
nodeText is undefined – compilation will fail.
handleCreateBlock references nodeText but the available variable is node.text. This results in a Cannot find name 'nodeText' TypeScript error and a runtime failure in JS builds.
- node: { text: `[[${nodeText}]]` },
+ node: { text: `[[${node.text}]]` },📝 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.
| await createBlock({ | |
| parentUid: blockUid, | |
| node: { text: `[[${node.text}]]` }, | |
| node: { text: `[[${nodeText}]]` }, | |
| }); | |
| setHydeFilteredNodes(hydeFilteredNodes.filter((n) => n.uid !== node.uid)); | |
| }; | |
| await createBlock({ | |
| parentUid: blockUid, | |
| node: { text: `[[${node.text}]]` }, | |
| }); |
🤖 Prompt for AI Agents
In apps/roam/src/components/DiscourseContextOverlay.tsx around lines 324 to 329,
the variable `nodeText` is used but not defined, causing a TypeScript error.
Replace `nodeText` with the correct variable `node.text` in the `createBlock`
call to fix the undefined reference and prevent compilation failure.
| export async function createClient() { | ||
| const cookieStore = await cookies(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
await cookies() is unnecessary and breaks type-checking.
next/headers.cookies() returns a synchronous RequestCookies object, not a Promise. Awaiting it triggers a TypeScript error (“await on a non-Promise”). Remove the await; keep the function async if you still want to return a Promise for parity with callers.
-export async function createClient() {
- const cookieStore = await cookies();
+export async function createClient() {
+ const cookieStore = cookies();📝 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 async function createClient() { | |
| const cookieStore = await cookies(); | |
| export async function createClient() { | |
| const cookieStore = cookies(); |
🤖 Prompt for AI Agents
In apps/website/app/utils/supabase/server.ts around lines 4 to 6, remove the
unnecessary await before the cookies() call because next/headers.cookies()
returns a synchronous object, not a Promise. Keep the function async if needed
for returning a Promise, but change "const cookieStore = await cookies();" to
"const cookieStore = cookies();" to fix the TypeScript error.
| const PersonDataInputSchema = z.object({ | ||
| name: z.string().trim().min(1, { message: "Name cannot be empty." }), | ||
| email: z.string().trim().email({ message: "Invalid email format." }), | ||
| orcid: z.string().nullable().optional(), | ||
| person_type: z.string().optional().default("Person"), | ||
| account_platform_id: z | ||
| .number() | ||
| .int() | ||
| .positive({ message: "account_platform_id must be a positive integer." }), | ||
| account_active: z.boolean().optional().default(true), | ||
| account_write_permission: z.boolean().optional(), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Default for account_write_permission is implicit undefined.
Currently, omitting account_write_permission leads to undefined, which your helper silently converts to true. This implicit coupling is not obvious to API consumers.
Either expose the default here:
- account_write_permission: z.boolean().optional(),
+ account_write_permission: z.boolean().optional().default(true),or remove the silent defaulting inside getOrCreateAccountInternal.
📝 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 PersonDataInputSchema = z.object({ | |
| name: z.string().trim().min(1, { message: "Name cannot be empty." }), | |
| email: z.string().trim().email({ message: "Invalid email format." }), | |
| orcid: z.string().nullable().optional(), | |
| person_type: z.string().optional().default("Person"), | |
| account_platform_id: z | |
| .number() | |
| .int() | |
| .positive({ message: "account_platform_id must be a positive integer." }), | |
| account_active: z.boolean().optional().default(true), | |
| account_write_permission: z.boolean().optional(), | |
| }); | |
| const PersonDataInputSchema = z.object({ | |
| name: z.string().trim().min(1, { message: "Name cannot be empty." }), | |
| email: z.string().trim().email({ message: "Invalid email format." }), | |
| orcid: z.string().nullable().optional(), | |
| person_type: z.string().optional().default("Person"), | |
| account_platform_id: z | |
| .number() | |
| .int() | |
| .positive({ message: "account_platform_id must be a positive integer." }), | |
| account_active: z.boolean().optional().default(true), | |
| account_write_permission: z.boolean().optional().default(true), | |
| }); |
🤖 Prompt for AI Agents
In apps/website/app/api/supabase/insert/person/route.ts between lines 14 and 25,
the schema for account_write_permission lacks an explicit default, causing it to
be undefined and silently converted to true later. To fix this, explicitly set a
default value for account_write_permission in the schema, such as
.default(false) or .default(true>, to make the default behavior clear to API
consumers and avoid hidden logic in getOrCreateAccountInternal.
| const getOrCreateDiscoursePlatformFromURL = async ( | ||
| supabasePromise: ReturnType<typeof createClient>, | ||
| currentContentURL: string, | ||
| ): Promise<GetOrCreateEntityResult<DiscoursePlatformRecord>> => { | ||
| let platformName: string | null = null; | ||
| let platformUrl: string | null = null; | ||
| const lowerCaseURL = currentContentURL.toLowerCase(); | ||
|
|
||
| if (lowerCaseURL.includes("roamresearch.com")) { | ||
| platformName = "roamresearch"; | ||
| platformUrl = "https://roamresearch.com"; | ||
| } else { | ||
| console.warn("Could not determine platform from URL:", currentContentURL); | ||
| return { | ||
| error: | ||
| "Could not determine platform from URL. Ensure it is a supported platform URL.", | ||
| entity: null, | ||
| created: false, | ||
| status: 400, | ||
| }; | ||
| } | ||
|
|
||
| if (!platformName || !platformUrl) { | ||
| return { | ||
| error: "Platform name or URL could not be derived even from a valid URL.", | ||
| entity: null, | ||
| created: false, | ||
| status: 400, | ||
| }; | ||
| } | ||
|
|
||
| const resolvedSupabaseClient = await supabasePromise; | ||
| return getOrCreateEntity<DiscoursePlatformRecord>( | ||
| resolvedSupabaseClient, | ||
| "DiscoursePlatform", | ||
| "id, name, url", | ||
| { url: platformUrl }, | ||
| { name: platformName, url: platformUrl }, | ||
| "DiscoursePlatform", | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Platform detection logic could be extended to support more platforms.
The function only supports "roamresearch.com" as a platform. While this may be sufficient for current needs, it would be better to implement a more extensible approach for future platform additions.
Consider implementing a more extensible approach for platform detection:
const lowerCaseURL = currentContentURL.toLowerCase();
- if (lowerCaseURL.includes("roamresearch.com")) {
- platformName = "roamresearch";
- platformUrl = "https://roamresearch.com";
- } else {
- console.warn("Could not determine platform from URL:", currentContentURL);
- return {
- error:
- "Could not determine platform from URL. Ensure it is a supported platform URL.",
- entity: null,
- created: false,
- status: 400,
- };
- }
+ // Define supported platforms
+ const platformMapping = [
+ {
+ pattern: "roamresearch.com",
+ name: "roamresearch",
+ url: "https://roamresearch.com"
+ },
+ // Add more platforms here as needed
+ // { pattern: "notion.so", name: "notion", url: "https://notion.so" },
+ // { pattern: "obsidian.md", name: "obsidian", url: "https://obsidian.md" },
+ ];
+
+ // Find matching platform
+ const matchedPlatform = platformMapping.find(platform =>
+ lowerCaseURL.includes(platform.pattern)
+ );
+
+ if (matchedPlatform) {
+ platformName = matchedPlatform.name;
+ platformUrl = matchedPlatform.url;
+ } else {
+ console.warn("Could not determine platform from URL:", currentContentURL);
+ return {
+ error:
+ "Could not determine platform from URL. Ensure it is a supported platform URL.",
+ entity: null,
+ created: false,
+ status: 400,
+ };
+ }This approach makes it easy to add support for new platforms without modifying the core logic.
📝 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 getOrCreateDiscoursePlatformFromURL = async ( | |
| supabasePromise: ReturnType<typeof createClient>, | |
| currentContentURL: string, | |
| ): Promise<GetOrCreateEntityResult<DiscoursePlatformRecord>> => { | |
| let platformName: string | null = null; | |
| let platformUrl: string | null = null; | |
| const lowerCaseURL = currentContentURL.toLowerCase(); | |
| if (lowerCaseURL.includes("roamresearch.com")) { | |
| platformName = "roamresearch"; | |
| platformUrl = "https://roamresearch.com"; | |
| } else { | |
| console.warn("Could not determine platform from URL:", currentContentURL); | |
| return { | |
| error: | |
| "Could not determine platform from URL. Ensure it is a supported platform URL.", | |
| entity: null, | |
| created: false, | |
| status: 400, | |
| }; | |
| } | |
| if (!platformName || !platformUrl) { | |
| return { | |
| error: "Platform name or URL could not be derived even from a valid URL.", | |
| entity: null, | |
| created: false, | |
| status: 400, | |
| }; | |
| } | |
| const resolvedSupabaseClient = await supabasePromise; | |
| return getOrCreateEntity<DiscoursePlatformRecord>( | |
| resolvedSupabaseClient, | |
| "DiscoursePlatform", | |
| "id, name, url", | |
| { url: platformUrl }, | |
| { name: platformName, url: platformUrl }, | |
| "DiscoursePlatform", | |
| ); | |
| }; | |
| const getOrCreateDiscoursePlatformFromURL = async ( | |
| supabasePromise: ReturnType<typeof createClient>, | |
| currentContentURL: string, | |
| ): Promise<GetOrCreateEntityResult<DiscoursePlatformRecord>> => { | |
| let platformName: string | null = null; | |
| let platformUrl: string | null = null; | |
| const lowerCaseURL = currentContentURL.toLowerCase(); | |
| // Define supported platforms | |
| const platformMapping = [ | |
| { | |
| pattern: "roamresearch.com", | |
| name: "roamresearch", | |
| url: "https://roamresearch.com" | |
| }, | |
| // Add more platforms here as needed | |
| // { pattern: "notion.so", name: "notion", url: "https://notion.so" }, | |
| // { pattern: "obsidian.md", name: "obsidian", url: "https://obsidian.md" }, | |
| ]; | |
| // Find matching platform | |
| const matchedPlatform = platformMapping.find(platform => | |
| lowerCaseURL.includes(platform.pattern) | |
| ); | |
| if (matchedPlatform) { | |
| platformName = matchedPlatform.name; | |
| platformUrl = matchedPlatform.url; | |
| } else { | |
| console.warn("Could not determine platform from URL:", currentContentURL); | |
| return { | |
| error: | |
| "Could not determine platform from URL. Ensure it is a supported platform URL.", | |
| entity: null, | |
| created: false, | |
| status: 400, | |
| }; | |
| } | |
| if (!platformName || !platformUrl) { | |
| return { | |
| error: "Platform name or URL could not be derived even from a valid URL.", | |
| entity: null, | |
| created: false, | |
| status: 400, | |
| }; | |
| } | |
| const resolvedSupabaseClient = await supabasePromise; | |
| return getOrCreateEntity<DiscoursePlatformRecord>( | |
| resolvedSupabaseClient, | |
| "DiscoursePlatform", | |
| "id, name, url", | |
| { url: platformUrl }, | |
| { name: platformName, url: platformUrl }, | |
| "DiscoursePlatform", | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In apps/website/app/api/supabase/insert/discourse-platform/route.ts between
lines 31 and 71, the platform detection logic only supports "roamresearch.com"
and is not easily extensible. Refactor the code to use a mapping or
configuration object that associates platform URLs with their names, then
iterate over this mapping to detect the platform. This will allow adding new
platforms by simply updating the mapping without changing the detection logic
itself.
| space_id: z.number().int().positive(), | ||
| author_id: z.number().int().positive(), | ||
| document_id: z.number().int().positive(), |
There was a problem hiding this comment.
Schema inconsistency between single and batch routes.
The batch content schema requires document_id as mandatory, while in the single content route (apps/website/app/api/supabase/insert/content/route.ts), it's optional. This inconsistency could cause confusion and issues for API consumers.
- document_id: z.number().int().positive(),
+ document_id: z.number().int().positive().optional(),📝 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.
| space_id: z.number().int().positive(), | |
| author_id: z.number().int().positive(), | |
| document_id: z.number().int().positive(), | |
| space_id: z.number().int().positive(), | |
| author_id: z.number().int().positive(), | |
| document_id: z.number().int().positive().optional(), |
🤖 Prompt for AI Agents
In apps/website/app/api/supabase/insert/content/batch/route.ts around lines 18
to 20, the batch content schema marks document_id as mandatory, but in the
single content route it is optional. To fix this inconsistency, update the batch
schema to make document_id optional by adjusting its validation rule to match
the single route schema, ensuring consistent API behavior for consumers.
| const embeddingToInsert = { | ||
| target_id, | ||
| model, | ||
| vector: vectorString, | ||
| obsolete, | ||
| }; | ||
|
|
||
| const result = await getOrCreateEntity<ContentEmbeddingRecord>( | ||
| supabase, | ||
| TARGET_EMBEDDING_TABLE, | ||
| "*", | ||
| { id: -1 }, | ||
| embeddingToInsert, | ||
| "ContentEmbedding", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
matchCriteria hard-codes id: -1, defeating “get-or-create” semantics
Using { id: -1 } guarantees the lookup always fails, so every call attempts an insert.
If the table has a unique constraint on (target_id, model) the insert will violate it and getOrCreateEntity’s conflict-recovery logic re-queries again with {id:-1}, returning 409.
Use the real uniqueness predicate instead:
- "*",
- { id: -1 },
+ "*",
+ { target_id, model },This lets the helper fetch an existing embedding and avoids spurious 409/500s.
📝 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 embeddingToInsert = { | |
| target_id, | |
| model, | |
| vector: vectorString, | |
| obsolete, | |
| }; | |
| const result = await getOrCreateEntity<ContentEmbeddingRecord>( | |
| supabase, | |
| TARGET_EMBEDDING_TABLE, | |
| "*", | |
| { id: -1 }, | |
| embeddingToInsert, | |
| "ContentEmbedding", | |
| const embeddingToInsert = { | |
| target_id, | |
| model, | |
| vector: vectorString, | |
| obsolete, | |
| }; | |
| const result = await getOrCreateEntity<ContentEmbeddingRecord>( | |
| supabase, | |
| TARGET_EMBEDDING_TABLE, | |
| "*", | |
| { target_id, model }, | |
| embeddingToInsert, | |
| "ContentEmbedding", |
🤖 Prompt for AI Agents
In
apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/routes.ts
between lines 50 and 63, the getOrCreateEntity call uses a hard-coded
matchCriteria of { id: -1 }, which causes the lookup to always fail and triggers
unnecessary inserts that violate unique constraints. Replace the matchCriteria
with the actual unique fields, such as { target_id, model }, to correctly check
for existing records before inserting and prevent conflict errors.
| const processAndCreateEmbedding = async ( | ||
| supabasePromise: ReturnType<typeof createClient>, | ||
| data: ContentEmbeddingDataInput, // data is now Zod-validated | ||
| ): Promise<GetOrCreateEntityResult<ContentEmbeddingRecord>> => { | ||
| const { target_id, model, vector, obsolete } = data; | ||
|
|
||
| const vectorString = JSON.stringify(vector); | ||
| const supabase = await supabasePromise; | ||
|
|
There was a problem hiding this comment.
createClient() is synchronous – awaiting it breaks the type contract
createClient() returns a SupabaseClient, not a Promise. Awaiting a non-promise compiles only with --suppressImplicitAnyIndexErrors or by accident, but at runtime it’s a no-op and confuses future readers / tooling.
-const processAndCreateEmbedding = async (
- supabasePromise: ReturnType<typeof createClient>,
+const processAndCreateEmbedding = async (
+ supabase: ReturnType<typeof createClient>,
data: ContentEmbeddingDataInput,
): Promise<GetOrCreateEntityResult<ContentEmbeddingRecord>> => {
- const { target_id, model, vector, obsolete } = data;
-
- const vectorString = JSON.stringify(vector);
- const supabase = await supabasePromise;
+ const { target_id, model, vector, obsolete } = data;
+ const vectorString = JSON.stringify(vector);Remember to pass the client directly from the caller as well (const supabase = createClient();).
This is a compile-time & runtime correctness issue.
🤖 Prompt for AI Agents
In
apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/routes.ts
around lines 41 to 49, the code incorrectly awaits the result of createClient(),
which is synchronous and returns a SupabaseClient directly, not a Promise.
Remove the await keyword when calling createClient() and pass the SupabaseClient
instance directly from the caller without awaiting it to fix both compile-time
and runtime correctness.
| const ContentEmbeddingDataInputSchema = z.object({ | ||
| target_id: z | ||
| .number() | ||
| .int() | ||
| .positive({ message: "target_id must be a positive integer." }), | ||
| model: z.string().min(1, { message: "Model name cannot be empty." }), | ||
| vector: z | ||
| .array(z.number()) | ||
| .nonempty({ message: "Vector array cannot be empty." }), | ||
| obsolete: z.boolean().optional().default(false), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add numerical‐safety refinements to the vector field
z.number() accepts NaN, Infinity, -Infinity, etc. Storing those values in Postgres will fail at runtime.
Add a refinement ensuring every component is finite (and, ideally, the expected dimensionality of 1536).
- vector: z
- .array(z.number())
- .nonempty({ message: "Vector array cannot be empty." }),
+ vector: z
+ .array(z.number().refine(Number.isFinite, { message: "Vector values must be finite numbers." }))
+ .length(1536, { message: "Vector must contain exactly 1 536 dimensions." }),📝 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 ContentEmbeddingDataInputSchema = z.object({ | |
| target_id: z | |
| .number() | |
| .int() | |
| .positive({ message: "target_id must be a positive integer." }), | |
| model: z.string().min(1, { message: "Model name cannot be empty." }), | |
| vector: z | |
| .array(z.number()) | |
| .nonempty({ message: "Vector array cannot be empty." }), | |
| obsolete: z.boolean().optional().default(false), | |
| }); | |
| const ContentEmbeddingDataInputSchema = z.object({ | |
| target_id: z | |
| .number() | |
| .int() | |
| .positive({ message: "target_id must be a positive integer." }), | |
| model: z.string().min(1, { message: "Model name cannot be empty." }), | |
| vector: z | |
| .array(z.number().refine(Number.isFinite, { message: "Vector values must be finite numbers." })) | |
| .length(1536, { message: "Vector must contain exactly 1 536 dimensions." }), | |
| obsolete: z.boolean().optional().default(false), | |
| }); |
🤖 Prompt for AI Agents
In
apps/website/app/api/supabase/insert/content-embedding-openai-text-embedding-3-small-1536/routes.ts
between lines 14 and 24, the vector field schema uses z.number() which allows
NaN and infinite values that will cause runtime errors when stored in Postgres.
To fix this, add a refinement to the vector array to ensure every number is
finite using a check like Number.isFinite, and also add a length check to
enforce the expected dimensionality of 1536 elements.
| } else if (data !== undefined && data !== null) { | ||
| const effectiveStatus = created ? 201 : status === 201 ? 200 : status; | ||
| response = NextResponse.json(data, { status: effectiveStatus }); | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Status code normalization silently downgrades 201 → 200
When created is false but status is 201 your current rule converts it to 200.
That makes callers think the resource already existed when it was in fact created.
Simpler & safer:
-const effectiveStatus = created ? 201 : status === 201 ? 200 : status;
+const effectiveStatus = created ? 201 : status;If a caller genuinely wants to coerce a 201 to 200 it can pass status: 200.
📝 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.
| } else if (data !== undefined && data !== null) { | |
| const effectiveStatus = created ? 201 : status === 201 ? 200 : status; | |
| response = NextResponse.json(data, { status: effectiveStatus }); | |
| } else { | |
| } else if (data !== undefined && data !== null) { | |
| - const effectiveStatus = created ? 201 : status === 201 ? 200 : status; | |
| + const effectiveStatus = created ? 201 : status; | |
| response = NextResponse.json(data, { status: effectiveStatus }); | |
| } else { |
🤖 Prompt for AI Agents
In apps/website/app/utils/supabase/apiUtils.ts around lines 31 to 34, the
current logic changes a 201 status to 200 when created is false, which
incorrectly signals the resource was not newly created. Remove the conditional
that downgrades status 201 to 200 and instead use the status as provided unless
created is true, in which case set status to 201. This ensures the status code
accurately reflects resource creation without silent downgrades.
| if (insertError.code === "23505") { | ||
| console.warn( | ||
| `Unique constraint violation on ${entityName} insert for`, | ||
| matchCriteria, | ||
| `Attempting to re-fetch.`, | ||
| ); | ||
| let reFetchQueryBuilder = supabase.from(tableName).select(selectQuery); | ||
| for (const key in matchCriteria) { | ||
| reFetchQueryBuilder = reFetchQueryBuilder.eq(key, matchCriteria[key]); | ||
| } | ||
| const { data: reFetchedEntity, error: reFetchError } = | ||
| await reFetchQueryBuilder.maybeSingle<T>(); | ||
|
|
||
| if (reFetchError) { | ||
| console.error( | ||
| `Error re-fetching ${entityName} after unique constraint violation:`, | ||
| reFetchError, | ||
| ); | ||
| return { | ||
| entity: null, | ||
| error: `Database error after unique constraint violation for ${entityName}.`, | ||
| details: reFetchError.message, | ||
| created: false, | ||
| status: 500, | ||
| }; | ||
| } | ||
| if (reFetchedEntity) { | ||
| console.log(`Found ${entityName} on re-fetch:`, reFetchedEntity); | ||
| return { | ||
| entity: reFetchedEntity, | ||
| error: null, | ||
| created: false, | ||
| status: 200, // Successfully fetched, though not created by this call | ||
| }; | ||
| } | ||
| return { | ||
| entity: null, | ||
| error: `Unique constraint violation on ${entityName} insert, and re-fetch failed to find the entity.`, | ||
| details: insertError.message, // Original insert error | ||
| created: false, | ||
| status: 409, // Conflict, and couldn't resolve by re-fetching | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Re-fetch after unique-constraint violation still uses the original (possibly wrong) matcher
If the uniqueness constraint is on (target_id, model) but callers supplied { url }, re-fetching with the same matcher will miss the existing row and return 409 even though the record exists.
Extract the conflicting keys from insertError.message (Postgres lists them) or require callers to pass matchCriteria that reflects the unique index.
At minimum, document this caveat so API route authors don’t repeat the { id: -1 } pattern.
Clarify unique‐constraint re-fetch matching logic
The current re-fetch loop (apps/website/app/utils/supabase/dbUtils.ts lines 73–114) uses exactly the caller-supplied matchCriteria, which may not correspond to the actual unique index columns. If the unique constraint is on (target_id, model) but callers pass { url }, the re-fetch will miss the existing row and return a 409 even though the record exists.
Please address this by either:
- Extracting the conflicting key(s) from
insertError.message(e.g. via a simple regex on the Postgres error text) and rebuildingmatchCriteriafrom those columns, or - Requiring (and validating) that callers pass a
matchCriteriaobject whose keys exactly match the unique index, and documenting this requirement in the function’s JSDoc/API docs.
Locations to update:
- apps/website/app/utils/supabase/dbUtils.ts lines 73–114: adjust re-fetch logic or add validation
- Function JSDoc (above the insert routine): document the requirement or caveat for
matchCriteria
🤖 Prompt for AI Agents
In apps/website/app/utils/supabase/dbUtils.ts around lines 73 to 114, the
re-fetch logic after a unique constraint violation uses the original
matchCriteria, which may not match the actual unique index columns causing false
409 errors. Fix this by either parsing the conflicting keys from
insertError.message using a regex to rebuild matchCriteria for re-fetch, or
enforce and validate that callers provide matchCriteria exactly matching the
unique index columns. Additionally, update the function JSDoc above the insert
routine to document this requirement or caveat for matchCriteria to guide API
authors.
maparent
force-pushed
the
insert-to-supabase
branch
from
62d0fcd to
30ea51a
Compare
|
new pr #167 using branch name according to devOps |
Summary by CodeRabbit
New Features
Improvements
Chores
@supabase/ssrand upgradingzod.