fix(core): jsonl write([]) should leave an empty file, not a stray newline - #7533
Conversation
…wline
write() built its content with join('\n') and then appended a trailing
newline. For an empty array the join produces '', so the file ends up as
a single '\n': one byte, no records.
That makes the module's own accessors contradict each other — exists()
tests size > 0 and returns true, while read() skips blank lines and
returns []. Clearing a JSONL file therefore leaves something that reports
as non-empty but has nothing in it.
Terminate each record instead of joining with separators. The output for
a non-empty array is byte-identical; an empty array now writes nothing.
|
Thanks for the PR! Template looks good ✓ Problem: this is a real, self-evident inconsistency in the module's own API — Direction: aligned — a correctness fix in core JSONL utilities. No CHANGELOG reference needed for an internal consistency fix like this. Size: core paths touched. Production lines: ~11 (jsonl-utils.ts), test lines: ~20 (jsonl-utils.test.ts). Well under any threshold. Approach: the scope is exactly right — one function, one behavioral fix, one test. Switching from Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是模块自身 API 中一个真实且显而易见的不一致—— 方向：对齐——核心 JSONL 工具的正确性修复。此类内部一致性修复无需 CHANGELOG 引用。 规模：触及核心路径。生产行数：约 11 行（jsonl-utils.ts），测试行数：约 20 行（jsonl-utils.test.ts）。远低于任何阈值。 方案：范围恰好——一个函数、一个行为修复、一个测试。从 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: for The PR takes exactly this approach. The diff is two lines of production code (plus a comment explaining the why) and one well-structured test. No correctness issues, no regressions, no convention violations. The comment in the production code is warranted — the why (empty-array edge case) is genuinely non-obvious from the code alone. No downstream consumers of Typecheck and ESLint both clean on the changed files. TestingNon-user-visible utility fix with no in-repo callers — unit tests are the appropriate verification (tmux not available on this runner). With the fix (29/29 pass): Without the fix (reverting only Fail-before / pass-after confirmed — the test pins exactly the bug being fixed. 中文说明代码审查独立方案： 对于 PR 采用了完全相同的方案。diff 包含两行生产代码（加一条解释 why 的注释）和一个结构良好的测试。无正确性问题、无回归、无规范违反。生产代码中的注释是合理的——空数组边界情况从代码本身确实不易看出。 仓库内没有 Typecheck 和 ESLint 在改动文件上均通过。 测试非用户可见的工具函数修复，仓库内无调用方——单元测试是合适的验证方式（此运行环境无 tmux）。 修复后 29/29 通过；仅还原 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is exactly what a good bugfix looks like: a real inconsistency in the module's own API, the simplest possible fix (two lines of production code), a comment that explains the non-obvious why, and a test that pins the behavior with proven fail-before/pass-after. The approach matches what I'd have done independently — per-item No in-repo callers hit this today, but 中文说明置信度：5/5 —— 每个阶段都干净，毫不犹豫地合并。 这是一个优秀 bugfix 的典范：模块自身 API 中的真实不一致、最简方案（两行生产代码）、解释非显而易见 why 的注释、以及经 fail-before/pass-after 验证的行为固定测试。方案与我独立想到的完全一致——逐条 仓库内目前没有调用方触发此问题，但 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
Review & Local Verification Report代码审查设计评价：经典的 off-by-one 修复。 本 PR 修复了 根因分析： // Before: join + trailing newline
const lines = data.map(item => JSON.stringify(item)).join('\n');
atomicWriteFileSync(filePath, `${lines}\n`);
// [].join('\n') = '' → '' + '\n' = '\n' → 1-byte file
// After: terminate each record
const lines = data.map(item => `${JSON.stringify(item)}\n`).join('');
atomicWriteFileSync(filePath, lines);
// [] → '' → 0-byte file ✅
// [{a:1}] → '{"a":1}\n' ✅影响： 修复前 测试： 验证 结论LGTM。 改动 3 行，语义正确，注释解释了 join vs terminate 的区别。 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
✅ Local Verification Report — PR #7533Verified by: @wenshao (maintainer) Verification Steps
Unit Test Results
Manual Verification —
|
| 步骤 | 命令 | 结果 |
|---|---|---|
| 单元测试 | npx vitest run src/utils/jsonl-utils.test.ts |
✅ 29/29 通过 |
| TypeScript 类型检查 | npx tsc --noEmit -p packages/core/tsconfig.json |
✅ 无错误 |
| ESLint | npx eslint packages/core/src/utils/jsonl-utils.{ts,test.ts} |
✅ 无警告 |
| 手动端到端验证 | 自定义脚本，分别测试 write([])、write([{...}])、write([{...},{...}]) |
✅ 3 个场景全部通过 |
单元测试结果
手动验证 — write([]) 行为
脚本直接调用修复后模块的 write()、read()、exists()、countLines()：
![手动验证 — write([]) 后所有访问器一致](https://raw.githubusercontent.com/wenshao/qwen-code/pr-7533-assets/pr-screenshots/pr7533-manual-verify.png)
手动运行的关键观察：
write(f, [])→ 文件内容""，大小0，exists()→false，read()→[]，countLines()→0。四个访问器现在完全一致。write(f, [{v:10},{v:20}])→'{"v":10}\n{"v":20}\n'— 与修复前非空数组的输出逐字节相同。write(f, [{x:1}])→'{"x":1}\n'— 单元素场景同样不变。
评估
修复最小、正确、安全：
- 根因 — 空数组经
join('\n') + '\n'产生'\n'（1 字节），导致exists()（size > 0）与read()（无记录）互相矛盾。 - 修复 — 改为逐条记录追加终止符（
map → JSON.stringify + '\n' → join('')），空数组写出空字符串。 - 非空输出 — 与旧实现逐字节相同（既有测试和手动脚本均验证）。
- 新增测试 — 精确覆盖回归场景：先写一条记录，再用
write([])清空，然后断言四个访问器一致。 - 无回归 — 全部 29 个测试通过；类型检查和 lint 均干净。
建议：✅ 可以合入。
Maintainer Local Verification ReportVerified locally on macOS (darwin 25.5.0, Node v22.22.2) against commit 1. Unit tests — with fix (29/29 pass)All 29 tests in
2. Fail-before — old code + new test (1 failed)Reverting only
3. Behavioral verification — OLD vs NEW implementationRan a standalone script comparing the old (
Verdict✅ Ready to merge. The fix is minimal (2 lines of production code), correct, byte-identical for all non-empty inputs, and well-tested. CI is green. No downstream in-repo callers pass an empty array today, but the fix protects SDK/extension consumers of the public API. 中文说明维护者本地验证报告在 macOS（darwin 25.5.0, Node v22.22.2）上针对分支 1. 单元测试——修复后（29/29 通过）
2. Fail-before——旧代码 + 新测试（1 个失败）仅将 3. 行为验证——新旧实现对比运行独立脚本，对比旧实现（
结论✅ 可以合入。 修复极简（2 行生产代码），正确，对所有非空输入逐字节一致，测试充分。CI 全绿。当前仓库内无调用方传入空数组，但此修复保护了公开 API 的 SDK/扩展使用方。 — Maintainer local verification · macOS · Node v22.22.2 · 2026-07-23 |
5 participants
What this PR does
jsonl.write(path, [])left a one-byte file containing a single\n. This terminates each record instead of joining with separators, so an empty array writes nothing. Output for a non-empty array is byte-identical.Why it's needed
For
data = []the join yields'', and the template literal then writes'\n'.That makes the module's own accessors contradict each other:
write(file, [])fs.statSync(file).size1exists(file)— documented as "exists and is not empty", testssize > 0trueread(file)— skips blank lines[]countLines(file)— counts non-empty lines0So clearing a JSONL file leaves something that reports as non-empty while containing no records, and any caller that branches on
exists()before reading takes the wrong path.writeis part of the public core API (export * from './utils/jsonl-utils.js'inpackages/core/src/index.ts), so this is reachable by SDK and extension consumers even though no in-repo caller passes an empty array today. Fixing it now keeps a futurewrite(file, [])from quietly hitting this.Reviewer Test Plan
How to verify
npx vitest run --root packages/core src/utils/jsonl-utils.test.ts→ 29/29.write() with an empty array leaves a genuinely empty filewrites a record, clears the file withwrite(file, []), then asserts all four accessors agree: content'',size === 0,read() === [],exists() === false. Reverting onlyjsonl-utils.tsfails it withexpected '\n' to be ''.write() full-file replaces existing content via atomic writeandwrite() creates parent dirs when missingtests are untouched and still pass — they pin the non-empty output ('{"x":1}\n') that must not change.Evidence (Before & After)
write(f, [])→ file is"\n",statSync().size === 1,exists(f) === true,read(f) === [].write(f, [])→ file is"",size === 0,exists(f) === false,read(f) === [].write(f, [{v:10},{v:20}])→'{"v":10}\n{"v":20}\n'in both cases.Tested on
macOS: the
jsonl-utilssuite passes locally (29 tests) with proven fail-before/pass-after; eslint clean. String construction with no platform-dependent behavior — the test asserts on\nexplicitly rather than on line endings the OS might vary — so no manual QA is required; CI covers Windows/Linux.Environment (optional)
Node v24;
@qwen-code/qwen-code-coreworkspace; vitest 3.2.Risk & Scope
join('\n') + '\n'and per-item+ '\n'differ only when the array is empty), which the untouched existing tests pin.writeLine/writeLineSyncare append helpers with no empty-input case and are unchanged.exists() === true— which is the bug being fixed.Linked Issues
None — found by reading
writeagainstexistsandreadin the same module.中文说明
本 PR 的作用
jsonl.write(path, [])会留下一个仅含单个\n的 1 字节文件。本 PR 改为逐条记录追加终止符，而非用分隔符 join，使空数组写出空内容。非空数组的输出逐字节完全一致。为什么需要
当
data = []时 join 得到''，模板字符串随即写入'\n'。这导致该模块自身的各个访问函数互相矛盾：
write(file, [])后fs.statSync(file).size1exists(file)——文档写明「存在且非空」，判断size > 0trueread(file)——跳过空行[]countLines(file)——统计非空行0于是清空一个 JSONL 文件后，会留下一个「报告为非空、却不含任何记录」的东西；任何在读取前依据
exists()分支的调用方都会走错路径。write属于 core 的公开 API（packages/core/src/index.ts中的export * from './utils/jsonl-utils.js'），因此 SDK 与扩展使用方都可触达它——尽管当前仓库内尚无调用方传入空数组。现在修好，可避免将来某处write(file, [])悄悄踩中。复核测试计划
如何验证
npx vitest run --root packages/core src/utils/jsonl-utils.test.ts→ 29/29 通过。write() with an empty array leaves a genuinely empty file先写入一条记录，再用write(file, [])清空，然后断言四个访问函数彼此一致：内容为''、size === 0、read() === []、exists() === false。仅还原jsonl-utils.ts时该测试失败：expected '\n' to be ''。write() full-file replaces existing content via atomic write与write() creates parent dirs when missing未作改动且仍然通过——它们固定了不得改变的非空输出（'{"x":1}\n'）。证据（修复前后对比）
write(f, [])→ 文件为"\n"，statSync().size === 1，exists(f) === true，read(f) === []。write(f, [])→ 文件为""，size === 0，exists(f) === false，read(f) === []。write(f, [{v:10},{v:20}])→ 两种实现均为'{"v":10}\n{"v":20}\n'。测试环境
macOS：
jsonl-utils套件本地通过（29 个测试），并验证了 fail-before/pass-after；eslint 干净。纯字符串构造，无平台相关行为——测试显式断言\n而非依赖系统可能不同的行尾——因此无需人工 QA；Windows/Linux 由 CI 覆盖。运行环境（可选）
Node v24；
@qwen-code/qwen-code-core工作区；vitest 3.2。风险与影响范围
join('\n') + '\n'与逐条+ '\n'只在数组为空时才有差异），这一点由未改动的既有测试固定。writeLine/writeLineSync是追加型辅助函数，不存在空输入场景，未作改动。exists() === true」，而那正是本次修复的 bug。关联 Issue
无——通过在同一模块内对照阅读
write、exists与read发现。