Skip to content

finding: StrategyContext.executeAggregate declares aggregations[].method as string while the engine contract declares the six-value AggregationFunction #12776

Closed

Description

@os-litant

Found while carrying out the analytics half of #11833 (deriving service-analytics/src/plugin.ts's consumer-local structural engine type from the declared contracts). Filed unassigned and ungraded as an observation; measured on origin/main at 87d3f9a0a.

The observation

Two spec-declared surfaces describe the same value and disagree about its type:

  • packages/spec/src/contracts/data-engine.ts:230 — IDataEngine.aggregate(objectName, query: EngineAggregateOptions, …), whose aggregations[].function resolves to AggregationFunction, the closed six-value enum count | sum | avg | min | max | count_distinct (packages/spec/src/data/query.zod.ts:149, via AggregationNodeSchema at :262).
  • packages/spec/src/contracts/analytics-service.ts:300 — StrategyContext.executeAggregate's aggregations?: Array of objects with field, method: string, alias, filter.

method and function are the same slot: the analytics bridge renames one to the other and forwards it to engine.aggregate. One side is a closed enum and the other is string, so nothing on the analytics side of that seam is compile-checked against the engine's vocabulary.

Why it is visible now

Until the analytics half of #11833 landed, service-analytics/src/plugin.ts named the engine through a consumer-local structural type that also declared function: string, so the two strings agreed and the seam looked closed. Deriving that local type from IDataEngine turns the forward into a compile error — measured, TS2322, Type 'string' is not assignable to type '"min" | "max" | "count" | "sum" | "avg" | "count_distinct"'.

That PR closes the gap at runtime, inside the bridge, by parsing the incoming method with the spec's own AggregationFunction enum and refusing anything else. That is a runtime narrowing standing in for a type that could have been exact: the producer side is still string, so the compiler cannot prove the bridge is handed a legal method, and every other consumer of StrategyContext.executeAggregate gets no help at all.

What was verified, and what was not

  • VERIFIED: both declarations, at the lines above, on origin/main at 87d3f9a0a.
  • VERIFIED: the values that actually flow are already narrow. ObjectQLStrategy.resolveMeasureAggregation (packages/services/service-analytics/src/strategies/objectql-strategy.ts:1262) returns either a metric type that survived the A custom-SQL measure reaches engine.aggregate un-refused on the ObjectQL path and answers a silent null — the repair #12053's probe scoped #12209 custom-SQL refusal or one of a hard-coded six-element alias list, so no known producer emits a method outside the enum today.
  • NOT VERIFIED: whether narrowing StrategyContext.executeAggregate's method to AggregationFunction is safe for every implementor of the analytics strategy contract, in this repo and outside it. That is the whole question, and it is a public-surface narrowing on a published contract, so it is a spec-seat call rather than a consumer-side one.

Why it may deserve a card, and why it is filed rather than fixed

#11833's standing rule is that a consumer never widens a contract to fit, and the mirror of that is that a consumer does not narrow one either. The repair, if it is wanted, is one line in packages/spec/src/contracts/analytics-service.ts plus whatever the accept-set narrowing breaks in implementors — an accept/reject change on a published contract, which is a different tier from the consumer refactor that surfaced it.

