fix(cicd): populate metrics_summary so oaeval test can evaluate thresholds

[Nitjsefnie]

Description

oaeval test fails for every threshold regardless of the actual scores, because the gate summary it reads carries no metrics_summary. OAEvalPlugin.run_evaluation returns the EvaluationResult built by ThresholdEvaluator.evaluate_all_gates, whose summary holds only gate/threshold counts, so cli/commands/test.py looks up metrics_summary, gets {}, and every threshold resolves to actual=None → Metric '<name>' not found in results.

This populates metrics_summary in the summary the plugin returns, so the CLI's threshold evaluation sees the same metric values the plugin evaluated its own gates against. ThresholdEvaluator is unchanged.

It also fixes a second defect in the same function, which the regression tests for the first one uncovered — see "Why this PR contains two fixes" below.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Test update
• CI/CD update

Related Issues

Closes #228
Closes #261

How Has This Been Tested?

Reproduced first on unmodified main (b00ab48) with a fully offline config using the in-repo mock providers, which echo ground_truth so exact_match scores 1.0 and a gte:0.5 threshold must pass:

$ oaeval test config.yaml -t exact_match:gte:0.5 --json
...
"actual": null,
"message": "Metric 'exact_match' not found in results"
$ echo $?
1

oaeval run on the same config exits 0 and reports exact_match: 1.0, confirming the metric is computed and the threshold genuinely ought to pass. After the fix the same command exits 0 with "status": "passed" and "actual": 1.0.

Three regression tests were added, all driving the real oaeval test command end to end via CliRunner:

