Skip to content

Fix/swe family tool dialect - #252

Open
wjurkowlaniec wants to merge 4 commits into
dwgx:masterfrom
wjurkowlaniec:fix/swe-1-7-tool-dialect
Open

Fix/swe family tool dialect#252
wjurkowlaniec wants to merge 4 commits into
dwgx:masterfrom
wjurkowlaniec:fix/swe-1-7-tool-dialect

Conversation

@wjurkowlaniec

Copy link
Copy Markdown

改了什么 / What changed

pickToolDialect in src/handlers/tool-emulation.js so that every model key starting with swe- is routed to the kimi_k2 tool-call dialect.

为什么 / Why

SWE-family models use the same � ... � section-token tool-call format as Kimi K2. the new rule makes the dialect dispatch model-agnostic for the whole SWE family, preventing "invalid tool call" errors while using external harness.

测试 / Testing

  • Unit tests: node --test test/tool-emulation.test.js54/54 pass, including the new routes all SWE family models to Kimi K2 vLLM dialect test.
  • Live integration: sent POST /v1/chat/completions with tool_choice=required to every SWE model served by the local instance:

Checklist

  • 代码风格和现有文件一致 / Code style matches existing files
  • 没有引入 npm 依赖 / No new npm dependencies (project is zero-dep)
  • 涉及 LS binary 协议改动时 在 PR 描述里注明字段号来源 / If touching LS protocol, document field-number source in the PR description
  • 涉及 dashboard UI 用 App.confirm / App.prompt 不用浏览器原生 alert/confirm / Uses App.confirm / App.prompt, not native dialogs (if dashboard)

@wjurkowlaniec
wjurkowlaniec marked this pull request as draft August 7, 2026 18:15
@wjurkowlaniec
wjurkowlaniec force-pushed the fix/swe-1-7-tool-dialect branch from 3126255 to 898af0f Compare August 7, 2026 18:15
@wjurkowlaniec
wjurkowlaniec marked this pull request as ready for review August 7, 2026 18:16
@wjurkowlaniec wjurkowlaniec changed the title Fix/swe 1 7 tool dialect Fix/swe family tool dialect Aug 7, 2026
@dwgx

dwgx commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Review: the premise is plausible — swe-1-7 is a Kimi K2 fine-tune, and this repo's
own release notes say so. But the change lands master red, and the evidence for the one
claim it rests on is not in the PR. Two blockers below.

What I ran

head 898af0f, worktree-isolated:

Item Result
test/devin-connect-openai.test.js on this branch 66 pass / 2 fail
same file on clean master 68 pass / 0 fail
bash-prefix-repair-boundary.json baseline spec expects 94, measured 95
schema-ref-fanout-budget.json baseline spec expects 72, measured 73

The dialect table itself does what you say it does — I drove it:

swe-1-7            kimi_k2          swe-1-6-slow   kimi_k2
swe-1.5            kimi_k2          claude-sonnet-4-6  openai_json_xml

So the routing works. What it does downstream is the problem.


B1 (blocker) — tool calls stop being extracted on the streaming path

ToolCallStreamParser is dialect-exclusive. For kimi_k2 the only sentinel it looks
for is <|tool_calls_section_begin|> (tool-emulation.js:1378), so <tool_call> XML is
invisible to it. Driven, same input to both paths:

model=swe-1-7  dialect=kimi_k2
  STREAM      toolCalls=0   markup reaches client as visible content
  NON-STREAM  toolCalls=1   content=null

Both failing tests are this:

✖ emits a tool_calls delta and finishes with finish_reason=tool_calls   (model swe-1-6-slow)
✖ extracts a tool call that arrived in the reasoning channel (stream)   (model swe-1-7)

The second one matters more than its name suggests. It was added with the #241/#243
rescue work, it uses swe-1-7, and the comment above it reads:

sees markup — worse than the empty turn the promotion exists to prevent, and on the
exact model this whole change set targets

