fix(provider): harden Kimi Moonshot tool-call compatibility#378
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a Kimi/Moonshot model classifier and applies Kimi-specific message filtering and schema sanitization; introduces and exports ChangesKimi/Moonshot Protocol Compatibility
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant SDK as resolveSDK
participant Transform as ProviderTransform
participant ProviderAPI as Upstream Provider
Client->>SDK: POST request (body JSON/string)
SDK->>SDK: if body is string, try parse JSON
SDK->>Transform: openAICompatibleRequestBodyText(model, body)
Transform->>Transform: isKimiMoonshotModel? → if true, strip empty assistant content when tool_calls present
Transform-->>SDK: transformed body string or undefined
SDK->>ProviderAPI: forward POST with transformed or original body
ProviderAPI-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 5/10 reviews remaining, refill in 27 minutes and 49 seconds. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function isKimiMoonshotModel to identify Kimi and Moonshot models across various identifier fields and applies message normalization to filter empty content for these models. Additionally, the schema transformation logic is enhanced to automatically infer missing type fields (object, array, or string) based on the presence of properties, items, or enums. A review comment suggests refining the regex used for model identification to correctly handle space-separated identifiers within the joined string.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/provider/transform.ts`:
- Around line 72-74: isKimiMoonshotModel currently includes model.name in the
identification string which allows display labels to opt into Kimi/Moonshot
behavior; remove model.name from the checked fields so only canonical
identifiers are considered. Update the function to build ids from model.id,
model.providerID, and model.api.id (filter(Boolean).join(" ").toLowerCase()) and
keep the existing substring checks and regex (/k2p.../) unchanged, ensuring
display label changes do not trigger Kimi/Moonshot rewrites.
- Around line 1113-1118: The missing-type inference branch (the if (!("type" in
result)) block) doesn't handle boolean enums; extend it to detect when
result.enum is an array of booleans (e.g., [true,false] or all values typeof
"boolean") and set result.type = "boolean" just like it sets "string" for string
enums; update the checks around result.enum in that block so it assigns
"boolean" when appropriate while leaving existing object/array/string handling
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48a2d281-5727-412a-8245-c3821179dd25
📒 Files selected for processing (2)
packages/opencode/src/provider/transform.tspackages/opencode/test/provider/transform.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/opencode/src/provider/transform.ts (1)
72-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep Kimi/Moonshot detection off display names.
Line 73 still includes
model.name, so a label like"Kimi-compatible proxy"can opt an unrelated model into empty-message filtering and schema rewrites. That breaks the stated goal of leaving non-Kimi providers unchanged.Suggested fix
- const ids = [model.id, model.providerID, model.name, model.api.id].filter(Boolean).join(" ").toLowerCase() + const ids = [model.id, model.providerID, model.api.id].filter(Boolean).join(" ").toLowerCase()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/provider/transform.ts` around lines 72 - 74, The isKimiMoonshotModel function is incorrectly checking model.name so display labels like "Kimi-compatible proxy" can trigger Kimi/Moonshot logic; remove model.name from the assembled ids string and only use model.id, model.providerID, and model.api.id (preserve the filter(Boolean).join(" ").toLowerCase() flow) so the includes("moonshot"), includes("kimi") checks and the k2p regex run only against canonical identifiers, leaving providers with Kimi-like display names unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/provider/provider.ts`:
- Around line 1516-1519: The conditional that rewrites POST bodies only checks
exact equality of model.api.npm to "@ai-sdk/openai-compatible", so
versioned/pinned specs like "npm:`@ai-sdk/openai-compatible`@2" are skipped;
update the condition around model.api.npm in the block that calls
ProviderTransform.openAICompatibleRequestBody (the if that also checks opts.body
and opts.method === "POST") to detect any OpenAI-compatible package spec (e.g.,
use a startsWith or regex match such as testing
model.api.npm?.includes("@ai-sdk/openai-compatible") or
/^(`@ai-sdk`\/openai-compatible)(@|$)/) so all aliased/versioned/pinned forms
trigger the same request-body rewrite.
In `@packages/opencode/src/provider/transform.ts`:
- Around line 95-117: schemaTypeFromValues and inferSchemaType currently default
ambiguous or mixed-type cases to "string"; change them to return undefined for
any non-conclusive inference so we don't falsely inject type: "string".
Specifically, update schemaTypeFromValues to return undefined when types.size >
1 (except the integer+number collapse which stays returning "number"), and
update inferSchemaType to return undefined as the final fallback instead of
"string" (keep existing branches for enum, const, OBJECT_SCHEMA_KEYS,
ARRAY_SCHEMA_KEYS, STRING_SCHEMA_KEYS, NUMBER_SCHEMA_KEYS). This ensures only
clear/obvious type inferences produce a concrete string type.
---
Duplicate comments:
In `@packages/opencode/src/provider/transform.ts`:
- Around line 72-74: The isKimiMoonshotModel function is incorrectly checking
model.name so display labels like "Kimi-compatible proxy" can trigger
Kimi/Moonshot logic; remove model.name from the assembled ids string and only
use model.id, model.providerID, and model.api.id (preserve the
filter(Boolean).join(" ").toLowerCase() flow) so the includes("moonshot"),
includes("kimi") checks and the k2p regex run only against canonical
identifiers, leaving providers with Kimi-like display names unaffected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 27958a11-6252-405a-93b9-ab4b787cee28
📒 Files selected for processing (3)
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.tspackages/opencode/test/provider/transform.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/opencode/test/provider/transform.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/provider/provider.ts`:
- Around line 1516-1519: The request-body rewrite assumes opts.body is a JSON
string and calls JSON.parse directly; wrap this in a safe guard: first check
opts.body is a string and the request has a JSON content-type (e.g., headers
include "application/json"), then attempt to parse inside a try/catch; on parse
success call ProviderTransform.openAICompatibleRequestBody(model, body) and
reassign opts.body to the stringified result, but on parse failure or non-JSON
content simply leave opts.body unchanged (fail open) so the request proceeds to
fetch. Target the conditional that checks
model.api.npm.includes("@ai-sdk/openai-compatible") && opts.body && opts.method
=== "POST" and update the parse logic there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0de195ba-7571-474b-8282-e2a23c351db6
📒 Files selected for processing (3)
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.tspackages/opencode/test/provider/transform.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/opencode/test/provider/transform.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/provider/transform.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/provider/transform.test.ts (1)
2014-2046: ⚡ Quick winAdd one negative test to lock the tool-call guard scope
Please add a regression asserting Kimi assistant messages with empty
contentbut withouttool_callsare left unchanged byopenAICompatibleRequestBody. That directly protects the “assistant tool-call messages only” boundary.Proposed test addition
test("omits empty content from final OpenAI-compatible assistant tool-call payloads", () => { @@ expect(result.messages[1].content).toEqual([{ type: "text", text: "not empty" }]) }) + + test("does not omit empty content when assistant message has no tool calls", () => { + const result = ProviderTransform.openAICompatibleRequestBody(kimiModel, { + model: "k2p6", + messages: [ + { + role: "assistant", + content: "", + }, + ], + }) as any + + expect(result.messages[0].content).toBe("") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/provider/transform.test.ts` around lines 2014 - 2046, Add a negative regression in the existing test that calls ProviderTransform.openAICompatibleRequestBody (using kimiModel) to verify that an assistant message with empty content but no tool_calls is NOT stripped—i.e., include an assistant message where content is "" and tool_calls is undefined/absent, call openAICompatibleRequestBody, and assert the resulting message still has the content property (equal to "" or unchanged) so the removal logic only applies when tool_calls are present; use the same test setup pattern as the surrounding assertions to locate where to place this extra expect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/opencode/test/provider/transform.test.ts`:
- Around line 2014-2046: Add a negative regression in the existing test that
calls ProviderTransform.openAICompatibleRequestBody (using kimiModel) to verify
that an assistant message with empty content but no tool_calls is NOT
stripped—i.e., include an assistant message where content is "" and tool_calls
is undefined/absent, call openAICompatibleRequestBody, and assert the resulting
message still has the content property (equal to "" or unchanged) so the removal
logic only applies when tool_calls are present; use the same test setup pattern
as the surrounding assertions to locate where to place this extra expect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 848c8e18-c89f-4791-9682-e7c391be49cc
📒 Files selected for processing (3)
packages/opencode/src/provider/provider.tspackages/opencode/src/provider/transform.tspackages/opencode/test/provider/transform.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/opencode/src/provider/provider.ts
- packages/opencode/src/provider/transform.ts
This was referenced May 3, 2026
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
1 participant
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.
Summary
typevalues in schema positions, following Kimi Code CLI's stricter-schema compatibility pattern.contentfield, matching Kimi Code CLI's empty-content workaround.Why
Kimi/Moonshot-compatible endpoints are stricter than many OpenAI-compatible providers about empty message content and JSON schema shape. PawWork already sanitized
$refsiblings and preserved root$defs/definitions, but Kimi for Coding aliases such askimi-for-coding/k2p6could miss that path, and schemas with obvious intent but missingtypecould still be rejected.This keeps the fix at the provider protocol layer. It does not add Kimi-specific GitHub behavior, prompt tuning, compaction changes, or loop containment. The repeated successful tool-call loop work remains tracked separately by #279.
Related Issue
Closes #377
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
Please focus on whether
isKimiMoonshotModelis narrow enough for known Kimi/Moonshot aliases, whether final request-bodycontentomission is correctly limited to empty assistant tool-call messages, and whether missing-type normalization stays limited to schema positions instead of metadata containers.Risk Notes
Low to moderate provider-layer risk. The empty-content filtering, final request-body cleanup, and schema type normalization are gated to Kimi/Moonshot-compatible models, but that detector intentionally includes
k2p*aliases so Kimi for Coding paths are covered.How To Verify
Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Bug Fixes
Tests