feat: add status field to EvaluationOutput

[max-rattray-aws]

Summary

Adds an optional status field to EvaluationOutput that lets evaluators signal when a result should not count toward aggregate scores.

Three values are supported:

• "graded" (default) - real verdict, included in all aggregations
• "could_not_evaluate" - evaluator tried but could not grade (preconditions not met, harness failure, missing data)
• "informational" - surfaces content for human review, not pass/fail

The _default_aggregator, Experiment report builder, and EvaluationReport.flatten() all filter to graded outputs before computing overall_score. The field defaults to "graded", making this fully backward compatible.

Usage

from strands_evals.types import EvaluationOutput

# Evaluator that only applies when tool errors exist
def evaluate(self, evaluation_case):
    if not self._has_errors(evaluation_case):
        return [EvaluationOutput(
            score=0.0,
            test_pass=False,
            reason="No tool errors occurred in trajectory",
            status="could_not_evaluate",
        )]
    # ... actual grading logic

What's tested

• EvaluationOutput.status field: default value, serialization roundtrip, backward compat
• _default_aggregator: filters non-graded outputs, handles all-CNE, mixed statuses, empty list
• EvaluationReport.flatten(): excludes non-graded from overall_score, preserves statuses
• File roundtrip: statuses survive to_file/from_file
• End-to-end: evaluators returning could_not_evaluate, informational, mixed outputs
• Custom aggregator override still works

All 190 relevant tests pass (24 new + 166 existing). Ruff formatting and linting clean.

Related to #346

[github-actions]

Issue (Important + Medium): Two things on statuses:

1. Same typing point as EvaluationOutput.status — prefer list[Literal["graded", "could_not_evaluate", "informational"]] over list[str] for validation and discoverability.

2. statuses defaults to [] while scores/test_passes are populated, and the rest of the code compensates per-index with ... if i < len(self.statuses) else "graded" (see flatten line 58 and _display line 146). This means a valid EvaluationReport can have len(statuses) != len(scores). It's guarded today, but any future zip(scores, statuses, strict=True) (like the one you added on line 60) would raise on such a report.

Suggestion: Consider a Pydantic validator that pads statuses to len(scores) with "graded" on construction, so the parallel-list invariant holds everywhere and the per-index guards can go away. At minimum, document the invariant in the docstring.

[github-actions]

Issue (Important): This nested conditional expression is hard to parse, and the roll-up rule is implicit: when no output is graded, it falls back to evaluation_outputs[0].status, so a case mixing could_not_evaluate and informational gets whichever happens to be first — an arbitrary tie-break that isn't documented anywhere.

Suggestion: Extract a small named helper with an explicit precedence rule, e.g.:

def _rollup_status(outputs: list[EvaluationOutput]) -> str:
    if not outputs or any(o.status == "graded" for o in outputs):
        return "graded"
    # define intended precedence when no graded output exists
    if any(o.status == "could_not_evaluate" for o in outputs):
        return "could_not_evaluate"
    return "informational"

This makes the intended behavior testable and removes the positional dependency. Please also confirm the desired precedence — the PR doesn't currently specify what a mixed non-graded case should roll up to.

[github-actions]

Thanks — _roll_up_status() is a nice improvement and the precedence is now explicit and documented. Keeping the first-non-graded-output tie-break is a reasonable call now that it's intentional rather than incidental.

One tiny follow-up: the helper is annotated evaluation_outputs: list (untyped element). Since EvaluationOutput is already imported here, list[EvaluationOutput] would be more precise and consistent with the repo's typing conventions. Non-blocking.

[github-actions]

Issue (Important): status is typed as bare str, so Pydantic accepts any value. A typo like status="could_not_evaluete" would silently be treated as non-graded and excluded from every aggregation — a scoring-corruption footgun with no error surfaced. The three values are also the public contract, so they should be discoverable via type hints/IDE autocomplete.

