Skip to content

Commit f6ed5b8

Browse files
committed
fix(objectql,cli): the recomputeUndefinedOnEmpty refusal answers INVALID_FIELD / 400 — the code every axis naming a field already takes (#15064)
Contract-review patch round on PR #15708 (verdict 5550099353), items 1, 3, 4. - `resolveRecomputeScope` stamps `code = 'INVALID_FIELD'`, `status = 400` and `field = unresolved[0]` beside `fields`, the shape of the engine's sibling producers (`assertProjectionHasNoDottedPaths`, `undeclaredWriteFieldErrors`). `FIELD_NOT_FOUND` / 404 had no producer in this repo; handing a never-emitted 404 its first producer for a condition the catalog already assigns to a 400 is the two-codes-one-condition drift ADR-0112 exists to prevent — and two of the refused shapes name a field that exists (a real non-summary field; a roll-up on an object `objects` left out), so this is an option value that could not be applied, not an addressed resource that was not found. - Pins retargeted: `summary-backfill.test.ts` "REFUSES a name it cannot resolve" (six code/status assertions, now also `field`/`fields`) and `summary-nulls.test.ts` "a refused scope entry … reaches the --json error envelope" (`code`). - `content/docs/api/error-catalog.mdx`: the `INVALID_FIELD` Cause gains the `recomputeUndefinedOnEmpty` / `--recompute-undefined-on-empty` axis; the `FIELD_NOT_FOUND` entry is untouched. - `SummaryBackfillReport.nullRows` TSDoc counts the named min/max/avg holes. - Option TSDoc, resolver docblock and the changeset paragraph updated wherever FIELD_NOT_FOUND / 404 was named. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
1 parent 7c96f73 commit f6ed5b8

5 files changed

Lines changed: 46 additions & 19 deletions

File tree

‎.changeset/summary-backfill-recompute-undefined-on-empty.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ the fact gets a way to say it; the predicate and the default run do not move.
3030
- A name that resolves to no roll-up owned by an object the run walks — a typo,
3131
a plain field, or an object `objects` left out — is **refused before any row
3232
is read**, dry run or apply, with an ADR-0112 envelope (`code:
33-
'FIELD_NOT_FOUND'`, `status: 404`). A silent no-op there would be the same
34-
false all-clear this option exists to end.
33+
'INVALID_FIELD'`, `status: 400` — the code every other axis that names a
34+
field already answers; `field` names the first unresolved entry, `fields`
35+
all of them). A silent no-op there would be the same false all-clear this
36+
option exists to end.
3537
- `SummaryBackfillReport.recomputedUndefinedOnEmpty: string[]` — the complement
3638
of `skippedUndefinedOnEmpty`, same `object.field (fn)` spelling; `[]` on an
3739
unscoped run. `SummaryBackfillFieldOutcome.fn` widens from `'count' | 'sum'`

‎content/docs/api/error-catalog.mdx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ reads those as field filters, so one naming no field could only match zero
7575
records and is rejected rather than answered with an empty page — plus every
7676
other read axis that names a field: `select`, `expand` (a real field that holds
7777
no reference gets its own message), `searchFields` (a real field outside the
78-
searchable set gets its own message), `groupBy`, and `aggregations[].field`.
78+
searchable set gets its own message), `groupBy`, and `aggregations[].field`.
79+
Off the request path the same code answers `backfillSummaryNulls`'s
80+
`recomputeUndefinedOnEmpty` — `os migrate summary-nulls
81+
--recompute-undefined-on-empty object.field` — when an entry is not a roll-up
82+
owned by an object the run walks (a real non-summary field, or a roll-up on an
83+
object `--object` left out, gets its own message).
7984
**Fix:** Check the object schema for valid field names. Use `os meta get object <name>` to inspect the object's fields. If the name was meant as a
8085
*parameter* rather than a field, use the real one — page size is `top` / `$top`
8186
/ `limit`, not `pageSize` / `perPage`; the response's `error` names the

‎packages/cli/src/commands/migrate/summary-nulls.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ describe('os migrate summary-nulls', () => {
129129
expect(options).toEqual({ apply: false, objects: undefined, recomputeUndefinedOnEmpty: undefined, maxRecordsPerObject: undefined });
130130
}, RUN_TIMEOUT);
131131

132-
it('a refused scope entry (FIELD_NOT_FOUND) reaches the --json error envelope with its code, and the command exits 1', async () => {
132+
it('a refused scope entry (INVALID_FIELD) reaches the --json error envelope with its code, and the command exits 1', async () => {
133133
const refusal = Object.assign(new Error('[summary-backfill] recomputeUndefinedOnEmpty names 1 roll-up(s) this run cannot find: customer.nope.'), {
134-
code: 'FIELD_NOT_FOUND', status: 404, fields: ['customer.nope'],
134+
code: 'INVALID_FIELD', status: 400, field: 'customer.nope', fields: ['customer.nope'],
135135
});
136136
vi.mocked(backfillSummaryNulls).mockRejectedValue(refusal);
137137

@@ -144,7 +144,7 @@ describe('os migrate summary-nulls', () => {
144144
expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1);
145145
const emitted = stdout.mock.calls.map((c: unknown[]) => String(c[0])).join('');
146146
const payload = JSON.parse(emitted);
147-
expect(payload).toMatchObject({ code: 'FIELD_NOT_FOUND' });
147+
expect(payload).toMatchObject({ code: 'INVALID_FIELD' });
148148
expect(payload.error).toContain('customer.nope');
149149
}, RUN_TIMEOUT);
150150
});

‎packages/objectql/src/summary-backfill.test.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ describe('backfillSummaryNulls — pre-#6013 NULL roll-ups (#6063)', () => {
534534
expect(maxOutcome(report)).toBeUndefined();
535535
});
536536

537-
it('REFUSES a name it cannot resolve — FIELD_NOT_FOUND, 404 — before any row is read, on a dry run as on apply', async () => {
537+
it('REFUSES a name it cannot resolve — INVALID_FIELD, 400, the code every axis naming a field answers — before any row is read, on a dry run as on apply', async () => {
538538
busyAndQuiet();
539539

540540
const refusals: Array<[string[], Record<string, unknown>]> = [
@@ -548,8 +548,12 @@ describe('backfillSummaryNulls — pre-#6013 NULL roll-ups (#6063)', () => {
548548
for (const [named, rest] of refusals) {
549549
const err = await backfillSummaryNulls(engine, quietLogger, { ...rest, recomputeUndefinedOnEmpty: named }).catch((e) => e);
550550
expect(err, named.join(',')).toBeInstanceOf(Error);
551-
expect(err.code, named.join(',')).toBe('FIELD_NOT_FOUND');
552-
expect(err.status, named.join(',')).toBe(404);
551+
expect(err.code, named.join(',')).toBe('INVALID_FIELD');
552+
expect(err.status, named.join(',')).toBe(400);
553+
// The engine's sibling producers' shape: `field` is the first entry
554+
// that did not resolve, `fields` every one of them.
555+
expect(err.field, named.join(',')).toBe(named[named.length - 1]);
556+
expect(err.fields, named.join(',')).toEqual([named[named.length - 1]]);
553557
expect(err.message, named.join(',')).toContain(named[named.length - 1]);
554558
}
555559
// Refused BEFORE the walk: no write at all, and even the count/sum holes

‎packages/objectql/src/summary-backfill.ts‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,10 @@ export interface SummaryBackfillReport {
128128
scannedObjects: string[];
129129
scannedRecords: number;
130130
fields: SummaryBackfillFieldOutcome[];
131-
/** Parent rows found holding `NULL` in a `count`/`sum` roll-up. */
131+
/** Parent rows found holding `NULL` in a roll-up this run fills — every
132+
* `count`/`sum`, plus the `min`/`max`/`avg` named in
133+
* `recomputeUndefinedOnEmpty` (there, a `NULL` whose recompute is the
134+
* empty-set reading is not a hole and is not counted). */
132135
nullRows: number;
133136
/** Values actually written (0 on a dry run). */
134137
filled: number;
@@ -186,9 +189,10 @@ export interface SummaryBackfillOptions {
186189
* value for it. Naming a `count`/`sum` is accepted and changes nothing —
187190
* those are in scope by default — so a caller can pass every column it just
188191
* declared without knowing the empty-set list. A name that resolves to no
189-
* roll-up owned by an object this run walks is refused (`FIELD_NOT_FOUND`,
190-
* 404) before any row is read. Omitted, the run is exactly the `count`/`sum`
191-
* backfill it always was.
192+
* roll-up owned by an object this run walks is refused (`INVALID_FIELD`,
193+
* 400 — the code every other axis that names a field answers) before any
194+
* row is read. Omitted, the run is exactly the `count`/`sum` backfill it
195+
* always was.
192196
*/
193197
recomputeUndefinedOnEmpty?: string[];
194198
/** Safety bound on parent rows read per object. */
@@ -252,9 +256,20 @@ function partitionDescriptors(
252256
* a roll-up, or an object `options.objects` left out of this run — is REFUSED
253257
* as a whole, with nothing written: the caller asserted a fact about a column
254258
* this run cannot see, and quietly walking the rest would hand back the
255-
* `filled: 0` false all-clear this option exists to end. `FIELD_NOT_FOUND`
256-
* (ADR-0112 standard catalog, 404) is the envelope: the named field is what
257-
* could not be found.
259+
* `filled: 0` false all-clear this option exists to end.
260+
*
261+
* `INVALID_FIELD` / 400 is the envelope — not a new code, and not a 404. The
262+
* engine's own rule for a field name that cannot be applied as written
263+
* (`assertProjectionHasNoDottedPaths` for `select`,
264+
* `undeclaredWriteFieldErrors` on a write, and `INVALID_SORT` beside them) is
265+
* that ONE condition keeps ONE wire code however the caller reached it, so a
266+
* host surfacing this error over HTTP answers the same envelope on both doors.
267+
* Two of the shapes refused here name a field that EXISTS — a real non-summary
268+
* field, or a roll-up on an object `objects` left out — so this is an option
269+
* value that could not be applied, never an addressed resource that was not
270+
* found (`FIELD_NOT_FOUND` has no producer in this repo, and gains none here).
271+
* `field` carries the first unresolved entry and `fields` all of them, the
272+
* sibling producers' shape (ADR-0112).
258273
*/
259274
function resolveRecomputeScope(
260275
engine: SummaryBackfillEngine,
@@ -278,9 +293,10 @@ function resolveRecomputeScope(
278293
`${unresolved.join(', ')}. Each entry is spelled object.field and must name a summary field owned by one ` +
279294
`of the ${candidates.length} object(s) this run walks (an object left out by \`objects\` is not walked). ` +
280295
'Refused before any row was read; nothing was written.',
281-
) as Error & { code: string; status: number; fields: string[] };
282-
err.code = 'FIELD_NOT_FOUND';
283-
err.status = 404;
296+
) as Error & { code: string; status: number; field: string; fields: string[] };
297+
err.code = 'INVALID_FIELD';
298+
err.status = 400;
299+
err.field = unresolved[0];
284300
err.fields = unresolved;
285301
throw err;
286302
}

0 commit comments

Comments
 (0)