Skip to content

refactor(sdk): unify TailorDB deploy pipeline on snapshot schema#1184

Open
toiroakr wants to merge 3 commits into
mainfrom
refactor/deploy-via-snapshot
Open

refactor(sdk): unify TailorDB deploy pipeline on snapshot schema#1184
toiroakr wants to merge 3 commits into
mainfrom
refactor/deploy-via-snapshot

Conversation

@toiroakr
Copy link
Copy Markdown
Contributor

@toiroakr toiroakr commented May 15, 2026

Summary

  • Route the tailor deploy TailorDB plan and apply phases through TailorDBSnapshotType — the same canonical shape used by tailordb migrate — so the entire pipeline operates on one normalized schema representation.
  • Move field normalization (including decimal scale defaulting to the platform value 6) to the snapshot construction boundary (createSnapshotType, loadSnapshot, convertRemoteFieldsToSnapshot) and add idempotent defense-in-depth normalize at the entries of compareSnapshots / compareRemoteWithSnapshot so callers assembling literals never produce silent false drift.
  • Tighten the snapshot type surface:
    • SnapshotPermissionOperand is now a proper discriminated union (SnapshotFieldRefOperand | SnapshotValueOperand) with an exported isSnapshotFieldRefOperand type guard; protoOperand / protoGqlOperand / convertOperandToProto are exhaustive switches with satisfies never and explicit throws. The GQL path rejects non-{ user } field-refs, matching the parser's Zod schema.
    • TailorDBSnapshotType.pluralForm is required at the type level; legacy snapshots are backfilled via normalizeSnapshotType at the load boundary instead of per-consumer fallbacks.
    • Rename SnapshotTypeTailorDBSnapshotType to disambiguate against generic "snapshot type" reading and align with TailorDBType.
  • Pre-build all TailorDB inputs once at the start of planTailorDB and pass them via context.tailorDBInputs so applyTailorDB and downstream plan helpers consume the same canonical input.
  • Rewrite snapshot tests to exercise the normalization path through createSnapshotFromLocalTypes / loadSnapshot instead of pre-normalized literals.
  • User-visible effect (patch): tailor deploy no longer shows spurious drift on decimal fields without an explicit scale. The @tailor-platform/sdk/cli subpath now exports the type as TailorDBSnapshotType (previously SnapshotType), and pluralForm is required on the type; neither change has known consumers (no in-tree or docs references to the old name or to constructing the type directly).

Motivation

Route the deploy plan/apply phases through SnapshotType — the same canonical shape used by tailordb migrate — so all comparisons and plan inputs share one normalized representation. Decimal scale is now normalized to the platform default (6) at the snapshot boundary (createSnapshotType, loadSnapshot, convertRemoteFieldsToSnapshot), replacing the per-comparison getEffectiveScale workaround.

