Skip to content

📝 design: client-uuid split (T9 follow-up)#2435

Merged
andrew-bierman merged 1 commit into
developmentfrom
design/client-uuid-split
May 17, 2026
Merged

📝 design: client-uuid split (T9 follow-up)#2435
andrew-bierman merged 1 commit into
developmentfrom
design/client-uuid-split

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

Summary

Design-only PR — no code changes. Adds docs/design/client-uuid-split.md (~650 lines).

Follow-up to PR #2433 (T9: server-side ID minting). T9 made every offline-syncable POST accept an optional id and mint one server-side when absent, which works but leaves a dual-ownership smell on the id column. This design proposes the cleaner split that Andrew flagged on the T9 PR: server-owned id (real PK) plus client-owned clientUuid (idempotency key supplied by offline-first stores).

What the doc covers

  • Background & motivation — what's hacky about T9's dual-ownership, what the clean pattern is.
  • Target schema — picks bigserial over UUIDv7 for the new server id, with reasoning. Drizzle pseudo-migration per affected table (packs, pack_items, weight_history, trips, pack_templates, pack_template_items, trail_condition_reports).
  • Migration path — three options (move/shim/hybrid) with a recommendation. Hybrid (Option C): Phase 1 adds client_uuid alongside existing text id; Phase 2 narrows id to bigserial once mobile rollout has drained.
  • Sync plugin rewire — concrete diffs for apps/expo/features/packs/store/{packs,packItems}.ts. Covers the FK edge (packItems → packs) during the offline window.
  • API surface change — additive in Phase 1 (accept clientUuid alongside id, return both). Recommends transition in place, no /v2/ prefix.
  • FKs in the offline window — local stores carry both packId (server) and packClientUuid (local); FK columns server-side reference id, never client_uuid.
  • Rollback story — Phase 1 is fully reversible at every layer.
  • Open questions — 8 items needing Andrew's call before implementation starts.
  • Implementation plan — six shippable PRs in order, each independently revertable.

Review focus

  • Is the hybrid (2-phase) migration the right risk profile, or should we big-bang it?
  • bigserial vs UUIDv7 for the new server id?
  • The local-store rewire in §4 — does the "block update until create settles" approach match how syncedCrud actually behaves in practice?
  • Open questions in §8.

Test plan

  • Andrew reads the doc end-to-end
  • Spot-check the file references (paths, line numbers, column names) against current main
  • Resolve the 8 open questions before implementation PR 1 opens
  • No code review needed — this PR has zero code

🤖 Generated with Claude Code

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Warning

Rate limit exceeded

@andrew-bierman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 27 minutes and 10 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 11540df2-e33e-40f9-8198-4a553a2688e7

📥 Commits

Reviewing files that changed from the base of the PR and between 2fb5f4e and 50e7818.

📒 Files selected for processing (1)
  • docs/design/client-uuid-split.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch design/client-uuid-split

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 16, 2026
@andrew-bierman andrew-bierman marked this pull request as ready for review May 16, 2026 22:49
Copilot AI review requested due to automatic review settings May 16, 2026 22:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a design document (docs/design/client-uuid-split.md) proposing a follow-up refactor to PR #2433 (T9). It outlines splitting the dual-owned id column on offline-first tables into a server-owned id (PK) and a client-owned clientUuid (idempotency key), with a phased migration plan, sync-plugin rewire sketches, API surface changes, and rollback story.

Changes:

  • Adds a ~650-line design doc covering motivation, target schema (bigserial PK + client_uuid), hybrid migration plan, sync-plugin rewire, API additive changes, and a 6-PR implementation plan.
  • Documents 8 open questions for the reviewer (Andrew) to resolve before implementation.
  • No code changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andrew-bierman andrew-bierman changed the base branch from main to development May 17, 2026 00:23
Architectural design doc proposing a clean split between server-owned `id`
(real PK) and client-owned `clientUuid` (idempotency key for offline-first
sync). Walks the codebase to ground the design in current schema, route
handlers, and Legend State stores. Covers schema target, migration path
(recommends hybrid 2-phase rollout), sync plugin rewire, API surface,
FK contract during offline window, rollback story, open questions, and
a 6-PR implementation plan.

Design only — no code changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@andrew-bierman andrew-bierman force-pushed the design/client-uuid-split branch from bd88349 to 50e7818 Compare May 17, 2026 00:59
@andrew-bierman andrew-bierman merged commit a14bdf0 into development May 17, 2026
7 checks passed
andrew-bierman added a commit that referenced this pull request May 17, 2026
…-split

T9 (optional id + server mintId) and T8 (POST /packs/:id/items/from-catalog)
proved hacky on a single id column with dual ownership. Per design doc
shipped in #2435 (docs/design/client-uuid-split.md), the cleaner pattern is
two columns: server-owned `id` (always minted) + client-owned `clientUuid`
(idempotency key with UNIQUE(user_id, client_uuid)).

Reverting T8/T9 here so #2433 ships only T1-T6 + T10 (lean endpoints +
API thickening). The optional-id work is preserved on feat/client-uuid-split
for the proper migration.

Removed:
- packages/api/src/utils/ids.ts (mintId helper)
- packages/api/src/routes/packs/index.ts: /:packId/items/from-catalog endpoint, mintId fallbacks
- packages/api/src/routes/trips/index.ts: mintId fallback
- packages/api/src/routes/trailConditions/reports.ts: mintId fallback, 23505-only-if-data.id branch
- packages/mcp/src/tools/packs.ts: add_pack_item_from_catalog tool
- packages/schemas/src/packs.ts: AddPackItemFromCatalogBodySchema
- packages/schemas/{trips,packs,trailConditions}.ts: id back to z.string() (required)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants