Uh oh!
There was an error while loading. Please reload this page.
feat(eval): batch simulate — --ingestion-wait-ms + per-example failures/sessions - #2098
Conversation
…ailures/sessions
Follow-ups from the batch-evaluation simulate review:
- Add --ingestion-wait-ms (default 180000, 0 skips); thread via InvokeDatasetInput.waitIngestionMs.
Removes the SIMULATE_INGESTION_WAIT_MS env var — tests pass the value through the input.
- runExamples now returns per-item failures (item + error), not a bare count + firstError; invokeDataset
surfaces failures: [{ exampleId, error }] so a partial failure names which examples dropped and why.
- batch simulate output renders sessions[] (exampleId <-> sessionId join key for a later get) and
failures[] (omitted when empty).Claude Security Review: no high-confidence findings. (run) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## refactor #2098 +/- ##
============================================
- Coverage 97.33% 97.23% -0.11%
============================================
Files 417 417 Lines 25250 25269 +19 ============================================
- Hits 24578 24570 -8 - Misses 672 699 +27 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Claude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
- inject newSessionId into EvalClient (default randomUUID) so replay fixtures + goldens are deterministic - teach makeRecordingSend to freeze/revive a streaming SDK response (InvokeAgentRuntime), which stringify couldn't serialize - add simulate fixture-golden case; move handler edges to batch-evaluation.test.tsx - split invokeDataset.test.ts into run.test.ts (pool) + load.test.ts (parse + GT-shape); delete it and simulate.test.tsx
Claude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
| ); | ||
| if (r.invoked === 0) { | ||
| const detail = r.firstError ? `; first error: ${r.firstError.message}` : ""; | ||
| const first = r.failures[0]; |
There was a problem hiding this comment.
would it make sense to just join all the errors? Or are these already communicated to the customer somewhere else?
Just wanted to make sure they'd be aware of individual datasets failing even if the overall command doesn't fail.
There was a problem hiding this comment.
Yes all the errors would show in the debug logs. I assumed that if all dataset examples have failed, it would be the same error. However, maybe we should convene these errors directly to the tui or cli. I'll look into this in a follow up
Uh oh!
There was an error while loading. Please reload this page.
| const STREAM_TAG = "$stream"; | ||
| async function freezeStream(response: unknown): Promise<unknown> { |
There was a problem hiding this comment.
took me a sec, but this makes sense and feels like a simple solution.
correct me if I misunderstood, but for streaming apis the response is not json serializable, so we convert the response via the stream tag. In the process, we resolve the whole stream, so we need to convert the data back to a stream before passing it downstream to consumers who are expecting a stream.
nice!
There was a problem hiding this comment.
Yep ur correct. The reviveStream name seems a little weird to me, maybe Ill use mimicStream.
nborges-aws
left a comment
There was a problem hiding this comment.
PR overall LGTM. One concern with a gap in testing. Tests are all either exercise happy path, or mock invokeDataset with a mocked failure object in the response. Nowhere is exercising actual invocation failures assemble that failure object correctly.
The deleted invokeDataset test is actually the only place that exercised this.
notgitika
left a comment
There was a problem hiding this comment.
Left a comment, could be followup
| if (typeof stream?.transformToString !== "function") return response; | ||
| return { | ||
| ...(response as Record<string, unknown>), | ||
| response: { [STREAM_TAG]: await stream.transformToString() }, |
There was a problem hiding this comment.
Could we serialize transformToByteArray() as base64 instead? transformToString() decodes as UTF-8, so binary or invalid UTF-8 runtime responses are changed during replay.
Uh oh!
There was an error while loading. Please reload this page.
jariy17
commented
Aug 27, 2026
I'll address the missing bad paths in a follow up |
What
Batch
simulatereview follow-ups (base:refactor):--ingestion-wait-msflag (default180000,0skips) →InvokeDatasetInput.waitIngestionMs. Drops theSIMULATE_INGESTION_WAIT_MSenv var — value flows through the input only.runExamplesreturnsfailures: {item,error}[];invokeDatasetsurfacesfailures: [{exampleId, error}]— names which examples dropped + why, not just a count.sessions[]+failures[](below).Output
{ "batchEvaluationId": "batch-eval-test", "status": "RUNNING", "examplesInvoked": 1, "examplesFailed": 1, "sessions": [ { "exampleId": "ok1", "sessionId": "s1" } ], "failures": [ { "exampleId": "bad", "error": "HTTP 500" } ] }sessions[]—exampleId ↔ sessionId. Join key: a latereval batch-evaluation getreturnsresults[]keyed bysessionId, mapping each score back to its dataset row.failures[]— always present ([]on a clean run); names which examples dropped + why.