Skip to content

fix(service-analytics): 按聚合种类填充「查询从未报告过的分组」,并补上 compareTo 这一道接缝 (#4708) - #4822

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-4708-filtered-measure-zero-fill
Aug 3, 2026
Merged

fix(service-analytics): 按聚合种类填充「查询从未报告过的分组」,并补上 compareTo 这一道接缝 (#4708)#4822
os-zhuang merged 3 commits into
mainfrom
claude/issue-4708-filtered-measure-zero-fill

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

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_callwon_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,修复前的输出:

{ lead_source: 'content',   revenue: 100, avg_deal: 50, won_count: 3,
                            revenue__compare: 80, avg_deal__compare: 40, won_count__compare: 2 },
{ lead_source: 'cold_call', revenue__compare: 10, avg_deal__compare: 10, won_count__compare: 1 }

cold_call 上个周期卖过、这个周期一单没有 —— 这一行的 revenue / won_count 全是空白,而正确答案是 0。偏置方向和 issue 抓到的那条一模一样:空白的恰好是表现最差的行。上期有业绩、本期归零的渠道,在仪表盘上渲染成「无数据」,而不是「归零了」—— 后者才是要看的那个信号。

修复后同一组输入:revenue: 0won_count: 0avg_deal: null,__compare 三列原样不动。

顺带把镜像的一侧也一起覆盖了:本期存在、上期完全没有的 bucket,revenue__compare / won_count__compare 现在读 0 而不是空白(「上个月卖了 0」是事实,不是缺口)。这两列走的是同一个 mergeByDimensions 调用、同一套聚合种类推理,分开修只会让下一个 issue 立刻开在隔壁那一行 —— 但它确实比 issue 的字面范围宽一点,所以在这里点名,reviewer 若认为该拆出去,砍掉三行即可。

严格按聚合种类,不一刀切

aggregate 分组被排除时 为什么
count / count_distinct 0 「有多少行匹配」在没有匹配时有确切答案
sum 0 空集合上的单位元
avg / min / max 保持 null 确实无定义 —— 没有东西可平均

一刀切填 0 会把「没有数据」和「平均值为零」混成一件事,制造一个方向相反的新谎。种类→单位元的映射仍然是 @objectstack/spec/dataemptyGroupValueFor(与 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 条。
  • 一个 changeset(patch,@objectstack/service-analytics)。

测试

pnpm --filter @objectstack/service-analytics test37 files / 502 tests passed(含新增 9 条)。

新测试对修复前的 executor 跑过一遍,证明它们真的钉住了东西:

× fills every base measure on a row only the previous period produced
× fills a comparison column the previous window never reported
× fills count/count_distinct/sum, leaves avg/min/max and reported values alone
× touches only the columns it is given, and never adds rows
Tests  4 failed | 5 passed (9)

那 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 对三个改动文件干净。

⚠️ 跨仓协调:hotcrm 侧会转红,这是设计好的信号

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)

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny


Generated by Claude Code

…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
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 8:53am

Request Review

@github-actions github-actions Bot added the size/m label Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/data-api.mdx (via @objectstack/service-analytics)
  • content/docs/api/index.mdx (via @objectstack/service-analytics)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/service-analytics)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/service-analytics)
  • content/docs/plugins/packages.mdx (via @objectstack/service-analytics)
  • content/docs/releases/implementation-status.mdx (via @objectstack/service-analytics)
  • content/docs/releases/v17.mdx (via @objectstack/service-analytics)
  • content/docs/releases/v9.mdx (via @objectstack/service-analytics)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

`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

Copy link
Copy Markdown
Contributor Author

已按复核意见改完并推上去(16627d68b):

其余照旧。期间 main 往前走了三个 commit(#4810 / #4806 / #4803),都不碰 service-analyticspackages/spec/src/data,重叠为空;仍按 AGENTS.md §10 合了 main 并把受影响包重跑了一遍:

Test Files  37 passed (37)
     Tests  502 passed (502)

PR 保持 draft,等你决定何时转正式评审。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 09:19
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit fa94b2c Aug 3, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4708-filtered-measure-zero-fill branch August 3, 2026 10:22
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/m tests tooling

Projects

None yet

2 participants