Eng 415 bulk content upsert function for sync #202
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 new composite types, utility functions, and upsert procedures for documents and content in both the database schema and TypeScript types. It adds unique constraints for embeddings, comprehensive SQL upsert logic for handling inline/local references, and detailed documentation with TypeScript usage examples. Type definitions and schema files are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SupabaseRPC
participant DB
Client->>SupabaseRPC: Call upsert_documents(space_id, docs_json)
SupabaseRPC->>DB: For each document:\n- Resolve inline author/platform\n- Insert/update Document
DB-->>SupabaseRPC: Return Document IDs
SupabaseRPC-->>Client: Document IDs
Client->>SupabaseRPC: Call upsert_content(space_id, content_json, creator_id)
SupabaseRPC->>DB: For each content:\n- Resolve inline document/author/creator\n- Insert/update Content\n- Upsert embedding if present
DB-->>SupabaseRPC: Return Content IDs
SupabaseRPC-->>Client: Content IDs
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 (
|
|
I had to delete and recreate this branch, because GitHub deleted it when the old point of comparison was deleted. |
Probably is good. But this will depend on what shows up after the scoping. As an aside, the comments on the RPC are helpful, but I'm still not satisfied with the DX. RPC's feel buried and not easy to understand what is required. For instance, in, take Maybe we need something like a helper file that has zod validation/wrappers // rpc-contracts.ts
import { z } from "zod";
export const upsertContentArgs = z.object({
space_id: z.number(),
data: z.any(), // fill this out
creator_id: z.number(),
content_as_document: z.boolean().optional(),
});
export type UpsertContentArgs = z.infer<typeof upsertContentArgs>;
export type UpsertContentResult = number[];// lib/rpc/upsertContent.ts
import { supabase } from "@/lib/supabaseClient";
import { upsertContentArgs, UpsertContentArgs, UpsertContentResult } from "./rpc";
// can add more comments here describing use
export const upsertContent = async (args: UpsertContentArgs): Promise<UpsertContentResult> => {
const parsed = upsertContentArgs.parse(args);
const { data, error } = await supabase.rpc("upsert_content", parsed);
if (error) throw error;
return data;
};
} |
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
572942c to
d4f3a52
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
d4f3a52 to
232d186
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
232d186 to
a764756
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
a764756 to
0958b76
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
444b4e8 to
32b241c
Compare
|
@CodeRabbit review |
✅ Actions performedReview 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: 8
🔭 Outside diff range comments (1)
packages/database/types.gen.ts (1)
291-303:⚠️ Potential issueType mismatch –
vectorisstringhere butnumber[]elsewhereRows & inserts for
ContentEmbedding_openai_text_embedding_3_small_1536exposevector: string, yet
inline_embedding_input.vectorand theupsert_content_embeddingRPC expectnumber[].This inconsistency forces consumers to
as unknown as stringcast, defeating type-safety.If Supabase code-gen can’t map
extensions.vectortonumber[], create a manual override:export type Vector = number[]; // … patch generated types via module-augmentationor change the RPC to accept
string.
♻️ Duplicate comments (2)
packages/database/supabase/schemas/content.sql (2)
306-353: Mirror thecontentsupdate fix inupsert_documents
The snapshot function in the schema omits updatingcontentson conflict, just like in the migration.
381-483: Unusedv_creator_idandcontent_as_documenthere too
As noted in the migration script, these parameters are never used. Adjust or remove.
🧹 Nitpick comments (8)
packages/database/doc/upsert_content.md (4)
1-1: Remove trailing full-stop in the headingMarkdown-lint (MD026) flags this; keeping headings punctuation-free prevents duplicated punctuation in ToC generators.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
5-7: Import statement should not carry a file-extension
typescript/ts-nodewill happily accept it, but most bundlers (Vite, Next, etc.) expect extension-less bare-specifiers so that they can rewrite to.js.-import type { LocalDocumentDataInput, LocalContentDataInput } from '@repo/database/input_types.ts'; +import type { LocalDocumentDataInput, LocalContentDataInput } from '@repo/database/input_types';
30-32: Example vector is unreadable – shorten or elideThe 1 536-dimension array overwhelms the doc and causes VS Code to choke on formatting. Provide 3-4 sample values followed by
/* … */so readers understand the shape without scrolling a thousand lines.
35-47: Code sample omits an async context / error handling
await client.rpcmust run inside anasyncfunction or top-level for await (Deno). Wrap both examples or add a note; otherwise newcomers will copy-paste into plain scripts and hit “await is only valid in async functions”.packages/database/input_types.ts (2)
1-2: Path portability – prefer extension-less importSame argument as in docs; ersatz bundlers dislike
.tsin bare specifiers.-import { Database, TablesInsert } from "./types.gen.ts"; +import { Database, TablesInsert } from "./types.gen";
3-5:Partial<…>swallows required columns – deliberate?Wrapping the intersected type in
Partialmakes every slot optional, includingsource_local_id,created, etc.
If some fields are genuinely mandatory (as enforced in SQL), keep them outside thePartial:export type LocalDocumentDataInput = Omit<Database['public']['CompositeTypes']['document_local_input'], 'author_inline'> & { // still optional author_inline?: Partial<TablesInsert<'PlatformAccount'>> };packages/database/supabase/migrations/20250606202159_content_upsert_function.sql (1)
1-2: Make index creation idempotent
Consider addingIF NOT EXISTStoCREATE UNIQUE INDEXto avoid failures on repeated runs in environments where the index may already exist.packages/database/supabase/schemas/content.sql (1)
122-124: Re-evaluateNULLS DISTINCTon content index
Same as above—ensure your Postgres version supportsNULLS DISTINCTor prefer aWHERE source_local_id IS NOT NULLclause for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/database/doc/upsert_content.md(1 hunks)packages/database/input_types.ts(1 hunks)packages/database/schema.yaml(1 hunks)packages/database/supabase/migrations/20250606202159_content_upsert_function.sql(1 hunks)packages/database/supabase/schemas/content.sql(2 hunks)packages/database/supabase/schemas/embedding.sql(1 hunks)packages/database/types.gen.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/database/input_types.ts (1)
packages/database/types.gen.ts (2)
Database(9-747)TablesInsert(778-799)
🪛 markdownlint-cli2 (0.17.2)
packages/database/doc/upsert_content.md
1-1: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
🔇 Additional comments (6)
packages/database/input_types.ts (1)
6-12:document_inlineshould probably be optional
LocalContentDataInputcurrently allows content objects that must embed a fulldocument_inline, even whendocument_local_idis supplied. Prefix the field with?so callers can choose either linkage strategy.- document_inline: LocalDocumentDataInput, + document_inline?: LocalDocumentDataInput,packages/database/types.gen.ts (1)
696-744: Composite types still contain nullable scalar fields – enforce at least one locator
content_local_inputallows all ofdocument_id,document_local_id, anddocument_inlineto be null simultaneously, making it impossible for the server to resolve the target document.Consider a check constraint in the PL/pgSQL wrapper or split the type into “by-id” vs “inline” variants.
packages/database/supabase/migrations/20250606202159_content_upsert_function.sql (3)
3-50: New composite input types look solid
Thedocument_local_input,inline_embedding_input, andcontent_local_inputtypes cleanly encapsulate both DB columns and local/inline references.
122-152:upsert_platform_account_inputis well-implemented
The use ofON CONFLICTwithCOALESCEensures safe upserts of platform accounts and returns the correctid.
208-227: Content embedding upsert is correct
The warning path prevents failures on invalid embeddings, and theON CONFLICTbranch properly refreshesvectorandobsolete.packages/database/supabase/schemas/content.sql (1)
45-46: Check PG version support forNULLS DISTINCT
NULLS DISTINCTin unique indexes requires PG 15+. Verify compatibility or replace with a partial index:CREATE UNIQUE INDEX … ON public."Document"(space_id, source_local_id) WHERE source_local_id IS NOT NULL;
packages/database/supabase/migrations/20250606202159_content_upsert_function.sql
Outdated
Show resolved
Hide resolved
packages/database/supabase/migrations/20250606202159_content_upsert_function.sql
Outdated
Show resolved
Hide resolved
packages/database/supabase/migrations/20250606202159_content_upsert_function.sql
Outdated
Show resolved
Hide resolved
packages/database/supabase/migrations/20250606202159_content_upsert_function.sql
Outdated
Show resolved
Hide resolved
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
32b241c to
c4f951c
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
c4f951c to
c09189b
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
c09189b to
c021cb2
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
c021cb2 to
a04ee1d
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
a04ee1d to
7b5b229
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
7b5b229 to
df4bfa4
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
df4bfa4 to
8e91745
Compare
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
8e91745 to
1b56e75
Compare
| @@ -0,0 +1,13 @@ | |||
| import { Database, TablesInsert } from "./types.gen.ts"; | |||
There was a problem hiding this comment.
nit: Typescript files are generally camelCase. If you change it, also update the .md file that references it.
There was a problem hiding this comment.
ah, missed that before merging. I'll make a small PR for this.
|
I agree validation would help. Typing is only helping up to a point. This is what we wanted to help with drizzle, later. I'm still -1 on introducing zod before drizzle is there, as it's wasted effort. |
maparent
force-pushed
the
ENG-415-content-upsert-function
branch
from
1b56e75 to
a20324f
Compare
2 participants
Proposed replacement for the
alpha-upload-discourse-nodesfunction insupabase/schema/upload_temp.sqlSummary by CodeRabbit
New Features
Documentation
Database