Skip to content

Commit 1c1b8c8

Browse files
feat(spec,objectql,metadata-protocol): grouped and aggregated queries honour search (#20487)
Fixes #20358 Clause-②: yes ## What this does A grouped or aggregated query now honours ADR-0061 `search`. Route (A) from triage (`5871414670`): - **`@objectstack/spec`** (`packages/spec/src/data/data-engine.zod.ts`): `EngineAggregateOptionsSchema` declares `search` and `searchFields` with the same Zod expression as `EngineQueryOptionsSchema` (`z.union([z.string(), FullTextSearchSchema])`, `z.array(z.string())`). The structured `search` arm carries flag defaults, so the aggregate options now have two shapes. ADR-0122 and `check:spec-parsed-alias` therefore require `EngineAggregateOptionsParsed`. It is declared, and its isomorphism pin leaves `type-alias-convention.pin.test.ts` (780 to 779). That is the one added export: `check:api-surface` reports 0 breaking, 1 added. - **`@objectstack/objectql`** (`packages/objectql/src/engine.ts`): `ENGINE_AGGREGATE_OPTION_KEYS` gains the two keys. `aggregate()` runs them through the existing `expandSearchOnAst`, via a small carrier helper (`expandSearchOnAggregateOptions`), and does not add a second expander. The expansion sits at the same point in the sequence as on `find`: after the `where` doors (`lowerWhereFilterArray`), and before the AST is built, tokens resolve and the security middlewares run. The helper returns a copy of the caller's bag, so the bag is never written through. - **`@objectstack/metadata-protocol`** (`packages/metadata-protocol/src/protocol.ts`): `findData`'s grouped branch passes `search` / `searchFields` to `engine.aggregate`. `searchFields` was already validated before the branch fork (the INVALID_FIELD gate), so both branches refuse the same bad override. **Mechanism hypotheses, measured:** - ① Only `EngineAggregateOptionsSchema` is read. `DataEngineAggregateOptionsSchema` (the deprecated legacy schema) has no runtime reader: `git grep` finds it only in `packages/spec` tests and the type-alias pin. It is left unwidened. - ② The grouped branch site held as described. - ③ At base, the engine refused `search` on aggregate (`rejectUnknownEngineOptions`), confirmed. - ④ The card's before-state reproduced at the public door (below). **Landing site beyond the three declared files:** the public-door pin went into `packages/rest/src/list-view-grouping-query-door.test.ts` as a new §10. That file already drives the card's 186-row fixture through the real `RestServer` → `findData` → `ObjectQL.aggregate` → sqlite `SqlDriver` chain on both aggregate tiers. The pin is test-only, and no second harness was written. ## Before / after at `POST /api/v1/data/:object/query` Before (reproduced by reverting only the protocol pass-through, which is the base's door behaviour; the engine change is inert when the key is not sent): `{ groupBy: ['business_unit'], aggregations: [count], search: 'harbour' }` answered **five** groups, `northgate_operations 86 / northgate_quality 61 / riverside_plant 31 / northgate_plant 7 / harbour_office 1`, the unsearched answer. On both tiers, the flat `{ search: 'harbour' }` returned one row. After: `[{ business_unit: 'harbour_office', count: 1 }]`, `total: 1`, on both tiers. ## Tests Head of this PR: `2196566d3f`. The full suites ran on `3576fd34e7` (this branch with `origin/main` `acd009521e` merged in). The two commits after it change only `packages/objectql/src/engine-aggregate-search.test.ts`: the find double now applies the caller's `limit` (`check:objectql-double-limit`), and three `as any` erasures became typed calls (`check:query-options-erasure`). That file and objectql's typecheck were re-run on `2196566d3f`. All vitest runs used `--maxWorkers=2`. | package | run | result | |:--|:--|:--| | `@objectstack/objectql` | `vitest run --project local` | 328 files, 6084 passed | | `@objectstack/objectql` | `vitest run --project repo` | 5 passed | | `@objectstack/objectql` | `typecheck` (tsc + scripts + test layer) | exit 0, re-run on `2196566d3f` | | `@objectstack/objectql` | `src/engine-aggregate-search.test.ts` on `2196566d3f` | 17 passed | | `@objectstack/metadata-protocol` | `vitest run` | 189 files (+3 skipped), 2750 passed, 19 skipped | | `@objectstack/metadata-protocol` | `typecheck` | exit 0 | | `@objectstack/rest` | `vitest run --project local` | 216 files, 3923 passed, 26 skipped | | `@objectstack/rest` | `vitest run --project repo` | 8 passed | | `@objectstack/rest` | `typecheck` (tsc + test layer) | exit 0 | | `@objectstack/spec` | `vitest run --project local` | 568 files, 16692 passed, 1 todo | | `@objectstack/spec` | `vitest run --project repo` | 690 passed | | `@objectstack/spec` | `typecheck` (tsc + scripts + test layer) | exit 0 | The filter direction is **the changed packages themselves**. `@objectstack/rest` is included because it hosts the public-door pin and its route tests cover grouped queries. The downstream consumers of the widened `EngineAggregateOptions` type are covered by the ones that compile against it here (objectql, metadata-protocol and rest typecheck); `check:api-surface` shows 0 removed or narrowed. **New pins:** - **`packages/rest/src/list-view-grouping-query-door.test.ts` §10**, the public door, both aggregate tiers (driver-sql native `GROUP BY`, and the in-memory lowering). Each tier assertion checks which face served the query. - The measured case: `search: 'harbour'` returns the ONE group, `total: 1`, and equals the grouping of the flat searched rows. The in-memory tier's rows read carries `$icontains` and no `search` key. - `search: 'northgate'`: every header number, `sum_amount` included, equals the grouping of the flat searched rows. - `searchFields: ['owner']` narrows the answer to the `owner_3` rows. The oracle is the fixture generator, not the door. The one-row unit drops out, and the narrowed answer differs from the default-field answer. - `search` + `having`: search drops `riverside_plant`, and `having` drops `northgate_plant`. - `aggregations` with no `groupBy`: 154, where the unsearched control gives 186. - A grouped query with an unscannable `searchFields` is `400` / `INVALID_FIELD`. - **`packages/objectql/src/engine-aggregate-search.test.ts`**, both tiers: - the grouped answer equals the grouping of `find()`'s searched rows (default fields, narrowed, structured form, AND-ed with `where`, multi-term); - **one expander**: the `where` the driver receives from `aggregate` deep-equals the one `find` sends for the same bag, and neither AST carries `search` / `searchFields`; - the aggregate over a search equals the aggregate over the same filter written as `where`; - no-`groupBy` aggregations, `search` + `having`, and the caller's frozen bag is not written through; - `$search` / `$searchFields` are still refused on aggregate, as unknown options; - **drift pin**: a proof table must cover `ENGINE_OPTION_KEY_SETS.aggregate` exactly, and every legal key must have an observable effect. This is the question `findOne`'s drift pin asks, now asked of this verb. - **`packages/spec/src/data/data-engine.test.ts`**: a parse keeps both keys in both `search` forms, and the aggregate options refuse the values the query options refuse. The two keys' input JSON Schemas deep-equal `EngineQueryOptionsSchema`'s (one declaration on both verbs). - The existing drift pin `engine-unknown-option.test.ts` (each legal set equals its schema's shape) holds unchanged. So does the unknown-key refusal pin for `aggregate` (`'bogus'`), byte for byte. ## Reverse verification (one-shot, not left in the tree) Both ablations committed the fix first, then mutated through `scripts/ablation-replace.mjs` (literal anchor, on-disk counts and blob hashes). Each armed an absolute-path restore trap, and each restore was proven by blob equality with `HEAD` and an empty `git diff HEAD`. - **A: protocol pass-through removed** (the base's door behaviour), pinned through `dist/`. - Mutation: the anchor `search: options.search,` + `searchFields: options.searchFields,` was deleted, taking the blob from `508f88c8c8e0` to `f8a47dbaf791`. `@objectstack/metadata-protocol` was rebuilt, and `ablation-dist-preflight --absent` confirmed the marker was gone from all 24 built files. - `list-view-grouping-query-door.test.ts`: **10 failed / 34 passed**. Every positive §10 case failed on both tiers, in the predicted direction: the measured case received the five unsearched groups, 86/61/31/7/1. The `searchFields` refusal case stayed green, because that gate sits before the branch fork. - Restore: rebuilt, the marker was present again in both dist chunks, the tree was clean, and the file returned to **44 passed**. - **B: engine expansion call removed** (the keys stay legal but are never executed), in source (objectql tests import `./engine.js`). - `engine-aggregate-search.test.ts`: **13 failed / 4 passed**. The 12 behavioural cases failed across both tiers, and so did the **drift pin**, on its `search` proof, which is the declared-but-unexecuted shape it exists to catch. The 4 that stayed green were the frozen-bag case and the two `$search` refusals, none of which depend on the expansion. - Restore: blob `177d256d61d4` equals `HEAD`, and `git diff HEAD` is empty. ## Gates `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 114 commands. All 114 were run on `2196566d3f`, each exit captured before any pipe and reconciled with `--ran`: `114 derived, 112 run, 2 NOT-MEASURED, 0 UNRUN`. 111 exited 0, including `check:spec-parsed-alias`, `check:api-surface`, `check:authorable-surface`, `check:docs`, `check:export-origins`, `check:generated`, `check:liveness`, `check:query-options-erasure`, `check:objectql-double-limit`, `check:where-matcher`, `check:engine-double-contract`, `check:adr-0087-registration`, `check:changeset-no-major` and `check:nul-bytes`. `check:skill-examples` refused its prerequisite on the first run (client-react not built). It was run after building `@objectstack/client-react...`, and 259 prose examples type-check (exit 0). - NOT MEASURED: `check:dual-build-cjs-loads`, reason: exit 3, PREREQUISITE NOT MET (it needs every package's `dist/`; CI builds them). - NOT MEASURED: `check:type-check-debt`, reason: my 420 s timeout fired mid re-measure (exit 124). It needs the whole `./packages` closure built, which `lint.yml` does first. - NOT MEASURED: `check-engine-split-ratio --days 90`, reason: the clone is shallow and the gate refused (exit 2) rather than compute over a truncated window. Spec artefacts, regenerated only where `check:generated` proved them stale: - `api-surface/data.json` (+`EngineAggregateOptionsParsed`: 0 breaking, 1 added); - `export-origins/data.json`; - `content/docs/references/data/data-engine.mdx`; - `authorable-surface/data.json` (+2 rows, written by the spec build's `gen:schema`). After the merge of `origin/main`, `check:generated` reported all 15 artifacts up to date against a freshly built `packages/spec/dist`. ## Acceptance notes - **Published skill row now understates the aggregate key set, and is not edited here.** In `skills/objectstack-query/SKILL.md`, the Calling Convention table lists engine `aggregate`'s legal keys as `context`, `where`, `groupBy`, `aggregations`, `having`, `timezone`, and after this PR the closed set also holds `search` and `searchFields`. The row understates the set: nothing it names is refused, but a reader would not know `search` works. `skills/**` is a Tier H governed surface and outside this card's declared file surface, so the one-row edit (0 net lines) is left for the seat to route. It is raised in the dev report. - **`findData`'s grouped branch still spells the bag `as any`.** Every key in it is now declared, so the erasure could go. Dropping it moves `protocol.ts`'s count in the shrink-only `scripts/query-options-erasure-baseline.json` (6 to 5), which is outside this card's surface. Carrier: whoever next touches that branch. - **The engine's unknown-option refusal is a bare `Error`, with no ADR-0112 `code` / `status`.** This is pre-existing and unchanged byte for byte. `findData` never builds the aggregate bag from caller keys, so the refusal is not reachable from `POST /data/:object/query`, and it stays pinned by message as before. - **`engine.count` still takes no `search`.** So the flat branch under `search` + `limit` still reports a page-local `total` estimate, which is documented behaviour and unchanged here. - **objectui follow-up (triage note 3).** The console's workaround, which hands grouping back to the page window while a toolbar search is active, can end once a release carries this. `compileListViewGroupQuery` does not carry `search` itself; a client spreads `search` onto the compiled body, as §10 does. That card is the accepting seat's to file, with `Blocked-by:` on this one. - **`DataEngineAggregateOptionsSchema` is deliberately not widened.** It is the deprecated legacy aggregate schema, with no runtime reader (measured by `git grep`: `packages/spec` tests and the type-alias pin only). --- _Generated by [Claude Code](https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9bf5e67 commit 1c1b8c8

12 files changed

Lines changed: 672 additions & 5 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/metadata-protocol": patch
5+
---
6+
7+
A grouped or aggregated query now honours `search`: the groups and every aggregated number are computed over the searched rows, exactly the rows the same query without `groupBy` / `aggregations` returns.
8+
9+
Clause-②: yes (widening) — `EngineAggregateOptionsSchema` gains two OPTIONAL keys, `search` and `searchFields`, so the accept set of the aggregate options grows. Nothing previously admitted is refused, no key is renamed or retired, and no producer is required to write them.
10+
11+
`QuerySchema.search` (ADR-0061) is declared on the query beside `groupBy` and `aggregations`, with no carve-out. Until now, `POST /data/:object/query` accepted a body such as `{ groupBy: ["business_unit"], aggregations: [{ function: "count", alias: "count" }], search: "harbour" }` and answered it with the UNSEARCHED groups — no error and no warning — while the same body without `groupBy` / `aggregations` returned only the searched rows. A grouped list view under a toolbar search would therefore show group headers that ignore what the user typed.
12+
13+
- **`@objectstack/spec`** — `EngineAggregateOptionsSchema` declares `search` (the bare string, or the structured `FullTextSearchSchema` form) and `searchFields`, identically to `EngineQueryOptionsSchema`. A parse used to strip them.
14+
- **`@objectstack/objectql`** — `engine.aggregate()` (and `ctx.api.object(name).aggregate()`) accepts the two keys it used to refuse as unknown options, and expands them through the same ADR-0061 expansion `find()` uses: the same server-resolved searchable fields, the same `searchFields` narrowing, AND-ed with `where` before the security middlewares run. There is one expander, not two. It applies on both aggregate paths, native `driver.aggregate()` and the in-memory lowering. A key the verb still does not execute, such as `$search`, is refused as before.
15+
- **`@objectstack/metadata-protocol`** — `findData`'s grouped branch passes `search` / `searchFields` to `engine.aggregate()`. `searchFields` is validated on that branch exactly as on the flat one: a column search cannot scan is `400 INVALID_FIELD`.
16+
17+
Nothing to migrate. A caller that worked around the gap, for example by grouping a page of searched rows on the client, can send the grouped query with its `search` instead.

‎content/docs/references/data/data-engine.mdx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ Options for DataEngine.aggregate operations
152152
| **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>; field?: string; alias: string; filter?: any }[]` | optional | |
153153
| **having** | `any` | optional | HAVING — filter over the aggregated rows (aggregation aliases + groupBy projections); applied engine-side after aggregation |
154154
| **timezone** | `string` | optional | |
155+
| **search** | `string \| { query: string; fields?: string[]; fuzzy?: boolean; operator?: Enum<'and' \| 'or'>; … }` | optional | |
156+
| **searchFields** | `string[]` | optional | |
155157
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
156158

157159

@@ -708,6 +710,8 @@ This schema accepts one of the following structures:
708710
| **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>; field?: string; alias: string; filter?: any }[]` | optional | |
709711
| **having** | `any` | optional | HAVING — filter over the aggregated rows (aggregation aliases + groupBy projections); applied engine-side after aggregation |
710712
| **timezone** | `string` | optional | |
713+
| **search** | `string \| { query: string; fields?: string[]; fuzzy?: boolean; operator?: Enum<'and' \| 'or'>; … }` | optional | |
714+
| **searchFields** | `string[]` | optional | |
711715
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |
712716

