Skip to content

feat(eval): add --model-provider to imperative evaluator create/update - #2299

Draft
jariy17 wants to merge 1 commit into
model-provider-projectfrom
model-provider-imperative
Draft

jariy17 wants to merge 1 commit into
model-provider-projectfrom
model-provider-imperative

Conversation

@jariy17

@jariy17 jariy17 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What

Adds --model-provider (Bedrock default / OpenResponses) to the imperative agentcore eval evaluator llm-as-a-judge create and update commands, and makes update provider-safe.

Part 2 of 2 — stacked on #2298. Base this on #2298; it will retarget to refactor once #2298 merges.

Changes

  • Shared module (sharedFlags.tsx): modelProviderFlag, resolveModelProvider, and buildEvaluatorModelConfig (picks the SDK modelConfig union arm). OpenResponses carries maxOutputTokens: 4096, temperature: 0, and omits topP to match the old CLI's project deployment defaults.
  • create: register the flag, default Bedrock, build the correct union arm.
  • update (update/index.tsx, core/eval.tsx, handlers/eval/types.tsx): the merge is now provider-aware — it detects and preserves the existing union arm and its tuning (Bedrock inferenceConfig, OpenResponses temperature/topP/reasoning), overlays only supplied values, and supports an explicit provider change. Changing provider requires a new --model, since model ids are not portable between provider APIs.

Why the update change matters

Before this, update only understood bedrockEvaluatorModelConfig. Once OpenResponses evaluators exist, an instructions-only update would fail, and a model update could silently convert the evaluator to Bedrock and drop its tuning.

Verification

  • bun test .../sharedFlags.test.tsx .../core/evalEvaluatorProvider.test.tsx — 15 pass (provider resolution, arm construction, and the update-safety matrix: model-only Bedrock preserves tuning; instructions-only OpenResponses stays on the responses arm; provider switch requires a model; switch selects the correct arm without leaking the old tuning).
  • Existing fixture-backed evaluator.test.tsx — 32 pass (Bedrock path unchanged).
  • bun run typecheck + bun run lint:check clean; create/update --help list both providers.

Follow-ups

  • OpenResponses lifecycle service fixtures for evaluator.test.tsx require recording against an account with OpenResponses access (bug-bash item).
  • CDK synthesis of modelProvider: OpenResponses is handled by @aws/agentcore-cdk in the generated app; deploy-time verification in the bug-bash notes.

Bug-bash recording against the explore account to follow in a comment.

@jariy17

jariy17 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Bug bash — imperative evaluator commands (live, sandbox account, us-west-2)

Recorded with the TUI harness against a live AgentCore account. All cases verified end-to-end; the created evaluators were deleted afterward (no residue).

# Case Result
1 create --model-provider OpenAI (unsupported) ❌ fails fast, before any AWS call: invalid --model-provider "OpenAI": expected Bedrock or OpenResponses
2 create --model-provider OpenResponses --model openai.gpt-5.4 ✅ ACTIVE; service stores responsesEvaluatorModelConfig with maxOutputTokens: 4096, temperature: 0, no topP
3 instructions-only update on the OpenResponses evaluator ✅ stays on the responses arm — not silently converted to Bedrock, tuning preserved
4 update --model-provider Bedrock (no new model) ❌ rejected: changing provider requires a new --model (model ids are not portable across provider APIs)
5 provider switch with a new --model ✅ selects the target arm and does not leak the previous provider's tuning

Baseline Bedrock create/get/update/delete unchanged and green.

A screen recording of this run is archived internally (Artifactory); it is not linked here because this repository is public.

@jariy17
jariy17 added this pull request to stack #2302 September 14, 2026 22:32
Support the OpenResponses judge provider on the imperative evaluator commands,
matching `project add evaluator llm-as-a-judge`. Bedrock stays the default.

- Shared module: modelProviderFlag, resolveModelProvider, and
  buildEvaluatorModelConfig, which selects the SDK modelConfig union arm.
  OpenResponses carries the deployment defaults (maxOutputTokens 4096,
  temperature 0) and omits topP to match the old CLI's project deployment.
- create: register --model-provider, default Bedrock, build the correct arm.
- update: make the merge provider-aware — detect and preserve the existing
  union arm and its tuning, overlay only supplied values, and support an
  explicit provider change (which requires a new --model, since model ids are
  not portable between provider APIs). Add --model-provider for CRUD parity.
@jariy17
jariy17 force-pushed the model-provider-imperative branch from 08361c7 to 1a0163c Compare September 15, 2026 18:41
@github-actions github-actions Bot added the size/m PR size: M label Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant