Uh oh!
There was an error while loading. Please reload this page.
feat(metrics): Adherence To Prompt - #1058
Conversation
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
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 |
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
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
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