Skip to content

feat(batch-evaluation): add --output-config, rename --qualifier to --endpoint, require timestamp timezones - #2265

Draft
jariy17 wants to merge 1 commit into
feat/eval-cli-router-groupsfrom
feat/batch-eval-output-config
Draft

feat(batch-evaluation): add --output-config, rename --qualifier to --endpoint, require timestamp timezones#2265
jariy17 wants to merge 1 commit into
feat/eval-cli-router-groupsfrom
feat/batch-eval-output-config

Conversation

@jariy17

@jariy17 jariy17 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Renamed --qualifier to --endpoint in agentcore eval batch-evaluation simulate to match other command's runtime-endpoint flag. We also introduce --output-config to agentcore eval batch-evaluation simulate and agentcore eval batch-evaluation evaluate which customer can control where their results go (docs).

simulate resolves output config before invoking

The replay bills the customer's Runtime once per dataset example. A malformed --output-config must surface before the first invocation, not after the whole run — so it's resolved up front, and there's a test asserting invokeDataset is never reached.

Live verification — account 725476964917 (EXPLORE_PROFILE), us-west-2

All three input forms submitted real jobs; the service echoed back exactly what was passed.

Inline JSON, custom dedicated group — job pr3_outputconfig_verify_1788996491-5011fdbc56, COMPLETED:

"outputConfig": {
  "cloudWatchConfig": {
    "logGroupName": "/company/pr3-verify-evaluations",
    "logStreamName": "run-pr3_outputconfig_verify_1788996491-5011fdbc56",
    "metricsNamespace": "Company/PR3Verify",
    "resultDestination": "DEDICATED_LOG_GROUP"
  }
}

The service created /company/pr3-verify-evaluations and a stream named after the job. It held 0 bytes because the target Runtime had no in-scope sessions — the destination wiring is what this proves, not result content.

file://, SOURCE_LOG_GROUP — job pr3_sourcegroup_verify_1788996663-8f6486e573, HTTP 202. No logGroupName echoed back, correct for that destination.

stdin (-) — job pr3_stdin_verify_1788996665-07d97ca802, HTTP 202. metricsNamespace honored; the service filled in its own default dedicated group.

Rejected locally, before any AWS call: malformed --output-config JSON, and --qualifier (now an unknown option).

Cleanup: the /company/pr3-verify-evaluations log group was deleted (confirmed 0 remaining). The three batch-evaluation job records remain — there is no delete API for them, the same limitation the existing fixture suite notes.

Tests

  • src/core/eval.test.ts (new, 2 tests) — this file exists because the other layers left a real gap. The TestCoreClient suites assert what a handler hands to Core; the fixture suites assert whole recorded requests but need an account to re-record. Deleting Core's outputConfig: input.outputConfig line passed every existing test. It stubs only the data client and asserts the constructed StartBatchEvaluationCommand.
  • batch-evaluation.test.tsx (+7) — output config from inline / file / stdin reaching the request unchanged; omitted staying undefined; malformed JSON rejected with an empty Core call log; --endpoint driving both the invocation qualifier and the graded session source; malformed output config aborting before invokeDataset.

Mutation-checked: dropping Core's outputConfig forwarding fails the new Core test.

Verification

  • bun test — 3201 pass, 0 fail
  • bun run typecheck, bun run lint:check, bun run format:check — clean

Follow-up

