fix(evals): Update Judge Metrics Prompt - #1104
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughEvaluation judging now uses integer scores from 0 to 5. Validation, judge prompts, verdict thresholds, consistency thresholds, persistence fixtures, documentation, and tests were updated for the new scale. ChangesEvaluation scoring
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…kend into fix/judge-metrics-prompt-update
OpenAPI changes ⚪ No API surface changesNote This PR does not modify the API contract.
|
…kend into fix/judge-metrics-prompt-update
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/app/crud/evaluations/judge.py (1)
249-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the score bounds.
Line 249 embeds
0and5directly in validation. DefineMIN_JUDGE_SCOREandMAX_JUDGE_SCOREand use them in validation and error text. This keeps the executable score contract in one place during future scale changes.As per coding guidelines, do not use magic values; extract repeated literals into constants, enums, or settings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/crud/evaluations/judge.py` around lines 249 - 253, Define MIN_JUDGE_SCORE and MAX_JUDGE_SCORE near the judge score validation code, then replace the hard-coded 0 and 5 in the condition with those constants. Update the ValueError message to interpolate the same bounds, keeping the existing integer conversion and validation behavior unchanged.Source: Coding guidelines
backend/app/tests/crud/evaluations/test_run_ai_summary.py (1)
58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a narrow summary-score fixture type.
list[dict]does not specify the requiredname,avg, andstdfields. Define aTypedDictfor this fixture and use it for_summary_scoresand the adjacent_callparameter.As per coding guidelines, “Use Python 3.11+ and provide narrow type hints for every function parameter and return value; do not use
-> Anyas a substitute for a specific annotation.”Proposed fix
+class SummaryScore(TypedDict): + name: str + avg: float + std: float + -def _summary_scores() -> list[dict]: +def _summary_scores() -> list[SummaryScore]:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/tests/crud/evaluations/test_run_ai_summary.py` around lines 58 - 64, Define a narrow TypedDict containing the required name, avg, and std fields, then use it as the return type of _summary_scores and the corresponding _call parameter type. Keep the existing fixture values and behavior unchanged, and avoid broad dict or Any annotations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/crud/evaluations/judge.py`:
- Line 154: Update the MetricScore.score annotation in
backend/app/crud/evaluations/judge.py at lines 154-154 from float to int,
matching its integer parsing contract. Also update the _both_metrics_response
score tuple annotations in backend/app/tests/crud/evaluations/test_fast_judge.py
at lines 172-173 from tuple[float, str] to tuple[int, str].
- Around line 249-253: Update the judge score validation around the score
parsing and normalization logic to reject boolean values, catch OverflowError
from float conversion, and validate math.isfinite(score) before int(score).
Ensure all invalid cases raise the documented ValueError so the row is isolated,
and add regression coverage for true, false, Infinity, and NaN.
---
Nitpick comments:
In `@backend/app/crud/evaluations/judge.py`:
- Around line 249-253: Define MIN_JUDGE_SCORE and MAX_JUDGE_SCORE near the judge
score validation code, then replace the hard-coded 0 and 5 in the condition with
those constants. Update the ValueError message to interpolate the same bounds,
keeping the existing integer conversion and validation behavior unchanged.
In `@backend/app/tests/crud/evaluations/test_run_ai_summary.py`:
- Around line 58-64: Define a narrow TypedDict containing the required name,
avg, and std fields, then use it as the return type of _summary_scores and the
corresponding _call parameter type. Keep the existing fixture values and
behavior unchanged, and avoid broad dict or Any annotations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cc4ff613-9d92-43fe-8410-49431b605113
📒 Files selected for processing (13)
backend/app/api/docs/evaluation/create_evaluation_v2.mdbackend/app/api/docs/evaluation/improve_prompt_v2.mdbackend/app/crud/evaluations/judge.pybackend/app/crud/evaluations/score.pybackend/app/crud/evaluations/summary.pybackend/app/services/evaluations/prompt_improvement.pybackend/app/tests/api/routes/test_improve_prompt_v2.pybackend/app/tests/crud/evaluations/test_fast_judge.pybackend/app/tests/crud/evaluations/test_judge.pybackend/app/tests/crud/evaluations/test_overall_summary.pybackend/app/tests/crud/evaluations/test_run_ai_summary.pybackend/app/tests/crud/evaluations/test_score.pybackend/app/tests/services/evaluations/test_evaluation_service_s3.py
| @dataclass | ||
| class MetricScore: | ||
| """One metric's outcome for a row: score in [0, 1] and its reasoning.""" | ||
| """One metric's outcome for a row: integer score in 0–5 and its reasoning.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files 'backend/app/**' | rg '(^backend/app/crud/evaluations/judge\.py$|backend/app/tests/crud/evaluations/test_fast_judge\.py$)' || true
echo
echo "judge.py outline:"
ast-grep outline backend/app/crud/evaluations/judge.py --view expanded || true
echo
echo "Relevant judge.py lines:"
sed -n '120,190p' backend/app/crud/evaluations/judge.py
echo
echo "Relevant test_fast_judge.py lines:"
sed -n '140,210p' backend/app/tests/crud/evaluations/test_fast_judge.py
echo
echo "Score annotation usages in judge.py:"
rg -n "MetricScore|\.score|ground_truth|prompt|tuple\[ (float|int) , str \]" backend/utils backend/app/crud/evaluations backend/app/tests/crud/evaluations || trueRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "judge.py `_parse_metric_score`:"
sed -n '230,260p' backend/app/crud/evaluations/judge.py
echo
echo "test_fast_judge.py helper around _both_metrics_response:"
sed -n '160,184p' backend/app/tests/crud/evaluations/test_fast_judge.py
echo
echo "target annotations:"
python3 - <<'PY'
from pathlib import Path
for path in [
Path("backend/app/crud/evaluations/judge.py"),
Path("backend/app/tests/crud/evaluations/test_fast_judge.py"),
]:
text = path.read_text()
for needle in [" score: float", "ground_truth: tuple[float, str]", "prompt: tuple[float, str]"]:
idx = text.find(needle)
print(f"{path}:{idx+1}:{needle}")
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 2816
Use an integer contract for judge scores.
MetricScore.score is parsed as an integer in backend/app/crud/evaluations/judge.py#L237, but the dataclass still annotates it as float. Update that field to int and change _both_metrics_response score tuples in backend/app/tests/crud/evaluations/test_fast_judge.py#L172-173 from tuple[float, str] to tuple[int, str] so the contracts stay aligned.
📍 Affects 2 files
backend/app/crud/evaluations/judge.py#L154-L154(this comment)backend/app/tests/crud/evaluations/test_fast_judge.py#L172-L173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/crud/evaluations/judge.py` at line 154, Update the
MetricScore.score annotation in backend/app/crud/evaluations/judge.py at lines
154-154 from float to int, matching its integer parsing contract. Also update
the _both_metrics_response score tuple annotations in
backend/app/tests/crud/evaluations/test_fast_judge.py at lines 172-173 from
tuple[float, str] to tuple[int, str].
Source: Coding guidelines
| if score != int(score) or not 0 <= score <= 5: | ||
| raise ValueError( | ||
| f"metric '{key.value}' score must be an integer 0-5: {raw.get('score')!r}" | ||
| ) | ||
| score = int(score) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python - <<'PY'
import json
for literal in ("true", "false", "Infinity", "NaN"):
raw = json.loads(f'{{"score": {literal}}}')["score"]
score = float(raw)
try:
normalized = int(score)
except Exception as exc:
normalized = type(exc).__name__
print(literal, repr(raw), normalized)
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'File candidates:\n'
fd -a 'judge\.py$' . | sed 's#^\./##'
printf '\nRelevant file outline:\n'
ast-grep outline backend/app/crud/evaluations/judge.py --view compact || true
printf '\nRelevant source lines 200-270:\n'
sed -n '200,270p' backend/app/crud/evaluations/judge.py | nl -ba -v200
printf '\nSearch for parse_score_usages/ValueError handling near this file:\n'
rg -n "parse_scores|ValueError|score must be an integer|metric '" backend/app/crud/evaluations backend/app -g '*.py' | head -200Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 632
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant source lines 200-270:\n'
sed -n '200,270p' backend/app/crud/evaluations/judge.py | awk '{printf "%6d\t%s\n", NR+199, $0}'
printf '\nSearch for parse_scores/usages and related validation:\n'
rg -n "parse_scores|ValueError|score must be an integer|metric '" backend/app/crud/evaluations backend/app -g '*.py' | head -200
printf '\nRelevant tests around score validation:\n'
rg -n "score|ValueError|boolean|Infinity|NaN" backend/app/tests/crud/evaluations/test_judge.py backend/app/tests/crud/evaluations/test_fast_judge.py -g '*.py' | head -200Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 48273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from typing import Any
from pathlib import Path
src = Path('backend/app/crud/evaluations/judge.py').read_text()
start = src.index('def _parse_metric_score')
end = src.index('\ndef _parse_judge_output', start)
code = src[start:end]
print(code)
print('\nBehavioral probes:')
for literal, raw_value in [('true', True), ('false', False), ('Infinity', 'Infinity'), ('NaN', 'NaN')]:
print(f'>{literal!r}:')
try:
score = float(raw_value)
print(f'float={score!r}')
print(f'isfinite=bool={score != score if score != score else (score == score)}')
if isinstance(score, float):
try:
normalized = int(score)
print(f'int={normalized!r}')
except Exception as exc:
print(f'int_exception={type(exc).__name__}:{exc}')
else:
print('not a float')
except Exception as exc:
print(f'float_exception={type(exc).__name__}:{exc}')
PY
python3 - <<'PY'
from typing import Any
from pathlib import Path
src = Path('backend/app/crud/evaluations/judge.py').read_text()
start = src.index('def _parse_judge_output')
end = src.index('\ndef _judge', end)
code = src[start:end]
print(code)
# Inspect exact exception handling around _parse_judge_output using static source.
lines = code.splitlines()
for i, line in enumerate(lines, 1):
if 'except' in line or '_parse_metric_score' in line or 'except ValueError' in line:
print(f'{i:04d}: {line}')
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1595
Reject boolean and non-finite judge scores before normalization.
float() converts JSON true and false to 1.0 and 0.0, so malformed boolean scores can be stored as valid scores. JSON Infinity raises OverflowError from int(score), bypassing the documented ValueError path that isolates the row.
Reject booleans, check math.isfinite(score) before converting to int, and catch OverflowError from float(). Add regression tests for true, false, Infinity, and NaN.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/crud/evaluations/judge.py` around lines 249 - 253, Update the
judge score validation around the score parsing and normalization logic to
reject boolean values, catch OverflowError from float conversion, and validate
math.isfinite(score) before int(score). Ensure all invalid cases raise the
documented ValueError so the row is isolated, and add regression coverage for
true, false, Infinity, and NaN.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
🎉 This PR is included in version 1.4.0-main.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
2 participants
Summary
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.