-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add StringCheckGrader support for OpenAI Evals backend #102
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,3 +33,10 @@ evaluators: | |
| threshold: 0.110 | ||
| executor: local | ||
|
|
||
| # OpenAI Evals API grader (requires OPENAI_API_KEY) | ||
| - name: city_name_check | ||
| type: openai_eval | ||
| grader: | ||
| type: string_check | ||
| operation: eq | ||
| reference: Paris | ||
|
Comment on lines
+41
to
+42
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This config runs against samples/helm.json, so eq + Paris can never pass. Also eq on a whole agent response is pretty much never what you want in practice. Can we make |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,8 @@ class RemoteEvaluatorDef(BaseEvaluatorDef): | |
| } | ||
| ) | ||
|
|
||
| _VALID_STRING_CHECK_OPERATIONS = frozenset({"eq", "ne", "like", "ilike"}) | ||
|
|
||
|
|
||
| class OpenAIEvalDef(BaseModel): | ||
| """An evaluator that delegates grading to the OpenAI Evals API.""" | ||
|
|
@@ -121,8 +123,18 @@ def _validate_grader(cls, v: dict[str, Any]) -> dict[str, Any]: | |
| invalid = [lbl for lbl in v["passing_labels"] if lbl not in v["labels"]] | ||
| if invalid: | ||
| raise ValueError(f"passing_labels contains labels not declared in labels: {invalid}") | ||
| elif grader_type == "string_check": | ||
| operation = v.get("operation") | ||
| if not operation: | ||
| raise ValueError("'operation' is required for string_check grader") | ||
| if operation not in _VALID_STRING_CHECK_OPERATIONS: | ||
| raise ValueError(f"Unknown operation '{operation}'. Valid: {sorted(_VALID_STRING_CHECK_OPERATIONS)}") | ||
| if not v.get("reference"): | ||
| raise ValueError("'reference' is required for string_check grader") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reference needs a type check. |
||
| else: | ||
| raise ValueError(f"Unsupported grader type: '{grader_type}'. Supported: label_model, text_similarity") | ||
| raise ValueError( | ||
| f"Unsupported grader type: '{grader_type}'. Supported: label_model, string_check, text_similarity" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like |
||
| ) | ||
| return v | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,15 @@ def _build_testing_criteria(evaluator_def: OpenAIEvalDef) -> dict[str, Any]: | |
| "passing_labels": grader["passing_labels"], | ||
| } | ||
|
|
||
| if grader_type == "string_check": | ||
| return { | ||
| "type": "string_check", | ||
| "name": evaluator_def.name, | ||
| "input": "{{ item.actual_response }}", | ||
| "reference": grader["reference"], | ||
| "operation": grader["operation"], | ||
| } | ||
|
|
||
| raise ValueError(f"Unsupported grader type: {grader_type}") | ||
|
|
||
|
|
||
|
|
@@ -131,7 +140,9 @@ async def evaluate_openai_eval( | |
| ) | ||
|
|
||
| items = _build_jsonl_items( | ||
| actual_invocations, expected_invocations or [], include_expected=(grader_type != "label_model") | ||
| actual_invocations, | ||
| expected_invocations or [], | ||
| include_expected=(grader_type == "text_similarity"), | ||
| ) | ||
| if not items: | ||
| return MetricResult( | ||
|
|
@@ -145,7 +156,7 @@ async def evaluate_openai_eval( | |
| try: | ||
| client = await asyncio.to_thread(_get_openai_client) | ||
|
|
||
| item_schema = _ACTUAL_ONLY_SCHEMA if grader_type == "label_model" else _TEXT_PAIR_SCHEMA | ||
| item_schema = _TEXT_PAIR_SCHEMA if grader_type == "text_similarity" else _ACTUAL_ONLY_SCHEMA | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to stay in sync with Can we derive both from one place, like the |
||
| eval_obj = await asyncio.to_thread( | ||
| client.evals.create, | ||
| name=f"agentevals-openai-{evaluator_def.name}", | ||
|
|
@@ -252,6 +263,8 @@ async def _collect_results(client: Any, eval_id: str, run_id: str, run: Any, eva | |
| elif grader["type"] == "label_model": | ||
| details["model"] = grader.get("model") | ||
| details["passing_labels"] = grader.get("passing_labels") | ||
| elif grader["type"] == "string_check": | ||
| details["operation"] = grader.get("operation") | ||
| per_criteria = getattr(run, "per_testing_criteria_results", None) | ||
| if per_criteria: | ||
| details["per_testing_criteria"] = [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,21 @@ def test_label_model_passing_labels_not_in_labels(self): | |
| with pytest.raises(Exception, match="passing_labels"): | ||
| OpenAIEvalDef(name="lm", grader=grader) | ||
|
|
||
| def test_string_check_valid(self): | ||
| d = OpenAIEvalDef(name="check", grader={"type": "string_check", "operation": "eq", "reference": "Paris"}) | ||
| assert d.grader["type"] == "string_check" | ||
|
|
||
| @pytest.mark.parametrize("field", ["operation", "reference"]) | ||
| def test_string_check_missing_required_field(self, field): | ||
| grader = {"type": "string_check", "operation": "eq", "reference": "Paris"} | ||
| grader.pop(field) | ||
| with pytest.raises(Exception, match=field): | ||
| OpenAIEvalDef(name="check", grader=grader) | ||
|
|
||
| def test_string_check_invalid_operation(self): | ||
| with pytest.raises(Exception, match="Unknown operation"): | ||
| OpenAIEvalDef(name="check", grader={"type": "string_check", "operation": "contains", "reference": "Paris"}) | ||
|
|
||
| def test_unsupported_grader_type(self): | ||
| with pytest.raises(Exception, match="Unsupported grader type"): | ||
| OpenAIEvalDef(name="x", grader={"type": "unknown"}) | ||
|
|
@@ -81,6 +96,17 @@ def test_label_model_shape(self): | |
| assert c["passing_labels"] == ["good"] | ||
| assert c["input"] == grader["input"] | ||
|
|
||
| def test_string_check_shape(self): | ||
| d = OpenAIEvalDef(name="check", grader={"type": "string_check", "operation": "ilike", "reference": "Paris"}) | ||
| c = _build_testing_criteria(d) | ||
| assert c == { | ||
| "type": "string_check", | ||
| "name": "check", | ||
| "input": "{{ item.actual_response }}", | ||
| "reference": "Paris", | ||
| "operation": "ilike", | ||
| } | ||
|
|
||
|
|
||
| class TestBuildJsonlItems: | ||
| def test_text_similarity_includes_expected(self): | ||
|
|
@@ -91,6 +117,10 @@ def test_label_model_excludes_expected(self): | |
| items = _build_jsonl_items([_invocation("hello")], [], include_expected=False) | ||
| assert "expected_response" not in items[0]["item"] | ||
|
|
||
| def test_string_check_excludes_expected(self): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this the same test as |
||
| items = _build_jsonl_items([_invocation("Paris")], [], include_expected=False) | ||
| assert items == [{"item": {"actual_response": "Paris"}}] | ||
|
|
||
| def test_missing_expected_falls_back_to_empty(self): | ||
| items = _build_jsonl_items([_invocation("hello")], [], include_expected=True) | ||
| assert items[0]["item"]["expected_response"] == "" | ||
|
|
@@ -115,3 +145,10 @@ async def test_label_model_does_not_require_expected(self, monkeypatch): | |
| d = OpenAIEvalDef(name="lm", grader=_label_grader()) | ||
| result = await evaluate_openai_eval(d, [_invocation("hi")], None) | ||
| assert "expected invocations" not in (result.error or "") | ||
|
|
||
| async def test_string_check_does_not_require_expected(self, monkeypatch): | ||
| monkeypatch.setenv("OPENAI_API_KEY", "test-key") | ||
| monkeypatch.setattr("agentevals.openai_eval_backend._get_openai_client", lambda: None) | ||
| d = OpenAIEvalDef(name="check", grader={"type": "string_check", "operation": "eq", "reference": "Paris"}) | ||
| result = await evaluate_openai_eval(d, [_invocation("Paris")], None) | ||
| assert "expected invocations" not in (result.error or "") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this to eval_config_openai_eval.yaml? That's where the label_model example lives, and it keeps this config runnable without an API key.