Repository navigation
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge This revision ( Files Reviewed (7 files)
Previous Review Summaries (10 snapshots, latest commit 835cb35)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 835cb35)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
Fix these issues in Kilo Cloud Previous review (commit f0b445b)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
Fix these issues in Kilo Cloud Previous review (commit 97db666)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit b132952)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit cbe4d89)Status: No Issues Found | Recommendation: Merge The previously reported fallback bypass is resolved by Files Reviewed (2 files)
Previous review (commit 6a42873)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 869f0ba)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit b93b74d)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit 5f2740e)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit ba4a8dc)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Reviewed by deepseek-flash-latest · Input: 59.8K · Output: 5.8K · Cached: 382.6K Review guidance: REVIEW.md from base branch |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba4a8dc686
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed both Codex review points in 5f2740e:
Verification: |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Merge-readiness check complete:
Ready for human review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f2740eddc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up: the Codex re-review findings were addressed in b93b74d (removed stray marker from the kilo-owned file; native Azure now passes |
|
Merge-readiness sweep complete: branch synced with current main (ffc0655, 0 behind), no merge conflicts, CI green on the latest head. Ready for human review. |
|
Review finding: the new GPT-5 gate is bypassed for custom model IDs whose heuristics return |
0892f59 to
869f0ba
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // kilocode_change start - `none`/`xhigh` are OpenAI rollout tiers only gpt-5-family | ||
| // models accept; non-OpenAI models on an OpenAI-compatible base URL (e.g. Grok | ||
| // via api.x.ai) reject them (issue #13342). See gpt5FamilyReasoningTiers. | ||
| if (!isFirstPartyOpenAI && !gpt5FamilyReasoningTiers(id)) return WIDELY_SUPPORTED_EFFORTS |
There was a problem hiding this comment.
WARNING: The gate is bypassed by the custom-provider fallback for non-gpt-5 ids
customProviderVariants() (packages/opencode/src/kilocode/provider/provider.ts) calls ProviderTransform.variants() and, when the heuristics return {}, falls back to FALLBACK_EFFORTS = ["none", "low", "medium", "high", "xhigh", "max"] for @ai-sdk/openai. Several ids are short-circuited by variants() before reaching this gate — qwen*, deepseek-*, minimax*, glm*, kimi*, big-pickle (the id.includes("qwen") block in variants()) — so a custom OpenAI-compatible provider using one of those ids still advertises none/xhigh, the same unsupported-tier shape as #13342. The existing packages/opencode/test/kilocode/provider-reasoning-options.test.ts cases assert exactly this fallback (["none","low","medium","high","xhigh","max"] for qwen-custom on @ai-sdk/openai). The new regression tests call ProviderTransform.variants() directly, so they do not cover this path. Consider gating the fallback for @ai-sdk/openai to the widely supported set unless the id is gpt-5 family, or documenting the exception with a regression test.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
Follow-up review at
With 1 and 2 fixed and 3 split out, this looks good to merge from my side. CI is green, and the branch is 6 commits behind |
github-actions
Bot
added
area:cli
|
Re-checked at
I'm happy to approve once those land and CI is green. |
d5cdfd2 to
b132952
Compare
| const efforts = openaiReasoningEfforts(model.api.id, model.release_date) | ||
| // kilocode_change start - providerID "openai" serves first-party OpenAI models, so a post-cutoff | ||
| // id like gpt-6 keeps rollout tiers via release-date logic instead of collapsing to low/medium/high | ||
| const efforts = openaiReasoningEfforts(model.api.id, model.release_date, model.providerID === "openai") |
There was a problem hiding this comment.
WARNING: The first-party bypass keys off providerID, but the endpoint is chosen by baseURL
This bypass treats providerID === "openai" as a first-party OpenAI transport. The actual endpoint, however, is determined by the provider's baseURL, which is user-overridable (documented in packages/kilo-docs/pages/code-with-ai/agents/custom-models.md). A config that keeps the openai provider id but points baseURL at an OpenAI-compatible endpoint (e.g. https://api.x.ai) yields providerID === "openai" with a non-OpenAI api.id such as grok-4.6, so the gate is skipped and none/xhigh are offered again - the exact #13342 failure. Consider keying first-party detection on the endpoint host (model.api.url) or on the model family, rather than on the provider id.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
github-actions
Bot
added
needs-review
and removed
needs-triage
b132952 to
97db666
Compare
|
Remaining before merge: the first-party exemption must respect the effective endpoint, not just |
github-actions
Bot
added
awaiting-contributor
| host = new URL(endpoint).hostname.toLowerCase() | ||
| } catch { | ||
| // Unexpanded config placeholders ({env:...}, ${...}) cannot be judged. | ||
| return true |
There was a problem hiding this comment.
SUGGESTION: Unparseable endpoint overrides still fall back to the first-party exemption
When the effective endpoint cannot be parsed as an absolute URL, this returns true (first-party), so the rollout-gated tiers are kept. resolveSDK expands ${VAR} references in baseURL from the environment after provider variants are built (packages/opencode/src/provider/provider.ts:1897), so a rerouted openai provider whose baseURL is an env-substituted host (e.g. https://${XAI_HOST}/v1) is judged first-party here and keeps offering none/xhigh for a non-gpt-5 id - the #13342 shape this PR fixes. If the intent is to avoid offering tiers the endpoint may reject, consider treating a present-but-unparseable override as non-first-party, and defaulting to first-party only when no override is configured at all.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
github-actions
Bot
added
needs-triage
|
Review finding addressed in 7a60c95: an unparseable non-empty endpoint override (e.g. |
…t-variants-non-openai
…t-variants-non-openai
…t-variants-non-openai
…t-variants-non-openai
…n-openai' into fix/reasoning-effort-variants-non-openai
…t-variants-non-openai
Fixes #13342
What
openaiReasoningEfforts()inpackages/opencode/src/provider/transform.tshanded out OpenAI's rollout-gated reasoning tiers —none(rolled out 2025-11-13) andxhigh(rolled out 2025-12-04) — to any model whose release date is new enough, regardless of whether it is actually a gpt-5-family model. Non-OpenAI models routed through@ai-sdk/openaion an OpenAI-compatible base URL (e.g. Grok viaapi.x.ai, the config shape the maintainer workaround in #13082 recommends) were therefore offered anxhighvariant they cannot honor, and selecting it failed withinvalid xai provider options(the AI SDK's zod validation rejectingreasoning_effort: "xhigh").Change
The gpt-5 family gate lives in a Kilo mirror (
src/kilocode/provider/transform.ts) per the fork-merge policy; the shared upstream file keeps a single markedgpt5FamilyReasoningTiers(id)call and a marked import. The family check accepts bare (gpt-5.5), slash-prefixed (openai/gpt-5.6), dot-prefixed (openai.gpt-5.5, Bedrock Mantle), and hyphen-prefixed deployment ids (azure-openai--gpt-5.4, SAP AI Core) while still excluding lookalikes likegpt-50/gpt-5o. Non-gpt-5 ids get only the widely-supportedlow/medium/highset — matching what the bundled@ai-sdk/xaiprovider already exposes for Grok.All existing gpt-5 / o-series / azure / mantle / SAP variant expectations are preserved (the shared
transform.test.tssuite passes, including theopenai.gpt-5.5Mantle case that still receivesnone/xhigh).Why this scope
Scanned the issue space before picking this: #13342 is unaddressed (no PR), and a search for related reasoning-effort PRs found no intersections. The same class of mis-offering exists in
openaiCompatibleReasoningEfforts()for@ai-sdk/gateway, but that path is deliberately tested to expose the full OpenAI set (Cloudflare's gateway translatesreasoning_effortupstream), so it is intentionally left unchanged to avoid a behavioral regression on the gateway's contract.Verification
test/kilocode/provider/transform.test.ts(Kilo-owned path), failing before the fix and passing after:grok-4.6via@ai-sdk/openaiwith a post-cutoff release date → onlylow/medium/high.low/medium/high.gpt-50/gpt-5oexcluded.openai.gpt-5.5(Mantle) andazure-openai--gpt-5.4(SAP) still receivenone/xhigh.bun test ./test/kilocode/provider/transform.test.ts→ 5 pass;bun test ./test/provider/transform.test.ts→ 422 pass.bun run typecheck(opencode) → clean.bun run script/check-opencode-annotations.ts --worktree→ clean.Note on CI
The JetBrains typecheck in the pre-push hook could not run in the sandbox (no Java 21 / SDKMAN available); it is unrelated to these opencode-only changes.
test/provider/provider.test.tsshows 3 pre-existing failures (defaultModel/opencode loaderresolve to the Kilo gatewaykilo-auto/efficientmodel instead of a direct provider) that are unrelated to this diff.