feat(metrics): Adherence To Prompt#1058
Conversation
github-actions
Bot
changed the title
OpenAPI changes 🟢 5 non-breaking changesTip Safe to merge from an API-contract perspective. Full changelog ·
|
| Method | Path | Change | |
|---|---|---|---|
| 🟢 | GET |
/api/v1/evaluations |
added the optional property data/anyOf[subschema #1]/items/is_judge_run to the response with the 200 status |
| 🟢 | POST |
/api/v1/evaluations |
added the optional property data/anyOf[subschema #1: EvaluationRunPublic]/is_judge_run to the response with the 200 status |
| 🟢 | GET |
/api/v1/evaluations/{evaluation_id} |
added the optional property data/anyOf[subschema #1: EvaluationRunPublic]/is_judge_run to the response with the 200 status |
| 🟢 | POST |
/api/v2/evaluations |
endpoint added |
| 🟢 | POST |
/api/v2/evaluations/datasets |
endpoint added |
main ↔ 3510e3bc · generated by oasdiff
Ayush8923
changed the title
Ayush8923
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| op.add_column( | ||
| "evaluation_run", | ||
| sa.Column( | ||
| "per_item_ground_truth", | ||
| JSONB(), | ||
| nullable=True, | ||
| comment=( | ||
| "Durable {ref: score} map of the Adherence to Ground Truth judge " | ||
| "scores (ref = trace_id when traced, else item_id); Kaapi's own store" | ||
| ), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Now, this column is not needed. because this column is not used anywhere.
📝 WalkthroughWalkthroughV2 evaluation runs now perform combined native judge scoring with prompt-adherence support. Configuration prompts gate metric availability, judge usage uses one ChangesNative judge evaluation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EvaluationRun
participant Stage3
participant ConfigResolver
participant JudgeRow
Stage3->>ConfigResolver: resolve instructions and prompt template
ConfigResolver-->>Stage3: config_prompt or unavailable
Stage3->>JudgeRow: judge row with selected metrics
JudgeRow-->>Stage3: combined ground-truth and prompt scores
Stage3->>EvaluationRun: persist cost and unscoreable state
Possibly related PRs
Suggested labels: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/crud/evaluations/judge.py (1)
285-317: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject judge responses that omit any enabled metric.
Line 312 accepts partial JSON, so a response containing only
ground_truthsilently dropspromptwithout marking the row failed. This produces biased trace and summary coverage. Validate every enabled key and raiseValueErrorwhen one is missing.🤖 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 285 - 317, Update _parse_judge_output to validate that every enabled metric in metrics has a corresponding key in the parsed data object before returning results. Raise ValueError when any metric is missing, while preserving the existing score parsing for complete responses and invalid-value handling.
🤖 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/cost.py`:
- Around line 118-119: Update the eval_run.cost carry-forward logic for writing
the combined judge stage so legacy per-metric keys such as ground_truth_judge
are removed before judge is persisted. Ensure deployment-retry and
partial-update flows retain only the combined judge entry and add coverage for
this behavior.
In `@backend/app/crud/evaluations/fast.py`:
- Around line 783-793: Update the prompt-section construction around the prompt
template check so the template is stripped before being appended. Only add the
PROMPT_TEMPLATE_LABEL section when the stripped template content is non-empty,
ensuring whitespace-only templates do not produce scoring input; preserve the
existing instructions handling and section joining.
- Around line 1064-1068: Update the judgeable-row filtering around the
response-results comprehension so prompt-adherence rows are retained when
generated_output exists even if ground_truth is empty. Separate row-level metric
selection from ground-truth-dependent metrics, and invoke the combined scoring
path with only the prompt-adherence metric when ground truth is unavailable,
while preserving existing behavior for metrics that require ground truth.
In `@backend/app/crud/evaluations/judge.py`:
- Around line 113-123: Rename enabled_metric_specs to list_enabled_metric_specs
to follow the repository’s plural-fetch naming convention, preserving its
filtering and return behavior. Update every caller, including orchestration code
and tests, to use the renamed function.
In `@backend/app/tests/crud/evaluations/test_fast_judge.py`:
- Around line 158-163: Complete the callable annotations across the affected
sites: in backend/app/tests/crud/evaluations/test_fast_judge.py lines 158-163,
annotate usage and add a precise return type to _both_metrics_response; in lines
326-800, add -> None to changed test methods and type all local judge callback
parameters and returns; in
backend/app/alembic/versions/075_add_judge_columns_to_evaluation_run.py lines
20-36, add -> None to upgrade and downgrade; and in
backend/app/tests/crud/evaluations/test_judge.py lines 56-60, annotate the
helper return as list[JudgeMetricSpec]. Use narrowed concrete types rather than
Any.
---
Outside diff comments:
In `@backend/app/crud/evaluations/judge.py`:
- Around line 285-317: Update _parse_judge_output to validate that every enabled
metric in metrics has a corresponding key in the parsed data object before
returning results. Raise ValueError when any metric is missing, while preserving
the existing score parsing for complete responses and invalid-value handling.
🪄 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
Run ID: 1f11cae0-4935-45bb-b4bd-f0c2e878cd76
📒 Files selected for processing (11)
backend/app/alembic/versions/075_add_judge_columns_to_evaluation_run.pybackend/app/api/routes/evaluations/dataset_v2.pybackend/app/api/routes/evaluations/evaluation_v2.pybackend/app/crud/evaluations/cost.pybackend/app/crud/evaluations/fast.pybackend/app/crud/evaluations/judge.pybackend/app/crud/evaluations/score.pybackend/app/models/evaluation.pybackend/app/tests/api/routes/test_evaluation_v2.pybackend/app/tests/crud/evaluations/test_fast_judge.pybackend/app/tests/crud/evaluations/test_judge.py
💤 Files with no reviewable changes (1)
- backend/app/models/evaluation.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/tests/crud/evaluations/test_fast_judge.py (1)
837-909: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd annotations to the new test callables.
The added helpers, test methods, and mock callbacks omit parameter and/or return annotations. Annotate test methods with
-> Noneand give callbacks concrete parameter/return types; also resolve Ruff’s unusedopenai_clientcallback parameter without weakening the mock contract.
backend/app/tests/crud/evaluations/test_fast_judge.py#L837-L909: annotate response helpers and response-chunk tests.
backend/app/tests/crud/evaluations/test_fast_judge.py#L1127-L1179: annotate_fake_calland file-search tests.
backend/app/tests/crud/evaluations/test_judge.py#L108-L134: annotate parser tests.
backend/app/tests/crud/evaluations/test_judge.py#L206-L249: annotate applicability tests.
backend/app/tests/crud/evaluations/test_judge.py#L313-L387: annotate row tests and_capturecallbacks.
backend/app/tests/crud/evaluations/test_judge.py#L444-L477: annotate enabled-metric tests.As per coding guidelines, “Use type hints on every function parameter and return value;
-> Anyis not acceptable unless the type can be narrowed.”🤖 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_fast_judge.py` around lines 837 - 909, Annotate all added helpers, test methods, and callbacks with concrete parameter and return types, using -> None for tests and avoiding -> Any unless unavoidable. In backend/app/tests/crud/evaluations/test_fast_judge.py:837-909, update _responses_item, _openai_response, and TestResponsesChunkCapture methods; at 1127-1179, annotate _fake_call and file-search tests, resolving its unused openai_client parameter without weakening the mock contract. Apply the same annotation updates to parser tests at backend/app/tests/crud/evaluations/test_judge.py:108-134, applicability tests at 206-249, row tests and _capture callbacks at 313-387, and enabled-metric tests at 444-477.Sources: Coding guidelines, Linters/SAST tools
backend/app/crud/evaluations/judge.py (1)
50-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the declared input order.
Line 54 says new inputs are appended last, but
CONFIG_PROMPTis inserted before the existing inputs at Line 57. Since the input-union/rendering machinery uses enum declaration order, moveCONFIG_PROMPTafterRETRIEVED_CHUNKSto preserve the documented prompt ordering.Proposed fix
- CONFIG_PROMPT = "config_prompt" QUESTION = "question" GENERATED_ANSWER = "generated_answer" GOLDEN_ANSWER = "golden_answer" RETRIEVED_CHUNKS = "retrieved_chunks" + CONFIG_PROMPT = "config_prompt"🤖 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 50 - 61, Update the declaration order in JudgeInputEnum by moving CONFIG_PROMPT after RETRIEVED_CHUNKS. Preserve the existing order of QUESTION, GENERATED_ANSWER, GOLDEN_ANSWER, and RETRIEVED_CHUNKS so rendered prompts remain unchanged.
🧹 Nitpick comments (1)
backend/app/tests/crud/evaluations/test_fast_judge.py (1)
868-905: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated test model literal.
"gpt-4o"is repeated across response and chunk test setup. Use oneTEST_MODELconstant to avoid test drift.Proposed refactor
+TEST_MODEL = "gpt-4o" + - base_params={"model": "gpt-4o"}, + base_params={"model": TEST_MODEL}, ... - return_value=({"model": "gpt-4o", "tools": tools}, []), + return_value=({"model": TEST_MODEL, "tools": tools}, []), ... - config=TextLLMParams(model="gpt-4o"), + config=TextLLMParams(model=TEST_MODEL),As per coding guidelines, “Do not use magic values in code; extract repeated literals into constants,
Enummembers, 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/tests/crud/evaluations/test_fast_judge.py` around lines 868 - 905, Extract the repeated "gpt-4o" test model value into a shared TEST_MODEL constant in backend/app/tests/crud/evaluations/test_fast_judge.py, then replace the occurrences in the response and chunk test setup at lines 868-905 and 1137-1155 with that constant to prevent test drift.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 `@docs/wiki/modules/evaluations.md`:
- Line 23: Update the “Judging is system-config only” statement in the
EvaluationRun documentation to say the judge uses the configured
EVAL_JUDGE_MODEL, with gpt-5-mini as the default, rather than always using the
fallback model. Preserve the existing built-in prompt and per-run configuration
constraints.
---
Outside diff comments:
In `@backend/app/crud/evaluations/judge.py`:
- Around line 50-61: Update the declaration order in JudgeInputEnum by moving
CONFIG_PROMPT after RETRIEVED_CHUNKS. Preserve the existing order of QUESTION,
GENERATED_ANSWER, GOLDEN_ANSWER, and RETRIEVED_CHUNKS so rendered prompts remain
unchanged.
In `@backend/app/tests/crud/evaluations/test_fast_judge.py`:
- Around line 837-909: Annotate all added helpers, test methods, and callbacks
with concrete parameter and return types, using -> None for tests and avoiding
-> Any unless unavoidable. In
backend/app/tests/crud/evaluations/test_fast_judge.py:837-909, update
_responses_item, _openai_response, and TestResponsesChunkCapture methods; at
1127-1179, annotate _fake_call and file-search tests, resolving its unused
openai_client parameter without weakening the mock contract. Apply the same
annotation updates to parser tests at
backend/app/tests/crud/evaluations/test_judge.py:108-134, applicability tests at
206-249, row tests and _capture callbacks at 313-387, and enabled-metric tests
at 444-477.
---
Nitpick comments:
In `@backend/app/tests/crud/evaluations/test_fast_judge.py`:
- Around line 868-905: Extract the repeated "gpt-4o" test model value into a
shared TEST_MODEL constant in
backend/app/tests/crud/evaluations/test_fast_judge.py, then replace the
occurrences in the response and chunk test setup at lines 868-905 and 1137-1155
with that constant to prevent test drift.
🪄 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
Run ID: bf7e844c-06ca-40aa-992d-c7eb11c6361f
📒 Files selected for processing (7)
backend/app/api/docs/evaluation/create_evaluation_v2.mdbackend/app/crud/evaluations/fast.pybackend/app/crud/evaluations/judge.pybackend/app/crud/evaluations/score.pybackend/app/tests/crud/evaluations/test_fast_judge.pybackend/app/tests/crud/evaluations/test_judge.pydocs/wiki/modules/evaluations.md
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/crud/evaluations/score.py
- backend/app/crud/evaluations/fast.py
|
|
||
| Key `EvaluationRun` JSONB fields: `score` (per-trace `scores` + `summary_scores`), `per_item_scores`, `cost` (per-stage), `unscoreable`. References `config_id`, `batch_job_id`. | ||
| v2 judge fields on `EvaluationRun`: `is_judge_run` (bool marker gating native judging + Langfuse skip), `per_item_ground_truth` (JSONB `{ref: score}`, Kaapi-native, not synced to Langfuse) backing the `ground_truth` metric. The `knowledge_base` metric has no backup column — its per-row score + reasoning live in the `score_trace_url` trace unit (the native source of truth). Judging is system-config only — always the fallback model (`gpt-5-mini`) + built-in prompt, no per-run config. | ||
| v2 judge field on `EvaluationRun`: `is_judge_run` (bool marker gating native judging + Langfuse skip). All judge metrics are trace-only — per-row score + reasoning live in the `score_trace_url` trace unit (the native source of truth); there is no per-metric backup column. Judging is system-config only — always the fallback model (`gpt-5-mini`) + built-in prompts, no per-run config. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the judge model as configurable.
The implementation reads settings.EVAL_JUDGE_MODEL, and backend/app/api/docs/evaluation/create_evaluation_v2.md describes gpt-5-mini as its default. Replace “always the fallback model” with “the configured EVAL_JUDGE_MODEL (default gpt-5-mini)” so deployments using an override are documented correctly.
🤖 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 `@docs/wiki/modules/evaluations.md` at line 23, Update the “Judging is
system-config only” statement in the EvaluationRun documentation to say the
judge uses the configured EVAL_JUDGE_MODEL, with gpt-5-mini as the default,
rather than always using the fallback model. Preserve the existing built-in
prompt and per-run configuration constraints.
2 participants
Issue
Closes #1046
Summary
Adds the second judge metric,
Adherence to Prompt— to the v2 native LLM-as-Judge, and simplifies how judge results are stored.Before: the v2 judge scored one metric (Adherence to Ground Truth), and each metric wrote its own per-row DB column.
Now: one combined call per row returns both metrics, each explicitly scoped to its own inputs, and per-row results live in a single place (the run's trace file) instead of duplicate DB columns.
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.Summary by CodeRabbit
New Features
is_judge_runflag to indicate native judged-run behavior.Bug Fixes
Documentation
Tests