Skip to content

feat(copilot): add seq ordinal to copilot_messages for order-preserving reads#4791

Merged
waleedlatif1 merged 1 commit into
stagingfrom
waleedlatif1/copilot-messages-seq
May 29, 2026
Merged

feat(copilot): add seq ordinal to copilot_messages for order-preserving reads#4791
waleedlatif1 merged 1 commit into
stagingfrom
waleedlatif1/copilot-messages-seq

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

What

Adds an integer seq column to copilot_messages = the message's 0-based index within the chat's JSONB array. This is the ground-truth order and unblocks the eventual read cutover (R+1) off the legacy copilot_chats.messages JSONB.

Reads still come from JSONB after this PR — this is purely the prerequisite that makes the new table safe to read from later.

Why

copilot_messages had no column that preserves message order:

  • created_at (set from each message's timestamp) ties at millisecond granularity in 58% of chats (30,796 / 53,484 in prod).
  • 112 chats even have out-of-order timestamps within their array.
  • The only other tiebreaker, id, is a random UUID v4.

So ORDER BY created_at, id renders same-timestamp user/assistant pairs swapped (assistant answer before the user question) — a proven production bug.

How

  • Migration 0219 (inline backfill, no script for self-hosters or us): ADD COLUMN seq → backfill from jsonb_array_elements ... WITH ORDINALITY (first-occurrence per (chat,id), gap-free via ROW_NUMBER) → CREATE INDEX (chat_id, seq) WHERE deleted_at IS NULL.
  • Dual-write (messages-dual-write.ts):
    • replace (update-messages snapshot, authoritative on order) overwrites seq = array index — re-densifies positions after a mid-conversation delete.
    • append preserves existing seq via COALESCE and assigns base + idx from a single MAX(seq) read (never MAX+i in SQL — multi-row batches would collide).
    • Dedupe message ids before insert (87 prod chats carry duplicate ids; a repeated id in one INSERT ... ON CONFLICT would otherwise throw).

Design decisions

  • seq is a tiebreaker, not the sole sort key — read order at cutover is ORDER BY seq NULLS LAST, created_at, id, so a concurrent-append seq tie or an un-backfilled NULL row can never re-introduce the swap.
  • Nullable now, defer NOT NULL — during rolling deploys, old pods insert rows without seq; a NOT NULL column would abort their best-effort inserts and silently drop dual-write coverage.

Performance

Neutral-to-better steady-state. The (chat_id, seq) index (int4) is smaller than the existing (chat_id, created_at, id); reads are the same O(log n + k). Appends add one cheap MAX(seq) probe; the snapshot-replace path assigns seq while already iterating. The only cost is the one-time backfill UPDATE, run inline with statement_timeout disabled — same shape/cost as the 0217 backfill that already ran cleanly in prod.

Verification

  • 459 test files / 7,149 tests pass; 15 dual-write unit tests cover seq base+idx, preserve-vs-overwrite, dedupe, and empty-chat start.
  • Backfill CTE validated against staging data: 0-based, gap-free, contiguous for every chat (0 bad ranges).
  • bun run check:api-validation passes.

Follow-ups (not this PR)

  • R+1: flip read paths to copilot_messages ORDER BY seq NULLS LAST, created_at, id behind a % rollout flag, gated on a parity query returning 0.
  • Later: SET NOT NULL on seq (after a NULL sweep) + drop copilot_messages_chat_created_at_idx.

🤖 Generated with Claude Code

@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 29, 2026 6:38pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 29, 2026

PR Summary

Medium Risk
Touches copilot message persistence and a production backfill migration; behavior is best-effort and nullable to avoid breaking rolling deploys, but ordering bugs were the motivation and concurrent appends can still briefly tie seq.

Overview
Adds a nullable integer seq on copilot_messages (migration 0219) and backfills it from legacy chat JSONB order, plus a partial index on (chat_id, seq).

Dual-write now writes seq on every insert: append uses MAX(seq) + index in application code and keeps existing seq on upsert via COALESCE; replace sets seq to the snapshot’s 0-based index and overwrites seq on conflict so order re-densifies after deletes. Duplicate message IDs in one batch are deduped before insert to avoid ON CONFLICT errors.

Unit tests are extended for seq assignment, preserve-vs-overwrite conflict behavior, and deduplication. Reads still use JSONB; this only prepares ordered reads from the new table later.

Reviewed by Cursor Bugbot for commit 38b9948. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR adds a nullable seq integer column to copilot_messages as a ground-truth ordering key, unblocking a future read cutover off the legacy copilot_chats.messages JSONB. Reads remain JSONB-based after this PR.

  • Migration 0219 adds the column, backfills 0-based seq from JSONB ordinals via a deduplicating CTE, and creates a partial index on (chat_id, seq) WHERE deleted_at IS NULL.
  • Dual-write: appendCopilotChatMessages reads MAX(seq) in JS then assigns base + idx, uses COALESCE(existing.seq, excluded.seq) on conflict, and deduplicates message IDs before insert. replaceCopilotChatMessages assigns array index directly and overwrites seq on conflict to re-densify after deletes.
  • Test suite adds 7 focused unit tests covering seq base/continuation, COALESCE-preserve vs overwrite, and duplicate-ID collapse for both code paths.

Confidence Score: 5/5

Safe to merge — purely additive shadow write with no read-path changes and a well-validated backfill.

The nullable column protects rolling deploys on old pods. The backfill CTE is correct (deduped by first occurrence, 0-based via ROW_NUMBER), the dual-write logic handles all edge cases (MAX in JS to avoid multi-row SQL collision, COALESCE-preserve on append, overwrite on replace, dedup before insert), and 15 new unit tests exercise every seq assignment path.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/copilot/chat/messages-dual-write.ts Adds seq dual-write: deduplication helper, toRow gains seq param, appendCopilotChatMessages reads MAX(seq) before insert and uses COALESCE-preserve on conflict, replaceCopilotChatMessages assigns 0-based array index and overwrites on conflict.
apps/sim/lib/copilot/chat/messages-dual-write.test.ts Adds 7 new test cases covering seq base+idx, preserve-vs-overwrite on conflict, dedupe collapse for both append and replace paths; introduces lastValuesRows() helper and makes MAX(seq) mocking explicit.
packages/db/migrations/0219_amused_leo.sql Adds nullable seq integer column, backfills via JSONB WITH ORDINALITY + ROW_NUMBER CTE (0-based, deduped by first occurrence), and creates a partial index on (chat_id, seq) WHERE deleted_at IS NULL.
packages/db/schema.ts Adds seq: integer('seq') (nullable) to copilotMessages and the chatSeqIdx partial index on (chatId, seq) WHERE deletedAt IS NULL; existing indexes unchanged.
packages/testing/src/mocks/schema.mock.ts Adds seq: 'seq' to the copilotMessages schema mock to keep it in sync with the real schema.

Reviews (2): Last reviewed commit: "feat(copilot): add seq ordinal to copilo..." | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/chat/messages-dual-write.ts
Comment thread apps/sim/lib/copilot/chat/messages-dual-write.test.ts
…ng reads

copilot_messages had no column preserving message order: created_at (set
from each message's timestamp) ties at millisecond granularity in 58% of
chats, and some chats have out-of-order timestamps within their array. The
only other tiebreaker, id, is a random UUID — so ORDER BY created_at, id
renders same-timestamp user/assistant pairs swapped. This blocks the R+1
read cutover.

Add an integer seq = the message's 0-based index within the chat's JSONB
array (ground-truth order), backfilled inline in migration 0219 (no script
for self-hosters or us). Reads will use ORDER BY seq NULLS LAST, created_at,
id at cutover; reads still come from JSONB after this PR.

Design:
- seq is a tiebreaker, not the sole sort key (concurrent-append/NULL safety).
- Nullable now; defer NOT NULL so rolling-deploy old pods don't fail inserts.
- replace (update-messages snapshot) overwrites seq = array index
  (re-densifies after a mid-conversation delete); append preserves existing
  seq via COALESCE and assigns base+idx from a single MAX(seq) read (never
  MAX+i in SQL — multi-row batches would collide). The non-atomic
  read-then-insert window is documented and bounded by the read tiebreak +
  snapshot re-densify.
- Dedupe message ids before insert (87 prod chats carry dup ids; a repeated
  id in one INSERT...ON CONFLICT would otherwise throw).
- Backfill picks first-occurrence per (chat,id), gap-free via ROW_NUMBER;
  validated on staging data (0-based, contiguous, 0 bad ranges).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 38b9948. Configure here.

@waleedlatif1 waleedlatif1 merged commit fd77bb4 into staging May 29, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/copilot-messages-seq branch May 29, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant