Commit 9ccebe0
authored
TML-3008: differ pairs siblings by (nodeKind, id) — delete the role-sigil workaround (#952)
## Linked issue
Refs
[TML-3008](https://linear.app/prisma-company/issue/TML-3008/schema-differ-pairs-siblings-by-nodekind-id-delete-the-role-sigil)
Unblocks [#950](#950)
(TML-2870), which rebases onto this and deletes its `role:`-sigil
workaround.
## At a glance
```ts
// packages/1-framework/1-core/framework-components/src/control/schema-diff.ts
export interface DiffableNode {
readonly id: string;
readonly nodeKind: string;
isEqualTo(other: DiffableNode): boolean;
children(): readonly DiffableNode[];
}
```
Before this PR the differ keyed a parent's children into one flat map on
bare `id` and threw `diffSchemas: duplicate id among siblings` on any
collision — so a Postgres role named `public` and a schema named
`public`, which are different objects that never pair against each
other, could not coexist as siblings in the diff tree.
## Decision
Two things:
1. **The differ pairs siblings by `(nodeKind, id)`, not by a
globally-unique `id`.** `DiffableNode` gains `readonly nodeKind: string`
— the per-node discriminant every SQL schema-IR node already declares,
so the SQL tree satisfies it with zero changes. An `id` now only needs
to be unique among same-kind siblings, which it already is by
construction (you can't declare two roles named `public`, or two columns
named `id` on one table). A node never folds its kind into its id string
to route around the differ.
2. **`SchemaDiffIssue.message` is deleted.** The differ reports
structured data (`path`, `reason`, the nodes); turning that into a human
sentence is the renderer's job. Every producer that baked prose into
`message` — including Mongo's hand-rolled `diffMongoSchemas` — stops;
every consumer renders from `path`/`reason` itself.
## Reviewer notes
- **Byte-identity is the review question.** Sibling pairing uses a
single `Map` keyed on `` `${nodeKind}\u0000${id}` `` — deliberately
*not* a nested per-kind map — so insertion order, and therefore
emitted-issue order, is bit-for-bit what it was. Every `nodeKind` in the
repo is a code-defined literal, so the NUL delimiter cannot collide. The
only input whose behavior changes is one that previously **threw**
(cross-kind sibling id collision); no previously-succeeding diff changes
output. Zero fixture/snapshot/golden files changed in this PR.
- **Mongo `db verify` failure text becomes path-based.**
`diffMongoSchemas` used to bake sentences like `Collection "users" is
missing from the database` into `message`; issues now render as `users`
/ `orders/validator` with the `reason` label. Accepted deliberately — no
committed snapshot pinned the prose, and a follow-up renderer can
rebuild richer text from the structured issue when wanted.
- **The `message` sweep is most of the diff.** The keying change itself
is ~40 lines; the other ~30 files are consumers and test literals that
carried `message`. One consumer was missed on the first pass and caught
by review: the CLI error-envelope formatter printed blank issue bodies
for SQLite/Mongo verify failures
([errors.ts](packages/1-framework/3-tooling/cli/src/utils/formatters/errors.ts)
now renders `message ?? path.join('/')`, since PSL diagnostics share
that envelope and legitimately carry `message`).
- **Slice process artifacts**
(`projects/postgres-rls/slices/differ-kind-keying/`) are committed for
review context and are cleaned up at project close-out, per this repo's
Drive convention.
## How it fits together
1. **The interface carries the discriminant.** `DiffableNode.nodeKind`
is an opaque string — no target vocabulary enters the framework. SQL
nodes inherit it from `SqlSchemaIRNode`'s existing abstract `nodeKind`;
the five Mongo schema-IR nodes each gain a genuine per-node literal
(`collection`, `index`, `validator`, `collectionOptions`, `schema`)
matching their existing `kind` discriminators 1:1.
2. **The differ keys on the pair.** `insertNode`/`diffChildren` build
the sibling map on the combined key and throw only on a genuine
same-kind/same-id duplicate
([schema-diff.ts](packages/1-framework/1-core/framework-components/src/control/schema-diff.ts)).
3. **Issues drop the baked message.** `pathMessage` and
`SchemaDiffIssue.message` are deleted; consumers that need a string
build it from `path` — the CLI verify formatter, `db verify`, the
sqlite/postgres issue planners' conflict summaries, and the CLI
error-envelope formatter.
4. **The ADR records the model.**
[adr-schema-diff-over-structured-ir.md](projects/postgres-rls/specs/adr-schema-diff-over-structured-ir.md)
now states per-kind sibling-id uniqueness and explicitly rejects folding
a node's kind into its `id()` (the guidance that produced #950's sigil).
## Behavior changes & evidence
- Same-name, different-kind siblings diff instead of throwing; each
pairs only against its own-kind counterpart. Implementation:
[schema-diff.ts](packages/1-framework/1-core/framework-components/src/control/schema-diff.ts).
Evidence:
[schema-diff.test.ts](packages/1-framework/1-core/framework-components/test/schema-diff.test.ts)
(role-`public`/namespace-`public` pair independently; same-kind/same-id
still throws; issues carry no `message`).
- Mongo schema-IR nodes declare `nodeKind`; the Mongo diff algorithm is
otherwise unchanged apart from dropping `message`. Implementation:
[schema-node.ts](packages/2-mongo-family/3-tooling/mongo-schema-ir/src/schema-node.ts),
[schema-diff.ts](packages/2-mongo-family/9-family/src/core/schema-diff.ts).
Evidence:
[cli.control-policy.mongo.e2e.test.ts](test/integration/test/cli.control-policy.mongo.e2e.test.ts).
- CLI issue rendering is path-based where prose was removed, with PSL
diagnostics keeping their messages. Implementation:
[verify.ts](packages/1-framework/3-tooling/cli/src/utils/formatters/verify.ts),
[errors.ts](packages/1-framework/3-tooling/cli/src/utils/formatters/errors.ts).
Evidence:
[output.errors.test.ts](packages/1-framework/3-tooling/cli/test/output.errors.test.ts)
(pins both producer shapes).
## Testing performed
- `pnpm build` (68/68), forced `pnpm typecheck` (143/143), `pnpm lint` +
`pnpm lint:deps` on touched packages
- `pnpm test:packages` (12k+ tests), `pnpm test:integration`, `pnpm
test:e2e` (109/109)
- `pnpm fixtures:check` — clean; `git status` after: no
fixture/snapshot/golden file changed
- Reproduced-and-ruled-pre-existing flakes: config-loader `/var`
symlink, sql-orm-client portal race (both known families, untouched
files)
## Follow-ups
- PR #950 rebases onto this and deletes its `role:` sigil +
`withCleanRoleMessage` (TML-2870).
- Optional richer issue rendering (human sentences rebuilt from
structured issues in the CLI formatter) if path-based output proves too
terse.
## Alternatives considered
- **Fold the kind into the id string (the `role:` sigil).** Rejected —
it re-encodes lost structural information into a name, then leaks into
paths and needs laundering (`withCleanRoleMessage`). That workaround is
what #950 currently ships and deletes after this merges.
- **Key the differ on the contract `EntityCoordinate`.** Rejected (and
the ADR's original rejection stands) — schema-IR-node identity is its
own domain; mapping a schema-IR node to a contract coordinate is a
separate consumer-side operation, not the diff walk's concern.
- **A nested `Map<nodeKind, Map<id, node>>`.** Rejected — regrouping by
kind changes iteration order and would break emitted-issue
byte-identity; the single combined-key map preserves insertion order
exactly.
- **Keep `message` populated with the path string.** Rejected — it
duplicates `path` as a pre-rendered string and invites producers (as
Mongo did) to bake presentation into data the renderer should own.
## Checklist
- [x] All commits DCO-signed (`Signed-off-by`)
- [x] Tests written/updated for the behavior changes
- [x] Title follows `TML-XXXX: <sentence case>` convention
- [x] Docs/ADR updated where behavior is described
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved schema-diff matching to avoid conflating items that share an
ID but differ by node type.
* Schema verification and CLI/database-check output now consistently
identify issues using their structured paths (with reason), including in
warnings and formatted error output.
* Migration/rebuild summaries now use deterministic, path-based
locations for conflicts.
* **Refactor**
* Schema-diff issues no longer rely on a preformatted message; they use
structured fields such as path, reason, expected, and actual.
* Mongo schema IR nodes now expose `nodeKind` instead of `kind` for type
discrimination.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>1 parent 813bc5a commit 9ccebe0
41 files changed
Lines changed: 306 additions & 153 deletions
File tree
- packages
- 1-framework
- 1-core/framework-components
- src/control
- test
- 3-tooling
- cli
- src
- commands
- utils/formatters
- test
- control-api
- utils
- migration/test/aggregate
- 2-mongo-family
- 3-tooling/mongo-schema-ir
- src
- test
- 9-family
- src/core
- test
- 2-sql/9-family/test
- 3-mongo-target/1-mongo-target/test
- 3-targets
- 3-targets
- postgres
- src/core/migrations
- test/migrations
- sqlite
- src/core/migrations
- operations
- test/migrations
- 6-adapters/postgres/test/migrations
- projects/postgres-rls
- slices/differ-kind-keying
- specs
- test
- e2e/framework/test/sqlite/migrations
- integration/test
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 22 additions & 22 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | 8 | | |
10 | 9 | | |
11 | 10 | | |
| |||
15 | 14 | | |
16 | 15 | | |
17 | 16 | | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
22 | 27 | | |
23 | 28 | | |
24 | 29 | | |
| 30 | + | |
25 | 31 | | |
26 | 32 | | |
27 | 33 | | |
28 | 34 | | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
29 | 42 | | |
30 | | - | |
| 43 | + | |
31 | 44 | | |
32 | | - | |
| 45 | + | |
33 | 46 | | |
34 | 47 | | |
35 | 48 | | |
36 | 49 | | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | 50 | | |
49 | 51 | | |
50 | 52 | | |
51 | 53 | | |
52 | 54 | | |
53 | 55 | | |
54 | | - | |
55 | 56 | | |
56 | 57 | | |
57 | 58 | | |
| |||
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
67 | | - | |
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
102 | 101 | | |
103 | 102 | | |
104 | 103 | | |
| |||
108 | 107 | | |
109 | 108 | | |
110 | 109 | | |
111 | | - | |
| 110 | + | |
| 111 | + | |
112 | 112 | | |
113 | 113 | | |
114 | 114 | | |
| |||
Lines changed: 52 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| 23 | + | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| 27 | + | |
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
28 | 31 | | |
29 | | - | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
30 | 37 | | |
31 | 38 | | |
32 | 39 | | |
| |||
105 | 112 | | |
106 | 113 | | |
107 | 114 | | |
108 | | - | |
| 115 | + | |
109 | 116 | | |
110 | | - | |
111 | | - | |
| 117 | + | |
112 | 118 | | |
113 | 119 | | |
114 | 120 | | |
| |||
162 | 168 | | |
163 | 169 | | |
164 | 170 | | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
165 | 212 | | |
166 | 213 | | |
167 | 214 | | |
| |||
242 | 289 | | |
243 | 290 | | |
244 | 291 | | |
245 | | - | |
| 292 | + | |
246 | 293 | | |
247 | 294 | | |
248 | 295 | | |
| |||
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
438 | 438 | | |
439 | 439 | | |
440 | 440 | | |
441 | | - | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
442 | 444 | | |
443 | 445 | | |
444 | 446 | | |
| |||
Lines changed: 4 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | | - | |
54 | | - | |
55 | | - | |
| 53 | + | |
| 54 | + | |
56 | 55 | | |
57 | 56 | | |
58 | 57 | | |
59 | 58 | | |
60 | 59 | | |
| 60 | + | |
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
70 | | - | |
| 70 | + | |
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| |||
Lines changed: 5 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
Lines changed: 0 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
285 | 285 | | |
286 | 286 | | |
287 | 287 | | |
288 | | - | |
289 | 288 | | |
290 | 289 | | |
291 | 290 | | |
| |||
Lines changed: 22 additions & 13 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
98 | 95 | | |
99 | 96 | | |
100 | 97 | | |
101 | 98 | | |
102 | 99 | | |
103 | 100 | | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
| 101 | + | |
109 | 102 | | |
110 | 103 | | |
111 | 104 | | |
112 | 105 | | |
113 | 106 | | |
114 | 107 | | |
115 | 108 | | |
116 | | - | |
117 | | - | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
118 | 127 | | |
119 | 128 | | |
120 | 129 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
335 | 335 | | |
336 | 336 | | |
337 | 337 | | |
338 | | - | |
339 | 338 | | |
340 | 339 | | |
341 | 340 | | |
| |||
377 | 376 | | |
378 | 377 | | |
379 | 378 | | |
380 | | - | |
381 | 379 | | |
382 | 380 | | |
383 | 381 | | |
| |||
390 | 388 | | |
391 | 389 | | |
392 | 390 | | |
393 | | - | |
394 | | - | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
395 | 395 | | |
396 | 396 | | |
397 | 397 | | |
| |||
427 | 427 | | |
428 | 428 | | |
429 | 429 | | |
430 | | - | |
431 | 430 | | |
432 | 431 | | |
433 | 432 | | |
| |||
562 | 561 | | |
563 | 562 | | |
564 | 563 | | |
565 | | - | |
566 | 564 | | |
567 | 565 | | |
568 | 566 | | |
| |||
695 | 693 | | |
696 | 694 | | |
697 | 695 | | |
698 | | - | |
699 | 696 | | |
700 | 697 | | |
701 | 698 | | |
702 | 699 | | |
703 | | - | |
704 | 700 | | |
705 | 701 | | |
706 | 702 | | |
| |||
0 commit comments