Skip to content

Commit 80dc9c0

Browse files
docs(skills): objectstack-query — engine aggregate row admits search / searchFields (#20776)
Fixes #20500 Clause-②: no ## What changed Two files, three places, all inside the claim surface (claim 5903909164 with amendments 5904148643 and 5905295272). In `skills/objectstack-query/SKILL.md`, the Calling Convention section: (1) The engine `aggregate` row (`:26`): the legal option keys now list `search` and `searchFields` after `timezone`, with one clause saying the two search keys filter the input rows **before** grouping, AND-ed with `where`, exactly as on `find`. (2) The passthrough paragraph (`:38-44`): it said all six driver passthrough keys are deliberately ILLEGAL on `count` and `aggregate`; `timezone` is one of the six and is a legal option on `aggregate` in its own right (in `ENGINE_AGGREGATE_OPTION_KEYS`, read by `aggregate()` for date bucketing), which the row above already showed. The paragraph now carves that one key out: `aggregate` reads it itself, so it is legal there and the row lists it; `count` refuses it with the rest. (3) In `skills/objectstack-query/rules/aggregation.md:49` (round 3, `cc924448`): "`fields` is not one of the six keys `engine.aggregate()` accepts" no longer states a count — "not one of the keys `engine.aggregate()` accepts" — so it cannot contradict the row it cites, which now lists eight. Net +2 lines across the three commits (`aggregation.md` net 0). Nothing else moved: the closed-set sentence at `SKILL.md:31` and the `fields` / `orderBy` sentence (now `:254`) stay, both still true. Landing sites: the row the dispatch expected (`:26`); the `:38-44` paragraph the seat added after the first report's out-of-scope finding; and `rules/aggregation.md:49`, which the at-tier contract review of `a87b7380` (5904504985, FAIL) found — a sentence that names a count, invisible to the first census's key-name probes. The `content/docs/**` half of the census (two docs sentences that enumerate the same set) is carded as #20792, ⛔ not in this PR. ## In-place fix (patch round, `a87b7380`) In-place fix under the bounded exemption (claim amendment 5904148643), all four conditions holding: ① the same defect class as the card — the skill misstated the legal `aggregate` option set (the `:26` row understated it; the `:38-42` paragraph overstated the illegal set by one key); ② mechanical, in an already-pinned form — the carve-out names `timezone` as the one passthrough key `aggregate` accepts and reads; ③ no other claim holds the file (no open PR touches `skills/objectstack-query/**`, per the claim's serial-constraints reading); ④ the same gate family — the list derived at `a87b7380` is the same 24 commands as at `f997cf8e`, no new verification surface. Measured on `origin/main` `c9c182ed` before writing: `ENGINE_COUNT_OPTION_KEYS` (`packages/objectql/src/engine.ts:556`) is `context`, `where`, and `count()` rejects against it (`:16168`), so `timezone` is refused on `count`; `ENGINE_AGGREGATE_OPTION_KEYS` (`:562-565`) contains `timezone` and none of `transaction`, `tenantId`, `tenantIds`, `bypassTenantAudit`, `preserveAudit` (0 hits each); `aggregate()` spans `:16267-16686` and reads `const tz = query.timezone` at `:16598`. The replacement sentence reuses the file's own vocabulary ("date bucketing", `:89` and `:257`; "the row above"). ## Why Since PR #20487, `ENGINE_AGGREGATE_OPTION_KEYS` (`packages/objectql/src/engine.ts:562` at `c9c182ed`) is `context`, `where`, `groupBy`, `aggregations`, `having`, `timezone`, `search`, `searchFields`. The row stopped at `timezone`, and together with the closed-set sentence it told an agent that a searched aggregate is refused. An agent reading that would group a searched page on the client, which is the wrong-number workaround #20358 retired: group counts over a page window instead of over all searched rows, with no error. ## Behaviour verified at the code, not at the comment `aggregate()` (`engine.ts:16267-16277`): `rejectUnknownEngineOptions(object, 'aggregate', query, ENGINE_AGGREGATE_OPTION_KEYS)` admits both keys, then `expandSearchOnAggregateOptions` (`:11104-11114`) builds a carrier AST from `where` + `search` + `searchFields` and runs the one ADR-0061 expander `find` uses, `expandSearchOnAst` (`:11062-11086`): each term becomes an `$or` of `$icontains` over the resolved searchable fields, the result is AND-ed with the caller's `where` (`{ $and: [where, searchFilter] }`), and the two search keys are deleted from the bag. All of that runs before the aggregate AST is built and before any grouping, so the grouped answer is the grouping of the searched rows, which is what the new clause says. ## Census (list-shaped probes, as the dispatch asked) At `c9c182ed`, over `skills/` and `content/docs/`: - lines naming both `having` and `timezone`: 1 hit, `skills/objectstack-query/SKILL.md:26` (the row corrected here); - lines naming `groupBy`, `aggregations` and `having` together: 2 hits, the same row and `content/docs/kernel/runtime-services/data-service.mdx:91`, which lists the `QueryAST` clauses (it already names `search`; it is not an enumeration of the engine option set, so no edit); - control, `ENGINE_AGGREGATE_OPTION_KEYS`: 2 hits, `SKILL.md:31` and `:254` (`:252` before this PR), both prose sentences, both still true; - control, the engine `aggregate` row spelling: 1 hit, `:26`. `skills/objectstack-query/rules/aggregation.md:104` already says `where` filters the input rows before grouping, the vocabulary the new clause reuses. **Round-3 census (count-shaped probes, after the review).** Over all six files of `skills/objectstack-query/**`: number words, `only` near `accept` / `key` / `option`, `keys` near `accepts` / `legal` / `closed set`, and list-shaped rows, with the literal `engine.aggregate()` as control — the one stale statement was `rules/aggregation.md:49` ("six keys"), fixed here; every other number word counts a set of that size. Over `content/docs/**`: two sentences enumerate the aggregate set falsely — `content/docs/protocol/objectql/query-syntax.mdx:1337` and `content/docs/data-modeling/queries.mdx:672` — carded as #20792 with the evidence and a proposed patch; they are outside this PR. The full probe list and hit counts are in the dev report 5904713155. ## The two `skills/**` readings Tokens in the ratchet's own convention, `ceil(utf8 bytes / 4)`. | Reading | Before (`c9c182ed`) | After (`cc924448`) | Delta | |:--|:--|:--|:--| | touched file `skills/objectstack-query/SKILL.md`, lines | 399 | 401 | +2 | | touched file, tokens (ceiling 5552) | 3915 | 3990 | +75, headroom 1562 | | touched file `skills/objectstack-query/rules/aggregation.md`, lines | 241 | 241 | 0 | | touched file, tokens (ceiling 2357) | 1846 | 1845 | −1, headroom 512 | | whole package `skills/**` (every file), lines | 13424 | 13426 | +2 | | whole package `skills/**` (every file), tokens | 155899 | 155973 | +74 | | all ten `SKILL.md`, lines | 4404 | 4406 | +2 | PM line budget for this card: net +2 lines at most; +2 used (0 by the row commit `f997cf8e`, +2 by the paragraph commit `a87b7380`, 0 by `cc924448`), not exceeded. No existing line was re-wrapped: `SKILL.md:38-41` are byte-identical to `main`, `:42` was extended in place and two lines follow it; `rules/aggregation.md:49` is replaced in place. ## Changeset `skip-changeset`, by measurement: no released package's `files[]` names a `skills` path (every `packages/**/package.json` scanned; positive control: `create-objectstack` lists `dist`, `README.md`, `CHANGELOG.md`). Scaffolded projects pull the catalog from the repository path `objectstack-ai/objectstack/skills` through the `skills` CLI (`packages/create-objectstack/src/skills-install.ts:62`), never from an npm tarball. ## Gates **Head `cc924448` (round 3):** the same 24 derived families plus `check:skill-refs` ran green on `cc924448` (dev report 5904713155); their `--ran` reconciliation is carried by the report's 47-family superset over the round's working tree, not by a 24-only run. CI on `cc924448`: 31 check-runs — 23 `success`, 7 path-filtered `skipped`, 1 `failure` (`Check Changeset`, the missing `skip-changeset` label; see *Changeset*); `Lint & Repo Gates`, `TypeScript Type Check` and `Type Check · source gates` (the token ratchet and the skill-doc gates) `success`. The table below is the round-2 record on `a87b7380`. Derived from the actual change with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` at head `a87b7380` (24 commands, the same list as at `f997cf8e` and as the dispatch), all run on head `a87b7380`, exit codes captured before any pipe, reconciled with `--ran`. | Gate (as `--commands` printed it) | Exit at `a87b7380` | |:--|:--| | `node scripts/check-ci-filter-parity.mjs` | 0 | | `node scripts/check-closing-keyword-parity.mjs` | 0 | | `node scripts/check-closing-keyword-parity.mjs --self-test` | 0 | | `node scripts/check-comment-mask-corpus.mjs` | 0 | | `node scripts/check-doc-route-spelling.mjs --advisory` | 0 | | `node scripts/check-doc-route-spelling.mjs --self-test` | 0 | | `node scripts/check-skills-token-ratchet.mjs` | 0 | | `node scripts/check-skills-token-ratchet.mjs --self-test` | 0 | | `pnpm --filter @objectstack/lint run check:doc-formula-expressions` | 0 | | `pnpm --filter @objectstack/spec run check:skill-docs` | 0 | | `pnpm check:agent-test-spelling` | 0 | | `pnpm check:corpus-claim-drift` | 0 | | `pnpm check:cross-package-test-inputs` | 0 | | `pnpm check:doc-authoring` | 0 | | `pnpm check:driver-memory-census` | 0 | | `pnpm check:gitlink-declared` | 0 | | `pnpm check:nul-bytes` | 0 | | `pnpm check:pm-governed-merges` | 0 | | `pnpm check:refd-timer-probe` | 0 | | `pnpm check:role-word` | 0 | | `pnpm check:skill-compatibility` | 0 | | `pnpm check:skill-frame-sync` | 0 | | `pnpm check:skill-identifier-liveness` | 0 | | `pnpm check:watch-hint-literal` | 0 | | `pnpm --filter @objectstack/spec run check:skill-refs` (extra, not derived; AGENTS.md names it for a `SKILL.md` edit) | 0 | `--ran` reconciliation at `a87b7380`: 24 derived, 24 run, 0 NOT-MEASURED, 0 UNRUN (`dispatch-gates --ran`, exit 0). Ratchet family: `check-skills-token-ratchet` prints `skills/objectstack-query/SKILL.md is 3990 tokens (ceiling 5552; headroom 1562)`. `check:doc-formula-expressions` needs `@objectstack/formula` and `@objectstack/lint` built; they were built under `os-verify-lock.sh` before the gate ran (turbo cache hit, VERDICT command-exit 0), so this round it passed on its first run. At the earlier head `f997cf8e` the same 24 were green too (that round's first `check:doc-formula-expressions` run exited 3 for the missing build, NOT MEASURED, and passed after the build). `check:skill-docs` reads frontmatter only (`packages/spec/scripts/build-skill-docs.ts`), so no generated artifact moves for a body edit; `skills/README.md` and `content/docs/ai/skills-reference.mdx` stay byte-identical. No package build or test is owed: the diff touches no package (`turbo ls --affected` is blind here by construction — the skill lives outside the package graph). A repo-wide `pnpm lint` (`eslint . --no-inline-config`) is CI-owned; it was not run locally and is not claimed (NOT MEASURED locally, CI reports it). Population reading from eslint's own configuration: `pnpm exec eslint --print-config skills/objectstack-query/SKILL.md` prints `undefined` (no config block matches the file), and every `files:` glob in `eslint.config.mjs` is a `{ts,tsx,mts,cts,js,jsx,mjs,cjs}` pattern, so the one edited file is outside eslint's population and this diff cannot move any lint verdict on any file. ## Acceptance notes - The first round's one out-of-scope observation — `SKILL.md:38-42` said all six driver passthrough keys are illegal on `aggregate`, while `timezone` is legal and read there — is fixed in this PR by the patch round `a87b7380`, under the bounded in-place exemption (see *In-place fix* above). - The `content/docs/**` half — `query-syntax.mdx:1337` and `queries.mdx:672` enumerate the aggregate option set as "only `where` / `groupBy` / `aggregations`" — is carded as **#20792** (filed for triage; `domain:devx` by the lane table), carrying the evidence and the round-3 dev's proposed patch. `Fixes #20500` closes the card over the `skills/**` half, which is the card's own defect; the docs half lives on #20792. - Noted, not carded: the doc comment on `ENGINE_DRIVER_PASSTHROUGH_KEYS` (`packages/objectql/src/engine.ts:507-513`) still says the six keys are "deliberately NOT legal" on `count` / `aggregate` — stale by `timezone` on `aggregate`; a code comment, not a shipped surface (carrier: the next edit of that block). ## 维护者速读(草稿) - **改了什么**:`skills/objectstack-query` 技能里「Calling Convention」表的 engine `aggregate` 一行,合法选项键补上 `search`、`searchFields`,并加一句:这两个键在分组之前过滤输入行,与 `where` 取交集,行为与 `find` 一致。另将同段落里「六个直通键在 count / aggregate 上一律非法」的表述修正为 timezone 例外(aggregate 自己读取它做日期分桶,count 仍拒绝);`rules/aggregation.md` 里「six keys」这一数字去掉(表里已是八个键)。技能包净 +2 行。`content/docs` 里同样写错的两句另立 #20792,不在本 PR。 - **为什么改**:PR #20487 落地后,运行时的 `aggregate` 已经接受并执行 `search` / `searchFields`,技能却还写着不接受。AI 读了这份技能会绕路:先查一页再在客户端分组,得到的分组计数只覆盖一页而不是全部命中行,且没有任何报错。写给 AI 的技能说错一句等于产品缺陷。 - **风险与代价(含回滚)**:纯文档改动,不动代码、不动发布包(`skills/**` 不在任何 npm 包的 `files[]` 里,脚手架项目从仓库路径拉取)。token 棘轮上限未动(SKILL.md 3915 → 3990 / 5552;aggregation.md 1846 → 1845 / 2357)。回滚即 revert 本 PR。 - **席位意见**: - **你要做的**:确认这一行的措辞,批准后由席位落地(受管面 Tier H,草稿 PR)。 --- _Generated by [Claude Code](https://claude.ai/code/session_01KTZmMfzVzjNvyaLyQ8mHvg)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7a09eee commit 80dc9c0

2 files changed

Lines changed: 5 additions & 3 deletions

File tree

‎skills/objectstack-query/SKILL.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ metadata:
2323
| Surface | Shape | Legal option keys |
2424
|:--|:--|:--|
2525
| engine `find` / `findOne` | `engine.find('task', {…}, { context })` | `context`, `where`, `fields`, `orderBy`, `limit`, `offset`, `search`, `searchFields`, `expand` — **plus** the six driver passthrough keys `transaction`, `tenantId`, `tenantIds`, `timezone`, `bypassTenantAudit`, `preserveAudit` |
26-
| engine `aggregate` | `engine.aggregate('deal', {…})` | `context`, `where`, `groupBy`, `aggregations`, `having`, `timezone` |
26+
| engine `aggregate` | `engine.aggregate('deal', {…})` | `context`, `where`, `groupBy`, `aggregations`, `having`, `timezone`, `search`, `searchFields` — the two search keys filter the input rows **before** grouping, AND-ed with `where`, exactly as on `find` |
2727
| engine `count` | `engine.count('task', {…})` | `context`, `where` |
2828
| protocol / REST | `findData({ object: 'task', query: {…} })` | `object` sits OUTSIDE the query |
2929
| nested `expand` value | a `QueryAST` — `{ object, fields, where }` | (see **Expand**) |
@@ -39,7 +39,9 @@ The passthrough six ride along on `find`/`findOne` (and on `update`/`delete`)
3939
because there the option bag IS the base of the driver options, which is how an
4040
explicit `tenantId` reaches the driver. `count` and `aggregate` never forward the
4141
bag, so on those two the same keys are deliberately ILLEGAL — accepting them
42-
would be the silently-ignored option this check exists to close.
42+
would be the silently-ignored option this check exists to close. The one
43+
exception is `timezone`: `aggregate` reads it itself, for date bucketing, so it
44+
is legal there and the row above lists it; `count` refuses it with the rest.
4345

4446
### Which filter dialect?
4547

‎skills/objectstack-query/rules/aggregation.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const rows = await engine.aggregate('sale', {
4646
```
4747

4848
Never list the grouped fields in `fields`: drivers auto-select every grouped
49-
field into the result rows, and `fields` is not one of the six keys
49+
field into the result rows, and `fields` is not one of the keys
5050
`engine.aggregate()` accepts — it is rejected by name (see the calling
5151
convention in `SKILL.md`).
5252

0 commit comments

Comments
 (0)