Fourth PR covers online-eval: --output-config (control-plane shape, deliberately not sharing this PR's data-plane type), --tags on create, --description on update, and widening the CLI-managed execution role so a custom destination is actually writable.

@github-actions github-actions Bot added the size/l PR size: L label Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress and removed agentcore-harness-reviewing AgentCore Harness review in progress labels Sep 9, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgentCore Harness Review

Verdict: Looks good

Nice, well-scoped PR. The three changes are cleanly separated, the tests cover each layer where it can fail (handler, SessionSource timestamp parsing, and — deliberately — the Core→SDK seam that the TestCoreClient and fixture suites can't cover), and the "resolve --output-config before any Runtime invocation" ordering in simulate is exactly the right call for a command that bills per example. The BatchOutputConfig module and long-form parameter help follow existing conventions and are appropriately API-shaped.

No blocking issues. A couple of small notes, take or leave:

  • eval ondemand simulate still uses --qualifier (src/handlers/eval/ondemand/simulate/index.tsx:19,69,92). The PR body justifies the rename by aligning simulate with its own command family (eval), but this sibling command in the same family is left inconsistent. If it's intentionally deferred, a follow-up TODO/issue reference would help; otherwise consider renaming it in the same breaking change so users only see one flag flip.
  • Timestamp regex accepts HH:MM with no seconds (sessionSource.tsx:171) — that's fine and matches ISO-8601, but the error message example (2026-09-01T00:00:00Z) and all tests use the seconds form; worth confirming 2026-09-01T00:00Z is intentionally allowed (looks like it is, and it's a superset of what customers will type).

Live verification, mutation-testing notes, and the explanation for the new src/core/eval.test.ts file (the gap between handler-level TestCoreClient assertions and re-record-only fixture assertions) are all appreciated.

@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.06%. Comparing base (e580808) to head (40cb910).

Additional details and impacted files
@@                     Coverage Diff                      @@
##           feat/eval-cli-router-groups    #2265   +/-   ##
============================================================
  Coverage                        97.06%   97.06%           
============================================================
  Files                              568      569    +1     
  Lines                            39340    39370   +30     
============================================================
+ Hits                             38185    38215   +30     
  Misses                            1155     1155           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jariy17
jariy17 force-pushed the feat/batch-eval-output-config branch from 2a11a5b to ccd369d Compare September 10, 2026 17:07
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 10, 2026
Comment thread src/core/eval.test.ts Outdated
import { createSilentLogger } from "../testing";
import type { OutputConfig } from "@aws-sdk/client-bedrock-agentcore";

// The TestCoreClient suites assert what a handler hands to Core; the fixture

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Golden tests.

@jariy17
jariy17 force-pushed the feat/batch-eval-output-config branch from ccd369d to 290780f Compare September 10, 2026 23:23
@github-actions github-actions Bot added size/m PR size: M and removed size/l PR size: L size/m PR size: M labels Sep 10, 2026
@@ -97,6 +99,8 @@ export const createSimulateBatchEvaluationHandler = (core: Core, _io: AppIO) =>

// Ctrl-C aborts the run (invokes, the ingestion wait, the dataset download).
// TODO(#1986): swap for the shared SIGINT/abort helper once it merges.

@jariy17 jariy17 Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this TODO later in a follow up PR.

Example:
--output-config '{"cloudWatchConfig":{"logGroupName":"/company/agent-evaluations","metricsNamespace":"Company/AgentEvaluations","resultDestination":"DEDICATED_LOG_GROUP"}}'`;

const RESULT_OUTPUT = "Result output:";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this

});
});

describe("eval batch-evaluation evaluate --output-config", () => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use golden tests.

Comment thread src/handlers/eval/types.tsx Outdated
// Already-parsed --ground-truth (SessionMetadataShape[]) → evaluationMetadata.
groundTruth?: SessionMetadataShape[];
kmsKeyArn?: string;
// Already-parsed --output-config, forwarded to the request untouched. Left

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this code comment

@jariy17
jariy17 force-pushed the feat/batch-eval-output-config branch from 290780f to 7a95dc7 Compare September 11, 2026 17:16
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 11, 2026
@jariy17
jariy17 force-pushed the feat/batch-eval-output-config branch from 7a95dc7 to 637cbdf Compare September 11, 2026 20:57
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 11, 2026
… --endpoint

--output-config lets a customer say where results and metrics are written
instead of taking the service-managed default. An API-shaped passthrough:
BatchOutputConfig resolves inline JSON, file://, or stdin and hands the parsed
object to StartBatchEvaluation with field names and nested values untouched.
Field documentation goes in the flag's `help:` block, rendered under "Parameter
details".

`simulate --qualifier` becomes `--endpoint`, matching SessionSource, which has
always called the same concept --endpoint. The InvokeDatasetInput field stays
`qualifier` — that is the Runtime API's name, not ours.

simulate resolves --output-config before invoking the Runtime, since the replay
bills the customer per dataset example and malformed JSON must not surface only
after the whole run.

Dependencies move to @aws-sdk/client-bedrock-agentcore 3.1129.0, the first
release exposing request-side outputConfig on StartBatchEvaluationRequest. The
generated type is used directly; no cast papers over an older model.

src/core/eval.test.ts covers the seam the other suites miss: the TestCoreClient
suites assert what a handler hands to Core, and the fixture suites need an
account to re-record. Deleting Core's outputConfig forwarding passed every test
until this one existed.

Timestamp handling is unchanged. An earlier revision required an explicit
timezone on --start-time/--end-time; that is a breaking change and has been
dropped.
@jariy17
jariy17 force-pushed the feat/batch-eval-output-config branch from 637cbdf to 40cb910 Compare September 11, 2026 21:10
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 11, 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.

2 participants