fix(service-analytics): 维度合并键改为长度前缀,并停止把「未分配」并进「空白」 (#4821) - #4957
Merged
Conversation
…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
|
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 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 #4821
mergeByDimensions的原键不是无分隔符拼接。合并后的origin/main上它是:其中
SOH是一个直接写进源码的裸 U+0001 字节。裸控制字符渲染为空 —— 在终端里、在GitHub issue 正文里、在这个文件里都一样 —— 所以 issue 正文读到的是
join(''),派发前的复核注释也是基于同一份被吃掉字节的引文写的。
实测(在
origin/main的键法上直接跑):['ab','c']vs['a','bc'](issue 头号复现)nullvs''1vs 字符串'1'所以本 PR 修的是后两条,外加把那个看不见的字节从源码里去掉 —— 它正是这张 issue 被写错
前提的直接原因。
真实缺陷
?? ''把真正为 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 之后的形状。
字段、导入记录),那是假设不是保证,且失效时同样静默。
影响面按派发注释所述确认:#4870 合并后 compare 路径也经
runMeasurePass扇出并逐measure 合并,所以一次碰撞同时污染当期列与
__compare列 —— 两个测试分别钉住。改法
长度前缀 + 显式空值哨兵,即复核注释指定的方向:
2:ab1:c与1:a2:bc对任意输入都不同,不保留任何字符,也不再有看不见的字节留给下一个读者误读。
null/''的区分由独立哨兵负责,与消歧解耦 —— 两件事的正确答案相反,耦进一个改动正是
JSON.stringify方案的问题所在。undefined与null刻意同键:驱动确实会在行对象里省略 null 列,在这里劈开等于把跨查询错配换个层级重新引入一遍。
为什么不照抄
cross-object-rebucket.ts的 JSON 键(留给下一个读者)两者的交易条件相反,这是本 PR 最需要被继承的一条推理:
rebucketCrossObject重新分桶的是同一个executeAggregate结果的行(
objectql-strategy.ts:548)。一列的值全部来自一次查询,只有一种类型 —— JSON 在那里免费,且能换来真实的区分(空桶
nullvs 字面量字符串"null")。mergeByDimensions做的是相反的事:跨不同查询对齐行 —— 主查询 vs 每个带 filter的 measure 的补充子查询,以及当期窗口 vs 位移后的 compare 窗口。而驱动确实会对同一个
分组返回不同的 JS 类型;本文件
compareValues的注释就记着 "numeric strings, whichis 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'保持分离(改动前后都通过 —— 属性钉,不是复现)nullvs''保持分离(改动前失败)null同组(刻意)1与字符串'1'仍然合并 —— 这一条是防止被否决的JSON.stringify方向日后被重新引入的守卫,请勿"修正"改动前的实现上:
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