chore(report): always return flattened report - #241
Conversation
|
Assessment: Approve Clean breaking change that simplifies the public API by eliminating the need for callers to index into a list or flatten manually. The implementation correctly handles single-evaluator (passthrough), multi-evaluator (flatten + tag), and zero-evaluator (empty report) cases. Review Details
Well-scoped change with clear migration docs and consistent handling across the Experiment, ChaosExperiment, and RedTeamExperiment subclasses. |
2508b4c to
d16a009
Compare
|
Assessment: Request Changes Good simplification of the public API. One critical bug in the new integration test file and a minor docstring inconsistency. Review Details
|
d16a009 to
9bf4f20
Compare
9bf4f20 to
65507c7
Compare
|
Assessment: Request Changes Clean breaking change that simplifies the public API — callers no longer need to index Review Details
Minor: PR title has a typo ("reprot" → "report"). |
|
Assessment: Approve Well-executed breaking change that simplifies the public API surface. No substantive issues found. Review Details
Note on prior reviews: The previous review comments flagging Minor: PR title has a typo ("reprot" → "report"). |
65507c7 to
c1c0be8
Compare
|
Assessment: Approve Clean breaking change that simplifies the public API by returning a single Review Details
Note on prior reviews: The two "Request Changes" reviews flagging Minor: PR title typo ("reprot" → "report"). |
jjbuck
left a comment
There was a problem hiding this comment.
Non-blocking nit: there's the possibility for evaluator name collision in the flattened evaluation report unless we add a first class "name" field to the evaluators.
E.g., if we run 3 cases × 2 evaluators, using deterministic evaluators (Contains + Equals), we get the following
return type: EvaluationReport ← PR delivers: one report, not list[…]
# rows (cases): 6 ← 3 cases × 2 evaluators, flattened
Keys on each row: actual_output, evaluator, expected_output, input, metadata, name, …
Concretely, this would produce the following output report.
# name evaluator score pass reason
0 capital-france Contains 1.00 True actual_output contains 'capital'
1 capital-japan Contains 0.00 False actual_output does not contain 'capital'
2 capital-italy Contains 1.00 True actual_output contains 'capital'
3 capital-france Equals 1.00 True actual_output matches expected value
4 capital-japan Equals 0.00 False actual_output does not match expected value
5 capital-italy Equals 1.00 True actual_output matches expected value
The evaluator tag comes from Evaluator.get_type_name(), which is just cls.name (evaluator.py:265). Two instances of the same class (e.g., two OutputEvaluators with different rubrics, two Contains for different substrings) are indistinguishable.
As a follow-up, we should give Evaluator an optional name/id (defaulting to the class name) and key evaluator_data + the row tag on that, so same-class instances stay distinct and the doubling disappears.
Resolve conflicts from strands-agents#241 (single flattened report) and strands-agents#244 (trace evaluators in defaults): - experiment.py: adopt base's single-EvaluationReport return; keep our max_workers guard, _run_meta, and cross-product swap/restore on top. - report.py: fold our run_meta merge into the new from_evaluation_report(report) signature (replacing the old plural from_evaluation_reports). - test_report.py: update our verbose/drilldown tests to the from_evaluation_report(_flatten(...)) shape. 1251 tests green, lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Description
Breaking change.
Experiment.run_evaluations()andrun_evaluations_async()(plus the chaos and red-team subclasses) now return a singleEvaluationReportinstead oflist[EvaluationReport]. Callers no longer need to writereports[0]or callEvaluationReport.flatten(reports)themselves.Every case row carries an
evaluatortag (report.cases[i]["evaluator"]), regardless of how many evaluators ran. With multiple evaluators, the report is flattened across(case, evaluator)pairs so callers can group/filter without an extra step.What changed
Experiment.run_evaluations[_async]returns a singleEvaluationReport.EvaluationReportno longer has anevaluator_namefield — each row's evaluator is on the row itself.EvaluationReport.flatten(reports)is a pure concatenation now: it preserves whateverevaluatortag each row already has and does not stamp a new one.RedTeamReport.from_evaluation_reports(list)was renamed tofrom_evaluation_report(report)to match the new shape.Migration
For multi-evaluator runs, walk the single report and read the per-row tag:
If you previously stored per-evaluator reports separately (e.g., across multiple experiments), tag each row before merging:
Related Issues
Documentation PR
Type of Change
Breaking change
Testing
Updated every test that previously indexed
reports[0]or assertedlen(reports) == N. Multi-evaluator tests look up rows by theirevaluatortag.Full unit suite: 1192 passed.
`hatch fmt --linter --check` and `hatch fmt --formatter --check` clean.
Updated `README.md`, `SKILL.md`, and `AGENTS.md` to describe the new return shape.
I ran `hatch run prepare`
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.