Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(copilot): add seq ordinal to copilot_messages for order-preserving reads#4791
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,9 +7,26 @@ import type { PersistedMessage } from '@/lib/copilot/chat/persisted-message' | ||
| const logger = createLogger('CopilotMessagesDualWrite') | ||
| /** | ||
| * Keep the first occurrence of each message id. A single `INSERT ... ON | ||
| * CONFLICT` cannot touch the same conflict target twice, so a repeated id | ||
| * would otherwise throw. | ||
| */ | ||
| function dedupeById(messages: PersistedMessage[]): PersistedMessage[] { | ||
| const seen = new Set<string>() | ||
| const out: PersistedMessage[] = [] | ||
| for (const m of messages) { | ||
| if (seen.has(m.id)) continue | ||
| seen.add(m.id) | ||
| out.push(m) | ||
| } | ||
| return out | ||
| } | ||
| function toRow( | ||
| chatId: string, | ||
| message: PersistedMessage, | ||
| seq: number, | ||
| options?: { chatModel?: string | null; streamId?: string | null } | ||
| ): typeof copilotMessages.$inferInsert { | ||
| const ts = new Date(message.timestamp) | ||
| @@ -18,6 +35,7 @@ function toRow( | ||
| messageId: message.id, | ||
| role: message.role, | ||
| content: message, | ||
| seq, | ||
| model: options?.chatModel ?? null, | ||
| streamId: options?.streamId ?? null, | ||
| createdAt: ts, | ||
| @@ -27,8 +45,15 @@ function toRow( | ||
| /** | ||
| * Append messages to the new `copilot_messages` table. Best-effort — errors | ||
| * are logged but never thrown, since the legacy `copilot_chats.messages` | ||
| * JSONB column remains the source of truth during the dual-write rollout. | ||
| * are logged but never thrown; the legacy `copilot_chats.messages` JSONB | ||
| * column stays the source of truth during the dual-write rollout. | ||
| * | ||
| * `seq` is `MAX(seq) + index`, computed in JS (not in SQL, where every row of | ||
| * a multi-row INSERT would read the same pre-insert MAX and collide). The | ||
| * read-then-insert is non-atomic, so interleaved appends to one chat can tie | ||
| * `seq`; that window is bounded by the cutover read order (`seq, created_at, | ||
| * id`) and `replaceCopilotChatMessages`, which re-densifies `seq` from the | ||
| * authoritative JSONB order on the next snapshot save. | ||
| */ | ||
| export async function appendCopilotChatMessages( | ||
| chatId: string, | ||
| @@ -37,16 +62,23 @@ export async function appendCopilotChatMessages( | ||
| ): Promise<void> { | ||
| if (messages.length === 0) return | ||
| try { | ||
| const deduped = dedupeById(messages) | ||
| const [maxRow] = await db | ||
| .select({ maxSeq: sql<number | null>`max(${copilotMessages.seq})` }) | ||
| .from(copilotMessages) | ||
| .where(eq(copilotMessages.chatId, chatId)) | ||
| const base = (maxRow?.maxSeq ?? -1) + 1 | ||
| await db | ||
| .insert(copilotMessages) | ||
| .values(messages.map((m) => toRow(chatId, m, options))) | ||
| .values(deduped.map((m, i) => toRow(chatId, m, base + i, options))) | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| .onConflictDoUpdate({ | ||
| target: [copilotMessages.chatId, copilotMessages.messageId], | ||
| set: { | ||
| content: sql`excluded.content`, | ||
| role: sql`excluded.role`, | ||
| model: sql`COALESCE(excluded.model, ${copilotMessages.model})`, | ||
| streamId: sql`COALESCE(excluded.stream_id, ${copilotMessages.streamId})`, | ||
| seq: sql`COALESCE(${copilotMessages.seq}, excluded.seq)`, | ||
| updatedAt: sql`now()`, | ||
| }, | ||
| }) | ||
| @@ -69,7 +101,8 @@ export async function replaceCopilotChatMessages( | ||
| options?: { chatModel?: string | null } | ||
| ): Promise<void> { | ||
| try { | ||
| const newMessageIds = messages.map((m) => m.id) | ||
| const deduped = dedupeById(messages) | ||
| const newMessageIds = deduped.map((m) => m.id) | ||
| await db.transaction(async (tx) => { | ||
| // Drop rows for messages not in the new snapshot. | ||
| await tx | ||
| @@ -82,19 +115,20 @@ export async function replaceCopilotChatMessages( | ||
| ) | ||
| : eq(copilotMessages.chatId, chatId) | ||
| ) | ||
| if (messages.length === 0) return | ||
| // Upsert remaining rows. ON CONFLICT preserves existing stream_id / model | ||
| // so a snapshot save doesn't clobber metadata set during streaming. | ||
| if (deduped.length === 0) return | ||
| // Snapshot is authoritative on order, so seq = array index is overwritten | ||
| // on conflict; stream_id / model are preserved via COALESCE. | ||
| await tx | ||
| .insert(copilotMessages) | ||
| .values(messages.map((m) => toRow(chatId, m, options))) | ||
| .values(deduped.map((m, i) => toRow(chatId, m, i, options))) | ||
| .onConflictDoUpdate({ | ||
| target: [copilotMessages.chatId, copilotMessages.messageId], | ||
| set: { | ||
| content: sql`excluded.content`, | ||
| role: sql`excluded.role`, | ||
| model: sql`COALESCE(excluded.model, ${copilotMessages.model})`, | ||
| streamId: sql`COALESCE(excluded.stream_id, ${copilotMessages.streamId})`, | ||
| seq: sql`excluded.seq`, | ||
| updatedAt: sql`now()`, | ||
| }, | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| ALTER TABLE "copilot_messages" ADD COLUMN "seq" integer;--> statement-breakpoint | ||
| WITH ordered AS ( | ||
| SELECT c."id" AS chat_id, elem.value->>'id' AS message_id, elem.ord AS ord | ||
| FROM "copilot_chats" c | ||
| CROSS JOIN LATERAL jsonb_array_elements(c."messages") WITH ORDINALITY AS elem(value, ord) | ||
| WHERE jsonb_typeof(c."messages") = 'array' AND jsonb_array_length(c."messages") > 0 | ||
| ), | ||
| first_occurrence AS ( | ||
| SELECT chat_id, message_id, MIN(ord) AS first_ord FROM ordered GROUP BY chat_id, message_id | ||
| ), | ||
| ranked AS ( | ||
| SELECT chat_id, message_id, | ||
| (ROW_NUMBER() OVER (PARTITION BY chat_id ORDER BY first_ord) - 1) AS seq | ||
| FROM first_occurrence | ||
| ) | ||
| UPDATE "copilot_messages" m SET "seq" = r.seq | ||
| FROM ranked r | ||
| WHERE m."chat_id" = r.chat_id AND m."message_id" = r.message_id;--> statement-breakpoint | ||
| CREATE INDEX "copilot_messages_chat_seq_idx" ON "copilot_messages" USING btree ("chat_id","seq") WHERE "copilot_messages"."deleted_at" IS NULL; |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.