713717
---
@@ -898,6 +902,8 @@ QueryAST-aligned options for DataEngine.aggregate operations
898902
| **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct'>; field?: string; alias: string; filter?: any }[]` | optional | |
899903
| **having** | `any` | optional | HAVING — filter over the aggregated rows (aggregation aliases + groupBy projections); applied engine-side after aggregation |
900904
| **timezone** | `string` | optional | |
905+
| **search** | `string \| { query: string; fields?: string[]; fuzzy: boolean; operator: Enum<'and' \| 'or'>; … }` | optional | |
906+
| **searchFields** | `string[]` | optional | |
901907

902908
### Nested Shape: `EngineAggregateOptions.context`
903909

@@ -954,6 +960,19 @@ QueryAST-aligned options for DataEngine.aggregate operations
954960
| **distinct** | `never` | optional | [REMOVED] `query.aggregations[].distinct` was removed in @objectstack/spec 17 (ADR-0049) — exactly ONE of the six faces that read an aggregation honoured it. The objectql in-memory fallback deduplicated the values before applying the function, while `driver-sql`, `driver-turso`, `driver-mongodb`, `driver-memory` and the service-analytics SQL builder all ignored it — so `{ function: 'sum', field: 'amount', distinct: true }` answered a DEDUPLICATED sum when the engine fell back in memory and an ordinary sum on every SQL datasource: one query, two numbers, chosen by which backend happened to serve it. Both answers are plausible, so nothing surfaced the divergence. Delete the key. For a deduplicated COUNT the live spelling is the `count_distinct` aggregation function, which every SQL face compiles to `COUNT(DISTINCT field)` and the in-memory fallback computes identically. `SUM(DISTINCT …)` / `AVG(DISTINCT …)` get no replacement: no backend ever computed them here, and a per-row measure that needs deduplicating is a modelling problem to fix in the data, not a flag on the read. |
955961
| **filter** | `any` | optional | Per-aggregation filter (SQL FILTER (WHERE …) semantics): narrows the source rows THIS aggregation reads, leaving sibling aggregations unfiltered. Enforced by engine.aggregate: lowered in memory for drivers without native conditional aggregation; a driver reached directly refuses rather than silently dropping it. |
956962

