fix(cli): default step models to interleaved reasoning#11879
fix(cli): default step models to interleaved reasoning#11879purple-poi wants to merge 2 commits into
Conversation
|
@purple-poi is attempting to deploy a commit to the Kilo Code Team on Vercel. A member of the Team first needs to authorize it. |
| interleaved: model.interleaved ?? false, | ||
| interleaved: | ||
| model.interleaved ?? | ||
| ((model.provider?.npm ?? provider.npm ?? "@ai-sdk/openai-compatible") === "@ai-sdk/openai-compatible" && |
There was a problem hiding this comment.
WARNING: Kilo-specific logic re-inlined into a shared upstream file instead of the kilocode helper
The first commit on this PR extracted this check into defaultInterleaved() in src/kilocode/provider/provider.ts and called it from here behind a single kilocode_change import. The follow-up commit removed that helper and duplicated the same "openai-compatible npm + id contains step/deepseek" condition inline in two places in this shared upstream file (here and in the layer merge path below), with no kilocode_change marker.
Per the Fork Isolation Rule in packages/opencode/AGENTS.md, Kilo-specific logic touching a shared upstream file should live in src/kilocode/<same/path>.ts and be called from a single marked line, to minimize upstream merge-conflict surface. Restoring the extracted helper (as the first commit had it) would avoid duplicating this logic and keep the diff against upstream provider.ts minimal.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| interleaved: | ||
| model.interleaved ?? | ||
| ((model.provider?.npm ?? provider.npm ?? "@ai-sdk/openai-compatible") === "@ai-sdk/openai-compatible" && | ||
| model.id.includes("step") |
There was a problem hiding this comment.
SUGGESTION: Broad substring match on "step" may false-positive on unrelated model IDs
model.id.includes("step") (and the same check on apiID in the layer merge path below) matches "step" anywhere in the model ID, not just as a StepFun-style prefix/word. The PR's own test asserts this matches ids like "step 3.5 flash", "step.3.5.flash", "step_3_5_flash", which is intentional for StepFun naming, but it would also match any unrelated openai-compatible model whose id happens to contain the substring "step" (e.g. a future model literally named with "step" as part of an unrelated word). Consider anchoring the match more precisely to the StepFun provider/model naming convention (e.g. a prefix check or provider id check) to avoid unintended defaults for unrelated models.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| (!existingModel && apiNpm === "@ai-sdk/openai-compatible" && apiID.includes("deepseek") | ||
| ? { field: "reasoning_content" } | ||
| : false), | ||
| (existingModel?.capabilities.interleaved |
There was a problem hiding this comment.
SUGGESTION: Fallback now overrides a previously-computed false even when an existing model is present
Before this change, the fallback here was gated by !existingModel, so a model that already had a computed capabilities.interleaved (even an explicit false) was never re-evaluated. The new ternary treats existingModel?.capabilities.interleaved === false the same as "absent" and falls through to the deepseek/step fallback regardless of whether existingModel exists. This looks intentional here (it's what lets previously-cached false values for step models get upgraded on this fix), but it also means any existingModel whose interleaved capability was legitimately computed as false for a model matching the deepseek/step substring check will now get silently overridden to { field: "reasoning_content" } on the next merge. Worth double-checking this is the desired behavior for all existingModel cases, not just the step upgrade path this PR targets.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Also noting: the PR description states "A patch changeset is included for Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by claude-sonnet-5-20260630 · Input: 48 · Output: 21.3K · Cached: 1.4M Review guidance: REVIEW.md from base branch |
Issue
No linked issue. Exception: provider compatibility fix identified during local validation.
Context
Step/StepFun OpenAI-compatible models can return reasoning in reasoning_content, but Kilo was not
defaulting those models to interleaved reasoning metadata unless it was explicitly supplied.
Implementation
The provider model construction paths now use narrow inline conditions instead of a shared helper:
The models.dev catalog path defaults only OpenAI-compatible model IDs containing step to { field:
"reasoning_content" }.
The config model merge path keeps explicit model.interleaved first, preserves existing model
capabilities second, and only then applies the OpenAI-compatible fallback for deepseek or step.
A patch changeset is included for @kilocode/cli.
Screenshots / Video
N/A. No visual changes.
before after
━━━━━━━━ ━━━━━━━
N/A N/A
How to Test
Manual/local verification
Reviewer test steps
Run the focused provider tests:
cd packages/opencode
bun test ./test/kilocode/provider/stepfun-interleaved.test.ts
bun test ./test/provider/provider.test.ts
Confirm Step/StepFun OpenAI-compatible models default to:
{ "field": "reasoning_content" }
Checklist
Get in Touch
N/A