Skip to content

TML-2732: collation-independent storage-sort comparator#721

Merged
wmadden merged 2 commits into
mainfrom
tml-2732-canonical-storage-hashing-depends-on-host-collation
Jun 5, 2026
Merged

TML-2732: collation-independent storage-sort comparator#721
wmadden merged 2 commits into
mainfrom
tml-2732-canonical-storage-hashing-depends-on-host-collation

Conversation

@wmadden-electric
Copy link
Copy Markdown
Contributor

@wmadden-electric wmadden-electric commented Jun 4, 2026

Replace localeCompare with a deterministic code-unit comparator in compareByNameProperty (packages/1-framework/0-foundation/contract/src/canonicalization-storage-sort.ts).

Why

storageHash stability must not depend on the host. compareByNameProperty feeds createStorageSort → the SQL canonicalization hook → canonicalizeContract, so ordering index/unique arrays with localeCompare (no locale/options) makes canonical bytes depend on the host's ICU/collation build — two machines with different ICU data could produce a different storageHash for the same contract. This was a verbatim carry-over flagged by CodeRabbit on #631 and deliberately deferred there (that PR had a byte-identical-output invariant; changing the comparator is a hashing-semantics change, so it could not ride along).

Change

  • Comparator → code-unit: nameA < nameB ? -1 : nameA > nameB ? 1 : 0. This is the only localeCompare on the canonicalization path.
  • Determinism regression test pinning compareByNameProperty({ name: 'Z' }, { name: 'a' }) < 0 — code-unit orders Z (U+005A) before a (U+0061); locale collation would not, so a reversion to localeCompare fails the test.

Scope / verification (reviewer ~1 min)

  • 2-file diff (comparator + test).
  • pnpm typecheck + 168/168 package tests + pnpm lint green.
  • Fixture stability: verified zero drift — no committed contract fixture (125 scanned) has a ≥2-entry named index/unique array, so the sort ordering is never exercised on fixture data; fixtures:check is a no-op for this change. (Current names are all lowercase ASCII snake_case, where code-unit and locale order coincide anyway.)

Refs: TML-2732

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Updated name-property sorting to use consistent code-unit ordering instead of locale-based collation, ensuring predictable and uniform sort results across environments.
  • Tests
    • Added a test to verify non-locale (code-unit) ordering behavior for name values to prevent regressions.

…onicalization-storage-sort

storageHash was non-deterministic across hosts with different ICU/collation builds because
compareByNameProperty used localeCompare. Replace with a code-unit ternary so ordering is
byte-stable regardless of locale. Existing idx_*/uq_* fixture names are lowercase ASCII
snake_case, so code-unit order equals locale order for them — fixtures are unaffected.

Adds a locale-independence regression test pinning Z < a (code-unit: U+005A < U+0061);
a future reversion to localeCompare would fail this case on most locale builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: wmadden-electric <286902546+wmadden-electric@users.noreply.github.com>
@wmadden-electric wmadden-electric requested a review from a team as a code owner June 4, 2026 15:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: a8443a2c-5467-493b-badd-1119a0476c76

📥 Commits

Reviewing files that changed from the base of the PR and between ff65d14 and 255cc50.

📒 Files selected for processing (2)
  • packages/1-framework/0-foundation/contract/src/canonicalization-storage-sort.ts
  • packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts
💤 Files with no reviewable changes (1)
  • packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts

📝 Walkthrough

Walkthrough

compareByNameProperty was changed to compare name strings by raw UTF-16 code units via a new compareCodeUnits helper instead of using localeCompare. A Vitest case was added to assert that { name: 'Z' } sorts before { name: 'a' }.

Changes

Code-unit comparison in string sorting

Layer / File(s) Summary
Introduce compareCodeUnits and switch comparator
packages/1-framework/0-foundation/contract/src/canonicalization-storage-sort.ts
Adds an internal compareCodeUnits(a, b) helper and updates compareByNameProperty to return compareCodeUnits(nameA, nameB) (keeps '' fallback for missing/non-string names).
Regression test for code-unit ordering
packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts
Adds a Vitest assertion verifying { name: 'Z' } compares as less than { name: 'a' }, validating code-unit ordering.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 I hopped through names both big and small,
Counting code units, not locales at all,
Z leaps before a with a cheerful thump,
Sorting now sings — no collation bump! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing locale-dependent comparison with deterministic UTF-16 code-unit comparison in the storage-sort comparator to ensure collation-independence.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tml-2732-canonical-storage-hashing-depends-on-host-collation

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.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 4, 2026

Open in StackBlitz

