Skip to content

docs(skills): objectstack-query — engine aggregate row admits search / searchFields - #20776

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-20500-query-skill-aggregate-search
Sep 30, 2026
Merged

os-zhuang merged 3 commits into
mainfrom
claude/issue-20500-query-skill-aggregate-search

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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

维护者速读(草稿)

  • 改了什么:skills/objectstack-query 技能里「Calling Convention」表的 engine aggregate 一行,合法选项键补上 search、searchFields,并加一句:这两个键在分组之前过滤输入行,与 where 取交集,行为与 find 一致。另将同段落里「六个直通键在 count / aggregate 上一律非法」的表述修正为 timezone 例外(aggregate 自己读取它做日期分桶,count 仍拒绝);rules/aggregation.md 里「six keys」这一数字去掉(表里已是八个键)。技能包净 +2 行。content/docs 里同样写错的两句另立 [finding] two content/docs sentences enumerate the engine aggregate option set as "only where / groupBy / aggregations" — false by context, having, search, searchFields (the docs half of #20500) #20792,不在本 PR。
  • 为什么改:PR feat(spec,objectql,metadata-protocol): grouped and aggregated queries honour search #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

…/ searchFields

The Calling Convention row for engine `aggregate` stopped at `timezone`,
while `ENGINE_AGGREGATE_OPTION_KEYS` admits `search` and `searchFields`
and `aggregate()` expands them through the same ADR-0061 expander `find`
uses. Together with the closed-set sentence the row told an agent that a
searched aggregate is refused. The row now lists both keys and says they
filter the input rows before grouping, AND-ed with `where`.

Claude-Session: https://claude.ai/code/session_01KTZmMfzVzjNvyaLyQ8mHvg
Co-authored-by: Claude <noreply@anthropic.com>
… aggregate accepts

The passthrough paragraph said all six driver passthrough keys are illegal
on `count` and `aggregate`. `timezone` is one of the six and is a legal
option on `aggregate` in its own right — `ENGINE_AGGREGATE_OPTION_KEYS`
lists it and `aggregate()` reads it for date bucketing — which the row
above already showed. The paragraph now carves that one key out; `count`
still refuses it with the other five.

Claude-Session: https://claude.ai/code/session_01KTZmMfzVzjNvyaLyQ8mHvg
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Check Changeset is red on head a87b7380 — skills seat 1, session_01KTZmMfzVzjNvyaLyQ8mHvg, 2026-09-30T04:42Z.

  • What fails: the check's own verdict line — "This PR adds no changeset … if it releases nothing (including any 'skills/**' change …), apply the 'skip-changeset' label". This PR releases nothing: the dev measured that no packages/**/package.json files[] names a skills path (report 5904112375 on [finding] skills/objectstack-query/SKILL.md tells an agent that engine aggregate refuses search / searchFields — false since PR #20487 #20500), so the correct fix is the label, not a changeset.
  • What blocks it: the dev's label write (skip-changeset + PR assignee os-warren) was refused by its session's permission classifier before any request left the container. The seat has put that item to the maintainer rather than performing it on the dev's behalf; the label and assignee go on once the maintainer answers.
  • The earlier red TypeScript Type Check / Test Core rows on f997cf8e are the supersession signature (members cancelled by the push of a87b7380), not failures of this PR.
  • The PR stays draft (Tier H, skills/**); the seat's review of the patch round is pending.

Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: a87b7380b91dc3bb7b57c979d6e55551177c746e
Local-runs: none

Inputs read: card #20500 (body + all 5 comments: triage 5877514141, claim 5903909164, dev report 5904112375, claim-surface amendment 5904148643, dev report 5904236951), PR #20776 (body, file list, diff endpoint against main), the check-runs on the head, the PR thread (1 comment, 5904191919), and the repository at origin/main 91e8fa194e / at the head via git show / git grep. Merge-base is c9c182ed; neither packages/objectql/src/engine.ts nor skills/objectstack-query/** moved between the merge-base and origin/main, so every reading below is a reading of main. The PR's own side is one file, skills/objectstack-query/SKILL.md, +4/-2, two hunks; no .changeset rows added or changed.

Check-runs on a87b7380, read 2026-09-30T05:04:14Z (third poll; unchanged since 04:5x): 38 runs — 26 success, 9 skipped, 2 failure, 1 in_progress.

  • Check Changeset — failure, twice (suite 99308323233 at 04:41Z and suite 99311420641 at 04:56Z). It is the pr-automation.yml gate: the PR adds no changeset and carries no skip-changeset label (labels at read time: documentation, size/xs; assignees empty). Advisory — not one of the seven required contexts — but the label it wants is owed (see ② and ③).
  • Lint & Repo Gates — in_progress (suite 99308323090, started 04:41:22Z, still running at 05:04Z). This is a REQUIRED context; it is not a pass. It carries pnpm lint, check:skill-identifier-liveness, check:doc-formula-expressions and the rest of the check:* families. The skill-specific gates (check-skills-token-ratchet + self-test, check:skill-docs, check:skill-refs, check:skill-frame-sync, check:skill-compatibility) run in typecheck-source-gates, whose check-run Type Check · source gates is success on this head.
  • Required seven: Lint & Repo Gates in_progress · TypeScript Type Check success · Test Core success · Dogfood Regression Gate success · Build Core skipped · Temporal Conformance (live PG + MySQL) skipped · Governed Surface Queue Guard success.

① Derived judgments

The diff changes no accept set and no public API: it edits a published skill's text only. The public surface that moves is the text an agent reads, so each changed statement is judged against packages/objectql/src/engine.ts on main.

  1. Row :26 now lists search, searchFields on engine aggregate — RIGHT. ENGINE_AGGREGATE_OPTION_KEYS (engine.ts:562-565) is context, where, groupBy, aggregations, having, timezone, search, searchFields; aggregate() rejects against exactly that set at :16272. Corroborated by EngineAggregateOptionsSchema (packages/spec/src/data/data-engine.zod.ts:352-), which declares search and searchFields after timezone.
  2. In-row clause "the two search keys filter the input rows before grouping, AND-ed with where, exactly as on find" — RIGHT. aggregate() calls expandSearchOnAggregateOptions at :16277, after the where doors and before the AST is built or any grouping runs; that method (:11104-11114) is a carrier into the one ADR-0061 expander expandSearchOnAst (:11062-11086) that find (:11266) and findOne (:11557) also call; the expander sets ast.where = ast.where ? { $and: [ast.where, searchFilter] } : searchFilter and deletes both search keys. Same expander, same AND composition, same point in the sequence as on find.
  3. Closed-set sentence :31 (untouched) — still RIGHT once the row is complete: rejectUnknownEngineOptions (:748) throws naming the key and the legal set; nothing is ignored.
  4. Paragraph :38-44 — RIGHT on every clause. "count and aggregate never forward the bag": count() builds driver options from buildDriverOptions(object, opCtx.context) (:16192), aggregate() likewise (:16624, :16668) — the bag is not forwarded on either. "The one exception is timezone: aggregate reads it itself, for date bucketing": const tz = query.timezone (:16598) drives tzRequiresInMemory (:16600) and is passed to applyInMemoryAggregation(raw, ast, tz, declaredFields) (:16669) — read and consumed, not a passthrough. ENGINE_AGGREGATE_OPTION_KEYS holds timezone and none of transaction, tenantId, tenantIds, bypassTenantAudit, preserveAudit. "count refuses it with the rest": ENGINE_COUNT_OPTION_KEYS is context, where (:556), rejected against at :16168. Lines :38-41 are byte-identical to main (the hunk's context lines); :42 is extended in place and :43-44 are new — exactly as the report says.
  5. fields / orderBy sentence (:254, untouched) — still RIGHT; neither key is in the aggregate set.
  6. Consistency inside the same skill — WRONG, one sentence. skills/objectstack-query/rules/aggregation.md:48-51 reads: "fields is not one of the six keys engine.aggregate() accepts — it is rejected by name (see the calling convention in SKILL.md)". The row it points at now lists eight keys. The count was already stale on main (the engine has accepted eight since the landing of feat(spec,objectql,metadata-protocol): grouped and aggregated queries honour search #20487) but agreed with the stale row; after this diff the skill contradicts itself on the very fact the card is about, and the sentence cross-references the row, so an agent reading the rule sees "six", follows the pointer, and finds eight. The dev's census (four list-shaped probes naming keys) could not see a sentence that names a count, which is why "no other enumeration" was concluded. The dispatch's claim surface already covers it: claim 5903909164 names "only if the census finds another enumeration of the aggregate option keys, that line in skills/objectstack-query/**". Remedy: one line in rules/aggregation.md:49 — drop the number ("not one of the keys engine.aggregate() accepts") or write "eight"; net 0 lines, no re-wrap, inside the token ceiling. This is the reason for the verdict below.
  7. No other sentence in the skill is contradicted. rules/aggregation.md:104 ("where filters the input rows before grouping") is the vocabulary the new clause reuses and agrees with it; rules/filters.md, rules/pagination.md, references/_index.md and evals/filters-pagination-search.json carry no statement about aggregate and search, timezone or the passthrough keys (grep at main, and at the head, over the whole skill directory).

② Semver level

  • skip-changeset is the right declaration; no changeset is owed. The diff publishes nothing from any released package: no packages/**/package.json files[] names a skills path (every manifest scanned at origin/main; zero hits), skills/ is not a pnpm-workspace.yaml member, and scaffolded projects fetch the catalog from the repository path objectstack-ai/objectstack/skills (SKILLS_CATALOG, packages/create-objectstack/src/skills-install.ts:62), never from a tarball. The gate's own text (pr-automation.yml, route 2) rules the same way: "'skills/**' is on that list … Take the label."
  • Clause-②: no — RIGHT and well-formed. No accept set widens or narrows; no arm is attached. The line sits in the PR body as the first line after Fixes #20500.
  • What is not yet true on the PR: the label is absent, so Check Changeset is red on both suites. The red is the missing label, not the diff. Applying skip-changeset fires a labeled run that goes green; the two stale reds do not clear themselves and need no re-run to be explained.

③ Boundary flags

Dev report 5904112375 (round 1):

  • Deviation "label-write refused" (skip-changeset + assignee os-warren on PR docs(skills): objectstack-query — engine aggregate row admits search / searchFields #20776): still owed at read time (labels documentation, size/xs; assignees empty). The seat has already put it to the maintainer (PR comment 5904191919). Not a defect in the diff; it is the whole cause of the Check Changeset reds. Answered: escalated, unchanged.
  • Deviation "check:doc-formula-expressions needed a build first": procedural, not a finding. CI's own verdict for that family is inside Lint & Repo Gates, which had not concluded at read time — so that family is unanswered on this head, honestly, not red.
  • open_questions: none.
  • Out-of-scope finding (the timezone overstatement in :38-42): taken in-place under amendment 5904148643 and verified right against the code in ① item 4. Answered.
  • Noted-not-filed (content/docs/kernel/runtime-services/data-service.mdx:91): a QueryAST clause list, not an engine option-key list per the dev's reading; no action required by this PR. Not re-read here.

Claim-surface amendment 5904148643 (the seat): the four in-place conditions hold as stated — same defect class (both statements are about the aggregate legal option set), mechanical pinned form (a one-key carve-out), same gate family (docs-only, same 24 derived commands). Condition ③ (no other claim on the file) is the seat's own serial-constraints reading; the mechanical counterpart on this head, No other open PR may claim the same single-writer path, is success. The amendment named :38-41; the dev's correction to :38-42 is right (the closing line is :42) and is not a scope change.

Dev report 5904236951 (round 2):

  • Deviation ":38-:42 not :38-:41": answered above, correct reading.
  • Deviation "label-write still undone": answered above, still owed.
  • open_questions: none.
  • out_of_scope_findings: "none new" — incomplete, two findings the census missed:
    1. In-skill, in-surface, blocking: skills/objectstack-query/rules/aggregation.md:49 "six keys" — ① item 6. Fix in this PR under the dispatch's own conditional clause; then re-review the new head.
    2. Out of the claim surface — escalated, not for this PR: content/docs/protocol/objectql/query-syntax.mdx:1336-1338 says "engine.aggregate() accepts only where / groupBy / aggregations (plus a timezone for bucketing)". Against engine.ts:562-565 it omits context, having, search, searchFields, and the word "only" makes it a false enumeration of the same set this card corrects. The card's "Where the fix lands" named content/docs/** for the census; the dispatch scoped the file surface to skills/**, so this is a class (a) finding with a named landing site for the seat to file (docs lane), not a rider on this PR. Dedupe words: query-syntax.mdx aggregate accepts only · engine.aggregate option keys docs enumeration having search · revenue by month aggregate only where groupBy aggregations.

Observation for the objectql lane, not a flag on this PR: the doc comment on ENGINE_DRIVER_PASSTHROUGH_KEYS (engine.ts:507-513) says the six keys are "deliberately NOT legal" on count/aggregate, which is stale by one key against the set at :562 (timezone on aggregate) — the same overstatement the skill paragraph carried. A code comment, not a customer surface; the judgments above were made against the code, as the brief requires.

Governance: skills/** is Tier H. This record does not lift the landing; the maintainer's approval does, after the fix above and once every check is green (Lint & Repo Gates concluded, skip-changeset applied so Check Changeset is answered by a labeled run). The PR is draft with auto_merge: null, as it must be.

Implemented-by: claude/issue-20500-query-skill-aggregate-search
Reviewed-by: session_01KTZmMfzVzjNvyaLyQ8mHvg

VERDICT: FAIL

One line to fix, inside the claim surface: skills/objectstack-query/rules/aggregation.md:49 still says engine.aggregate() accepts "six keys" while the row it points at now lists eight. Everything the diff itself states is true of the engine on main; the changeset declaration is right; the label and the required Lint & Repo Gates conclusion are outstanding state, not diff defects.

… aggregate keys

`rules/aggregation.md` said `fields` is not one of the "six keys"
`engine.aggregate()` accepts and pointed at the calling-convention row,
which now lists eight. A count drifts every time the set moves; the
sentence now says "not one of the keys" and leaves the enumeration to the
row it cites. Net zero lines, no re-wrap.

Claude-Session: https://claude.ai/code/session_01KTZmMfzVzjNvyaLyQ8mHvg
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: cc9244482c23099bb460224dc4681a51cbd9e82e
Local-runs: none

Inputs read: card #20500 (body and all 8 comments: triage 5877514141, claim 5903909164, dev reports 5904112375 / 5904236951 / 5904713155, claim-surface amendments 5904148643 / 5904535717, round-3 state 5904782579), PR #20776 (body, file list, the diff endpoint against main), the check-runs on cc924448, the PR thread (5904191919 and the previous record 5904504985, FAIL on a87b7380), and the repository read only through git show / git grep at the head and at origin/main 085ca6bc (fetched into owned refs). Merge-base is c9c182ed. Between the merge-base and main, packages/objectql/src/engine.ts moved 20 lines, all inside the migration-flag reads (find to findOne at :9606, :9780, :9883) — none in the option-key sets, in count() / aggregate(), or in the search expander; skills/objectstack-query/** did not move. So every reading below is a reading of current main. The PR's side is two files, three hunks, +5/−3: skills/objectstack-query/SKILL.md (+4/−2) and skills/objectstack-query/rules/aggregation.md (+1/−1); no .changeset row added or changed; head repo = base repo (not a fork); draft, auto_merge null, assignees empty, labels documentation + size/xs.

Check-runs on cc924448 (31 runs, read this round after the last completion at 05:34:38Z): 23 success, 7 skipped, 1 failure, 0 in_progress.

  • Check Changeset — failure (job 109749545697, pr-automation.yml). Its annotation: the PR adds no changeset and carries no skip-changeset label; its own text rules that a change that "releases nothing (including any 'skills/**' change)" takes the label. Advisory (not one of the required seven). The red is the missing label, not the diff — see ② and ③.
  • Skipped, all by path filter, none a fault: Build Core and Temporal Conformance (live PG + MySQL) (both required contexts; both gated needs.filter.outputs.core != 'false' in ci.yml, and skills/** sits in the crosspkg list, not core, so a skills-only diff skips them by design); Build Docs (docs filter — no content/** at this head); Console Pin Gate (console filter); Dogfood Regression Gate (matrix) and Dogfood Verify CLI (core filter; the gate job Dogfood Regression Gate runs if: always() and is success); Packed-tarball smoke (opt-in).
  • Required seven: Lint & Repo Gates success (the context the previous record found in_progress on a87b7380; it concluded 05:34:38Z on this head) · TypeScript Type Check success · Test Core success · Dogfood Regression Gate success · Build Core skipped · Temporal Conformance (live PG + MySQL) skipped · Governed Surface Queue Guard success — this last one is the pull_request leg (types opened, synchronize, reopened, ready_for_review); the merge_group leg is the one that refuses, so its success is not a landing clearance.
  • Also success: Type Check · source gates (carries check-skills-token-ratchet, check:skill-docs, check:skill-refs, check:skill-frame-sync, check:skill-compatibility), No other open PR may claim the same single-writer path, The card this PR closes must claim this branch, Part-of PR must not also close its card, Check PR Size.

① Derived judgments

The diff changes no accept set and no public API: no code, no schema, no generated artifact, no changeset. The public surface that moves is the text of a published skill (skills/** ships verbatim to scaffolded projects), so each changed statement is judged against packages/objectql/src/engine.ts on main 085ca6bc.

  1. SKILL.md:26 — the engine aggregate row now lists context, where, groupBy, aggregations, having, timezone, search, searchFields — RIGHT. ENGINE_AGGREGATE_OPTION_KEYS (engine.ts:562-565) is exactly those eight; aggregate() rejects against it at :16274 through rejectUnknownEngineOptions (:748-770), which throws naming the offending key and the sorted legal set. Corroborated by EngineAggregateOptionsSchema (packages/spec/src/data/data-engine.zod.ts:352), which declares search and searchFields after timezone.
  2. SKILL.md:26 in-row clause "the two search keys filter the input rows before grouping, AND-ed with where, exactly as on find" — RIGHT. aggregate() calls expandSearchOnAggregateOptions at :16278, after the where doors (foldEngineOptionAliases, rejectUnknownEngineOptions, lowerWhereFilterArray) and before the aggregate AST is built or any middleware runs. That method (:11104-11114) builds a carrier { object, where, search, searchFields } and calls expandSearchOnAst (:11062-11086) — the one ADR-0061 expander that find (:11266) and findOne (:11557) call — which sets ast.where = ast.where ? { $and: [ast.where, searchFilter] } : searchFilter and deletes both search keys. Same function, same AND composition, same server-side field resolution (searchFields ?? raw.fields, intersected with the searchable set) on both verbs.
  3. SKILL.md:42-44 new sentence "The one exception is timezone: aggregate reads it itself, for date bucketing, so it is legal there and the row above lists it; count refuses it with the rest" — RIGHT on every clause. timezone is in ENGINE_AGGREGATE_OPTION_KEYS; aggregate() reads const tz = query.timezone at :16600, it drives tzRequiresInMemory (:16602, only when a groupBy node carries dateGranularity) and is passed to applyInMemoryAggregation(raw, ast, tz, declaredFields) at :16671 — read and consumed, not forwarded. ENGINE_COUNT_OPTION_KEYS is context, where (:556) and count() rejects against it at :16170, so timezone on count throws. The other five passthrough keys (transaction, tenantId, tenantIds, bypassTenantAudit, preserveAudit) are absent from the aggregate set.
  4. SKILL.md:40-42 retained clause "count and aggregate never forward the bag, so on those two the same keys are deliberately ILLEGAL" — RIGHT once the carve-out follows it. count() builds its driver options from buildDriverOptions(object, opCtx.context) (:16192); aggregate() likewise on both paths (:16626 native drv.aggregate, :16670 in-memory driver.find) — the option bag is never the base of the driver options on either verb. Lines :38-41 are byte-identical to main (the hunk's context lines); :42 is extended in place; :43-44 are new.
  5. rules/aggregation.md:49 "fields is not one of the keys engine.aggregate() accepts" — RIGHT, and the previous FAIL's cause is gone. fields is not in the set; the word "six" is dropped, so the sentence no longer states a count the row it cites can contradict; its cross-reference to the Calling Convention row now resolves to a row it agrees with. Net 0 lines, neighbours byte-identical to main.

Untouched sentences that depend on the changed ones, re-judged: :31 closed-set sentence — still RIGHT (rejectUnknownEngineOptions refuses by name; nothing is ignored). :25 find / findOne row — RIGHT against ENGINE_FIND_OPTION_KEYS (:543-547: context, where, fields, orderBy, limit, offset, search, searchFields, expand, plus the six of ENGINE_DRIVER_PASSTHROUGH_KEYS :515-517). :27 count row — RIGHT. :38 "(and on update/delete)" — RIGHT (ENGINE_UPDATE_OPTION_KEYS :548, ENGINE_DELETE_OPTION_KEYS :552 spread the passthrough six). :254 "fields and orderBy are NOT in ENGINE_AGGREGATE_OPTION_KEYS" — RIGHT. :314-316 "one knob, three spellings" — RIGHT (the expander reads searchFields ?? raw.fields).

Self-contradiction sweep, all six files under skills/objectstack-query/** at cc924448: SKILL.md and rules/aggregation.md read in full; rules/filters.md, rules/pagination.md, references/_index.md, evals/filters-pagination-search.json grepped for aggregat, searchFields, timezone, passthrough, whole-word six, option key, accepts, refus, ILLEGAL, closed set. Hits: filters.md:225 (token resolution wired into find/findOne/count/aggregate/update/delete — where tokens expand, not an option set); pagination.md:202-204 (an aggregate call shape with groupBy + aggregations — consistent); evals:27 ("No fields / orderBy in the aggregate bag" — consistent with :254 and the rule); evals:37 (find with search + searchFields — consistent); _index.md none. Every remaining number word counts a set of that size: "All six are tombstoned" (:82, six rows of the removal table), "The six functions" (:237, aggregation.md:16), "the six driver passthrough keys" (:25, :38, six entries at :515-517). No sentence in the skill contradicts another, and none contradicts the engine on main. The four untouched files are byte-identical between main and the head. Generated artifacts: none owed — check:skill-docs reads frontmatter only, the frontmatter is untouched, and Type Check · source gates is success on this head.

Commit hygiene: the three commits carry the model-free trailer pair (Claude-Session + Co-authored-by: Claude); no model identifier in any title or body.

② Semver level

  • skip-changeset is the right declaration; no changeset is owed. The diff publishes nothing from any released package: every packages/**/package.json on main 085ca6bc was scanned and zero files[] entries name a skills path; skills/ is not a pnpm-workspace.yaml member; scaffolded projects fetch the catalog from the repository path (SKILLS_CATALOG = 'objectstack-ai/objectstack/skills', packages/create-objectstack/src/skills-install.ts:62), never from a tarball. The gate's own annotation on this head rules the same way.
  • Clause-②: no — RIGHT and well-formed. No accept set widens or narrows; no arm attached; the line is the first after Fixes #20500.
  • State, not diff: the label is absent, so Check Changeset is failure. Applying skip-changeset fires a labeled run that reads the labels live (pr-automation.yml:344) and goes green; nothing else is needed for that gate.

③ Boundary flags

Dev report 5904112375 (round 1). Deviation "label-write refused" (skip-changeset + assignee os-warren on the PR): still owed at this read; escalated by the seat in PR comment 5904191919; unchanged; it is the whole cause of the one red check. Deviation "check:doc-formula-expressions needed a build first": procedural; CI's own verdict for that family is inside Lint & Repo Gates, success on this head. open_questions: none. Out-of-scope finding (the timezone overstatement): taken under amendment 5904148643, landed at a87b7380, verified RIGHT (① items 3-4). Noted-not-filed content/docs/kernel/runtime-services/data-service.mdx:91: re-read on main — it lists the QueryAST clauses (where/fields/orderBy/limit/offset plus the AST-only search, expand, aggregations, groupBy, having); it names search and is the AST's clause list, not the engine option set. Correct; no action.

Amendment 5904148643 (seat). The four in-place conditions held (previous record); the mechanical counterpart of condition ③, No other open PR may claim the same single-writer path, is success on this head as well.

Dev report 5904236951 (round 2). Deviation ":38-:42 not :38-:41": right reading. Deviation "label still owed": answered above. open_questions: none. out_of_scope_findings: none new was incomplete (the previous record found rules/aggregation.md:49 and query-syntax.mdx:1336-1338): the first is fixed at cc924448 (① item 5); the second entered the claim surface by amendment 5904535717 and is the unlanded half below.

Amendment 5904535717 (seat). content/docs/** taken on the claiming seat's follow-through path, not by moving the card; the ENGINE_DRIVER_PASSTHROUGH_KEYS doc comment (engine.ts:507-513) excluded — right, a code comment is not a shipped surface. Confirmed stale by one key on main ("count/aggregate never forward the bag, so these keys are deliberately NOT legal there" — timezone is legal on aggregate): the objectql lane's, not this PR's.

Dev report 5904713155 (round 3, status rework). Classifier refusal 1 (read-only byte readings of the two docs files refused): the consequence is confined to the unlanded half; nothing in this diff is unmeasured — the skills readings at cc924448 are stated (SKILL.md 401 lines / 3990 tokens, ceiling 5552; aggregation.md 241 / 1845, ceiling 2357) and check-skills-token-ratchet sits inside the green Type Check · source gates. Classifier refusal 2 (commit + push of the two content/docs edits refused): so the docs half is not on the remote and the PR's file list is two files; not retried on any route, which is the os-dev rule; the item is with the maintainer (5904782579). check:skill-examples first exit 3: prerequisite, procedural. The docs census's second hit, content/docs/data-modeling/queries.mdx:672: premise verified on main — packages/metadata-protocol/src/protocol.ts:11324-11343, the one wire path into engine.aggregate, forwards where, groupBy, aggregations, having, search, searchFields, context and not timezone; so "forwards only where / groupBy / aggregations" is stale by four keys, "does not forward timezone into the aggregate call" is the true fact, and the callout's conclusion (REST buckets on UTC) stands — inside amendment 2's wording; judged for the premise only, since the edit is not in this diff. The 24-family record at cc924448 not reconciled with --ran on its own: honest; CI on cc924448 answers every derived family (Lint & Repo Gates and Type Check · source gates success). open_questions: none. Out-of-scope: the engine.ts:507-513 comment (objectql lane, confirmed above); content/docs/references/data/data-engine.mdx:143 and :701 (confirmed on main: the aggregate query shape is printed with an ellipsis, generated — nothing to hand-edit).

Round-3 state 5904782579 (seat) matches what was read: cc924448 carries the whole skills/** half; the content/docs/** half is unlanded; the verbatim old-to-new text of both edits is preserved in 5904713155 docs_patch.

Is the PR complete for the card at this head? Plainly:

  • Complete for the card's defect and for the published skill. The row the card names (SKILL.md:26), the two other sentences inside skills/objectstack-query/** that stated the same set (the passthrough paragraph :38-44 and rules/aggregation.md:49), and no other statement of the aggregate option set anywhere in the six files. Nothing in skills/** is left.
  • Not complete for the claim surface as amended. Amendment 5904535717 put content/docs/protocol/objectql/query-syntax.mdx:1336-1338 and any other content/docs/** enumeration inside the surface. On main 085ca6bc that sentence still reads "engine.aggregate() accepts only where / groupBy / aggregations (plus a timezone for bucketing)" — false by context, having, search, searchFields against engine.ts:562-565 — and content/docs/data-modeling/queries.mdx:671-672 still reads "route forwards only where / groupBy / aggregations" — false by having, search, searchFields, context against protocol.ts:11324-11343. Both are customer-facing docs pages, and the card body's own "Where the fix lands" names content/docs/** for exactly this census. The PR carries neither.
  • Before Fixes #20500 may close the card, exactly one of these must be on record:
    (a) the docs commit lands on this PR — the two one-for-one line replacements in 5904713155 docs_patch, pushed to the branch from the dev's worktree or re-typed from the verbatim text. That moves the head: the body is then rewritten for the four-file form (the dev's pr_body_replacements text) and a new contract review is rendered on the new head; the gate derivation grows to 47 families and CI runs the docs-path jobs (Build Docs, the content/** gates).
    (b) the docs half is filed as its own card — class (a), reach: two published docs pages, evidence: the two sentences with their line numbers against engine.ts:562-565 and protocol.ts:11324-11343, carrying the verbatim patch and the dedupe words from 5904535717 — and amendment 5904535717's inclusion of content/docs/** is withdrawn on the card by a Release:-style line so the claim surface and the PR agree; the PR body's Acceptance notes then name that card, and this PR closes the card over the skills half.
    Until one is on record, Fixes #20500 in the body is premature: a merge would close the card while a half the claim took is neither landed nor carded, which the seat's own state comment says must not happen silently.

PR body — what would mislead a merger, on cc924448. The body was written for a87b7380 and must be rewritten before the maintainer's word is sought: "What changed" names one file in two places (the diff is two files in three; the rules/aggregation.md:49 fix is absent from the body); the Census concludes "no other enumeration … no third landing site" (false: a third was found in-skill and fixed, and two more in content/docs); the Acceptance notes say "nothing is left for a card" (false at this head — the docs half is left, for route (a) or (b)); and the 维护者速读 "你要做的" bullet does not tell the maintainer that the refused docs write is the decision waiting on them. The Gates table's head (a87b7380) and the readings table's "After" column are stale but not misleading on the verdict, since the check-runs on cc924448 answer them and SKILL.md's numbers did not move in round 3.

Governance. skills/** is Tier H (GOVERNED_SURFACES row skills-catalog, scripts/pm/check-governed-merges.mjs:1095). This record does not lift the landing; an authorized APPROVED review does — to be requested only after the body rewrite, the closing-keyword question settled by (a) or (b), and skip-changeset applied so every check is green. The PR is draft with auto_merge null and head repo = base repo, as it must be. This PASS is a verdict on the diff at cc924448; it is not a finding that the card is done.

Implemented-by: claude/issue-20500-query-skill-aggregate-search
Reviewed-by: session_01KTZmMfzVzjNvyaLyQ8mHvg

VERDICT: PASS

Every statement the diff makes is true of packages/objectql/src/engine.ts on main, the skill no longer contradicts itself anywhere under skills/objectstack-query/** (the previous FAIL's rules/aggregation.md:49 is fixed), the changeset declaration is right, and every required check that runs is green. Outstanding state, not diff defects: the skip-changeset label (the one red check), the stale body, and the unlanded content/docs/** half — which must land here or be carded before Fixes #20500 closes the card.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

维护者速读(终稿)— PR #20776 · query 技能:aggregate 的合法选项键补全(#20500)

skills 席 1 · session_01KTZmMfzVzjNvyaLyQ8mHvg · 2026-09-30T06:26Z · 所审 head cc924448

改了什么:只改 skills/objectstack-query 下两个文件,共三处(+5 / −3,技能包净 +2 行)。

  • 「Calling Convention」表里 engine aggregate 一行,合法选项键补上 search、searchFields,并注明这两个键在分组之前过滤输入行,与 where 取交集,和 find 一致。
  • 同段原文说「六个直通键在 count / aggregate 上一律非法」,改为 timezone 例外:aggregate 自己读它做日期分桶,count 仍然拒绝。
  • rules/aggregation.md 里「six keys」这个数字去掉。表里现在是八个键,写死的数字会再次过期。

为什么改:PR #20487 之后,运行时的 aggregate 已经接受并执行 search / searchFields,技能却还写着会拒绝。AI 读了会绕路:先查一页,再在客户端分组。这样得到的分组计数只覆盖一页而非全部命中行,而且不报任何错。写给 AI 的技能说错一句,等于产品缺陷。

风险与代价(含回滚):纯文档,不动代码,不发布任何包。token 上限都没动。回滚:revert 本 PR。

席位意见:ACCEPT,建议批准。

  • 三轮成形:第 1 轮补表格行;第 2 轮就地修正 timezone 例外;第 3 轮修掉达档复核找出的「six keys」。
  • 契约复核 PASS(评论 5904966795,由隔离的达档子代理出具,席位核验后采纳)。复核对照 main 上的 engine.ts 核实了每一句,并扫过技能的全部六个文件,确认没有自相矛盾。
  • content/docs 里有两句同样写错的枚举。dev 的提交被权限分类器拒了,没进本 PR,已另立 [finding] two content/docs sentences enumerate the engine aggregate option set as "only where / groupBy / aggregations" — false by context, having, search, searchFields (the docs half of #20500) #20792(附完整补丁,交分诊路由到 devx 车道)。本 PR 合并后 Fixes #20500 关卡,覆盖的是这张卡本身的缺陷,也就是发布的技能。
  • CI 除 Check Changeset 外全绿。这一红只因缺 skip-changeset 标签:本 PR 不发布任何包,本来就该挂这个标签。dev 挂标签时被拒,需要你决定是否由席位代挂(assignee os-warren 同理)。

你要做的:在本 PR 上给 APPROVED,并确认是否由席位挂上 skip-changeset 与 assignee。两者都到位后,由本席位翻 ready 并入队。


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] skills/objectstack-query/SKILL.md tells an agent that engine aggregate refuses search / searchFields — false since PR #20487

2 participants