From a5942ef2de4c5c838345499f566dee0897db0f74 Mon Sep 17 00:00:00 2001 From: Josh Bendavid Date: Fri, 24 Jul 2026 15:54:46 +0200 Subject: [PATCH] fix(omp): limit thinking levels to model's reported efforts (#2171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(omp): limit thinking levels to model's reported efforts The omp provider exposed all six thinking levels (off/minimal/low/ medium/high/xhigh) whenever a model had reasoning enabled, ignoring the per-model thinking config that omp reports via RPC. The OmpModelSchema didn't parse the thinking field at all, so the model's efforts subset and defaultLevel were discarded. Parse model.thinking (efforts/defaultLevel/effortMap) in OmpModelSchema and filter the thinking options in mapOmpModel to only the model's reported efforts. The default is the reported defaultLevel when it's in the filtered set, otherwise the first (lowest) effort. Older omp versions that don't report thinking.efforts keep getting the full set. Also fix createSession to not hardcode 'medium' as the thinking fallback when the client doesn't send a thinkingOptionId — pass undefined so omp uses its own model default instead of overriding it with a level that may not even be in the model's effort set. * test(omp): drop --thinking medium from provider-registry launch assertions The thinking-level filtering PR changed createSession to pass undefined instead of "medium" when no thinking option is selected, so buildOmpLaunch no longer emits --thinking. agent.test.ts was updated but provider-registry.test.ts still expected --thinking medium in two OMP launch assertions, failing server-tests on ubuntu and windows. --------- Co-authored-by: Mohamed Boudra --- .../server/agent/provider-registry.test.ts | 4 +- .../server/agent/providers/omp/agent.test.ts | 19 ++- .../src/server/agent/providers/omp/agent.ts | 50 +----- .../agent/providers/omp/map-omp-model.test.ts | 146 ++++++++++++++++++ .../agent/providers/omp/map-omp-model.ts | 91 +++++++++++ .../server/agent/providers/omp/rpc-types.ts | 11 ++ 6 files changed, 268 insertions(+), 53 deletions(-) create mode 100644 packages/server/src/server/agent/providers/omp/map-omp-model.test.ts create mode 100644 packages/server/src/server/agent/providers/omp/map-omp-model.ts diff --git a/packages/server/src/server/agent/provider-registry.test.ts b/packages/server/src/server/agent/provider-registry.test.ts index 4596b6d91..262051844 100644 --- a/packages/server/src/server/agent/provider-registry.test.ts +++ b/packages/server/src/server/agent/provider-registry.test.ts @@ -541,7 +541,7 @@ test("OMP is a disabled built-in backed by the real OMP adapter", async () => { expect.objectContaining({ cwd: "/tmp/registry-omp", protocolMode: "rpc-ui", - argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "yolo", "--thinking", "medium"], + argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "yolo"], }), ]); await session.close(); @@ -598,8 +598,6 @@ test("built-in OMP override keeps the real OMP adapter enabled and launchable", "rpc-ui", "--approval-mode", "yolo", - "--thinking", - "medium", ]); await session.close(); }); diff --git a/packages/server/src/server/agent/providers/omp/agent.test.ts b/packages/server/src/server/agent/providers/omp/agent.test.ts index f0adadee7..4e01109be 100644 --- a/packages/server/src/server/agent/providers/omp/agent.test.ts +++ b/packages/server/src/server/agent/providers/omp/agent.test.ts @@ -91,7 +91,7 @@ describe("OMP agent client and session", () => { cwd: "/tmp/paseo-omp-agent-test", protocolMode: "rpc-ui", modeId: "ask", - argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "always-ask", "--thinking", "medium"], + argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "always-ask"], }); expect(omp.registeredHostTools()).toEqual([ [expect.objectContaining({ name: "create_agent" })], @@ -117,10 +117,25 @@ describe("OMP agent client and session", () => { cwd: "/tmp/paseo-omp-agent-test", protocolMode: "rpc-ui", modeId: "write", - argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "write", "--thinking", "medium"], + argv: ["omp", "--mode", "rpc-ui", "--approval-mode", "write"], }); }); + test("passes --thinking when a thinking option is provided", async () => { + const omp = new OmpHarness(); + await omp.start({ modeId: "ask", thinkingOptionId: "xhigh" }, createToolCatalog()); + + expect(omp.launchConfiguration().argv).toEqual([ + "omp", + "--mode", + "rpc-ui", + "--approval-mode", + "always-ask", + "--thinking", + "xhigh", + ]); + }); + test("streams a prompt through completion", async () => { const omp = new OmpHarness(); await omp.start(); diff --git a/packages/server/src/server/agent/providers/omp/agent.ts b/packages/server/src/server/agent/providers/omp/agent.ts index 8337c250a..01ad77a36 100644 --- a/packages/server/src/server/agent/providers/omp/agent.ts +++ b/packages/server/src/server/agent/providers/omp/agent.ts @@ -107,9 +107,9 @@ import { buildOmpRpcUiPermissionResponse, mapOmpRpcUiPermissionRequest, } from "./rpc-ui-permission-mapper.js"; +import { DEFAULT_OMP_THINKING_LEVEL, mapOmpModel } from "./map-omp-model.js"; const OMP_PROVIDER = "omp"; -const DEFAULT_OMP_THINKING_LEVEL: OmpThinkingLevel = "medium"; const OMP_CATALOG_REQUEST_TIMEOUT_MS = 120_000; const QUESTION_RESPONSE_HEADER = "Response"; const QUESTION_COMMENT_HEADER = "Comment"; @@ -129,21 +129,6 @@ const OMP_CORE_CAPABILITIES: AgentCapabilityFlags = { supportsRewindBoth: false, }; -const OMP_THINKING_OPTIONS: ReadonlyArray<{ - id: OmpThinkingLevel; - label: string; - description: string; - isDefault?: boolean; -}> = [ - { id: "off", label: "Off", description: "No extra reasoning" }, - { id: "minimal", label: "Minimal", description: "Light reasoning" }, - { id: "low", label: "Low", description: "Faster reasoning" }, - { id: "medium", label: "Medium", description: "Balanced reasoning", isDefault: true }, - { id: "high", label: "High", description: "Deeper reasoning" }, - { id: "xhigh", label: "XHigh", description: "Extra-high reasoning" }, - { id: "max", label: "Max", description: "Maximum reasoning" }, -] as const; - export interface OmpAgentClientOptions { logger: Logger; runtimeSettings?: ProviderRuntimeSettings; @@ -320,21 +305,6 @@ function parseAutoCompactMode(value: string | undefined): AutoCompactMode { return "unknown"; } -function mapThinkingOption(option: (typeof OMP_THINKING_OPTIONS)[number]) { - const mappedOption = { - id: option.id, - label: option.label, - description: option.description, - }; - if (option.isDefault) { - return { - ...mappedOption, - isDefault: true, - }; - } - return mappedOption; -} - function toAgentUsage(stats: OmpSessionStats): AgentUsage | undefined { const inputTokens = stats.tokens?.input ?? 0; const cachedInputTokens = stats.tokens?.cacheRead ?? 0; @@ -883,21 +853,6 @@ function buildExtensionUiResponse( return { value: answer }; } -function mapOmpModel(model: OmpModel, provider: AgentProvider): AgentModelDefinition { - return { - provider, - id: `${model.provider}/${model.id}`, - label: `${model.provider}/${model.name ?? model.id}`, - description: `${model.provider}/${model.id}`, - metadata: { - provider: model.provider, - modelId: model.id, - }, - thinkingOptions: model.reasoning ? OMP_THINKING_OPTIONS.map(mapThinkingOption) : undefined, - defaultThinkingOptionId: model.reasoning ? DEFAULT_OMP_THINKING_LEVEL : undefined, - }; -} - function createRuntime( logger: Logger, runtimeSettings: ProviderRuntimeSettings | undefined, @@ -2301,8 +2256,7 @@ export class OmpAgentClient implements AgentClient { cwd: config.cwd, protocolMode: "rpc-ui", model: config.model, - thinkingOptionId: - normalizeOmpThinkingOption(config.thinkingOptionId) ?? DEFAULT_OMP_THINKING_LEVEL, + thinkingOptionId: normalizeOmpThinkingOption(config.thinkingOptionId) ?? undefined, noSession: config.internal === true, modeId: launchMode.modeId, extraArgs: launchMode.extraArgs, diff --git a/packages/server/src/server/agent/providers/omp/map-omp-model.test.ts b/packages/server/src/server/agent/providers/omp/map-omp-model.test.ts new file mode 100644 index 000000000..5a380e553 --- /dev/null +++ b/packages/server/src/server/agent/providers/omp/map-omp-model.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, test } from "vitest"; + +import { mapOmpModel } from "./map-omp-model.js"; +import type { OmpModel } from "./rpc-types.js"; + +function baseModel(overrides: Partial = {}): OmpModel { + return { + provider: "pioneer", + id: "canada-quant/glm-5.2", + name: "GLM-5.2", + ...overrides, + }; +} + +describe("mapOmpModel thinking options", () => { + test("limits thinking options to the model's reported efforts", () => { + const model = baseModel({ + reasoning: true, + thinking: { + mode: "effort", + efforts: ["high", "xhigh"], + defaultLevel: "xhigh", + effortMap: { high: "high", xhigh: "max" }, + }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual(["high", "xhigh"]); + expect(result.defaultThinkingOptionId).toBe("xhigh"); + expect(result.thinkingOptions?.find((option) => option.isDefault)?.id).toBe("xhigh"); + }); + + test("exposes the full set when reasoning is true but no thinking config is reported", () => { + const model = baseModel({ reasoning: true }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual([ + "off", + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + ]); + expect(result.defaultThinkingOptionId).toBe("medium"); + }); + + test("exposes the full set when efforts is empty", () => { + const model = baseModel({ + reasoning: true, + thinking: { mode: "effort", efforts: [], defaultLevel: "high" }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual([ + "off", + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + ]); + }); + + test("omits thinking options entirely when reasoning is false", () => { + const model = baseModel({ reasoning: false }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions).toBeUndefined(); + expect(result.defaultThinkingOptionId).toBeUndefined(); + }); + + test("omits thinking options when reasoning is absent", () => { + const model = baseModel({}); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions).toBeUndefined(); + expect(result.defaultThinkingOptionId).toBeUndefined(); + }); + + test("falls back to the first available option when defaultLevel is not in efforts", () => { + const model = baseModel({ + reasoning: true, + thinking: { mode: "effort", efforts: ["low", "high"], defaultLevel: "xhigh" }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual(["low", "high"]); + expect(result.defaultThinkingOptionId).toBe("low"); + expect(result.thinkingOptions?.find((option) => option.isDefault)?.id).toBe("low"); + }); + + test("uses the first option as default when defaultLevel is absent", () => { + const model = baseModel({ + reasoning: true, + thinking: { mode: "effort", efforts: ["low", "medium", "high"] }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual(["low", "medium", "high"]); + expect(result.defaultThinkingOptionId).toBe("low"); + }); + + test("falls back to the full set when every reported effort is unknown", () => { + const model = baseModel({ + reasoning: true, + thinking: { mode: "effort", efforts: ["ultra", "turbo"], defaultLevel: "turbo" }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.thinkingOptions?.map((option) => option.id)).toEqual([ + "off", + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + ]); + expect(result.defaultThinkingOptionId).toBe("medium"); + }); + + test("preserves provider and model id in the mapped definition", () => { + const model = baseModel({ + reasoning: true, + thinking: { mode: "effort", efforts: ["high", "xhigh"], defaultLevel: "xhigh" }, + }); + + const result = mapOmpModel(model, "omp"); + + expect(result.provider).toBe("omp"); + expect(result.id).toBe("pioneer/canada-quant/glm-5.2"); + expect(result.label).toBe("pioneer/GLM-5.2"); + expect(result.metadata).toEqual({ provider: "pioneer", modelId: "canada-quant/glm-5.2" }); + }); +}); diff --git a/packages/server/src/server/agent/providers/omp/map-omp-model.ts b/packages/server/src/server/agent/providers/omp/map-omp-model.ts new file mode 100644 index 000000000..6cffc7794 --- /dev/null +++ b/packages/server/src/server/agent/providers/omp/map-omp-model.ts @@ -0,0 +1,91 @@ +import type { + AgentModelDefinition, + AgentProvider, + AgentSelectOption, +} from "../../agent-sdk-types.js"; +import type { OmpModel, OmpThinkingLevel } from "./rpc-types.js"; + +export const DEFAULT_OMP_THINKING_LEVEL: OmpThinkingLevel = "medium"; + +export const OMP_THINKING_OPTIONS: ReadonlyArray<{ + id: OmpThinkingLevel; + label: string; + description: string; + isDefault?: boolean; +}> = [ + { id: "off", label: "Off", description: "No extra reasoning" }, + { id: "minimal", label: "Minimal", description: "Light reasoning" }, + { id: "low", label: "Low", description: "Faster reasoning" }, + { id: "medium", label: "Medium", description: "Balanced reasoning", isDefault: true }, + { id: "high", label: "High", description: "Deeper reasoning" }, + { id: "xhigh", label: "XHigh", description: "Extra-high reasoning" }, + { id: "max", label: "Max", description: "Maximum reasoning" }, +] as const; + +function mapThinkingOption( + option: (typeof OMP_THINKING_OPTIONS)[number], + isDefault?: boolean, +): AgentSelectOption { + const mapped: AgentSelectOption = { + id: option.id, + label: option.label, + description: option.description, + }; + if (isDefault ?? option.isDefault) { + mapped.isDefault = true; + } + return mapped; +} + +export function mapOmpModel(model: OmpModel, provider: AgentProvider): AgentModelDefinition { + const { thinkingOptions, defaultThinkingOptionId } = resolveOmpThinkingConfig(model); + return { + provider, + id: `${model.provider}/${model.id}`, + label: `${model.provider}/${model.name ?? model.id}`, + description: `${model.provider}/${model.id}`, + metadata: { + provider: model.provider, + modelId: model.id, + }, + thinkingOptions, + defaultThinkingOptionId, + }; +} + +function resolveOmpThinkingConfig(model: OmpModel): { + thinkingOptions: AgentSelectOption[] | undefined; + defaultThinkingOptionId: string | undefined; +} { + if (!model.reasoning) { + return { thinkingOptions: undefined, defaultThinkingOptionId: undefined }; + } + const efforts = model.thinking?.efforts; + if (!efforts || efforts.length === 0) { + // Older omp versions don't report per-model thinking config; expose the full set. + return { + thinkingOptions: OMP_THINKING_OPTIONS.map((option) => mapThinkingOption(option)), + defaultThinkingOptionId: DEFAULT_OMP_THINKING_LEVEL, + }; + } + const effortSet = new Set(efforts); + const filtered = OMP_THINKING_OPTIONS.filter((option) => effortSet.has(option.id)); + if (filtered.length === 0) { + // All reported efforts are unrecognized; fall back to the full set with the standard default. + return { + thinkingOptions: OMP_THINKING_OPTIONS.map((option) => mapThinkingOption(option)), + defaultThinkingOptionId: DEFAULT_OMP_THINKING_LEVEL, + }; + } + const reportedDefault = model.thinking?.defaultLevel; + const defaultThinkingOptionId = + reportedDefault && filtered.some((option) => option.id === reportedDefault) + ? reportedDefault + : (filtered[0]?.id ?? DEFAULT_OMP_THINKING_LEVEL); + return { + thinkingOptions: filtered.map((option) => + mapThinkingOption(option, option.id === defaultThinkingOptionId), + ), + defaultThinkingOptionId, + }; +} diff --git a/packages/server/src/server/agent/providers/omp/rpc-types.ts b/packages/server/src/server/agent/providers/omp/rpc-types.ts index 31a057291..e6e530317 100644 --- a/packages/server/src/server/agent/providers/omp/rpc-types.ts +++ b/packages/server/src/server/agent/providers/omp/rpc-types.ts @@ -87,12 +87,22 @@ export const OmpAgentMessageSchema = z.discriminatedUnion("role", [ OmpBashExecutionMessageSchema, ]); +export const OmpModelThinkingSchema = z + .object({ + mode: z.string().optional(), + efforts: z.array(z.string()).optional(), + defaultLevel: z.string().optional(), + effortMap: z.record(z.string(), z.string()).optional(), + }) + .passthrough(); + export const OmpModelSchema = z .object({ provider: z.string(), id: z.string(), name: z.string().optional(), reasoning: z.boolean().optional(), + thinking: OmpModelThinkingSchema.optional(), contextWindow: z.number().optional(), maxTokens: z.number().nullable().optional(), api: z.string().optional(), @@ -568,6 +578,7 @@ export type OmpToolCallContent = z.infer; export type OmpAssistantContent = z.infer; export type OmpAgentMessage = z.infer; export type OmpModel = z.infer; +export type OmpModelThinking = z.infer; export type OmpSessionState = z.infer; export type OmpSessionStats = z.infer; export type OmpRpcSlashCommand = z.infer;