Why non-stream survives, and why that is not reassuring. It is not that the
non-stream path handles the dialect. parseToolCallsFromText has a salvage pass that
runs only when the primary parser returns zero (tool-emulation.js:1697) — it recovers
the call as whitespace_bare_json and leaves the shell behind:

parseToolCallsFromText(xml, {modelKey:'swe-1-7'})
  -> toolCalls: 1
  -> text: "<tool_call>\n\n</tool_call>"      ← still leaking

and then devin-connect-openai.js:458 sets message.content = null because a tool call
exists, which throws the shell away. Non-stream passes by accident, through two
unrelated mechanisms.
The streaming path has neither, so it emits the raw markup with
finish_reason: 'stop' and no tool call — the agent loop does not advance.

That is a streaming-vs-non-streaming fork on one finish_reason, which is the second of
the three dimensions this repo checks on any cross-layer change. It has been the defect
shape here six times.

B2 (blocker) — the prefix match is the mistake its neighbour already made

I nearly filed the failing tests as the whole finding, then checked the neighbours —
and the neighbour twelve lines up has the scar:

if (normalizedProvider === 'moonshot' || normalizedModelKey.startsWith('kimi')) {
  // The Kimi K2 vLLM dialect is verified working only against the original
  // `kimi-k2` and `kimi-k2-thinking` SKUs. Newer Moonshot SKUs … are served by a
  // different upstream runtime that rejects vLLM markup.
  if (normalizedModelKey === 'kimi-k2' || normalizedModelKey === 'kimi-k2-thinking') return 'kimi_k2';
  return 'openai_json_xml';
}

The kimi branch matches on a prefix and then deliberately narrows to two exact model
keys
, because the dialect is a property of the serving runtime, not of the base
model. swe-1-7 being a K2 fine-tune is an argument about the base model; Windsurf
serves it through Cascade, not Moonshot's vLLM.

The history is sharper than the comment. On 2026-05-07, a7e36d1 moved all Moonshot
SKUs off kimi_k2"Cascade returns ~16 token empty responses instead of tool calls"
and f0a6598 put the narrow form back the same day once the empty responses turned out
to be an upstream outage rather than a dialect fault. kimi_k2 has been reachable by
exactly two model keys ever since. startsWith('swe') opens it to every current and
future swe-*.

Related, from the #238 ledger entry on this same model family:

修复键在失效特征(reasoning-only finish + 有 tools)而不是模型名上 —— 本仓库吃过几次按模型名硬编码的亏

The claim I cannot check, and what would settle it

// SWE-1.X use the same <|tool_call_begin|> / <|tool_call_end|> dialect as
// Kimi K2 — observed in production traffic 2026-08-07.

I have no way to verify that from here, and I am not asserting it is wrong — if swe-1-7
really does emit section tokens, the direction of this PR is right and the two tests are
encoding the old prompt. But note the tests cannot settle it either, in either
direction: pickToolDialect picks the protocol header and the parser, so a fixture's
markup is co-determined by the prompt we sent. That is exactly why the observation needs
to come from the wire.

What would make this mergeable:

  1. A capture. One raw upstream response from swe-1-7 with tools present, showing
    <|tool_calls_section_begin|> in the channel it arrived on. LOG_LEVEL=debug, or a
    fixture under test/_research/. This is the whole load-bearing claim.
  2. Green master. Update the two devin-connect-openai expectations to the dialect
    you are moving to, so the change is visible as intended rather than as a regression.
  3. Narrow the match, or say in the comment why swe is safe as a family when kimi
    was not. swe-1-5 / swe-1-6 / swe-1-7 are different upstream deployments;
    verifying one does not carry to the others, and the free-tier default (swe-1-7) is
    the one most deployments hit.
  4. Two spec baselines, because your new test shifts them and nothing in CI will tell
    you: bash-prefix-repair-boundary.json 94 → 95 and schema-ref-fanout-budget.json
    72 → 73. The suite does not run the mutation specs, so this stays invisible until
    someone runs npm run mutate and gets a red that looks unrelated to your PR.

