Skip to content

Commit 5ab0926

Browse files
committed
docs(sql-orm-many-to-many): slice 0 dispatch briefs, trace, learnings (TML-2784)
Orchestrator artifacts for the slice-0 build loop (3 dispatches, D3 in 2 rounds). Review log under reviews/ is gitignored by repo convention. Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
1 parent 46c607b commit 5ab0926

6 files changed

Lines changed: 184 additions & 0 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Learnings — sql-orm-many-to-many
2+
3+
Working ledger of patterns surfaced during this run. Reviewed at close-out; cross-cutting lessons migrate to durable docs.
4+
5+
## Harness lacks subagent resume
6+
7+
This harness exposes no `SendMessage`/resume for spawned subagents — the `Agent` tool always spawns fresh. The `drive-build-workflow` § Subagent continuity default (one persistent implementer + reviewer resumed across dispatches) degrades to its documented fallback: **fresh subagent per dispatch/round with a full-context brief**, with the AC scoreboard + findings carried on-disk via `code-review.md` rather than transcript. Acceptable here because dispatches touch disjoint surfaces (D1 contract / D2 sql-orm-client rename / D3 resolver) and prior work is committed, so the "re-does committed work" failure mode doesn't bite. Worth surfacing upstream: the continuity rule should name on-disk-artifact carry as the first-class fallback, not just "long-lived chat."
8+
9+
## Pre-existing `fixtures:check` env failure
10+
11+
`pnpm fixtures:check` fails at `fixtures:emit` in this sandbox (CLI not on PATH / "Failed to load config" for sql-builder + sql-orm-client emit scripts) — pre-existing, not introduced (matches the TML-2729 gotcha). Additivity is verified instead via a direct golden git-diff (`git diff -- ':(glob)**/contract.json' …`); CI runs the real gate. Don't treat the local `fixtures:check` red as a dispatch failure.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Brief: D1 — contract shape validates M:N
2+
3+
## Task
4+
5+
Extend the SQL contract relation shape so a many-to-many relation is **validatable** and round-trips. In the three places that define the relation shape, add `'N:M'` to the relation cardinality enum and an optional `through` object `{ table, parentColumns[], childColumns[], targetColumns[] }`: the arktype validator (`packages/2-sql/1-core/contract/src/validators.ts`, `ContractReferenceRelationSchema` — it currently rejects both `'N:M'` and the undeclared `through` key via `'+': 'reject'`), the JSON schema (`packages/2-sql/2-authoring/contract-ts/schemas/data-contract-sql-v1.json`, `ModelRelation`), and the `ContractReferenceRelation` TS type (in `@prisma-next/sql-contract` types — grep for the type the `as ContractRelation['cardinality']` cast in `build-contract.ts` references). Then update `build-contract.ts`: delete that cast (now unnecessary), rename the emitted `parentCols/childCols` → `parentColumns/childColumns` (match lowering), and populate `targetColumns` from the target model's anchor (PK the junction's child FK references). **Write the test first**: author an M:N relation via `rel.manyToMany('Tag', { through: 'UserTag', from: 'userId', to: 'tagId' })`, emit the contract, assert `validateContract` passes (it fails on `main` today).
6+
7+
## Scope
8+
9+
**In:** `validators.ts` (`ContractReferenceRelationSchema`: cardinality enum + optional `through` object, give `through` its own `'+': 'reject'`, array columns); `data-contract-sql-v1.json` `ModelRelation`; the `ContractReferenceRelation` TS type; `build-contract.ts` (cast deletion + `parentCols/childCols``parentColumns/childColumns` + `targetColumns`); the new round-trip test.
10+
11+
**Out:** anything in `packages/3-extensions/sql-orm-client/` (the `'M:N'``'N:M'` rename is D2, the resolver is D3); any include/filter/write runtime; `IncludeExpr`. Do not touch these even if adjacent.
12+
13+
## Completed when
14+
15+
- [ ] A new round-trip test authors an M:N relation, emits, and `validateContract` **passes** (fails on `main`).
16+
- [ ] `through` emits as `{ table, parentColumns, childColumns, targetColumns }` — no `parentCols`/`childCols` remain (`rg "parentCols|childCols" packages/2-sql` empty).
17+
- [ ] The `as ContractRelation['cardinality']` cast in `build-contract.ts` is removed (no bare cast reintroduced — use the now-correct type).
18+
- [ ] Gate: `pnpm --filter @prisma-next/sql-contract build` then downstream `pnpm typecheck` green (the relation type is consumed elsewhere — the optional `through` must not break consumers).
19+
- [ ] Gate: `pnpm fixtures:check` green. The change is **additive** (existing non-M:N relations emit byte-identically), so expect **no** golden drift; if any fixture changes, investigate and surface before committing — non-additive drift is a halt condition.
20+
21+
## Standing instruction
22+
23+
Stay focused on the goal; control scope. Trivial-and-related fixes that obviously serve the goal go in the same dispatch with a one-line note in your wrap-up. Anything that pulls you off the goal — even if useful — halts and surfaces.
24+
25+
## References
26+
27+
- Slice spec: `projects/sql-orm-many-to-many/slices/00-contract-resolver-foundation/spec.md` — chosen design (the `through`/`ResolvedRelation` shapes), pre-investigated edge cases.
28+
- Slice plan: same dir `plan.md` § Dispatch 1.
29+
- Project spec: `projects/sql-orm-many-to-many/spec.md` § Contract-impact.
30+
- Pre-investigated edge cases that apply here: arktype `'+': 'reject'` (must declare `through` explicitly + give it its own reject policy); `parentCols/childCols` vs `parentColumns/childColumns` drift; composite-key junctions (columns are arrays, never scalar); implicit target PK (derive `targetColumns`); fixtures additivity.
31+
32+
## Operational metadata
33+
34+
- **Model tier:** mid — contract substrate change with one design judgment (the `through` schema shape), bounded surface.
35+
- **Branch:** `tml-2784-slice-0-contract-resolver-foundation`. Commit with explicit staging + `-s` sign-off. **Do not push** (the orchestrator pushes at slice DoD).
36+
- **Time-box:** ~90 min wall-clock. Overrun → halt and surface.
37+
- **Halt conditions:** an out-of-scope surface (esp. anything under `sql-orm-client/`) needs touching to complete the task; `fixtures:check` shows non-additive golden drift; the `ContractReferenceRelation` type change ripples to a consumer that needs migration (would mean the change isn't purely additive — surface it).
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Brief: D2 — sql-orm-client canonicalises on `'N:M'`
2+
3+
## Task
4+
5+
The SQL contract emits relation cardinality as `'N:M'` (and D1 just made that validatable). `sql-orm-client` is the lone holdout still spelling it `'M:N'`, which means a real emitted M:N relation parses to `undefined` (and the mutation guard silently never fires). Flip the four `'M:N'` sites in `packages/3-extensions/sql-orm-client/src/` to `'N:M'`:
6+
7+
1. `types.ts``RelationCardinalityTag = '1:1' | 'N:1' | '1:N' | 'M:N'``'N:M'`.
8+
2. `mutation-executor.ts` — the `partitionByOwnership` guard `cardinality === 'M:N'`.
9+
3. `collection-internal-types.ts` — the to-many type-level check `... extends '1:N' | 'M:N' ? ...`.
10+
4. `collection-contract.ts``parseRelationCardinality` accepting `'M:N'`.
11+
12+
Also move the existing M:N-nested-mutation **rejection** unit test (in `mutation-executor.test.ts`) off its hand-built `'M:N'` literal to `'N:M'` so it exercises the live guard branch. It **stays a rejection test** — do not flip it to a positive assertion (that's a later slice).
13+
14+
## Scope
15+
16+
**In:** the 4 `'M:N'` literal sites in `sql-orm-client/src/` + the rejection test's cardinality literal.
17+
18+
**Out:** any `through`/resolver logic (that's the next dispatch, D3); any read/filter/write behaviour change; flipping the rejection test to positive (a later slice). Do not touch the contract packages (D1, done) or mongo-orm.
19+
20+
## Completed when
21+
22+
- [ ] `rg "'M:N'" packages/3-extensions/sql-orm-client/src` returns empty (only `'N:M'` remains).
23+
- [ ] The M:N rejection unit test uses `'N:M'` and still passes as a rejection.
24+
- [ ] Gate: `pnpm --filter @prisma-next/sql-orm-client typecheck` + `pnpm --filter @prisma-next/sql-orm-client test` green.
25+
26+
## Standing instruction
27+
28+
Stay focused on the goal; control scope. This is a mechanical spelling canonicalisation — resist refactoring adjacent code. Anything that pulls you off the goal halts and surfaces.
29+
30+
## References
31+
32+
- Slice spec: `projects/sql-orm-many-to-many/slices/00-contract-resolver-foundation/spec.md` (the cardinality-split edge case).
33+
- Slice plan: same dir `plan.md` § Dispatch 2.
34+
- D1 landed `'N:M'` + `through` in the contract (commit `f962fd47d`) — the contract side already speaks `'N:M'`.
35+
36+
## Operational metadata
37+
38+
- **Model tier:** mid (sonnet) — mechanical rename + one test-literal move; small bounded surface.
39+
- **Branch:** `tml-2784-slice-0-contract-resolver-foundation`. Explicit staging + `-s` sign-off. **Do not push.**
40+
- **Time-box:** ~30 min.
41+
- **Halt conditions:** a 5th `'M:N'` site outside the four named (surface, don't silently expand); any site where flipping the spelling changes behaviour beyond the literal (would mean a real semantic dependency — surface it).
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Brief: D3 — resolver surfaces the `through` descriptor
2+
3+
## Task
4+
5+
Make the single shared ORM resolver surface a **complete** `through` descriptor for M:N relations, so the later slices (read / filter / write) consume one uniform primitive instead of each re-deriving the junction walk. All in `packages/3-extensions/sql-orm-client/src/collection-contract.ts`:
6+
7+
1. Extend the `ResolvedRelation` interface (~line 199) with an optional `through?: { readonly table: string; readonly parentColumns: readonly string[]; readonly childColumns: readonly string[]; readonly targetColumns: readonly string[]; readonly requiredPayloadColumns: readonly string[] }`.
8+
2. In `resolveModelRelations` (~line 249), read `through` from the contract relation — D1 (commit `f962fd47d`) emits it as `{ table, parentColumns, childColumns, targetColumns }` — and populate the resolved `through`.
9+
3. **Derive `requiredPayloadColumns`**: the junction model's storage columns that are (a) NOT NULL, (b) have no default, and (c) are not in `parentColumns ∪ childColumns`. This is what slice 3 uses to disable nested `.create` on required-payload junctions. Surface it as an **array of column names** (decided working position — not a boolean).
10+
11+
**Write the resolver unit test FIRST**, covering four cases: (a) a simple single-column-FK M:N`through` populated, `requiredPayloadColumns` empty; (b) a **composite-key** junction (multi-column FKs) — arrays carry all columns; (c) a junction with a **required non-FK payload column**`requiredPayloadColumns` contains it; (d) a junction whose only extra columns are nullable or defaulted — `requiredPayloadColumns` empty.
12+
13+
## Scope
14+
15+
**In:** `collection-contract.ts` (`ResolvedRelation.through` + `resolveModelRelations` population + `requiredPayloadColumns` derivation); the resolver unit test.
16+
17+
**Out:** `resolveIncludeRelation` / `buildJoinWhere` / `mutation-executor` *consumption* of `through` (those are slices 1/2/3 — do not wire them); any SQL emission; `IncludeExpr`. Do not touch the contract packages (D1, done).
18+
19+
## Completed when
20+
21+
- [ ] `ResolvedRelation.through` is populated for an M:N relation including `targetColumns` and `requiredPayloadColumns`.
22+
- [ ] The resolver unit test covers cases (a)–(d) above and passes.
23+
- [ ] Gate: `pnpm --filter @prisma-next/sql-orm-client typecheck` + `pnpm --filter @prisma-next/sql-orm-client test` green.
24+
25+
## Standing instruction
26+
27+
Stay focused on the goal; control scope. The judgment site here is the `requiredPayloadColumns` derivation — get it right against the real contract storage shape; don't wire any consumer.
28+
29+
## References
30+
31+
- Slice spec: `projects/sql-orm-many-to-many/slices/00-contract-resolver-foundation/spec.md` — the `ResolvedRelation` shape + § Open Questions working positions (array not boolean; carry `targetColumns` explicitly).
32+
- D1 commit `f962fd47d` — the contract `through` shape you read from.
33+
- Grounding: `collection-contract.ts:199` (`ResolvedRelation`), `:249` (`resolveModelRelations`). You'll need to find how the contract model storage exposes a column's **nullability and default-presence** (to compute `requiredPayloadColumns`) — grep the contract storage types (`ModelStorageField` / `ModelStorage` in `@prisma-next/contract` or `@prisma-next/sql-contract`).
34+
35+
## Operational metadata
36+
37+
- **Model tier:** mid (sonnet) — bounded surface, but real judgment in the `requiredPayloadColumns` derivation; take care.
38+
- **Branch:** `tml-2784-slice-0-contract-resolver-foundation`. Explicit staging + `-s` sign-off. **Do not push.**
39+
- **Time-box:** ~60 min.
40+
- **Halt conditions (surface, do not work around):** the contract relation doesn't carry `targetColumns`, OR the junction model storage doesn't expose column nullability / default-presence in a usable way — either means a slice-spec assumption is **false** (D1's emit or the contract shape needs adjustment); halt and surface as a falsified assumption rather than approximating `requiredPayloadColumns`. Also halt if completing the task requires touching an out-of-scope consumer.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Brief: D3 R2 — clear F1 (bare casts in `resolveThrough`)
2+
3+
## Task
4+
5+
R1 (commit `3a87c7c55`) landed the `through` descriptor correctly — AC-2 is PASS. One blocking finding remains:
6+
7+
**F1 (must-fix):** `resolveThrough` in `packages/3-extensions/sql-orm-client/src/collection-contract.ts` introduced **5 new bare `as` casts** in production (≈ lines 274–275 and 289–291): `parentColumns as string[]` and `childColumns as string[]` in the `Set` spread, and `parentColumns/childColumns/targetColumns as readonly string[]` in the return object. After the `Array.isArray()` guard the inferred type is `unknown[]`; the contract validator guarantees `string` elements, so these are declarative widenings. They would fail the `no-bare-cast` Biome plugin + the `pnpm lint:casts` ratchet (HEAD cast count > merge-base).
8+
9+
Replace all 5 with `castAs<readonly string[]>(…)` (import `castAs` from `@prisma-next/utils/casts`) — the declarative-widening helper is exactly the right tool since the value already satisfies the type. If a small element-type narrowing helper reads cleaner and removes the casts entirely, that's also acceptable — your call.
10+
11+
## Scope
12+
13+
**In:** the 5 cast sites in `resolveThrough` (and the `castAs` import). Nothing else — the logic, tests, and `through` shape are already accepted.
14+
15+
**Out:** any behaviour change; the tests (they pass); any other file.
16+
17+
## Completed when
18+
19+
- [ ] No bare `as` casts remain in `resolveThrough` (the test-file `as unknown as Contract<…>` is exempt and out of scope).
20+
- [ ] Gate: `pnpm --filter @prisma-next/sql-orm-client typecheck` + `test` still green.
21+
- [ ] Gate: `pnpm lint:casts` passes (the ratchet does not report an increase from this branch's casts).
22+
23+
## Standing instruction
24+
25+
Surgical fix — touch only the 5 cast sites + the import. Do not re-open the accepted logic.
26+
27+
## References
28+
29+
- Finding F1 in `projects/sql-orm-many-to-many/reviews/code-review.md § Findings log` — exact locations + recommended action.
30+
- `.agents/rules/no-bare-casts.mdc` — the rule + the `castAs`/`blindCast` decision tree.
31+
- R1 commit: `3a87c7c55`.
32+
33+
## Operational metadata
34+
35+
- **Model tier:** cheap/mid — surgical, well-specified fix.
36+
- **Branch:** `tml-2784-slice-0-contract-resolver-foundation`. New commit (do not amend `3a87c7c55`). Explicit staging + `-s` sign-off. **Do not push.**
37+
- **Time-box:** ~20 min.
38+
- **Halt conditions:** if `castAs<readonly string[]>` doesn't satisfy the type (e.g. the value is genuinely wider than `readonly string[]`), surface rather than reaching for `blindCast` or a wider cast.

0 commit comments

Comments
 (0)