Repository navigation
Fix/scoring and gate bugs - #6
Conversation
- numeric_diff scores 1.0 within rel_tol/abs_tol, so the tolerance is no longer overridden by the default threshold of 1.0. - evaluate_suite skips its implicit exact-match check when a reference scorer grades against expected_output. - GateConfig.min_average_score and max_failed_cases default to None; the default gate still requires every case to pass. - tool_call_precision and no_redundant_tool_calls return a score on a trace with no tool calls instead of raising and aborting the run. - RubricTemplate.parse_verdict reads the stated verdict rather than the first standalone letter, and raises on an ambiguous completion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PRAgent ReviewVerdict: Reviewed 18 changed file(s) and found 5 issue(s). Final Pre-Merge SummaryChanged files: CHANGELOG.md (modified, +30/-0); docs/index.html (modified, +1/-1); examples/near_miss_similarity.py (modified, +8/-8); pyproject.toml (modified, +1/-1); src/agentic_evals/init.py (modified, +1/-1); src/agentic_evals/gate.py (modified, +12/-4); src/agentic_evals/runner.py (modified, +10/-1); src/agentic_evals/scorers/rubric.py (modified, +55/-14); src/agentic_evals/scorers/text.py (modified, +26/-7); src/agentic_evals/scorers/trajectory.py (modified, +22/-5); 8 more file(s). Impact On Existing CodeThe PR may break or weaken existing behavior because high-severity findings were detected. The owner should require fixes or explicit justification before merge. Owner Permission RequiredOwner approval required: do not merge until the owner confirms fixes or explicitly accepts the risk. Quality GateQuality gate retained 5 actionable finding(s) and suppressed 1 low-signal or informational finding(s).
Code ReviewCode review of PR #6: Fixes for scoring, gate, and rubric parsing behaviors. Security ReviewNo security issues found in the PR diff; fixes improve correctness and robustness without introducing vulnerabilities. Findings1. GateConfig defaults changed to optional, changing gate behavior
Location: The fields min_average_score and max_failed_cases changed default values from a strict check (1.0 and 0) to None (not checked). This changes the operational behavior of gates that rely on these parameters, potentially passing when previously they would have failed. Recommendation: Review all existing gate configurations and usage to ensure explicit settings are applied where the original strict behavior is desired. Document clearly this change to users. 2. Skipping implicit exact-match check when a reference scorer grades expected_output
Location: The evaluate_suite function now skips the exact-match scoring check if the case has a reference grader (one of predefined scorer names) grading against expected_output. This changes logic so that exact-match is not forced when similarity or other reference scorers are configured without their own reference. Recommendation: Ensure this adjustment is well understood and clearly documented for users who may rely on strict exact matching. Consider documenting when this fallback behavior applies. 3. RubricTemplate.parse_verdict now stricter and more complex
Location: The verdict parser now looks for explicit verdicts (bare token, with 'Verdict:' or similar, parenthetical, leading or trailing token, etc.) and raises an exception if ambiguous or missing, instead of picking first standalone letter. This improves correctness but may break some existing rubric outputs that relied on looser matching. Recommendation: Confirm all rubric completions comply with the new stricter parser. Add or update tests for rubric parsing edge cases and communicate the breaking change clearly. 4. tool_call_precision and no_redundant_tool_calls scorers no longer raise on empty tool call traces
Location: These scorers used to raise an exception if the trace contained no tool calls, aborting evaluation. Now they return fixed scores (0.0 and 1.0 respectively). This improves robustness but changes semantics and may impact existing workflows. Recommendation: Review downstream consumers to ensure they handle these scores appropriately and do not depend on exceptions. Document this behavioral change to users. 5. numeric_diff scorer scores 1.0 within tolerance instead of just under 1.0
Location: Previously, numeric_diff scored just under 1.0 when actual output was within tolerance thresholds, causing failures with a default threshold of 1.0. Now it explicitly returns 1.0 if within tolerance, matching thresholds better. Recommendation: No changes needed; this fix improves correctness. Confirm numeric_diff users are aware this change may affect pass/fail for edge cases. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfb5245d1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _graded_against_expected_output(case: TestCase) -> bool: | ||
| """True when a reference scorer on the case grades against `expected_output` itself.""" | ||
| return any( | ||
| config.name in REFERENCE_SCORER_NAMES and not config.config.get("reference") |
There was a problem hiding this comment.
When a case has expected_output and a reference scorer with required=False, this condition suppresses the implicit required exact-match check even though the scorer's failure is later ignored. For example, an optional levenshtein_similarity without its own reference can score 0 while the case still passes, making the configured expected output entirely non-gating; only a required reference scorer should replace the exact-match criterion.
Useful? React with 👍 / 👎.
| alternatives = [f"(?i:{'|'.join(words)})"] if words else [] | ||
| if letters: | ||
| alternatives.append("|".join(letters)) |
There was a problem hiding this comment.
Single-letter alternatives are now case-sensitive outside the bare-token branch, so natural judge completions such as Verdict: d, I choose (d), or d. because ... raise ValueError and abort evaluation, even though bare d is accepted and the previous implementation normalized the whole completion to uppercase. Make the explicit marked, parenthesized, leading, and trailing forms case-insensitive while retaining stricter handling only for ambiguous prose fallback matches.
Useful? React with 👍 / 👎.
2 participants
Summary
Fixes five cases where the engine returned a wrong result or aborted a run. Two of the fixes change behaviour for existing users; see "Behaviour changes" below.
Fixes
numeric_diffignored its tolerance. An output withinrel_tol/abs_tolscored just under 1.0, so the default threshold of 1.0 failed every non-identical value. It now scores 1.0 when within tolerance.expected_outputwas set.evaluate_suitealways added a strict exact-match check for that field, so a case usinglevenshtein_similarityat threshold 0.9 failed at 0.96. The implicit check is now skipped when a reference scorer is grading againstexpected_output.GateConfig(min_pass_rate=0.95)still required zero failed cases and an average score of 1.0.min_average_scoreandmax_failed_casesnow default toNone(not checked).tool_call_precisionandno_redundant_tool_callsraisedValueError. They now return a score: 0.0 and 1.0 respectively.Behaviour changes
GateConfig()still requires every case to pass (min_pass_rate=1.0). A suite where every case passes but the average score is below 1.0 used to fail the default gate and now passes. To keep the previous behaviour, setmin_average_score=1.0, max_failed_cases=0explicitly.expected_outputplus one oflevenshtein_similarity,embedding_similarity,json_diff,numeric_diff,starts_with, orends_withno longer requires an identical string. The skip is keyed on these built-in scorer names, and only applies when the scorer has noreferenceof its own in its config.Not changed
CallableEvaluatorstill derivespassedfromvalue >= thresholdand takesrequiredfrom the evaluator config.LLMRubricEvaluator, as the skills document.Also updated
CHANGELOG.md: new Unreleased section.define-a-release-gateandwrite-a-scorerskills, andexamples/near_miss_similarity.py, which described the old behaviour.Testing
ruff check,ruff format --check, andmypypass.pytest: 129 passed. New tests cover each fix; two existing tests that asserted the old behaviour were updated (test_missing_sample_fails_gate,test_parse_verdict_prefers_earliest_match), andtest_tool_call_precision_requires_a_tool_callwas replaced.examples/andvoice-agent-evals/run_evals.pyrun clean.