Skip to content

fix(service-analytics): 维度合并键改为长度前缀,并停止把「未分配」并进「空白」 (#4821) - #4957

Merged
xuyushun441-sys merged 1 commit into
mainfrom
claude/issue-4821-mergebydimensions-key
Aug 3, 2026
Merged

fix(service-analytics): 维度合并键改为长度前缀,并停止把「未分配」并进「空白」 (#4821)#4957
xuyushun441-sys merged 1 commit into
mainfrom
claude/issue-4821-mergebydimensions-key

Conversation

@xuyushun441-sys

Copy link
Copy Markdown
Contributor

Fixes #4821

⚠️ 先更正一条前提:issue 正文的头号复现其实不成立

mergeByDimensions 的原键不是无分隔符拼接。合并后的 origin/main 上它是:

const keyOf = (row) => dimensions.map((d) => String(row[d] ?? '')).join(SOH);

其中 SOH 是一个直接写进源码的裸 U+0001 字节。裸控制字符渲染为空 —— 在终端里、在
GitHub issue 正文里、在这个文件里都一样 —— 所以 issue 正文读到的是 join(''),派发前
的复核注释也是基于同一份被吃掉字节的引文写的。

实测(在 origin/main 的键法上直接跑):

场景 main 上的实际行为
['ab','c'] vs ['a','bc'](issue 头号复现) distinct —— 分隔符一直在,只是看不见
null vs '' COLLIDES ← 真实缺陷
值本身含 U+0001 COLLIDES ← 真实缺陷(窄)
数字 1 vs 字符串 '1' merges(现行语义,须保留)

所以本 PR 修的是后两条,外加把那个看不见的字节从源码里去掉 —— 它正是这张 issue 被写错
前提的直接原因。

真实缺陷

  1. ?? '' 把真正为 null 的维度与空字符串维度键成同一个值。「未分配」被并进
    「空白」:一行吞掉另一行的 measure,另一行的列整个缺失 —— 而 A filtered dataset measure returns ABSENT (not 0) for a group its filter excludes, so every derived ratio over it blanks — on exactly the worst-performing row #4708 的空组填充随后
    给它填上一个理直气壮的 0一个真实计数为 3 的分组显示成 0,这是本缺陷在
    A filtered dataset measure returns ABSENT (not 0) for a group its filter excludes, so every derived ratio over it blanks — on exactly the worst-performing row #4708 之后的形状。
  2. 单字符分隔符只在「没有任何维度值包含该字符」时无歧义。 维度值是用户数据(文本
    字段、导入记录),那是假设不是保证,且失效时同样静默。

影响面按派发注释所述确认:#4870 合并后 compare 路径也经 runMeasurePass 扇出并逐
measure 合并,所以一次碰撞同时污染当期列与 __compare 列 —— 两个测试分别钉住。

改法

长度前缀 + 显式空值哨兵,即复核注释指定的方向:

每段 = `< 长度 >:< 值 >`;null/undefined = 哨兵段(以非数字开头,真实值段不可能产生)

2:ab1:c1:a2:bc 对任意输入都不同,不保留任何字符,也不再有看不见的字节留给
下一个读者误读。null / '' 的区分由独立哨兵负责,与消歧解耦 —— 两件事的正确答案
相反,耦进一个改动正是 JSON.stringify 方案的问题所在。

undefinednull 刻意同键:驱动确实会在行对象里省略 null 列,在这里劈开等于把跨
查询错配换个层级重新引入一遍。

为什么照抄 cross-object-rebucket.ts 的 JSON 键(留给下一个读者)

两者的交易条件相反,这是本 PR 最需要被继承的一条推理:

  • rebucketCrossObject 重新分桶的是同一个 executeAggregate 结果的行
    (objectql-strategy.ts:548)。一列的值全部来自一次查询,只有一种类型 —— JSON 在那里
    免费,且能换来真实的区分(空桶 null vs 字面量字符串 "null")。
  • mergeByDimensions 做的是相反的事:跨不同查询对齐行 —— 主查询 vs 每个带 filter
    的 measure 的补充子查询,以及当期窗口 vs 位移后的 compare 窗口。而驱动确实会对同一个
    分组返回不同的 JS 类型;本文件 compareValues 的注释就记着 "numeric strings, which
    is how some drivers return SUM results"

改用 JSON.stringify 会把 1"1" 渲染成两个键,让今天能正确合并的行不再合并
—— 分组无声裂开、数字散到两行。用一个新的静默缺陷换掉旧的,不算修好。因此逐段
String() 强制被刻意保留,并由回归钉测试锁住。

测试

新增 dataset-merge-dimension-key.test.ts,9 例;其中 5 例在改动前的实现上失败
(已实测,见下)。单元层 5 例 + 经真实 executor 的集成层 4 例(measure-filter 合并与
compareTo 合并各自钉住):

  • 相邻值对 'ab'|'c' vs 'a'|'bc' 保持分离(改动前后都通过 —— 属性钉,不是复现)
  • 值含旧分隔符时无法伪造边界(改动前失败)
  • null vs '' 保持分离(改动前失败)
  • 缺失维度列与 null 同组(刻意)
  • 回归钉:数字 1 与字符串 '1' 仍然合并 —— 这一条是防止被否决的
    JSON.stringify 方向日后被重新引入的守卫,请勿"修正"
 Test Files  39 passed (39)
      Tests  520 passed (520)

改动前的实现上:

 ❯ dataset-merge-dimension-key.test.ts (9 tests | 5 failed)
   × cannot be forged by a value that CONTAINS the old separator
   × keeps a NULL dimension apart from an empty-string one
   × gives every group its own won_count, unassigned and blank included
   × does not render the unassigned group as an empty group
   × attaches each group its OWN __compare columns

tsc --noEmit:7 个既有错误(该包在 check-type-check-coverage 里带 DEBT 条目、无
typecheck 脚本),本 PR 触碰的三个文件零错误check:nul-bytes、eslint 通过。

顺带更新了 dataset-compare-measure-filters.test.ts 里引用本 issue 的一句注释 —— 它复述
了同一条被误读的前提。

约束

仅改内部合并键,响应中的任何值都不变。packages/spec/**
packages/metadata-protocol/src/protocol.ts 零改动;未触碰 content/docs/releases/

顺带发现(未在本 PR 修复)

裸 U+0001 分隔符字节还存在于 strategies/cross-object-rebucket.ts:131
packages/services/service-storage/src/verify-file-references.ts:107;check:nul-bytes
只拦 0x00,不覆盖其它控制字符。本单已被它误导过一次(整张 issue 的头号机制就是这么来
的)。已按 Prime Directive #10 另开 issue 记录,不在本 PR 扩大范围。


🤖 Generated with Claude Code

https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX


Generated by Claude Code

…op merging "unassigned" into "blank" (#4821)

`mergeByDimensions` is the seam every multi-query dataset result is assembled
through: the primary pass against each measure-scoped supplementary pass, and
— since #4870 — the current window against the shifted `compareTo` window,
which now fans out per measure the same way. A key collision there does not
fail; one group silently absorbs another's numbers.

The reported mechanism was not quite the real one, which is worth recording.
The old key was `String(row[d] ?? '')` joined on a RAW U+0001 byte written
literally into the source. A raw control byte renders as nothing, so #4821 was
filed reading `join('')`, and its headline repro (`['ab','c']` vs `['a','bc']`
both keying "abc") never actually reproduced — the separator was there, merely
invisible. Two things did:

  - `?? ''` keyed a genuinely NULL dimension the same as an empty-string one,
    so "unassigned" merged into "blank": one row absorbed the other's measure
    and the other's column went absent — which #4708's empty-group fill then
    turns into a confident 0. A group whose real count is 3 renders as 0.
  - A one-character separator is unambiguous only while no dimension VALUE
    contains it, and dimension values are user data (text fields, imports).

Fixed by length-prefixing each segment (`2:ab1:c` vs `1:a2:bc` differ for every
possible input, no character is reserved, and no invisible byte is left in the
source for the next reader to misread) plus an explicit sentinel for
null/undefined, kept separate from the disambiguation concern.

The per-segment `String()` coercion is deliberately KEPT, and it is not the
trade-off `cross-object-rebucket.ts` makes one file over. That function
re-buckets ONE query's rows, where a column carries one type, so its JSON key
is free and buys a real distinction. This key aligns rows across DIFFERENT
queries, and drivers do type the same group differently across them — this
file's own `compareValues` records it ("numeric strings, which is how some
drivers return SUM results"). A `JSON.stringify` key would render `1` and `'1'`
as two keys and split groups that merge correctly today, trading one silent
defect for a new one. Pinned by a regression test.

Tests: 9 new cases in `dataset-merge-dimension-key.test.ts` — the adjacent-value
pair, a value carrying the old separator, null vs empty string, an absent
dimension column, the numeric-vs-string regression pin, and the same properties
through the real executor on both the measure-filter and `compareTo` merges.
5 of the 9 fail against the pre-fix implementation.

Fixes #4821

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
@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 5:12pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels 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.

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

Development

Successfully merging this pull request may close these issues.

mergeByDimensions joins its dimension key with no delimiter, so two distinct groups can merge into one row

2 participants