@prisma-next/extension-author-tools

npm i https://pkg.pr.new/@prisma-next/extension-author-tools@721

@prisma-next/mongo-runtime

npm i https://pkg.pr.new/@prisma-next/mongo-runtime@721

@prisma-next/family-mongo

npm i https://pkg.pr.new/@prisma-next/family-mongo@721

@prisma-next/sql-runtime

npm i https://pkg.pr.new/@prisma-next/sql-runtime@721

@prisma-next/family-sql

npm i https://pkg.pr.new/@prisma-next/family-sql@721

@prisma-next/extension-arktype-json

npm i https://pkg.pr.new/@prisma-next/extension-arktype-json@721

@prisma-next/middleware-cache

npm i https://pkg.pr.new/@prisma-next/middleware-cache@721

@prisma-next/mongo

npm i https://pkg.pr.new/@prisma-next/mongo@721

@prisma-next/extension-paradedb

npm i https://pkg.pr.new/@prisma-next/extension-paradedb@721

@prisma-next/extension-pgvector

npm i https://pkg.pr.new/@prisma-next/extension-pgvector@721

@prisma-next/extension-postgis

npm i https://pkg.pr.new/@prisma-next/extension-postgis@721

@prisma-next/postgres

npm i https://pkg.pr.new/@prisma-next/postgres@721

@prisma-next/sql-orm-client

npm i https://pkg.pr.new/@prisma-next/sql-orm-client@721

@prisma-next/sqlite

npm i https://pkg.pr.new/@prisma-next/sqlite@721

@prisma-next/target-mongo

npm i https://pkg.pr.new/@prisma-next/target-mongo@721

@prisma-next/adapter-mongo

npm i https://pkg.pr.new/@prisma-next/adapter-mongo@721

@prisma-next/driver-mongo

npm i https://pkg.pr.new/@prisma-next/driver-mongo@721

@prisma-next/contract

npm i https://pkg.pr.new/@prisma-next/contract@721

@prisma-next/utils

npm i https://pkg.pr.new/@prisma-next/utils@721

@prisma-next/config

npm i https://pkg.pr.new/@prisma-next/config@721

@prisma-next/errors

npm i https://pkg.pr.new/@prisma-next/errors@721

@prisma-next/framework-components

npm i https://pkg.pr.new/@prisma-next/framework-components@721

@prisma-next/operations

npm i https://pkg.pr.new/@prisma-next/operations@721

@prisma-next/ts-render

npm i https://pkg.pr.new/@prisma-next/ts-render@721

@prisma-next/contract-authoring

npm i https://pkg.pr.new/@prisma-next/contract-authoring@721

@prisma-next/ids

npm i https://pkg.pr.new/@prisma-next/ids@721

@prisma-next/psl-parser

npm i https://pkg.pr.new/@prisma-next/psl-parser@721

@prisma-next/psl-printer

npm i https://pkg.pr.new/@prisma-next/psl-printer@721

@prisma-next/cli

npm i https://pkg.pr.new/@prisma-next/cli@721

@prisma-next/cli-telemetry

npm i https://pkg.pr.new/@prisma-next/cli-telemetry@721

@prisma-next/emitter

npm i https://pkg.pr.new/@prisma-next/emitter@721

@prisma-next/migration-tools

npm i https://pkg.pr.new/@prisma-next/migration-tools@721

prisma-next

npm i https://pkg.pr.new/prisma-next@721

@prisma-next/vite-plugin-contract-emit

npm i https://pkg.pr.new/@prisma-next/vite-plugin-contract-emit@721

@prisma-next/mongo-codec

npm i https://pkg.pr.new/@prisma-next/mongo-codec@721

@prisma-next/mongo-contract

npm i https://pkg.pr.new/@prisma-next/mongo-contract@721

@prisma-next/mongo-value

npm i https://pkg.pr.new/@prisma-next/mongo-value@721

@prisma-next/mongo-contract-psl

npm i https://pkg.pr.new/@prisma-next/mongo-contract-psl@721

@prisma-next/mongo-contract-ts

npm i https://pkg.pr.new/@prisma-next/mongo-contract-ts@721

@prisma-next/mongo-emitter

npm i https://pkg.pr.new/@prisma-next/mongo-emitter@721

@prisma-next/mongo-schema-ir

npm i https://pkg.pr.new/@prisma-next/mongo-schema-ir@721

@prisma-next/mongo-query-ast

npm i https://pkg.pr.new/@prisma-next/mongo-query-ast@721

@prisma-next/mongo-orm

npm i https://pkg.pr.new/@prisma-next/mongo-orm@721

