From 1bf551bdc7bcfbb87b58106fa81cabf02735be00 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 12:36:03 +0000 Subject: [PATCH] fix(metadata-protocol): report the pass-2 back-fill dropped when the source record has no id (#5127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveDeferredUpdates()` looked the source record's internal id up in `insertedRecords` and, when it wasn't there, ran off the end of an `if` with no `else`. Pass 2 had already RESOLVED the target, and the back-fill then evaporated: no write, no `errors`/`allErrors` entry (so the load still reported `success: true`), no `errored`, not one log line. The only trace was the `referencesDeferred` the record booked in pass 1 and never gave back — only a SUCCESSFUL back-fill decrements it — i.e. a result object carrying a dangling number with nothing in it that explains the number, while the declared association stayed absent forever. It is the deeper cousin of the two branches on either side of it: #4729 fixed "counted, but logged at `warn`"; #4997 fixed "counted, never logged"; this one was "never counted, never logged". The loss now goes through `recordDeferredError` under the same objective criterion (does the outcome enter `errors`/`allErrors`?) and is logged once at `error` per AGENTS.md → "Degradation log levels" (#4632), with the consequence and the fix. `referencesDeferred` keeps its meaning — "deferred references that never landed", decremented only by a successful back-fill, exactly as the two sibling failure branches leave it — so what changes is that a leftover count now always has a matching entry in `errors`. The two ways to get here are worded differently because they are different failures. An EMPTY `recordExternalId` is the PURE silent loss: `externalIdKey` returns `''` when a component of a composite key is blank, the row itself wrote perfectly, and nothing else in the load reports anything — this line is the only signal that will ever exist, so it names which key came out empty (carried on `DeferredUpdate.externalIdLabel`, added for it). A real key simply absent from the map means the source row never landed, and that write failure was already reported at `error` by the pass-1 site, so the line points AT that error instead of restating it — one line, not a second flood over the same root cause. Regression tests cover both triggers, the counter/error pairing, and a control proving the failure comes from the EMPTY key rather than from composite keys. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 --- .changeset/plenty-poems-brush.md | 27 ++ .../src/seed-loader-deferred-dropped.test.ts | 392 ++++++++++++++++++ packages/metadata-protocol/src/seed-loader.ts | 120 +++++- 3 files changed, 536 insertions(+), 3 deletions(-) create mode 100644 .changeset/plenty-poems-brush.md create mode 100644 packages/metadata-protocol/src/seed-loader-deferred-dropped.test.ts diff --git a/.changeset/plenty-poems-brush.md b/.changeset/plenty-poems-brush.md new file mode 100644 index 0000000000..13526af857 --- /dev/null +++ b/.changeset/plenty-poems-brush.md @@ -0,0 +1,27 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +seed-loader: a pass-2 back-fill dropped for a missing source-record id is now reported, not silently discarded + +`resolveDeferredUpdates()` looked the source record's internal id up in `insertedRecords` +and, when it was not there, ran off the end of an `if` with no `else`. Pass 2 had already +RESOLVED the target, and the back-fill then evaporated: no write, no entry in +`errors`/`allErrors` (so the load still reported `success: true`), no `errored`, and not +one log line. The only trace was the `referencesDeferred` the record booked in pass 1 and +never gave back — a dangling number with nothing in the result explaining it, while the +declared association stayed absent forever. + +It now records the loss through `recordDeferredError` (→ `errors`/`allErrors` + `errored`, +so the load reports `success: false`) and logs it once at `error`, per the same objective +criterion applied in #4729/#4997 and the "Degradation log levels" rule. The two ways to +get here are worded differently because they are different failures: an EMPTY +`recordExternalId` — `externalIdKey` returns `''` when any component of a composite +externalId is blank — is the pure silent loss, where the row wrote perfectly, nothing else +in the load reports anything and the reference stays NULL forever; a real key that is +simply absent from the map means the source row never landed, and that write failure was +already reported at `error`, so this line points at it instead of restating it. + +A load that hits this path previously returned `success: true` with clean counters and now +returns `success: false` with the loss counted — the seed data was always incomplete; it +just was not saying so. diff --git a/packages/metadata-protocol/src/seed-loader-deferred-dropped.test.ts b/packages/metadata-protocol/src/seed-loader-deferred-dropped.test.ts new file mode 100644 index 0000000000..eab51637cf --- /dev/null +++ b/packages/metadata-protocol/src/seed-loader-deferred-dropped.test.ts @@ -0,0 +1,392 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; +import { SeedLoaderService } from './seed-loader'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; + +/** + * #5127 — pass 2 RESOLVES the target and then has no record to write it onto. + * + * `resolveDeferredUpdates()` looked the source record's internal id up in + * `insertedRecords` and, when it wasn't there, ran off the end of an `if` + * with no `else`: no write, no `errors`/`allErrors` entry (so `success` stayed + * `true`), no `errored`, not one log line. The single trace left behind was the + * `referencesDeferred` the record booked in pass 1 and never gave back — only a + * SUCCESSFUL back-fill decrements it — i.e. a result object carrying a dangling + * number with nothing in it that explains the number. + * + * It is the deeper cousin of the two branches on either side of it: #4729 fixed + * "counted, but logged at `warn`"; #4997 fixed "counted, never logged"; this one + * was "never counted, never logged". + * + * The two ways to reach it are NOT the same failure and are pinned separately: + * + * - PURE SILENT LOSS — the row wrote perfectly, but its natural key was never + * registered because `externalIdKey` returned `''` (any component of a + * composite externalId absent or blank). NOTHING else in the load reports + * anything: this is the only signal that will ever exist. Mandatory coverage. + * - SOURCE ROW NEVER LANDED — its pass-1 write failed, which the write site + * already reported at `error` (#4729). Still recorded here under the same + * objective criterion, but as exactly ONE line that points AT that error + * rather than a second flood over the same root cause. + */ + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +function createFaithfulEngine(): { engine: IDataEngine; store: Record } { + const store: Record = {}; + let idCounter = 0; + + const engine = { + find: vi.fn(async (objectName: string, query?: any) => { + let records = store[objectName] || []; + if (query?.where) { + records = records.filter((r) => + Object.entries(query.where).every(([k, v]) => r[k] === v), + ); + } + if (typeof query?.limit === 'number') records = records.slice(0, query.limit); + return records; + }), + findOne: vi.fn(async (objectName: string, query?: any) => { + const rows = await (engine.find as any)(objectName, { ...query, limit: 1 }); + return rows[0] ?? null; + }), + insert: vi.fn(async (objectName: string, data: any) => { + if (!store[objectName]) store[objectName] = []; + if (Array.isArray(data)) { + const records = data.map((d) => ({ id: `gen-${++idCounter}`, ...d })); + store[objectName].push(...records); + return records; + } + const record = { id: `gen-${++idCounter}`, ...data }; + store[objectName].push(record); + return record; + }), + update: vi.fn(async (objectName: string, data: any) => { + const records = store[objectName] || []; + const idx = records.findIndex((r) => r.id === data.id); + if (idx >= 0) { records[idx] = { ...records[idx], ...data }; return records[idx]; } + return data; + }), + delete: vi.fn(async () => ({ deleted: 1 })), + count: vi.fn(async (objectName: string) => (store[objectName] || []).length), + aggregate: vi.fn(async () => []), + } as unknown as IDataEngine; + + return { engine, store }; +} + +// Two objects that reference each other → a circular dependency, which is what +// forces the multi-pass deferral: `drop_department.head_id` cannot resolve until +// `drop_worker` "Alice" exists, so pass 1 defers it and pass 2 back-fills it. +// `drop_department` additionally carries a `region`, so a dataset can key it on +// the COMPOSITE ['name', 'region'] and leave one component blank. +function createMetadata(): IMetadataService { + const objects: Record = { + drop_department: { + name: 'drop_department', + fields: { + name: { type: 'text' }, + region: { type: 'text' }, + head_id: { type: 'lookup', reference: 'drop_worker' }, + }, + }, + drop_worker: { + name: 'drop_worker', + fields: { + name: { type: 'text' }, + department_id: { type: 'lookup', reference: 'drop_department' }, + }, + }, + }; + return { + getObject: vi.fn(async (name: string) => objects[name]), + listObjects: vi.fn(async () => Object.values(objects)), + register: vi.fn(async () => {}), + get: vi.fn(async (_t: string, name: string) => objects[name]), + list: vi.fn(async () => []), + unregister: vi.fn(async () => {}), + exists: vi.fn(async () => false), + listNames: vi.fn(async () => []), + } as unknown as IMetadataService; +} + +const CONFIG = { + dryRun: false, + haltOnError: false, + multiPass: true, + defaultMode: 'insert', + batchSize: 1000, + transaction: false, +} as any; + +/** Composite-keyed department seed; `region` decides whether its key registers. */ +const compositeSeeds = (region: string) => [ + { + object: 'drop_department', + externalId: ['name', 'region'], + mode: 'insert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'Engineering', region, head_id: 'Alice' }], + }, + { + object: 'drop_worker', + externalId: 'name', + mode: 'insert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'Alice', department_id: 'Engineering' }], + }, +] as any[]; + +const deferredDropLines = (logger: ReturnType) => + logger.error.mock.calls.filter((c: unknown[]) => String(c[0]).includes('Deferred reference DROPPED')); + +describe('pass 2 resolves the target but the source record has no id (#5127)', () => { + /** + * (c) THE PURE SILENT LOSS — the case that must be covered. + * + * The department row is written successfully and sits in the database. Its + * composite externalId ['name', 'region'] evaluates to `''` because `region` + * is blank, so `externalIdKey` returns the empty key and nothing registers it + * in `insertedRecords`. Pass 2 then resolves 'Alice' perfectly and finds no id + * to write onto. No other site in the loader says a word about this: before + * the fix the entire outcome was one un-explained `referencesDeferred: 1`. + */ + it('reports the back-fill it had to drop, with the row itself seeded fine', async () => { + const { engine, store } = createFaithfulEngine(); + const logger = createLogger(); + + const result = await new SeedLoaderService(engine, createMetadata(), logger).load({ + seeds: compositeSeeds(''), + config: CONFIG, + }); + + // The row IS there — this is not a write failure. That is the whole point: + // every row counter reads healthy. + const engineering = store.drop_department.find((r) => r.name === 'Engineering'); + expect(engineering, 'the department row was not seeded — wrong scenario').toBeDefined(); + const deptResult = result.results.find((r: { object: string }) => r.object === 'drop_department')!; + expect(deptResult.inserted).toBe(1); + expect(deptResult.errored).toBe(1); + + // …while the association it declared is permanently absent. + expect(engineering!.head_id == null).toBe(true); + // Nothing was even attempted against the department (no id to update). + expect( + (engine.update as any).mock.calls.some(([obj]: [string]) => obj === 'drop_department'), + 'a back-fill write was attempted without a record id', + ).toBe(false); + + // So the load must NOT report clean success — it used to. + expect(result.success).toBe(false); + expect(result.summary.totalErrored).toBe(1); + const dropped = result.errors.find((e: { field: string }) => e.field === 'head_id')!; + expect(dropped, 'the dropped back-fill was not recorded as an error').toBeDefined(); + expect(dropped.sourceObject).toBe('drop_department'); + expect(dropped.targetObject).toBe('drop_worker'); + expect(dropped.recordIndex).toBe(0); + expect(dropped.message).toContain('Deferred reference dropped'); + expect(dropped.message).toContain('name+region'); + }); + + it('logs it exactly once at ERROR, naming the empty key, the consequence and the fix (#4632)', async () => { + const { engine } = createFaithfulEngine(); + const logger = createLogger(); + + await new SeedLoaderService(engine, createMetadata(), logger).load({ + seeds: compositeSeeds(''), + config: CONFIG, + }); + + // ONE line — and, since nothing else in this load fails, the ONLY error line. + const lines = deferredDropLines(logger); + expect(lines.length, 'the dropped back-fill was not logged exactly once at error').toBe(1); + expect(logger.error.mock.calls.length).toBe(1); + + const [message, err, meta] = lines[0]; + // Locating info: object, field, target, and which record. + expect(String(message)).toContain('drop_department.head_id'); + expect(String(message)).toContain('drop_worker.name'); + // The consequence, concretely. + expect(String(message)).toContain('stays NULL'); + expect(String(message)).toContain('The row itself WAS seeded'); + // WHY there is no id — the empty composite key, named. + expect(String(message)).toContain('`name+region`'); + expect(String(message)).toContain('EMPTY key'); + // The fix. + expect(String(message)).toMatch(/non-empty value/); + expect(String(message)).toMatch(/re-run the seed/); + // Logger contract is `(message, error, meta)`; there is no thrown error here. + expect(err).toBeUndefined(); + expect(meta).toMatchObject({ + object: 'drop_department', + field: 'head_id', + target: 'drop_worker.name', + recordIndex: 0, + recordExternalId: '', + }); + + // NOT downgraded to warn — the level the count has to agree with. + expect( + logger.warn.mock.calls.some((c: unknown[]) => String(c[0]).includes('DROPPED')), + 'the dropped back-fill is being reported at warn', + ).toBe(false); + }); + + /** + * The result object may no longer carry a `referencesDeferred` that nothing + * explains. The counter itself keeps its meaning — "deferred references that + * never landed", decremented only by a SUCCESSFUL back-fill, exactly as the + * two sibling failure branches leave it — so what this pins is the pairing: + * a leftover count now always has a matching entry in `errors`. + */ + it('leaves no dangling referencesDeferred without an error explaining it', async () => { + const { engine } = createFaithfulEngine(); + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: compositeSeeds(''), + config: CONFIG, + }); + + const deptResult = result.results.find((r: { object: string }) => r.object === 'drop_department')!; + expect(deptResult.referencesDeferred).toBe(1); // booked in pass 1, never landed + expect(deptResult.errors.length).toBe(1); // …and now explained + expect(deptResult.errors[0].field).toBe('head_id'); + + for (const entry of result.results) { + if (entry.referencesDeferred > 0) { + expect( + entry.errors.length, + `${entry.object} reports ${entry.referencesDeferred} deferred reference(s) that never landed and zero errors explaining them`, + ).toBeGreaterThan(0); + } + } + expect(result.summary.totalReferencesDeferred).toBe(1); + }); + + /** + * (b) The control: same composite key, `region` filled in. The key registers, + * pass 2 back-fills normally, and the new branch stays out of the way — which + * proves the failure above is caused by the EMPTY key, not by composite keys. + */ + it('a composite key whose components are all present back-fills normally and says nothing', async () => { + const { engine, store } = createFaithfulEngine(); + const logger = createLogger(); + + const result = await new SeedLoaderService(engine, createMetadata(), logger).load({ + seeds: compositeSeeds('APAC'), + config: CONFIG, + }); + + const aliceId = store.drop_worker.find((r) => r.name === 'Alice')!.id; + expect(store.drop_department.find((r) => r.name === 'Engineering')!.head_id).toBe(aliceId); + expect(result.success).toBe(true); + expect(result.summary.totalErrored).toBe(0); + expect(result.summary.totalReferencesDeferred).toBe(0); // decremented by the successful back-fill + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.warn).not.toHaveBeenCalled(); + }); +}); + +/** + * (c, other half) THE SOURCE ROW NEVER LANDED. + * + * A real natural key ('Engineering'), but its pass-1 insert failed, so no id was + * ever registered for it. That failure was ALREADY reported at `error` by the + * write site (#4729), so the dropped back-fill is recorded under the same + * criterion and logged as exactly ONE additional line — one that points at the + * pass-1 error instead of restating it. This case is distinguishable from the + * pure loss above by its message and by the non-empty `recordExternalId`. + */ +describe('pass 2 finds no id because the source row failed in pass 1 (#5127)', () => { + const SEEDS = [ + { + object: 'drop_department', + externalId: 'name', + mode: 'insert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'Engineering', region: 'APAC', head_id: 'Alice' }], + }, + { + // No `department_id` value: this dataset exists to make the dependency + // graph circular (the FIELD is declared in metadata), not to add a second + // deferred update that would muddy the assertions below. + object: 'drop_worker', + externalId: 'name', + mode: 'insert', + env: ['prod', 'dev', 'test'], + records: [{ name: 'Alice' }], + }, + ] as any[]; + + function loadWithFailingDepartmentInsert() { + const { engine, store } = createFaithfulEngine(); + const logger = createLogger(); + const realInsert = (engine.insert as any).getMockImplementation(); + (engine.insert as any).mockImplementation(async (obj: string, data: any, opts: any) => { + // A validation-style (non-transient) rejection: the row genuinely never lands. + if (obj === 'drop_department') throw new Error('CHECK constraint failed: drop_department'); + return realInsert(obj, data, opts); + }); + return { engine, store, logger }; + } + + it('records the dropped back-fill and points at the already-reported pass-1 error', async () => { + const { engine, store, logger } = loadWithFailingDepartmentInsert(); + + const result = await new SeedLoaderService(engine, createMetadata(), logger).load({ + seeds: SEEDS, + config: CONFIG, + }); + + // The row really is absent — nothing to back-fill onto. + expect(store.drop_department ?? []).toHaveLength(0); + expect(result.success).toBe(false); + + // Two DISTINCT losses are reported: the row, and the link it would have carried. + const dropped = result.errors.find( + (e: { field: string; message?: string }) => e.field === 'head_id' && String(e.message).includes('Deferred reference dropped'), + )!; + expect(dropped, 'the dropped back-fill was not recorded as an error').toBeDefined(); + expect(dropped.message).toContain("no internal id was registered for drop_department 'Engineering'"); + expect(result.errors.length).toBeGreaterThan(1); // …the pass-1 write error is still there too + + // Exactly ONE extra line for the dropped back-fill — no second flood over + // the same root cause — and it sends the reader to the pass-1 error. + const lines = deferredDropLines(logger); + expect(lines.length).toBe(1); + const [message, err, meta] = lines[0]; + expect(String(message)).toContain('drop_department.head_id'); + expect(String(message)).toContain("record 'Engineering'"); + expect(String(message)).toContain('pass-1 write FAILED'); + expect(String(message)).toMatch(/re-run the seed/); + expect(err).toBeUndefined(); + expect(meta).toMatchObject({ + object: 'drop_department', + field: 'head_id', + recordExternalId: 'Engineering', + }); + + // The pass-1 write error (#4729) is untouched — this fix adds a line, it + // does not replace or suppress the one that was already correct. + expect( + logger.error.mock.calls.some((c: unknown[]) => String(c[0]).includes('CHECK constraint failed')), + 'the pass-1 write error stopped being reported', + ).toBe(true); + }); + + it('does not claim the reference "stays NULL" on a row that was never written', async () => { + const { engine, logger } = loadWithFailingDepartmentInsert(); + + await new SeedLoaderService(engine, createMetadata(), logger).load({ seeds: SEEDS, config: CONFIG }); + + // The pure-loss wording would be a lie here: there is no row to hold a NULL, + // and telling the reader to fix an externalId component sends them nowhere. + const [message] = deferredDropLines(logger)[0]; + expect(String(message)).not.toContain('EMPTY key'); + expect(String(message)).not.toContain('stays NULL'); + }); +}); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index eeefc755d9..e4aae7192d 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -782,6 +782,7 @@ export class SeedLoaderService implements ISeedLoaderService { deferredUpdates.push({ objectName, recordExternalId: this.externalIdKey(record, externalId), + externalIdLabel: this.externalIdLabel(externalId), field: ref.field, targetObject: ref.targetObject, targetField: ref.targetField, @@ -1170,6 +1171,96 @@ export class SeedLoaderService implements ISeedLoaderService { this.recordDeferredError(deferred, allResults, allErrors, `Failed to write deferred reference: ${deferred.objectName}.${deferred.field} = '${this.formatAttempted(deferred.attemptedValue)}' → ${deferred.targetObject}.${deferred.targetField}: ${err?.message ?? String(err)}`); } + } else { + // THE TARGET RESOLVED BUT THE SOURCE ROW HAS NO ID (#5127). + // + // Pass 2 did its job — `resolvedValue` is a real internal id — and + // then found no internal id to write it ONTO: this load never + // registered `deferred.recordExternalId` in `insertedRecords`. Until + // now this branch did not exist, so the back-fill simply evaporated: + // no write, no `errors`/`allErrors` entry (so `success` stayed true), + // no `errored`, no log. The ONLY trace left was the `referencesDeferred` + // this record booked in pass 1 and never gave back — a number with + // nothing in the result explaining it. + // + // It is the deeper cousin of the two branches around it: #4729 fixed + // "counted but logged at `warn`", #4997 fixed "counted, never logged", + // this one was "never counted, never logged". Same objective criterion + // as both (does the outcome enter `errors`/`allErrors`?) — so it is + // recorded AND logged at `error` per AGENTS.md → "Degradation log + // levels" (#4632): the row is in the database, every row counter reads + // healthy, and the association it declares is permanently absent. + // + // `referencesDeferred` deliberately stays booked, exactly as the two + // sibling failure branches leave it: the counter means "deferred + // references that never landed", and only a SUCCESSFUL back-fill + // decrements it. What was missing was never the arithmetic — it was + // the error that explains the leftover, which `recordDeferredError` + // now supplies (the load reports `success: false`, `errored` counts + // the loss, and the dangling count has a matching entry in `errors`). + // + // The two ways to get here are NOT the same failure, so they do not + // get the same line. An EMPTY `recordExternalId` is the pure silent + // loss: `externalIdKey` returns `''` when any component of a composite + // key is blank, the row itself wrote fine, and nothing anywhere else + // reports it — this line is the only one a reader will ever see. A + // NON-empty key that is simply absent from the map means the source + // row did not land (or its write returned no id); that failure was + // already reported at `error` by the pass-1 write site (#4729), so + // this line points AT that error instead of restating it — one line, + // not a second flood over the same root cause. + const missedTarget = this.formatAttempted(deferred.attemptedValue); + const where = `${deferred.targetObject}.${deferred.targetField} = '${missedTarget}'`; + if (deferred.recordExternalId === '') { + this.logger.error( + `[SeedLoader] Deferred reference DROPPED — ${deferred.objectName}.${deferred.field} stays NULL ` + + `FOREVER on record #${deferred.recordIndex}. Pass 2 RESOLVED the target (${where}) and then had ` + + `nowhere to write it: that record's externalId (\`${deferred.externalIdLabel}\`) evaluated to the ` + + `EMPTY key, which is what \`externalIdKey\` returns when the key field is absent or blank — or, ` + + `for a composite key, when ANY one of its components is — so no internal id was ever registered ` + + `for it in this load. The row itself WAS ` + + `seeded, so every row counter looks healthy while the relationship is MISSING, and nothing ` + + `retries this: pass 2 is the last one. Give every \`${deferred.externalIdLabel}\` component a ` + + `non-empty value in the ${deferred.objectName} seed data (or declare an externalId whose parts ` + + `are always present) and re-run the seed to complete the link.`, + undefined, + { + object: deferred.objectName, + field: deferred.field, + target: `${deferred.targetObject}.${deferred.targetField}`, + recordIndex: deferred.recordIndex, + recordExternalId: deferred.recordExternalId, + }, + ); + this.recordDeferredError(deferred, allResults, allErrors, + `Deferred reference dropped: ${deferred.objectName}.${deferred.field} = '${missedTarget}' → ` + + `${deferred.targetObject}.${deferred.targetField} resolved, but ${deferred.objectName} record ` + + `#${deferred.recordIndex} has an empty \`${deferred.externalIdLabel}\` externalId, so no internal ` + + `id was registered for it and the back-fill could not be written`); + } else { + this.logger.error( + `[SeedLoader] Deferred reference DROPPED — ${deferred.objectName}.${deferred.field} is never written ` + + `on record '${deferred.recordExternalId}'. Pass 2 RESOLVED the target (${where}) and then had ` + + `nowhere to write it: this load registered no internal id for that ${deferred.objectName} record, ` + + `because its pass-1 write FAILED (reported as its own \`error\` above) or returned no id. Nothing ` + + `retries this: pass 2 back-fills only rows this load actually seeded, and it is the last pass. ` + + `Fix the pass-1 write error reported for ${deferred.objectName} '${deferred.recordExternalId}' ` + + `and re-run the seed — the row and this link land together or not at all.`, + undefined, + { + object: deferred.objectName, + field: deferred.field, + target: `${deferred.targetObject}.${deferred.targetField}`, + recordIndex: deferred.recordIndex, + recordExternalId: deferred.recordExternalId, + }, + ); + this.recordDeferredError(deferred, allResults, allErrors, + `Deferred reference dropped: ${deferred.objectName}.${deferred.field} = '${missedTarget}' → ` + + `${deferred.targetObject}.${deferred.targetField} resolved, but no internal id was registered for ` + + `${deferred.objectName} '${deferred.recordExternalId}' in this load (its pass-1 write failed), so ` + + `the back-fill could not be written`); + } } } else { // Still unresolved after pass 2 — the target never materialized. Name @@ -1241,9 +1332,12 @@ export class SeedLoaderService implements ISeedLoaderService { * Record a pass-2 (deferred) reference failure as a first-class error: it * lands in the object's per-result `errors`, bumps its `errored` count (so * `summary.totalErrored` is truthful), and joins `allErrors` (so the load - * reports `success: false`). Both pass-2 failure modes — target still - * missing, or the back-fill write threw — route through here so neither can - * leave an incomplete relationship reported as a clean load (framework#2805). + * reports `success: false`). ALL THREE pass-2 failure modes route through here + * so none can leave an incomplete relationship reported as a clean load + * (framework#2805): the target is still missing, the back-fill write threw, + * or — the mode that used to fall out of `resolveDeferredUpdates` without + * reaching any of this (#5127) — the target resolved but the source record has + * no registered internal id to write it onto. */ private recordDeferredError( deferred: DeferredUpdate, @@ -1974,7 +2068,27 @@ interface SummaryRecomputeLike { interface DeferredUpdate { objectName: string; + /** + * The source record's natural key, as {@link SeedLoaderService.externalIdKey} + * computed it in pass 1 — the key pass 2 looks the record's internal id up by. + * + * May legitimately be `''`: `externalIdKey` returns the empty string when the + * key field is absent or blank, and — the case that actually bites — when ANY + * ONE component of a composite externalId is. An empty key is + * never registered in `insertedRecords`, so a deferred update carrying one can + * never find its record in pass 2 — which is precisely the "wrote the row, + * dropped the link" case #5127 exists to report. Carried verbatim (not + * normalised to `undefined`) so pass 2 can tell that case apart from a real + * key whose record simply failed to write. + */ recordExternalId: string; + /** + * Human-readable name of the source dataset's externalId (`name`, or `a+b` for + * a composite) — pass 2 has no other handle on it, and an empty + * `recordExternalId` is only actionable if the message can say WHICH key came + * out empty (#5127). + */ + externalIdLabel: string; field: string; targetObject: string; targetField: string;