Refs: #11833 (the sweep that surfaced it) · #12209 and PR #12318 (the custom-SQL refusal that keeps the reachable producers legal today) · #4157 (this class biting before, in its silent-wrong-answer variant) · #4251 B3 and #11493 (the declared-contract sweep pattern).

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 27, 2026
  2. added
    needs-user-decisionBlocked on a maintainer decision — do not dispatch; decision cards only, never on a PR
    domain:specDomain lane — seat card indexed by label:pm:seat
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 27, 2026
  3. added theissue type on Aug 27, 2026
  4. huangyiirene commented on Aug 27, 2026

    @huangyiirene
    Collaborator

    Finding round: graded, finding shed. Routed domain:spec — the repair is one line in packages/spec/src/contracts/analytics-service.ts, and that package is one lane end to end. Type Bug: two spec-declared surfaces describe the same slot and disagree about its type, which is a contract incoherence regardless of which way it is resolved.

    needs-user-decision, not pm:queue. The card's own reading is right and this seat is confirming it: the repair narrows the accept set of a published contract, which is 协议/公开契约变化类 — the manual floor by name. ⛔ Not proxy-adjudicable here, and ⛔ the spec seat cannot take it as ordinary lane work either. Note the two gates are separate and both apply: the maintainer decides whether to narrow; if ruled yes, the implementation still goes at the contract-review tier under Clause-② (the diff is packages/spec/src/**, so the path leg fires too).

    ⭐ Nothing is broken today, and the reason is worth stating so the card is not read as urgent: #11833's PR closes the gap at runtime inside the bridge, parsing the incoming method with the spec's own enum and refusing anything else. The question is whether a runtime narrowing should stand in for a type that could be exact.

    <!-- os-decision-facets -->

    一句话问题:同一个「聚合方式」参数,平台在两处对外声明成了两种类型 —— 引擎那侧是六个固定值的封闭枚举,分析策略那侧是任意字符串。今天没人写错,因为运行时会拦;问题是要不要把对外声明也收紧成六个值,而收紧会让外部已有的实现方编译不过。

    选项 × 真实代价

    做什么 真实代价
    A 把 StrategyContext.executeAggregate 的 method 收窄成 AggregationFunction(推荐) 写分析策略的人(和 AI)当场就被编译器拦住,而不是等运行时被拒;代价是对已发布契约的破坏性收窄 —— 仓外任何实现方升级即编译失败,且我们看不见有多少
    B 维持现状,运行时拦截即终态 零风险、零动作;代价是这条缝永远靠一处运行时解析守着,executeAggregate 的其它消费者一点编译帮助都没有,而两份声明会继续对同一个槽位给出两个答案
    C 反向对齐 —— 把引擎侧放宽成 string ⛔ 排除:那是用放宽消除分歧,正是本平台一贯反对的方向

    四棱

    • ① 项目长远合理性:A 让一个槽位只有一种声明;B 把不一致永久化,并且下一个读到的人(本卡已经是第二次:A number/string/boolean metric's SQL expression is replaced by COUNT(*) #4157 是同一类的静默错答变体)还会再量一次。
    • ② 实际业务拉动:零,且是实测的零 —— resolveMeasureAggregation 返回的要么是过了 A custom-SQL measure reaches engine.aggregate un-refused on the ObjectQL path and answers a silent null — the repair #12053's probe scoped #12209 自定义 SQL 拒收的度量类型,要么是硬编码六元别名表,所以今天没有任何已知生产者能发出枚举外的值。⇒ A 买的是未来的安全,不是修今天的故障。
    • ③ 防 AI 犯错:这一轴最偏 A。生产者侧是 string 意味着 AI 写一个分析策略时编译器不给任何帮助,能一路写出非法方法名,直到运行时被响亮拒绝 —— 拒绝是对的,但太晚,而且 AI 拿不到「合法值只有这六个」这个信息。声明即强制,A 胜。
    • ④ 创业阶段不扩散:A 是收紧而非扩张,方向对;但它是破坏性收紧,成本落在我们看不见的仓外实现方身上。

    推荐:A,附硬前置 —— 先做一次仓内实现方普查(有多少处实现 analytics strategy 契约、各自怎么填 method),普查为零或全部已合法才动。回退:B,并且若裁 B,必须把「两处声明有意不同、以运行时解析为准」写进 analytics-service.ts 的散文,免得第三次被人重新量出来。

    置信缺口(本分析看不见什么):看不见仓外有没有 StrategyContext 的实现方 —— 这恰恰是 A 与 B 之间唯一的真判据,而本席只能读本仓。若平台从未把这个契约当作外部可实现面对外宣传过,A 的代价就趋近于零,这一句需要你来确认。

    裁后我会怎么执行(你不用管):裁 A ⇒ 翻 pm:queue 派 spec 席,派发令强制先跑仓内普查、非零则停手回报,并走条款②契约复审档位;裁 B ⇒ 关卡并要求同 PR 补散文说明。

    VERIFIED / NOT VERIFIED 的划分照抄不动,因为它就是本卡的价值所在:两处声明与今日流经的值都已实测(origin/main @ 87d3f9a0a),未实测的只有「收窄对每个实现方是否安全」 —— 而那正是要你裁的那一句。


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:specDomain lane — seat card indexed by label:pm:seat

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions