From d296bed9050b8a94926df230b52ad64720dcea03 Mon Sep 17 00:00:00 2001 From: Rafael Dias Zendron Date: Sat, 18 Jul 2026 15:14:32 -0300 Subject: [PATCH] feat: generalize ensureThinkingBudget to all providers + preserve server-side tool invocations on antigravity (#6979) * fix(6914,6912): enable server-side tool invocations on antigravity + remove clinepass gate from ensureThinkingBudget #6914: Antigravity executor was not passing include_server_side_tool_invocations: true in toolConfig, causing server-side tool calls to be silently dropped. #6912: ensureThinkingBudget was gated to clinepass providers only, leaving non-clinepass reasoning models (nvidia, deepseek, etc.) vulnerable to empty content when the thinking budget consumed all of max_tokens. Gate removed so the budget floor applies universally. * fix(6914,6912): address code review + CI file-size antigravity.ts: preserve includeServerSideToolInvocations through sanitizeAntigravityGeminiRequest by reading it from the raw toolConfig before rebuilding (gemini-code-assist high). default.ts: use whichever key (max_tokens or max_completion_tokens) was already on the body, avoiding re-introducing max_tokens alongside max_completion_tokens for recent OpenAI models (gemini-code-assist medium). file-size-baseline.json: rebaseline executor-antigravity.test.ts 942->977 (+35, server-side tool invocation test) and default.ts 877->879 (+2, tokenKey logic). * fix(ci): update default.ts file-size baseline 879->881 (thinking-budget generalization) * refactor(executors): compact generalized thinking-budget block (file-size cap) default.ts is frozen at 877 LOC with zero headroom; the generalized ensureThinkingBudget() + max_completion_tokens-key handling added ~4 net lines. Tighten the accompanying comments (no behavior change) so the file stays within the existing 877 cap instead of raising it. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * chore(quality): drop obsolete antigravity-test rebaseline, annotate codex-test bump Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> * test(antigravity): move #6914 server-side-tools cases to own file (test-size cap) Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: rafaumeu <53516504+rafaumeu@users.noreply.github.com> --- config/quality/file-size-baseline.json | 3 +- open-sse/executors/antigravity.ts | 15 ++++++- open-sse/executors/default.ts | 27 ++++++------ ...antigravity-server-side-tools-6914.test.ts | 43 +++++++++++++++++++ tests/unit/clinepass-thinking-budget.test.ts | 16 ++++--- tests/unit/executor-antigravity.test.ts | 2 +- 6 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 tests/unit/antigravity-server-side-tools-6914.test.ts diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 383f7fb39a..0c7d6db2a5 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -143,6 +143,7 @@ "_rebaseline_2026_06_20_1449_1444_test_route": "Re-baseline providers test route.ts 842->887: combined growth of sibling fixes #1449 (bound OAuth connection-test probe with a timeout) + #1444 (label a deactivated account distinctly from a revoked token), both at the same connection-test chokepoint. Cohesive route handler; not extractable without hiding the test flow.", "_rebaseline_2026_06_20_1409_1294_models": "Re-baseline src/lib/db/models.ts 1184->1221: combined growth of sibling fixes #1409 (cascade-delete orphaned model aliases when a provider is removed) + #1294 (persist max_input_tokens/max_output_tokens on custom models), both adding CRUD at the existing models domain module. Cohesive db module; not extractable.", "_rebaseline_2026_06_20_4389_thinking_toolchoice": "Re-baseline base.ts 1387->1399 (#4389): tool_choice-forced thinking guard at the existing Claude wire-image injection chokepoint (effThinking gate avoids the Anthropic 400 when tool_choice forces a tool). Cohesive guard; structural shrink tracked in #3501.", + "_rebaseline_2026_07_18_6979_codex_test": "PR #6979 own growth: executor-codex.test.ts 1340->1347 (+7 = generalized ensureThinkingBudget assertion added to the existing codex thinking-budget cases). antigravity-test bump 942->977 REVERTED here: #7408's test split dropped that file to 888, so this PR's +35 fits under the original 942 frozen cap.", "cap": 800, "frozen": { "_rebaseline_2026_07_02_5816_qoder": "PR #5816 (@AgentKiller45, qoder PAT via qodercli): qoderCli.ts 666->989, new-above-cap frozen (owner-approved baseline freeze). The growth is the legitimate PAT job-token exchange + quota parsing CLI transport (the pure-JS Cosy path 500'd on every PAT request); extracting the spawn/parse helpers now would just add indirection to a contributor PR mid-merge. Test frozen also raised for this PR's coverage growth: providers-page-utils.test.ts 1052->1092. Additionally clears an inherited base-red from the already-merged #5933 (codex json_schema->text.format): translator-openai-responses-req.test.ts 1097->1172 (+75 regression tests, no offending branch left). All remain frozen (cannot grow further); release captain's rebaseline-at-release supersedes.", @@ -316,7 +317,7 @@ "tests/unit/db-settings-crud.test.ts": 941, "tests/unit/deepseek-web.test.ts": 1092, "tests/unit/executor-antigravity.test.ts": 942, - "tests/unit/executor-codex.test.ts": 1340, + "tests/unit/executor-codex.test.ts": 1347, "tests/unit/executor-default-base.test.ts": 1523, "tests/unit/grok-web.test.ts": 2437, "tests/unit/image-generation-handler.test.ts": 2019, diff --git a/open-sse/executors/antigravity.ts b/open-sse/executors/antigravity.ts index e77fa7211f..e9f13e5bb5 100644 --- a/open-sse/executors/antigravity.ts +++ b/open-sse/executors/antigravity.ts @@ -475,7 +475,18 @@ function sanitizeAntigravityGeminiRequest( const geminiTools = buildGeminiTools(request.tools); if (geminiTools) { clean.tools = geminiTools; - clean.toolConfig = { functionCallingConfig: { mode: "VALIDATED" } }; + // #6914: Preserve includeServerSideToolInvocations from the raw request's + // toolConfig when present (set by transformRequest when tools exist). The + // sanitize whitelist would otherwise rebuild toolConfig without it. + const rawToolConfig = asRecord(request.toolConfig); + const rawFnConfig = asRecord(rawToolConfig?.functionCallingConfig); + const includeServerSide = rawFnConfig?.includeServerSideToolInvocations === true; + clean.toolConfig = { + functionCallingConfig: { + mode: "VALIDATED", + ...(includeServerSide ? { includeServerSideToolInvocations: true } : {}), + }, + }; } else if (asRecord(request.toolConfig)) { clean.toolConfig = request.toolConfig; } @@ -708,7 +719,7 @@ export class AntigravityExecutor extends BaseExecutor { safetySettings: getAntigravitySafetySettings(normalizedRequest?.safetySettings), toolConfig: Array.isArray(normalizedRequest?.tools) && normalizedRequest.tools.length > 0 - ? { functionCallingConfig: { mode: "VALIDATED" } } + ? { functionCallingConfig: { mode: "VALIDATED", includeServerSideToolInvocations: true } } : normalizedRequest?.toolConfig, }; diff --git a/open-sse/executors/default.ts b/open-sse/executors/default.ts index 9fad2305a4..5ce3772f8d 100644 --- a/open-sse/executors/default.ts +++ b/open-sse/executors/default.ts @@ -562,9 +562,7 @@ export class DefaultExecutor extends BaseExecutor { withDefaults && typeof withDefaults === "object" && !Array.isArray(withDefaults) && - (this.provider === "cerebras" || - this.provider === "mistral" || - this.provider === "nvidia") && + (this.provider === "cerebras" || this.provider === "mistral" || this.provider === "nvidia") && Object.prototype.hasOwnProperty.call(withDefaults, "client_metadata") ) { const withoutClientMetadata = { ...(withDefaults as Record) }; @@ -732,10 +730,8 @@ export class DefaultExecutor extends BaseExecutor { } } - // ClinePass reasoning models burn all of max_tokens on the thinking phase - // when the budget is too small, leaving content empty (finish_reason: - // "length"). Bump max_tokens to a safe floor when reasoning is enabled and - // the budget is undersized. CLINEPASS-GATED — no-op for every other provider. + // Reasoning models burn all of max_tokens on the thinking phase when the budget is too + // small, leaving content empty (finish_reason: "length"); applies to all providers (#6912). if (typeof withDefaults === "object" && withDefaults !== null) { this.ensureThinkingBudget(withDefaults as Record, model); } @@ -760,12 +756,10 @@ export class DefaultExecutor extends BaseExecutor { return withDefaults; } - // ClinePass / OpenRouter-style thinking models leave content empty when the - // reasoning budget consumes all of max_tokens. Bump max_tokens to a safe - // minimum only when reasoning is enabled and the budget is undersized. - // CLINEPASS-GATED: returns early for every other provider. + // Reasoning models (ClinePass, OpenRouter, etc.) leave content empty when the reasoning + // budget consumes all of max_tokens; bump max_tokens to a safe minimum when undersized. ensureThinkingBudget(body: Record, model: string): Record { - if (!body || this.provider !== "clinepass") return body; + if (!body) return body; const outboundModel = typeof body.model === "string" ? body.model : model; const entry = getRegistryEntry(this.provider); @@ -789,10 +783,15 @@ export class DefaultExecutor extends BaseExecutor { const target = Math.min(MIN_TOKENS, maxOutput); const current = body.max_tokens ?? body.max_completion_tokens; + // #6912: keep whichever token key transformRequest already set (o1/o3/o4/gpt-5 use + // max_completion_tokens) instead of re-introducing max_tokens alongside it. + const tokenKey = + body.max_completion_tokens !== undefined ? "max_completion_tokens" : "max_tokens"; + if (typeof current !== "number" || current <= 0) { - body.max_tokens = target; + body[tokenKey] = target; } else if (current < MIN_TOKENS && current < maxOutput) { - body.max_tokens = MIN_TOKENS; + body[tokenKey] = MIN_TOKENS; } return body; } diff --git a/tests/unit/antigravity-server-side-tools-6914.test.ts b/tests/unit/antigravity-server-side-tools-6914.test.ts new file mode 100644 index 0000000000..73dfd18d7b --- /dev/null +++ b/tests/unit/antigravity-server-side-tools-6914.test.ts @@ -0,0 +1,43 @@ +// #6914: server-side tool invocations must be requested from Antigravity whenever the +// caller sends tools, and must NOT be forced when the request carries none. Lives in +// its own file (not executor-antigravity.test.ts) because that suite is frozen at the +// test-file-size cap. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { AntigravityExecutor } from "../../open-sse/executors/antigravity.ts"; + +test("AntigravityExecutor.transformRequest includes includeServerSideToolInvocations when tools are present", async () => { + const executor = new AntigravityExecutor(); + const body = { + request: { + contents: [{ role: "user", parts: [{ text: "Hello" }] }], + tools: [{ functionDeclarations: [{ name: "search" }] }], + }, + }; + + const result = await executor.transformRequest("antigravity/gemini-3.1-pro", body, true, { + projectId: "project-1", + }); + + if (result instanceof Response) throw new Error("Unexpected Response from transformRequest"); + assert.deepEqual(result.request.toolConfig, { + functionCallingConfig: { mode: "VALIDATED", includeServerSideToolInvocations: true }, + }); +}); + +test("AntigravityExecutor.transformRequest does not include includeServerSideToolInvocations when no tools", async () => { + const executor = new AntigravityExecutor(); + const body = { + request: { + contents: [{ role: "user", parts: [{ text: "Hello" }] }], + }, + }; + + const result = await executor.transformRequest("antigravity/gemini-3.1-pro", body, true, { + projectId: "project-1", + }); + + if (result instanceof Response) throw new Error("Unexpected Response from transformRequest"); + assert.equal(result.request.toolConfig, undefined); +}); diff --git a/tests/unit/clinepass-thinking-budget.test.ts b/tests/unit/clinepass-thinking-budget.test.ts index db49bac583..ade39fa1b7 100644 --- a/tests/unit/clinepass-thinking-budget.test.ts +++ b/tests/unit/clinepass-thinking-budget.test.ts @@ -3,8 +3,9 @@ import assert from "node:assert/strict"; import { DefaultExecutor } from "../../open-sse/executors/default.ts"; -// DefaultExecutor.ensureThinkingBudget — clinepass-gated max_tokens floor for +// DefaultExecutor.ensureThinkingBudget — max_tokens floor for // reasoning models (prevents empty content when the budget is undersized). +// Previously gated to clinepass only; now applies to all providers (#6912). test("bumps undersized max_tokens to 4096 for a clinepass reasoning model", () => { const executor = new DefaultExecutor("clinepass"); @@ -64,14 +65,17 @@ test("no-op for a non-reasoning clinepass model", () => { assert.equal(body.max_tokens, 100); }); -test("no-op for a non-clinepass provider (gate)", () => { - const executor = new DefaultExecutor("openrouter"); +test("bumps undersized max_tokens for a non-clinepass reasoning provider (gate removed, #6912)", () => { + // Issue #6912: ensureThinkingBudget was gated to clinepass only. + // Now it applies to all providers. Use nvidia (non-clinepass) which has + // deepseek-ai/deepseek-v4-pro with supportsReasoning in the registry. + const executor = new DefaultExecutor("nvidia"); const body = { - model: "cline-pass/deepseek-v4-pro", + model: "deepseek-ai/deepseek-v4-pro", reasoning_effort: "high", max_tokens: 100, } as Record; - executor.ensureThinkingBudget(body, "cline-pass/deepseek-v4-pro"); - assert.equal(body.max_tokens, 100); + executor.ensureThinkingBudget(body, "deepseek-ai/deepseek-v4-pro"); + assert.equal(body.max_tokens, 4096); }); diff --git a/tests/unit/executor-antigravity.test.ts b/tests/unit/executor-antigravity.test.ts index 99cf20b606..465af7a718 100644 --- a/tests/unit/executor-antigravity.test.ts +++ b/tests/unit/executor-antigravity.test.ts @@ -149,7 +149,7 @@ test("AntigravityExecutor.transformRequest normalizes model, project and content assert.equal(generationConfig.topK, 40); assert.equal(generationConfig.topP, 1.0); assert.deepEqual(result.request.toolConfig, { - functionCallingConfig: { mode: "VALIDATED" }, + functionCallingConfig: { mode: "VALIDATED", includeServerSideToolInvocations: true }, }); assert.deepEqual(result.request.contents[0].parts, [{ text: "keep me" }]); assert.equal(result.request.contents[1].role, "user");