fix(service-analytics): 按聚合种类填充「查询从未报告过的分组」,并补上 compareTo 这一道接缝 (#4708) - #4822
Merged
Conversation
…m, by aggregate kind (#4708) A dataset measure carrying its own `filter` runs as a separate grouped sub-query merged back by dimension key. A `GROUP BY` over a filtered row set emits no group at all for a dimension value the filter excludes entirely, so the measure comes back ABSENT rather than 0, and `computeDerived` treats an absent operand as unknowable — every ratio over it goes null too. The cell renders blank: visually identical to "no data for this row", which is the opposite of what the row means. The bias runs the worst possible way — the rows that blank are the ones whose numerator matched nothing, i.e. the worst-performing ones. `fillEmptyGroups` now fills the identity element by aggregate kind: count/count_distinct/sum → 0 (measured facts), avg/min/max → null (genuinely undefined; flattening those would trade this lie for its mirror image). The kind→identity mapping stays `emptyGroupValueFor` in @objectstack/spec/data, so the runtime and the authoring-side coherence checks cannot drift. Only cells on rows that already exist are filled — no group is invented. The fill previously ran BEFORE the compareTo merge, and that merge appends a row for every bucket the previous window had and this one does not; every base measure on those rows — including unfiltered ones — stayed absent, so a lead source that sold last month and nothing this month rendered as "no data" instead of 0. It now runs after every merge and covers all base measures plus their `<measure>__compare` columns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Aug 3, 2026
`fillEmptyGroups` 从包根 index.ts 导出,属于新增公共 API,按 #4815 (objectql / AuditAbortSignal)与 #4791(service-automation / sealNodeTypeVocabulary)确立的同一条 bar,changeset 从 patch 改为 minor, 并在正文点名这个新导出。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
Contributor
Author
|
已按复核意见改完并推上去(
其余照旧。期间 PR 保持 draft,等你决定何时转正式评审。 Generated by Claude Code |
os-zhuang
marked this pull request as ready for review
August 3, 2026 09:19
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 3, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 3, 2026
This was referenced Aug 3, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 3, 2026
This was referenced Aug 3, 2026
This was referenced Aug 3, 2026
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
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 #4708
按 issue 作者的推荐与 PM 裁定取方向 1(在 merge 时按聚合种类填充),不取方向 2(
missingAs: 'zero')。理由就是 issue 自己写的那条:方向 2 把决定重新分配给每一个未来的作者,而忘记的失败模式是静默的 —— 对一个从邻近声明里模式匹配的 AI 作者来说,必须被记住的正确性开关等价于终将被漏掉的正确性开关。方向 1 没有这个表面积。零改动:
packages/spec/**。本 PR 只动packages/services/service-analytics。现场复核:一半已经在 main 上,另一半还在
先说一个必须讲清楚的事实,免得 reviewer 以为这个 PR 比实际大或比实际小。
issue 描述的机制里,「measure 自带 filter 的那道接缝」在 main 上已经修好了 —— objectui#3136 曾经加过一段按聚合种类的填充(
emptyGroupValueFor),就在executeSelection里。我用 issue 的复现表逐字跑了一遍真实 executor,四行全部正确:cold_call的won_count是 0、win_rate是 0%。但那段代码一个测试都没有。issue 的验收标准 1/2/3 要求把这个行为钉死,而当时没有任何东西拦得住它回退 —— 事实上它下面就摆着一次回退的活证据(见下)。所以本 PR 的第一半是:把 issue 的复现表原样变成回归测试。
第二半是真正的代码修复:同一个缺陷在下一道接缝上原封不动地活着。
还活着的那一半:compareTo 追加出来的行
填充过去跑在
compareTo合并之前。而那次合并是会追加行的(mergeByDimensions对只在对比窗口出现的 bucket 做 outer-ish 追加,注释里写着「so comparison-only buckets still surface」)。被追加出来的行上,每一个 base measure 都是缺失的 —— 包括没有 filter 的那些,因为主查询压根没返回过这个分组。用假 service 跑真 executor,修复前的输出:
cold_call上个周期卖过、这个周期一单没有 —— 这一行的revenue/won_count全是空白,而正确答案是 0。偏置方向和 issue 抓到的那条一模一样:空白的恰好是表现最差的行。上期有业绩、本期归零的渠道,在仪表盘上渲染成「无数据」,而不是「归零了」—— 后者才是要看的那个信号。修复后同一组输入:
revenue: 0、won_count: 0、avg_deal: null,__compare三列原样不动。顺带把镜像的一侧也一起覆盖了:本期存在、上期完全没有的 bucket,
revenue__compare/won_count__compare现在读 0 而不是空白(「上个月卖了 0」是事实,不是缺口)。这两列走的是同一个mergeByDimensions调用、同一套聚合种类推理,分开修只会让下一个 issue 立刻开在隔壁那一行 —— 但它确实比 issue 的字面范围宽一点,所以在这里点名,reviewer 若认为该拆出去,砍掉三行即可。严格按聚合种类,不一刀切
count/count_distinct0sum0avg/min/maxnull一刀切填 0 会把「没有数据」和「平均值为零」混成一件事,制造一个方向相反的新谎。种类→单位元的映射仍然是
@objectstack/spec/data的emptyGroupValueFor(与 authoring 侧的一致性检查共用一个源,两边不会漂移),本 PR 没有碰它。只填格子,不造行(验收标准 4):填充只遍历已经存在于网格里的行。某个维度值如果没有任何一个查询报告过,它就是真的没有数据,不会被凭空物化成一行 0。测试里有一条专门钉这个。
改了什么
dataset-executor.ts— 新导出fillEmptyGroups(rows, columnAggregates),把原先内联在循环后的那段提出来;调用点从「supplementary 合并之后」移到「所有合并之后、derived 求值之前」,覆盖范围从filtered扩到全部 base measure 加它们的__compare列。index.ts— 导出fillEmptyGroups(与既有的mergeByDimensions同级,便于直接单测)。__tests__/dataset-empty-group-fill.test.ts— 新增 9 条。@objectstack/service-analytics)。测试
pnpm --filter @objectstack/service-analytics test→ 37 files / 502 tests passed(含新增 9 条)。新测试对修复前的 executor 跑过一遍,证明它们真的钉住了东西:
那 5 条修复前就通过 —— 它们正是 issue 复现表的回归钉,钉的是 objectui#3136 已经修好、但一直裸奔的行为。
tsc --noEmit:7 处报错,全部在本 PR 未触碰的既有测试文件里(analytics-service.test.ts的 TS6133、measure-source-field-gate.test.ts的 TS2339、objectql-timedimension-projection.test.ts的 TS7053),我改动的三个文件 0 报错。该包在check-type-check-coverage.mjs里带 DEBT 条目,本 PR 不增不减。ESLint 对三个改动文件干净。issue 说明 hotcrm 侧钉了一条**故意写成「平台修好后会失败」**的断言,用来把平台的修复暴露成一条红测试,而不是让仪表盘悄悄改变数字。
本 PR 合并后,那条 app 侧测试会转红 —— 这是预期信号,不是回归,也不是撞车。 我们不改那个仓;请对面按 hotcrm#656 的原计划把断言翻过来(顺带可以撤掉
?? 0之类的消费侧兜底,以及把decided_count换回derived: { op: 'sum', of: ['won_count','lost_count'] }的写法 —— 现在lost_count会被填 0,那个「从没输过的销售分母变空」的坑不复存在了)。相关:hotcrm#656、hotcrm#593、#4698。
顺手发现的、没有在本 PR 里修的两件事(Prime Directive #10)
<measure>__comparesilently reports a different measure than the column beside it #4820 —compareTo完全丢弃 measure 级 filter:runCompare只带baseFilter发一条 shifted 查询,compiled.measureFilters从未被查阅。于是won_count__compare数的是上个周期的全部行,不是赢单行 —— 与它并排那一列是两个不同的度量。这是数错而不是留白,补法(按主路径拆成每个 filtered measure 一条 shifted 子查询)和影响面都不同,单独修。mergeByDimensionsjoins its dimension key with no delimiter, so two distinct groups can merge into one row #4821 —mergeByDimensions的维度键用空串拼接(.join('')),多维度下'ab'|'c'与'a'|'bc'撞键,两个分组会合并成一行;String(row[d] ?? '')还把null和''压成同一个键。隔壁cross-object-rebucket.ts已经用 JSON 编码解决过同一个问题。🤖 Generated with Claude Code
https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
Generated by Claude Code