From 551e07be3c1b22791992e4ce172770bdae98fa5f Mon Sep 17 00:00:00 2001 From: jariy17 Date: Mon, 14 Sep 2026 21:54:46 +0000 Subject: [PATCH] feat(project): add --model-provider to project add evaluator llm-as-a-judge Support choosing the judge model provider on `agentcore project add evaluator llm-as-a-judge`. Bedrock stays the default and remains implicit in agentcore.json so existing projects are unchanged; OpenResponses is persisted explicitly as `modelProvider`. - Extend the evaluator project schema with EvaluatorModelProviderSchema, a provider-neutral model-id schema, and optional modelProvider on LlmAsAJudgeConfigSchema (kept optional for backward compatibility). - Validate the model per resolved provider: Bedrock via isValidBedrockModelId, OpenResponses as a non-empty printable identifier without spaces. --- .../evaluator/llm-as-a-judge/index.test.ts | 165 ++++++++++++++++++ .../add/evaluator/llm-as-a-judge/index.ts | 43 ++++- src/projectSchemas/evaluator.test.ts | 33 ++++ src/projectSchemas/evaluator.ts | 20 ++- 4 files changed, 254 insertions(+), 7 deletions(-) diff --git a/src/handlers/project/add/evaluator/llm-as-a-judge/index.test.ts b/src/handlers/project/add/evaluator/llm-as-a-judge/index.test.ts index 0fbbb40f9..1f7a10d1b 100644 --- a/src/handlers/project/add/evaluator/llm-as-a-judge/index.test.ts +++ b/src/handlers/project/add/evaluator/llm-as-a-judge/index.test.ts @@ -64,6 +64,171 @@ describe("project add evaluator llm-as-a-judge", () => { expect(evaluator.config.llmAsAJudge.ratingScale.categorical).toBeUndefined(); }); + test("omits modelProvider for the default Bedrock provider", async () => { + const { projectRoot, cleanup } = await initProject(); + cleanups.push(cleanup); + await run([ + "add", + "evaluator", + "llm-as-a-judge", + "--name", + "bedrock_default", + "--level", + "SESSION", + "--model", + MODEL, + "--instructions", + "Judge the answer.", + "--rating-scale", + "pass-fail", + ]); + + const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + const llaj = spec.evaluators.find((e: { name: string }) => e.name === "bedrock_default").config + .llmAsAJudge; + expect(llaj.model).toBe(MODEL); + expect(llaj.modelProvider).toBeUndefined(); + }); + + test("explicit Bedrock behaves identically to the default", async () => { + const { projectRoot, cleanup } = await initProject(); + cleanups.push(cleanup); + await run([ + "add", + "evaluator", + "llm-as-a-judge", + "--name", + "bedrock_explicit", + "--level", + "SESSION", + "--model-provider", + "Bedrock", + "--model", + MODEL, + "--instructions", + "Judge the answer.", + "--rating-scale", + "pass-fail", + ]); + + const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + const llaj = spec.evaluators.find((e: { name: string }) => e.name === "bedrock_explicit").config + .llmAsAJudge; + expect(llaj.modelProvider).toBeUndefined(); + }); + + test("persists modelProvider for OpenResponses", async () => { + const { projectRoot, cleanup } = await initProject(); + cleanups.push(cleanup); + await run([ + "add", + "evaluator", + "llm-as-a-judge", + "--name", + "openresponses", + "--level", + "SESSION", + "--model-provider", + "OpenResponses", + "--model", + "openai.gpt-5.4", + "--instructions", + "Judge the answer using {context}.", + "--rating-scale", + "pass-fail", + ]); + + const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + const llaj = spec.evaluators.find((e: { name: string }) => e.name === "openresponses").config + .llmAsAJudge; + expect(llaj.modelProvider).toBe("OpenResponses"); + expect(llaj.model).toBe("openai.gpt-5.4"); + }); + + test.each<[string, string[]]>([ + [ + "invalid --model-provider", + [ + "--name", + "x", + "--level", + "SESSION", + "--model-provider", + "OpenAI", + "--model", + MODEL, + "--instructions", + "i", + "--rating-scale", + "pass-fail", + ], + ], + [ + "invalid Bedrock --model", + [ + "--name", + "x", + "--level", + "SESSION", + "--model", + "not a model", + "--instructions", + "i", + "--rating-scale", + "pass-fail", + ], + ], + [ + "invalid OpenResponses --model (contains a space)", + [ + "--name", + "x", + "--level", + "SESSION", + "--model-provider", + "OpenResponses", + "--model", + "bad model", + "--instructions", + "i", + "--rating-scale", + "pass-fail", + ], + ], + ])("rejects %s", async (_label, flags) => { + const { cleanup } = await initProject(); + cleanups.push(cleanup); + await expect(run(["add", "evaluator", "llm-as-a-judge", ...flags])).rejects.toBeInstanceOf( + InputValidationError, + ); + }); + + test("accepts a valid OpenResponses model id", async () => { + const { projectRoot, cleanup } = await initProject(); + cleanups.push(cleanup); + await run([ + "add", + "evaluator", + "llm-as-a-judge", + "--name", + "or_ok", + "--level", + "SESSION", + "--model-provider", + "OpenResponses", + "--model", + "openai.gpt-5.4", + "--instructions", + "Judge using {context}.", + "--rating-scale", + "pass-fail", + ]); + const spec = await Bun.file(join(projectRoot, "agentcore", "agentcore.json")).json(); + expect( + spec.evaluators.find((e: { name: string }) => e.name === "or_ok").config.llmAsAJudge.model, + ).toBe("openai.gpt-5.4"); + }); + test("writes a categorical preset evaluator", async () => { const { projectRoot, cleanup } = await initProject(); cleanups.push(cleanup); diff --git a/src/handlers/project/add/evaluator/llm-as-a-judge/index.ts b/src/handlers/project/add/evaluator/llm-as-a-judge/index.ts index e8ba98de8..b29a4a20b 100644 --- a/src/handlers/project/add/evaluator/llm-as-a-judge/index.ts +++ b/src/handlers/project/add/evaluator/llm-as-a-judge/index.ts @@ -3,9 +3,12 @@ import { createHandler, flag, ProjectKey } from "../../../../../router"; import { InputValidationError } from "../../../../../errors"; import { SourceResolver } from "../../../../../io"; import { + EvaluatorModelProviderSchema, EvaluatorSchema, isValidBedrockModelId, + isValidOpenResponsesModelId, RatingScaleSchema, + type EvaluatorModelProvider, type RatingScale, } from "../../../../../projectSchemas/evaluator"; import { TagsSchema } from "../../../../../projectSchemas/tags"; @@ -26,9 +29,14 @@ export const createAddLlmAsAJudgeEvaluatorHandler = (config: AddProjectResourceC flags: [ flag("name", "the name of the evaluator", z.string().optional()), flag("level", "what to score: SESSION, TRACE, or TOOL_CALL", z.string().optional()), + flag( + "model-provider", + "model provider for the judge: Bedrock (default) or OpenResponses", + z.string().optional(), + ), flag( "model", - "Bedrock model ID or inference-profile/foundation-model ARN for the judge", + "judge model: a Bedrock model ID / inference-profile-or-foundation-model ARN, or an OpenResponses model ID", z.string().optional(), ), flag( @@ -65,10 +73,8 @@ export const createAddLlmAsAJudgeEvaluatorHandler = (config: AddProjectResourceC "required option '--rating-scale ' not specified", ); - if (!isValidBedrockModelId(flags["model"])) - throw new InputValidationError( - `invalid --model "${flags["model"]}": expected a Bedrock model ID (e.g. anthropic.claude-3-5-sonnet-20240620-v1:0) or an inference-profile/foundation-model ARN`, - ); + const modelProvider = resolveModelProvider(flags["model-provider"]); + validateModel(modelProvider, flags["model"]); const ratingScale = resolveRatingScale(flags["rating-scale"]); @@ -81,6 +87,9 @@ export const createAddLlmAsAJudgeEvaluatorHandler = (config: AddProjectResourceC description: flags["description"], config: { llmAsAJudge: { + // Bedrock is the default and stays implicit so existing Bedrock + // agentcore.json files are unchanged; only OpenResponses is written. + ...(modelProvider === "OpenResponses" ? { modelProvider } : {}), model: flags["model"], instructions, ratingScale, @@ -107,6 +116,30 @@ export const createAddLlmAsAJudgeEvaluatorHandler = (config: AddProjectResourceC }, }); +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; +} + +function validateModel(provider: EvaluatorModelProvider, model: string): void { + if (provider === "Bedrock") { + if (!isValidBedrockModelId(model)) + throw new InputValidationError( + `invalid --model "${model}": expected a Bedrock model ID (e.g. anthropic.claude-3-5-sonnet-20240620-v1:0) or an inference-profile/foundation-model ARN`, + ); + return; + } + if (!isValidOpenResponsesModelId(model)) + throw new InputValidationError( + `invalid --model "${model}": expected an OpenResponses model ID (a non-empty identifier without spaces, e.g. openai.gpt-5.4)`, + ); +} + // A preset name expands to a fresh copy of the shared table; anything else is // treated as an inline JSON rating scale and validated against the schema. function resolveRatingScale(value: string): RatingScale { diff --git a/src/projectSchemas/evaluator.test.ts b/src/projectSchemas/evaluator.test.ts index 9180c8260..390dfd196 100644 --- a/src/projectSchemas/evaluator.test.ts +++ b/src/projectSchemas/evaluator.test.ts @@ -2,9 +2,12 @@ import { describe, expect, it } from "bun:test"; import { CodeBasedConfigSchema, EvaluatorConfigSchema, + EvaluatorModelProviderSchema, + LlmAsAJudgeConfigSchema, RatingScaleSchema, isValidBedrockModelId, isValidKmsKeyArn, + isValidOpenResponsesModelId, } from "./evaluator"; const numerical = [{ value: 1, label: "bad", definition: "Bad response" }]; const categorical = [{ label: "pass", definition: "Passes" }]; @@ -29,6 +32,36 @@ describe("evaluator custom validation", () => { expect(EvaluatorConfigSchema.safeParse({ llmAsAJudge }).success).toBe(true); expect(EvaluatorConfigSchema.safeParse({ llmAsAJudge, codeBased }).success).toBe(false); }); + it("keeps modelProvider optional and defaults existing Bedrock configs unchanged", () => { + const bedrockNoProvider = { + model: "us.anthropic.claude-sonnet-4-5-20250929-v1:0", + instructions: "Judge", + ratingScale: { numerical }, + }; + expect(LlmAsAJudgeConfigSchema.safeParse(bedrockNoProvider).success).toBe(true); + expect( + LlmAsAJudgeConfigSchema.safeParse({ ...bedrockNoProvider, modelProvider: "Bedrock" }).success, + ).toBe(true); + expect( + LlmAsAJudgeConfigSchema.safeParse({ + modelProvider: "OpenResponses", + model: "openai.gpt-5.4", + instructions: "Judge", + ratingScale: { numerical }, + }).success, + ).toBe(true); + }); + it("rejects unknown model providers", () => { + expect(EvaluatorModelProviderSchema.safeParse("Bedrock").success).toBe(true); + expect(EvaluatorModelProviderSchema.safeParse("OpenResponses").success).toBe(true); + expect(EvaluatorModelProviderSchema.safeParse("OpenAI").success).toBe(false); + }); + it("validates OpenResponses model ids as non-empty printable identifiers without spaces", () => { + expect(isValidOpenResponsesModelId("openai.gpt-5.4")).toBe(true); + expect(isValidOpenResponsesModelId("some/model-id_v2")).toBe(true); + expect(isValidOpenResponsesModelId("has space")).toBe(false); + expect(isValidOpenResponsesModelId("")).toBe(false); + }); it("validates model identifiers and KMS key ARNs through owned helpers", () => { expect(isValidBedrockModelId("anthropic.claude-v2:1")).toBe(true); expect(isValidBedrockModelId("us.anthropic.claude-sonnet-4-5-20250929-v1:0")).toBe(true); diff --git a/src/projectSchemas/evaluator.ts b/src/projectSchemas/evaluator.ts index df65622c7..30c269bca 100644 --- a/src/projectSchemas/evaluator.ts +++ b/src/projectSchemas/evaluator.ts @@ -45,9 +45,25 @@ const BEDROCK_ARN_PATTERN = export function isValidBedrockModelId(value: string): boolean { return BEDROCK_MODEL_ID_PATTERN.test(value) || BEDROCK_ARN_PATTERN.test(value); } -export const BedrockModelIdSchema = z.string().min(1, "Model ID is required"); +// The evaluator's judge model provider. Bedrock is the default and stays +// implicit in project config; OpenResponses is persisted explicitly. +export const EvaluatorModelProviderSchema = z.enum(["Bedrock", "OpenResponses"]); +export type EvaluatorModelProvider = z.infer; +// OpenResponses model ids are opaque, provider-defined strings, so we only +// require a non-empty printable identifier with no spaces rather than enforce a +// catalog (a hard-coded list would reject valid models the moment the provider +// adds one). +const OPEN_RESPONSES_MODEL_ID_PATTERN = /^[\x21-\x7e]+$/; +export function isValidOpenResponsesModelId(value: string): boolean { + return OPEN_RESPONSES_MODEL_ID_PATTERN.test(value); +} +// Provider-neutral: the schema stores whatever model id the resolved provider +// accepts; per-provider format checks live in the handler where the provider is +// known. +export const EvaluatorModelIdSchema = z.string().min(1, "Model ID is required"); export const LlmAsAJudgeConfigSchema = z.object({ - model: BedrockModelIdSchema, + modelProvider: EvaluatorModelProviderSchema.optional(), + model: EvaluatorModelIdSchema, instructions: z.string().min(1, "Evaluation instructions are required"), ratingScale: RatingScaleSchema, });