Skip to content

Commit ae8ebdb

Browse files
etoyamaclaude
andcommitted
fix(#108): risk_evaluator -- guard zero median to prevent ZeroDivisionError
When median_estimated_rows is 0.0, skip extrapolation and return HIGH with history_uninformative flag instead of crashing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d18ee41 commit ae8ebdb

2 files changed

Lines changed: 86 additions & 3 deletions

File tree

skills/premortem/lib/risk_evaluator.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,28 @@ def _evaluate_with_history(
122122
source_checks: SourceChecks,
123123
) -> RiskDecision:
124124
"""Extrapolation-based risk when history is sufficient."""
125-
assert history.median_elapsed_min is not None
126-
assert history.median_estimated_rows is not None
127-
assert history.success_rate is not None
125+
if history.median_elapsed_min is None:
126+
raise ValueError(
127+
"median_elapsed_min must not be None when history is sufficient"
128+
)
129+
if history.median_estimated_rows is None:
130+
raise ValueError(
131+
"median_estimated_rows must not be None when history is sufficient"
132+
)
133+
if history.success_rate is None:
134+
raise ValueError("success_rate must not be None when history is sufficient")
128135

129136
estimated_rows = source_checks.estimated_rows or 0
130137

138+
# Q1: guard against zero median_estimated_rows to prevent ZeroDivisionError
139+
if history.median_estimated_rows == 0.0:
140+
return RiskDecision(
141+
level=RiskLevel.HIGH,
142+
reasons=["median_estimated_rows is zero; extrapolation not possible"],
143+
flags=["history_uninformative"],
144+
extrapolated_time_min=None,
145+
)
146+
131147
extrapolated = (
132148
history.median_elapsed_min
133149
* (estimated_rows / history.median_estimated_rows)

tests/batch_harness/test_risk_evaluator.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,3 +600,70 @@ def test_hard_block_takes_priority_over_api_failure(self) -> None:
600600
source_checks=checks,
601601
)
602602
assert result.level == RiskLevel.HARD_BLOCK
603+
604+
605+
# ---------------------------------------------------------------------------
606+
# Q1: zero median_estimated_rows must not raise ZeroDivisionError
607+
# ---------------------------------------------------------------------------
608+
609+
610+
class TestRiskEvaluatorZeroMedian:
611+
"""Q1: history with median_estimated_rows=0.0 must not crash."""
612+
613+
def test_zero_median_history_does_not_raise(self) -> None:
614+
"""Feed history with median_estimated_rows=0.0, expect no exception."""
615+
history = HistoryStats(
616+
n=5,
617+
median_elapsed_min=30.0,
618+
median_estimated_rows=0.0,
619+
success_rate=1.0,
620+
)
621+
# Must not raise ZeroDivisionError
622+
result = evaluate(
623+
design={"status": "analyzing"},
624+
history=history,
625+
config=_default_config(),
626+
source_checks=_good_source_checks(estimated_rows=1_000_000),
627+
)
628+
assert result.level in {RiskLevel.HIGH, RiskLevel.MEDIUM, RiskLevel.LOW}
629+
630+
def test_zero_median_history_flags_uninformative(self) -> None:
631+
"""Confirm the flag signals why extrapolation was skipped."""
632+
history = HistoryStats(
633+
n=5,
634+
median_elapsed_min=30.0,
635+
median_estimated_rows=0.0,
636+
success_rate=1.0,
637+
)
638+
result = evaluate(
639+
design={"status": "analyzing"},
640+
history=history,
641+
config=_default_config(),
642+
source_checks=_good_source_checks(estimated_rows=1_000_000),
643+
)
644+
assert "history_uninformative" in result.flags
645+
646+
647+
# ---------------------------------------------------------------------------
648+
# Q2: assert replaced with ValueError for -O safety
649+
# ---------------------------------------------------------------------------
650+
651+
652+
class TestRiskEvaluatorAssertReplacement:
653+
"""Q2: runtime invariants must raise ValueError, not AssertionError."""
654+
655+
def test_invalid_input_raises_value_error_not_assertion(self) -> None:
656+
"""Trigger the condition that used to assert, confirm ValueError."""
657+
history = HistoryStats(
658+
n=5,
659+
median_elapsed_min=None, # violates invariant
660+
median_estimated_rows=1_000_000.0,
661+
success_rate=1.0,
662+
)
663+
with pytest.raises(ValueError):
664+
evaluate(
665+
design={"status": "analyzing"},
666+
history=history,
667+
config=_default_config(),
668+
source_checks=_good_source_checks(),
669+
)

0 commit comments

Comments
 (0)