One more to think about before the next round: formatAssistantToolCallForDialect
(tool-emulation.js:1062) serializes stored history in the same dialect. Flipping it
rewrites prior <tool_call> turns as section tokens, and the parser can no longer read
the old form. That is the shape of #86 "上下文会丢". Worth a sentence in the PR on what
happens to a conversation that is mid-flight when this ships.

The dialect-per-family idea is sound and the code is in the right function. It needs the
wire evidence and a green suite. Ping me when it is updated and I will re-run the gate
plus the adversarial pass.

@dwgx

dwgx commented Aug 9, 2026

Copy link
Copy Markdown
Owner

评审:方言这条观察我认为是对的,但 startsWith('swe') 的范围太宽 —— 它把 swe-1-6-slow 也改了,而那是每个账号唯一都能到的免费 selector,现有测试正好证明它用的不是 kimi_k2 请收窄,改完就合。

先说一句与你无关的:这个 PR 的 CI 从 08-07 起一直卡在 action_required(首次贡献者需要我点一下放行),三天里没人看得到它是红的。那是我的疏忽,不是你的。刚放行了。

我实跑过什么

head 898af0f,worktree 隔离,Node 24:

结果
干净 origin/mastertest/devin-connect-openai.test.js 68 pass / 0 fail
本 PR 同一个文件 66 pass / 2 fail
CI shard 3 fail(test/devin-connect-openai.test.js: exit 1

两条失败:

✖ emits a tool_calls delta and finishes with finish_reason=tool_calls
  AssertionError: a tool_calls delta was emitted

✖ extracts a tool call that arrived in the reasoning channel (stream)
  AssertionError: raw tool markup reached the client as the visible answer
    —— the promoted text must go through the same extraction the content path uses

第二条的措辞值得注意:原始工具标记直接当可见答案发给客户端了。

M1(blocker)— startsWith('swe') 命中了一个反例,而它是免费档唯一可达的

两条失败测试用的模型是 swe-1-6-slowtest/devin-connect-openai.test.js:261),喂的是 <tool_call>{...}</tool_call>。我驱动了方言判定:

                master              本 PR
swe-1-6-slow    openai_json_xml  →  kimi_k2      ← 反例
swe-1-7         openai_json_xml  →  kimi_k2
swe-1.5         openai_json_xml  →  kimi_k2
m               openai_json_xml  →  openai_json_xml

解析器改判成 kimi_k2<|tool_call_begin|>)后不再认 <tool_call> 标记,所以工具调用提取不出来,标记原样流给客户端。

为什么这条不能算"测试过时": swe-1-6-slow 不是 fixture 里编的名字,是活的上游 selector,而且是特殊的那一个:

// src/devin-connect-models.js:150
export const FREE_REACHABLE_SELECTORS = new Set(['swe-1-6-slow']);

src/auth.js:315 那段注释说明了它的地位 —— 它"appears in no catalog snapshot at all",是 drought 模式下"the one selector every account can reach"。也就是说,这条回归打在免费档用户的工具调用上,而那是这个代理被用得最多的路径之一。

顺带一个佐证:swe-1-6-slow 也不在 src/models.jsMODELS 表里(catalog 里的 SWE 只有 swe-1-7 / swe-1-7-lightning / swe-1.5{,-fast,-thinking} / swe-1.6{,-fast})。所以前缀匹配把一个表外但线上活着的 selector 一起卷进来了 —— 这正是前缀匹配最容易出的那种错。

修法:按你实际观察到的那批列举

你的证据是"SWE-1.X 在生产流量里用这个方言,2026-08-07 观察"。那就让代码只覆盖被观察过的:

// 举例,具体名单按你的观察来
if (/^swe-1[.-](5|6|7)(-|$)/.test(normalizedModelKey)
    && !normalizedModelKey.endsWith('-slow')) {
  return 'kimi_k2';
}

或者显式白名单。哪种都行,我要拦的只是"未经观察的 SWE 变体被默认卷入"这件事 —— 上游随时可能再加一个 swe-*,而下一个未必是 kimi_k2

