fix(seed-loader): multi-value lookup natural keys, and count the references they used to lose (#3911, #3932) - #3926
Merged
Conversation
…3911) A `multiple: true` lookup / `user` field stores an ARRAY of ids, so its seed value is an array of natural keys (`authors: ['Alice', 'Bob']`). Reference resolution only ever accepted one string: the array tripped the "expected a natural-key string but got an object. Pass the target's name value as a plain string" guard — impossible advice for a field holding several references — and was then dropped from the record. The row landed with the whole association missing and only a warn in the log. Resolution is now per element and shared by both passes (`resolveReferenceItem`): in-load records first, then the database, then pass 2. The field lands as an array of target ids, and a lone string is accepted as one-element shorthand for the array shape the field stores. Deferral stays all-or-nothing per field — a partially-written array is a corrupt association, so pass 2 re-resolves the whole authored array — and a key that never materializes is a reported load error naming that element, not a silent drop. An array on a genuinely single-value reference field is still rejected, now with advice an author can act on: declare the field `multiple: true`, or pass one natural key. `ReferenceResolution` gains an optional `multiple` flag carrying the field's array-ness into resolution (additive, defaulted-absent). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 117 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…multiple Generated file — `check:docs` flagged it out of date after the new optional `multiple` property landed on ReferenceResolutionSchema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
github-actions
Bot
added
documentation
…3911) A `multiple: true` lookup lands in a JSON column, so the resolved id ARRAY takes a serialize → SQLite TEXT → parse round-trip the in-memory mock never performs — and that round-trip is exactly what the replay comparison reads back on the next boot. The unit suite proves the resolution; only the real driver can prove the stored shape, and that a seeded association is stable across restarts instead of being rewritten or duplicated. Three cases on better-sqlite3 + the real ObjectQL engine: the array round-trips as an array of ids, a second load over the same database skips all three rows (no `updated_at` churn, no duplicate book), and natural keys resolve against authors that already existed in the database. All three fail against pre-fix `metadata-protocol` (verified by rebuilding its dist from the reverted source — a source-only revert proves nothing here, the runtime tests resolve that package from `dist`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
`field_zoo` covered every field type with a real seeded value EXCEPT a multi-value reference — the one shape #3911 was about. Adds `f_lookups: Field.lookup('showcase_account', { multiple: true })`, seeded from an array of natural keys (`['Northwind', 'Contoso']`) so each element resolves independently to an account id. That makes the specimen complete and doubles as a boot-time regression guard, which is what field_zoo exists for: before the fix the array was rejected as a non-string reference and the field vanished from the row, so the gallery shipped silently missing one relational type. The multi-value USER field (`f_users`) stays unseeded, now with the reason recorded on the object: `sys_user` rows come from SIGN-UP, not seeds (see security/seed-approval-demo.ts), so on a fresh boot no human user exists for a natural key to resolve against. Authoring one would make the whole showcase seed load report success: false on every first boot. The multi-value reference mechanics the two fields share are covered by `f_lookups`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
…3932) The loader had two failure outcomes and counted one. A record it cannot write lands in `errored`. But an unusable reference VALUE (an object where a natural key belongs, an array on a single-value field) is removed from the record — never written as NULL, which would sever an existing link on upsert replay — and the row is written without it. Nothing counted that. So a load that quietly severed N associations reported `totalErrored: 0` and every count-driven surface read clean. The boot banner — the one seed signal that survives `os dev`'s boot-quiet window and the default warn level — printed `showcase 42 rows`, and the warn line said "0 dropped record(s)": true, and useless. That reporting gap is why #3911 was reported as silent. Adds `SeedLoadResult.referencesDropped` + `SeedLoaderSummary .totalReferencesDropped`, deliberately NOT folded into `errored`: the row WAS written, so that would break the `inserted + updated + skipped` reconciliation against `total`. Threaded through both seed-summary producers (AppPlugin's inline seed, the marketplace heal) to the banner, which now names it: ⚠ Seeds: showcase 42 ok / 3 lost links ⚠ and to the app-plugin warn line, which no longer reports "0 dropped record(s)" over a load that lost associations. Both counters are additive with a 0 default, so existing producers and consumers of `SeedLoaderResult` are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
os-zhuang
changed the title
CI caught the other half of #3911: the RUNTIME resolved a natural-key array fine, but `defineSeed`'s record typing still declared every lookup as `string | null`, so authoring the array the loader now wants was a compile error (`Type 'string[]' is not assignable to type 'string'`) — the showcase seed added in the previous commit is exactly that case. `SeedFieldValue` widens a `multiple: true` lookup to `string | string[] | null`. A lone string stays legal there (the loader accepts it as one-element shorthand for the array shape the field stores); `master_detail` is inherently single and is not widened; an array on a single-value lookup and a wrapper object inside a multi array both remain compile errors — the guard that comment promises is intact, verified in both directions. Reaching that required `Field.lookup` to stop widening its config: with `config: FieldInput` the literal `multiple: true` collapses to `boolean` before it ever reaches the seed type. It is now generic over a `const` type parameter, with the return type intersected with `FieldInput` so the optional surface is unchanged — narrowing to just the passed keys broke consumers that read `.options` off a field union (app-todo). Type-level only; the returned object is identical at runtime. Repo-wide `pnpm -r typecheck` is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
The f_lookups field added to the field zoo declared an English label with no zh-CN counterpart, tripping the i18n coverage ratchet (456 -> 457 untranslated declared strings). Translate it so the new string does not widen the baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3911. Fixes #3932.
Three pieces: the resolution bug, a showcase example, and the reporting gap that made the bug feel silent.
1. Multi-value lookups drop their natural-key array (#3911)
A
multiple: truelookup /userfield stores an array of ids, so its seed value is an array of natural keys (authors: ['Alice', 'Bob']). Reference resolution only ever accepted a single string:An array is an object in JS, so the value tripped that guard, emitted impossible-to-act-on advice ("Pass the target's
namevalue as a plain string" — for a field that holds several references), and was dropped from the record. The row landed with the whole association missing. The loader had no notion that a reference field could be multi-value:buildDependencyGraphnever carriedmultipleintoReferenceResolution.Fix
ReferenceResolutiongains an optionalmultipleflag, populated inbuildDependencyGraphfrom the spec's sharedisMultiValueField.lookup/userwithmultiple: trueare recognized;master_detail(inherently single) correctly is not. Additive and defaulted-absent.resolveReferenceItemresolves one reference value — records seeded this load → DB probe → miss — and both passes share it, so an element deferred to pass 2 resolves by exactly the same rules. An array is just N calls to it.2. Showcase demonstrates it (#3911)
field_zoocovered every field type with a real seeded value except a multi-value reference — the one shape this bug was about. Addsf_lookups: Field.lookup('showcase_account', { multiple: true }), seeded from['Northwind', 'Contoso']. That completes the specimen and doubles as a boot-time regression guard, which is whatfield_zooexists for.The multi-value user field (
f_users) stays unseeded, with the reason now recorded on the object:sys_userrows come from sign-up, not seeds (seesecurity/seed-approval-demo.ts— "users can't be seeded (they sign up)"), so on a fresh boot no human user exists for a natural key to resolve against. Authoring one would make the whole showcase seed load reportsuccess: falseon every first boot. The multi-value reference mechanics the two fields share are covered byf_lookups.3. A dropped reference field was invisible in every count (#3932)
The loader had two failure outcomes and counted one. A record it cannot write lands in
errored. But an unusable reference value is removed from the record — never written as NULL, which would sever an existing link on upsert replay — and the row is written without it. Nothing counted that.So a load that quietly severed N associations reported
totalErrored: 0, and every count-driven surface read clean. The CLI boot banner — the one seed signal that survivesos dev's boot-quiet window and the defaultwarnlevel — printedshowcase 42 rows, and the warn line said0 dropped record(s): true, and useless. That reporting gap is why #3911 was reported as silent.Adds
SeedLoadResult.referencesDropped+SeedLoaderSummary.totalReferencesDropped, deliberately not folded intoerrored(the row was written, so that would break theinserted + updated + skippedreconciliation againsttotal). Threaded through both seed-summary producers — AppPlugin's inline seed and the marketplace heal — to the banner:Scoped out deliberately: a pass-2 deferred reference that never resolves already bumps
erroredand reports loudly, so it is not invisible. Re-classifying it would change existing behavior and the #2805 regression test for no gain here.Verification
seed-loader-multi-value-reference.test.ts(new, 14 cases) — the issue's repro, DB-side resolution, internal-id passthrough mixed with natural keys, pass-2 back-fill via a circular graph, all-or-nothing deferral, the no-multiPassdrop, single-value rejection, wrapper objects, replay idempotence, the graph flag, and the three seed-loader: a dropped reference FIELD is invisible in the summary —totalErroredcounts dropped records only, so the boot banner reads clean #3932 counter cases (including thatinserted + updated + skipped + errored == totalstill reconciles).seed-multi-value-lookup-real-driver.integration.test.ts(new, 3 cases) — better-sqlite3 + the real ObjectQL engine. Amultiple: truelookup lands in a JSON column, so the id array takes a serialize → TEXT → parse round-trip the in-memory mock never performs, and that round-trip is what the replay comparison reads back on the next boot. Proves the stored shape, that a second load skips all rows (noupdated_atchurn, no duplicate), and resolution against pre-existing rows.format.seed-summary.test.ts(3 new cases) — the banner names lost links even when every row landed, is singular-aware, and stays quiet at zero.Both new suites were confirmed to fail against pre-fix code. For the real-driver one that required rebuilding
metadata-protocol'sdistfrom the reverted source — the runtime tests resolve that package fromdist, so a source-only revert proves nothing.Also green: full
spec(6823),metadata-protocol(84),runtime(754),cli(764),cloud-connection(64), andexample-showcase(58) suites.Docs
content/docs/data-modeling/seed-data.mdxgains a "Multi-Value Lookups (multiple: true)" section covering the array shape, the one-element shorthand, and the all-or-nothing deferral. Generated spec references regenerated. Two changesets included.🤖 Generated with Claude Code
https://claude.ai/code/session_01Gusro3gdGv4wgbBnaFy9ah