Skip to content

feat: add status field to EvaluationOutput #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
max-rattray-aws wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: add status field to EvaluationOutput #359

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d1255e
feat: add status field to EvaluationOutput for non-gradable results
max-rattray-aws Aug 10, 2026
593a8fa
fix: address review feedback
max-rattray-aws Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/strands_evals/evaluators/evaluator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,16 @@ def _get_model_id(self, model: Model | str | None) -> str:

@staticmethod
def _default_aggregator(outputs: list[EvaluationOutput]) -> tuple[float, bool, str]:
# Handle empty outputs list to avoid division by zero
if not outputs:
return (0.0, False, "No evaluation outputs produced")
# Filter to only graded outputs for aggregation
graded = [o for o in outputs if o.status == "graded"]

avg_score = sum(o.score for o in outputs) / len(outputs)
all_pass = all(o.test_pass for o in outputs)
combined_reason = " | ".join(o.reason for o in outputs if o.reason)
# Handle empty graded list to avoid division by zero
if not graded:
return (0.0, False, "No gradable evaluation outputs produced")

avg_score = sum(o.score for o in graded) / len(graded)
all_pass = all(o.test_pass for o in graded)
combined_reason = " | ".join(o.reason for o in graded if o.reason)
return avg_score, all_pass, combined_reason

def evaluate(self, evaluation_case: EvaluationData[InputT, OutputT]) -> list[EvaluationOutput]:
Expand Down
17 changes: 16 additions & 1 deletion src/strands_evals/experiment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,13 @@ async def _evaluate_with_retry(evaluator=evaluator, evaluation_context=evaluatio
"score": aggregate_score,
"reason": aggregate_reason or "",
"detailed_results": evaluation_outputs,
"status": (

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"graded"
if any(o.status == "graded" for o in evaluation_outputs)
else evaluation_outputs[0].status
if evaluation_outputs
else "graded"
),
}

except RetryError as e:
Expand All @@ -443,6 +450,7 @@ async def _evaluate_with_retry(evaluator=evaluator, evaluation_context=evaluatio
"score": 0,
"reason": f"Evaluator error: {str(original_exception)}",
"detailed_results": [],
"status": "could_not_evaluate",
}
except Exception as e:
# Catch non-throttling errors and record as failure (error isolation)
Expand All @@ -453,6 +461,7 @@ async def _evaluate_with_retry(evaluator=evaluator, evaluation_context=evaluatio
"score": 0,
"reason": f"Evaluator error: {str(e)}",
"detailed_results": [],
"status": "could_not_evaluate",
}

async def _run_diagnosis(
Expand Down Expand Up @@ -569,6 +578,7 @@ async def _worker(
"score": 0,
"reason": f"An error occurred: {str(e)}",
"detailed_results": [],
"status": "could_not_evaluate",
}
)
results[index] = {
Expand Down Expand Up @@ -659,6 +669,7 @@ async def run_evaluations_async(
"detailed_results": [],
"diagnoses": [],
"recommendations": [],
"statuses": [],
}
for evaluator in self._evaluators
}
Expand All @@ -678,21 +689,25 @@ async def run_evaluations_async(
evaluator_data[eval_name]["detailed_results"].append(eval_result["detailed_results"])
evaluator_data[eval_name]["diagnoses"].append(diagnosis)
evaluator_data[eval_name]["recommendations"].append(recommendation)
evaluator_data[eval_name]["statuses"].append(eval_result.get("status", "graded"))

reports = []
for evaluator in self._evaluators:
eval_name = evaluator.get_name()
data = evaluator_data[eval_name]
scores = data["scores"]
statuses = data["statuses"]
graded_scores = [s for s, st in zip(scores, statuses, strict=True) if st == "graded"]
report = EvaluationReport(
overall_score=sum(scores) / len(scores) if scores else 0,
overall_score=sum(graded_scores) / len(graded_scores) if graded_scores else 0,
scores=scores,
test_passes=data["test_passes"],
cases=data["cases"],
reasons=data["reasons"],
detailed_results=data["detailed_results"],
diagnoses=data["diagnoses"],
recommendations=data["recommendations"],
statuses=statuses,
)
reports.append(report)

Expand Down
13 changes: 13 additions & 0 deletions src/strands_evals/types/evaluation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,22 @@ class EvaluationOutput(BaseModel):
test_pass: Whether the test pass or fail.
reason: The reason for the score for each test case.
label: The categorical label corresponding to the score.
status: The grading status of the evaluation output. Controls whether
this result is included in aggregate score computations.

- "graded" (default): The score and test_pass are real verdicts.
Included in all aggregations. Backward compatible with every
existing evaluator.
- "could_not_evaluate": The evaluator tried to grade but could not
(preconditions not met, harness failure, missing data). Excluded
from pass-rate and score aggregations.
- "informational": The evaluator does not pass/fail by design. It
surfaces content for human review but is excluded from numeric
aggregates.
"""

score: float
test_pass: bool
reason: str | None = None
label: str | None = None
status: str = "graded"

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

16 changes: 14 additions & 2 deletions src/strands_evals/types/evaluation_report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ class EvaluationReport(BaseModel):
the evaluator that produced that row.
test_passes: A list of booleans indicating whether the test pass or fail.
reasons: A list of reason for each test case.
statuses: A list of status strings for each test case. Controls whether
the result is included in overall_score aggregation. Values are
"graded", "could_not_evaluate", or "informational".
"""

overall_score: float
Expand All @@ -28,6 +31,7 @@ class EvaluationReport(BaseModel):
detailed_results: list[list[EvaluationOutput]] = []
diagnoses: list[dict | None] = []
recommendations: list[str | None] = []
statuses: list[str] = []

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


@classmethod
def flatten(cls, reports: list["EvaluationReport"]) -> "EvaluationReport":
Expand All @@ -40,7 +44,7 @@ def flatten(cls, reports: list["EvaluationReport"]) -> "EvaluationReport":
if not reports:
return cls(overall_score=0.0, scores=[], cases=[], test_passes=[])

scores, cases, passes, reasons, detailed, diags, recs = [], [], [], [], [], [], []
scores, cases, passes, reasons, detailed, diags, recs, statuses = [], [], [], [], [], [], [], []

for report in reports:
for i, case in enumerate(report.cases):
Expand All @@ -51,16 +55,20 @@ def flatten(cls, reports: list["EvaluationReport"]) -> "EvaluationReport":
detailed.append(report.detailed_results[i] if i < len(report.detailed_results) else [])
diags.append(report.diagnoses[i] if i < len(report.diagnoses) else None)
recs.append(report.recommendations[i] if i < len(report.recommendations) else None)
statuses.append(report.statuses[i] if i < len(report.statuses) else "graded")

graded_scores = [s for s, st in zip(scores, statuses, strict=True) if st == "graded"]

return cls(
overall_score=sum(scores) / len(scores) if scores else 0.0,
overall_score=sum(graded_scores) / len(graded_scores) if graded_scores else 0.0,
scores=scores,
cases=cases,
test_passes=passes,
reasons=reasons,
detailed_results=detailed,
diagnoses=diags,
recommendations=recs,
statuses=statuses,
)

@staticmethod
Expand Down Expand Up @@ -134,6 +142,10 @@ def _display(
details_dict["evaluator"] = self.cases[i]["evaluator"]
details_dict["score"] = f"{self.scores[i]:.2f}"
details_dict["test_pass"] = self.test_passes[i]
# Include status when it differs from the default "graded"
status = self.statuses[i] if i < len(self.statuses) else "graded"
if status != "graded":
details_dict["status"] = status
details_dict["reason"] = reason
if include_input:
details_dict["input"] = self.format_input_for_display(self.cases[i].get("input"))
Expand Down
Loading
Loading