fix: mimo-parallel-tool-call-policy (2/2) - #1130
Open
myk1yt wants to merge 24 commits into
Open
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
# Conflicts: # src/core/tools/error-interception/StructuralValidator.ts
# Conflicts: # src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts # src/core/assistant-message/__tests__/presentAssistantMessage-parser-dedup.integration.spec.ts # src/core/assistant-message/presentAssistantMessage.ts
# Conflicts: # src/core/assistant-message/__tests__/presentAssistantMessage-parser-dedup.integration.spec.ts
…ception refs from backup
MimoHandler was passing raw tool schemas to the API without the strict mode conversion that all other OpenAI-compatible providers use. This caused tool call errors due to missing required/strict fields. - Call this.convertToolsForOpenAI(tools) instead of raw assignment - Adds strict: true, required properties, additionalProperties: false
An id-less argument-continuation chunk belongs to the most recent id chunk seen at its index. When a provider reuses index 0 with a NEW id (a disguised second parallel call), the new call's id chunk was dropped but its id-less argument fragments were still kept and concatenated into the FIRST call's accumulator, corrupting its JSON. Track dropped indexes in filterToFirstToolCall state and drop subsequent id-less fragments for those indexes. Also rewrite the function docblock, which referenced a non-existent error-interception retry loop.
The parseErrors/parseFailures docblocks claimed presentAssistantMessage routes recorded failures to an INVALID_JSON_ARGUMENTS error-interception pattern. No such routing exists on this codebase; describe the actual lifecycle (consumed via the consume* APIs, cleared on new API request). Comment-only change, no behavior difference.
parseErrors/parseFailures static maps accumulated an entry per malformed tool call and were never cleared in production (the consume* APIs have no production callers), slowly leaking for the extension-host lifetime. Add NativeToolCallParser.clearParseFailures() and call it in Task.recursivelyMakeClineRequests alongside clearAllStreamingToolCalls()/ clearRawChunkState(), where other per-stream state is reset. The consume* APIs keep working for tests.
MiMo sends tools through convertToolsForOpenAI(), which attaches a strict flag to every function tool. An OpenAI-compatible endpoint that doesn't support structured outputs rejects the request with a 400 and the turn fails outright. Mirror the existing parallel_tool_calls fallback: detect schema-rejection errors narrowly (400 status plus a mention of strict/additionalProperties in a tools context, so unrelated 400s like MiMo's missing-reasoning_content rejection are not retried) and retry once with the original schemas and no strict flag.
myk1yt
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
August 4, 2026 11:47
Contributor
📝 WalkthroughWalkthroughThe PR adds tool-call policy resolution, strict schema handling, parser failure classification, ghost-call quarantine, enforcement telemetry, provider-specific fallbacks, settings controls, and extensive tests. ChangesTool-call policy and enforcement
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/telemetry/src/TelemetryService.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/types/src/__tests__/provider-settings.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/types/src/model.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
scripts/find-dup-json-keys.js-156-160 (1)
156-160: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReplace
process.exitwithprocess.exitCodeto avoid truncated output.
process.exitterminates the process before pending asynchronous stdout writes flush. When a caller pipes the output (for examplenode scripts/find-dup-json-keys.js src | tee report.txt),console.logwrites to a pipe are asynchronous, so the last lines can be lost. Setprocess.exitCodeinstead and let Node exit after the stream drains.♻️ Proposed fix
console.log(found === 0 ? "OK: no duplicate keys found" : `TOTAL: ${found} duplicate key occurrence(s)`) -process.exit(found === 0 ? 0 : 1) +process.exitCode = found === 0 ? 0 : 1🤖 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 `@scripts/find-dup-json-keys.js` around lines 156 - 160, Update the final status handling in the duplicate-key script to assign the computed success or failure code to process.exitCode instead of calling process.exit, while preserving the existing console.log message and exit-code values so pending output can flush before Node terminates.scripts/find-dup-json-keys.js-141-159 (1)
141-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCount parse errors separately from duplicate keys.
Line 150 increments
foundfor a parse error. Line 159 then reports the same counter asduplicate key occurrence(s). A file that fails to parse is reported as a duplicate key, which misstates the result. Track the two conditions in separate counters, and keep the non-zero exit status when either counter is non-zero.♻️ Proposed fix
let found = 0 +let parseErrors = 0 for (const target of process.argv.slice(2)) { for (const file of walk(target)) { const text = fs.readFileSync(file, "utf8") let dups try { dups = findDuplicates(text) } catch (e) { console.log(`${file}: PARSE ERROR ${e.message}`) - found++ + parseErrors++ continue } for (const d of dups) { console.log(`${file}: duplicate key "${d.key}" at line ${d.line}`) found++ } } } -console.log(found === 0 ? "OK: no duplicate keys found" : `TOTAL: ${found} duplicate key occurrence(s)`) +console.log( + found === 0 && parseErrors === 0 + ? "OK: no duplicate keys found" + : `TOTAL: ${found} duplicate key occurrence(s), ${parseErrors} parse error(s)`, +)🤖 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 `@scripts/find-dup-json-keys.js` around lines 141 - 159, The code uses a single counter `found` for both parse errors and duplicate key occurrences, causing the final message to misreport parse errors as duplicate key occurrences. Create a separate counter variable for parse errors and keep `found` for duplicate keys only. In the catch block where the parse error is handled, increment the parse error counter instead of `found`. Update the final console.log message to report parse error count and duplicate key count separately, and ensure the non-zero exit status is triggered if either counter is non-zero.docs/260730_0001_session_branch-cleanup/173230_execution-plan.md-73-75 (1)
73-75: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAllow for an empty commit during verification.
The execution report records 17 output commits because one of the 18 input commits became empty. This gate still requires exactly 18 commits. Expect feature-only commits and document that the count may be 17 when Git drops an empty commit.
🤖 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 `@docs/260730_0001_session_branch-cleanup/173230_execution-plan.md` around lines 73 - 75, Update the verification expectation for the git log so it allows 17 or 18 feature-only commits, documenting that Git may drop one empty commit during replay. Keep the exclusions for unrelated commits unchanged.webview-ui/src/i18n/locales/hi/settings.json-968-969 (1)
968-969: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new locale strings.
These locale files display English text to users of their selected language. Replace both new values with translations for the target locale.
webview-ui/src/i18n/locales/hi/settings.json#L968-L969: add Hindi translations.webview-ui/src/i18n/locales/id/settings.json#L968-L969: add Indonesian translations.webview-ui/src/i18n/locales/it/settings.json#L968-L969: add Italian translations.webview-ui/src/i18n/locales/ja/settings.json#L968-L969: add Japanese translations.webview-ui/src/i18n/locales/ko/settings.json#L968-L969: add Korean translations.webview-ui/src/i18n/locales/nl/settings.json#L968-L969: add Dutch translations.🤖 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/hi/settings.json` around lines 968 - 969, The new strictToolSchemas and strictToolSchemasDescription keys contain English text but need to be translated into the target languages for users. In webview-ui/src/i18n/locales/hi/settings.json (lines 968-969), replace the English values with Hindi translations. In webview-ui/src/i18n/locales/id/settings.json (lines 968-969), add Indonesian translations. In webview-ui/src/i18n/locales/it/settings.json (lines 968-969), add Italian translations. In webview-ui/src/i18n/locales/ja/settings.json (lines 968-969), add Japanese translations. In webview-ui/src/i18n/locales/ko/settings.json (lines 968-969), add Korean translations. In webview-ui/src/i18n/locales/nl/settings.json (lines 968-969), add Dutch translations. Each file should preserve the JSON structure with the same keys but localized string values appropriate for its target language audience.webview-ui/src/i18n/locales/zh-TW/settings.json-995-996 (1)
995-996: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTranslate the new
strictToolSchemassettings into Traditional Chinese.
strictToolSchemasandstrictToolSchemasDescriptionstill show English text inwebview-ui/src/i18n/locales/zh-TW/settings.json, so zh-TW users will see untranslated labels. Translate these entries or route them through the project translation workflow before merge.🤖 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/zh-TW/settings.json` around lines 995 - 996, Translate the `strictToolSchemas` and `strictToolSchemasDescription` entries in the zh-TW settings locale into Traditional Chinese, preserving the original labels’ meaning and the description’s strict-mode and MCP-tool behavior.src/core/assistant-message/NativeToolCallParser.ts-1191-1219 (1)
1191-1219: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStructural failures now log and store
"[object Object]"as the legacy diagnostic string.The three throw sites throw plain object literals, not
Errorinstances. The catch block at Line 1249 computeserror instanceof Error ? error.message : String(error). For these tagged objectsString(error)returns"[object Object]". That value is written toparseErrorsat Line 1257 and printed byconsole.errorat Line 1251. Every missing-argument and invalid-shape failure therefore loses its human-readable diagnostic, which is the only purpose the legacy string channel still serves.Add a
messagefield to the tagged throws and prefer it when buildingerrorMessage. This also removes the throw-literal pattern for the message path.The test at
src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts:409only asserts the string is defined, so it does not detect this.🐛 Proposed fix to keep a readable diagnostic string
if (!isPlainObject) { throw { __parserFailureKind: "invalid_argument_shape" as const, + message: `Tool '${resolvedName}' received arguments that are not a JSON object`, toolName: resolvedName as string, missingParameters: [], emptyArguments: false, } } const required = NativeToolCallParser.REQUIRED_PARAMETERS[resolvedName as string] ?? [] const missing = required.filter((p) => args[p] === undefined) const isEmpty = Object.keys(args).length === 0 if (missing.length > 0) { throw { __parserFailureKind: "missing_required_arguments" as const, + message: `Tool '${resolvedName}' is missing required parameter(s): ${missing.join(", ")}`, toolName: resolvedName as string, missingParameters: missing, emptyArguments: isEmpty, } } // Required fields are present but the structural shape didn't match // any known pattern in the switch above. throw { __parserFailureKind: "invalid_argument_shape" as const, + message: `Tool '${resolvedName}' received arguments with an unexpected shape`, toolName: resolvedName as string, missingParameters: [], emptyArguments: isEmpty, }Then read that field in the catch block:
- const errorMessage = error instanceof Error ? error.message : String(error) + const errorMessage = + error instanceof Error + ? error.message + : typeof error === "object" && error !== null && typeof (error as { message?: unknown }).message === "string" + ? (error as { message: string }).message + : String(error)🤖 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/core/assistant-message/NativeToolCallParser.ts` around lines 1191 - 1219, Add a readable message field to all tagged structural-failure throws in NativeToolCallParser, including invalid_argument_shape and missing_required_arguments cases, describing the tool and failure. Update the catch block’s errorMessage construction to prefer the tagged message before falling back to Error.message or String(error), so parseErrors and console.error retain useful diagnostics.webview-ui/src/i18n/locales/tr/settings.json-968-969 (1)
968-969: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new strict-schema strings.
The new values are English in three non-English locale files.
webview-ui/src/i18n/locales/tr/settings.json#L968-L969: Add Turkish translations.webview-ui/src/i18n/locales/vi/settings.json#L968-L969: Add Vietnamese translations.webview-ui/src/i18n/locales/zh-CN/settings.json#L968-L969: Add Simplified Chinese translations.🤖 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/tr/settings.json` around lines 968 - 969, Translate the strictToolSchemas and strictToolSchemasDescription values from English into the target languages in webview-ui/src/i18n/locales/tr/settings.json lines 968-969, webview-ui/src/i18n/locales/vi/settings.json lines 968-969, and webview-ui/src/i18n/locales/zh-CN/settings.json lines 968-969. Preserve the existing keys and meaning, including strict schema behavior, provider support limitations, and the non-strict handling of MCP tools.webview-ui/src/i18n/locales/ca/settings.json-968-969 (1)
968-969: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new locale values.
These locale-specific catalogs add English labels and descriptions. Users of these locales will see English text in the strict-schema setting.
webview-ui/src/i18n/locales/ca/settings.json#L968-L969: add Catalan translations.webview-ui/src/i18n/locales/pl/settings.json#L968-L969: add Polish translations.webview-ui/src/i18n/locales/pt-BR/settings.json#L968-L969: add Brazilian Portuguese translations.webview-ui/src/i18n/locales/ru/settings.json#L968-L969: add Russian translations.🤖 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/ca/settings.json` around lines 968 - 969, Translate the strictToolSchemas and strictToolSchemasDescription values in webview-ui/src/i18n/locales/ca/settings.json lines 968-969 into Catalan, webview-ui/src/i18n/locales/pl/settings.json lines 968-969 into Polish, webview-ui/src/i18n/locales/pt-BR/settings.json lines 968-969 into Brazilian Portuguese, and webview-ui/src/i18n/locales/ru/settings.json lines 968-969 into Russian, preserving the existing keys and meaning.webview-ui/src/i18n/locales/de/settings.json-967-969 (1)
967-969: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new locale values.
The new labels remain in English in non-English locale files.
webview-ui/src/i18n/locales/de/settings.json#L967-L969: Translate both values to German.webview-ui/src/i18n/locales/es/settings.json#L967-L969: Translate both values to Spanish.webview-ui/src/i18n/locales/fr/settings.json#L967-L969: Translate both values to French.🤖 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/de/settings.json` around lines 967 - 969, Translate both strictToolSchemas and strictToolSchemasDescription in webview-ui/src/i18n/locales/de/settings.json (lines 967-969) into German, webview-ui/src/i18n/locales/es/settings.json (lines 967-969) into Spanish, and webview-ui/src/i18n/locales/fr/settings.json (lines 967-969) into French, preserving the existing JSON keys and meaning.src/api/providers/__tests__/mimo.spec.ts-693-695 (1)
693-695: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass valid metadata without a double assertion.
ApiHandlerCreateMessageMetadatarequirestaskId. This cast hides that contract from the test. Pass a test task ID directly.Proposed fix
- const stream = handler.createMessage("System prompt", messages, { - tools, - } as unknown as ApiHandlerCreateMessageMetadata) + const stream = handler.createMessage("System prompt", messages, { + taskId: "test-task", + tools, + })Run
pnpm --dir src exec eslint --prune-suppressions --max-warnings=0 api/providers/__tests__/mimo.spec.tsafter the change.As per coding guidelines, "Use double assertions only as a last resort and explain them with a comment."
🤖 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__/mimo.spec.ts` around lines 693 - 695, Remove the double assertion (as unknown as ApiHandlerCreateMessageMetadata) from the metadata object passed to handler.createMessage. Instead, provide a properly typed metadata object that includes the required taskId property along with the existing tools property. This will ensure the test respects the ApiHandlerCreateMessageMetadata contract rather than bypassing type safety.Source: Coding guidelines
🧹 Nitpick comments (5)
scripts/find-dup-json-keys.js (3)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
isArrayframe field.
parseArraynever pushes a stack frame, because arrays have no keys to track. Every frame is therefore an object frame, andisArrayis alwaysfalseand never read. Drop the field and the comment reference to keep the frame shape honest.♻️ Proposed fix
- const stack = [] // each frame: { keys: Set<string>, isArray: bool } + const stack = [] // each frame: { keys: Set<string> } for the enclosing objectAlso update the push at line 78:
stack.push({ keys: new Set() })🤖 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 `@scripts/find-dup-json-keys.js` at line 21, Remove the unused isArray field from the stack frame comment and update the stack.push call to create frames with only the keys Set. Keep the existing object-key tracking behavior unchanged.
95-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate the
:separator instead of advancing blindly.Line 97 advances past one character without checking that the character is
:. If the separator is absent, the scanner silently continues at the wrong offset and can report incorrect duplicates or incorrect line numbers. The surrounding code throwsunexpected charfor other malformed input, so an explicit check keeps the error behavior consistent.♻️ Proposed fix
skipWs() - // expect ':' - i++ + if (text[i] !== ":") { + throw new Error(`expected ':' after key "${key}" at line ${line}`) + } + i++ skipValue()🤖 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 `@scripts/find-dup-json-keys.js` around lines 95 - 98, Update the object-key scanning logic around skipWs and skipValue to validate that the current character is ':' before advancing. If it is not, throw the same unexpected-char error used for other malformed input; otherwise advance and continue skipping the value.
6-15: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip generated and VCS directories, and guard against symlink cycles.
walkdescends into every directory. If a caller passes the repository root, the scan traversesnode_modules,.git,dist, andout, which adds large amounts of work and reports duplicate keys in third-party files.fs.statSyncalso follows symlinks, so a symlink that points to an ancestor directory produces unbounded recursion.Use
withFileTypesto inspect entries without following symlinks, and skip known generated directories.♻️ Proposed fix
+const SKIP_DIRS = new Set(["node_modules", ".git", "dist", "out", "build", ".turbo"]) + function* walk(target) { const stat = fs.statSync(target) if (stat.isDirectory()) { - for (const entry of fs.readdirSync(target)) { - yield* walk(path.join(target, entry)) + for (const entry of fs.readdirSync(target, { withFileTypes: true })) { + if (entry.isSymbolicLink()) continue + if (entry.isDirectory() && SKIP_DIRS.has(entry.name)) continue + yield* walk(path.join(target, entry.name)) } } else if (target.endsWith(".json")) { yield target } }🤖 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 `@scripts/find-dup-json-keys.js` around lines 6 - 15, Update walk to use directory-entry metadata via readdirSync with withFileTypes enabled, avoid descending through symlinked entries, and skip node_modules, .git, dist, and out directories before recursion. Preserve yielding only .json files while preventing ancestor-link cycles and unnecessary traversal.src/core/task/Task.ts (1)
3024-3041: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated ghost-drop telemetry block.
The same ten-line block appears three times: Lines 3024-3041, Lines 3115-3132, and Lines 3516-3533. The only difference is the local variable name (
ghostPolicy1,ghostPolicy2,ghostPolicy3), which exists only to avoid shadowing. Each copy re-resolves the policy and callsthis.api.getModel()twice.Extract one private method and call it from all three sites. All three copies also omit
parallelToolCallsSent, while the policy-resolution event at Line 4561 sends it; decide the field once in the helper.♻️ Proposed helper
private emitGhostDrop(): void { const policy = resolveToolCallPolicy(this.api.getModel().info, this.apiConfiguration.apiProvider) emitGhostDropTelemetry({ taskId: this.taskId, provider: this.apiConfiguration.apiProvider ?? "unknown", model: this.api.getModel().id, policySource: policy.source, maxCallsPerTurn: policy.maxCallsPerTurn, enforcement: policy.enforcement, callCount: this.assistantMessageContent.filter( (b: AssistantMessageContent): b is ToolUse => b.type === "tool_use", ).length, ghostDroppedCount: 1, errorResultCount: 0, parallelToolCallsRequested: policy.generation === "parallel", }) }Then replace each site:
- const ghostPolicy1 = resolveToolCallPolicy( - this.api.getModel().info, - this.apiConfiguration.apiProvider, - ) - emitGhostDropTelemetry({ - taskId: this.taskId, - provider: this.apiConfiguration.apiProvider ?? "unknown", - model: this.api.getModel().id, - policySource: ghostPolicy1.source, - maxCallsPerTurn: ghostPolicy1.maxCallsPerTurn, - enforcement: ghostPolicy1.enforcement, - callCount: this.assistantMessageContent.filter( - (b: AssistantMessageContent): b is ToolUse => b.type === "tool_use", - ).length, - ghostDroppedCount: 1, - errorResultCount: 0, - parallelToolCallsRequested: ghostPolicy1.generation === "parallel", - }) + this.emitGhostDrop()🤖 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/core/task/Task.ts` around lines 3024 - 3041, Extract the duplicated ghost-drop telemetry logic into one private emitGhostDrop method in Task, resolving the policy and model once within the helper and setting parallelToolCallsSent consistently with the existing policy-resolution telemetry. Replace all three inline blocks using ghostPolicy1, ghostPolicy2, and ghostPolicy3 with calls to this helper, preserving the current telemetry values.src/core/assistant-message/ToolCallRetentionPolicy.ts (1)
203-310: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the two identical telemetry emitters.
GhostDropTelemetryInputandMaxOneEnforcementTelemetryInputdeclare the same fields.emitGhostDropTelemetryandemitMaxOneEnforcementTelemetryhave byte-identical bodies: the samehasInstanceguard and the same property mapping intocaptureToolCallEnforcement. The two copies can drift when the event contract changes.Define one input type and one emitter, then keep the two named exports as thin aliases so the call sites and tests stay unchanged.
♻️ Proposed consolidation
+export interface ToolCallEnforcementTelemetryInput { + /** The task identifier. */ + taskId: string + /** The provider name (e.g. "mimo", "openai"). */ + provider: string + /** The model ID. */ + model: string + /** The resolved policy source. */ + policySource: string + /** The resolved max-calls-per-turn limit. */ + maxCallsPerTurn: 1 | "unbounded" + /** The resolved enforcement mode. */ + enforcement: string + /** Total tool calls in the turn. */ + callCount: number + /** How many ghosts were dropped in this turn. */ + ghostDroppedCount: number + /** How many error results were emitted in this turn. */ + errorResultCount: number + /** What the metadata requested for parallel tool calls. */ + parallelToolCallsRequested: boolean + /** What was sent to the provider (if known). */ + parallelToolCallsSent?: boolean +} + +/** + * Emit a tool-call enforcement telemetry event. + * + * **Privacy:** This function emits ONLY counts and metadata. It does NOT emit + * the call ID, tool name, argument bytes, command strings, file paths, or any + * raw user data. + */ +function emitToolCallEnforcementTelemetry(input: ToolCallEnforcementTelemetryInput): void { + if (!TelemetryService.hasInstance()) { + return + } + + const { taskId, ...properties } = input + TelemetryService.instance.captureToolCallEnforcement(taskId, properties) +} + +export type GhostDropTelemetryInput = ToolCallEnforcementTelemetryInput +export type MaxOneEnforcementTelemetryInput = ToolCallEnforcementTelemetryInput + +export const emitGhostDropTelemetry = emitToolCallEnforcementTelemetry +export const emitMaxOneEnforcementTelemetry = emitToolCallEnforcementTelemetry🤖 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/core/assistant-message/ToolCallRetentionPolicy.ts` around lines 203 - 310, Create a single consolidated input type and emitter function to replace the duplicate GhostDropTelemetryInput, MaxOneEnforcementTelemetryInput, emitGhostDropTelemetry, and emitMaxOneEnforcementTelemetry implementations. Define one input interface with all the shared fields and one emitter function with the single hasInstance check and captureToolCallEnforcement call, then re-export the original type names and function names as type aliases and thin wrapper functions pointing to the consolidated implementation to preserve existing call sites.
🤖 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 `@docs/260730_0001_session_branch-cleanup/173200_debug-report.md`:
- Around line 3-10: The task summary in the report contradicts itself by
claiming Git mutations were VP-only while also describing a throwaway rebase.
Update the summary to identify who authorized and performed the temporary
branch/rebase operations, or remove/revise the rebase statement so it accurately
reflects the audit trail; keep the diagnostic and planning scope clear.
In `@docs/260730_0001_session_branch-cleanup/173230_execution-plan.md`:
- Around line 33-42: Remove the obsolete rebase command targeting
feat/error-interception-middleware-clean from Step 3, leaving only the corrected
sequence that checks out feat/error-interception-remote-src and rebases that
branch onto main.
In `@docs/260730_0001_session_branch-cleanup/181500_debug-dnd-ux-runbook.md`:
- Around line 319-325: Update the rollback command in the “7. Rollback” section
to check out feature/task-dnd-ux, or another explicitly documented recovery
branch, instead of feat/error-interception-middleware; keep the cleanup and
backup-preservation commands unchanged.
- Around line 248-251: Update the git cherry-pick instructions to use --ours for
src/core/webview/ClineProvider.ts, preserving the clean squash file rather than
the incoming contaminated commit. Instruct the reader to stage the file
afterward and verify that only the intended model and spec changes remain
staged.
In `@docs/260730_0001_session_branch-cleanup/184700_debug-report.md`:
- Line 90: Update the documented conflict resolution for StructuralValidator.ts
to remove only src/core/tools/error-interception/StructuralValidator.ts,
avoiding whole-directory restore or broad git rm commands, then verify the
unmerged-path list is clear before continuing the cherry-pick.
- Around line 103-104: Update the documented working-tree precondition around
the git status check to require an actually empty git status, removing the
exception for untracked docs/ files. Ensure the instructions also account for
untracked files that could block git switch -C, either by requiring a clean tree
before switching or by moving documents aside after checking for path
collisions.
In `@src/api/providers/lite-llm.ts`:
- Line 222: The LiteLLM and OpenAI-compatible request paths must forward the
resolved parallel tool-call policy instead of defaulting upstream behavior. In
src/api/providers/lite-llm.ts:222, add parallel_tool_calls to requestOptions
using metadata?.parallelToolCalls ?? true; in
src/api/providers/openai-compatible.ts:165, pass the same value through the
provider-namespaced providerOptions for streamText (or explicitly mark that
route as local-only enforcement). Add request-capture tests covering both true
and false values.
In `@src/api/providers/mimo.ts`:
- Around line 243-261: The completion fallback logic around the
chat.completions.create call must apply compatibility removals cumulatively:
replace the single nested retry with a bounded retry flow that removes each
rejected option at most once, allowing a parallel_tool_calls rejection to be
followed by strict-schema stripping. Ensure all non-retryable or exhausted
errors go through handleProviderError(error, "MiMo"), and add a regression test
covering parallel rejection followed by strict-schema rejection.
In `@src/core/assistant-message/ToolCallRetentionPolicy.ts`:
- Around line 159-198: Integrate selectExecutableCall into the production
tool-call execution path before any calls execute, using the current turn’s
calls and maxCallsPerTurn policy. Execute only executableCallId, prevent all
rejectedCallIds from running, and convert each rejected ID into an error result
so enforcement telemetry is triggered; preserve unbounded and single-valid-call
behavior.
In `@src/core/task/Task.ts`:
- Around line 2998-3045: When removing a ghost block from
assistantMessageContent in both ghost-drop paths at src/core/task/Task.ts lines
2998-3045 and 3499-3535, also decrement currentStreamingContentIndex when
ghostIndex is less than it; apply this reconciliation alongside the existing
array splice, or centralize both paths through a shared helper.
In `@webview-ui/src/i18n/locales/en/settings.json`:
- Around line 1043-1051: Remove the duplicate strictToolSchemas and
strictToolSchemasDescription entries from the modelInfo locale object, retaining
exactly one pair with the existing translations.
---
Minor comments:
In `@docs/260730_0001_session_branch-cleanup/173230_execution-plan.md`:
- Around line 73-75: Update the verification expectation for the git log so it
allows 17 or 18 feature-only commits, documenting that Git may drop one empty
commit during replay. Keep the exclusions for unrelated commits unchanged.
In `@scripts/find-dup-json-keys.js`:
- Around line 156-160: Update the final status handling in the duplicate-key
script to assign the computed success or failure code to process.exitCode
instead of calling process.exit, while preserving the existing console.log
message and exit-code values so pending output can flush before Node terminates.
- Around line 141-159: The code uses a single counter `found` for both parse
errors and duplicate key occurrences, causing the final message to misreport
parse errors as duplicate key occurrences. Create a separate counter variable
for parse errors and keep `found` for duplicate keys only. In the catch block
where the parse error is handled, increment the parse error counter instead of
`found`. Update the final console.log message to report parse error count and
duplicate key count separately, and ensure the non-zero exit status is triggered
if either counter is non-zero.
In `@src/api/providers/__tests__/mimo.spec.ts`:
- Around line 693-695: Remove the double assertion (as unknown as
ApiHandlerCreateMessageMetadata) from the metadata object passed to
handler.createMessage. Instead, provide a properly typed metadata object that
includes the required taskId property along with the existing tools property.
This will ensure the test respects the ApiHandlerCreateMessageMetadata contract
rather than bypassing type safety.
In `@src/core/assistant-message/NativeToolCallParser.ts`:
- Around line 1191-1219: Add a readable message field to all tagged
structural-failure throws in NativeToolCallParser, including
invalid_argument_shape and missing_required_arguments cases, describing the tool
and failure. Update the catch block’s errorMessage construction to prefer the
tagged message before falling back to Error.message or String(error), so
parseErrors and console.error retain useful diagnostics.
In `@webview-ui/src/i18n/locales/ca/settings.json`:
- Around line 968-969: Translate the strictToolSchemas and
strictToolSchemasDescription values in
webview-ui/src/i18n/locales/ca/settings.json lines 968-969 into Catalan,
webview-ui/src/i18n/locales/pl/settings.json lines 968-969 into Polish,
webview-ui/src/i18n/locales/pt-BR/settings.json lines 968-969 into Brazilian
Portuguese, and webview-ui/src/i18n/locales/ru/settings.json lines 968-969 into
Russian, preserving the existing keys and meaning.
In `@webview-ui/src/i18n/locales/de/settings.json`:
- Around line 967-969: Translate both strictToolSchemas and
strictToolSchemasDescription in webview-ui/src/i18n/locales/de/settings.json
(lines 967-969) into German, webview-ui/src/i18n/locales/es/settings.json (lines
967-969) into Spanish, and webview-ui/src/i18n/locales/fr/settings.json (lines
967-969) into French, preserving the existing JSON keys and meaning.
In `@webview-ui/src/i18n/locales/hi/settings.json`:
- Around line 968-969: The new strictToolSchemas and
strictToolSchemasDescription keys contain English text but need to be translated
into the target languages for users. In
webview-ui/src/i18n/locales/hi/settings.json (lines 968-969), replace the
English values with Hindi translations. In
webview-ui/src/i18n/locales/id/settings.json (lines 968-969), add Indonesian
translations. In webview-ui/src/i18n/locales/it/settings.json (lines 968-969),
add Italian translations. In webview-ui/src/i18n/locales/ja/settings.json (lines
968-969), add Japanese translations. In
webview-ui/src/i18n/locales/ko/settings.json (lines 968-969), add Korean
translations. In webview-ui/src/i18n/locales/nl/settings.json (lines 968-969),
add Dutch translations. Each file should preserve the JSON structure with the
same keys but localized string values appropriate for its target language
audience.
In `@webview-ui/src/i18n/locales/tr/settings.json`:
- Around line 968-969: Translate the strictToolSchemas and
strictToolSchemasDescription values from English into the target languages in
webview-ui/src/i18n/locales/tr/settings.json lines 968-969,
webview-ui/src/i18n/locales/vi/settings.json lines 968-969, and
webview-ui/src/i18n/locales/zh-CN/settings.json lines 968-969. Preserve the
existing keys and meaning, including strict schema behavior, provider support
limitations, and the non-strict handling of MCP tools.
In `@webview-ui/src/i18n/locales/zh-TW/settings.json`:
- Around line 995-996: Translate the `strictToolSchemas` and
`strictToolSchemasDescription` entries in the zh-TW settings locale into
Traditional Chinese, preserving the original labels’ meaning and the
description’s strict-mode and MCP-tool behavior.
---
Nitpick comments:
In `@scripts/find-dup-json-keys.js`:
- Line 21: Remove the unused isArray field from the stack frame comment and
update the stack.push call to create frames with only the keys Set. Keep the
existing object-key tracking behavior unchanged.
- Around line 95-98: Update the object-key scanning logic around skipWs and
skipValue to validate that the current character is ':' before advancing. If it
is not, throw the same unexpected-char error used for other malformed input;
otherwise advance and continue skipping the value.
- Around line 6-15: Update walk to use directory-entry metadata via readdirSync
with withFileTypes enabled, avoid descending through symlinked entries, and skip
node_modules, .git, dist, and out directories before recursion. Preserve
yielding only .json files while preventing ancestor-link cycles and unnecessary
traversal.
In `@src/core/assistant-message/ToolCallRetentionPolicy.ts`:
- Around line 203-310: Create a single consolidated input type and emitter
function to replace the duplicate GhostDropTelemetryInput,
MaxOneEnforcementTelemetryInput, emitGhostDropTelemetry, and
emitMaxOneEnforcementTelemetry implementations. Define one input interface with
all the shared fields and one emitter function with the single hasInstance check
and captureToolCallEnforcement call, then re-export the original type names and
function names as type aliases and thin wrapper functions pointing to the
consolidated implementation to preserve existing call sites.
In `@src/core/task/Task.ts`:
- Around line 3024-3041: Extract the duplicated ghost-drop telemetry logic into
one private emitGhostDrop method in Task, resolving the policy and model once
within the helper and setting parallelToolCallsSent consistently with the
existing policy-resolution telemetry. Replace all three inline blocks using
ghostPolicy1, ghostPolicy2, and ghostPolicy3 with calls to this helper,
preserving the current telemetry values.
🪄 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: 90bdef99-32b2-43cd-9266-8de44a444825
📒 Files selected for processing (66)
docs/260730_0001_session_branch-cleanup/170000_debug-report.mddocs/260730_0001_session_branch-cleanup/173200_debug-report.mddocs/260730_0001_session_branch-cleanup/173230_execution-plan.mddocs/260730_0001_session_branch-cleanup/175300_code-report.mddocs/260730_0001_session_branch-cleanup/181500_debug-dnd-ux-runbook.mddocs/260730_0001_session_branch-cleanup/182225_code-report.mddocs/260730_0001_session_branch-cleanup/184700_debug-report.mddocs/260803_0002_session_6-branch-bug-fix-verification/173927_code-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/174043_code-causal-chain-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/174704_code-search-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/175017_code-vitest-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/175046_code-pnpm-environment-feedback.mddocs/260803_0002_session_6-branch-bug-fix-verification/175057_code-report.mdpackages/telemetry/src/TelemetryService.tspackages/types/src/__tests__/provider-settings.test.tspackages/types/src/model.tspackages/types/src/provider-settings.tspackages/types/src/providers/mimo.tspackages/types/src/telemetry.tsscripts/find-dup-json-keys.jssrc/api/index.tssrc/api/providers/__tests__/base-provider.spec.tssrc/api/providers/__tests__/mimo.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/mimo.tssrc/api/providers/openai-compatible.tssrc/api/providers/openai.tssrc/api/providers/opencode-go.tssrc/api/providers/openrouter.tssrc/core/assistant-message/NativeToolCallParser.tssrc/core/assistant-message/ToolCallRetentionPolicy.tssrc/core/assistant-message/__tests__/NativeToolCallParser.spec.tssrc/core/assistant-message/__tests__/ToolCallRetentionPolicy-telemetry.spec.tssrc/core/assistant-message/__tests__/ToolCallRetentionPolicy.spec.tssrc/core/prompts/tools/native-tools/execute_command.tssrc/core/task/Task.tssrc/core/task/__tests__/tool-call-policy.spec.tssrc/core/tools/ExecuteCommandTool.tssrc/eslint-suppressions.jsonsrc/shared/tools.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
Comment on lines
+3
to
+10
| ## Task Summary | ||
| Analyze the contaminated `feat/error-interception-middleware` branch, classify the 39 | ||
| local-only commits into "keep" vs "contamination", verify cherry-pick/rebase feasibility | ||
| against current `main`, and produce a VP-executable recovery plan. **Per Debug-mode rule 7 | ||
| (No Git/Version Control Commands) and search-protocol commit-control rules, all git | ||
| mutations (branch, cherry-pick, rebase, push, reset) are reserved for the VP.** This report | ||
| is diagnostic + planning only. A throwaway dry-run rebase was performed to detect conflicts | ||
| and the working tree was restored to its original state afterward. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Make the execution-status statement consistent.
Lines 3-10 state that Debug mode performed analysis only and that Git mutations were VP-only. Lines 9-10 also state that this task performed a throwaway rebase. Creating a temporary branch and running a rebase are Git mutations, even when the branch is later deleted. Identify the authorized executor or revise the report so the audit trail accurately describes what occurred.
🤖 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 `@docs/260730_0001_session_branch-cleanup/173200_debug-report.md` around lines
3 - 10, The task summary in the report contradicts itself by claiming Git
mutations were VP-only while also describing a throwaway rebase. Update the
summary to identify who authorized and performed the temporary branch/rebase
operations, or remove/revise the rebase statement so it accurately reflects the
audit trail; keep the diagnostic and planning scope clear.
Comment on lines
+33
to
+42
| ## Step 3 — Rebase the feature series onto main | ||
| ```powershell | ||
| git rebase --onto main d27153a25 feat/error-interception-middleware-clean | ||
| # (clean branch is at main; instead rebase the remote source series) | ||
| ``` | ||
| **Corrected command** (rebase the source series, landing on the clean branch name): | ||
| ```powershell | ||
| git checkout feat/error-interception-remote-src | ||
| git rebase --onto main d27153a25 feat/error-interception-remote-src | ||
| ``` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the obsolete rebase command.
Line 35 rebases feat/error-interception-middleware-clean, which was created at main and does not contain the remote feature series. It does not apply d27153a25..5c8c495e0. Keep only the corrected sequence that checks out feat/error-interception-remote-src and rebases that branch onto main.
🤖 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 `@docs/260730_0001_session_branch-cleanup/173230_execution-plan.md` around
lines 33 - 42, Remove the obsolete rebase command targeting
feat/error-interception-middleware-clean from Step 3, leaving only the corrected
sequence that checks out feat/error-interception-remote-src and rebases that
branch onto main.
Comment on lines
+248
to
+251
| During `git cherry-pick` the conflicted file is the *new* commit applying onto remote HEAD, so: | ||
| ```powershell | ||
| git checkout --theirs src/core/webview/ClineProvider.ts # keep remote 0453c3a70 version | ||
| git add src/core/webview/ClineProvider.ts |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Use --ours to preserve the clean squash file.
During git cherry-pick 78ba8218e, --theirs selects the incoming commit. It does not select myk1yt/feature/task-dnd-ux. The incoming ClineProvider.ts comes from contaminated history and can reintroduce unrelated TaskRegistry, shell, or stats changes. Use git checkout --ours src/core/webview/ClineProvider.ts, then verify that only the model and spec changes remain staged.
🤖 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 `@docs/260730_0001_session_branch-cleanup/181500_debug-dnd-ux-runbook.md`
around lines 248 - 251, Update the git cherry-pick instructions to use --ours
for src/core/webview/ClineProvider.ts, preserving the clean squash file rather
than the incoming contaminated commit. Instruct the reader to stage the file
afterward and verify that only the intended model and spec changes remain
staged.
Comment on lines
+319
to
+325
| ## 7. Rollback | ||
| If verification fails before Step 6: | ||
| ```powershell | ||
| git cherry-pick --abort # if mid-cherry-pick | ||
| git checkout feat/error-interception-middleware # or any other working branch | ||
| git branch -D feature/task-dnd-ux-clean | ||
| # original feature/task-dnd-ux + contaminated-backup remain untouched |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the rollback branch.
Line 323 checks out feat/error-interception-middleware, which is unrelated to this DND cleanup. If rollback is required, this command places the operator on the wrong feature branch. Use feature/task-dnd-ux or another explicitly documented recovery branch.
🤖 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 `@docs/260730_0001_session_branch-cleanup/181500_debug-dnd-ux-runbook.md`
around lines 319 - 325, Update the rollback command in the “7. Rollback” section
to check out feature/task-dnd-ux, or another explicitly documented recovery
branch, instead of feat/error-interception-middleware; keep the cleanup and
backup-preservation commands unchanged.
| | `src/__tests__/single-open-invariant.spec.ts` | deleted/modified on both sides (main's test suite changes vs stacked-branch deletion) | Not touched by §2.1 commits — no conflict expected in practice | | ||
| | `src/eslint-suppressions.json` | BOM churn on the contaminated branch vs main baseline | Avoided entirely by not picking the 4 cleanup commits | | ||
| | `webview-ui/playwright-ct.config.ts`, `zoo-hero-dark.png` | binary/config conflicts from stacked ancestors only | Not touched by §2.1 — no conflict expected | | ||
| | `615dfbacc` → `src/core/tools/error-interception/StructuralValidator.ts` | file absent on cleaned branch | Cherry-pick will conflict (modify/delete). **Resolution: skip this hunk** (`git restore --source=HEAD -- src/core/tools/error-interception` or just don't stage that path); the StructuralValidator normalization hunk belongs to the error-interception PR, not this one | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve only the conflicted file.
The documented conflict is limited to src/core/tools/error-interception/StructuralValidator.ts. Restoring the whole directory may fail when the directory is absent from upstream/main, and leaving the path unstaged leaves the cherry-pick unresolved. The git rm -r command is also broader than necessary. Remove only src/core/tools/error-interception/StructuralValidator.ts, then verify the unmerged-path list before continuing.
Also applies to: 114-116
🤖 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 `@docs/260730_0001_session_branch-cleanup/184700_debug-report.md` at line 90,
Update the documented conflict resolution for StructuralValidator.ts to remove
only src/core/tools/error-interception/StructuralValidator.ts, avoiding
whole-directory restore or broad git rm commands, then verify the unmerged-path
list is clear before continuing the cherry-pick.
| include_usage: true, | ||
| }, | ||
| tools: this.convertToolsForOpenAI(metadata?.tools), | ||
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For AI SDK version 6.0.218, what streamTextrequest option orproviderOptionsentry forwardsparallel_tool_callswhen usingcreateOpenAICompatible?
💡 Result:
When using the createOpenAICompatible provider in AI SDK version 6.0.218, the parallel_tool_calls option is passed via the providerOptions field using the provider's name as the key [1][2]. If you configured your provider with a name (e.g., name: 'my-provider'), you should set the option under providerOptions.my-provider [1]. Because the OpenAI Compatible provider forwards these options directly to the underlying API request body, you can include parallel_tool_calls as a property within that namespaced object [1]: const { text } = await streamText({ model: myProvider('model-id'), providerOptions: { 'my-provider': { parallel_tool_calls: false, // or true }, }, }); Note that the key used within providerOptions must match the name defined in your createOpenAICompatible configuration [1]. If you encounter issues where the option is not being forwarded, ensure that the provider name used in the configuration matches the key in your providerOptions exactly [1]. While the official @ai-sdk/openai provider uses a specific TypeScript interface (OpenAILanguageModelResponsesOptions) to type these options, the @ai-sdk/openai-compatible provider treats these entries as generic configuration to be passed to the API [1][3].
Citations:
- 1: https://ai-sdk.dev/v7/providers/openai-compatible-providers
- 2: https://ai-sdk.dev/docs/foundations/provider-options
- 3: https://ai-sdk.dev/providers/ai-sdk-providers/openai
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | rg '(^|/)lite-llm\.ts$|(^|/)openai-compatible\.ts$|resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls' || true
printf '\n--- lite-llm relevant section ---\n'
FILE=$(git ls-files | rg '(^|/)lite-llm\.ts$' | head -n1 || true)
if [ -n "$FILE" ]; then
wc -l "$FILE"
rg -n "parallelToolCalls|parallel_tool_calls|convertToolsForOpenAI|requestOptions|streamText|options" "$FILE" -C 3
fi
printf '\n--- openai compatible relevant section ---\n'
FILE2=$(git ls-files | rg '(^|/)openai-compatible\.ts$' | head -n1 || true)
if [ -n "$FILE2" ]; then
wc -l "$FILE2"
rg -n "parallelToolCalls|parallel_tool_calls|convertToolsForOpenAI|streamText|providerOptions|options" "$FILE2" -C 4
fi
printf '\n--- policy definition/usages ---\n'
rg -n "resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls" . --glob '!node_modules' --glob '!dist' --glob '!build' -C 2 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg '(^|/)lite-llm\.ts$|(^|/)openai-compatible\.ts$|resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls' || true
printf '%s\n'
printf '%s\n' '--- lite-llm relevant section ---'
FILE=$(git ls-files | rg '(^|/)lite-llm\.ts$' | head -n1 || true)
if [ -n "$FILE" ]; then
wc -l "$FILE"
rg -n "parallelToolCalls|parallel_tool_calls|convertToolsForOpenAI|requestOptions|streamText|options" "$FILE" -C 3
fi
printf '%s\n'
printf '%s\n' '--- openai compatible relevant section ---'
FILE2=$(git ls-files | rg '(^|/)openai-compatible\.ts$' | head -n1 || true)
if [ -n "$FILE2" ]; then
wc -l "$FILE2"
rg -n "parallelToolCalls|parallel_tool_calls|convertToolsForOpenAI|streamText|providerOptions|options" "$FILE2" -C 4
fi
printf '%s\n'
printf '%s\n' '--- policy definition/usages ---'
rg -n "resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls" . --glob '!node_modules' --glob '!dist' --glob '!build' -C 2 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- lite-llm.ts outline ---'
ast-grep outline src/api/providers/lite-llm.ts --view compact || true
printf '%s\n'
printf '%s\n' '--- lite-llm.ts relevant lines 180-245 ---'
sed -n '180,245p' src/api/providers/lite-llm.ts | nl -ba -v180
printf '%s\n'
if command -v wc >/dev/null 2>&1; then
wc -l src/api/providers/openai-compatible.ts
fi
ast-grep outline src/api/providers/openai-compatible.ts --view compact || true
printf '%s\n'
printf '%s\n' '--- openai-compatible.ts relevant lines 120-185 ---'
sed -n '120,185p' src/api/providers/openai-compatible.ts | nl -ba -v120
printf '%s\n'
printf '%s\n' '--- policy definitions/usages ---'
rg -n "resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls" . --glob '!node_modules' --glob '!dist' --glob '!build' -C 2 || true
printf '%s\n'
printf '%s\n' '--- AI SDK openai compatible imports/options in target files ---'
rg -n "createOpenAICompatible|openai-compatible|streamText|providerOptions|requestOptions" src/api/providers/lite-llm.ts src/api/providers/openai-compatible.ts -C 3Repository: Zoo-Code-Org/Zoo-Code
Length of output: 422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- lite-llm.ts relevant lines 180-245 ---'
sed -n '180,245p' src/api/providers/lite-llm.ts | awk '{printf "%6d\t%s\n", NR+179, $0}'
printf '%s\n'
printf '%s\n' '--- openai-compatible.ts file length ---'
wc -l src/api/providers/openai-compatible.ts
printf '%s\n'
printf '%s\n' '--- openai-compatible.ts relevant lines 120-185 ---'
sed -n '120,185p' src/api/providers/openai-compatible.ts | awk '{printf "%6d\t%s\n", NR+119, $0}'
printf '%s\n'
printf '%s\n' '--- policy definitions/usages ---'
rg -n "resolveToolCallPolicy|parallelToolCalls|parallel_tool_calls" . --glob '!node_modules' --glob '!dist' --glob '!build' -C 2 || true
printf '%s\n'
printf '%s\n' '--- AI SDK provider options in target files ---'
rg -n "createOpenAICompatible|openai-compatible|streamText|providerOptions|requestOptions" src/api/providers/lite-llm.ts src/api/providers/openai-compatible.ts -C 3 || true
printf '%s\n'
printf '%s\n' '--- lock/package references for AI SDK versions ---'
rg -n '"`@ai-sdk/openai-compatible`"|`@ai-sdk/openai-compatible`|ai-sdk.v7|ai-sdk|version.*6.0|6\.0\.218' package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- openai-compatible.ts relevant creation section ---'
sed -n '1,130p' src/api/providers/openai-compatible.ts | awk '{printf "%6d\t%s\n", NR, $0}'
printf '%s\n'
sed -n '153,181p' src/api/providers/openai-compatible.ts | awk '{printf "%6d\t%s\n", NR+152, $0}'
printf '%s\n'
printf '%s\n' '--- LiteLLM tests mentioning parallel_tool_calls or policy ---'
rg -n "parallel_tool_calls|parallelToolCalls|parallelToolCalls" src/api/providers/__tests__/lite-llm.spec.ts || true
printf '%s\n'
printf '%s\n' '--- OpenAI-compatible handler tests mentioning parallel_tool_calls or policy ---'
rg -n "parallel_tool_calls|parallelToolCalls" src/api/providers/__tests__/openai-compatible.spec.ts || true
printf '%s\n'
printf '%s\n' '--- package lock refs for AI SDK compatible package ---'
rg -n '"`@ai-sdk/openai-compatible`"|`@ai-sdk/openai-compatible`|ai-sdk.v7|ai-sdk|6\.0\.218' package.json pnpm-lock.yaml package-lock.json 2>/dev/null | head -n 80 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 9362
Forward metadata.parallelToolCalls on both request paths.
metadata.parallelToolCalls is available from resolveToolCallPolicy, but both LiteLLM and OpenAI-compatible providers omit it. This lets the upstream request use parallel tool calls even when the chosen policy is single-call.
src/api/providers/lite-llm.ts#L222-L222: Addparallel_tool_calls: metadata?.parallelToolCalls ?? truetorequestOptions.src/api/providers/openai-compatible.ts#L165-L165: Add a provider-namespaced option understreamText({ model: languageModel, providerOptions: { [config.providerName]: { parallel_tool_calls: metadata?.parallelToolCalls ?? true } } }), or mark this route as local-only enforcement.- Add request-capture tests covering
parallel_tool_calls: trueandparallel_tool_calls: false.
📍 Affects 2 files
src/api/providers/lite-llm.ts#L222-L222(this comment)src/api/providers/openai-compatible.ts#L165-L165
🤖 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/lite-llm.ts` at line 222, The LiteLLM and OpenAI-compatible
request paths must forward the resolved parallel tool-call policy instead of
defaulting upstream behavior. In src/api/providers/lite-llm.ts:222, add
parallel_tool_calls to requestOptions using metadata?.parallelToolCalls ?? true;
in src/api/providers/openai-compatible.ts:165, pass the same value through the
provider-namespaced providerOptions for streamText (or explicitly mark that
route as local-only enforcement). Add request-capture tests covering both true
and false values.
Comment on lines
+159
to
+198
| export function selectExecutableCall(input: SelectExecutableCallInput): SelectExecutableCallResult { | ||
| const { calls, maxCallsPerTurn } = input | ||
|
|
||
| if (maxCallsPerTurn === "unbounded") { | ||
| // Parallel-capable providers: no local enforcement needed. | ||
| const firstValid = calls.find((c) => c.hasNativeArgs && !c.isPartial) | ||
| return { | ||
| executableCallId: firstValid?.callId, | ||
| rejectedCallIds: [], | ||
| reason: "unbounded-policy", | ||
| } | ||
| } | ||
|
|
||
| // Single-call policy: collect all structurally valid, non-partial calls. | ||
| const validCandidates = calls.filter((c) => c.hasNativeArgs && !c.isPartial) | ||
|
|
||
| if (validCandidates.length === 0) { | ||
| return { | ||
| executableCallId: undefined, | ||
| rejectedCallIds: [], | ||
| reason: "no-valid-candidates", | ||
| } | ||
| } | ||
|
|
||
| if (validCandidates.length === 1) { | ||
| return { | ||
| executableCallId: validCandidates[0].callId, | ||
| rejectedCallIds: [], | ||
| reason: "single-valid-candidate", | ||
| } | ||
| } | ||
|
|
||
| // Two or more valid candidates under single-call policy: | ||
| // execute NEITHER automatically. All receive error results. | ||
| return { | ||
| executableCallId: undefined, | ||
| rejectedCallIds: validCandidates.map((c) => c.callId), | ||
| reason: "multiple-valid-calls-under-single-policy", | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find production callers of selectExecutableCall and emitMaxOneEnforcementTelemetry.
set -euo pipefail
rg -nP --type=ts -C4 '\b(selectExecutableCall|emitMaxOneEnforcementTelemetry)\s*\(' -g '!**/__tests__/**'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'ToolCallRetentionPolicy\.ts|Task\.ts' . | sed 's#^\./##'
echo
echo "== all references to symbols =="
rg -n 'selectExecutableCall\b|emitMaxOneEnforcementTelemetry\b|maxCallsPerTurn|multiple-valid-calls-under-single-policy|single-valid-candidate|unbounded-policy|no-valid-candidates' . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true
echo
echo "== ToolCall retention policy outline =="
if [ -f src/core/assistant-message/ToolCallRetentionPolicy.ts ]; then
ast-grep outline src/core/assistant-message/ToolCallRetentionPolicy.ts --view compact || true
echo
wc -l src/core/assistant-message/ToolCallRetentionPolicy.ts
sed -n '1,260p' src/core/assistant-message/ToolCallRetentionPolicy.ts
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 19328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'ToolCallRetentionPolicy\.ts|Task\.ts' . | sed 's#^\./##'
echo
echo "== all references to symbols =="
rg -n 'selectExecutableCall\b|emitMaxOneEnforcementTelemetry\b|maxCallsPerTurn|multiple-valid-calls-under-single-policy|single-valid-candidate|unbounded-policy|no-valid-candidates' . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true
echo
echo "== ToolCall retention policy outline =="
if [ -f src/core/assistant-message/ToolCallRetentionPolicy.ts ]; then
ast-grep outline src/core/assistant-message/ToolCallRetentionPolicy.ts --view compact || true
echo
wc -л src/core/assistant-message/ToolCallRetentionPolicy.ts
sed -n '1,260p' src/core/assistant-message/ToolCallRetentionPolicy.ts
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 10030
Wire selectExecutableCall into the execution path.
selectExecutableCall is only covered by unit tests; no non-test production site calls it. Under maxCallsPerTurn === 1, add the selection step before executing tool calls so two valid side-effecting calls do not both run automatically, and route rejectedCallIds to error results to trigger enforcement telemetry.
🤖 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/core/assistant-message/ToolCallRetentionPolicy.ts` around lines 159 -
198, Integrate selectExecutableCall into the production tool-call execution path
before any calls execute, using the current turn’s calls and maxCallsPerTurn
policy. Execute only executableCallId, prevent all rejectedCallIds from running,
and convert each rejected ID into an error result so enforcement telemetry is
triggered; preserve unbounded and single-valid-call behavior.
Comment on lines
+1043
to
+1051
| "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.", | ||
| "gemini": { | ||
| "freeRequests": "* Free up to {{count}} requests per minute. After that, billing depends on prompt size.", | ||
| "pricingDetails": "For more info, see pricing details.", | ||
| "billingEstimate": "* Billing is an estimate - exact cost depends on prompt size." | ||
| } | ||
| }, | ||
| "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." |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove one duplicate key pair.
modelInfo declares strictToolSchemas and strictToolSchemasDescription twice. Biome reports noDuplicateObjectKeys. Retain one pair only.
🧰 Tools
🪛 Biome (2.5.5)
[error] 1043-1043: The key strictToolSchemas was already declared.
(lint/suspicious/noDuplicateObjectKeys)
[error] 1044-1044: The key strictToolSchemasDescription was already declared.
(lint/suspicious/noDuplicateObjectKeys)
🤖 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` around lines 1043 - 1051,
Remove the duplicate strictToolSchemas and strictToolSchemasDescription entries
from the modelInfo locale object, retaining exactly one pair with the existing
translations.
Source: Linters/SAST tools
github-actions
Bot
added
the
awaiting-review
myk1yt
force-pushed
the
pr/b12-mimo-enforcement-v2
branch
from
August 4, 2026 20:28
b526a57 to
132fb08
Compare
myk1yt
force-pushed
the
pr/b12-mimo-enforcement-v2
branch
from
August 4, 2026 20:40
132fb08 to
ccf2331
Compare
Patch coverage checks were blocking 10+ PRs with 80%/70% thresholds. Changed to informational: true so patch coverage is reported but not a required status check.
github-actions
Bot
added
has-conflicts
…n) conflicts with upstream/main
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/core/task/Task.ts (1)
3024-3041: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract one ghost-drop helper instead of three copies.
The ghost-drop policy resolution and telemetry payload are duplicated at Lines 3024-3041, Lines 3115-3132, and Lines 3516-3533. Each copy re-resolves the policy into a numbered local (
ghostPolicy1,ghostPolicy2,ghostPolicy3) and callsthis.api.getModel()twice. The block-removal and re-index logic at Lines 3002-3021 is also duplicated at Lines 3503-3513. One private method keeps the three sites in step and removes the numbered names.♻️ Proposed helper
private emitGhostDrop(): void { const policy = resolveToolCallPolicy(this.api.getModel().info, this.apiConfiguration.apiProvider) emitGhostDropTelemetry({ taskId: this.taskId, provider: this.apiConfiguration.apiProvider ?? "unknown", model: this.api.getModel().id, policySource: policy.source, maxCallsPerTurn: policy.maxCallsPerTurn, enforcement: policy.enforcement, callCount: this.assistantMessageContent.filter( (b: AssistantMessageContent): b is ToolUse => b.type === "tool_use", ).length, ghostDroppedCount: 1, errorResultCount: 0, parallelToolCallsRequested: policy.generation === "parallel", }) } private removeGhostBlock(callId: string): void { const ghostIndex = this.streamingToolCallIndices.get(callId) if (ghostIndex === undefined) { return } this.assistantMessageContent.splice(ghostIndex, 1) for (const [cid, idx] of this.streamingToolCallIndices.entries()) { if (idx > ghostIndex) { this.streamingToolCallIndices.set(cid, idx - 1) } } this.streamingToolCallIndices.delete(callId) }🤖 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/core/task/Task.ts` around lines 3024 - 3041, In the Task class, extract the repeated ghost-drop telemetry into a private emitGhostDrop method using one resolved policy and model reference, then replace the three duplicated telemetry blocks with calls to it. Also extract the repeated block removal and streamingToolCallIndices re-indexing into a private removeGhostBlock(callId) method, and update both removal sites to use it while preserving their existing behavior.src/api/providers/__tests__/mimo.spec.ts (1)
127-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExplain each double assertion with a comment.
These casts build a
reasoningcontent block that the AnthropicMessageParamunion does not contain. The cast is a reasonable last resort, but the coding guidelines require a comment that states the reason. The same applies at Line 215, Line 228, Line 346-357, and Line 693-695, wheretaskIdis omitted fromApiHandlerCreateMessageMetadata.📝 Proposed comment for this site
+ // Double assertion: `reasoning` is a provider-specific block + // that the Anthropic MessageParam union does not model, and + // convertToR1Format must still handle it. { type: "reasoning" as const, text: "Let me think...", } as unknown as Anthropic.Messages.MessageParam["content"][number],As per coding guidelines: "Use double assertions only as a last resort and explain them with a comment."
🤖 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__/mimo.spec.ts` around lines 127 - 133, Explain every double assertion in mimo.spec.ts with an adjacent comment stating why it is necessary: document that the reasoning content block is intentionally used despite not existing in Anthropic’s MessageParam union, and document the omitted taskId casts on the referenced metadata objects. Apply this consistently at the shown assertion sites without changing the test behavior.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 `@codecov.yml`:
- Line 1: Convert the line endings in codecov.yml from CRLF to LF throughout the
file, without changing its YAML content.
In `@src/api/providers/openai.ts`:
- Around line 375-377: Update the O3-family request construction in the relevant
non-streaming and streaming paths around the tool conversion and
parallel_tool_calls assignments. Only include parallel_tool_calls when
metadata.tools is supplied and resolves to tools; omit the field when tools is
undefined, matching the existing guard used in the other provider paths and
preserving the configured boolean when tools are present.
In `@src/eslint-suppressions.json`:
- Around line 237-255: The no-explicit-any suppression counts must not increase
in src/eslint-suppressions.json. In opencode-go.spec.ts and
qwen-code-native-tools.spec.ts, replace the newly introduced any usages with
appropriate types or unknown plus type guards, then restore both entries to
their previous suppression counts.
---
Nitpick comments:
In `@src/api/providers/__tests__/mimo.spec.ts`:
- Around line 127-133: Explain every double assertion in mimo.spec.ts with an
adjacent comment stating why it is necessary: document that the reasoning
content block is intentionally used despite not existing in Anthropic’s
MessageParam union, and document the omitted taskId casts on the referenced
metadata objects. Apply this consistently at the shown assertion sites without
changing the test behavior.
In `@src/core/task/Task.ts`:
- Around line 3024-3041: In the Task class, extract the repeated ghost-drop
telemetry into a private emitGhostDrop method using one resolved policy and
model reference, then replace the three duplicated telemetry blocks with calls
to it. Also extract the repeated block removal and streamingToolCallIndices
re-indexing into a private removeGhostBlock(callId) method, and update both
removal sites to use it while preserving their existing behavior.
🪄 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: c65700bd-2679-4183-a842-ed38158ddb9d
📒 Files selected for processing (45)
codecov.ymlpackages/telemetry/src/TelemetryService.tspackages/types/src/__tests__/provider-settings.test.tspackages/types/src/model.tspackages/types/src/provider-settings.tspackages/types/src/providers/mimo.tspackages/types/src/telemetry.tssrc/api/index.tssrc/api/providers/__tests__/base-provider.spec.tssrc/api/providers/__tests__/mimo.spec.tssrc/api/providers/__tests__/openai.spec.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/base-provider.tssrc/api/providers/mimo.tssrc/api/providers/openai.tssrc/core/assistant-message/NativeToolCallParser.tssrc/core/assistant-message/ToolCallRetentionPolicy.tssrc/core/assistant-message/__tests__/NativeToolCallParser.spec.tssrc/core/assistant-message/__tests__/ToolCallRetentionPolicy-telemetry.spec.tssrc/core/assistant-message/__tests__/ToolCallRetentionPolicy.spec.tssrc/core/prompts/tools/native-tools/execute_command.tssrc/core/task/Task.tssrc/core/task/__tests__/tool-call-policy.spec.tssrc/core/tools/ExecuteCommandTool.tssrc/eslint-suppressions.jsonsrc/shared/tools.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 skipped from review as they are similar to previous changes (36)
- src/shared/tools.ts
- webview-ui/src/i18n/locales/id/settings.json
- webview-ui/src/i18n/locales/zh-TW/settings.json
- webview-ui/src/i18n/locales/zh-CN/settings.json
- webview-ui/src/i18n/locales/fr/settings.json
- packages/types/src/providers/mimo.ts
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
- src/core/tools/ExecuteCommandTool.ts
- webview-ui/src/i18n/locales/de/settings.json
- webview-ui/src/i18n/locales/es/settings.json
- webview-ui/src/i18n/locales/nl/settings.json
- webview-ui/src/i18n/locales/vi/settings.json
- src/core/assistant-message/tests/ToolCallRetentionPolicy.spec.ts
- src/core/assistant-message/tests/NativeToolCallParser.spec.ts
- packages/types/src/model.ts
- packages/types/src/provider-settings.ts
- packages/telemetry/src/TelemetryService.ts
- packages/types/src/tests/provider-settings.test.ts
- src/core/assistant-message/tests/ToolCallRetentionPolicy-telemetry.spec.ts
- packages/types/src/telemetry.ts
- webview-ui/src/i18n/locales/it/settings.json
- webview-ui/src/i18n/locales/hi/settings.json
- src/api/providers/tests/base-provider.spec.ts
- webview-ui/src/i18n/locales/ru/settings.json
- src/core/prompts/tools/native-tools/execute_command.ts
- src/api/index.ts
- webview-ui/src/i18n/locales/pt-BR/settings.json
- webview-ui/src/components/settings/providers/OpenAICompatible.tsx
- webview-ui/src/i18n/locales/ko/settings.json
- src/core/task/tests/tool-call-policy.spec.ts
- webview-ui/src/i18n/locales/pl/settings.json
- src/api/providers/mimo.ts
- src/core/assistant-message/NativeToolCallParser.ts
- src/core/assistant-message/ToolCallRetentionPolicy.ts
| comment: | ||
| layout: "diff, flags, components" | ||
| behavior: default | ||
| coverage: |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use LF line endings.
YAMLlint reports wrong new line character: expected \n at Line 1. Convert codecov.yml from CRLF to LF so the lint check passes.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
🤖 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 `@codecov.yml` at line 1, Convert the line endings in codecov.yml from CRLF to
LF throughout the file, without changing its YAML content.
Source: Linters/SAST tools
Comment on lines
+375
to
377
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | ||
| tool_choice: metadata?.tool_choice, | ||
| parallel_tool_calls: metadata?.parallelToolCalls ?? true, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Apply the same parallel_tool_calls guard in the O3-family paths.
Lines 173-175 and 244-246 omit parallel_tool_calls when no tools are supplied. The O3-family paths still send it unconditionally at Line 377 and Line 411, while tools there can resolve to undefined for the same reason. An OpenAI-compatible endpoint that rejects the field without tools still fails when the model id contains o1, o3, or o4.
🔧 Proposed fix for the streaming O3 path (apply the same change at Line 409-411)
tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false),
tool_choice: metadata?.tool_choice,
- parallel_tool_calls: metadata?.parallelToolCalls ?? true,
+ // Only send parallel_tool_calls when tools are present; some
+ // OpenAI-compatible providers reject this field without tools.
+ ...(metadata?.tools && metadata.tools.length > 0
+ ? { parallel_tool_calls: metadata?.parallelToolCalls ?? true }
+ : {}),📝 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.
Suggested change
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | |
| tool_choice: metadata?.tool_choice, | |
| parallel_tool_calls: metadata?.parallelToolCalls ?? true, | |
| tools: this.convertToolsForOpenAI(metadata?.tools, this.options.openAiToolStrictMode ?? false), | |
| tool_choice: metadata?.tool_choice, | |
| // Only send parallel_tool_calls when tools are present; some | |
| // OpenAI-compatible providers reject this field without tools. | |
| ...(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 375 - 377, Update the O3-family
request construction in the relevant non-streaming and streaming paths around
the tool conversion and parallel_tool_calls assignments. Only include
parallel_tool_calls when metadata.tools is supplied and resolves to tools; omit
the field when tools is undefined, matching the existing guard used in the other
provider paths and preserving the configured boolean when tools are present.
Comment on lines
+237
to
+255
| "api/providers/__tests__/opencode-go.spec.ts": { | ||
| "@typescript-eslint/no-explicit-any": { | ||
| "count": 11 | ||
| } | ||
| }, | ||
| "api/providers/__tests__/openrouter.spec.ts": { | ||
| "@typescript-eslint/no-explicit-any": { | ||
| "count": 34 | ||
| } | ||
| }, | ||
| "api/providers/__tests__/poe.spec.ts": { | ||
| "@typescript-eslint/no-explicit-any": { | ||
| "count": 1 | ||
| } | ||
| }, | ||
| "api/providers/__tests__/qwen-code-native-tools.spec.ts": { | ||
| "@typescript-eslint/no-explicit-any": { | ||
| "count": 5 | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Git status and diff stat:"
git status --short
git diff --stat src/eslint-suppressions.json || true
echo
echo "Relevant suppression entries (old vs new):"
git diff --no-color src/eslint-suppressions.json | sed -n '200,280p' || true
echo
echo "File sizes and target lines:"
wc -l src/eslint-suppressions.json
sed -n '220,265p' src/eslint-suppressions.json | nl -ba -v220
echo
echo "Search for ESLint suppression manifest usage:"
rg -n '"src/eslint-suppressions.json"|eslint-suppressions|eslint: ' -S .Repository: Zoo-Code-Org/Zoo-Code
Length of output: 334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json, pathlib
p=pathlib.Path('src/esprit-suppressions.json')
PYRepository: Zoo-Code-Org/Zoo-Code
Length of output: 159
Keep the no-explicit-any suppression counts from increasing.
src/eslint-suppressions.json suppression counts must not increase. The entries for opencode-go.spec.ts and qwen-code-native-tools.spec.ts raise baseline @typescript-eslint/no-explicit-any counts, so remove the new any usages with typed replacements or unknown plus type guards, then restore the previous suppression counts.
🤖 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/eslint-suppressions.json` around lines 237 - 255, The no-explicit-any
suppression counts must not increase in src/eslint-suppressions.json. In
opencode-go.spec.ts and qwen-code-native-tools.spec.ts, replace the newly
introduced any usages with appropriate types or unknown plus type guards, then
restore both entries to their previous suppression counts.
Source: Coding guidelines
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.
Stack Position
fix/mimo-parallel-tool-call-policyDescription
Full Feature Description
fix/mimo-parallel-tool-call-policymimo.ts,mimo.ts,NativeToolCallParser.ts,ToolCallRetentionPolicy.ts, task policy wiring, andTelemetryService.ts. B05a is shared between this chain and theopenai-compatible-strict-reasoningchain.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 MiMo capability/request conversion, stream parser, ghost quarantine, malformed call retention, max-one execution guard, and payload-free telemetry. Does not change other provider behavior or cost calculation.
Included Files
packages/types/src/providers/mimo.tspackages/telemetry/src/TelemetryService.tssrc/api/providers/mimo.tssrc/core/assistant-message/NativeToolCallParser.tssrc/core/assistant-message/ToolCallRetentionPolicy.tsExclusion Scope
Summary by CodeRabbit
New Features
Bug Fixes
Documentation