Skip to content

Commit 647a8cf

Browse files
committed
docs(sql-orm-many-to-many): slice 1 dispatch briefs, trace, learnings (TML-2785)
Orchestrator artifacts for the read slice (fixture / read-path / integration dispatches; read-path took 3 rounds incl. a truncation recovery). Review log under reviews/ is gitignored. Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
1 parent 89b0bd6 commit 647a8cf

7 files changed

Lines changed: 230 additions & 0 deletions

File tree

projects/sql-orm-many-to-many/learnings.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ This harness exposes no `SendMessage`/resume for spawned subagents — the `Agen
99
## Pre-existing `fixtures:check` env failure
1010

1111
`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.
12+
13+
## PGlite/WASM JIT flakiness on broad integration runs
14+
15+
Running the whole sql-orm-client integration suite at once (`cd test/integration && pnpm test test/sql-orm-client/`) can crash with V8 `jit_page.has_value()` (WASM JIT) failures — **pre-existing PGlite/Node env flakiness**, reproduces on the parent branch, not introduced by M:N work. Targeted reruns (per-file, or the same suite again) pass cleanly. Verify integration blast radius with targeted per-file runs; don't trust a single broad-run red.
16+
17+
## Dispatch truncation recovery (no subagent resume)
18+
19+
A substantial dispatch can exhaust the implementer's budget mid-work and return a truncated report with **uncommitted WIP** (happened on the slice-1 read path). Recovery: inspect `git status`/`git diff`, then dispatch a fresh continuation implementer pointed at the WIP with a focused completion brief (it commits the WIP + completion as one commit). Keep dispatches tight and tell implementers to implement-then-test-then-gate rather than over-explore (over-exploration is what burned the budget).
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Brief: S1-D1 — integration fixture gains an M:N relation
2+
3+
## Task
4+
5+
The sql-orm-client integration fixture has no many-to-many relation, so M:N read tests have nothing to run against. Add a **User ↔ Tag** M:N relation through a **`UserTag`** junction model (`userId`, `tagId`, composite primary key, **no payload columns** — the canonical pure-junction case) to the integration fixture's **source schema**, then **re-emit** the generated `contract.json` + `contract.d.ts`. The `Tag` model already exists in the fixture (`id`, `name`); add the junction + the `rel.manyToMany` relation on `User` (and the reverse on `Tag` if the fixture convention declares both sides).
6+
7+
Find the fixture source: the generated artifacts live under `test/integration/test/sql-orm-client/fixtures/generated/` (and/or `packages/3-extensions/sql-orm-client/test/fixtures/generated/`); the emit is wired via a `package.json` script (grep for `emit` / `contract emit`). Modify the **source**, not the generated files by hand; re-run the emit.
8+
9+
## Scope
10+
11+
**In:** the fixture source schema (add `UserTag` junction + User↔Tag M:N); the re-emitted `contract.json` + `contract.d.ts`.
12+
13+
**Out:** any read/projection code (S1-D2); any test (S1-D3); other fixture relations. Do not hand-edit generated files except as the emitter produces them.
14+
15+
## Completed when
16+
17+
- [ ] The fixture source declares an M:N User↔Tag relation via a `UserTag` junction (composite PK `userId`,`tagId`); the relation emits with `cardinality: 'N:M'` and a populated `through { table, parentColumns, childColumns, targetColumns }`.
18+
- [ ] `contract.json` + `contract.d.ts` are re-emitted from source and committed; the emitted M:N relation **round-trips `validateContract`**.
19+
- [ ] Change is additive — existing fixture models/relations emit unchanged (verify by diffing the generated files: only the new junction + relation appear).
20+
21+
## Standing instruction
22+
23+
Stay focused: add exactly the pure-junction M:N relation and re-emit. No extra models, no payload columns (a payload-junction fixture is a later concern if a test needs it).
24+
25+
## References
26+
27+
- Slice spec: `projects/sql-orm-many-to-many/slices/01-correlated-read-through-junction/spec.md` (§ Open Questions fixes the User↔Tag shape).
28+
- Slice 0 added `rel.manyToMany` validation; commit `f962fd47d` shows the emitted `through` shape.
29+
30+
## Operational metadata
31+
32+
- **Model tier:** mid (sonnet) — schema authoring + mechanical re-emit.
33+
- **Branch:** `tml-2785-slice-1-correlated-read`. Explicit staging + `-s` sign-off. **Do not push.**
34+
- **Time-box:** ~45 min.
35+
- **Halt conditions:** the local `fixtures:emit` / `pnpm fixtures:check` fails on the pre-existing CLI-on-PATH / config env issue (known) — if so, emit via the most direct working path you can (e.g. the package's own emit script) and verify the generated `contract.json` by inspection + `validateContract`; note it. If re-emit is genuinely impossible in-sandbox, surface to the orchestrator (do not hand-fabricate `contract.json`). Halt if adding the relation forces touching read/test code.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Brief: S1-D2 — read path correlates through the junction
2+
3+
## Task
4+
5+
Teach the correlated include read path to walk an M:N relation through its junction. When the resolved include relation carries `through` (surfaced by slice 0 on `ResolvedRelation`), `db.orm.User.include('tags')` must resolve to `tags: Tag[]` via a **single correlated subquery**: the child subquery selects from the **target** (`tags`) joined to the **junction** (`user_tags`) on `junction.childColumns = target.targetColumns`, correlated to the parent on `junction.parentColumns = parent`'s anchor key — i.e. the target rows whose PK appears in junction rows pointing at the current parent row. No `LATERAL`, no second query.
6+
7+
Concretely:
8+
1. `resolveIncludeRelation` (`collection-contract.ts`) surfaces the `through` descriptor onto its `ResolvedIncludeRelation` result.
9+
2. `IncludeExpr` (`types.ts`) gains an optional `through?` mirroring the descriptor.
10+
3. `buildCorrelatedIncludeProjection` (`query-plan-select.ts`) — today it correlates `child.targetColumn = parent.localColumn` for FK relations. Add the M:N branch: when `include.through` is present, build the child subquery against the target joined to the junction, with the parent correlation on the junction side. AND across all column pairs for composite keys.
11+
4. Whatever include-child **decode / graft** is needed (`collection-dispatch.ts` / the include decode path) to assemble the aggregated `tags: Tag[]` under the relation key — mirror the existing FK include child handling.
12+
13+
**Write unit tests first** (`query-plan-select.test.ts` or the nearest existing suite): assert the compiled AST for an M:N include is a single correlated subquery joining through the junction, with the composite-key correlation AND-ed, and **no `LATERAL`** node. Use a hand-built M:N contract (mirror slice 0's `buildManyToManyContract` resolver test, or the fixture).
14+
15+
## Scope
16+
17+
**In:** `resolveIncludeRelation` + `IncludeExpr.through` (`collection-contract.ts`, `types.ts`); the M:N branch in `buildCorrelatedIncludeProjection` + include-child decode (`query-plan-select.ts`, `collection-dispatch.ts`); unit tests for the compiled AST.
18+
19+
**Out:** integration tests (S1-D3 — those run against the fixture/DB); filter EXISTS (slice 2); nested write (slice 3); any `IncludeExpr` change beyond carrying `through`; the FK include path (don't regress it).
20+
21+
## Completed when
22+
23+
- [ ] An M:N `include` compiles to a **single correlated subquery** walking parent → junction → target (composite-key correlation AND-ed); unit test asserts the AST shape and the **absence of any `LATERAL`** node.
24+
- [ ] The FK (non-M:N) include path is unchanged (its existing unit tests still pass).
25+
- [ ] Gate: `pnpm --filter @prisma-next/sql-orm-client typecheck` + `pnpm --filter @prisma-next/sql-orm-client test` green.
26+
27+
## Standing instruction
28+
29+
Stay focused on the M:N read correlation. The judgment site is the junction-join in the correlated builder — get it right; mirror the FK path's decode/aggregation rather than inventing a parallel one.
30+
31+
## References
32+
33+
- Slice spec: `projects/sql-orm-many-to-many/slices/01-correlated-read-through-junction/spec.md` (the parent→junction→target correlation shape).
34+
- Slice 0: `ResolvedRelation.through` in `collection-contract.ts` (`{ table, parentColumns[], childColumns[], targetColumns[], requiredPayloadColumns[] }`).
35+
- The FK correlated path in `buildCorrelatedIncludeProjection` (`query-plan-select.ts`) is the pattern to extend — find the `child.targetColumn = parent.localColumn` correlation site.
36+
37+
## Operational metadata
38+
39+
- **Model tier:** mid→orchestrator (sonnet) — this is the slice's design-judgment dispatch (correlated junction subquery). Take care; the AST mechanics are the hard part.
40+
- **Branch:** `tml-2785-slice-1-correlated-read`. Explicit staging + `-s` sign-off. **Do not push.**
41+
- **Time-box:** ~90 min.
42+
- **Halt conditions (surface, don't work around):** the correlated builder cannot express the junction join without a **new AST primitive / a LATERAL** (would falsify the slice's "correlated-only through the junction" premise — surface to me); composite-key correlation needs data not present on `through`; surfacing `through` onto `IncludeExpr` forces touching an out-of-scope consumer.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Brief: S1-D2 R2 — finish the read-path correlation (continue from WIP)
2+
3+
## Situation
4+
5+
The first R1 implementer ran out of budget mid-dispatch and **did not commit**. There is **uncommitted WIP** in the working tree (run `git status` + `git diff` to see it):
6+
7+
- `src/types.ts``IncludeExpr` gained `through?` (likely done).
8+
- `src/collection-contract.ts``resolveIncludeRelation` surfacing `through` (likely done).
9+
- `src/collection.ts` — small change.
10+
- `test/helpers.ts` — a `buildManyToManyContract` helper (was mid-edit when cut off — verify it's coherent).
11+
- `test/query-plan-select.test.ts` — new M:N unit tests (written; the impl they assert is **missing**, so they currently fail).
12+
13+
**What's missing (the core task):** `query-plan-select.ts` (`buildCorrelatedIncludeProjection`) was **not touched** — the M:N junction-correlation branch is not implemented. The include-child decode (`collection-dispatch.ts`) may also be needed.
14+
15+
## Task
16+
17+
**First, read the uncommitted diff** to understand what R1 left. Then finish the dispatch:
18+
19+
1. Implement the M:N branch in `buildCorrelatedIncludeProjection` (`query-plan-select.ts`): when `include.through` is present, compile a **single correlated subquery** — target JOIN junction ON `junction.childColumns = target.targetColumns`, correlated to the parent on `junction.parentColumns = parent` anchor; AND across all column pairs for composite keys; **no `LATERAL`**.
20+
2. Wire whatever include-child decode/graft is needed (`collection-dispatch.ts`) to aggregate `tags: Tag[]` — mirror the FK include path, don't fork it.
21+
3. Reconcile the WIP test helper + unit tests so they pass and assert the AST shape (single correlated subquery through the junction, composite-key AND-ed, no `LATERAL`). If R1's test helper or tests are incoherent/incomplete, fix them.
22+
4. Don't regress the FK include path.
23+
24+
## Completed when
25+
26+
- [ ] An M:N `include` compiles to a single correlated subquery through the junction (composite-key AND-ed); unit test asserts the AST + absence of `LATERAL`, and **passes**.
27+
- [ ] FK include path unchanged (its tests pass).
28+
- [ ] `pnpm --filter @prisma-next/sql-orm-client typecheck` + `test` green.
29+
- [ ] Committed as **one coherent commit** (the WIP + your completion together), explicit staging + `-s` sign-off, **no push**.
30+
31+
## Standing instruction
32+
33+
Finish the goal: the M:N correlated read. Keep R1's coherent WIP; complete the missing builder/decode; make the suite green. No bare `as` casts (use `castAs`/`blindCast` if unavoidable).
34+
35+
## References
36+
37+
- R1 brief: `./02-read-path.md` (full task spec — the correlation shape).
38+
- Slice spec: `projects/sql-orm-many-to-many/slices/01-correlated-read-through-junction/spec.md`.
39+
- Slice 0 `ResolvedRelation.through` in `collection-contract.ts`.
40+
41+
## Operational metadata
42+
43+
- **Model tier:** sonnet.
44+
- **Branch:** `tml-2785-slice-1-correlated-read`. The WIP is already on it (uncommitted). Explicit staging + `-s` sign-off; **do not push**.
45+
- **Time-box:** ~75 min. To reduce truncation risk: implement the builder branch first, get the targeted unit test green, then run the package gate — don't over-explore.
46+
- **Halt + surface to me:** if the correlated builder cannot express the junction join without a new AST primitive / `LATERAL` (falsifies the correlated-only premise); if R1's WIP is in a state you can't reconcile coherently (describe it, don't force it).
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Brief: S1-D2 R3 — clear F1 + F2
2+
3+
The read-path commit `e587b433c` is structurally accepted (correct LATERAL-free M:N correlation; FK path intact). Two `should-fix` findings remain; both block dispatch close.
4+
5+
## F1 (should-fix) — bare casts
6+
7+
`packages/3-extensions/sql-orm-client/src/query-plan-select.ts`, in `buildManyToManyJunctionArtifacts`: two bare `as AnyExpression` casts — `(joinOnPairs[0] as AnyExpression)` and `(correlationPairs[0] as AnyExpression)`. Both are pure widenings (`BinaryExpr` is a member of `AnyExpression`). Replace with `castAs<AnyExpression>(…)` (import `castAs` from `@prisma-next/utils/casts`).
8+
9+
## F2 (should-fix) — missing test for M:N + distinct + non-leaf
10+
11+
`buildDistinctNonLeafChildRowsSelect` applies `junctionJoins` (lines ~582–587) but no test exercises **M:N + `.distinct(…)` + nested include** together. Add a unit test in `test/query-plan-select.test.ts`: build an M:N include whose `nested` state carries a `distinct` + a further (non-leaf) include, call `compileSelectWithIncludes`, and assert (a) the junction join attaches to the innermost `baseInner` SELECT and (b) the correlated WHERE is present at that level. Reuse/extend the existing `buildManyToManyContract` / `buildManyToManyIncludeExpr` helpers. The test must genuinely exercise the distinct-non-leaf branch (not the plain branch).
12+
13+
## Completed when
14+
15+
- [ ] No bare `as` casts in `buildManyToManyJunctionArtifacts` (only `castAs<…>`); `pnpm lint:casts` passes (no count increase from this branch).
16+
- [ ] New M:N + distinct + non-leaf unit test added and **passing**, asserting the junction join + correlation at the inner select.
17+
- [ ] `pnpm --filter @prisma-next/sql-orm-client typecheck` + `test` green.
18+
19+
## Standing instruction
20+
21+
Surgical: the two cast sites + one new test. Don't reshape the accepted builder logic.
22+
23+
## References
24+
25+
- Findings F1, F2 in `projects/sql-orm-many-to-many/reviews/code-review.md § Findings log` (exact locations + recommended actions).
26+
- `.agents/rules/no-bare-casts.mdc`.
27+
- R2 commit: `e587b433c`.
28+
29+
## Operational metadata
30+
31+
- **Model tier:** sonnet — surgical fix + one test.
32+
- **Branch:** `tml-2785-slice-1-correlated-read`. New commit (do not amend `e587b433c`). Explicit staging + `-s` sign-off. **Do not push.**
33+
- **Time-box:** ~40 min.
34+
- **Halt:** if the M:N + distinct + non-leaf path turns out to be **incorrect** when you write the test (the test fails because the junction join is mis-placed), that's a real bug in `e587b433c` — surface it to me with the evidence rather than papering over it (it would mean F2 is a `must-fix`, not just missing coverage).
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Brief: S1-D3 — M:N include integration tests (operator standard)
2+
3+
## Task
4+
5+
Prove the M:N include works end-to-end against the database, following the project's **integration-test standard**. The fixture (S1-D1) defines `User.tags` (→ `Tag` via `UserTag`); the read path (S1-D2) compiles the correlated junction subquery. Add integration tests under `test/integration/test/sql-orm-client/` (alongside `include.test.ts` / `nested-includes.test.ts`), using the existing harness (`withCollectionRuntime`, PGlite). Seed users, tags, and `user_tags` junction rows, then assert `db.orm.User.include('tags')` returns the expected shape.
6+
7+
**The standard (all three apply):**
8+
1. **Whole-row assertions**`toEqual` (or snapshot) on the complete returned rows; never cherry-pick individual fields.
9+
2. **Explicit `.select(...)` in most tests** — project the fields each test asserts (user-level and nested `tags`-level), so adding a field to `User`/`Tag` later doesn't churn these assertions. Assert the whole *selected* shape.
10+
3. **≥1 implicit/default-selection test** — at least one test does `include('tags')` with **no** `.select`, asserting the full default row shape (all `User` fields + `tags: Tag[]` with all `Tag` fields) comes back.
11+
12+
Plus:
13+
- **Single execution** — assert the M:N include runs in **one** SQL execution (the harness's query-count/exec hook if available; otherwise assert no `LATERAL` in the emitted SQL). Mirror how `include.test.ts` / `nested-includes-strategy.test.ts` verify execution count if they do.
14+
- **Depth-2** — a test that nests another include under the M:N read (or M:N nested under a 1:N), to prove the junction walk composes with deeper includes.
15+
- Edge: a user with **no** tags returns `tags: []`; a tag connected to multiple users still resolves correctly.
16+
17+
## Scope
18+
19+
**In:** new integration test file(s) under `test/integration/test/sql-orm-client/`; any seed/helper needed there.
20+
21+
**Out:** filter (slice 2); write (slice 3); production code changes (D2 owns the read path — if a test reveals a read **bug**, surface it, don't fix production here without flagging). Do not modify the fixture (D1 owns it).
22+
23+
## Completed when
24+
25+
- [ ] M:N `include('tags')` integration tests pass on PGlite: whole-row `toEqual`; **most** use explicit `.select`; **≥1** uses implicit/default selection; depth-2 covered; empty-tags and shared-tag cases covered.
26+
- [ ] A single-execution assertion (no `LATERAL`, one query) for the M:N include.
27+
- [ ] Gate: the new tests run green — `cd test/integration && pnpm test test/sql-orm-client/<your-file>` (this is how the sql-orm-client integration suite runs in-sandbox; the CLI-journey e2e tests are the ones with the known env limitation, not these).
28+
29+
## Standing instruction
30+
31+
Match the existing integration corpus's style (whole-row `toEqual`); add the explicit-select-dominant + implicit-select cases the standard requires. If a test surfaces a real read-path bug, **surface it to me** with the failing assertion — that would be a `must-fix` against D2, not something to patch here.
32+
33+
## References
34+
35+
- Existing corpus: `test/integration/test/sql-orm-client/include.test.ts`, `nested-includes.test.ts`, `nested-includes-strategy.test.ts` (assertion + execution-count patterns to mirror).
36+
- Slice spec: `projects/sql-orm-many-to-many/slices/01-correlated-read-through-junction/spec.md` (§ done conditions — the standard).
37+
- Fixture: `User.tags` M:N via `UserTag` (commit `fcecac5b3`).
38+
39+
## Operational metadata
40+
41+
- **Model tier:** sonnet.
42+
- **Branch:** `tml-2785-slice-1-correlated-read`. Explicit staging + `-s` sign-off. **Do not push.**
43+
- **Time-box:** ~75 min — write the core whole-row + implicit-select tests first, get them green, then add depth-2/edge cases; don't over-explore.
44+
- **Halt + surface to me:** if the sql-orm-client integration harness genuinely cannot run in-sandbox (PGlite spin-up failure unrelated to your tests), describe the failure — don't claim green without running. If `include('tags')` returns a wrong shape (read-path bug in D2), surface it.

0 commit comments

Comments
 (0)