• a should-pass case asserting the actual comparison (exact_match: 1.0000 gte 0.5000) and a passing gate row — not merely the absence of the error string;
• a should-fail case (exact_match:gte:1.5) asserting exit code 1 and exact_match: 1.0000 NOT gte 1.5000;
• a case that completes an asyncio.run(...) first and then asserts the evaluation still runs (the [BUG] OAEvalPlugin.run_evaluation returns a silent error result in any process where asyncio.run has already completed #261 regression).

The first two were checked against a deliberately broken implementation in which missing metrics are treated as passing — that variant waves even gte:1.5 through as passed, and both tests fail on it, so the pair detects enforcement being silently skipped rather than only detecting the original symptom. Each test was also confirmed to fail with its corresponding source change reverted.

• Unit tests pass (uv run pytest tests/unit -v --tb=short → 1011 passed, 4 skipped; the 4 skips are pre-existing environment skips for chromadb and OPENAI_API_KEY)
• Whole suite with coverage, matching the CI Coverage job (uv run pytest --cov=openagent_eval --cov-report=term-missing --cov-fail-under=75 → 1065 passed, 4 skipped, 81.08%)
• Linter passes (uv run ruff check .) — see note below
• Type checker passes (uv run mypy openagent_eval/) — see note below
• Manual testing performed (the before/after run above)

Checklist

• My code follows the project's coding standards
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Additional Notes

Why this PR contains two fixes. The metrics_summary fix alone is not observable in any process that has already completed an asyncio.run(...), and I only found that out because the new regression tests passed in isolation and failed when the whole suite ran in one process. The cause is separate and is filed as #261: run_evaluation called asyncio.get_event_loop(), but asyncio.run sets the current loop to None when it finishes, so that call raises RuntimeError before the if loop.is_running() test is reached, and the broad except Exception converts it into a returned result with an error key — which the CLI then reports as "metric not found", i.e. indistinguishable from the bug being fixed here. The fix uses asyncio.get_running_loop() instead, so the behaviour no longer depends on the event-loop policy's current-loop state; the existing thread-pool offload and its timeout are unchanged for the case where a loop really is running.

I kept them together because the first fix is not demonstrably correct without the second, and separating them would mean landing a regression test that fails in your own CI. Happy to split it into two PRs if you would rather review them independently — say the word and I will.

On the two unchecked test boxes. ruff check . exits 1 on pre-existing findings at main itself, so I could not honestly tick that box for this branch and did not want to imply otherwise. Instead I compared per-file, at main and on this branch, for the two files this PR touches: identical results on both sides (UP035 and B904 in plugin.py, I001 in the test file, plus pre-existing format drift), so this diff adds no new findings. I have deliberately not reformatted or fixed those pre-existing findings, since that would bury a small behavioural fix in unrelated churn — happy to send that separately if you want it. The mypy box is unticked for the same reason: it is not clean at main, and this diff does not change that either way.

No documentation changes seemed warranted, as this restores the documented behaviour of existing functionality rather than changing it — say the word if you would like the CI/CD docs to call it out.

One thing worth your judgement. The mapping I store in metrics_summary is the plugin's threshold-target mapping, which is the engine's metric set plus the three execution counts the plugin already supports as gate targets (total_items, successful_evaluations, failed_evaluations). It is therefore a superset of what the engine puts in its own metrics_summary, and the code comment says so explicitly rather than claiming parity. I chose the superset deliberately: narrowing it to metrics only would make oaeval test -t total_items:gte:1 report "metric not found", which is the same class of failure this PR fixes. The tradeoff is that a consumer iterating metrics_summary as metric-name → value would now also see those three counts. Nothing in the repo does that with a plugin result today, but if you would rather keep the two summaries schema-identical, narrowing it is a one-line change — your call.

Generated by Claude Opus 5 (brief, review), Kimi K3 (implementation)

[himanshu231204]

/oc review pr

[opencode-agent]

Review: fix/228-gate-metrics-summary

Overall: ✅ Looks good, ready to merge

Two bugs, two fixes, clean code, solid tests.

---

Bug #228 — metrics_summary not populated

Root cause: OAEvalPlugin.run_evaluation calls ThresholdEvaluator.evaluate_all_gates(flat_metrics), which evaluates gates correctly but returns a EvaluationResult.summary containing only gate/threshold counts (lines 171-178 in thresholds.py). No metrics_summary key. Downstream in cli/commands/test.py:177, result.summary.get("metrics_summary", {}) returns {}, so every threshold gets actual=None → "Metric '...' not found in results".

Fix (plugin.py:248): stores flat_metrics in eval_result.summary["metrics_summary"] — the same dict the evaluator already consumed, so the CLI sees exactly what the evaluator used.

Bug #261 — asyncio.run aftermath kills evaluation

Root cause: Old code was asyncio.get_event_loop().is_running(). After any asyncio.run() completes in the same process, the policy's current loop is set to None, so get_event_loop() raises RuntimeError before is_running() is reached. The broad except Exception eats it and returns an error summary → CLI reports "metric not found", indistinguishable from #228.

Fix (plugin.py:188): uses asyncio.get_running_loop() — either returns the running loop or raises RuntimeError, independent of the policy's stale current-loop state. Clean.

Tests — thorough, with deliberate anti-regression design

Three CliRunner-based end-to-end tests:

Test	Threshold	Asserts
passing_threshold_is_not_reported_missing	exact_match:gte:0.5	✓ exit 0, ✓ actual 1.0000, ✓ "PASS", ✗ "not found"
failing_threshold_is_enforced	exact_match:gte:1.5	✓ exit 1, ✓ "1.0000 NOT gte 1.5000", ✗ "not found"
after_completed_asyncio_run	exact_match:gte:0.5	same as first, but preceded by asyncio.run(asyncio.sleep(0))

The bidirectional pair (pass + fail) is important — the author verified both detect a deliberately broken implementation that silently skips missing metrics.

Minor notes (none blocking)

• Double evaluation: CLI's test_command parses thresholds into its own cicd_config, calls run_evaluation (which does not receive them since thresholds isn't passed), then re-evaluates its gates against the result's metrics_summary. Pre-existing, not introduced here. Could be cleaned up in a follow-up by either (a) passing thresholds to run_evaluation and reading its .passed, or (b) having run_evaluation skip eval and defer entirely to the CLI.
• flat_metrics superset of engine metrics: includes total_items, successful_evaluations, failed_evaluations in addition to metric names. Intentionally documented; no downstream consumer breaks because nothing iterates plugin result metrics_summary as a pure metric dict today.
• No new lint/violations: confirmed identical per-file findings at main vs this branch (UP035, B904 in plugin.py; I001 in test file).

opencode session  |  github run

[Nitjsefnie]

Thanks for running that. On the bot's first note — we noticed the same double-evaluation path while working on this: test_command parses thresholds into its own cicd_config, calls run_evaluation without passing them, then re-evaluates against the returned metrics_summary. It is pre-existing and harmless once this lands, so I left it alone here rather than widening the diff.

Happy to send it as a separate PR if you want it — your pick between passing thresholds straight into run_evaluation and reading its .passed, or having run_evaluation skip evaluation and defer to the CLI entirely.

[github-actions]

🎉 Congratulations @Nitjsefnie!

Your pull request has been successfully merged into main. 🚀

Thank you for contributing to OpenAgentHQ and helping improve the project.

We truly appreciate your contribution and hope to see you back with more amazing PRs!

Happy Open Sourcing! ❤️