diff --git a/.changeset/summary-backfill-recompute-undefined-on-empty.md b/.changeset/summary-backfill-recompute-undefined-on-empty.md new file mode 100644 index 0000000000..4ab5fa0ed8 --- /dev/null +++ b/.changeset/summary-backfill-recompute-undefined-on-empty.md @@ -0,0 +1,57 @@ +--- +"@objectstack/objectql": minor +"@objectstack/cli": minor +--- + +feat(objectql,cli): `backfillSummaryNulls` accepts `recomputeUndefinedOnEmpty` — a caller who KNOWS a `min`/`max`/`avg` roll-up column was just declared can have it filled; `os migrate summary-nulls --recompute-undefined-on-empty object.field` surfaces it (#15064) + +A roll-up value has three producers — the insert-time seed, the child-write +recompute, and the one-off backfill — and **declaring a summary field on an +object that already has rows reaches none of them**. For `count`/`sum` the +backfill repairs that as a side effect (every `NULL` is a hole to it). For +`min`/`max`/`avg` it could not: `summaryNullIsBackfillable` decides on the +function alone, so "never computed" and "no child rows" were indistinguishable, +the column stayed `NULL` on every pre-existing parent, and the report said +`filled: 0` — a false all-clear that a timed flow built on the column then +turned into "matches nothing" (the customer case behind cloud#1908). + +**What changes** — maintainer ruling on #15064, option A: the caller who holds +the fact gets a way to say it; the predicate and the default run do not move. + +- `SummaryBackfillOptions.recomputeUndefinedOnEmpty?: string[]` — `object.field` + roll-ups the caller knows were never computed. A named `min`/`max`/`avg` is + walked like a `count`: every `NULL` parent is recomputed through the same + `aggregateSummaryValue` the engine writes. A parent whose aggregate is the + empty-set reading (`null` — no child rows) already holds the engine's own + value, so it is neither counted as a hole nor written; the scoped run is + therefore idempotent in the same "re-run until it reports zero" sense. + Naming a `count`/`sum` is accepted and changes nothing, so a publish path can + pass every column it just declared without knowing the empty-set list. +- A name that resolves to no roll-up owned by an object the run walks — a typo, + a plain field, or an object `objects` left out — is **refused before any row + is read**, dry run or apply, with an ADR-0112 envelope (`code: + 'INVALID_FIELD'`, `status: 400` — the code the projection and write axes + that name a field already answer, while sorting keeps `INVALID_SORT`; + `field` names the first unresolved entry, `fields` all of them). A silent + no-op there would be the same false all-clear this option exists to end. +- `SummaryBackfillReport.recomputedUndefinedOnEmpty: string[]` — the complement + of `skippedUndefinedOnEmpty`, same `object.field (fn)` spelling; `[]` on an + unscoped run. `SummaryBackfillFieldOutcome.fn` widens from `'count' | 'sum'` + to every roll-up function, since a named `max` now appears in `fields`. +- `os migrate summary-nulls --recompute-undefined-on-empty object.field` + (repeatable) passes the scope through; the confirmation prompt names the + columns; `formatSummaryBackfillReport` lists them under "Recomputed on + request" and explains a `NULL` that remains. + +**What does not change:** without the option the walk, the writes, every +counter and the human-readable report are byte-for-byte what they were (pinned +against output captured on `main` before this change); `min`/`max`/`avg` stay +out of scope and keep being reported under `skippedUndefinedOnEmpty`; the +predicate `summaryNullIsBackfillable` is untouched, so `os migrate +summary-nulls` keeps its meaning on every deployment. The only visible delta on +an unscoped run is the one additive report key, `recomputedUndefinedOnEmpty: []`. + +`minor` for both packages: an optional parameter on a published exported +function, a new report key, and a new CLI flag are each a purely additive +widening of a published surface, which takes at least `minor` (bump-level rule, +2026-09-04); the `fix`-shaped motivation does not lower it. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index dc208c30e3..dca760eb88 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -75,7 +75,13 @@ reads those as field filters, so one naming no field could only match zero records and is rejected rather than answered with an empty page — plus every other read axis that names a field: `select`, `expand` (a real field that holds no reference gets its own message), `searchFields` (a real field outside the -searchable set gets its own message), `groupBy`, and `aggregations[].field`. +searchable set gets its own message), `groupBy`, and `aggregations[].field`. +Off the request path the same code answers `backfillSummaryNulls`'s +`recomputeUndefinedOnEmpty` (`os migrate summary-nulls +--recompute-undefined-on-empty object.field`) when an entry is not a roll-up +owned by an object the run walks — a typo, a real non-summary field, or a +roll-up on an object `--object` left out are refused alike, one message naming +every unresolved entry and how many objects the run walked. **Fix:** Check the object schema for valid field names. Use `os meta get object ` to inspect the object's fields. If the name was meant as a *parameter* rather than a field, use the real one — page size is `top` / `$top` / `limit`, not `pageSize` / `perPage`; the response's `error` names the diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 9e8c1af4c1..baa5469255 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -912,6 +912,8 @@ os migrate summary-nulls # Dry run: full report, writes nothi os migrate summary-nulls --apply # Recompute and write (prompts) os migrate summary-nulls --apply --yes --json # CI / scripts os migrate summary-nulls --object project # Restrict to one object (repeatable) +os migrate summary-nulls --apply --recompute-undefined-on-empty customer.last_follow_up_at + # Also fill a min/max/avg column you know was never computed ``` **Each affected row is recomputed, not set to 0.** A pre-upgrade parent that @@ -920,9 +922,25 @@ them — writing 0 there would replace a missing value with a wrong one, and the next child write would change it back. The report separates the two: `N NULL row(s), M with real child data`. -`min` / `max` / `avg` are **never touched**. They are undefined on an empty set, -so a `null` there is the correct reading of "no child rows"; the report lists -them as deliberately skipped. +`min` / `max` / `avg` are **never touched by default**. They are undefined on an +empty set, so a `null` there is the correct reading of "no child rows"; the +report lists them as deliberately skipped. + +The one case that reading gets wrong is a summary field **declared after its +parent rows already existed**: nothing has ever computed it — the insert-time +seed is create-time, the recompute runs only on a child write — so every +pre-existing parent reads `NULL` whether or not it has children, and a flow +built on the column matches nothing. The migration cannot tell that `NULL` +from a legitimate one; the operator (or the publish path) who just declared +the column can. Name it with `--recompute-undefined-on-empty object.field` +(repeatable) and it is walked like a `count`: every `NULL` parent is recomputed +through the same aggregate the engine writes, a parent with no child rows keeps +`NULL` (that is the aggregate's own value, and it is neither counted nor +written), and the report lists the column under "recomputed on request". A +name that is not a roll-up this run walks — a typo, a plain field, or an object +`--object` left out — is refused before any row is read. Naming a `count` / +`sum` is accepted and changes nothing, so a caller can pass every column it +just declared. Idempotent — every write turns a `NULL` into a number, so a second run finds nothing and writes nothing. Re-running until the report says zero *is* the diff --git a/packages/cli/src/commands/migrate/summary-nulls.test.ts b/packages/cli/src/commands/migrate/summary-nulls.test.ts new file mode 100644 index 0000000000..d288965f77 --- /dev/null +++ b/packages/cli/src/commands/migrate/summary-nulls.test.ts @@ -0,0 +1,150 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate summary-nulls` command shape, and the #15064 scope it surfaces. + * + * The backfill itself is proven in `@objectstack/objectql`'s + * `summary-backfill.test.ts`. What is pinned here is what a unit test of the + * backfill cannot see: that the command is dry-run-by-default (#2186), and + * that `--recompute-undefined-on-empty object.field` reaches + * `backfillSummaryNulls` as `recomputeUndefinedOnEmpty` — every entry, in + * order — while a run without the flag hands the option through as `undefined` + * (the unscoped run the ruling keeps byte-for-byte). The seams that would boot + * a database or walk a real engine are replaced; the command's own parse and + * control flow run for real. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import MigrateSummaryNulls from './summary-nulls.js'; +import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { isExitSignal } from '../../utils/format.js'; +import { backfillSummaryNulls } from '@objectstack/objectql'; + +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: vi.fn() })); +vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ + OCCUPANCY_HINT: 'occupancy hint', + probeMigrationTarget: vi.fn(), +})); +vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) })); +vi.mock('@objectstack/objectql', () => ({ + backfillSummaryNulls: vi.fn(), + formatSummaryBackfillReport: vi.fn(() => []), +})); + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +/** oclif builds its whole command table on the first `run()` in a process. */ +const RUN_TIMEOUT = 60_000; + +/** The engine surface the command checks before it runs: the roll-up index + * verb, and at least one loaded app object (a `sys_`-only stack is refused). */ +const engine = { + getOwnedSummaryDescriptors: () => [], + getConfigs: () => ({ customer: {}, sys_user: {} }), +}; + +const EMPTY_REPORT = { + scannedObjects: [], scannedRecords: 0, fields: [], nullRows: 0, filled: 0, + skippedUndefinedOnEmpty: [], recomputedUndefinedOnEmpty: [], applied: false, + truncated: false, unreadableObjects: [], failures: [], +}; + +let stdout: ReturnType; +let log: ReturnType; +beforeEach(() => { + vi.mocked(probeMigrationTarget).mockResolvedValue({ status: 'free' } as any); + vi.mocked(bootSchemaStack).mockResolvedValue({ + kernel: { getService: () => engine }, + dbLabel: 'file:test.db', + shutdown: vi.fn(async () => {}), + } as any); + vi.mocked(backfillSummaryNulls).mockReset(); + vi.mocked(backfillSummaryNulls).mockResolvedValue(EMPTY_REPORT as any); + // `emitJson` awaits the write's DRAIN callback (a `--json` payload must be + // fully written before the process can exit), so the double has to invoke + // it — a bare `() => true` hangs the command forever. + stdout = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, enc?: unknown, cb?: unknown) => { + const done = typeof enc === 'function' ? enc : cb; + if (typeof done === 'function') done(); + return true; + }) as typeof process.stdout.write); + log = vi.spyOn(console, 'log').mockImplementation(() => {}); +}); +afterEach(() => { + stdout.mockRestore(); + log.mockRestore(); +}); + +const optionsHandedToBackfill = () => { + expect(vi.mocked(backfillSummaryNulls)).toHaveBeenCalledTimes(1); + const options = vi.mocked(backfillSummaryNulls).mock.calls[0][2]; + expect(options).toBeDefined(); + return options!; +}; + +describe('os migrate summary-nulls', () => { + it('is a dry run by default — --apply is opt-in (#2186)', () => { + expect(MigrateSummaryNulls.flags.apply.default).toBe(false); + }); + + it('requires explicit confirmation to write — --yes is opt-in', () => { + expect(MigrateSummaryNulls.flags.yes.default).toBe(false); + }); + + it('declares --recompute-undefined-on-empty as a repeatable object.field list, and shows it in --help', () => { + const flag = MigrateSummaryNulls.flags['recompute-undefined-on-empty']; + expect(flag.multiple).toBe(true); + expect(flag.description).toContain('object.field'); + expect(flag.description).toMatch(/min\/max\/avg/); + expect(flag.description).toContain('never computed'); + expect(MigrateSummaryNulls.examples).toEqual( + expect.arrayContaining([expect.stringContaining('--recompute-undefined-on-empty customer.last_follow_up_at')]), + ); + }); + + it('hands every --recompute-undefined-on-empty entry to backfillSummaryNulls as recomputeUndefinedOnEmpty, in order (#15064)', async () => { + await MigrateSummaryNulls.run([ + '--json', + '--object', 'customer', + '--recompute-undefined-on-empty', 'customer.last_follow_up_at', + '--recompute-undefined-on-empty', 'customer.first_follow_up_at', + ], { root: CLI_ROOT }); + + expect(optionsHandedToBackfill()).toEqual({ + apply: false, + objects: ['customer'], + recomputeUndefinedOnEmpty: ['customer.last_follow_up_at', 'customer.first_follow_up_at'], + maxRecordsPerObject: undefined, + }); + }, RUN_TIMEOUT); + + it('without the flag the option is absent — the unscoped run the ruling keeps as it was', async () => { + await MigrateSummaryNulls.run(['--json'], { root: CLI_ROOT }); + + const options = optionsHandedToBackfill(); + expect(options.recomputeUndefinedOnEmpty).toBeUndefined(); + expect(options).toEqual({ apply: false, objects: undefined, recomputeUndefinedOnEmpty: undefined, maxRecordsPerObject: undefined }); + }, RUN_TIMEOUT); + + it('a refused scope entry (INVALID_FIELD) reaches the --json error envelope with its code, and the command exits 1', async () => { + const refusal = Object.assign(new Error('[summary-backfill] recomputeUndefinedOnEmpty names 1 roll-up(s) this run cannot find: customer.nope.'), { + code: 'INVALID_FIELD', status: 400, field: 'customer.nope', fields: ['customer.nope'], + }); + vi.mocked(backfillSummaryNulls).mockRejectedValue(refusal); + + const err = await MigrateSummaryNulls.run( + ['--json', '--recompute-undefined-on-empty', 'customer.nope'], + { root: CLI_ROOT }, + ).catch((e: unknown) => e); + + expect(isExitSignal(err)).toBe(true); + expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1); + const emitted = stdout.mock.calls.map((c: unknown[]) => String(c[0])).join(''); + const payload = JSON.parse(emitted); + expect(payload).toMatchObject({ code: 'INVALID_FIELD' }); + expect(payload.error).toContain('customer.nope'); + }, RUN_TIMEOUT); +}); diff --git a/packages/cli/src/commands/migrate/summary-nulls.ts b/packages/cli/src/commands/migrate/summary-nulls.ts index 65a70cb04e..68779b9340 100644 --- a/packages/cli/src/commands/migrate/summary-nulls.ts +++ b/packages/cli/src/commands/migrate/summary-nulls.ts @@ -57,7 +57,15 @@ async function confirm(question: string): Promise { * nothing left to do. * * `min`/`max`/`avg` are never touched — undefined on an empty set, so a `null` - * there is the correct reading of "no child rows", not a defect. + * there is the correct reading of "no child rows", not a defect — UNLESS the + * operator names one with `--recompute-undefined-on-empty object.field` + * (#15064). A summary field declared after its parent rows already existed has + * never been computed by anything (the insert-time seed is create-time, the + * recompute runs on a child write, and this run skips the function), and the + * one who just declared it is the one who knows that; named, the column is + * walked like a `count` and every `NULL` parent is recomputed through the same + * aggregate the engine writes. A parent with no child rows keeps `NULL` there — + * the aggregate's own value. Unnamed, nothing about this command changes. * * ## No deployment flag, deliberately * @@ -77,6 +85,7 @@ export default class MigrateSummaryNulls extends Command { '$ os migrate summary-nulls --apply', '$ os migrate summary-nulls --apply --yes --json', '$ os migrate summary-nulls --object project', + '$ os migrate summary-nulls --apply --object customer --recompute-undefined-on-empty customer.last_follow_up_at', ]; static override flags = { @@ -97,6 +106,14 @@ export default class MigrateSummaryNulls extends Command { description: 'Restrict to this object (repeatable; default: every object owning a count/sum roll-up)', multiple: true, }), + 'recompute-undefined-on-empty': Flags.string({ + description: + 'Also recompute this min/max/avg roll-up, spelled object.field (repeatable) — for a column you KNOW was never ' + + 'computed, e.g. one declared after its parent rows already existed. Every NULL parent is recomputed through ' + + 'the same aggregate the engine writes; a parent with no child rows keeps NULL. A name that is not a roll-up ' + + 'this run walks is refused before any row is read. Without this flag min/max/avg are never touched.', + multiple: true, + }), 'max-records': Flags.integer({ description: 'Safety bound on parent rows read per object — exceeding it truncates the walk', }), @@ -152,8 +169,13 @@ export default class MigrateSummaryNulls extends Command { this.exit(1); return; } + const named = flags['recompute-undefined-on-empty'] ?? []; const ok = await confirm( - chalk.bold('\nRecompute and write every NULL count/sum roll-up value on this database? [y/N] '), + chalk.bold( + '\nRecompute and write every NULL count/sum roll-up value' + + (named.length > 0 ? ` — and every NULL in ${named.join(', ')} —` : '') + + ' on this database? [y/N] ', + ), ); if (!ok) { printInfo('Aborted — no changes made.'); @@ -207,6 +229,7 @@ export default class MigrateSummaryNulls extends Command { const report = await backfillSummaryNulls(engine, logger, { apply, objects: flags.object, + recomputeUndefinedOnEmpty: flags['recompute-undefined-on-empty'], maxRecordsPerObject: flags['max-records'], }); diff --git a/packages/objectql/src/summary-backfill.test.ts b/packages/objectql/src/summary-backfill.test.ts index 150919aa62..55e1b05495 100644 --- a/packages/objectql/src/summary-backfill.test.ts +++ b/packages/objectql/src/summary-backfill.test.ts @@ -136,6 +136,7 @@ describe('backfillSummaryNulls — pre-#6013 NULL roll-ups (#6063)', () => { // NOT a defect. Deliberately out of this migration's scope. avg_estimate: { type: 'summary', summaryOperations: { object: 'task', field: 'estimate', function: 'avg' } }, max_estimate: { type: 'summary', summaryOperations: { object: 'task', field: 'estimate', function: 'max' } }, + min_estimate: { type: 'summary', summaryOperations: { object: 'task', field: 'estimate', function: 'min' } }, }, } as any); engine.registry.registerObject({ @@ -318,4 +319,266 @@ describe('backfillSummaryNulls — pre-#6013 NULL roll-ups (#6063)', () => { expect.arrayContaining([expect.stringContaining('could not be recomputed')]), ); }); + + describe('a roll-up column declared AFTER its parent rows existed (#15064)', () => { + // The card's own repro. Every row below is stored WITHOUT the min/max/avg + // columns — exactly what the database holds the moment a summary field is + // added to an object that already has rows: nothing has ever computed it, + // children or not (`initializeSummaryFields` is create-time and seeds + // nothing for these functions anyway; the recompute runs on a child + // write; the unscoped backfill skips the function). + const busyAndQuiet = () => { + legacyParent('p_busy'); + legacyTask('t1', 'p_busy', { estimate: 10, status: 'done' }); + legacyTask('t2', 'p_busy', { estimate: 32, status: 'todo' }); + legacyParent('p_quiet'); + }; + const maxOutcome = (report: Awaited>) => + report.fields.find((f) => f.field === 'max_estimate'); + + // What the UNSCOPED run produced for `busyAndQuiet()` on `origin/main` + // 791a0cbe6, captured BEFORE this option existed (one throw-away test that + // printed `JSON.stringify` of the report and the formatter lines). The + // ruling this option lands under is 「Without the scope the run behaves + // exactly as today」, so the unscoped pins below compare against these + // literals whole — the one delta a reader should find is the additive + // `recomputedUndefinedOnEmpty: []`, appended at the end of each report. + const BASE_DRY_LINES = [ + 'Scanned 2 parent row(s) across 1 object(s) for count/sum roll-up columns still stored as NULL.', + 'Would backfill 6 value(s) in 3 column(s):', + ' • project.task_count (count over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + ' • project.total_estimate (sum over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + ' • project.done_count (count over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + 'Rows "with real child data" are why this run recomputes instead of writing 0:', + 'their correct value is the aggregate over their children, not the empty-set value.', + '· Untouched by design (no empty-set value — a null there means "no child rows"): project.avg_estimate (avg), project.max_estimate (max), project.min_estimate (min)', + 'Dry run — nothing was written. Re-run with --apply to backfill.', + ]; + const BASE_APPLY_LINES = [ + 'Scanned 2 parent row(s) across 1 object(s) for count/sum roll-up columns still stored as NULL.', + 'Backfilled 6 value(s) in 3 column(s):', + ' • project.task_count (count over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + ' • project.total_estimate (sum over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + ' • project.done_count (count over task) — 2 NULL row(s), 1 with real child data\n e.g. p_busy, p_quiet', + 'Rows "with real child data" are why this run recomputes instead of writing 0:', + 'their correct value is the aggregate over their children, not the empty-set value.', + '· Untouched by design (no empty-set value — a null there means "no child rows"): project.avg_estimate (avg), project.max_estimate (max), project.min_estimate (min)', + ]; + const BASE_APPLY_REPORT = { + scannedObjects: ['project'], + scannedRecords: 2, + fields: [ + { object: 'project', field: 'task_count', fn: 'count', childObject: 'task', nullRows: 2, nonEmpty: 1, filled: 2, sampleRecordIds: ['p_busy', 'p_quiet'] }, + { object: 'project', field: 'total_estimate', fn: 'sum', childObject: 'task', nullRows: 2, nonEmpty: 1, filled: 2, sampleRecordIds: ['p_busy', 'p_quiet'] }, + { object: 'project', field: 'done_count', fn: 'count', childObject: 'task', nullRows: 2, nonEmpty: 1, filled: 2, sampleRecordIds: ['p_busy', 'p_quiet'] }, + ], + nullRows: 6, + filled: 6, + skippedUndefinedOnEmpty: ['project.avg_estimate (avg)', 'project.max_estimate (max)', 'project.min_estimate (min)'], + applied: true, + truncated: false, + unreadableObjects: [], + failures: [], + recomputedUndefinedOnEmpty: [], // the one additive key + }; + + it('UNSCOPED: the run is what it always was — report and wording byte-for-byte, the max still NULL and still listed as skipped', async () => { + busyAndQuiet(); + + const dry = await backfillSummaryNulls(engine, quietLogger, {}); + expect(formatSummaryBackfillReport(dry)).toEqual(BASE_DRY_LINES); + expect(d.writes).toEqual([]); + + const report = await backfillSummaryNulls(engine, quietLogger, { apply: true }); + + expect(report).toEqual(BASE_APPLY_REPORT); + expect(formatSummaryBackfillReport(report)).toEqual(BASE_APPLY_LINES); + // The card's measured symptom, unchanged by design: the just-declared + // max stays NULL on the parent that has children, reported under + // `skippedUndefinedOnEmpty` — and NOT under `fields`, so `filled` does + // not count it. + expect(project('p_busy')).toEqual({ id: 'p_busy', name: 'p_busy', task_count: 2, total_estimate: 42, done_count: 1 }); + expect(project('p_quiet')).toEqual({ id: 'p_quiet', name: 'p_quiet', task_count: 0, total_estimate: 0, done_count: 0 }); + expect(maxOutcome(report)).toBeUndefined(); + expect(d.writes.every((w) => !('max_estimate' in w.data))).toBe(true); + }); + + it('SCOPED: naming the max fills every parent that has children — through the aggregate the engine itself writes', async () => { + busyAndQuiet(); + + const report = await backfillSummaryNulls(engine, quietLogger, { + apply: true, + objects: ['project'], + recomputeUndefinedOnEmpty: ['project.max_estimate'], + }); + + expect(project('p_busy').max_estimate).toBe(32); + expect(report.recomputedUndefinedOnEmpty).toEqual(['project.max_estimate (max)']); + // A named count was never skipped, so it is not "recomputed on request" + // either, and a duplicate entry resolves once. + const named = await backfillSummaryNulls(engine, quietLogger, { + recomputeUndefinedOnEmpty: ['project.task_count', 'project.max_estimate', 'project.max_estimate'], + }); + expect(named.recomputedUndefinedOnEmpty).toEqual(['project.max_estimate (max)']); + // The scope is per column: the sibling avg/min stay out, and stay listed. + expect(report.skippedUndefinedOnEmpty).toEqual(['project.avg_estimate (avg)', 'project.min_estimate (min)']); + expect(maxOutcome(report)).toEqual({ + object: 'project', field: 'max_estimate', fn: 'max', childObject: 'task', + nullRows: 1, nonEmpty: 1, filled: 1, sampleRecordIds: ['p_busy'], + }); + expect(report.filled).toBe(BASE_APPLY_REPORT.filled + 1); + expect(d.writes.filter((w) => 'max_estimate' in w.data)).toEqual([ + { object: 'project', id: 'p_busy', data: expect.objectContaining({ max_estimate: 32 }) }, + ]); + // One definition of "what does this roll-up equal": the next child write + // moves the column exactly as the engine's own recompute would, from the + // value the backfill left there. + await engine.insert('task', { title: 't3', estimate: 40, status: 'todo', project: 'p_busy' }); + expect(project('p_busy').max_estimate).toBe(40); + }); + + it('SCOPED: a parent with no child rows keeps NULL — the aggregate\'s own reading, not a hole — neither counted nor written', async () => { + busyAndQuiet(); + + const report = await backfillSummaryNulls(engine, quietLogger, { + apply: true, + recomputeUndefinedOnEmpty: ['project.max_estimate'], + }); + + expect(project('p_quiet')).not.toHaveProperty('max_estimate'); // no write ever named it + expect(d.writes.filter((w) => w.id === 'p_quiet' && 'max_estimate' in w.data)).toEqual([]); + // Both parents were examined (the recompute ran for p_quiet and came back + // as the empty-set reading); only p_busy was a hole. + expect(report.scannedRecords).toBe(2); + expect(maxOutcome(report)).toMatchObject({ nullRows: 1, nonEmpty: 1, filled: 1, sampleRecordIds: ['p_busy'] }); + }); + + it('SCOPED: count control — a count fills identically with or without the scope, and naming one is accepted', async () => { + busyAndQuiet(); + + const report = await backfillSummaryNulls(engine, quietLogger, { + apply: true, + // A publish path passes every column it just declared, function unknown. + recomputeUndefinedOnEmpty: ['project.task_count', 'project.max_estimate'], + }); + + expect(project('p_busy').task_count).toBe(2); + expect(project('p_quiet').task_count).toBe(0); + // The count column's outcome is byte-identical to the unscoped run's — + // the control the scope pins above are read against: it asserts nothing + // the scope handling produces, so it stays green when that handling is + // ablated while the scoped pins go red. + expect(report.fields.find((f) => f.field === 'task_count')).toEqual( + BASE_APPLY_REPORT.fields.find((f) => f.field === 'task_count'), + ); + }); + + it('SCOPED: min, max and avg all compute through aggregateSummaryValue', async () => { + busyAndQuiet(); + + await backfillSummaryNulls(engine, quietLogger, { + apply: true, + recomputeUndefinedOnEmpty: ['project.min_estimate', 'project.max_estimate', 'project.avg_estimate'], + }); + + expect(project('p_busy')).toMatchObject({ min_estimate: 10, max_estimate: 32, avg_estimate: 21 }); + for (const field of ['min_estimate', 'max_estimate', 'avg_estimate']) { + expect(project('p_quiet')).not.toHaveProperty(field); + } + }); + + it('SCOPED: dry run reports what apply then writes, and writes nothing', async () => { + busyAndQuiet(); + + const dry = await backfillSummaryNulls(engine, quietLogger, { recomputeUndefinedOnEmpty: ['project.max_estimate'] }); + + expect(dry.applied).toBe(false); + expect(d.writes).toEqual([]); + expect(maxOutcome(dry)).toMatchObject({ nullRows: 1, nonEmpty: 1, filled: 0 }); + expect(project('p_busy')).not.toHaveProperty('max_estimate'); + + const applied = await backfillSummaryNulls(engine, quietLogger, { apply: true, recomputeUndefinedOnEmpty: ['project.max_estimate'] }); + expect(applied.nullRows).toBe(dry.nullRows); + expect(applied.filled).toBe(dry.nullRows); + expect(maxOutcome(applied)!.filled).toBe(maxOutcome(dry)!.nullRows); + }); + + it('SCOPED: idempotent — the second scoped run finds nothing and writes nothing', async () => { + busyAndQuiet(); + const scope = { apply: true, recomputeUndefinedOnEmpty: ['project.max_estimate'] }; + + const first = await backfillSummaryNulls(engine, quietLogger, scope); + expect(maxOutcome(first)!.filled).toBe(1); + const writesAfterFirst = d.writes.length; + + const second = await backfillSummaryNulls(engine, quietLogger, scope); + + // p_quiet's max is still NULL and is re-confirmed, not re-counted. + expect(second.nullRows).toBe(0); + expect(second.filled).toBe(0); + expect(second.fields).toEqual([]); + expect(second.recomputedUndefinedOnEmpty).toEqual(['project.max_estimate (max)']); + expect(d.writes.length).toBe(writesAfterFirst); + expect(formatSummaryBackfillReport(second)).toEqual( + expect.arrayContaining([expect.stringContaining('No NULL roll-up values found')]), + ); + }); + + it('SCOPED: never overwrites a max already stored', async () => { + legacyParent('p_imported', { max_estimate: 99 }); + legacyTask('t1', 'p_imported', { estimate: 10 }); + + const report = await backfillSummaryNulls(engine, quietLogger, { apply: true, recomputeUndefinedOnEmpty: ['project.max_estimate'] }); + + expect(project('p_imported').max_estimate).toBe(99); + expect(maxOutcome(report)).toBeUndefined(); + }); + + it('REFUSES a name it cannot resolve — INVALID_FIELD, 400, the code the projection and write axes naming a field answer — before any row is read, on a dry run as on apply', async () => { + busyAndQuiet(); + + const refusals: Array<[string[], Record]> = [ + [['project.nope'], { apply: true }], // no such field + [['max_estimate'], { apply: true }], // not spelled object.field + [['project.name'], { apply: true }], // a field, not a roll-up + [['task.max_estimate'], { apply: true }], // the child owns no roll-up + [['project.max_estimate'], { apply: true, objects: ['task'] }], // object left out of this run + [['project.max_estimate', 'project.nope'], {}], // one good, one bad, dry run: refused whole + ]; + for (const [named, rest] of refusals) { + const err = await backfillSummaryNulls(engine, quietLogger, { ...rest, recomputeUndefinedOnEmpty: named }).catch((e) => e); + expect(err, named.join(',')).toBeInstanceOf(Error); + expect(err.code, named.join(',')).toBe('INVALID_FIELD'); + expect(err.status, named.join(',')).toBe(400); + // The engine's sibling producers' shape: `field` is the first entry + // that did not resolve, `fields` every one of them. + expect(err.field, named.join(',')).toBe(named[named.length - 1]); + expect(err.fields, named.join(',')).toEqual([named[named.length - 1]]); + expect(err.message, named.join(',')).toContain(named[named.length - 1]); + } + // Refused BEFORE the walk: no write at all, and even the count/sum holes + // this run would otherwise have filled are still holes. + expect(d.writes).toEqual([]); + expect(project('p_busy').task_count ?? null).toBeNull(); + expect(project('p_busy')).not.toHaveProperty('max_estimate'); + }); + + it('the formatter names the scoped columns and explains a NULL that remains', async () => { + busyAndQuiet(); + + const report = await backfillSummaryNulls(engine, quietLogger, { apply: true, recomputeUndefinedOnEmpty: ['project.max_estimate'] }); + const lines = formatSummaryBackfillReport(report); + + expect(lines[0]).toBe( + 'Scanned 2 parent row(s) across 1 object(s) for count/sum roll-up columns (plus 1 named min/max/avg column(s)) still stored as NULL.', + ); + expect(lines).toEqual(expect.arrayContaining([ + expect.stringContaining(' • project.max_estimate (max over task) — 1 NULL row(s), 1 with real child data'), + expect.stringContaining('· Recomputed on request'), + expect.stringContaining('A parent with no child rows keeps NULL there'), + ])); + expect(lines.find((l) => l.startsWith('· Recomputed on request'))).toContain('project.max_estimate (max)'); + expect(lines.find((l) => l.startsWith('· Untouched by design'))).not.toContain('max_estimate'); + }); + }); }); diff --git a/packages/objectql/src/summary-backfill.ts b/packages/objectql/src/summary-backfill.ts index 131c1ab6a9..899c10e8b5 100644 --- a/packages/objectql/src/summary-backfill.ts +++ b/packages/objectql/src/summary-backfill.ts @@ -30,13 +30,35 @@ * own child-write recompute uses, over the identical descriptors the engine * maintains (maintainer ruling on #6063, 2026-08-07). * - * ## Scope: `count`/`sum` only + * ## Scope: `count`/`sum` by default * * A `NULL` `min`/`max`/`avg` is the LEGITIMATE reading of "no child rows" — * those aggregates are undefined on the empty set, which is exactly why #6013 - * leaves them `null` at insert. They are not a defect and are never written - * here; {@link summaryNullIsBackfillable} is the single predicate, derived from - * the same empty-set list. + * leaves them `null` at insert. They are not a defect and are not written + * here unless the caller says otherwise; {@link summaryNullIsBackfillable} is + * the single predicate, derived from the same empty-set list. + * + * ## …and, on request, a column the caller KNOWS was never computed (#15064) + * + * That predicate decides on the FUNCTION alone, so it cannot tell apart two + * states that are identical in storage: a `null` `max` meaning "no child rows", + * and a `null` `max` on a column declared AFTER its parent rows already existed + * — which nothing has ever computed (the insert-time seed is create-time, the + * recompute visits a parent only on a child write, and this run skipped the + * function). The narrowing is right for the hole it was written for; the defect + * was that the one caller holding the fact "this column is new" — the publish + * path that just declared it — had no way to say so. `recomputeUndefinedOnEmpty` + * names such roll-ups (`object.field`): a named `min`/`max`/`avg` is walked like + * a `count`, and every `NULL` parent is recomputed through the SAME + * {@link aggregateSummaryValue}. A parent whose aggregate comes back as the + * empty-set reading (`null` — no child rows) already holds the value the engine + * would write, so it is neither counted as a hole nor written; one whose + * aggregate is defined is filled. Unnamed, the run is exactly what it was, and + * `os migrate summary-nulls` keeps its meaning on every deployment (maintainer + * ruling on #15064, 2026-09-05: a caller-supplied scope, not a relaxed + * predicate). A name that resolves to no roll-up this run walks is REFUSED + * before any row is read: the caller's fact could not be acted on, and a silent + * no-op there is exactly the false all-clear #15064 was filed over. * * ## Driver-agnostic, and no `IS NULL` pushdown * @@ -52,7 +74,10 @@ * * Every write turns a `NULL` into a number, so the second run's walk finds no * `NULL` rows and writes nothing. A partially-completed run is safe to repeat, - * and "re-run until it reports zero" is the operator's own verification. + * and "re-run until it reports zero" is the operator's own verification. The + * same holds under `recomputeUndefinedOnEmpty`: a childless parent's `null` + * `max` is re-examined and re-confirmed on every run, never counted or written, + * so a scoped re-run reports zero too. */ import { keysetWalk, type KeysetPageQuery } from '@objectstack/types'; @@ -68,13 +93,20 @@ import { export interface SummaryBackfillFieldOutcome { object: string; field: string; - fn: 'count' | 'sum'; + /** `min`/`max`/`avg` appear here only when named in + * `recomputeUndefinedOnEmpty` (#15064); an unscoped run lists `count`/`sum`. */ + fn: SummaryDescriptor['fn']; /** The child object aggregated — so a report names the whole relationship. */ childObject: string; - /** Parent rows found holding `NULL` in this column. */ + /** Parent rows found holding `NULL` in this column that this run fills. For a + * `min`/`max`/`avg` named in `recomputeUndefinedOnEmpty`, a `NULL` whose + * recompute is the empty-set reading (`null`) is not a hole and is not + * counted — the row already holds the engine's own value. */ nullRows: number; /** Of those, the ones whose recomputed value is a real aggregate (> 0 rows - * of children) — the rows a `SET col = 0` shortcut would have corrupted. */ + * of children) — the rows a `SET col = 0` shortcut would have corrupted. A + * defined `min`/`max`/`avg` implies a child row, so under the scope every + * counted row is one of these. */ nonEmpty: number; /** Values written (equal to `nullRows` on an apply run with no failures; * always 0 on a dry run — a dry run writes nothing). */ @@ -96,7 +128,10 @@ export interface SummaryBackfillReport { scannedObjects: string[]; scannedRecords: number; fields: SummaryBackfillFieldOutcome[]; - /** Parent rows found holding `NULL` in a `count`/`sum` roll-up. */ + /** Parent rows found holding `NULL` in a roll-up this run fills — every + * `count`/`sum`, plus the `min`/`max`/`avg` named in + * `recomputeUndefinedOnEmpty` (there, a `NULL` whose recompute is the + * empty-set reading is not a hole and is not counted). */ nullRows: number; /** Values actually written (0 on a dry run). */ filled: number; @@ -107,6 +142,13 @@ export interface SummaryBackfillReport { * narrowing is a decision an operator should be able to SEE. */ skippedUndefinedOnEmpty: string[]; + /** + * The complement of `skippedUndefinedOnEmpty`, in the same `object.field (fn)` + * spelling: the `min`/`max`/`avg` roll-ups the caller named in + * `recomputeUndefinedOnEmpty` and this run therefore walked like a `count` + * (#15064). Empty on an unscoped run. + */ + recomputedUndefinedOnEmpty: string[]; /** False on a dry run — no writes were made. */ applied: boolean; /** A per-object cap or an unreadable object cut the walk short. */ @@ -137,6 +179,22 @@ export interface SummaryBackfillOptions { apply?: boolean; /** Restrict to these objects (default: every object owning a roll-up). */ objects?: string[]; + /** + * `object.field` roll-ups the caller KNOWS have never been computed — a + * column declared after its parent rows already existed — so a stored `null` + * there is a hole whatever the function (#15064). A `min`/`max`/`avg` named + * here is recomputed for every `NULL` parent through the same aggregate the + * engine writes, instead of being reported under `skippedUndefinedOnEmpty`; + * a parent with no child rows keeps its `null`, which is already the engine's + * value for it. Naming a `count`/`sum` is accepted and changes nothing — + * those are in scope by default — so a caller can pass every column it just + * declared without knowing the empty-set list. A name that resolves to no + * roll-up owned by an object this run walks is refused (`INVALID_FIELD`, + * 400 — the code the projection and write axes that name a field answer; + * sorting keeps `INVALID_SORT`) before any row is read. Omitted, the run is + * exactly the `count`/`sum` backfill it always was. + */ + recomputeUndefinedOnEmpty?: string[]; /** Safety bound on parent rows read per object. */ maxRecordsPerObject?: number; /** Retry policy for one row's aggregate + update; defaults to the transient @@ -151,33 +209,109 @@ const MAX_SAMPLE_IDS = 5; * tenant-scope this" — the same context the other `os migrate` data runs use. */ const SYSTEM_CTX = { isSystem: true } as const; -/** The `count`/`sum` roll-ups `object` owns, and the `min`/`max`/`avg` ones it - * owns that are deliberately out of scope. */ -function partitionDescriptors( - engine: SummaryBackfillEngine, - object: string, -): { backfillable: SummaryDescriptor[]; skipped: string[] } { - let owned: SummaryDescriptor[] = []; +/** `object.field` — the spelling `recomputeUndefinedOnEmpty` takes, and the + * key a named roll-up is resolved by. */ +const rollupKey = (desc: SummaryDescriptor): string => `${desc.parentObject}.${desc.summaryField}`; +/** `object.field (fn)` — the spelling both report lists share. */ +const rollupLabel = (desc: SummaryDescriptor): string => `${rollupKey(desc)} (${desc.fn})`; + +/** The engine's OWN roll-up index for `object`; an index that cannot be read + * contributes nothing rather than aborting the run. */ +function ownedDescriptors(engine: SummaryBackfillEngine, object: string): SummaryDescriptor[] { try { - owned = engine.getOwnedSummaryDescriptors(object) ?? []; + return engine.getOwnedSummaryDescriptors(object) ?? []; } catch { - owned = []; + return []; } +} + +/** The roll-ups `object` owns that this run walks — every `count`/`sum`, plus + * the `min`/`max`/`avg` the caller named in `recomputeUndefinedOnEmpty` — and + * the `min`/`max`/`avg` it owns that stay out of scope. */ +function partitionDescriptors( + engine: SummaryBackfillEngine, + object: string, + scope: ReadonlySet, +): { backfillable: SummaryDescriptor[]; skipped: string[]; recomputed: string[] } { const backfillable: SummaryDescriptor[] = []; const skipped: string[] = []; - for (const desc of owned) { + const recomputed: string[] = []; + for (const desc of ownedDescriptors(engine, object)) { if (summaryNullIsBackfillable(desc.fn)) backfillable.push(desc); - else skipped.push(`${desc.parentObject}.${desc.summaryField} (${desc.fn})`); + else if (scope.has(rollupKey(desc))) { + backfillable.push(desc); + recomputed.push(rollupLabel(desc)); + } else skipped.push(rollupLabel(desc)); } - return { backfillable, skipped }; + return { backfillable, skipped, recomputed }; } /** - * Recompute every `count`/`sum` roll-up column still stored as `NULL`. + * Resolve `recomputeUndefinedOnEmpty` against the roll-ups owned by the objects + * this run walks, BEFORE any row is read (#15064). + * + * Every entry must name a summary field the engine's own index owns on one of + * `candidates`; an entry naming a `count`/`sum` resolves too (already in scope + * — nothing to add). One that resolves to nothing — a typo, a field that is not + * a roll-up, or an object `options.objects` left out of this run — is REFUSED + * as a whole, with nothing written: the caller asserted a fact about a column + * this run cannot see, and quietly walking the rest would hand back the + * `filled: 0` false all-clear this option exists to end. + * + * `INVALID_FIELD` / 400 is the envelope — not a new code, and not a 404. The + * engine's own rule for a field name that cannot be applied as written + * (`assertProjectionHasNoDottedPaths` for `select`, + * `undeclaredWriteFieldErrors` on a write, and `INVALID_SORT` beside them) is + * that ONE condition keeps ONE wire code however the caller reached it, so a + * host surfacing this error over HTTP answers the same envelope on both doors. + * Two of the shapes refused here name a field that EXISTS — a real non-summary + * field, or a roll-up on an object `objects` left out — so this is an option + * value that could not be applied, never an addressed resource that was not + * found (`FIELD_NOT_FOUND` has no producer in this repo, and gains none here). + * `field` carries the first unresolved entry and `fields` all of them, the + * sibling producers' shape (ADR-0112). + */ +function resolveRecomputeScope( + engine: SummaryBackfillEngine, + candidates: string[], + named: string[] | undefined, +): Set { + const scope = new Set(); + if (!named || named.length === 0) return scope; + const owned = new Set(); + for (const object of candidates) { + for (const desc of ownedDescriptors(engine, object)) owned.add(rollupKey(desc)); + } + const unresolved: string[] = []; + for (const entry of named) { + if (owned.has(entry)) scope.add(entry); + else if (!unresolved.includes(entry)) unresolved.push(entry); + } + if (unresolved.length > 0) { + const err = new Error( + `[summary-backfill] recomputeUndefinedOnEmpty names ${unresolved.length} roll-up(s) this run cannot find: ` + + `${unresolved.join(', ')}. Each entry is spelled object.field and must name a summary field owned by one ` + + `of the ${candidates.length} object(s) this run walks (an object left out by \`objects\` is not walked). ` + + 'Refused before any row was read; nothing was written.', + ) as Error & { code: string; status: number; field: string; fields: string[] }; + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = unresolved[0]; + err.fields = unresolved; + throw err; + } + return scope; +} + +/** + * Recompute every `count`/`sum` roll-up column still stored as `NULL` — and, + * for the `min`/`max`/`avg` roll-ups named in `options.recomputeUndefinedOnEmpty`, + * every `NULL` parent whose aggregate is defined (#15064). * * Dry run unless `options.apply` — and a dry run writes nothing at all, so the * report can be reviewed (and diffed against what actually happened) before any - * row changes. + * row changes. An unresolvable `recomputeUndefinedOnEmpty` entry throws before + * the walk starts, on a dry run as on an apply run. */ export async function backfillSummaryNulls( engine: SummaryBackfillEngine, @@ -190,17 +324,23 @@ export async function backfillSummaryNulls( options.objects ?? (typeof engine.getConfigs === 'function' ? Object.keys(engine.getConfigs()) : []); + // Resolved (and refused) BEFORE the walk: an apply run must not write half + // the deployment and then discover the caller named a column it cannot see. + const scope = resolveRecomputeScope(engine, candidates, options.recomputeUndefinedOnEmpty); + const scannedObjects: string[] = []; const outcomes = new Map(); const skippedUndefinedOnEmpty: string[] = []; + const recomputedUndefinedOnEmpty: string[] = []; const unreadableObjects: string[] = []; const failures: SummaryBackfillFailure[] = []; let scannedRecords = 0; let truncated = false; for (const object of candidates) { - const { backfillable, skipped } = partitionDescriptors(engine, object); + const { backfillable, skipped, recomputed } = partitionDescriptors(engine, object, scope); skippedUndefinedOnEmpty.push(...skipped); + recomputedUndefinedOnEmpty.push(...recomputed); if (backfillable.length === 0) continue; scannedObjects.push(object); @@ -208,7 +348,7 @@ export async function backfillSummaryNulls( outcomes.set(`${object}.${desc.summaryField}`, { object, field: desc.summaryField, - fn: desc.fn as 'count' | 'sum', + fn: desc.fn, childObject: desc.childObject, nullRows: 0, nonEmpty: 0, @@ -243,9 +383,14 @@ export async function backfillSummaryNulls( if (row[desc.summaryField] != null) continue; const outcome = outcomes.get(`${object}.${desc.summaryField}`)!; outcome.nullRows++; - if (outcome.sampleRecordIds.length < MAX_SAMPLE_IDS) { - outcome.sampleRecordIds.push(String(parentId)); - } + const sampled = outcome.sampleRecordIds.length < MAX_SAMPLE_IDS; + if (sampled) outcome.sampleRecordIds.push(String(parentId)); + // True only for a `min`/`max`/`avg` the caller named in + // `recomputeUndefinedOnEmpty` (#15064) — nothing else undefined on + // the empty set reaches this loop. Its empty-set reading is `null`, + // which is what the row already holds, so an aggregate over no + // child rows is nothing to write, and not a hole either. + const undefinedOnEmpty = !summaryNullIsBackfillable(desc.fn); try { // Counters are bumped AFTER the retry settles, never inside the // retried closure — a retried attempt would otherwise count the @@ -256,13 +401,24 @@ export async function backfillSummaryNulls( engine, desc, String(parentId), { ...SYSTEM_CTX }, ); if (!apply) return; + if (undefinedOnEmpty && computed == null) return; await engine.update( desc.parentObject, { id: parentId, [desc.summaryField]: computed }, { context: { ...SYSTEM_CTX } }, ); }, options.retry ?? {}); - if (typeof computed === 'number' && computed !== 0) outcome.nonEmpty++; + if (undefinedOnEmpty && computed == null) { + // Reclassified: the stored `null` IS the engine's value for this + // parent. Undo the count and the sample taken above. + outcome.nullRows--; + if (sampled) outcome.sampleRecordIds.pop(); + continue; + } + // A defined `min`/`max`/`avg` implies at least one child row by + // construction; `count`/`sum` keep the `!== 0` reading the report + // has always used. + if (undefinedOnEmpty || (typeof computed === 'number' && computed !== 0)) outcome.nonEmpty++; if (apply) outcome.filled++; } catch (err) { // One row's failure must not abort the rest — the same rule the @@ -305,6 +461,7 @@ export async function backfillSummaryNulls( nullRows: fields.reduce((n, f) => n + f.nullRows, 0), filled: fields.reduce((n, f) => n + f.filled, 0), skippedUndefinedOnEmpty, + recomputedUndefinedOnEmpty, applied: apply, truncated, unreadableObjects, @@ -315,12 +472,16 @@ export async function backfillSummaryNulls( /** Human-readable report body, shared by the CLI's dry-run and apply output. */ export function formatSummaryBackfillReport(report: SummaryBackfillReport): string[] { const lines: string[] = []; + // Unscoped wording is byte-identical to what it always was; the scoped run + // (#15064) says which extra columns it walked and why a NULL may remain. + const named = report.recomputedUndefinedOnEmpty.length; lines.push( `Scanned ${report.scannedRecords} parent row(s) across ${report.scannedObjects.length} object(s) ` + - 'for count/sum roll-up columns still stored as NULL.', + `for count/sum roll-up columns${named > 0 ? ` (plus ${named} named min/max/avg column(s))` : ''} ` + + 'still stored as NULL.', ); if (report.fields.length === 0) { - lines.push('✓ No NULL count/sum roll-up values found — nothing to backfill.'); + lines.push(`✓ No NULL ${named > 0 ? '' : 'count/sum '}roll-up values found — nothing to backfill.`); } else { const verb = report.applied ? 'Backfilled' : 'Would backfill'; lines.push(`${verb} ${report.applied ? report.filled : report.nullRows} value(s) in ${report.fields.length} column(s):`); @@ -344,6 +505,13 @@ export function formatSummaryBackfillReport(report: SummaryBackfillReport): stri report.skippedUndefinedOnEmpty.join(', '), ); } + if (report.recomputedUndefinedOnEmpty.length > 0) { + lines.push( + '· Recomputed on request (named as never computed — a column declared after its parent rows existed): ' + + report.recomputedUndefinedOnEmpty.join(', ') + + '\n A parent with no child rows keeps NULL there: that is the aggregate\'s own value, not a hole.', + ); + } if (report.failures.length > 0) { lines.push(`✗ ${report.failures.length} row(s) could not be recomputed:`); for (const f of report.failures.slice(0, MAX_SAMPLE_IDS)) {