你的测试(test/tool-emulation.test.js 里那 8 条 assert.equal(pickToolDialect(...), 'kimi_k2'))也请补一条反向断言swe-1-6-slow 必须仍是 openai_json_xml。只有正向断言的话,下一次范围再放宽仍然全绿。

M2(合并前必改)— 两个突变 spec 的 baseline 会失效

这个 PR 给 test/tool-emulation.test.js 加了 1 条测试,而两个 spec 都把这个文件计入基数。我在 master 上实测:

spec 实测 master spec 声明 合本 PR 后
bash-prefix-repair-boundary.json 94 94 ✅ 95
schema-ref-fanout-budget.json 72 72 ✅ 73

scripts/mutate-verify.mjs:208 在不匹配时是 die(),而测试套件不跑 spec,所以三个绿灯(你的 CI、本地 npm test、合并后门禁)都看不见这条。改 M1 时如果测试数又变,这两个数请一并按实测重量,不要从 94+1 推。

其余

方言判定放在 pickToolDialect 里、跟着 normalizedModelKey 走,位置是对的;改动只有 5 行加一条测试,范围克制。这个 PR 的价值在观察本身 —— SWE 系列跟 Kimi K2 共用 section-token 格式这件事,不看生产流量是想不到的。收窄之后我直接合。

改完 ping 我,我重跑门禁 + 那两个 spec。

- src/handlers/tool-emulation.js: constrain observed SWE routing, preserve native Kimi history continuity, and parse native plus legacy streamed forms (PR dwgx#252).

- src/runtime-config.js, src/windsurf.js: suppress only contradictory tool syntax reinforcement (PR dwgx#252).

- scripts/mutate-verify.mjs: read Node 22 and Node 24 TAP summary counters so the required mutation gate is portable (R2).

- test/: pin stream/non-stream parity, history serialization, wire evidence, and measured mutation baselines (R2).
@wjurkowlaniec

wjurkowlaniec commented Aug 9, 2026

Copy link
Copy Markdown
Author

Thanks @dwgx for your review!

I narrowed this back to the evidence from real SWE/Kimi traffic. The final scope is deliberately small:

  1. Route only observed SWE-1.5, SWE-1.6, and SWE-1.7 variants to kimi_k2. swe-1-6-slow, swe-1-7-lightning, and unknown future variants retain the safe openai_json_xml default.
  2. Add a sanitized swe-1-7 wire capture showing the native Kimi section-token format.
  3. Support native Kimi section tokens plus the legacy XML JSON fallback in both streaming and non-streaming parsing.
  4. Preserve Kimi/SWE tool history: OpenAI call_* IDs map to functions.<name>:<index>, parallel calls remain one native section, and consecutive results become one native continuation. This prevents completed calls from being treated as new work.
  5. Keep an XML reinforcement from contradicting a Kimi preamble, while leaving other prompt paths unchanged.

I removed the unverified wrapperless-parser branch and the unrelated Node TAP reporter change, rather than broadening the PR without production evidence.

Validation:

  • Full CI-equivalent Node 24 run: syntax/JSON checks plus all 4 test shards passed
  • Focused affected suites: 205 pass / 0 fail
  • npm run secret-scan
  • Fresh live Hermes smoke with swe-1-7-medium: the terminal tool executed and returned the requested working directory

lightning was probed separately and does not emit Kimi section tokens, so it is intentionally not presented as Kimi-compatible here. Its narrative/NLU-recovery behavior is a separate issue.

As a result, "unlimited" use of SWE model with external harness 😎

- src/handlers/tool-emulation.js: keep swe-1-7-lightning on openai_json_xml after the live probe showed narrative calls rather than Kimi section tokens (PR dwgx#252).

- test/: pin the lightning exclusion and keep wrapperless grammar coverage bound to the actual kimi_k2 dialect.
- Remove wrapperless parser paths and generic prompt protocol detection without production evidence (PR dwgx#252).
- Retain section/XML compatibility, narrow SWE routing, and Kimi history continuity.
- Revert unrelated Node TAP reporter handling from this PR.
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.

2 participants