-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(adapters): protect tiny standalone GLM summary compaction from reasoning exhaustion #5953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import { modelRecordValue } from "../../reasoning-effort"; | ||
| import type { OcxParsedRequest, OcxProviderConfig } from "../../types"; | ||
|
|
||
| export function resolveMaxTokens(provider: OcxProviderConfig, parsed: OcxParsedRequest): number | undefined { | ||
| return parsed.options.maxOutputTokens | ||
| ?? modelRecordValue(provider.modelMaxOutputTokens, parsed.modelId) | ||
| ?? provider.defaultMaxOutputTokens; | ||
| } | ||
|
|
||
| function textContent(value: unknown): string | undefined { | ||
| if (typeof value === "string") return value; | ||
| if (!Array.isArray(value)) return undefined; | ||
| const text: string[] = []; | ||
| for (const part of value) { | ||
| if (!part || part.type !== "text" || typeof part.text !== "string") return undefined; | ||
| text.push(part.text); | ||
| } | ||
| return text.join("\n"); | ||
| } | ||
|
|
||
| /** Aside's emergency checkpoint is a standalone summary, not an ordinary short answer. | ||
| * Runs at the physical Chat destination, after all combo effort overrides. | ||
| */ | ||
| export function protectGlmSummaryBudget(body: Record<string, unknown>): boolean { | ||
| if (typeof body.model !== "string" | ||
| || !/^(?:(?:zai|z-ai|zai-org)\/)?glm-5\.3-flash$/i.test(body.model)) return false; | ||
| if (body.tools !== undefined && (!Array.isArray(body.tools) || body.tools.length > 0)) return false; | ||
| const cap = body.max_completion_tokens ?? body.max_tokens; | ||
| if (typeof cap !== "number" || !Number.isInteger(cap) || cap < 1 || cap > 1024) return false; | ||
| const messages = body.messages; | ||
| if (!Array.isArray(messages) || messages.length !== 2) return false; | ||
| const [system, user] = messages; | ||
| if (!system || !user || !["system", "developer"].includes(system.role) || user.role !== "user" | ||
| || system.tool_calls || user.tool_calls || system.function_call || user.function_call) return false; | ||
| const instruction = textContent(system.content); | ||
| const transcript = textContent(user.content); | ||
| if (instruction === undefined || transcript === undefined) return false; | ||
| const summaryInstruction = /\bcontext[-\s]+summari[sz](?:ation|er|ing)\b/i.test(instruction); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Require a checkpoint-specific signal before changing an ordinary turn.
🤖 Prompt for AI Agents |
||
| const checkpointTranscript = /\b(?:summari[sz]e|summary|checkpoint)\b/i.test(instruction) | ||
| && /<conversation>[\s\S]*<\/conversation>/i.test(transcript); | ||
| if (!summaryInstruction && !checkpointTranscript) return false; | ||
| // Update both if supplied: gateways differ on which cap takes precedence. | ||
| if (body.max_tokens !== undefined) body.max_tokens = 4096; | ||
| if (body.max_completion_tokens !== undefined) body.max_completion_tokens = 4096; | ||
|
Comment on lines
+43
to
+44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Raise eligible caps to the required 8192 tokens. The PR objective specifies 8192, but both assignments set 4096. A checkpoint that needs more than 4096 output tokens can still be truncated despite matching this mitigation. Set both supplied cap fields to 8192 and update the cap assertions in 🤖 Prompt for AI Agents |
||
| return true; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { buildOpenAIChatPassthroughRequest, createOpenAIChatAdapter } from "../../../src/adapters/openai-chat"; | ||
| import { chatCompletionsToResponsesBody } from "../../../src/chat/inbound"; | ||
| import { concreteComboRequestBody } from "../../../src/combos/request"; | ||
| import { parseRequest } from "../../../src/responses/parser"; | ||
| import type { OcxProviderConfig } from "../../../src/types"; | ||
|
|
||
| const model = "glm-5.3-flash"; | ||
| const provider: OcxProviderConfig = { | ||
| adapter: "openai-chat", baseUrl: "https://api.z.ai/api/coding/paas/v4", | ||
| reasoningEfforts: ["low", "medium", "high", "max"], | ||
| }; | ||
| const messages = [ | ||
| { role: "system", content: "You are a context-summarization assistant. Produce a checkpoint." }, | ||
| { role: "user", content: "<conversation>User: implement the feature.</conversation>" }, | ||
| ]; | ||
| function bodies(overrides: Record<string, unknown> = {}, config = provider) { | ||
| const raw = { model, messages, max_tokens: 512, reasoning_effort: "max", ...overrides }; | ||
| const parsed = parseRequest(chatCompletionsToResponsesBody(raw)); | ||
| return [ | ||
| JSON.parse(createOpenAIChatAdapter(config).buildRequest(parsed).body), | ||
| JSON.parse(buildOpenAIChatPassthroughRequest(config, raw, String(raw.model), false).body), | ||
| ]; | ||
| } | ||
|
|
||
| describe("GLM tiny standalone summary compatibility", () => { | ||
| test.each([1, 512, 819, 1024])("raises cap %i and lowers effort on both Chat paths", cap => { | ||
| for (const body of bodies({ max_tokens: cap })) { | ||
| expect(body.max_tokens).toBe(4096); | ||
| expect(body.reasoning_effort).toBe("low"); | ||
| expect(body.messages).toEqual(messages); | ||
| } | ||
| }); | ||
| test("final adapter wins after successive combo force overrides", () => { | ||
| let raw = chatCompletionsToResponsesBody({ model, messages, max_tokens: 819, reasoning_effort: "high" }); | ||
| for (const target of [{ provider: "proxy", model: "inner" }, { provider: "zai", model }]) { | ||
| raw = concreteComboRequestBody(raw, target, "max", provider.reasoningEfforts, "strict", "force"); | ||
| } | ||
| const parsed = parseRequest(raw); | ||
| parsed.modelId = model; | ||
| expect(parsed.options.reasoning).toBe("max"); | ||
| const body = JSON.parse(createOpenAIChatAdapter(provider).buildRequest(parsed).body); | ||
| expect(body.max_tokens).toBe(4096); | ||
| expect(body.reasoning_effort).toBe("low"); | ||
| expect(parsed.options.maxOutputTokens).toBe(819); | ||
| expect(parsed.options.reasoning).toBe("max"); | ||
| }); | ||
| test.each([0, -1, 1025, 4096, undefined])("preserves cap outside the mitigation: %s", cap => { | ||
| for (const body of bodies({ max_tokens: cap })) { | ||
| expect(body.max_tokens).toBe(cap); | ||
| expect(body.reasoning_effort).toBe("max"); | ||
| } | ||
| }); | ||
| test("does not change other models, ordinary prompts, tools, or ongoing conversations", () => { | ||
| for (const overrides of [ | ||
| { model: "glm-5.3" }, { model: "glm-5.3-flashx" }, { model: "gpt-5" }, | ||
| { messages: [{ role: "system", content: "Be helpful." }, { role: "user", content: "Summarize this article." }] }, | ||
| { messages: [...messages, { role: "assistant", content: "Previous checkpoint" }] }, | ||
| { tools: [{ type: "function", function: { name: "read", parameters: { type: "object", properties: {} } } }] }, | ||
| ]) for (const body of bodies(overrides)) { | ||
| expect(body.max_tokens).toBe(512); | ||
| expect(body.reasoning_effort).toBe("max"); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check both caps when both fields are supplied.
The passthrough builder forwards
max_tokensandmax_completion_tokens. For an otherwise eligible request withmax_completion_tokens: 4096andmax_tokens: 1, this expression checks only 4096 and returns without changing either field. A gateway that usesmax_tokensretains the one-token limit. Evaluate each supplied cap before excluding the request, then update the field that can impose the tiny limit.🤖 Prompt for AI Agents