Suggestion: Use a Literal, which is the established convention in this package (types/multimodal.py, types/detector.py):

from typing import Literal

status: Literal["graded", "could_not_evaluate", "informational"] = "graded"

This gives validation for free and makes the accepted values self-documenting. Aligns with the "obvious path is the happy path" tenet.

[github-actions]

Issue (Suggestion): The test name says not_in_serialized_when_default, but the body asserts the opposite — that status is present in model_dump(). The name contradicts the behavior and will mislead readers.

Suggestion: Rename to something like test_status_included_in_serialized_when_default.

[github-actions]

Issue (Suggestion): The aggregator tests assert score, passed, and reason field-by-field. Since _default_aggregator returns a fixed 3-tuple, a single equality check would also guard against the reason string/ordering regressing:

assert Evaluator._default_aggregator(outputs) == (0.0, False, "No gradable evaluation outputs produced")

Suggestion: For the deterministic cases (e.g. test_all_could_not_evaluate, test_all_informational, test_empty_outputs_list), assert the full tuple. Keep per-field assertions only where the score is a float approximation.

[github-actions]

Assessment: Comment

Clean, well-tested, backward-compatible addition — the graded-only filtering is applied consistently across _default_aggregator, the Experiment report builder, and EvaluationReport.flatten(). My main asks are typing the new status values as Literal and tightening the status roll-up logic before merge.

Review Categories

• API typing: EvaluationOutput.status and EvaluationReport.statuses use bare str/list[str]; the package convention (and validation safety) calls for Literal. An invalid value currently fails silently by being dropped from aggregation.
• API review process: This adds a new field to the publicly-exported EvaluationOutput. Consider adding the needs-api-review label — the PR description already has good usage/examples to support that review.
• Readability / correctness: The status roll-up in experiment.py is a nested ternary with an undocumented, position-dependent tie-break for mixed non-graded outputs. Extracting a helper with an explicit precedence rule would make the intent clear and testable.
• Data invariant: statuses defaulting to [] while other parallel lists are populated creates a length-mismatch that's currently patched with per-index guards; a validator that pads to len(scores) would make it robust.
• Tests: Strong coverage overall. One misleading test name and a few deterministic tuple assertions that could be single-equality checks.

Nice job keeping this fully backward compatible via the "graded" default.

[github-actions]

Re-review of 593a8fa (fix: address review feedback) — all points from the previous round are addressed:

• ✅ EvaluationOutput.status and EvaluationReport.statuses now use Literal[...]; added a test confirming invalid values are rejected by Pydantic.
• ✅ Status roll-up extracted into _roll_up_status() with documented precedence.
• ✅ model_validator pads statuses to len(scores), and the legacy-load test was correctly updated to reflect the new padding behavior.
• ✅ Misleading test renamed and now uses full-object equality.

Verified locally: 34/34 status tests pass, 928 related tests pass, ruff check clean.

Only a single non-blocking nit remains (element type on the _roll_up_status parameter, noted inline). Assessment: Approve once that's addressed or consciously skipped. Thanks for the thorough turnaround.

[pdebjyot]

Nice — this matches the design in #346 closely. I'd independently put up #357 for the same issue a few hours earlier; happy to close mine as superseded by this one since it's already through review. Cross-linking for provenance.

One thing worth a follow-up (applies to this implementation as merged, not a blocker): the graded-only filter is applied to overall_score, but the raw scores / test_passes lists still carry the 0.0 / False placeholders for could_not_evaluate rows. Any consumer that computes a pass-rate directly off test_passes — rather than reading overall_score — will still count skipped/overflowed cases as hard failures. Might be worth either a docstring note steering consumers to overall_score/statuses, or a small pass_rate helper that applies the same graded-only filter, so the two aggregation paths can't drift.

Good to see the _worker task-crash path tagged too, not just the _run_evaluator catches — that's the case that most often bites in practice.