diff --git a/src/core/eval.tsx b/src/core/eval.tsx index d22a11982..0239b2eb1 100644 --- a/src/core/eval.tsx +++ b/src/core/eval.tsx @@ -41,6 +41,7 @@ import { type DeleteOnlineEvaluationConfigResponse, type DatasetStatus, type EvaluatorConfig, + type EvaluatorModelConfig, type GetConfigurationBundleResponse, type GetConfigurationBundleVersionResponse, type GetDatasetResponse, @@ -162,6 +163,7 @@ import { isTerminalStatus, readEvaluationResults } from "./batchEvaluationResult import { applyExampleIds, diffExamples, indexRemoteById, parseJsonl } from "./datasetDiff"; import type { Addition } from "./datasetDiff"; import type { AwsClients, CoreFetch, CoreOptions } from "./types"; +import type { EvaluatorModelProvider } from "../projectSchemas/evaluator"; import type { Logger } from "../logging"; import { FilteredPaginator } from "./filteredPaginator"; import { toClientConfig } from "./utils"; @@ -264,14 +266,35 @@ export class EvalClient implements CoreEvalClient { const instructions = update.instructions ?? existing?.instructions; const ratingScale = update.ratingScale ?? existing?.ratingScale; - // Preserve the existing Bedrock model config (inferenceConfig, - // additionalModelRequestFields, ...) and override only the model id, so an - // update that touches other fields does not drop model tuning. - const existingModel = + + // Detect the current provider from whichever union arm is set, so an update + // that never mentions the provider stays on it. Each arm carries + // provider-specific tuning (Bedrock inferenceConfig, OpenResponses + // temperature/topP/reasoning, ...) preserved by spreading it below. + const existingBedrock = existing?.modelConfig && "bedrockEvaluatorModelConfig" in existing.modelConfig ? existing.modelConfig.bedrockEvaluatorModelConfig : undefined; - const modelId = update.model ?? existingModel?.modelId; + const existingResponses = + existing?.modelConfig && "responsesEvaluatorModelConfig" in existing.modelConfig + ? existing.modelConfig.responsesEvaluatorModelConfig + : undefined; + const currentProvider: EvaluatorModelProvider = existingResponses ? "OpenResponses" : "Bedrock"; + const targetProvider = update.modelProvider ?? currentProvider; + const providerChanged = targetProvider !== currentProvider; + + // A model id from one provider's API is not valid for the other, so a + // provider switch cannot reuse the existing id — require a fresh one. + if (providerChanged && !update.model) { + throw new InputValidationError( + `Changing evaluator "${id}" to the ${targetProvider} provider requires a new --model`, + { meta: { evaluatorId: id } }, + ); + } + + const modelId = + update.model ?? + (targetProvider === "OpenResponses" ? existingResponses?.modelId : existingBedrock?.modelId); if (!instructions || !ratingScale || !modelId) { throw new InputValidationError( @@ -281,11 +304,23 @@ export class EvalClient implements CoreEvalClient { ); } + // On a provider switch there is no same-provider tuning to keep, so start + // from the OpenResponses deployment defaults (Bedrock supplies its own). + const modelConfig: EvaluatorModelConfig = + targetProvider === "OpenResponses" + ? { + responsesEvaluatorModelConfig: { + ...(providerChanged ? { maxOutputTokens: 4096, temperature: 0 } : existingResponses), + modelId, + }, + } + : { bedrockEvaluatorModelConfig: { ...(providerChanged ? {} : existingBedrock), modelId } }; + const evaluatorConfig: EvaluatorConfig = { llmAsAJudge: { instructions, ratingScale, - modelConfig: { bedrockEvaluatorModelConfig: { ...existingModel, modelId } }, + modelConfig, }, }; diff --git a/src/core/evalEvaluatorProvider.test.tsx b/src/core/evalEvaluatorProvider.test.tsx new file mode 100644 index 000000000..65eaa7cab --- /dev/null +++ b/src/core/evalEvaluatorProvider.test.tsx @@ -0,0 +1,148 @@ +import { describe, expect, test } from "bun:test"; +import { + GetEvaluatorCommand, + UpdateEvaluatorCommand, + type BedrockAgentCoreControlClient, + type EvaluatorConfig, + type UpdateEvaluatorRequest, +} from "@aws-sdk/client-bedrock-agentcore-control"; +import type { BedrockAgentCoreClient } from "@aws-sdk/client-bedrock-agentcore"; +import type { IAMClient } from "@aws-sdk/client-iam"; +import type { CloudWatchLogsClient } from "@aws-sdk/client-cloudwatch-logs"; +import { CoreClient } from "./index"; +import { InputValidationError } from "../errors"; +import { createSilentLogger } from "../testing"; + +const OPTIONS = { region: "us-west-2", endpointUrl: undefined }; +const ID = "eval-0000000000"; +const RATING_SCALE = { categorical: [{ label: "pass", definition: "ok" }] }; + +// captureUpdate builds a CoreClient whose GetEvaluator returns `existing` and +// whose UpdateEvaluator records the request it receives, so a test can assert +// the merged evaluatorConfig without a live service. +function captureUpdate(existing: EvaluatorConfig): { + core: CoreClient; + sent: () => UpdateEvaluatorRequest; +} { + let captured: UpdateEvaluatorRequest | undefined; + const control = { + send: async (command: unknown) => { + if (command instanceof GetEvaluatorCommand) { + return { evaluatorId: ID, level: "SESSION", evaluatorConfig: existing }; + } + if (command instanceof UpdateEvaluatorCommand) { + captured = command.input; + return { evaluatorId: ID }; + } + throw new Error(`unexpected control command: ${(command as object).constructor.name}`); + }, + } as unknown as BedrockAgentCoreControlClient; + + const core = new CoreClient({ + createControlClient: () => control, + createDataClient: () => ({}) as BedrockAgentCoreClient, + createIamClient: () => ({}) as IAMClient, + createLogsClient: () => ({}) as CloudWatchLogsClient, + logger: createSilentLogger(), + }); + return { + core, + sent: () => { + if (!captured) throw new Error("UpdateEvaluator was never called"); + return captured; + }, + }; +} + +const bedrockExisting = (extra: Record = {}): EvaluatorConfig => ({ + llmAsAJudge: { + instructions: "Judge from {context}.", + ratingScale: RATING_SCALE, + modelConfig: { + bedrockEvaluatorModelConfig: { modelId: "bedrock.model-v1:0", ...extra }, + }, + }, +}); + +const responsesExisting = (extra: Record = {}): EvaluatorConfig => ({ + llmAsAJudge: { + instructions: "Judge from {context}.", + ratingScale: RATING_SCALE, + modelConfig: { + responsesEvaluatorModelConfig: { + modelId: "openai.gpt-5.4", + maxOutputTokens: 4096, + temperature: 0, + ...extra, + }, + }, + }, +}); + +function llaj(request: UpdateEvaluatorRequest) { + const config = request.evaluatorConfig; + if (!config || !("llmAsAJudge" in config) || !config.llmAsAJudge) { + throw new Error("expected an llmAsAJudge config on the update request"); + } + return config.llmAsAJudge; +} + +describe("updateLlmAsAJudgeEvaluator provider handling", () => { + test("a Bedrock model-only update preserves inferenceConfig and extra fields", async () => { + const { core, sent } = captureUpdate( + bedrockExisting({ + inferenceConfig: { temperature: 0, maxTokens: 512 }, + additionalModelRequestFields: { anthropic_version: "x" }, + }), + ); + await core.eval.updateLlmAsAJudgeEvaluator(ID, { model: "bedrock.model-v2:0" }, OPTIONS); + + const model = llaj(sent()).modelConfig?.bedrockEvaluatorModelConfig; + expect(model?.modelId).toBe("bedrock.model-v2:0"); + expect(model?.inferenceConfig).toEqual({ temperature: 0, maxTokens: 512 }); + expect(model?.additionalModelRequestFields).toEqual({ anthropic_version: "x" }); + }); + + test("an instructions-only update on OpenResponses stays on the responses arm", async () => { + const { core, sent } = captureUpdate( + responsesExisting({ topP: 0.9, reasoning: { effort: "high" } }), + ); + await core.eval.updateLlmAsAJudgeEvaluator(ID, { instructions: "New {context}." }, OPTIONS); + + const config = llaj(sent()).modelConfig!; + expect("responsesEvaluatorModelConfig" in config).toBe(true); + const model = config.responsesEvaluatorModelConfig!; + expect(model.modelId).toBe("openai.gpt-5.4"); + expect(model.maxOutputTokens).toBe(4096); + expect(model.temperature).toBe(0); + expect(model.topP).toBe(0.9); + expect(model.reasoning).toEqual({ effort: "high" }); + expect(llaj(sent()).instructions).toBe("New {context}."); + }); + + test("changing provider without a new model is rejected before the SDK call", async () => { + const { core } = captureUpdate(bedrockExisting()); + await expect( + core.eval.updateLlmAsAJudgeEvaluator(ID, { modelProvider: "OpenResponses" }, OPTIONS), + ).rejects.toBeInstanceOf(InputValidationError); + }); + + test("changing provider with a new model selects the correct arm and defaults", async () => { + const { core, sent } = captureUpdate(bedrockExisting({ inferenceConfig: { temperature: 0 } })); + await core.eval.updateLlmAsAJudgeEvaluator( + ID, + { modelProvider: "OpenResponses", model: "openai.gpt-5.4" }, + OPTIONS, + ); + + const config = llaj(sent()).modelConfig!; + expect("responsesEvaluatorModelConfig" in config).toBe(true); + // The Bedrock inferenceConfig must not leak into the new arm. + expect("bedrockEvaluatorModelConfig" in config).toBe(false); + expect(config.responsesEvaluatorModelConfig).toEqual({ + modelId: "openai.gpt-5.4", + maxOutputTokens: 4096, + temperature: 0, + }); + }); +}); diff --git a/src/handlers/eval/evaluator/llm-as-a-judge/create/index.tsx b/src/handlers/eval/evaluator/llm-as-a-judge/create/index.tsx index e4d643ed9..68ad4b75a 100644 --- a/src/handlers/eval/evaluator/llm-as-a-judge/create/index.tsx +++ b/src/handlers/eval/evaluator/llm-as-a-judge/create/index.tsx @@ -5,7 +5,14 @@ import { JsonRendererKey } from "../../../../../tui"; import { SourceResolver, type AppIO } from "../../../../../io"; import type { Core } from "../../../../types"; import { coreOptsFromCtx, parseJsonFlag } from "../../../../utils"; -import { instructionsFlag, ratingScaleFlag, resolveRatingScale } from "../sharedFlags"; +import { + buildEvaluatorModelConfig, + instructionsFlag, + modelProviderFlag, + ratingScaleFlag, + resolveModelProvider, + resolveRatingScale, +} from "../sharedFlags"; import { LEVELS } from "../../levels"; export const createLlmAsAJudgeCreateHandler = (core: Core, io: AppIO) => @@ -15,7 +22,12 @@ export const createLlmAsAJudgeCreateHandler = (core: Core, io: AppIO) => flags: [ flag("name", "the name of the evaluator", z.string().optional()), flag("level", `evaluation level (${LEVELS.join(" | ")})`, z.enum(LEVELS).optional()), - flag("model", "the Bedrock model ID used to judge", z.string().optional()), + modelProviderFlag, + flag( + "model", + "judge model: a Bedrock model ID / ARN, or an OpenResponses model ID", + z.string().optional(), + ), instructionsFlag, ratingScaleFlag, flag("kms-key-arn", "customer managed KMS key ARN for evaluator data", z.string().optional()), @@ -32,6 +44,7 @@ export const createLlmAsAJudgeCreateHandler = (core: Core, io: AppIO) => throw new InputValidationError("required option '--level ' not specified"); if (!flags["model"]) throw new InputValidationError("required option '--model ' not specified"); + const modelProvider = resolveModelProvider(flags["model-provider"]); const source = new SourceResolver({ stdin: io.stdin }); const instructions = await source.resolveText("instructions", flags["instructions"]); @@ -59,7 +72,7 @@ export const createLlmAsAJudgeCreateHandler = (core: Core, io: AppIO) => llmAsAJudge: { instructions, ratingScale, - modelConfig: { bedrockEvaluatorModelConfig: { modelId: flags["model"] } }, + modelConfig: buildEvaluatorModelConfig(modelProvider, flags["model"]), }, }, kmsKeyArn: flags["kms-key-arn"], diff --git a/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.test.tsx b/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.test.tsx index edce6d1c9..d594e49ca 100644 --- a/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.test.tsx +++ b/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.test.tsx @@ -3,9 +3,10 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { SourceResolver } from "../../../../io"; +import { InputValidationError } from "../../../../errors"; import { testIO } from "../../../../testing"; import { ratingScaleFromPreset } from "../../ratingScale"; -import { resolveRatingScale } from "./sharedFlags"; +import { buildEvaluatorModelConfig, resolveModelProvider, resolveRatingScale } from "./sharedFlags"; // resolveRatingScale is the only branching logic behind --rating-scale: a value is // either a known preset id or a source-aware JSON RatingScale. These cases never @@ -59,3 +60,44 @@ describe("resolveRatingScale", () => { ); }); }); + +describe("resolveModelProvider", () => { + test("defaults to Bedrock when the flag is omitted", () => { + expect(resolveModelProvider(undefined)).toBe("Bedrock"); + }); + + test("accepts the supported providers", () => { + expect(resolveModelProvider("Bedrock")).toBe("Bedrock"); + expect(resolveModelProvider("OpenResponses")).toBe("OpenResponses"); + }); + + test("rejects an unsupported provider before any SDK call", () => { + expect(() => resolveModelProvider("OpenAI")).toThrow(InputValidationError); + }); +}); + +describe("buildEvaluatorModelConfig", () => { + test("Bedrock selects the bedrock arm with only the model id", () => { + expect( + buildEvaluatorModelConfig("Bedrock", "us.anthropic.claude-sonnet-4-5-20250929-v1:0"), + ).toEqual({ + bedrockEvaluatorModelConfig: { modelId: "us.anthropic.claude-sonnet-4-5-20250929-v1:0" }, + }); + }); + + test("OpenResponses selects the responses arm with the deployment defaults and no topP", () => { + const config = buildEvaluatorModelConfig("OpenResponses", "openai.gpt-5.4"); + expect(config).toEqual({ + responsesEvaluatorModelConfig: { + modelId: "openai.gpt-5.4", + maxOutputTokens: 4096, + temperature: 0, + }, + }); + expect("bedrockEvaluatorModelConfig" in config).toBe(false); + expect( + (config as { responsesEvaluatorModelConfig: { topP?: number } }).responsesEvaluatorModelConfig + .topP, + ).toBeUndefined(); + }); +}); diff --git a/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.tsx b/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.tsx index ddcde6242..5a813217a 100644 --- a/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.tsx +++ b/src/handlers/eval/evaluator/llm-as-a-judge/sharedFlags.tsx @@ -1,14 +1,55 @@ import z from "zod"; -import type { RatingScale } from "@aws-sdk/client-bedrock-agentcore-control"; +import type { EvaluatorModelConfig, RatingScale } from "@aws-sdk/client-bedrock-agentcore-control"; import { flag } from "../../../../router"; +import { InputValidationError } from "../../../../errors"; import { parseJsonFlag } from "../../../utils"; import { RATING_SCALE_PRESET_IDS, isRatingScalePreset, ratingScaleFromPreset, } from "../../ratingScale"; +import { + EvaluatorModelProviderSchema, + type EvaluatorModelProvider, +} from "../../../../projectSchemas/evaluator"; import type { SourceResolver } from "../../../../io"; +// The token budget and temperature the old CLI's project deployment applies to +// an OpenResponses judge. topP is deliberately omitted so behavior matches it. +const OPEN_RESPONSES_DEFAULTS = { maxOutputTokens: 4096, temperature: 0 } as const; + +export const modelProviderFlag = flag( + "model-provider", + "model provider for the judge: Bedrock (default) or OpenResponses", + z.string().optional(), +); + +// resolveModelProvider turns the raw --model-provider value into a provider, +// defaulting to Bedrock when the flag is omitted so existing invocations are +// unchanged. +export function resolveModelProvider(value: string | undefined): EvaluatorModelProvider { + if (value === undefined) return "Bedrock"; + const parsed = EvaluatorModelProviderSchema.safeParse(value); + if (!parsed.success) + throw new InputValidationError( + `invalid --model-provider "${value}": expected Bedrock or OpenResponses`, + ); + return parsed.data; +} + +// buildEvaluatorModelConfig selects the SDK modelConfig union arm for the +// resolved provider. OpenResponses carries the deployment token/temperature +// defaults; Bedrock passes the model id alone (the service supplies its own). +export function buildEvaluatorModelConfig( + provider: EvaluatorModelProvider, + modelId: string, +): EvaluatorModelConfig { + if (provider === "OpenResponses") { + return { responsesEvaluatorModelConfig: { modelId, ...OPEN_RESPONSES_DEFAULTS } }; + } + return { bedrockEvaluatorModelConfig: { modelId } }; +} + export const instructionsFlag = flag( "instructions", "evaluation instructions (inline, file://, or - for stdin)", diff --git a/src/handlers/eval/evaluator/llm-as-a-judge/update/index.tsx b/src/handlers/eval/evaluator/llm-as-a-judge/update/index.tsx index f345b0061..598810bbe 100644 --- a/src/handlers/eval/evaluator/llm-as-a-judge/update/index.tsx +++ b/src/handlers/eval/evaluator/llm-as-a-judge/update/index.tsx @@ -5,7 +5,13 @@ import { JsonRendererKey } from "../../../../../tui"; import { SourceResolver, type AppIO } from "../../../../../io"; import type { Core } from "../../../../types"; import { coreOptsFromCtx } from "../../../../utils"; -import { instructionsFlag, ratingScaleFlag, resolveRatingScale } from "../sharedFlags"; +import { + instructionsFlag, + modelProviderFlag, + ratingScaleFlag, + resolveModelProvider, + resolveRatingScale, +} from "../sharedFlags"; export const createLlmAsAJudgeUpdateHandler = (core: Core, io: AppIO) => createHandler({ @@ -14,7 +20,12 @@ export const createLlmAsAJudgeUpdateHandler = (core: Core, io: AppIO) => flags: [ flag("id", "the ID of the evaluator to update", z.string().optional()), instructionsFlag, - flag("model", "the Bedrock model ID used to judge", z.string().optional()), + modelProviderFlag, + flag( + "model", + "judge model: a Bedrock model ID / ARN, or an OpenResponses model ID (required when changing provider)", + z.string().optional(), + ), ratingScaleFlag, flag("kms-key-arn", "customer managed KMS key ARN for evaluator data", z.string().optional()), ], @@ -30,6 +41,12 @@ export const createLlmAsAJudgeUpdateHandler = (core: Core, io: AppIO) => { instructions, model: flags["model"], + // Only forward the provider when the caller set it, so an update that + // leaves it out preserves the evaluator's existing provider. + modelProvider: + flags["model-provider"] !== undefined + ? resolveModelProvider(flags["model-provider"]) + : undefined, ratingScale, kmsKeyArn: flags["kms-key-arn"], }, diff --git a/src/handlers/eval/types.tsx b/src/handlers/eval/types.tsx index d5a252242..06cec2394 100644 --- a/src/handlers/eval/types.tsx +++ b/src/handlers/eval/types.tsx @@ -55,6 +55,7 @@ import type { DataSourceConfig as DataPlaneDataSourceConfig, } from "@aws-sdk/client-bedrock-agentcore"; import type { CoreOptions } from "../../core/types"; +import type { EvaluatorModelProvider } from "../../projectSchemas/evaluator"; // SessionWindow is the resolved explicit time filter for a batch evaluation's // session source (from --start-time/--end-time). Maps directly to the SDK's @@ -129,6 +130,10 @@ export type GetBatchEvaluationResult = { export type LlmAsAJudgeUpdate = { instructions?: string; model?: string; + // The judge model provider to switch to. Undefined preserves the evaluator's + // current provider; a change requires `model`, since model ids are not + // portable between provider APIs. + modelProvider?: EvaluatorModelProvider; ratingScale?: RatingScale; kmsKeyArn?: string; };