963+
### Nested Shape: `EngineAggregateOptions.search`
964+
965+
| Property | Type | Required | Description |
966+
| :--- | :--- | :--- | :--- |
967+
| **query** | `string` | ✅ | Search query text |
968+
| **fields** | `string[]` | optional | Fields to search in (if not specified, searches all text fields) |
969+
| **fuzzy** | `boolean` | optional (default: `false`) | [EXPERIMENTAL — not enforced] Fuzzy matching (tolerate typos). The ADR-0061 expansion reads only `query` + `fields`; no executor receives this flag. |
970+
| **operator** | `Enum<'and' \| 'or'>` | optional (default: `"or"`) | [EXPERIMENTAL — not enforced] Logical operator between terms. The ADR-0061 expansion applies its own term semantics; no executor receives this flag. |
971+
| **boost** | `Record<string, number>` | optional | [EXPERIMENTAL — not enforced] Field-specific relevance boosting (field name -> boost factor). No executor scores results. |
972+
| **minScore** | `number` | optional | [EXPERIMENTAL — not enforced] Minimum relevance score threshold. No executor scores results. |
973+
| **language** | `string` | optional | [EXPERIMENTAL — not enforced] Language for text analysis (e.g., "en", "zh", "es"). No executor selects an analyzer. |
974+
| **highlight** | `boolean` | optional (default: `false`) | [EXPERIMENTAL — not enforced] Search result highlighting. No executor emits highlights. |
975+
957976

958977
---
959978

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11067,6 +11067,16 @@ export class ObjectStackProtocolImplementation implements
1106711067
// was finding 1: the one wire path to aggregate() lost the
1106811068
// clause before any executor could ever see it.
1106911069
having: options.having,
11070+
// ADR-0061 `search` is declared on the query beside `groupBy` /
11071+
// `aggregations` with no carve-out, and the flat branch below
11072+
// hands it to `engine.find`. Leaving it out of THIS bag answered
11073+
// a grouped query under a search with the UNSEARCHED groups —
11074+
// no error, no warning. The engine expands it through the same
11075+
// expander `find` uses, so the header numbers are the grouping
11076+
// of exactly the rows the flat query returns. `searchFields` was
11077+
// validated above on both branches (#4254 gate).
11078+
search: options.search,
11079+
searchFields: options.searchFields,
1107011080
context: options.context,
1107111081
} as any);
1107211082
// Apply limit client-side (EngineAggregateOptions doesn't carry limit).

0 commit comments

Comments
 (0)