Skip to content

feat(evaluation): add KMS, tags, online data source, and updated_at to batch eval - #533

Merged
jariy17 merged 2 commits into
mainfrom
feat/batch-eval-kms-online-source
Jun 23, 2026
Merged

feat(evaluation): add KMS, tags, online data source, and updated_at to batch eval#533
jariy17 merged 2 commits into
mainfrom
feat/batch-eval-kms-online-source

Conversation

@jariy17

@jariy17 jariy17 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

What

Expose batch-evaluation API parameters that StartBatchEvaluation / GetBatchEvaluation already support but the SDK did not surface:

  • BatchEvaluationRunConfig.kms_key_arnStartBatchEvaluation.kmsKeyArn — encrypt evaluation data at rest with a customer-managed KMS key.
  • BatchEvaluationRunConfig.tagsStartBatchEvaluation.tags.
  • OnlineEvaluationDataSourceConfig — new DataSourceConfig subclass emitting onlineEvaluationConfigSource (onlineEvaluationConfigArn + optional sessionFilterConfig), to evaluate sessions captured by an existing OnlineEvaluationConfig.
  • BatchEvaluationResult.kms_key_arn and .updated_at — populated from the GetBatchEvaluation response.
  • Bump boto3/botocore floor to 1.43.31, the first release whose bedrock-agentcore model includes these parameters.
  • Export OnlineEvaluationDataSourceConfig from bedrock_agentcore.evaluation.

Why

These are existing, optional service parameters that the runner silently dropped. KMS support in particular is a common requirement for customers with data-at-rest encryption mandates. All additions are backward compatible: every new field is optional and omitted from the request when unset.

Testing

  • Unit tests added for each path (KMS/tags sent & omitted, result surfacing, online-source to_data_source_config with/without time range, ARN validation, updated_at surfacing). Full evaluation suite: 359 passed (2 pre-existing collection errors in optional strands-agents-evals integration tests, unrelated to this change).
  • ruff check / ruff format: clean.
  • Manual verification against live bedrock-agentcore (us-west-2):
    • updated_at — a real GetBatchEvaluation response was parsed by the result model; updated_at populated correctly. ✅
    • kms_key_arn — a real StartBatchEvaluation call accepted and acted on kmsKeyArn; it reached the service's KMS-encrypt step (request rejected only because the test key's policy lacks kms:GenerateDataKey for the service role — i.e. the parameter is wired through and honored end-to-end). ✅
    • online source / botocore param validation — verified via botocore.Stubber against the real service model. ✅

Note: the live kms_key_arn test stopped at the KMS key-policy permission boundary by design — a full green run needs a key whose policy grants the AgentCore service role kms:GenerateDataKey.

…o batch eval

Surface batch-evaluation API parameters that StartBatchEvaluation/
GetBatchEvaluation support but the SDK did not expose:

- BatchEvaluationRunConfig.kms_key_arn -> StartBatchEvaluation kmsKeyArn,
  for encrypting evaluation data at rest with a customer-managed key.
- BatchEvaluationRunConfig.tags -> StartBatchEvaluation tags.
- OnlineEvaluationDataSourceConfig: new DataSourceConfig that emits
  onlineEvaluationConfigSource (onlineEvaluationConfigArn + optional
  sessionFilterConfig) to evaluate sessions captured by an existing
  OnlineEvaluationConfig.
- BatchEvaluationResult.kms_key_arn and .updated_at, populated from the
  GetBatchEvaluation response.

Bump boto3/botocore floor to 1.43.31, the first release whose
bedrock-agentcore model includes these parameters.

Adds unit tests for each path and exports
OnlineEvaluationDataSourceConfig from bedrock_agentcore.evaluation.
@github-actions

Copy link
Copy Markdown
Contributor

✅ No Breaking Changes Detected

No public API breaking changes found in this PR.

@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 17, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review did not actually analyze this PR (model took 0 turns — the skill likely failed during setup). See the run for details; a later push or re-run is needed for a real review.

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 17, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review did not analyze this PR (model took 0 turns). See the run for details; a later push or re-run is needed.

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review did not analyze this PR (model took 0 turns). See the run for details; a later push or re-run is needed.

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026

@notgitika notgitika 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.

looks good to me, it is backwards compatible and just config changes. curious to see if you think adding integ tests here would be useful

Comment thread pyproject.toml
Comment on lines -29 to -30
"boto3>=1.43.0",
"botocore>=1.43.0",

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.

can we remove the dependency bump from this PR?

@jariy17 jariy17 Jun 23, 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.

What's wrong with a dependency bump here? We need an updated boto3 so the new parameters shows up.

polling_timeout_seconds: int = 1800
polling_interval_seconds: int = 30
simulation_config: Optional[SimulationConfig] = None
kms_key_arn: Optional[str] = None

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.

maybe adding some client side validation on the kms key shape could be good

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.

Good Idea but if a customers put an invalid arn here, the service will return a ValidationException.

@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 23, 2026
@jariy17
jariy17 merged commit de79b6b into main Jun 23, 2026
49 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants