Repository navigation
fix(eval): add online logs and include batch evaluation errors in get - #2582
Hweinstock wants to merge 11 commits into
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
The new eval online-eval logs and eval batch-evaluation logs subcommands are a clean reuse of createLogsHandler and mirror the existing harness-logs pattern. The log-source resolution in EvalClient.evaluationLogSource correctly layers (1) explicit CloudWatch output log group, (2) rejection of non-CloudWatch output configs, (3) the service default log group, and (4) the SOURCE_LOG_GROUP fall-through — including the one-hop indirection through onlineEvaluationConfigSource for batch evaluations spawned by an AB test. Error types (InputValidationError vs. MalformedServiceResponseError) are used appropriately.
Nothing I'd block the merge on. A couple of optional follow-ups the author may want to consider:
src/core/eval.tsx—evaluationLogSourcehas several branches (default/explicit/source-group, with and without the AB-test indirection) but no unit tests. The manual verification in the PR description covers the paths well; a small table-driven test around this method would make future refactors safer. Not a blocker — the file has no existing unit test to extend.src/handlers/eval/logs/index.tsx— typingresolveLogSourceasCoreEvalClient["resourceBatchEvaluationLogSource"]works only because both resolver signatures are identical today. A named shared type (e.g.,(id: string, options: CoreOptions) => Promise<LogSource>) would read more clearly, but it's just style.
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2582 +/- ##
=========================================
Coverage 97.52% 97.52%
=========================================
Files 634 635 +1
Lines 47260 47328 +68
=========================================
+ Hits 46090 46158 +68
Misses 1170 1170 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
| }); | ||
| } | ||
|
|
||
| async resourceBatchEvaluationLogSource( |
There was a problem hiding this comment.
Do we need batch-evaluation logs? batch-evaluation get already returns the log results?
• get reads the job's own stream (logGroupName + logStreamName) and parses each result (src/core/eval.tsx:518-560).
However, it doesn't return errors from the logGroup like this one:
message: {"name":"gen_ai.evaluation.result","attributes":{"error.type":"LogEventMissingException","error.message":"Session span data is incomplete. Span with ID: <span-id> and name: invoke_agent Strands Agents is missing a corresponding log event. Please retry the request with the complete span data"}}
We should expand this command to return errors like this.
| const { detail: batchEvaluation } = await this.getBatchEvaluation(batchEvaluationId, options, { | ||
| includeResults: false, | ||
| }); | ||
| return this.evaluationLogSource({ |
There was a problem hiding this comment.
This will show every job in that log group. You need to add a filter in the cloudwatch query or filter by logStream because each job get their own logstream.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
Superseded by #2588, authored by @jariy17, with the same changes. Review history is retained here.
What is this PR doing?
Adds
agentcore eval online-eval logsso customers can stream or search evaluation logs using an online evaluation configuration ID. It follows the existing log command conventions: tail by default, search with--since/--until, filter with--level/--query, limit search results with--limit, and emit JSON Lines with--json.Also expands
agentcore eval batch-evaluation getto return evaluation errors alongside scores, including failures that have no evaluator ID. Batch results and errors are retrieved throughget; this PR does not add a separatebatch-evaluation logscommand.Why is this PR doing this?
Online evaluations run continuously. Customers need to inspect their output from the CLI without looking up the CloudWatch log group themselves, whether results go to a dedicated group or the configured source group.
Batch evaluations already expose results through
get, but the parser dropped failures without an evaluator name and omitted the standarderror.messageanderror.typefields. A failed or partially successful job could therefore hide the information needed to diagnose it. Returning scores and errors together keeps the output tied to the requested job's exact log stream; searching a shared output group could mix records from different jobs.How is this PR doing this?
logshandler underonline-eval/logsand reuses the existing shared log handler and CloudWatch search/tail implementation.SOURCE_LOG_GROUP. Returns actionable errors for unsupported output configurations and ambiguous source groups. The resolver stays specific to online evaluations.error.messageanderror.type, retaining legacygen_ai.evaluation.errorsupport and scope identifiers. Unnamed errors must be evaluation-result records, so unrelated application errors are excluded.--disable-cw-results, and nonfatal warnings when CloudWatch results cannot be read.Testing
Verified the built Node CLI against live AWS in
us-east-1on October 13, 2026:Builtin.Correctnessscore. Recorded these calls for golden tests that replay offline.Rechecked all four batch cases after the final test changes. Confirmed metadata-only output for each case and failures for a missing
--id, a nonexistent job, and the removed batchlogscommand. The first mixed-job attempt hit log ingestion delay; rerunning after ingestion produced the mixed outcome above.All local checks passed:
bun test(4,041 passed, 0 failed),bun run typecheck,bun run lint:check,bun run format:check,bun run secrets:check, andbun run build. New recordings are sanitized and replay offline. The live checks exercise the changed AWS interaction; no deployment E2E suite was run locally.Captured output
These screenshots render captured CLI stdout.
jqselects the fields shown; job and span identifiers are redacted.Successful evaluation
Evaluation errors
Mixed results in a custom output group