fix(6813): fix thinking budget zero drop and default thinkingConfig injection - #6943
Conversation
… (ABI 148) (diegosouzapw#6605) fix(electron): bump electron 42→43 + rebuild better-sqlite3 from source against the Electron ABI (148). Electron 43 raises NODE_MODULE_VERSION to 148; better-sqlite3@12.11.1 has no electron-v148 prebuild, so the packaged app died with 'Nenhum driver SQLite disponível'. prepare-electron-standalone now compiles better-sqlite3 from source against the electron headers into build/Release (where 'bindings' resolves it). Validated by Electron Package Smoke (green) + local (node_register_module_v148). Supersedes diegosouzapw#6378. (--admin: the only reds are SonarQube/SonarCloud failing on a coverage-report artifact digest-mismatch — a GitHub Actions infra flake, not this diff; Sonar is green on main and the diff touches only the electron build.)
…iegosouzapw#6588) deps: bump the development group (6 updates). Rebased onto current main; all checks green after the electron-smoke fix (diegosouzapw#6605).
…7) + production deps bump (diegosouzapw#6620) fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump. undici 8.6+ changed ProxyAgent to forward plain-HTTP via request-proxy instead of CONNECT, breaking OAuth refresh through a connection proxy (501). proxyDispatcher now passes proxyTunnel:true. Validated: Unit Tests 3/8 (the OAuth-proxy test) green, new regression test green (fails without the fix on undici 8.7), SonarQube green. Supersedes diegosouzapw#6380. (--admin: the only red is Electron Package Smoke failing on a next-build artifact 'digest-mismatch' — a GitHub Actions infra flake corrupting the asar ('file data stream has unexpected number of bytes'); the better-sqlite3 rebuild itself succeeded (gyp ok) and the electron path is unchanged from diegosouzapw#6605 which passed the smoke. Not this diff.)
…njection - Fix truthy check for budget_tokens to allow 0 - Stop injecting default thinkingConfig when no knobs present - Add tests covering all scenarios Related: diegosouzapw#6813
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks for tackling #6813 — the "default thinkingConfig injection" half (defect 2) is real and genuinely not fixed yet anywhere on release. A few things need to be sorted before this can merge, though:
Happy to take another look once it's rebased onto |
…njection Merge origin/release/v3.8.47 and resolve conflict in openai-to-gemini.ts: the budget_tokens:0 drop (defect 1) was already fixed on release via diegosouzapw#6821 with a stricter includeThoughts check, so this drops the redundant/inferior duplicate here and keeps the already-merged version (plus the gemma-4 guard from diegosouzapw#6708). Rework the default-thinkingConfig-injection fix (defect 2) so it does not regress the diegosouzapw#4170 golden invariant: modern Gemini 2.5+ models think by default even with no thinkingConfig sent, so the no-knob-at-all case must keep injecting includeThoughts:true (unchanged from release). The real, golden-safe delta for diegosouzapw#6813 defect 2 is an explicit off-switch: reasoning_effort:"none" now maps to thinkingBudget:0/includeThoughts:false in the OpenAI-format branch, giving callers a documented way to opt out without touching the no-knob default path. Also drops the dead `reasoning_effort === "none" ? 0 : defaultBudget` ternary from the default-injection block — unreachable since block 1 already handles every truthy reasoning_effort value, including "none". Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Merged — reworked to preserve the #4170 golden invariant: block-3 default-injection stays unconditional (matches the golden no-knob case), and the documented off-switch from #6813 is delivered via |
…aults test with #6943 (unit-full pre-flight)
…rt path The thinking-budget-cap guard added in this branch skipped thinkingConfig entirely whenever a model's thinkingBudgetCap was 0 (e.g. gemini-3-flash), including on the reasoning_effort/budgetMap path. That regressed the pre-diegosouzapw#6943 native-defaults contract (thinkingBudget 0 / includeThoughts false must still be present) and crashed callers that read `.thinkingConfig.thinkingBudget` unconditionally (translator-openai-to-gemini-defaults.test.ts). Also restore includeThoughts:true on the Claude-format explicit thinking.budget_tokens path (openai-to-gemini.ts's Claude-format field and claude-to-gemini.ts's native thinking field): budget_tokens:0 there is the client's dynamic-thinking sentinel (diegosouzapw#6813), not an off-switch, and must stay true even after the new capping — the cap must only clamp positive explicit values, never flip the zero sentinel's semantics. Updates two tests this branch added that encoded the incorrect "omit thinkingConfig / includeThoughts:false for the 0 sentinel" behavior, to match the pre-existing, still-required contracts above. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…8312) * fix(translator): cap thinking budget on explicit budget_tokens path * fix(translator): stop dropping thinkingConfig on cap-0 reasoning_effort path The thinking-budget-cap guard added in this branch skipped thinkingConfig entirely whenever a model's thinkingBudgetCap was 0 (e.g. gemini-3-flash), including on the reasoning_effort/budgetMap path. That regressed the pre-#6943 native-defaults contract (thinkingBudget 0 / includeThoughts false must still be present) and crashed callers that read `.thinkingConfig.thinkingBudget` unconditionally (translator-openai-to-gemini-defaults.test.ts). Also restore includeThoughts:true on the Claude-format explicit thinking.budget_tokens path (openai-to-gemini.ts's Claude-format field and claude-to-gemini.ts's native thinking field): budget_tokens:0 there is the client's dynamic-thinking sentinel (#6813), not an off-switch, and must stay true even after the new capping — the cap must only clamp positive explicit values, never flip the zero sentinel's semantics. Updates two tests this branch added that encoded the incorrect "omit thinkingConfig / includeThoughts:false for the 0 sentinel" behavior, to match the pre-existing, still-required contracts above. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * fix(providers): revert scope-creep flip of Gemini 3.5/3.6 Flash supportsThinking The thinking-budget-cap fix accidentally expanded 5 shorthand modelSpecs entries (gemini-3.5-flash, gemini-3.5-flash-low, gemini-3.6-flash-high/ medium/low) into explicit objects setting supportsThinking:true and thinkingBudgetCap:24576. That flip was unrelated to the two proven test regressions (translator-openai-to-gemini-defaults.test.ts and claude-to-gemini-budget-tokens-zero-6813.test.ts, which only exercise gemini-3-flash-preview, gemini-3.1-pro and gemini-2.5-pro) and reopens a deliberately closed path from #8013: Antigravity still rejects client-supplied thinking params for these Gemini 3.5/3.6 Flash tier ids, so supportsThinking must stay false (inherited from GEMINI_35_FLASH_MODEL_SPEC). Reverted all 5 entries back to the release shorthand `{ ...GEMINI_35_FLASH_MODEL_SPEC }`. gemini-3-flash, gemini-3.1-pro and gemini-2.5-pro (the models the regression tests actually exercise) were already correctly specced in the release baseline and are untouched. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…njection (diegosouzapw#6943) * fix(electron): bump electron 42→43 + build better-sqlite3 from source (ABI 148) (diegosouzapw#6605) fix(electron): bump electron 42→43 + rebuild better-sqlite3 from source against the Electron ABI (148). Electron 43 raises NODE_MODULE_VERSION to 148; better-sqlite3@12.11.1 has no electron-v148 prebuild, so the packaged app died with 'Nenhum driver SQLite disponível'. prepare-electron-standalone now compiles better-sqlite3 from source against the electron headers into build/Release (where 'bindings' resolves it). Validated by Electron Package Smoke (green) + local (node_register_module_v148). Supersedes diegosouzapw#6378. (--admin: the only reds are SonarQube/SonarCloud failing on a coverage-report artifact digest-mismatch — a GitHub Actions infra flake, not this diff; Sonar is green on main and the diff touches only the electron build.) * deps: bump the development group across 1 directory with 6 updates (diegosouzapw#6588) deps: bump the development group (6 updates). Rebased onto current main; all checks green after the electron-smoke fix (diegosouzapw#6605). * fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump (diegosouzapw#6620) fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump. undici 8.6+ changed ProxyAgent to forward plain-HTTP via request-proxy instead of CONNECT, breaking OAuth refresh through a connection proxy (501). proxyDispatcher now passes proxyTunnel:true. Validated: Unit Tests 3/8 (the OAuth-proxy test) green, new regression test green (fails without the fix on undici 8.7), SonarQube green. Supersedes diegosouzapw#6380. (--admin: the only red is Electron Package Smoke failing on a next-build artifact 'digest-mismatch' — a GitHub Actions infra flake corrupting the asar ('file data stream has unexpected number of bytes'); the better-sqlite3 rebuild itself succeeded (gyp ok) and the electron path is unchanged from diegosouzapw#6605 which passed the smoke. Not this diff.) * fix(6813): fix thinking budget zero drop and default thinkingConfig injection - Fix truthy check for budget_tokens to allow 0 - Stop injecting default thinkingConfig when no knobs present - Add tests covering all scenarios Related: diegosouzapw#6813 --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…n gemini defaults test with diegosouzapw#6943 (unit-full pre-flight)
…egosouzapw#6943), eslint suppression match, file-size/zizmor rebaselines
…iegosouzapw#8312) * fix(translator): cap thinking budget on explicit budget_tokens path * fix(translator): stop dropping thinkingConfig on cap-0 reasoning_effort path The thinking-budget-cap guard added in this branch skipped thinkingConfig entirely whenever a model's thinkingBudgetCap was 0 (e.g. gemini-3-flash), including on the reasoning_effort/budgetMap path. That regressed the pre-diegosouzapw#6943 native-defaults contract (thinkingBudget 0 / includeThoughts false must still be present) and crashed callers that read `.thinkingConfig.thinkingBudget` unconditionally (translator-openai-to-gemini-defaults.test.ts). Also restore includeThoughts:true on the Claude-format explicit thinking.budget_tokens path (openai-to-gemini.ts's Claude-format field and claude-to-gemini.ts's native thinking field): budget_tokens:0 there is the client's dynamic-thinking sentinel (diegosouzapw#6813), not an off-switch, and must stay true even after the new capping — the cap must only clamp positive explicit values, never flip the zero sentinel's semantics. Updates two tests this branch added that encoded the incorrect "omit thinkingConfig / includeThoughts:false for the 0 sentinel" behavior, to match the pre-existing, still-required contracts above. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * fix(providers): revert scope-creep flip of Gemini 3.5/3.6 Flash supportsThinking The thinking-budget-cap fix accidentally expanded 5 shorthand modelSpecs entries (gemini-3.5-flash, gemini-3.5-flash-low, gemini-3.6-flash-high/ medium/low) into explicit objects setting supportsThinking:true and thinkingBudgetCap:24576. That flip was unrelated to the two proven test regressions (translator-openai-to-gemini-defaults.test.ts and claude-to-gemini-budget-tokens-zero-6813.test.ts, which only exercise gemini-3-flash-preview, gemini-3.1-pro and gemini-2.5-pro) and reopens a deliberately closed path from diegosouzapw#8013: Antigravity still rejects client-supplied thinking params for these Gemini 3.5/3.6 Flash tier ids, so supportsThinking must stay false (inherited from GEMINI_35_FLASH_MODEL_SPEC). Reverted all 5 entries back to the release shorthand `{ ...GEMINI_35_FLASH_MODEL_SPEC }`. gemini-3-flash, gemini-3.1-pro and gemini-2.5-pro (the models the regression tests actually exercise) were already correctly specced in the release baseline and are untouched. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…njection (diegosouzapw#6943) * fix(electron): bump electron 42→43 + build better-sqlite3 from source (ABI 148) (diegosouzapw#6605) fix(electron): bump electron 42→43 + rebuild better-sqlite3 from source against the Electron ABI (148). Electron 43 raises NODE_MODULE_VERSION to 148; better-sqlite3@12.11.1 has no electron-v148 prebuild, so the packaged app died with 'Nenhum driver SQLite disponível'. prepare-electron-standalone now compiles better-sqlite3 from source against the electron headers into build/Release (where 'bindings' resolves it). Validated by Electron Package Smoke (green) + local (node_register_module_v148). Supersedes diegosouzapw#6378. (--admin: the only reds are SonarQube/SonarCloud failing on a coverage-report artifact digest-mismatch — a GitHub Actions infra flake, not this diff; Sonar is green on main and the diff touches only the electron build.) * deps: bump the development group across 1 directory with 6 updates (diegosouzapw#6588) deps: bump the development group (6 updates). Rebased onto current main; all checks green after the electron-smoke fix (diegosouzapw#6605). * fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump (diegosouzapw#6620) fix(proxy): force CONNECT tunnel for HTTP proxied requests (undici 8.7) + production deps bump. undici 8.6+ changed ProxyAgent to forward plain-HTTP via request-proxy instead of CONNECT, breaking OAuth refresh through a connection proxy (501). proxyDispatcher now passes proxyTunnel:true. Validated: Unit Tests 3/8 (the OAuth-proxy test) green, new regression test green (fails without the fix on undici 8.7), SonarQube green. Supersedes diegosouzapw#6380. (--admin: the only red is Electron Package Smoke failing on a next-build artifact 'digest-mismatch' — a GitHub Actions infra flake corrupting the asar ('file data stream has unexpected number of bytes'); the better-sqlite3 rebuild itself succeeded (gyp ok) and the electron path is unchanged from diegosouzapw#6605 which passed the smoke. Not this diff.) * fix(6813): fix thinking budget zero drop and default thinkingConfig injection - Fix truthy check for budget_tokens to allow 0 - Stop injecting default thinkingConfig when no knobs present - Add tests covering all scenarios Related: diegosouzapw#6813 --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…n gemini defaults test with diegosouzapw#6943 (unit-full pre-flight)
…egosouzapw#6943), eslint suppression match, file-size/zizmor rebaselines
…iegosouzapw#8312) * fix(translator): cap thinking budget on explicit budget_tokens path * fix(translator): stop dropping thinkingConfig on cap-0 reasoning_effort path The thinking-budget-cap guard added in this branch skipped thinkingConfig entirely whenever a model's thinkingBudgetCap was 0 (e.g. gemini-3-flash), including on the reasoning_effort/budgetMap path. That regressed the pre-diegosouzapw#6943 native-defaults contract (thinkingBudget 0 / includeThoughts false must still be present) and crashed callers that read `.thinkingConfig.thinkingBudget` unconditionally (translator-openai-to-gemini-defaults.test.ts). Also restore includeThoughts:true on the Claude-format explicit thinking.budget_tokens path (openai-to-gemini.ts's Claude-format field and claude-to-gemini.ts's native thinking field): budget_tokens:0 there is the client's dynamic-thinking sentinel (diegosouzapw#6813), not an off-switch, and must stay true even after the new capping — the cap must only clamp positive explicit values, never flip the zero sentinel's semantics. Updates two tests this branch added that encoded the incorrect "omit thinkingConfig / includeThoughts:false for the 0 sentinel" behavior, to match the pre-existing, still-required contracts above. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * fix(providers): revert scope-creep flip of Gemini 3.5/3.6 Flash supportsThinking The thinking-budget-cap fix accidentally expanded 5 shorthand modelSpecs entries (gemini-3.5-flash, gemini-3.5-flash-low, gemini-3.6-flash-high/ medium/low) into explicit objects setting supportsThinking:true and thinkingBudgetCap:24576. That flip was unrelated to the two proven test regressions (translator-openai-to-gemini-defaults.test.ts and claude-to-gemini-budget-tokens-zero-6813.test.ts, which only exercise gemini-3-flash-preview, gemini-3.1-pro and gemini-2.5-pro) and reopens a deliberately closed path from diegosouzapw#8013: Antigravity still rejects client-supplied thinking params for these Gemini 3.5/3.6 Flash tier ids, so supportsThinking must stay false (inherited from GEMINI_35_FLASH_MODEL_SPEC). Reverted all 5 entries back to the release shorthand `{ ...GEMINI_35_FLASH_MODEL_SPEC }`. gemini-3-flash, gemini-3.1-pro and gemini-2.5-pro (the models the regression tests actually exercise) were already correctly specced in the release baseline and are untouched. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Summary
Fix truthy check for
budget_tokensto allow0(was dropping zero budgets), and stop injecting defaultthinkingConfigwhen no knobs present.Changes
.planning/6813-openai-to-gemini-thinking-budget-zero.md.planning/6813-openai-to-gemini-thinking-budget-zero-PLAN.mdopen-sse/translator/request/openai-to-gemini.ts— fix truthy checkbudget_tokens > 0(wasbudget_tokens) and guard default injectionopen-sse/translator/request/__tests__/openai-to-gemini.test.ts— +85 lines covering all scenariosVerification
npm run lint— passesnpm run typecheck:core— passesRelated
Fixes #6813