feat(eval): ondemand simulate — replay a dataset, evaluate synchronously - #2071
feat(eval): ondemand simulate — replay a dataset, evaluate synchronously#2071jariy17 wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2071 +/- ##
============================================
+ Coverage 97.19% 97.21% +0.02%
============================================
Files 471 472 +1
Lines 28731 28857 +126
============================================
+ Hits 27925 28054 +129
+ Misses 806 803 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
634f6f9 to
94a16ac
Compare
94a16ac to
623bbfc
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
ec6636c to
f6776cc
Compare
|
Claude Security Review: no high-confidence findings. (run) |
f6776cc to
c29ad31
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| ); | ||
| } | ||
|
|
||
| const controller = new AbortController(); |
There was a problem hiding this comment.
We should use the shared withUserCancellation helper here
There was a problem hiding this comment.
Good Idea, Ill update it in my pr.
| }, | ||
| }); | ||
|
|
||
| function toReferenceInputs(s: InvokedSession): EvaluationReferenceInput[] { |
There was a problem hiding this comment.
are we intentionally omitting expectedResponses here? Without it a dataset that only returns expected responses won't have reference inputs
There was a problem hiding this comment.
Ah nice catch. I didn't realized it was missing expectedResponses. I'll update my pr for this.
| { | ||
| agent: flags["runtime-id"], | ||
| endpoint: flags["qualifier"], | ||
| sessionIds: replay.sessions.map((s) => s.sessionId), |
There was a problem hiding this comment.
If the dataset is very large and has lots of session Ids, would we need batching here?
There was a problem hiding this comment.
Yes but I expect customers to use batch-evaluations simulate for heavier loads. We can add batching here if customers want it in the future.
… expectedResponse as a reference input Addresses review on #2071: swap the hand-rolled SIGINT/AbortController for the shared withUserCancellation helper, and include per-turn expectedResponse in the Evaluate reference inputs so an expected-response-only dataset still contributes ground truth.
|
Claude Security Review: no high-confidence findings. (run) |
…late turn→traceId) Bug bash against a live agent showed the Evaluate API rejects expectedResponse under a session-only context. Correlate each turn's expectedResponse to its trace id instead, and extend the fixture golden dataset with an expected_response turn graded by Builtin.Correctness so the real Evaluate call validates the mapping at record time.
|
Claude Security Review: no high-confidence findings. (run) |
| const serviceName = runtimeServiceName(runtimeName, qualifier); | ||
|
|
||
| const endMs = input.window ? +input.window.endTime : Date.now(); | ||
| const endMs = input.window ? +input.window.endTime : this.now(); |
There was a problem hiding this comment.
why did we make this change just curious
There was a problem hiding this comment.
For the Golden tests, it requires the inputs to be deterministic. However, Date.now() is not deterministic. Therefore, I needed to inject a now function that could mocked by our unit tests. Look at our unit tests for this.
| 'JSON payload template; {input} is the scenario input, e.g. {"prompt":"{input}"}', | ||
| z.string().optional(), | ||
| ), | ||
| flag("header", "an ordered application header (repeatable)", z.array(z.string()).optional()), |
There was a problem hiding this comment.
wdym repeatable?
like --header <> --header <> ?
There was a problem hiding this comment.
Yes, its repeatable like that.
| ...(gt.expectedTrajectory && { expectedTrajectory: gt.expectedTrajectory }), | ||
| }); | ||
| } | ||
| (gt.turns ?? []).forEach((turn, i) => { |
There was a problem hiding this comment.
how confident are we that traceIds always has exactly one entry per term in the expected order?
could partial or extra telemetry shift this mapping and attach an expected response to the wrong trace?
There was a problem hiding this comment.
actually I just checked this:
see L15 where it says the user prompts "hi" when in fact we prompted "what is 2+2"
There was a problem hiding this comment.
Currently, we assume that each traceId is exactly one entry per turn in an expected order. HOWEVER, you did found issue with our golden test.
Our golden tests uses the same session ids. Therefore, we could be invoking an existing session with the same conversation. To fix this, I'm creating a function that returns a unique identifier when running unit tests and generates a new one when we are recording.
…fix stale-trace pollution)
|
Claude Security Review: no high-confidence findings. (run) |
What
eval ondemand simulate— replay a dataset against a runtime, then evaluate the sessions synchronously, client-side (scores print inline). The on-demand twin ofbatch-evaluation simulate.Pipeline:
invokeDataset(replay) →getTracesForAgent(CloudWatch) →evaluate(Evaluate API).Follows the batch-evaluation simulate pattern
--ingestion-wait-ms(default 180000; 0 to skip)invokeDatasetcompositionfailures[0])sessions[](exampleId ↔ sessionId) +failures[]Differs by design: no
--name/--kms-key-arn(no async job); output is inline scores, not a job id.Output
{ "sessionsEvaluated": 1, "results": [ { "evaluatorId": "Builtin.Helpfulness", "value": 0.83, "label": "Very Helpful", "explanation": "…", "tokenUsage": {…} } ], "examplesInvoked": 1, "examplesFailed": 0, "sessions": [ { "exampleId": "greet", "sessionId": "…" } ], "failures": [] }Tests (batch pattern)
ondemand.test.tsx(TestCoreClient): required-flag validation, refuse-when-nothing-invoked,--ingestion-wait-mspassthrough.ondemand.fixture.test.tsx: real invoke → CloudWatch traces → Evaluate, recorded against a live agent, replayed offline viamatchGolden. Deterministic via the injectednewSessionIdseam plus anow()clock seam — on-demand's trace-query window is otherwiseDate.now()-based, so itsStartQueryfixture key would drift between record and replay (batch has no client query;evaluatepins an explicit--start-time/--end-time).Notes
ignoredReferenceInputFields); the handler still forwards them (adapter exercised).refactor(was stacked on the pre-merge batch-simulate work).