sofhiaazzhr Claude Opus 4.8 commited on
Commit
760ba69
·
1 Parent(s): 61e6fa3

[NOTICKET] fix(eval): don't let a crashed help case pass its rules (flag errored)

Browse files

Review follow-up on eval/help: if HelpAgent.astream raised, output became "ERROR:..."
and assertions still ran on it — must_not_contain_any trivially passed and language_match
fell back to Indonesian, so an infra failure could inflate compliance. Now such a case is
flagged `errored`, its rules are not scored, it counts as a failure, and it's surfaced
under a separate ERRORED line (not mistaken for a rule violation). Also: type
_build_state -> AnalysisState (mypy strict), and document the known limitations
(temperature variance, detector-graded language, errored handling) in the README.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Files changed (2) hide show
  1. eval/help/README.md +15 -0
  2. eval/help/run_eval.py +29 -5
eval/help/README.md CHANGED
@@ -60,3 +60,18 @@ two are reported separately.
60
  `help_dataset.json` — see the `_about` / `_carried_over` doc keys in the file. Language
61
  detection reuses `help._detect_reply_language`; `report_ready.missing` uses the codes
62
  `analysis` / `delta` mapped to the real `is_report_ready` strings in the runner.
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
60
  `help_dataset.json` — see the `_about` / `_carried_over` doc keys in the file. Language
61
  detection reuses `help._detect_reply_language`; `report_ready.missing` uses the codes
62
  `analysis` / `delta` mapped to the real `is_report_ready` strings in the runner.
63
+
64
+ ## Known limitations
65
+
66
+ - **Compliance is approximate across runs.** `HelpAgent` runs at `temperature=0.3`, so the
67
+ reply varies; a borderline case can flip pass/fail between runs. Treat the rate as a
68
+ signal, not a fixed number — re-run before trusting a single-point drop.
69
+ - **`language_match` grades with the same detector the feature uses** (`_detect_reply_language`
70
+ over the reply). It verifies the model obeyed the `[Reply language]` directive, assuming the
71
+ detector is correct — the detector itself is unit-tested separately in
72
+ `tests/unit/agents/handlers/test_help.py`. It can also misfire on a reply that mixes
73
+ languages (e.g. an Indonesian reply quoting an English business question).
74
+ - **Errored cases (stream crash) count as failures, not rule violations.** If `astream` raises
75
+ (Azure down, timeout), the case is flagged `errored` and reported under a separate `ERRORED`
76
+ line — assertions are NOT run on the error string (a crash must not trivially "pass" a
77
+ `must_not_contain_any`). A run with errors is not a clean pass; re-run once the cause clears.
eval/help/run_eval.py CHANGED
@@ -47,7 +47,7 @@ from langchain_core.callbacks import BaseCallbackHandler
47
  from langchain_core.messages import AIMessage, BaseMessage, HumanMessage
48
  from langchain_core.outputs import LLMResult
49
 
50
- from src.agents.gate import stub_analysis_state
51
  from src.agents.handlers.help import HelpAgent, ReportReadiness, _detect_reply_language
52
  from src.agents.report.readiness import _MISSING_ANALYSIS, _MISSING_DELTA
53
 
@@ -143,6 +143,7 @@ class CaseResult:
143
  all_passed: bool | None # None when manual_review (not auto-scored)
144
  latency_ms: float
145
  tokens: dict[str, int]
 
146
 
147
 
148
  def load_cases(path: Path) -> list[dict[str, Any]]:
@@ -151,7 +152,7 @@ def load_cases(path: Path) -> list[dict[str, Any]]:
151
  return list(data["cases"])
152
 
153
 
154
- def _build_state(spec: dict[str, Any]):
155
  """Build an AnalysisState from a case's `state` block (defaults from the stub)."""
156
  return stub_analysis_state().model_copy(
157
  update={
@@ -205,13 +206,23 @@ async def run_case(case: dict[str, Any]) -> CaseResult:
205
  latency_ms = round((time.perf_counter() - start) * 1000, 1)
206
 
207
  manual = bool(case.get("manual_review"))
 
208
  asserts: list[AssertResult] = []
209
- if not manual:
 
 
 
 
 
 
 
 
 
210
  for spec in case.get("asserts", []):
211
  check = _ASSERT_CHECKS[spec["type"]]
212
  passed, detail = check(output, spec)
213
  asserts.append(AssertResult(type=spec["type"], passed=passed, detail=detail))
214
- all_passed = None if manual else all(a.passed for a in asserts)
215
 
216
  return CaseResult(
217
  id=case["id"],
@@ -223,6 +234,7 @@ async def run_case(case: dict[str, Any]) -> CaseResult:
223
  all_passed=all_passed,
224
  latency_ms=latency_ms,
225
  tokens=collector.tokens,
 
226
  )
227
 
228
 
@@ -246,6 +258,7 @@ def summarize(results: list[CaseResult]) -> dict[str, Any]:
246
  for g in GROUPS
247
  if any(r.group == g for r in results)
248
  }
 
249
  return {
250
  "total": len(results),
251
  "scored": len(scored),
@@ -257,6 +270,7 @@ def summarize(results: list[CaseResult]) -> dict[str, Any]:
257
  "by_group": by_group,
258
  "held_out": _compliance([r for r in scored if not r.carried_over]),
259
  "carried_over": _compliance([r for r in scored if r.carried_over]),
 
260
  }
261
 
262
 
@@ -314,12 +328,21 @@ def format_summary(summary: dict[str, Any], results: list[CaseResult]) -> str:
314
  f" carried_over {co['passed']}/{co['n']} "
315
  f"{co['compliance'] * 100:.0f}% <- regression"
316
  )
317
- failures = [r for r in results if r.all_passed is False]
 
 
318
  lines.append("")
319
  lines.append(f" FAILURES ({len(failures)})")
320
  for r in failures:
321
  bad = [f"{a.type}({a.detail})" for a in r.asserts if not a.passed]
322
  lines.append(f" {r.id:<20} {r.group:<13} {'; '.join(bad)}")
 
 
 
 
 
 
 
323
  return "\n".join(lines)
324
 
325
 
@@ -339,6 +362,7 @@ def build_report(
339
  "by_group": summary["by_group"],
340
  "held_out": summary["held_out"],
341
  "carried_over": summary["carried_over"],
 
342
  "cases": [asdict(r) for r in results],
343
  }
344
 
 
47
  from langchain_core.messages import AIMessage, BaseMessage, HumanMessage
48
  from langchain_core.outputs import LLMResult
49
 
50
+ from src.agents.gate import AnalysisState, stub_analysis_state
51
  from src.agents.handlers.help import HelpAgent, ReportReadiness, _detect_reply_language
52
  from src.agents.report.readiness import _MISSING_ANALYSIS, _MISSING_DELTA
53
 
 
143
  all_passed: bool | None # None when manual_review (not auto-scored)
144
  latency_ms: float
145
  tokens: dict[str, int]
146
+ errored: bool = False # the astream call raised — infra failure, not a rule verdict
147
 
148
 
149
  def load_cases(path: Path) -> list[dict[str, Any]]:
 
152
  return list(data["cases"])
153
 
154
 
155
+ def _build_state(spec: dict[str, Any]) -> AnalysisState:
156
  """Build an AnalysisState from a case's `state` block (defaults from the stub)."""
157
  return stub_analysis_state().model_copy(
158
  update={
 
206
  latency_ms = round((time.perf_counter() - start) * 1000, 1)
207
 
208
  manual = bool(case.get("manual_review"))
209
+ errored = output.startswith("ERROR:")
210
  asserts: list[AssertResult] = []
211
+ if errored:
212
+ # Don't run rule checks on an error string — a crash must not "pass" a
213
+ # must_not_contain_any (the pattern is trivially absent) or a language check.
214
+ # Count it as a failure, but flag it as errored so it reads as infra, not a
215
+ # rule violation (overrides manual_review — a crash isn't reviewable).
216
+ asserts = [AssertResult(type="stream", passed=False, detail=_truncate(output, 100))]
217
+ all_passed: bool | None = False
218
+ elif manual:
219
+ all_passed = None
220
+ else:
221
  for spec in case.get("asserts", []):
222
  check = _ASSERT_CHECKS[spec["type"]]
223
  passed, detail = check(output, spec)
224
  asserts.append(AssertResult(type=spec["type"], passed=passed, detail=detail))
225
+ all_passed = all(a.passed for a in asserts)
226
 
227
  return CaseResult(
228
  id=case["id"],
 
234
  all_passed=all_passed,
235
  latency_ms=latency_ms,
236
  tokens=collector.tokens,
237
+ errored=errored,
238
  )
239
 
240
 
 
258
  for g in GROUPS
259
  if any(r.group == g for r in results)
260
  }
261
+ errored = [r for r in results if r.errored]
262
  return {
263
  "total": len(results),
264
  "scored": len(scored),
 
270
  "by_group": by_group,
271
  "held_out": _compliance([r for r in scored if not r.carried_over]),
272
  "carried_over": _compliance([r for r in scored if r.carried_over]),
273
+ "errored": {"count": len(errored), "ids": [r.id for r in errored]},
274
  }
275
 
276
 
 
328
  f" carried_over {co['passed']}/{co['n']} "
329
  f"{co['compliance'] * 100:.0f}% <- regression"
330
  )
331
+ # Rule failures (real disobedience) vs errored (infra/stream crash) kept apart so
332
+ # a crashed run isn't misread as the model breaking a rule.
333
+ failures = [r for r in results if r.all_passed is False and not r.errored]
334
  lines.append("")
335
  lines.append(f" FAILURES ({len(failures)})")
336
  for r in failures:
337
  bad = [f"{a.type}({a.detail})" for a in r.asserts if not a.passed]
338
  lines.append(f" {r.id:<20} {r.group:<13} {'; '.join(bad)}")
339
+ err = summary["errored"]
340
+ if err["count"]:
341
+ lines.append("")
342
+ lines.append(
343
+ f" ERRORED ({err['count']}) - stream crashed, counted as fail NOT a rule miss"
344
+ f" -> {', '.join(err['ids'])}"
345
+ )
346
  return "\n".join(lines)
347
 
348
 
 
362
  "by_group": summary["by_group"],
363
  "held_out": summary["held_out"],
364
  "carried_over": summary["carried_over"],
365
+ "errored": summary["errored"],
366
  "cases": [asdict(r) for r in results],
367
  }
368