Skip to content

Commit 605afbe

Browse files
committed
docs(migration-graph-rendering): record edges-on-plan and empty-origin-as-null follow-up slices
Signed-off-by: Will Madden <madden@prisma.io>
1 parent d5e442b commit 605afbe

3 files changed

Lines changed: 146 additions & 0 deletions

File tree

projects/migration-graph-rendering/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,20 @@ Presentation polish (independent of the sequence above):
6363
touches the tree renderer + the `graph` command, behind unchanged layout and
6464
`--json`/`--dot`.
6565

66+
Ledger cleanups (follow-ups from the TML-2769 / PR #665 review; sequenced after the ledger foundation):
67+
68+
- **Consolidate the per-edge breakdown onto the plan**
69+
[`slices/edges-on-plan/spec.md`](./slices/edges-on-plan/spec.md). The ledger
70+
foundation threads `migrationEdges` as a sibling of `plan` on the runner
71+
options, requiring a hand-maintained consistency guard. This slice moves the
72+
breakdown onto `MigrationPlan.edges` so the runner reads one object and the
73+
guard's reason to exist disappears.
74+
- **Stop spelling the empty-contract origin as a fake hash**
75+
[`slices/empty-origin-as-null/spec.md`](./slices/empty-origin-as-null/spec.md).
76+
∅ is modelled as `null` at the read boundary but as `sha256:empty` (not a real
77+
hash) in storage/graph, bridged by a coercion helper. This slice gives ∅ one
78+
honest representation (cut chosen with the graph-layer owner at pickup).
79+
6680
## Contents
6781

6882
- [`spec.md`](./spec.md)**slice 1's spec + dispatch plan.** Pins the
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
# Slice: consolidate the per-edge breakdown onto the migration plan
2+
3+
_Parent project `projects/migration-graph-rendering/`. Outcome this slice contributes to: the ledger-foundation slice (TML-2769) threads the per-edge breakdown to the runners as a **sibling field** (`migrationEdges`) next to `plan: MigrationPlan`. The edges are really the plan's own finer structure, so the two fields must be kept consistent by hand (a `Σ operationCount === plan.operations.length` guard exists only because they can desync). This slice moves the breakdown **onto the plan** so the runner reads `plan.edges`, the sibling field disappears, and the guard's reason to exist goes away._
4+
5+
## At a glance
6+
7+
Today — two parallel fields the runner must reconcile:
8+
9+
```ts
10+
runner.execute({
11+
plan: MigrationPlan, // aggregate shape: origin → destination + flat operations[]
12+
migrationEdges: [ // sibling: per-edge breakdown (dirName, hash, from, to, opCount)
13+
{ migrationHash, dirName, from, to, operationCount },
14+
],
15+
//
16+
});
17+
```
18+
19+
After — the breakdown is part of the plan:
20+
21+
```ts
22+
runner.execute({
23+
plan: {
24+
...MigrationPlan,
25+
edges: [{ migrationHash, dirName, from, to, operationCount }],
26+
},
27+
//
28+
});
29+
```
30+
31+
## Chosen design
32+
33+
`MigrationPlan` carries only the aggregate shape — one `origin``destination` and a **flat** `operations[]`. The per-edge breakdown (per-edge `dirName`, `migrationHash`, intermediate `from`/`to`, per-edge `operationCount`) is what the ledger journal needs (one row per applied migration), and it is **not** a pure duplicate of the plan — only the endpoints (first edge's `from` = `plan.origin`, last edge's `to` = `plan.destination`) and the op-count total overlap. But threading it as a **sibling** of `plan` on the runner options is the smell: `PerSpacePlan` already carries `migrationEdges` (the planner builds it alongside the plan), and the producer copies it onto the runner options next to `plan`, so the runner receives two fields describing the same apply and must guard against their desync.
34+
35+
This slice:
36+
37+
- **Adds `readonly edges` to `MigrationPlan`** (in `framework-components`). The element type stays a **structural inline** object (`{ migrationHash; dirName; from; to; operationCount }`) for the same layering reason the sibling field is inline today: `framework-components` (layer 1-core) cannot import `AggregateMigrationEdgeRef` from `migration-tools` (layer 3-tooling).
38+
- **Runners read `plan.edges`** instead of `options.migrationEdges` (mongo, postgres, sqlite).
39+
- **Drops the sibling `migrationEdges`** from `MigrationRunnerPerSpaceOptions`, `MongoMigrationRunnerExecuteOptions`, and the SQL family's per-space option shape.
40+
- **Producer stamps `edges` onto the plan it already builds.** The planner already holds `PerSpacePlan.migrationEdges`; it sets `plan.edges` from the same source rather than emitting a separate field. `apply.ts` no longer copies a sibling.
41+
- **Retires the `Σ operationCount === plan.operations.length` desync guard** — once `edges` and `operations` are constructed together on one object, they can't drift independently. (Optionally keep it as a cheap internal `assert` in the runner; settle at pickup.)
42+
43+
## Scope
44+
45+
**In:**
46+
47+
- `MigrationPlan.edges` (structural inline, framework-components).
48+
- Runners read `plan.edges`; sibling `migrationEdges` removed from all runner-option shapes.
49+
- Producer (`apply.ts` / planner) stamps `edges` onto the plan.
50+
- Migrate every construction site — the package runner tests (`synthEdges(plan)` helpers) and the five example-app manual/chain tests — from sibling `migrationEdges` to `plan.edges`.
51+
52+
**Out:**
53+
54+
- The per-edge data itself (unchanged — same five fields).
55+
- `PerSpacePlan.migrationEdges` naming on the planner side (internal; can stay or be renamed `edges` for symmetry — decide at pickup).
56+
- The ∅-origin spelling (`from: ''` / `sha256:empty` / `null`) — see sibling slice `empty-origin-as-null`.
57+
58+
## Open Questions
59+
60+
1. **Keep the op-count guard as an internal assertion?** Once edges live on the plan the desync path is gone, but a cheap `assert(Σ edges.operationCount === operations.length)` in the runner still catches a malformed producer. Decide whether the assertion earns its keep.
61+
2. **`edges` required vs optional on `MigrationPlan`.** The sibling field is currently required (synth/at-head plans carry a single synthesised edge). Keep it required for the same reason, or model at-head as an empty array — settle at pickup.
62+
3. **Rename `PerSpacePlan.migrationEdges``edges`?** Cosmetic symmetry with the new plan field; optional.
63+
64+
## References
65+
66+
- Parent project: `projects/migration-graph-rendering/spec.md`.
67+
- Predecessor: `slices/ledger-foundation/spec.md` (TML-2769) introduced the sibling `migrationEdges`; this slice consolidates it.
68+
- Surfaced by the TML-2769 / PR #665 review (the "single structure of migration runners" thread). The blast radius of making the sibling field required — five example-app call sites plus every runner-option test — is itself evidence that the breakdown belongs on the plan.
69+
- Linear issue: _to be filed at pickup (standalone, related to TML-2769 / TML-2774)._
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Slice: stop spelling the empty-contract origin as a fake hash
2+
3+
_Parent project `projects/migration-graph-rendering/`. Outcome this slice contributes to: the "no origin" state (the very first migration, from ∅) is modelled inconsistently. The read boundary models it honestly as `null` (`LedgerEntryRecord.from: string | null`), but the storage/graph layer spells it `sha256:empty` — a string that is **not a valid sha256 hash** masquerading as one. A coercion helper (`ledgerOriginFromStored`) exists only to bridge the two. This slice removes the typology lie so ∅ has one honest representation._
4+
5+
## At a glance
6+
7+
The same "no origin" fact, spelled three ways:
8+
9+
```ts
10+
// read boundary — honest: null means "no origin"
11+
interface LedgerEntryRecord {
12+
readonly from: string | null; // ∅ ⇒ null
13+
//
14+
}
15+
16+
// storage / graph — a fake hash used as an in-band node key
17+
const EMPTY_CONTRACT_HASH = 'sha256:empty'; // not a real sha256
18+
19+
// the bridge that only exists because of the divergence
20+
ledgerOriginFromStored(stored); // '' | 'sha256:empty' | null → null
21+
```
22+
23+
The question this slice answers: _if `from` permits `null`, why does `sha256:empty` exist at all?_
24+
25+
## Chosen design
26+
27+
Deferred — the **shape** of the fix is settled, the exact cut is chosen at pickup because the blast radius reaches the graph layer. Two candidate cuts:
28+
29+
1. **Model ∅ as `null` end-to-end.** Graph nodes keyed by `string | null`, edge `from` nullable, no sentinel anywhere. Honest, but the largest blast radius: `migration-graph` (the node map keys nodes by string hash), `graph-walk`, `check-integrity`, and the renderer all assume a non-null string key today.
30+
2. **Keep a sentinel but drop the misleading `sha256:` prefix** (e.g. a bare `` / `empty` token). Smaller than (1), but still touches every site that compares against or emits the constant, and a sentinel string is still a weaker model than `null`.
31+
32+
The operator has ruled that the **constant's value is owned by the graph/storage layer** ("not our fight" — TML-2769 review): this slice is about the **typology honesty at the boundary**, not about unilaterally reformatting a value the graph layer depends on. So the realistic scope is: pick the cut with the graph layer's owner, then land it.
33+
34+
Cheap immediate win, independent of the cut (can land first):
35+
36+
- **One-line doc note on `LedgerEntryRecord.from`** explaining `null` = ∅, and that the storage/graph spelling (`sha256:empty`) is normalised to `null` on read by `ledgerOriginFromStored`. Stops the next reader asking the same question.
37+
38+
Already done in TML-2769 / PR #665 (not re-litigated here): the constant was **deduplicated** — Mongo no longer redefines its own `EMPTY_ORIGIN_HASH`; it imports the shared one.
39+
40+
## Scope
41+
42+
**In:**
43+
44+
- The doc note on `LedgerEntryRecord.from` (immediate).
45+
- Whichever ∅-spelling cut is agreed with the graph-layer owner: either ∅ = `null` end-to-end, or a de-prefixed sentinel.
46+
- Collapse `ledgerOriginFromStored` accordingly (it disappears entirely under cut 1; it simplifies under cut 2).
47+
48+
**Out:**
49+
50+
- The ledger journal structure (TML-2769) and the per-edge breakdown (`edges-on-plan` slice) — orthogonal.
51+
- Unilaterally changing the constant's value without the graph layer's owner — explicitly not in scope.
52+
53+
## Open Questions
54+
55+
1. **Which cut — `null` end-to-end or a de-prefixed sentinel?** Settle with whoever owns `migration-graph`'s node-keying. (1) is the honest model; (2) is the smaller change. The decision turns on how much the graph node map and renderer rely on a non-null string key.
56+
2. **Does the graph node map tolerate a `null` key?** If yes, cut (1) is much cheaper than it looks. Investigate at pickup.
57+
58+
## References
59+
60+
- Parent project: `projects/migration-graph-rendering/spec.md`.
61+
- Predecessor: `slices/ledger-foundation/spec.md` (TML-2769) — introduced `LedgerEntryRecord.from: string | null` and the `ledgerOriginFromStored` bridge; deduped the constant.
62+
- Surfaced by the TML-2769 / PR #665 review (the `sha256:empty`-is-not-a-hash comment and the `from`-permits-null-but-we-use-the-constant comment).
63+
- Linear issue: _to be filed at pickup (standalone, related to TML-2769 / TML-2774)._

0 commit comments

Comments
 (0)