Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix(dingtalk): reopen code fences without inserting a blank line#5204

Merged
wenshao merged 1 commit into
QwenLM:mainQwenLM/qwen-code:mainfrom
he-yufeng:fix/dingtalk-fence-blank-linehe-yufeng/qwen-code:fix/dingtalk-fence-blank-lineCopy head branch name to clipboard
Jun 16, 2026
Merged

fix(dingtalk): reopen code fences without inserting a blank line#5204
wenshao merged 1 commit into
QwenLM:mainQwenLM/qwen-code:mainfrom
he-yufeng:fix/dingtalk-fence-blank-linehe-yufeng/qwen-code:fix/dingtalk-fence-blank-lineCopy head branch name to clipboard

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

What this PR does

In splitChunks, when a code block spans the chunk limit the next chunk reopens the fence with a value that already ends in a newline, and the line-append step then adds a second newline. The continued chunk therefore begins with the fence followed by a blank line instead of the code. This drops the trailing newline from the reopened fence and lets the existing append add the single separator.

Why it's needed

A fenced code block longer than the 3800-char DingTalk limit gets a spurious blank first line in every continued chunk, which renders as leading whitespace inside the code. The existing fence test only checked trimStart().startsWith('```'), so it didn't catch the extra newline.

Reviewer Test Plan

How to verify

Run npx vitest run packages/channels/dingtalk/src/markdown.test.ts. The new test reopens code fences without inserting a blank line builds a code block past the chunk limit and asserts the continued chunk starts with the fence directly followed by the code, not a blank line. It fails before this change and passes after; the other 16 markdown tests are unchanged.

Evidence (Before & After)

N/A — non–user-visible string handling, covered by the new unit test.

splitChunks reopened a fence with a value ending in a newline and then appended another, so a continued code block started with a blank line. Drop the trailing newline from the reopened fence so the existing append adds the single separator.
@yiliang114
yiliang114 requested a review from yuanyuanAli June 16, 2026 15:27

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

✅ Local verification — LGTM

Built and ran real tests for this PR in an isolated git worktree checked out at the PR head (6b4552d5c), Node v22.22.2 / Vitest 3.2.4. Verdict: the fix is correct and the new test genuinely guards it. Safe to merge.

What the fix does

In splitChunks, when a fenced code block crosses the 3800-char boundary the next chunk reopens the fence. The buggy reset used a value already ending in a newline, and the subsequent line-append (buf += (buf ? '\n' : '') + line) added a second \n because buf was already truthy. The continued chunk therefore began with fence + blank line, rendering as spurious leading whitespace inside the code block. The fix drops that trailing newline so the existing append contributes the single separator:

-      buf = inCode ? '```\n' : '';
+      buf = inCode ? '```' : '';

A/B test — the new test actually catches the bug

Same PR test file; only the 1-line source fix was toggled:

Source under test markdown.test.ts result
PR (fixed) — reset to bare fence 17 passed (16 existing + 1 new) ✅
Pre-fix — reset to fence + \n (revert just that line) 1 failed / 16 passed

Failure on the buggy source:

× splitChunks > reopens code fences without inserting a blank line
  → expected true to be false   // chunks[1].startsWith(fence+'\n\n') === true
  ❯ src/markdown.test.ts:104:48

This is the meaningful proof: the new test fails before the change and passes after, so it is a real guard rather than a tautology.

Behavioral A/B on the real splitChunks module

Input: one code block over the limit (fence + x\n×2000 + fence) → 2 chunks. Inspecting the raw start of chunks[1]:

PRE-FIX  : "```\n\nx\nx"   hasBlankLine=true    L1 = (blank)  ← spurious whitespace
PR FIX   : "```\nx\nx\n"   hasBlankLine=false   L1 = x        ← code starts immediately

Why the existing test didn't catch it

The pre-existing boundary test asserts chunks[1].trimStart().startsWith(fence)trimStart() strips the leading blank line, so it was structurally blind to this bug. The new test uses raw startsWith, closing that gap.

Scope / risk

  • One-line change, confined to the inCode branch of splitChunks (DingTalk channel only). The non-code reset (: '') is untouched.
  • Full @qwen-code/channel-dingtalk package suite: 17/17 passing on the restored PR head.
  • No other test files in the package; no collateral.
🇨🇳 中文版本(点击展开)

✅ 本地验证 —— 建议合并

在隔离的 git worktree(检出 PR HEAD 6b4552d5c)中真实构建并运行了测试,环境 Node v22.22.2 / Vitest 3.2.4。结论:修复正确,新增测试确实能守护该行为,可以合并。

修复内容

splitChunks 中,当代码围栏跨越 3800 字符上限时,下一个分片会重新打开围栏。有 bug 的重置写法用的是一个本身就以换行结尾的值,随后的行追加逻辑(buf += (buf ? '\n' : '') + line)因为此时 buf 已非空而又多加了一个 \n,于是续接分片以「围栏 + 一个空行」开头,渲染时表现为代码块内部多出前导空白。修复去掉了那个末尾换行,让已有的追加逻辑只贡献那一个分隔符:

-      buf = inCode ? '```\n' : '';
+      buf = inCode ? '```' : '';

A/B 测试 —— 新测试确实能抓到这个 bug

同一份 PR 测试文件,仅切换那一行源码修复:

被测源码 markdown.test.ts 结果
PR(已修复) —— 重置为纯围栏 17 通过(16 旧 + 1 新)✅
修复前 —— 重置为 围栏 + \n(仅回退该行) 1 失败 / 16 通过

在有 bug 的源码上的失败信息:

× splitChunks > reopens code fences without inserting a blank line
  → expected true to be false   // chunks[1].startsWith(fence+'\n\n') === true
  ❯ src/markdown.test.ts:104:48

这正是关键证据:新测试在修复前失败、修复后通过,说明它是真实的守护,而非恒真断言。

对真实 splitChunks 模块的行为 A/B

输入:一个超长代码块(围栏 + x\n×2000 + 围栏)→ 2 个分片。检查 chunks[1] 的原始开头:

修复前 : "```\n\nx\nx"   hasBlankLine=true    第1行 = (空行)  ← 多余空白
PR修复 : "```\nx\nx\n"   hasBlankLine=false   第1行 = x        ← 代码紧跟围栏

为什么原有测试没抓到

原有的边界测试断言是 chunks[1].trimStart().startsWith(fence)trimStart() 会把开头那行空行去掉,因此从结构上就对该 bug 视而不见。新测试改用原始 startsWith,补上了这个缺口。

影响范围 / 风险

  • 单行改动,仅限 splitChunksinCode 分支(仅影响 DingTalk 渠道)。非代码块的重置分支(: '')未改动。
  • @qwen-code/channel-dingtalk 包完整测试套件在还原后的 PR HEAD 上 17/17 全通过
  • 该包内无其他测试文件,无连带影响。

Verified locally via tmux in an isolated worktree (A/B: only the 1-line fix toggled). Logs: post-fix 17/17, pre-fix 1 fail @ markdown.test.ts:104.

@wenshao
wenshao merged commit e89390c into QwenLM:main Jun 16, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.