fix(bindx-client): union nested relation selections across sibling creates (#70)

[matej21]

Refs #70 — delivers the issue's expected behaviour 1 robustly. Expected behaviour 2 is still not implemented; see "What remains" before closing the issue.

Problem

buildSelectionFromOps unioned scalar fields across sibling create/update ops but kept nested relations in a Map keyed by field name — so the last sibling's shape won:

if (typeof value === 'object') {
    nestedFields.set(key, value as Record<string, unknown>)   // last sibling wins
}

With sibling A's button.create = { label, modalTitle } and sibling B's button.create = { label, link: { create } }, the emitted node selection was button { id label link { … } } — no modalTitle. The response therefore could not be content-matched back to sibling A. Its nested creates kept their __temp_… IDs while still being committed as existing on the server, so the next edit went out as updateButton(by: {id: "__temp_…"}) and the API answered Expected type "UUID". Every subsequent save of the page failed the same way until a full reload.

Change

Nested payloads are accumulated per field name across every sibling and fed back through buildSelectionFromOps, so the union is recursive by construction — has-one and has-many, nested-in-nested included. The walker is rebuilt around one primitive (buildSelectionFromDataObjects).

Incidental improvements from the rewrite:

• the file is now cast-free (isRecord type guard replaces every as Record<string, unknown>);
• a latent Object.entries(null) crash is fixed — the old ('data' in update ? update['data'] : update) reached Object.entries with a present-but-null data;
• id is no longer emitted twice when the data already carries one.

Scope

Only the selection-union defect. The issue also proposes refusing to commit an entity whose temp ID could not be reconciled. That was implemented and then reverted, because tests/nestedHasManyCreate.test.ts documents an unreconciled create as an expected steady state under row-level ACL ("The other review (missing from response due to ACL) keeps its temp ID"). Leaving such an entity uncommitted would re-emit its create on the next save — silent duplicate rows. A client-side guard that refuses the mutation was also tried and reverted: it aborts the whole batch, and since ContemberAdapter has no persistTransaction the sequential fallback means one unreconciled create would block every unrelated edit in the app, with no recovery short of a reload. That is a separate decision and deserves its own issue.

Tests

• tests/unit/persistence/nestedCreateTempIdLeak.test.ts — the reporter's reproducer, cherry-picked unmodified. Fails on main, passes here.
• tests/unit/persistence/mutationSelectionUnion.test.ts — sibling union, recursive union through a nested has-many, and id dedup. Placed under tests/unit/ rather than tests/bindx-client/ because the CI test script only covers tests/unit, tests/react and tests/cases.

Verified: tests/unit/persistence + tests/nestedHasManyCreate.test.ts green (13 pass on the targeted set), full CI suite 1545 pass, tests/bindx-client 38 pass, typecheck clean.

Not verified

No live API was exercised — both new tests use mock adapters, and the reproducer derives its mock response from the function under test, so it proves "if the server echoes what we selected, matching works" but cannot see over-selection or server-applied defaults. A second-order effect worth knowing: newly selecting nested has-one creates makes isCreateDataMatchingNode recurse where it previously skipped, so a server-side transform of a nested create's scalar would now fail a match that previously passed vacuously.

The Browser Tests check is red for an unrelated known reason — CI installs agent-browser unpinned and the popover click behaviour changed in the 0.32.x line. The suite is 66/66 green locally on an older driver. Being fixed separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GiniFfaE4gb5EuQpQ3Ncee

---

Second commit: pair by elimination, not greedy first-fit

An independent review reproduced a regression in the first commit, and fixing it turned out to matter more than the union itself.

The problem. The node selection is also the content matcher's input. Selecting a nested relation that last-wins previously dropped makes isCreateDataMatchingNode recurse into a subtree it used to skip, so any scalar the server does not echo back byte-identically — an ordinary datetime normalisation is enough — failed the match for the entire parent create op, and the greedy loop discarded that sibling and everything nested under it.

The fix. extractNestedResultsFromNode's greedy first-fit loop becomes a pairing pass:

1. pair every op that has exactly one candidate row, remove that row, and loop — consuming a row often makes another op unique;
2. fall back to first-fit for ops that are still ambiguous (indistinguishable siblings, where every bijection is equally correct);
3. if exactly one op and one row remain unpaired, pair them.

Step 1 is what makes step 3 sound. Bare elimination on top of the greedy loop would have widened a pre-existing bug — a subset payload steals its sibling's row today and the sibling ends up unmapped, but with plain elimination the sibling would be mis-mapped onto the subset's row, cross-wiring two rows instead of leaking one. Pairing the precise payload first removes that precondition.

isCreateDataMatchingNode is untouched. Strict comparison is still the evidence; it is just no longer the sole arbiter. The principle: the matcher must not treat "cannot identify" as "discard".

Measured, same shape as the reported regression (server normalises a nested publishedAt):

	blockA	buttonA	linkA	blockB	buttonB
old builder (pre-#70)	server-2	server-3	null	server-4	server-5
union only	null	null	null	server-5	server-6
union + elimination	server-2	server-3	server-4	server-5	server-6

Better than the pre-#70 baseline: linkA was never resolvable before, because link was not in the selection at all.

Bonus, and it was not optional. This also repairs the pre-existing bug the review found independently: when one sibling's create payload is a strict subset of another's and the server returns the node array reordered, sibling A was mapped to sibling B's server row, so A's later edits wrote to B's row. The uniqueness pass is exactly what prevents that, so the repair falls out of the safety requirement rather than being a separate change.

What remains

• Elimination is capped at one op and one row. With two simultaneously unmatchable siblings the entities keep their temp IDs rather than being guessed at — a test pins this deliberately. So the issue's expected behaviour 2 ("an entity whose temp ID could not be resolved must never be usable as the target of an update") is still unimplemented for that shape, and nothing stops such an entity from later being updated by temp ID. Decide whether to close Nested create keeps its temp ID after a successful persist and is then updated by __temp_ id #70 on that basis.
• Matching is now O(n²) per round over the siblings in one hasMany rather than a single greedy pass. Not a hot path — n is the number of siblings created in one mutation — but unbenchmarked.
• Still all mock adapters: which values a real backend normalises (datetimes, decimals, enum casing, trimming) is untested. The tests assert the strategy copes, not a catalogue.

Verification

Branch verified in isolation (its own bun install, without the other in-flight units): typecheck clean, bun run test 1536 pass / 0 fail, tests/bindx-client 38 pass.

Four new tests in tests/unit/persistence/nestedCreateNormalisedScalar.test.ts, each checked against a reverted matcher to confirm it bites: the normalisation shape fails without the fix; the subset+reorder shape fails without it; the two-unmatchable and indistinguishable-siblings cases are boundary pins that pass either way.