fix(spec): EngineAggregateOptions.groupBy declares the GroupByNodeSchema union the engine already reads (#8032) - #8236
Merged
Conversation
…chema union (#8032) The engine has always read structured { field, dateGranularity, alias } buckets on this key (date bucketing, credential-aggregation guard); the declaration said string[], so every correct caller had to cast. Catch the declaration up to the enforced contract: - spec: EngineAggregateOptionsSchema.groupBy -> z.array(GroupByNodeSchema), pin tests in both directions (structured buckets parse; plain strings parse byte-identically; bad dateGranularity / field-less bucket rejected) - objectql: engine drops its own groupBy casts; the two landed test casts removed (internal-fields structured-bucket case now type-checks honestly; filter-array-lowering cast narrowed to the deliberately off-contract where slot) - mcp: McpDataBridge.aggregate retyped onto the engine's own EngineAggregateOptions slices - closes function:string vs the six-name enum, dateGranularity:string vs the five-name vocabulary, and distinct?:boolean vs the #6815 retiredKey tombstone; stdio bridge compiles with zero casts No engine/runtime behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123k4cam2jEAkPmbJeoaY3r
…upby-declared-union
…pBy union (#8032) check:generated proved exactly one artifact stale (gen:docs); regenerated that one only. gen:openapi restored after gen:schema's cleanup (untracked, kept for the local rest runs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123k4cam2jEAkPmbJeoaY3r
…upby-declared-union
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 114 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
This was referenced Aug 12, 2026
…upby-declared-union
…merge (#8032) os-regen-merge took main's side of references/**; gen:docs re-applies the three EngineAggregateOptions.groupBy rows on top of the merged state. check:generated: all 13 artifacts up to date. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123k4cam2jEAkPmbJeoaY3r
huangyiirene
marked this pull request as ready for review
August 13, 2026 01:38
huangyiirene
pushed a commit
that referenced
this pull request
Aug 13, 2026
os-regen merge lap for landing: merge origin/main (committed first, per scripts/pm/os-regen-merge.sh), then wholesale regen of the os-regen'd references/api pages from the merged tree. Sibling assertions on the regenerated tree: #8230 retirement state (def names only in retirement prose + migration entry), #8232 currency fraction-digit guard, #8234's 13 comparand-door exports in api-surface/data.json, #8236's groupBy union row all survive; MONGODB_MULTI_TENANT_UNSUPPORTED stays absent from the ledger and the references ErrorCode union reads main-minus-one (+265 -> +264; the union grew by 2 on main since the previous lap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123k4cam2jEAkPmbJeoaY3r
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8032
What
EngineAggregateOptionsSchema.groupBydeclaredz.array(z.string())while the engine has always read the structured{ field, dateGranularity, alias }bucket form on the same key (date bucketing, and the credential-aggregation guard walks both spellings) — so every correct caller had to lie to the type. The canonical union already shipped in the same package (GroupByNodeSchemainquery.zod.ts;QuerySchema.groupByuses it), makingEngineAggregateOptionsSchemathe one outlier face. This PR is the declaration catching up to the enforced contract — no engine/runtime behavior change.packages/spec:EngineAggregateOptionsSchema.groupByis nowz.array(GroupByNodeSchema), with pin tests in both directions: structured buckets parse; plain strings parse byte-identically; an out-of-vocabularydateGranularityand a field-less bucket are rejected. The RPC face (DataEngineAggregateRequestSchema) inherits the fix by reference. The deprecated legacyDataEngineAggregateOptionsSchemais deliberately untouched. Regeneratedreferences/**ride (the one artifactcheck:generatedproved stale).packages/objectql: the engine no longer casts its owngroupByreads (rejectCredentialAggregation's walk, the AST build, the date-bucket capability probe) — the declared type now admits what the code reads.packages/mcp:McpDataBridge.aggregatenow declares itsgroupBy/aggregationsinputs as the engine's ownEngineAggregateOptionsslices instead of a hand-mirrored copy. That closes the whole [finding]EngineAggregateOptions.groupByis declaredstring[]but the engine also reads{ field, dateGranularity }buckets — every correct caller must lie to the type #8032 addendum in one move:function: stringvs the six-name enum,dateGranularity?: stringvs the five-name vocabulary, anddistinct?: booleanvs the engine'sretiredKey()tombstone ([spec/drivers]AggregationNode.distinctis honoured by the in-memory fallback and ignored by every SQL face — one query, two numbers (ADR-0049) #6815) — a caller passingdistinct: truewas silently dropped before; now it fails to compile with the retirement prescription. The stdio bridge's engine call carries zero casts.The three landed cast sites (acceptance)
packages/objectql/src/internal-fields.test.ts—as unknown as EngineAggregateOptionsremoved from the structured-bucket credential case. Reverse-verified the case still fails for its ORIGINAL defect (see Verification, RV1).packages/objectql/src/engine-filter-array-lowering.test.ts— the whole-options cast was doing double duty: it also covered the deliberately off-contract FilterArraywherethis file exists to drive (its own module note says so). Theas unknown as EngineAggregateOptionson the options object is gone; the remaining cast is scoped to the one slot whose contract is deliberately bypassed (where), per the file's own documented convention.groupByandaggregationsnow type-check honestly.packages/mcp/src/stdio-data-bridge.ts— both casts (groupBy,aggregations) removed;@objectstack/mcptypechecks with the honest code.Addendum item measured (bridge
function: string→ enum)Pure declaration catch-up, included: the
aggregate_recordstool's zod schema already enforces exactly the six-name enum (and the five-name granularity vocabulary) before any value reaches the bridge, and the runtime HTTP bridge implementation types its opts parameter asany, so no accepted input changes on any path. The only behavioral delta is compile-time: a caller who read the old wide interface and passeddistinct: true(silently dropped before) is now refused bytscwith the tombstone's prescription — the fix the issue thread asked for.#4918
check:query-options-erasureratchetDirection observed: no movement. The ratchet holds at its exact baseline (67 non-test sites / 17 files; test surface at main's own ceiling). The two
as anygroupBy casts removed fromengine.tswere evidently not among the rule's counted shapes, and the removed test casts wereas unknown as, which the rule deliberately does not count — so the baseline needed no--update. Verified on both pre- and post-merge states.Changesets
@objectstack/specminor — the declared accept-set widens on the engine-options face (a request surface gains a declared form it always executed). Not breaking: every previously-valid payload validates byte-identically, so no ADR-0087 disposition is required (not a declared-breaking changeset).@objectstack/mcppatch — interface declaration catch-up; runtime acceptance unchanged on every path; body carries thedistinctFROM → TO prescription (delete the key;count_distinctis the deduplicating spelling).Verification (all local, full farm is CI's)
Round 1 (merged
main@ 30536e3): spec build/typecheck green; spec 10199 tests / 385 files passed;check:generated— exactly 1 stale (gen:docs), regenerated,gen:openapirestored aftergen:schemacleanup; downstream consumers (prefix direction — dependents of@objectstack/spec): objectql 3400/192, mcp 138/13, rest 1731/103, runtime 2261/146, all typechecks green; examples validate ×3 green; gate farm green (adr-anchors, changeset-gate-self-tests, cross-package-test-inputs, doc-formula-expressions, docs-audit-scope, i18n, merge-driver, release-body, spec-parsed-alias, nul-bytes).Round 2 (after re-merging
main@ fa48973 family): spec 10199/385, objectql 3401/192 (+1 = #8206's merged test), mcp 138/13,check:generatedgreen, erasure ratchet holds.RV1 (guard walk, runtime) — predicted: blinding
rejectCredentialAggregation's structured-bucket read turns exactly the un-cast credential case red. Observed:Tests 1 failed | 17 passed, the failing case being "rejects the flagged field as a structured {field} groupBy bucket". The case is not vacuous.RV2 (type direction, rebuilt
.d.ts) — predicted: an off-vocabularydateGranularity: 'decade'in a compiled consumer goes red naming the enum. Observed via@objectstack/mcptypecheck:TS2345 … Type '"decade"' is not assignable to type '"year" | "month" | "day" | "week" | "quarter" | undefined'— the error text quotes the new union from the freshly built spec declarations. Note: objectql's owntypecheckcannot carry this proof — its tsconfig excludes**/*.test.ts(known, margin-carrying entry in the type-check debt ledger), which is why the proof runs through the mcp seam.Known-red on main, not this diff:
check:type-check-debtreports@objectstack/service-analyticsDEBT 10 → 14 (+4); the four errors referenceCROSS_FIELD_OPERAND_NAMES/diagnosticIncludes— surfaces PR #8198 changed today. Filed as #8235; every PR merging today'smainwill see the same red until stanched.Generated by Claude Code