Skip to content

Fix:Add KimiExecutor to fix the error of reasoning_content is missing in kimi thing mode - #3132

Merged
diegosouzapw merged 3 commits into
diegosouzapw:mainfrom
bypanghu:main
Jun 4, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:mainfrom
bypanghu:main

Conversation

@bypanghu

@bypanghu bypanghu commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Describe the user-facing or operational change.

Related Issues

  • Closes #
  • Related to #

Validation

  • npm run lint
  • npm run test:unit
  • npm run test:coverage
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Tests Added Or Updated

  • List every changed or added automated test file.
  • If no production code changed, state that here.

Coverage Notes

  • If this PR changes src/, open-sse/, electron/, or bin/, explain which tests cover the change.
  • If coverage moved down in any touched file, explain why and what follow-up task will recover it.

Reviewer Notes

  • Call out any risky areas, migrations, feature flags, or manual validation that reviewers should know about.

@bypanghu
bypanghu requested a review from diegosouzapw as a code owner June 4, 2026 02:09

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces KimiExecutor to handle Kimi-specific request transformations, specifically ensuring reasoning_content is present in tool calls when thinking is active, and registers it under the "kimi-coding-apikey" alias. Feedback highlights a bug in applyKimiRequestDefaults that bypasses general provider defaults, incorrect provider registration and constructor arguments in index.ts, and a violation of the repository style guide due to missing unit tests for the new executor.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +65 to +75
function applyKimiRequestDefaults(body: unknown, defaults?: JsonRecord | null): unknown {
const record = asRecord(body);
if (hasActiveKimiThinking(record ?? {}) && hasTools(record ?? {})) {
// 此处 kimi 开启了 thinking 功能且存在工具调用,确保 tool_calls 中包含 reasoning_content 字段以兼容老版本 Kimi 后端的要求
return ensureToolCallReasoningContent(record ?? {});
}

// 其他默认值
return applyProviderRequestDefaults(body, defaults);

}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

In applyKimiRequestDefaults, if Kimi thinking is active and tools are present, the function immediately returns the result of ensureToolCallReasoningContent and completely bypasses applyProviderRequestDefaults. This means any other configured request defaults (such as temperature or max_tokens) will not be applied.

Furthermore, if thinking is enabled via the defaults rather than the initial request body, hasActiveKimiThinking will evaluate to false before the defaults are applied, missing the opportunity to inject reasoning_content.

To fix this, apply the provider request defaults first, and then perform the Kimi-specific check and transformation on the resulting object.

function applyKimiRequestDefaults(body: unknown, defaults?: JsonRecord | null): unknown {
  const withDefaults = applyProviderRequestDefaults(body, defaults);
  const record = asRecord(withDefaults);
  if (record && hasActiveKimiThinking(record) && hasTools(record)) {
    // 此处 kimi 开启了 thinking 功能且存在工具调用,确保 tool_calls 中包含 reasoning_content 字段以兼容老版本 Kimi 后端的要求
    return ensureToolCallReasoningContent(record);
  }
  return withDefaults;
}

v0: new V0VercelWebExecutor(), // Alias
"kimi-web": new KimiWebExecutor(),
kimi: new KimiWebExecutor(), // Alias
"kimi-coding-apikey": new KimiExecutor(), // Alias

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Currently, "kimi-coding-apikey" is registered using new KimiExecutor(), which defaults the internal provider name to "kimi-coding". This means any provider-specific checks or configurations matching "kimi-coding-apikey" will not be correctly resolved within the executor instance.

Additionally, the "kimi-coding" provider itself is not registered to use KimiExecutor, meaning it will not benefit from the reasoning_content fix when thinking mode is active.

To resolve this, pass the correct provider name to the constructor for both "kimi-coding" and "kimi-coding-apikey", and register both to use KimiExecutor.

Suggested change
"kimi-coding-apikey": new KimiExecutor(), // Alias
"kimi-coding": new KimiExecutor("kimi-coding"),
"kimi-coding-apikey": new KimiExecutor("kimi-coding-apikey"),

Comment on lines +77 to +80
export class KimiExecutor extends DefaultExecutor {
constructor(provider = "kimi-coding") {
super(provider);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

According to the Repository Style Guide (Rule 9), tests must always be included when changing or adding production code in open-sse/. Please add unit tests for KimiExecutor under the tests/unit/ directory (per Rule 1) to verify the request transformation and reasoning_content injection logic.

References
  1. Rule 9: Always include tests when changing production code (src/, open-sse/, electron/, bin/). Rule 1: ALL unit tests, integration tests, ecosystem tests, or Vitest files MUST strictly be placed within the tests/ directory. (link)

@diegosouzapw
diegosouzapw merged commit 74ce4fd into diegosouzapw:main Jun 4, 2026
2 checks passed
diegosouzapw added a commit that referenced this pull request Jun 4, 2026
…ic-asset build fix

- #3131 Kiro Opus 4.8 catalog (thanks @artickc)
- #3132 Kimi thinking-mode reasoning_content fix (thanks @bypanghu)
- #3130 connectionId fallback + kilo call logging (thanks @androw)
- build: standalone static-asset path fix (white login screen after build-output reorg)
- contributors hall: +@artickc +@bypanghu +@androw (15 total)
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
… in kimi thing mode (diegosouzapw#3132)

* Fix the error of reasoning_content is missing in kimi thing mode

* fix: kimi always use default

* fix: add kimi-coding to use KimiExecutor
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…uzapw#3131/diegosouzapw#3132) + static-asset build fix

- diegosouzapw#3131 Kiro Opus 4.8 catalog (thanks @artickc)
- diegosouzapw#3132 Kimi thinking-mode reasoning_content fix (thanks @bypanghu)
- diegosouzapw#3130 connectionId fallback + kilo call logging (thanks @androw)
- build: standalone static-asset path fix (white login screen after build-output reorg)
- contributors hall: +@artickc +@bypanghu +@androw (15 total)
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
… in kimi thing mode (diegosouzapw#3132)

* Fix the error of reasoning_content is missing in kimi thing mode

* fix: kimi always use default

* fix: add kimi-coding to use KimiExecutor
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…uzapw#3131/diegosouzapw#3132) + static-asset build fix

- diegosouzapw#3131 Kiro Opus 4.8 catalog (thanks @artickc)
- diegosouzapw#3132 Kimi thinking-mode reasoning_content fix (thanks @bypanghu)
- diegosouzapw#3130 connectionId fallback + kilo call logging (thanks @androw)
- build: standalone static-asset path fix (white login screen after build-output reorg)
- contributors hall: +@artickc +@bypanghu +@androw (15 total)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… in kimi thing mode (diegosouzapw#3132)

* Fix the error of reasoning_content is missing in kimi thing mode

* fix: kimi always use default

* fix: add kimi-coding to use KimiExecutor
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…uzapw#3131/diegosouzapw#3132) + static-asset build fix

- diegosouzapw#3131 Kiro Opus 4.8 catalog (thanks @artickc)
- diegosouzapw#3132 Kimi thinking-mode reasoning_content fix (thanks @bypanghu)
- diegosouzapw#3130 connectionId fallback + kilo call logging (thanks @androw)
- build: standalone static-asset path fix (white login screen after build-output reorg)
- contributors hall: +@artickc +@bypanghu +@androw (15 total)
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