diff --git a/.changeset/6825-aggregate-spec-shape-strict-where.md b/.changeset/6825-aggregate-spec-shape-strict-where.md new file mode 100644 index 000000000..17c3d35f2 --- /dev/null +++ b/.changeset/6825-aggregate-spec-shape-strict-where.md @@ -0,0 +1,56 @@ +--- +'@object-ui/data-objectstack': minor +--- + +`aggregate()`'s spec-shape branch now REFUSES an unlowered `where` instead of +posting it (objectui#6825, maintainer ruling 2026-08-30 — option A). + +**Breaking for callers that were already broken, so read this if you call +`aggregate()` with a `where`.** `aggregate()` has two branches. The analytics +branch takes `filter` and lowers a rule-shaped array before the wire (#6302). +The spec-shape branch — entered when `params` carries an array `groupBy`, an +array `aggregations`, or ANY `where` key — takes `where` and posts it to +`POST /data/:object/query` verbatim. It never lowered, and it still does not: +it now says so. + +**What now throws that previously went through.** A `where` that is an ARRAY +the spec's own `isFilterAST` gate rejects — an unlowered +`[{ field, operator, value }, ...]` above all, plus the infix join dialect +(`[condA, 'or', condB]`), a tuple whose operator is outside the AST vocabulary, +`['and']` with nothing to join, and an element that is not a condition. The +throw is an `UnloweredAggregateWhereError` (exported), carrying the same +`code: 'INVALID_FILTER'` / `httpStatus: 400` pair as `MalformedFilterError`, so +`isMalformedFilterError()` recognises it and a failed widget renders "this +filter is malformed" rather than "check your connection". + +**This adds no new failure — it relocates one.** Every shape now refused is one +the receiving engine already refused (`is not a filter`, 400 `INVALID_FILTER`, +before the store is touched). What changed is WHERE you find out and whether you +can act on it: previously the predicate was lost on the wire — or dropped +outright, leaving a chart rendering confident, wrong numbers with no signal to +its author. The refusal is now raised at the producer, names the value it +received, names the shape expected and where the spec declares it, and says +nothing was sent. + +**What an affected caller should send instead.** Lower the rules to a filter AST +BEFORE calling `aggregate()`: + +```diff +- adapter.aggregate('opportunity', { +- groupBy: ['stage'], aggregations: [...], +- where: [{ field: 'stage', operator: 'equals', value: 'won' }], +- }) ++ adapter.aggregate('opportunity', { ++ groupBy: ['stage'], aggregations: [...], ++ where: ['stage', '=', 'won'], ++ }) +``` + +Or keep using the analytics branch, which lowers for you: pass the rules under +`filter` with the legacy `field` / `function` / `groupBy` params. + +**Unchanged, deliberately.** The analytics branch still lowers `filter` exactly +as #6302 left it. On the spec-shape branch a `FilterCondition` object +(`{ stage: 'won' }` — what `QuerySchema.where` actually declares), an empty +array (`[]` is "no filter", and the engine agrees), and every already-valid +filter AST all reach `client.data.query` byte-unchanged. diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index bd3b07e4a..da20ba772 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -175,6 +175,43 @@ one condition of an `and` would widen the result set and report success. Non-array filters are passed through unchanged on the aggregate path: a MongoDB-style object is already what `/analytics/query` accepts. +#### `aggregate({ where })` does NOT lower — it refuses + +The lowering above is the `filter` parameter, on the analytics path. +`aggregate()` has a second, spec-shape branch — entered when the params carry +an array `groupBy`, an array `aggregations`, or **any** `where` key — which +posts `where` to `POST /data/:object/query` verbatim. That `where` is the spec +Query DSL's `where`, so it must ALREADY be lowered, and since objectui#6825 an +array that the spec's own `isFilterAST` gate rejects is refused here rather +than shipped: + +```typescript +// ✅ lowered — reaches the wire unchanged +await dataSource.aggregate('opportunity', { + groupBy: ['stage'], + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], + where: ['stage', '=', 'won'], +}); + +// ⛔ throws UnloweredAggregateWhereError — authoring sugar, not a filter +await dataSource.aggregate('opportunity', { + groupBy: ['stage'], + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], + where: [{ field: 'stage', operator: 'equals', value: 'won' }], +}); +``` + +This is a producer-side refusal of a value the server already refused (`is not +a filter`, `400 INVALID_FILTER`): the point is that you now find out at the call +site, with the offending value named, instead of on the wire — or not at all, +with a chart quietly rendering unfiltered numbers. Lower the rules in whatever +built the params, or use the analytics branch (`filter` + the legacy `field` / +`function` / `groupBy` params), which lowers for you. + +Two shapes are deliberately NOT refused, because the receiving door accepts +them: a `FilterCondition` object (`{ stage: 'won' }` — what `QuerySchema.where` +declares), and an empty array (`[]` means "no filter"). + ### Sorting ```typescript @@ -305,6 +342,16 @@ import { // SYMBOL is prefixed because @objectstack/spec/kernel // owns `ValidationError` for a { field, message, code? } // record (objectui#3160). + MalformedFilterError, // A filter rule that cannot be translated (400 + // INVALID_FILTER) — thrown rather than dropped, + // because dropping one condition of an `and` widens + // the result set and reports success. + UnloweredAggregateWhereError, // aggregate()'s spec-shape `where` was an array + // the spec's filter-AST gate rejects (400 + // INVALID_FILTER). See "aggregate({ where }) does + // NOT lower" above. + isMalformedFilterError, // Recognises BOTH of the two above, and the server's + // own version of the same refusal. } from '@object-ui/data-objectstack'; ``` @@ -362,6 +409,8 @@ All errors include unique error codes for programmatic handling: - `CONNECTION_ERROR` - Connection/network error - `AUTHENTICATION_ERROR` - Authentication failure - `VALIDATION_ERROR` - Data validation error +- `INVALID_FILTER` - A filter the adapter refuses to send (`MalformedFilterError`, + `UnloweredAggregateWhereError`); matches the data API's own code for the same refusal - `UNSUPPORTED_OPERATION` - Unsupported operation - `NOT_FOUND` - Resource not found - `UNKNOWN_ERROR` - Unknown error diff --git a/packages/data-objectstack/src/aggregate-spec-shape-where.test.ts b/packages/data-objectstack/src/aggregate-spec-shape-where.test.ts new file mode 100644 index 000000000..da6a14629 --- /dev/null +++ b/packages/data-objectstack/src/aggregate-spec-shape-where.test.ts @@ -0,0 +1,342 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `aggregate()`'s SPEC-SHAPE branch keeps `where` strict and refuses at the + * producer (objectui#6825, maintainer ruling 2026-08-30 — option A). + * + * WHY THIS FILE EXISTS. `aggregate()` has two branches and they take a filter + * by different names. The analytics branch takes `filter` and, since #6302, + * lowers a rule-shaped array through `translateFilterArray` before posting it + * to `/analytics/query`. The spec-shape branch takes `where` and posts it to + * `POST /data/:object/query` VERBATIM — no lowering, before or after #6302. One + * authored chart can reach either branch, and which one it reaches is decided + * by whether it carries `groupBy`/`aggregations` — a property with nothing to + * do with its filter. + * + * The card's census found a STRUCTURAL zero in this repo: no producer, fixture + * or test can reach the spec-shape branch with a rule-shaped `where` + * (`ObjectChart`'s gate needs a non-array `aggregate.groupBy`, and all three + * in-tree constructors pass a string). So this is not about a failure RATE. It + * is about the failure MODE, and about the half the census could not measure: + * `ObjectChart`'s props are `any`, so an out-of-repo host may already send this + * shape. + * + * What the old behaviour did with it: nothing visible. The array went to the + * wire, the receiving engine refused it ("is not a filter" — objectstack's + * `engine-filter-array-lowering.test.ts` pins `engine.aggregate` rejecting the + * same shape with `400 INVALID_FILTER`, refused before the store is touched), + * and where the predicate was instead dropped the chart rendered confident, + * wrong numbers with no signal to their author. Neither end is a degradation; + * both are failures. The ruling moves the refusal to the producer, where an + * author can read it and act on it. + * + * ⛔ Three things this deliberately does NOT do, each a refused option: + * - it does not LOWER on the spec-shape branch (option B — the tolerant- + * consumer direction, and it would bless a shape `AggregateParams` does not + * declare); + * - it does not RETIRE the branch (option D — no in-repo producer, but the + * `props: any` half is unmeasurable); + * - it does not touch `AggregateParams` (`packages/types`), which is carded + * separately. + * + * ⭐ WHICH BRANCH RAN IS ASSERTED, NOT ASSUMED. The two branches post to + * different endpoints, so the wire proves the branch for anything that reaches + * it. A refusal reaches nothing, so for the refusing inputs the branch is + * proven a second way: `looksLikeSpecShape` is transcribed into + * `branchSelectors()` below and the refused params are asserted to produce the + * SAME selector triple as a control that is observed on the spec-shape wire. + * Without that, a test that accidentally landed in the analytics branch would + * pass for the wrong reason. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { isFilterAST } from '@objectstack/spec/data'; +import { + ObjectStackAdapter, + clearSharedDiscoveryCache, + isMalformedFilterError, + UnloweredAggregateWhereError, +} from './index'; + +/** Rows the spec-shape door answers with, so nothing degrades to a fallback. */ +const QUERY_ROWS = { success: true, data: { object: 'opportunity', records: [{ stage: 'won', n: 2 }], total: 1 } }; +/** Rows the analytics door answers with, carrying the measure it was asked for. */ +const ANALYTICS_ROWS = { rows: [{ amount_sum: 150 }] }; + +/** + * A fetch mock that keeps the two branches' doors apart. + * + * `POST /api/v1/data/:object/query` is the spec-shape branch (`client.data.query`). + * `POST /api/v1/analytics/query` is the analytics branch (`client.analytics.query`). + * `GET /api/v1/data/:object` is the client-side fallback `aggregateViaFind` uses — + * recorded too, so "nothing was sent" can be asserted against ALL THREE, not + * just the one a test happens to look at. + */ +function makeAdapter() { + const specShapeBodies: any[] = []; + const analyticsBodies: any[] = []; + const urls: string[] = []; + const fetchImpl = vi.fn(async (url: any, init?: any) => { + const u = String(url); + urls.push(u); + if (u.includes('/api/v1/discovery')) { + return { + ok: true, status: 200, statusText: 'OK', + json: async () => ({ success: true, data: { version: 'v1', routes: {} } }), + } as any; + } + if (u.includes('/api/v1/data/') && u.endsWith('/query')) { + specShapeBodies.push(init?.body ? JSON.parse(String(init.body)) : undefined); + return { ok: true, status: 200, statusText: 'OK', json: async () => QUERY_ROWS } as any; + } + if (u.includes('/api/v1/analytics/query')) { + analyticsBodies.push(init?.body ? JSON.parse(String(init.body)) : undefined); + return { ok: true, status: 200, statusText: 'OK', json: async () => ANALYTICS_ROWS } as any; + } + return { + ok: true, status: 200, statusText: 'OK', + json: async () => ({ success: true, data: { object: 'opportunity', records: [], total: 0 } }), + } as any; + }); + const adapter = new ObjectStackAdapter({ + baseUrl: 'http://localhost:3000', token: 't', autoReconnect: false, fetch: fetchImpl as any, + }); + return { adapter, specShapeBodies, analyticsBodies, urls }; +} + +/** Which door a call came out of. `null` = it never reached the wire at all. */ +type Door = 'spec-shape' | 'analytics' | 'find-fallback' | null; + +function doorOf(urls: string[]): Door { + if (urls.some((u) => u.includes('/api/v1/data/') && u.endsWith('/query'))) return 'spec-shape'; + if (urls.some((u) => u.includes('/api/v1/analytics/query'))) return 'analytics'; + if (urls.some((u) => u.includes('/api/v1/data/opportunity'))) return 'find-fallback'; + return null; +} + +/** Run `aggregate()` and report the door it came out of plus what it posted. */ +async function run(params: unknown) { + const { adapter, specShapeBodies, analyticsBodies, urls } = makeAdapter(); + const error = await adapter.aggregate('opportunity', params as any).then(() => null, (e) => e); + return { door: doorOf(urls), specShapeBodies, analyticsBodies, urls, error }; +} + +/** + * `looksLikeSpecShape`, transcribed from `aggregate()`. + * + * The three disjuncts, kept separate so an assertion can show WHICH one decided + * the branch. This is a transcription on purpose: it lets a test state that two + * different `params` take the same branch even when neither reaches the wire. + * If `aggregate()`'s predicate ever changes, the control rows below — which + * observe the real branch on the real wire — go red and bring this with them. + */ +function branchSelectors(params: any): [boolean, boolean, boolean] { + return [ + params != null && Array.isArray(params.groupBy), + params != null && Array.isArray(params.aggregations), + params != null && params.where !== undefined, + ]; +} + +/** Spec-shape params, filter-free. Every case below varies only `where`. */ +const SPEC_SHAPE = { + groupBy: ['stage'], + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], +}; + +/** A properly-lowered `where`: the AST the spec's own gate accepts. */ +const AST_WHERE = ['stage', '=', 'won']; +/** The unlowered shape the ruling is about: authoring sugar, not a filter. */ +const RULE_WHERE = [{ field: 'stage', operator: 'equals', value: 'won' }]; + +describe('the spec-shape branch is the branch under test — asserted on the wire', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + it('spec-shape params with a lowered `where` go to POST /data/:object/query, NOT to analytics', async () => { + const r = await run({ ...SPEC_SHAPE, where: AST_WHERE }); + expect(r.error).toBeNull(); + // The positive control the whole file rests on: this IS the spec-shape branch. + expect(r.door).toBe('spec-shape'); + expect(r.analyticsBodies).toHaveLength(0); + expect(r.urls.some((u) => u === 'http://localhost:3000/api/v1/data/opportunity/query')).toBe(true); + }); + + it('legacy analytics params go to POST /analytics/query — the OTHER branch, still lowering (#6302)', async () => { + // The control that keeps "we now refuse" from meaning "we now refuse + // everything": the analytics branch is untouched by this change and still + // LOWERS the very rule array the spec-shape branch refuses. + const r = await run({ function: 'sum', field: 'amount', groupBy: '_all', filter: RULE_WHERE }); + expect(r.error).toBeNull(); + expect(r.door).toBe('analytics'); + expect(r.analyticsBodies[0].where).toEqual(['stage', '=', 'won']); + expect(r.specShapeBodies).toHaveLength(0); + }); + + it('a `where` key alone flips legacy params onto the spec-shape branch', async () => { + // Same legacy params as the row above; only the KEY NAME changes. This is + // the asymmetry the card reported, and it is what makes the branch + // selection provable for params that never reach the wire. + const r = await run({ function: 'sum', field: 'amount', groupBy: '_all', where: AST_WHERE }); + expect(r.door).toBe('spec-shape'); + expect(r.specShapeBodies[0].where).toEqual(AST_WHERE); + }); +}); + +describe('an unlowered `where` is refused at the producer, loudly', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + it('throws UnloweredAggregateWhereError instead of posting a rule array', async () => { + const r = await run({ ...SPEC_SHAPE, where: RULE_WHERE }); + expect(r.error).toBeInstanceOf(UnloweredAggregateWhereError); + // Same `INVALID_FILTER` / 400 pair the data API and `MalformedFilterError` + // use, so a failed widget renders "this filter is malformed" rather than + // "check your connection" (objectui#3066). + expect(r.error.code).toBe('INVALID_FILTER'); + expect(r.error.httpStatus).toBe(400); + expect(isMalformedFilterError(r.error)).toBe(true); + expect(r.error.where).toEqual(RULE_WHERE); + expect(r.error.resource).toBe('opportunity'); + }); + + it('sends NOTHING — not to the spec-shape door, not to analytics, not to the find() fallback', async () => { + // The silent failure this replaces ended in wrong numbers on a rendered + // chart. A refusal that quietly degraded to `aggregateViaFind` would + // reproduce it, so all three doors are asserted, not just the one. + const r = await run({ ...SPEC_SHAPE, where: RULE_WHERE }); + expect(r.door).toBeNull(); + expect(r.specShapeBodies).toHaveLength(0); + expect(r.analyticsBodies).toHaveLength(0); + expect(r.urls.every((u) => u.includes('/api/v1/discovery'))).toBe(true); + }); + + it('the refused params take the SAME branch as the observed control', async () => { + // ⭐ The branch-selection proof for an input that never reaches the wire. + // The control above is observed landing on the spec-shape door; these two + // params differ in nothing `looksLikeSpecShape` reads, so the refusal + // cannot have come from the analytics branch. + const control = { ...SPEC_SHAPE, where: AST_WHERE }; + const refused = { ...SPEC_SHAPE, where: RULE_WHERE }; + expect(branchSelectors(refused)).toEqual(branchSelectors(control)); + expect(branchSelectors(control)).toEqual([true, true, true]); + expect(Object.keys(refused).sort()).toEqual(Object.keys(control).sort()); + // …and the control really is on that door, in this same test. + expect((await run(control)).door).toBe('spec-shape'); + }); + + it('the message tells an author what arrived, what is expected, and where to fix it', async () => { + const r = await run({ ...SPEC_SHAPE, where: RULE_WHERE }); + const msg = String(r.error.message); + // 1. what arrived — verbatim, so the producer is identifiable from a log. + expect(msg).toContain(JSON.stringify(RULE_WHERE)); + // 2. what is expected, and where it is declared. + expect(msg).toContain('QuerySchema.where'); + expect(msg).toContain('data/query.zod.ts'); + expect(msg).toContain("['stage','=','won']"); + expect(msg).toContain('ViewFilterRule[]'); + // 3. where to fix it — the producer, not this adapter. + expect(msg).toContain('lower it in the producer that built these aggregate params, not here'); + // 4. and that no numbers were invented on the way out. + expect(msg).toContain('Nothing was sent to the server'); + // It names the branch, so a reader of the log knows which of the two ran. + expect(msg).toContain('spec-shape branch'); + }); +}); + +describe('the refusal is exactly the set the receiving door already refuses', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + // Every row is an array `isFilterAST` rejects, which is the same gate the + // server ingress runs. So none of these could ever have produced a correct + // number: this relocates a failure, it does not add one. + const REFUSED: Array<[string, unknown]> = [ + ['a single rule object', RULE_WHERE], + ['several rule objects', [ + { field: 'stage', operator: 'eq', value: 'won' }, + { field: 'amount', operator: 'gt', value: 100 }, + ]], + ['rules spread into a logical node', ['and', { field: 'stage', operator: 'eq', value: 'won' }, ['amount', '>', 100]]], + ['the infix join dialect the spec never declared', [['stage', '=', 'won'], 'or', ['stage', '=', 'lost']]], + ['a tuple whose operator is outside the AST vocabulary', ['stage', 'sounds_like', 'won']], + ['a logical node with nothing to join', ['and']], + ['an element that is not a condition', [42]], + ]; + + for (const [name, where] of REFUSED) { + it(`refuses ${name}, and nothing reaches any door`, async () => { + // Stated against the contract, not against our own opinion of the shape. + expect(isFilterAST(where)).toBe(false); + const r = await run({ ...SPEC_SHAPE, where }); + expect(r.error).toBeInstanceOf(UnloweredAggregateWhereError); + expect(r.door).toBeNull(); + }); + } +}); + +describe('everything the receiving door accepts still passes through untouched', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + // The other half of the bar: "we now refuse" must not mean "we now refuse + // everything". Each row lands on the spec-shape door with `where` byte-equal + // to what the caller passed — this branch lowers nothing, before or after. + const ACCEPTED: Array<[string, unknown]> = [ + ['a comparison tuple', ['stage', '=', 'won']], + ['a logical AST node', ['and', ['stage', '=', 'won'], ['amount', '>', 100]]], + ['an `or` node', ['or', ['stage', '=', 'won'], ['stage', '=', 'lost']]], + ['a legacy nested array of nodes', [['stage', '=', 'won'], ['amount', '>', 100]]], + ]; + + for (const [name, where] of ACCEPTED) { + it(`${name} reaches client.data.query unchanged`, async () => { + expect(isFilterAST(where)).toBe(true); + const r = await run({ ...SPEC_SHAPE, where }); + expect(r.error).toBeNull(); + expect(r.door).toBe('spec-shape'); + expect(r.specShapeBodies[0].where).toEqual(where); + }); + } + + it('a FilterCondition OBJECT is not an array, and this gate leaves it alone', async () => { + // `QuerySchema.where` IS `FilterConditionSchema` — the MongoDB-style + // condition object. `isFilterAST` says `false` about it, which is why the + // gate is scoped to arrays: predicate-only would have refused the declared + // contract itself. + expect(isFilterAST({ stage: 'won' })).toBe(false); + const r = await run({ ...SPEC_SHAPE, where: { stage: 'won' } }); + expect(r.error).toBeNull(); + expect(r.door).toBe('spec-shape'); + expect(r.specShapeBodies[0].where).toEqual({ stage: 'won' }); + }); + + it('an EMPTY array is "no filter" and still reaches the wire as `[]`', async () => { + // The second measured carve-out. `isFilterAST([])` is `false`, but + // objectstack's `engine-filter-array-lowering.test.ts` pins `where: []` + // returning every row from `find()` and `3` from `count()` — the receiving + // door accepts it, so refusing it would be a refusal nobody ruled. + expect(isFilterAST([])).toBe(false); + const r = await run({ ...SPEC_SHAPE, where: [] }); + expect(r.error).toBeNull(); + expect(r.door).toBe('spec-shape'); + expect(r.specShapeBodies[0].where).toEqual([]); + }); + + it('no `where` at all still posts no `where` key', async () => { + const r = await run(SPEC_SHAPE); + expect(r.error).toBeNull(); + expect(r.door).toBe('spec-shape'); + expect(r.specShapeBodies[0]).not.toHaveProperty('where'); + expect(r.specShapeBodies[0].groupBy).toEqual(['stage']); + expect(r.specShapeBodies[0].aggregations).toEqual(SPEC_SHAPE.aggregations); + }); + + it('`limit` is still forwarded alongside a lowered `where`', async () => { + const r = await run({ ...SPEC_SHAPE, where: AST_WHERE, limit: 5 }); + expect(r.error).toBeNull(); + expect(r.specShapeBodies[0]).toEqual({ ...SPEC_SHAPE, where: AST_WHERE, limit: 5 }); + }); +}); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index a9168e261..38f24fd4d 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -13,6 +13,12 @@ import type { DroppedFieldsEvent } from '@objectstack/spec/data'; // is read off the pin instead of hand-copied here (a hand copy is the drift // this seam already paid for once — see `DroppedFieldsEvent`'s comment below). import { DroppedFieldsEventSchema } from '@objectstack/spec/data'; +// #6825 — the spec's OWN filter-AST gate, imported as a VALUE for the same +// reason: `aggregate()`'s spec-shape branch decides whether a `where` is a +// filter by asking the contract, not by a hand-rolled shape sniff that could +// disagree with the door it is protecting. Same predicate the server ingress +// runs, so the producer-side refusal and the wire-side one cannot drift. +import { isFilterAST } from '@objectstack/spec/data'; import type { ApiError } from '@objectstack/spec/api'; // #4237 — the metadata save door's advisory reader, shared with `MetadataClient` // rather than forked. ONE reader, two call sites: the other client class calls it @@ -180,6 +186,87 @@ export function isMalformedFilterError(error: unknown): boolean { return e.code === 'INVALID_FILTER' || e.name === 'MalformedFilterError'; } +/** + * A `where` that reached `aggregate()`'s SPEC-SHAPE branch as an array the spec + * cannot read as a filter — an unlowered `ViewFilterRule[]` above all. + * + * WHY THIS IS A REFUSAL AND NOT A LOWERING (objectui#6825, maintainer ruling + * 2026-08-30, option A). The two branches of `aggregate()` take a filter by + * different names: the analytics branch takes `filter` and lowers it through + * `translateFilterArray` (objectui#6302), the spec-shape branch takes `where` + * and posts it to `POST /data/:object/query` verbatim. Lowering here too was + * option B and was REFUSED: it is the tolerant-consumer direction AGENTS.md + * #0.1 exists to stop, and it would bless as supported a params shape + * `AggregateParams` does not even declare. Retiring the branch was option D and + * was refused as the riskier move — this repo has no producer that can reach it + * (`ObjectChart`'s gate requires a non-array `aggregate.groupBy`, and all three + * in-tree constructors pass a string), but `ObjectChart`'s props are `any` and + * an out-of-repo host may already send this shape. So the branch stays, and it + * says no out loud. + * + * What that buys is the FAILURE MODE, not a failure count. The receiving engine + * already refuses these arrays ("is not a filter" — objectstack's + * `engine-filter-array-lowering.test.ts`), so nothing that reaches this point + * could have produced a correct number; the pre-existing behaviour was to find + * that out on the wire, or worse, to have the predicate quietly dropped while + * the chart rendered confident, wrong figures with no signal to their author. + * This moves the same refusal to the producer, where the author can act on it. + * + * Carries the `INVALID_FILTER` / 400 pair its sibling {@link MalformedFilterError} + * carries, so `isMalformedFilterError()` recognises it and a failed widget renders + * "this filter is malformed" rather than "check your connection" (objectui#3066). + */ +export class UnloweredAggregateWhereError extends Error { + readonly code = 'INVALID_FILTER'; + readonly httpStatus = 400; + /** The value as received, so a caller can log what its producer actually built. */ + readonly where: unknown; + /** The object `aggregate()` was called for. */ + readonly resource: string; + constructor(where: unknown, resource: string) { + const shown = JSON.stringify(where) ?? String(where); + super( + `aggregate('${resource}'): the spec-shape branch received a \`where\` array ` + + `that is not a filter — ${shown}. This branch posts \`where\` to ` + + 'POST /data/:object/query verbatim, so it must ALREADY be lowered: either a ' + + 'FilterCondition object (`QuerySchema.where`, @objectstack/spec ' + + 'data/query.zod.ts), or a FilterArray the ingress can lower — a comparison ' + + "tuple (['stage','=','won']), a logical node (['and',[..],[..]]), or an array " + + 'of nodes (data/filter.zod.ts, gate `isFilterAST`). A ViewFilterRule[] ' + + '([{ field, operator, value }, ...]) is authoring sugar, not a filter: lower it ' + + 'in the producer that built these aggregate params, not here. Nothing was sent ' + + 'to the server, so no unfiltered numbers came back.', + ); + this.name = 'UnloweredAggregateWhereError'; + this.where = where; + this.resource = resource; + } +} + +/** + * Gate the spec-shape branch's `where` on the spec's own AST predicate. + * + * Scoped to ARRAYS on purpose, and the two carve-outs are measured, not assumed: + * + * - A NON-array `where` is the declared shape. `QuerySchema.where` is + * `FilterConditionSchema` — the MongoDB-style condition object — so refusing + * `{ stage: 'won' }` here would refuse the contract itself. Untouched. + * - An EMPTY array is "no filter", and the engine says so in as many words: + * objectstack's `engine-filter-array-lowering.test.ts` pins `where: []` + * returning every row from `find()` and `3` from `count()`. `isFilterAST([])` + * is `false`, so gating on the predicate alone would refuse a value the + * receiving door accepts — a refusal the ruling did not order. + * + * Every remaining array is one the receiving door refuses too, so this adds no + * new failure — it moves an existing one to where an author can read it. + */ +function assertSpecShapeWhereIsFilterAst(where: unknown, resource: string): void { + if (!Array.isArray(where)) return; + if (where.length === 0) return; + if (isFilterAST(where)) return; + throw new UnloweredAggregateWhereError(where, resource); +} + function objectFilterEntryToAST(entry: any): [string, string, any] | null { if (!entry || typeof entry !== 'object') return null; // `field` only. A `?? entry.name` fallback lived here from the day the @@ -4684,7 +4771,17 @@ export class ObjectStackAdapter implements DataSource { const queryAst: Record = {}; if (Array.isArray(params.groupBy)) queryAst.groupBy = params.groupBy; if (Array.isArray(params.aggregations)) queryAst.aggregations = params.aggregations; - if (params.where !== undefined) queryAst.where = params.where; + if (params.where !== undefined) { + // STRICT, deliberately — objectui#6825, maintainer ruling 2026-08-30 + // (option A). Unlike the analytics branch below, this one does NOT lower: + // it refuses. `where` here is the spec Query DSL's `where`, and an array + // the spec's AST gate rejects (an unlowered `ViewFilterRule[]`, say) is + // off-contract at the PRODUCER. Lowering it here was option B and was + // refused; see `UnloweredAggregateWhereError` for the full ruling and + // for why the two carve-outs (non-array, empty array) are carve-outs. + assertSpecShapeWhereIsFilterAst(params.where, resource); + queryAst.where = params.where; + } if (typeof params.limit === 'number') queryAst.limit = params.limit; const result: any = await this.client.data.query(resource, queryAst as any); // client.data.query returns { object, records, total, hasMore }