diff --git a/openagent_eval/cicd/plugin.py b/openagent_eval/cicd/plugin.py index 71bb3d6..0da2a59 100644 --- a/openagent_eval/cicd/plugin.py +++ b/openagent_eval/cicd/plugin.py @@ -175,18 +175,29 @@ def run_evaluation( dataset_items = load_dataset_for_run(eval_config) - # Run async evaluation - loop = asyncio.get_event_loop() - if loop.is_running(): - # If we're already in an async context, create a new task + # Run async evaluation. Use get_running_loop() instead of + # get_event_loop(): after any completed asyncio.run() in the + # process, the event-loop policy holds no current loop + # (asyncio.run calls set_event_loop(None) on completion), so + # get_event_loop() raises RuntimeError before is_running() is + # ever reached — and the broad `except Exception` below then + # swallowed it into an error summary (issue #261). + # get_running_loop() only asks whether a loop is running right + # now, independent of the policy's current-loop state. + try: + asyncio.get_running_loop() + except RuntimeError: + # No loop is running: the normal synchronous case. + result = asyncio.run(engine.run(dataset_items)) + else: + # We're already inside a live async context; offload to a + # thread and run it there, preserving the timeout. import concurrent.futures with concurrent.futures.ThreadPoolExecutor() as pool: result = pool.submit( asyncio.run, engine.run(dataset_items) ).result(timeout=timeout) - else: - result = loop.run_until_complete(engine.run(dataset_items)) # Extract metrics from summary metrics = result.summary.get("metrics_summary", {}) @@ -227,6 +238,14 @@ def run_evaluation( # Evaluate thresholds evaluator = ThresholdEvaluator(cicd_config) eval_result = evaluator.evaluate_all_gates(flat_metrics) + # Expose the plugin's threshold-target mapping so downstream + # consumers (e.g. the `oaeval test` threshold path) can evaluate + # gates against it. This is the engine's metric set plus the + # execution counts (total_items, successful_evaluations, + # failed_evaluations) that the plugin already supports as gate + # targets, so it is a superset of the engine summary's own + # `metrics_summary`, not the same set. + eval_result.summary["metrics_summary"] = flat_metrics eval_result.summary["duration_seconds"] = duration return eval_result diff --git a/tests/unit/test_cicd/test_cli_test.py b/tests/unit/test_cicd/test_cli_test.py index 0a9ea5e..fc65c2a 100644 --- a/tests/unit/test_cicd/test_cli_test.py +++ b/tests/unit/test_cicd/test_cli_test.py @@ -1,5 +1,7 @@ """Unit tests for CLI test command.""" +import asyncio +import json import re from typer.testing import CliRunner @@ -15,6 +17,53 @@ def _strip_ansi(text: str) -> str: return re.sub(r"\x1b\[[0-9;]*m", "", text) +def _write_offline_config(tmp_path): + """Write a fully offline dataset + config for CLI test runs. + + The mock providers echo ``ground_truth``, so ``exact_match`` scores + exactly 1.0. Returns the config path. + """ + dataset_path = tmp_path / "data.json" + dataset_path.write_text( + json.dumps( + [ + { + "question": "What is RAG?", + "ground_truth": "RAG combines retrieval with generation.", + "context": "RAG combines retrieval with generation.", + "ground_truth_contexts": [ + "RAG combines retrieval with generation." + ], + } + ] + ) + ) + config_path = tmp_path / "config.yaml" + config_path.write_text( + f""" +dataset: + path: {dataset_path} +llm: + provider: mock + model: mock-model +retriever: + provider: mock + settings: + collection_name: c +metrics: + retrieval: [] + generation: ["exact_match"] + performance: [] + cost: [] +report: + output: json + output_dir: {tmp_path / "reports_out"} +parallel: false +""" + ) + return config_path + + class TestTestCommand: """Tests for oaeval test command.""" @@ -77,3 +126,84 @@ def test_test_command_threshold_option(self): output = _strip_ansi(result.output) assert "threshold" in output.lower() assert "-t" in output + + def test_test_command_passing_threshold_is_not_reported_missing(self, tmp_path): + """Regression test for issue #228. + + A threshold that should pass must not fail with "metric not found": + the gate summary returned by ``OAEvalPlugin.run_evaluation`` must + carry ``metrics_summary`` so the CLI threshold-evaluation path can + look the metric up. Uses fully offline mock providers (the mock + echoes ``ground_truth``, so ``exact_match`` scores exactly 1.0). + + Also asserts the positive behaviour: the threshold was actually + evaluated against the real computed value, not silently waved + through. + """ + config_path = _write_offline_config(tmp_path) + + result = runner.invoke( + app, + ["test", str(config_path), "-t", "exact_match:gte:0.5"], + ) + output = _strip_ansi(result.output) + + assert "not found in results" not in output + # The threshold must be evaluated against the real value (1.0), + # and the gate must be reported as passing. + assert "exact_match: 1.0000 gte 0.5000" in output, output + assert "PASS" in output, output + assert result.exit_code == 0, output + + def test_test_command_failing_threshold_is_enforced(self, tmp_path): + """Complement to the #228 regression test. + + A threshold that genuinely cannot be met (``exact_match`` scores + exactly 1.0 with the offline mock providers, so ``gte:1.5`` must + fail) must exit non-zero and report the failure with the real + actual value. This catches an implementation that auto-passes or + skips missing/unresolvable metrics: a suite that only checks the + passing direction cannot notice that enforcement stopped. + """ + config_path = _write_offline_config(tmp_path) + + result = runner.invoke( + app, + ["test", str(config_path), "-t", "exact_match:gte:1.5"], + ) + output = _strip_ansi(result.output) + + # The metric must resolve (not "not found") and the failure must + # be reported against the real computed value (1.0). + assert "not found in results" not in output + assert "exact_match: 1.0000 NOT gte 1.5000" in output, output + assert result.exit_code == 1, output + + def test_test_command_after_completed_asyncio_run(self, tmp_path): + """Regression test for issue #261. + + ``asyncio.run()`` calls ``asyncio.set_event_loop(None)`` when it + completes, so afterwards ``asyncio.get_event_loop()`` in the main + thread raises ``RuntimeError`` instead of auto-creating a loop. + ``run_evaluation`` must not depend on that policy state: after a + completed ``asyncio.run`` in the same process, the evaluation must + still actually run and the threshold must be evaluated against the + real metrics — not swallowed into an ``error`` summary that the + CLI then reports as "not found in results". + """ + asyncio.run(asyncio.sleep(0)) + + config_path = _write_offline_config(tmp_path) + + result = runner.invoke( + app, + ["test", str(config_path), "-t", "exact_match:gte:0.5"], + ) + output = _strip_ansi(result.output) + + assert "not found in results" not in output + # The evaluation must have actually run: the threshold is + # evaluated against the real computed value (1.0) and passes. + assert "exact_match: 1.0000 gte 0.5000" in output, output + assert "PASS" in output, output + assert result.exit_code == 0, output