This consolidates schema normalization in one place (preventing recurrences like the decimal scale drift fixed in #1155) and prepares for a future tailordb migration sync command.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

⚡ pkg.pr.new

@tailor-platform/sdk

pnpm add https://pkg.pr.new/@tailor-platform/sdk@fab80e8
pnpm dlx https://pkg.pr.new/@tailor-platform/sdk@fab80e8 --help

@tailor-platform/create-sdk

pnpm add https://pkg.pr.new/@tailor-platform/create-sdk@fab80e8
pnpm dlx https://pkg.pr.new/@tailor-platform/create-sdk@fab80e8 my-app

commit: fab80e8

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 15, 2026

🦋 Changeset detected

Latest commit: fab80e8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@tailor-platform/sdk Patch
@tailor-platform/create-sdk Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

This comment has been minimized.

…ries

Address review feedback on the deploy-via-snapshot refactor:

- SnapshotPermissionOperand: replace the collapsing `| unknown` with a
  proper discriminated union (SnapshotFieldRefOperand |
  SnapshotValueOperand) and rewrite protoOperand / protoGqlOperand /
  convertOperandToProto as exhaustive switches with `satisfies never`
  and explicit throws. The GQL path now rejects non-`{ user }`
  field-refs, matching the parser's Zod schema.
- TailorDBSnapshotType.pluralForm is now required at the type level;
  legacy snapshots are backfilled via normalizeSnapshotType at the load
  boundary instead of per-consumer fallbacks.
- Centralize normalizeSnapshotField / normalizeSnapshotType at every
  snapshot construction boundary and add idempotent defense-in-depth
  normalize at the entries of compareSnapshots and
  compareRemoteWithSnapshot, so callers building literals never produce
  silent false drift.
- Rename SnapshotType to TailorDBSnapshotType to disambiguate against
  generic "snapshot type" reading and align with TailorDBType.
- Rewrite snapshot tests to exercise the normalization path through
  createSnapshotFromLocalTypes / loadSnapshot, and add pluralForm to
  all TailorDBSnapshotType literals (required by the new contract).
- Reword the changeset around the user-visible effect: tailor deploy
  no longer shows spurious drift on decimal fields without an
  explicit scale.
@github-actions

This comment has been minimized.

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

Unifies the tailor deploy TailorDB pipeline on the canonical TailorDBSnapshotType shape that tailordb migrate already uses. Field normalization (notably defaulting decimal scale to 6) is now centralized at snapshot construction (createSnapshotType, loadSnapshot, convertRemoteFieldsToSnapshot) with idempotent defense-in-depth normalization at compare entry points. The permission operand surface is tightened into a proper discriminated union with an exported isSnapshotFieldRefOperand guard, and SnapshotType is renamed to TailorDBSnapshotType with pluralForm required (legacy snapshots are backfilled at load time).

Changes:

  • Centralize snapshot normalization (decimal scale, pluralForm backfill) and add safety-net normalization in compareSnapshots / compareRemoteWithSnapshot.
  • Pre-convert each TailorDBService to a snapshot-shaped TailorDBDeployInput once in planTailorDB, passed via context.tailorDBInputs for downstream plan/apply consumers; permission-operand helpers become exhaustive switches.
  • Rename SnapshotTypeTailorDBSnapshotType, make pluralForm required, and rewrite tests / type-manifest generation to consume the snapshot shape.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/sdk/src/cli/lib.ts Re-export rename from SnapshotType to TailorDBSnapshotType.
packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts Adds normalizeSnapshotField / normalizeSnapshotType, splits operand union, makes pluralForm required, normalizes at create/load/compare boundaries.
packages/sdk/src/cli/commands/tailordb/migrate/snapshot.test.ts Adds pluralForm to every literal snapshot and rewrites the decimal-scale tests to flow through createSnapshotFromLocalTypes / loadSnapshot.
packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts / .test.ts Switches to TailorDBSnapshotType, drops pluralForm fallback, uses isSnapshotFieldRefOperand.
packages/sdk/src/cli/commands/tailordb/migrate/template-generator.test.ts, db-types-generator.ts / .test.ts Type rename and pluralForm additions in fixtures.
packages/sdk/src/cli/commands/deploy/tailordb/index.ts Introduces TailorDBDeployInput, threads snapshot-shaped types through plan/apply, and rewires manifest/permission builders to snapshot types.
packages/sdk/src/cli/commands/deploy/tailordb/index.test.ts, deploy.test.ts Adds tailorDBInputs to plan-context fixtures.
.changeset/unify-tailordb-deploy-on-snapshot.md Patch-level changeset for the decimal-scale fix.

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

getMigrationFilePath,
type SchemaSnapshot,
type SnapshotType,
type TailorDBSnapshotType,
…napshot

# Conflicts:
#	packages/sdk/src/cli/commands/deploy/tailordb/index.ts
@github-actions
Copy link
Copy Markdown

Code Metrics Report (packages/sdk)

main (b2bd4aa) #1184 (74c861c) +/-
Coverage 62.3% 62.5% +0.2%
Code to Test Ratio 1:0.4 1:0.4 +0.0
Details
  |                    | main (b2bd4aa) | #1184 (74c861c) |  +/-  |
  |--------------------|----------------|-----------------|-------|
+ | Coverage           |          62.3% |           62.5% | +0.2% |
  |   Files            |            364 |             364 |     0 |
  |   Lines            |          12773 |           12792 |   +19 |
+ |   Covered          |           7967 |            8007 |   +40 |
+ | Code to Test Ratio |          1:0.4 |           1:0.4 |  +0.0 |
  |   Code             |          83913 |           83980 |   +67 |
+ |   Test             |          35136 |           35202 |   +66 |

Code coverage of files in pull request scope (59.4% → 61.5%)

Files Coverage +/- Status
packages/sdk/src/cli/commands/deploy/tailordb/index.ts 38.9% -0.2% modified
packages/sdk/src/cli/commands/tailordb/migrate/db-types-generator.ts 97.2% 0.0% modified
packages/sdk/src/cli/commands/tailordb/migrate/snapshot-manifest.ts 78.5% -1.7% modified
packages/sdk/src/cli/commands/tailordb/migrate/snapshot.ts 76.5% +5.4% modified

SDK Configure Bundle Size

main (b2bd4aa) #1184 (74c861c) +/-
configure-index-size 18KB 18KB 0KB
dependency-chunks-size 33.52KB 33.52KB 0KB
total-bundle-size 51.51KB 51.51KB 0KB

Runtime Performance

main (b2bd4aa) #1184 (74c861c) +/-
Generate Median 2,279ms 2,944ms 665ms
Generate Max 2,294ms 2,967ms 673ms
Apply Build Median 2,317ms 2,987ms 670ms
Apply Build Max 2,453ms 3,021ms 568ms

Type Performance (instantiations)

main (b2bd4aa) #1184 (74c861c) +/-
tailordb-basic 35,130 35,130 0
tailordb-optional 3,841 3,841 0
tailordb-relation 7,428 7,428 0
tailordb-validate 2,566 2,566 0
tailordb-hooks 5,767 5,767 0
tailordb-object 12,136 12,136 0
tailordb-enum 2,462 2,462 0
resolver-basic 9,424 9,424 0
resolver-nested 26,111 26,111 0
resolver-array 18,187 18,187 0
executor-schedule 4,234 4,234 0
executor-webhook 873 873 0
executor-record 8,166 8,166 0
executor-resolver 4,369 4,369 0
executor-operation-function 869 869 0
executor-operation-gql 869 869 0
executor-operation-webhook 888 888 0
executor-operation-workflow 1,714 1,714 0

Reported by octocov

@toiroakr toiroakr marked this pull request as ready for review May 20, 2026 15:00
@toiroakr toiroakr requested a review from remiposo as a code owner May 20, 2026 15:00
@claude
Copy link
Copy Markdown

claude Bot commented May 20, 2026

📖 Docs Consistency Check

✅ No inconsistencies found between documentation and implementation.

Checked areas:

  • CLI documentation (packages/sdk/docs/cli/application.md): Deploy command and schema check documentation remains accurate
  • TailorDB field documentation (packages/sdk/docs/services/tailordb.md): Decimal field documentation at lines 88-103 already correctly states "When scale is omitted, the platform default of 6 is used"—this describes the expected behavior that the PR now correctly implements
  • Public API exports (packages/sdk/src/cli/lib.ts): The rename from SnapshotType to TailorDBSnapshotType affects an advanced/internal export with no documented usage patterns
  • Example code: No usage of the renamed type or direct snapshot construction found
  • CLAUDE.md: No updates needed

Summary:

This PR fixes a bug where tailor deploy showed spurious drift for decimal fields without explicit scale. The documentation already describes the correct behavior (platform materializes default scale of 6), so no doc updates are required. The type rename (SnapshotTypeTailorDBSnapshotType) and pluralForm requirement are internal API changes with no documented consumers.


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.

3 participants