@prisma-next/mongo-query-builder

npm i https://pkg.pr.new/@prisma-next/mongo-query-builder@721

@prisma-next/mongo-lowering

npm i https://pkg.pr.new/@prisma-next/mongo-lowering@721

@prisma-next/mongo-wire

npm i https://pkg.pr.new/@prisma-next/mongo-wire@721

@prisma-next/sql-contract

npm i https://pkg.pr.new/@prisma-next/sql-contract@721

@prisma-next/sql-errors

npm i https://pkg.pr.new/@prisma-next/sql-errors@721

@prisma-next/sql-operations

npm i https://pkg.pr.new/@prisma-next/sql-operations@721

@prisma-next/sql-schema-ir

npm i https://pkg.pr.new/@prisma-next/sql-schema-ir@721

@prisma-next/sql-contract-psl

npm i https://pkg.pr.new/@prisma-next/sql-contract-psl@721

@prisma-next/sql-contract-ts

npm i https://pkg.pr.new/@prisma-next/sql-contract-ts@721

@prisma-next/sql-contract-emitter

npm i https://pkg.pr.new/@prisma-next/sql-contract-emitter@721

@prisma-next/sql-lane-query-builder

npm i https://pkg.pr.new/@prisma-next/sql-lane-query-builder@721

@prisma-next/sql-relational-core

npm i https://pkg.pr.new/@prisma-next/sql-relational-core@721

@prisma-next/sql-builder

npm i https://pkg.pr.new/@prisma-next/sql-builder@721

@prisma-next/target-postgres

npm i https://pkg.pr.new/@prisma-next/target-postgres@721

@prisma-next/target-sqlite

npm i https://pkg.pr.new/@prisma-next/target-sqlite@721

@prisma-next/adapter-postgres

npm i https://pkg.pr.new/@prisma-next/adapter-postgres@721

@prisma-next/adapter-sqlite

npm i https://pkg.pr.new/@prisma-next/adapter-sqlite@721

@prisma-next/driver-postgres

npm i https://pkg.pr.new/@prisma-next/driver-postgres@721

@prisma-next/driver-sqlite

npm i https://pkg.pr.new/@prisma-next/driver-sqlite@721

commit: 255cc50

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts (1)

100-102: ⚡ Quick win

Drop the inline explanatory block and keep just the test case.

The test name and assertion already capture intent; these comments add noise in a file where concision is preferred.

As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Don't add comments if avoidable, prefer code that expresses its intent."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts`
around lines 100 - 102, Remove the three-line explanatory comment block about
locale-independence/code-unit order that precedes the test in
canonicalization-storage-sort.test.ts and leave only the test case and its
assertion; do not change the test name or logic (the test already documents
intent), simply delete the inline comment to keep the file concise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts`:
- Around line 100-102: Remove the three-line explanatory comment block about
locale-independence/code-unit order that precedes the test in
canonicalization-storage-sort.test.ts and leave only the test case and its
assertion; do not change the test name or logic (the test already documents
intent), simply delete the inline comment to keep the file concise.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: f75707a0-5bde-4f74-b490-29bc065cd214

📥 Commits

Reviewing files that changed from the base of the PR and between c159cd4 and ff65d14.

📒 Files selected for processing (2)
  • packages/1-framework/0-foundation/contract/src/canonicalization-storage-sort.ts
  • packages/1-framework/0-foundation/contract/test/canonicalization-storage-sort.test.ts

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

size-limit report 📦

Path Size
postgres / no-emit 145.22 KB (0%)
postgres / emit 117.03 KB (+0.01% 🔺)
mongo / no-emit 75.97 KB (0%)
mongo / emit 70.78 KB (0%)
cf-worker / no-emit 174.93 KB (+0.01% 🔺)
cf-worker / emit 143.71 KB (+0.02% 🔺)

Comment thread packages/1-framework/0-foundation/contract/src/canonicalization-storage-sort.ts Outdated
Copy link
Copy Markdown
Contributor

@wmadden wmadden left a comment

Choose a reason for hiding this comment

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

I stand corrected, < is exactly what we should do.

Address review on #721: name the deterministic comparator so its intent
reads from the code (not a bare `<`/`>`), and drop the verbose inline
comment from the regression test (the test name carries the intent).
No behavioural change — still UTF-16 code-unit, collation-independent order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
@wmadden wmadden merged commit b0902a7 into main Jun 5, 2026
21 checks passed
@wmadden wmadden deleted the tml-2732-canonical-storage-hashing-depends-on-host-collation branch June 5, 2026 02:51
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.

2 participants