Uh oh!
There was an error while loading. Please reload this page.
fix(service-analytics): narrow the executeAggregate config mirrors' aggregations[].method to AggregationFunction - #13361
Conversation
…ggregations[].method to AggregationFunction #12776 narrowed StrategyContext.executeAggregate's aggregations[].method from `string` to the six-value AggregationFunction enum, but this package's own consumer-local mirrors of that slot kept declaring `string`, so a custom-bridge author still typed their handler against the open vocabulary. Narrows all three sites the tree carries: the AnalyticsServicePluginOptions mirror (plugin.ts), the AnalyticsServiceConfig twin (analytics-service.ts) and parseEngineAggregateFunction's parameter, and repairs the two comments that explained the auto-bridge's runtime parse by pointing at the old `method: string` declaration. The parse itself is untouched: with both ends of the `method` -> `function` rename now declaring the same enum, it is defence in depth behind a compile-time check rather than the only check. Type-only; no runtime change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WkdHQwHr2KQmaX7P1BHzi
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 864525b3b12f52c693b27c0b3b8e01aec2e3d96e && git checkout 864525b3b12f52c693b27c0b3b8e01aec2e3d96e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e5ed9434365e6a7520b5070da441f0552030b37d 978a4a9abb79e731d905ba468aea0a2a37d4f01d && git checkout -B drift-repro e5ed9434365e6a7520b5070da441f0552030b37d && git merge --no-ff 978a4a9abb79e731d905ba468aea0a2a37d4f01d
node scripts/docs-audit/affected-docs.mjs --json e5ed9434365e6a7520b5070da441f0552030b37d
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#12940
Type-only.
#12776narrowedStrategyContext.executeAggregate'saggregations[].methodfromstringto the six-valueAggregationFunction; this brings@objectstack/service-analytics' own consumer-local mirrors of that same slot into line, so the compile-time vocabulary reaches the people who write a custom bridge instead of stopping at the package boundary.Sites: three, not two
The card enumerated two. Deriving the population from the tree (
grep -rn 'method:' packages/services/service-analytics/src) found three declarations of the slot, all narrowed here:plugin.ts—AnalyticsServicePluginOptions.executeAggregateanalytics-service.ts—AnalyticsServiceConfig.executeAggregateplugin.ts—parseEngineAggregateFunction(method, alias)Producers were already narrow before this PR (
ObjectQLStrategy's aggregation locals carryAggregationFunctionend to end since #12776), so this closes the split rather than opening a new one.Prose repaired
analytics-service.ts) now states the lockstep member by member and namesmethodas the enum, so the next drift is visible in review rather than silently true-then-false.parseEngineAggregateFunction's docblock no longer explains the parse by pointing at amethod: stringdeclaration that no longer exists. Both now use the wording the finding:StrategyContext.executeAggregatedeclaresaggregations[].methodasstringwhile the engine contract declares the six-valueAggregationFunction#12776 changeset already settled on: with both ends of themethodtofunctionrename declaring the same enum, the parse is defence in depth behind a compile-time check rather than the only check.CubeSchema, which is the pathaggregate-bridge-function-vocabulary.test.tsdrives end to end). finding: two consumer-local structuralDataEngineLikere-declarations remain after the #11493 deletion (datasource-admin-plugin, service-analytics) #11833 is why it exists.Verification (all at
978a4a9)pnpm --filter '@objectstack/service-analytics^...' buildthenpnpm --filter @objectstack/service-analytics typecheck—tsc --noEmit, exit 0.pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2— 83 files, 1805 tests passed.tsc --noEmit --listFilesputs 81 test files fromsrc/__tests__in the program, including all nine that carry their own{ field: string; method: string; alias: string }doubles. Those nine needed no edit — a handler acceptingmethod: stringaccepts a superset and stays assignable (parameter contravariance), exactly as the finding:StrategyContext.executeAggregatedeclaresaggregations[].methodasstringwhile the engine contract declares the six-valueAggregationFunction#12776 changeset predicted for implementors. Noany, no@ts-expect-error, no widening, and nothing excluded from the tsc program.method: 'median'against both config mirrors:tscexit 2 withTS2322: Type '"median"' is not assignable to type '"min" | "max" | "count" | "sum" | "avg" | "count_distinct"'at both sites, while a siblingmethod: 'sum'call stayed clean. Probe removed;git diff HEADempty;tscexit 0.stringturnedtscred atplugin.ts(346,52),TS2345: Argument of type 'string' is not assignable to parameter of type ..., i.e. at theparseEngineAggregateFunctioncall. That is what makes the third site load-bearing rather than decorative. Mutation confirmed on disk by anchoredgrep -c(1 widened / 0 narrowed) and bygit hash-object; restored withgit checkout HEAD --and proven byte-identical to the HEAD blob,git diff HEADempty,tscexit 0 again.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(26 matched families plus the convention-triggeredcheck:dispatcher-error-vocabulary) — all green, includingcheck:type-check-coverage(OK - 66/78 workspace packages type-checked),check:test-source-alias,check:type-source-resolution,check:cross-package-test-inputs,check-adr-0087-registration(this PR adds no declared-breaking changeset),check-empty-changeset,check-changeset-no-major.check-test-completeness.mjsandscripts/pm/check-half-states.mjsboth exit 3 = PREREQUISITE NOT MET in this container (no test log to hand the first; no valid GitHub credential for the second).check:type-check-debtexits 1 only in its--re-measureleg, which refuses to measure without a built closure for 48 workspace dependencies; its--self-testand coverage legs passed. That refusal cannot hide a ratchet red from this diff: noDEBTentry depends onservice-analytics, and of the threeTEST_DEBTentries that do (runtime,cli,verify), onlyruntimereferences the narrowed surface at all — one test double typedoptions: any. CI builds the closure before the step, aslint.ymldoes.eslint --no-inline-config --format jsonover the 2 changed files, 0 errors / 0 warnings. The narrowing is a measurement, not a skip: this repo runs oneeslint.config.mjswhich, by its own documented and positively-controlled statement, "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file", so a declaration change here cannot move a verdict on a file it does not edit.Changeset:
patchfor@objectstack/service-analytics.Generated by Claude Code