Skip to content

Commit 627b188

Browse files
claudeos-zhuang
authored andcommitted
fix(seed-loader): count reference fields dropped from written rows (#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
1 parent ca0a1f8 commit 627b188

11 files changed

Lines changed: 186 additions & 6 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/runtime": patch
5+
"@objectstack/cli": patch
6+
"@objectstack/cloud-connection": patch
7+
---
8+
9+
fix(seed-loader): count reference fields dropped from rows that were still written
10+
11+
The loader had two failure outcomes and only counted one. A record it cannot
12+
write is counted in `errored`. But an unusable **reference value** (an object
13+
where a natural key belongs, an array on a single-value field) is removed from
14+
the record — never written as NULL, which would sever an existing link on
15+
upsert replay — and the row is written **without it**. Nothing counted that.
16+
17+
So a load that quietly severed N associations reported `totalErrored: 0`, and
18+
every count-driven surface read clean. The CLI boot banner — the one seed signal
19+
that survives `os dev`'s boot-quiet window and the default `warn` level — printed
20+
`showcase 42 rows`, and the warn line said `0 dropped record(s)`: true, and
21+
useless ([#3932](https://github.com/objectstack-ai/objectstack/issues/3932)).
22+
23+
`SeedLoadResult.referencesDropped` and `SeedLoaderSummary.totalReferencesDropped`
24+
now count it. It is deliberately **not** folded into `errored` — the row *was*
25+
written, so that would break the `inserted + updated + skipped` reconciliation
26+
against `total`. The banner names it separately:
27+
28+
```
29+
⚠ Seeds: showcase 42 ok / 3 lost links ⚠
30+
```
31+
32+
Both counters are additive with a `0` default, so an existing producer or
33+
consumer of `SeedLoaderResult` is unaffected.

content/docs/references/data/seed-loader.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ Result of loading a single dataset
150150
| **total** | `integer` || Total records in dataset |
151151
| **referencesResolved** | `integer` || References resolved via externalId |
152152
| **referencesDeferred** | `integer` || References deferred to second pass |
153+
| **referencesDropped** | `integer` || Reference fields dropped from records that were still written |
153154
| **errors** | `{ sourceObject: string; field: string; targetObject: string; targetField: string; … }[]` || Reference resolution errors |
154155

155156

packages/cli/src/utils/format.seed-summary.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,28 @@ describe('printServerReady seed summary (#3415/#3430)', () => {
4747
expect(lines.some((l) => l.includes('OS_LOG_LEVEL=info'))).toBe(true);
4848
});
4949

50+
it('screams about lost links even when every row landed (#3932)', () => {
51+
// The "wrote the row, lost the link" case: nothing rejected, the row counts
52+
// look perfect, an association is silently missing. This line used to read
53+
// `showcase 42 rows` — clean, and wrong.
54+
printServerReady({ ...base, seeds: [s({ source: 'showcase', inserted: 42, droppedRefs: 3 })] });
55+
expect(seedLines()).toHaveLength(1);
56+
expect(seedLines()[0]).toContain('showcase 42 ok / 3 lost links ⚠');
57+
expect(lines.some((l) => l.includes('OS_LOG_LEVEL=info'))).toBe(true);
58+
});
59+
60+
it('reports rejected records and lost links together, singular-aware', () => {
61+
printServerReady({ ...base, seeds: [s({ source: 'showcase', inserted: 20, rejected: 1, droppedRefs: 1 })] });
62+
expect(seedLines()[0]).toContain('showcase 20 ok / 1 error / 1 lost link ⚠');
63+
});
64+
65+
it('stays quiet when no reference was dropped', () => {
66+
printServerReady({ ...base, seeds: [s({ source: 'showcase', inserted: 42, droppedRefs: 0 })] });
67+
expect(seedLines()[0]).toContain('showcase 42 rows');
68+
expect(seedLines()[0]).not.toContain('lost link');
69+
expect(seedLines()[0]).not.toContain('⚠');
70+
});
71+
5072
it('labels a marketplace package and marks a fresh-DB heal', () => {
5173
printServerReady({
5274
...base,

packages/cli/src/utils/format.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,12 @@ export interface SeedSourceSummary {
331331
skipped: number;
332332
/** Records dropped by validation/reference errors — the silent-loss case. */
333333
rejected: number;
334+
/**
335+
* Reference FIELDS dropped from rows that WERE written (#3932). The row
336+
* count stays healthy — the association is what went missing — so unless
337+
* this is said out loud, nothing on this line hints at it.
338+
*/
339+
droppedRefs?: number;
334340
/**
335341
* Rows were (re)seeded onto a fresh/empty database during rehydrate — the
336342
* "swap the DB out from under an installed package" self-heal (#3430).
@@ -460,20 +466,26 @@ function printSeedSummary(sources: SeedSourceSummary[]) {
460466
const shown = sources.filter((s) => {
461467
// Empty installs and rejections are ALWAYS shown (they're the whole point);
462468
// a source that touched no rows and had no problem is noise — drop it.
463-
if (s.emptyInstall || s.rejected > 0) return true;
469+
if (s.emptyInstall || s.rejected > 0 || (s.droppedRefs ?? 0) > 0) return true;
464470
return s.inserted + s.updated + s.skipped > 0;
465471
});
466472
if (shown.length === 0) return;
467473

468-
const anyProblem = shown.some((s) => s.rejected > 0 || s.emptyInstall);
474+
const anyProblem = shown.some((s) => s.rejected > 0 || (s.droppedRefs ?? 0) > 0 || s.emptyInstall);
469475

470476
const fragment = (s: SeedSourceSummary): string => {
471477
const label = s.marketplace ? `${s.source}(marketplace)` : s.source;
472478
if (s.emptyInstall) return `${label} installed but 0 rows ⚠`;
473479
const ok = s.inserted + s.updated + s.skipped;
480+
// A dropped reference leaves the row in place, so it never shows up in the
481+
// row counts — name it separately or the line reads clean over a severed
482+
// association (#3932).
483+
const dropped = s.droppedRefs ?? 0;
484+
const lostLinks = dropped > 0 ? ` / ${dropped} lost link${dropped === 1 ? '' : 's'}` : '';
474485
if (s.rejected > 0) {
475-
return `${label} ${ok} ok / ${s.rejected} error${s.rejected === 1 ? '' : 's'} ⚠`;
486+
return `${label} ${ok} ok / ${s.rejected} error${s.rejected === 1 ? '' : 's'}${lostLinks} ⚠`;
476487
}
488+
if (dropped > 0) return `${label} ${ok} ok${lostLinks} ⚠`;
477489
return `${label} ${ok} rows${s.healed ? ' (healed on fresh db)' : ''}`;
478490
};
479491

packages/cloud-connection/src/marketplace-install-local-plugin.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
263263
updated: summary.updated ?? 0,
264264
skipped: summary.skipped ?? 0,
265265
rejected: summary.errors ?? 0,
266+
droppedRefs: summary.droppedRefs ?? 0,
266267
healed: true,
267268
});
268269
} else {
@@ -310,6 +311,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
310311
updated: number;
311312
skipped: number;
312313
rejected: number;
314+
droppedRefs?: number;
313315
healed?: boolean;
314316
emptyInstall?: boolean;
315317
},
@@ -1014,7 +1016,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
10141016
ctx: PluginContext,
10151017
datasets: any[],
10161018
organizationId?: string,
1017-
): Promise<{ inserted: number; updated: number; skipped: number; errors: number; errorSample?: string }> => {
1019+
): Promise<{ inserted: number; updated: number; skipped: number; errors: number; droppedRefs: number; errorSample?: string }> => {
10181020
const ql: any = ctx.getService('objectql');
10191021
let metadata: any;
10201022
try { metadata = ctx.getService('metadata'); } catch { /* none */ }
@@ -1040,6 +1042,9 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
10401042
updated: result.summary.totalUpdated,
10411043
skipped: result.summary.totalSkipped ?? 0,
10421044
errors: result.errors.length,
1045+
// Reference fields dropped from rows that WERE written (#3932) —
1046+
// invisible in every row count, so carried explicitly.
1047+
droppedRefs: result.summary.totalReferencesDropped ?? 0,
10431048
// Surface the first write/resolution failure so the caller can
10441049
// report WHY nothing landed (e.g. a locked DB, a missing table,
10451050
// a failed validation) instead of a bare "0 rows".

packages/metadata-protocol/src/seed-loader-multi-value-reference.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,48 @@ describe('seed reference resolution — multi-value lookup (multiple: true)', ()
277277
expect(store.book[0].reviewer).toBeUndefined();
278278
expect(store.book[0].name).toBe('Refactoring');
279279
expect(logger.warn).toHaveBeenCalled();
280+
281+
// framework#3932: the row WAS written, so `errored` stays 0 and the row
282+
// counters all look healthy — the loss only shows up here.
283+
expect(result.summary.totalErrored).toBe(0);
284+
expect(result.summary.totalReferencesDropped).toBe(1);
285+
expect(result.results.find((r) => r.object === 'book')!.referencesDropped).toBe(1);
286+
});
287+
288+
it('counts a dropped reference field without disturbing the row counters (#3932)', async () => {
289+
const { engine, store } = createEngine(SCHEMAS);
290+
291+
const result = await new SeedLoaderService(engine, createEmptyMetadata(), createLogger()).load({
292+
seeds: [AUTHOR_SEED, bookSeed([
293+
{ name: 'Refactoring', authors: [{ externalId: 'Alice' }] },
294+
{ name: 'Clean Code', authors: ['Alice'] },
295+
])] as any,
296+
config: CONFIG,
297+
});
298+
299+
const book = result.results.find((r) => r.object === 'book')!;
300+
// Both rows were written — one just lost its association.
301+
expect(store.book).toHaveLength(2);
302+
expect(book.inserted).toBe(2);
303+
expect(book.errored).toBe(0);
304+
expect(book.referencesDropped).toBe(1);
305+
// The reconciliation `errored` must not break: inserted + updated + skipped
306+
// still accounts for every record in the dataset.
307+
expect(book.inserted + book.updated + book.skipped + book.errored).toBe(book.total);
308+
// Still a failed load — the counter reports damage, it does not excuse it.
309+
expect(result.success).toBe(false);
310+
});
311+
312+
it('reports zero dropped references on a clean load', async () => {
313+
const { engine } = createEngine(SCHEMAS);
314+
315+
const result = await new SeedLoaderService(engine, createEmptyMetadata(), createLogger()).load({
316+
seeds: [AUTHOR_SEED, bookSeed([{ name: 'Refactoring', authors: ['Alice', 'Bob'] }])] as any,
317+
config: CONFIG,
318+
});
319+
320+
expect(result.success).toBe(true);
321+
expect(result.summary.totalReferencesDropped).toBe(0);
280322
});
281323

282324
it('still rejects a wrapper object inside a multi-value array', async () => {

packages/metadata-protocol/src/seed-loader.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,13 @@ export class SeedLoaderService implements ISeedLoaderService {
251251
let errored = 0;
252252
let referencesResolved = 0;
253253
let referencesDeferred = 0;
254+
/**
255+
* Reference FIELDS dropped from records that were still written — the
256+
* "wrote the row, lost the link" outcome. Kept apart from `errored` (which
257+
* counts dropped RECORDS) so `inserted + updated + skipped + errored`
258+
* still reconciles against `total`. See framework#3932.
259+
*/
260+
let referencesDropped = 0;
254261
const errors: ReferenceResolutionError[] = [];
255262

256263
// Ensure the object's record map exists
@@ -507,8 +514,12 @@ export class SeedLoaderService implements ISeedLoaderService {
507514
this.logger.warn(`[SeedLoader] ${error.message}`, { recordIndex: i });
508515
// Drop the unwritable value so it never reaches the driver. Removing
509516
// the key (not writing null) matters on the upsert UPDATE path — see
510-
// the deferred-reference note below.
517+
// the deferred-reference note below. The row itself still gets
518+
// written, so this is a dropped FIELD, not a dropped record — counted
519+
// separately (framework#3932) or every count-driven surface, notably
520+
// the CLI boot banner, reads clean over a severed association.
511521
delete record[ref.field];
522+
referencesDropped++;
512523
continue;
513524
}
514525

@@ -558,7 +569,9 @@ export class SeedLoaderService implements ISeedLoaderService {
558569
// Removing the key (not writing null) matters on the upsert UPDATE
559570
// path: an explicit null would overwrite the existing row's valid
560571
// reference, silently severing the link on every seed replay.
572+
// Counted as a dropped FIELD (see the array branch above).
561573
delete record[ref.field];
574+
referencesDropped++;
562575
continue;
563576
}
564577

@@ -720,6 +733,7 @@ export class SeedLoaderService implements ISeedLoaderService {
720733
total: dataset.records.length,
721734
referencesResolved,
722735
referencesDeferred,
736+
referencesDropped,
723737
errors,
724738
};
725739
}
@@ -1458,6 +1472,7 @@ export class SeedLoaderService implements ISeedLoaderService {
14581472
totalErrored: 0,
14591473
totalReferencesResolved: 0,
14601474
totalReferencesDeferred: 0,
1475+
totalReferencesDropped: 0,
14611476
circularDependencyCount: 0,
14621477
durationMs,
14631478
},
@@ -1480,6 +1495,7 @@ export class SeedLoaderService implements ISeedLoaderService {
14801495
totalErrored: results.reduce((sum, r) => sum + r.errored, 0),
14811496
totalReferencesResolved: results.reduce((sum, r) => sum + r.referencesResolved, 0),
14821497
totalReferencesDeferred: results.reduce((sum, r) => sum + r.referencesDeferred, 0),
1498+
totalReferencesDropped: results.reduce((sum, r) => sum + (r.referencesDropped ?? 0), 0),
14831499
circularDependencyCount: graph.circularDependencies.length,
14841500
durationMs,
14851501
};

packages/runtime/src/app-plugin.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,11 @@ export class AppPlugin implements Plugin {
911911
});
912912
const result = await seedLoader.load(request);
913913
const { totalInserted, totalUpdated, totalSkipped, totalErrored } = result.summary;
914+
// "Wrote the row, lost the link" (#3932): a reference field
915+
// dropped from a row that WAS written moves none of the row
916+
// counters, so it needs carrying separately or the banner
917+
// reads clean over a severed association.
918+
const totalRefsDropped = result.summary.totalReferencesDropped ?? 0;
914919
// #3415/#3430: stash a per-source outcome on the kernel so
915920
// the CLI boot banner can print a Seeds line. The logs below
916921
// never reach `os dev` output — info is under the default
@@ -923,6 +928,7 @@ export class AppPlugin implements Plugin {
923928
updated: totalUpdated,
924929
skipped: totalSkipped,
925930
rejected: totalErrored,
931+
droppedRefs: totalRefsDropped,
926932
});
927933
if (result.success) {
928934
ctx.logger.info('[Seeder] Seed loading complete', {
@@ -936,13 +942,20 @@ export class AppPlugin implements Plugin {
936942
// invisible (the summary only logged errors.length and
937943
// omitted totalErrored). Report the count AND each
938944
// actionable reason so broken seeds can't pass silently.
945+
// Dropped reference FIELDS are named separately — the
946+
// old line said "0 dropped record(s)" over a load that
947+
// had severed associations, which is true and useless.
948+
const lostLinks = totalRefsDropped > 0
949+
? `, ${totalRefsDropped} dropped reference field(s) on written rows,`
950+
: '';
939951
ctx.logger.warn(
940-
`[Seeder] Seed loading completed with ${totalErrored} dropped record(s) and ${result.errors.length} error(s) for ${appId}`,
952+
`[Seeder] Seed loading completed with ${totalErrored} dropped record(s)${lostLinks} and ${result.errors.length} error(s) for ${appId}`,
941953
{
942954
inserted: totalInserted,
943955
updated: totalUpdated,
944956
skipped: totalSkipped,
945957
errored: totalErrored,
958+
referencesDropped: totalRefsDropped,
946959
},
947960
);
948961
for (const e of result.errors.slice(0, 20)) {

packages/runtime/src/seed-summary.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ export interface SeedSourceOutcome {
3131
skipped: number;
3232
/** Rows dropped by validation/reference errors — the silent-loss case. */
3333
rejected: number;
34+
/**
35+
* Reference FIELDS dropped from rows that were still written — "wrote the
36+
* row, lost the link" (framework#3932). A distinct loss from `rejected`:
37+
* the row is there, so a row count looks healthy while an association is
38+
* silently missing. Surfaced separately so the banner can say so.
39+
*/
40+
droppedRefs?: number;
3441
/**
3542
* The rows were (re)seeded onto a fresh/empty database during rehydrate —
3643
* the "swap the DB out from under an installed package" self-heal. Surfaced
@@ -91,6 +98,7 @@ export function recordSeedOutcome(ctx: unknown, outcome: SeedSourceOutcome): voi
9198
existing.updated += outcome.updated;
9299
existing.skipped += outcome.skipped;
93100
existing.rejected += outcome.rejected;
101+
existing.droppedRefs = (existing.droppedRefs ?? 0) + (outcome.droppedRefs ?? 0);
94102
existing.healed = existing.healed || outcome.healed;
95103
existing.emptyInstall = existing.emptyInstall || outcome.emptyInstall;
96104
} else {

packages/spec/authorable-surface.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3752,6 +3752,7 @@
37523752
"data/SeedLoadResult:mode",
37533753
"data/SeedLoadResult:object",
37543754
"data/SeedLoadResult:referencesDeferred",
3755+
"data/SeedLoadResult:referencesDropped",
37553756
"data/SeedLoadResult:referencesResolved",
37563757
"data/SeedLoadResult:skipped",
37573758
"data/SeedLoadResult:total",

0 commit comments

Comments
 (0)