feat(evals): Update prompt iteration flow - #1080
Conversation
📝 WalkthroughWalkthroughAdds a v2 prompt-improvement endpoint for judged evaluation runs, typed recommendation callbacks, metric-aware LLM drafting, preserved v1 behavior, and route/worker tests covering validation, callbacks, redelivery, and metric handling. ChangesV2 prompt improvement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant PromptImprovementV2Route
participant PromptImprovementService
participant EvaluationRun
participant LLM
participant CallbackURL
Client->>PromptImprovementV2Route: POST improve-prompt with callback_url
PromptImprovementV2Route->>PromptImprovementService: enqueue with require_judge_run=true
PromptImprovementService->>EvaluationRun: validate completed judged run and score trace
PromptImprovementService-->>Client: 202 job handle
PromptImprovementService->>LLM: draft prompt from metric scores and reasoning
LLM-->>PromptImprovementService: structured prompt recommendation
PromptImprovementService->>CallbackURL: POST APIResponse with PromptRecommendationJobPublic
🚥 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 |
OpenAPI changes 🟢 1 non-breaking changeTip Safe to merge from an API-contract perspective. Full changelog ·
|
| Method | Path | Change | |
|---|---|---|---|
| 🟢 | POST |
/api/v2/evaluations/{evaluation_id}/improve-prompt |
endpoint added |
main ↔ 0fac46ff · generated by oasdiff
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/evaluations/prompt_improvement.py (2)
390-402: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winEarly failures on a v2 job emit the v1 error payload.
is_judge_runstaysFalseuntil re-validation succeeds, so any failure before line 402 (re-validation raising, or a soft time limit) sendsPromptImprovementJobPublicfor a job the client requested via the v2 endpoint. Same root cause as the redelivery fallback: the judge flag isn't known/durable outside the happy path.🤖 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/services/evaluations/prompt_improvement.py` around lines 390 - 402, Initialize the payload-version state from durable job/request data before the try block, rather than defaulting is_judge_run to False. Update the early failure and timeout callback paths around the prompt-improvement job so v2 requests consistently emit the v2 payload even when validate_improve_prompt fails or run is not yet bound, while preserving the existing assignment from the validated run when available.
644-654: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRe-raise Celery/timeouts before the catch-all in the LLM path.
SoftTimeLimitExceededis defined as anExceptionsubclass, so the broad handler here masks the task’s soft-time_limit control flow and converts it intoprompt_generation_failedinstead of reaching the worker’s timeout branch. Re-raiseSoftTimeLimitExceededandTimeoutbeforeexcept Exception.🛡️ Proposed fix
+ except (Timeout, SoftTimeLimitExceeded): + # Let the worker's timeout branch own these; they are not LLM faults. + raise + except Exception as exc: logger.error( f"[_call_prompt_drafting_llm] [KAAPI] Unexpected error during LLM call "🤖 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/services/evaluations/prompt_improvement.py` around lines 644 - 654, Update the exception handling around the LLM call to add dedicated handlers before the catch-all `except Exception` in the prompt-generation path. Re-raise both Celery `SoftTimeLimitExceeded` and the relevant `Timeout` exception unchanged so the worker timeout branch receives them; preserve the existing logging and `RuntimeError` conversion for other unexpected exceptions.
🧹 Nitpick comments (6)
backend/app/tests/api/routes/test_improve_prompt_v2.py (2)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: import
Iteratorfromcollections.abc.Ruff UP035;
typing.Iteratoris deprecated.♻️ Proposed tweak
-from typing import Any, Iterator +from collections.abc import Iterator +from typing import Any🤖 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/api/routes/test_improve_prompt_v2.py` at line 26, Update the import in test_improve_prompt_v2.py to import Iterator from collections.abc instead of typing, while retaining Any from typing.Source: Linters/SAST tools
509-518: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage: the v2 failure-path callback shape.
Every worker test exercises the success path. A test where drafting raises on a judge run would pin down which payload model the error callback emits — the gap flagged at
prompt_improvement.pylines 390-402. Want me to draft it?Also applies to: 589-627
🤖 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/api/routes/test_improve_prompt_v2.py` around lines 509 - 518, Add a TestV2Worker test covering a judge-run failure in execute_prompt_improvement: make the drafting step raise, execute the worker with a pending PROMPT_IMPROVEMENT job, and assert the failure callback uses the expected v2 payload model and error details. Follow the existing success-path test setup and assertions.backend/app/api/routes/evaluations/prompt_improvement_v2.py (1)
37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: chain the original exception.
raise HTTPException(...) from excpreserves the cause for tracebacks; the detail string already carries the message, so this is purely for debuggability.♻️ Proposed tweak
except ValueError as exc: - raise HTTPException(status_code=422, detail=f"invalid_callback_url: {exc}") + raise HTTPException( + status_code=422, detail=f"invalid_callback_url: {exc}" + ) from exc🤖 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/api/routes/evaluations/prompt_improvement_v2.py` around lines 37 - 40, Update the exception handling around validate_callback_url in the request flow to raise HTTPException from the caught ValueError using explicit exception chaining, while preserving the existing 422 status and detail message.backend/app/services/evaluations/prompt_improvement.py (3)
246-276: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: narrow the return annotation.
-> dict[str, Any]is more precise than baredict(same applies toexecute_prompt_improvement). Branch logic itself is correct and the v1 shape is preserved.As per coding guidelines: "provide narrow type hints for every function parameter and return value".
🤖 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/services/evaluations/prompt_improvement.py` around lines 246 - 276, Update the return annotations for the affected callback-builder function and execute_prompt_improvement to use dict[str, Any] instead of bare dict, importing Any if needed. Keep the existing branch behavior and response shapes unchanged.Source: Coding guidelines
390-402: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winv1/v2 callback shape is derived at runtime instead of being durable, so two paths can emit the wrong model.
is_judge_runis only known after a successful re-read/re-validation of the evaluation run; whenever that read is unavailable, the callback silently degrades to the v1PromptImprovementJobPublicfor a job the client submitted through the v2 endpoint. Persisting the flag (orrecommendation_type) on theJobrow at enqueue time fixes both sites and removes a query.
backend/app/services/evaluations/prompt_improvement.py#L390-L402: seedis_judge_runfrom the persisted job record before thetry, so timeout and pre-validation failures emit the v2 error payload.backend/app/services/evaluations/prompt_improvement.py#L357-L379: read the persisted flag instead ofbool(redelivery_run and redelivery_run.is_judge_run), so a deleted/moved run can't flip the redelivered callback back to the v1 shape.🤖 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/services/evaluations/prompt_improvement.py` around lines 390 - 402, Persist the v1/v2 payload decision on the Job at enqueue time and reuse it for callbacks. In backend/app/services/evaluations/prompt_improvement.py lines 390-402, initialize is_judge_run from the persisted job record before the try block, while retaining re-validation updates when available; in lines 357-379, derive the redelivery callback shape from that persisted flag instead of redelivery_run.is_judge_run, so deleted or moved runs cannot change the payload type.
543-552: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit:
"instructions"is a repeated literal.The same key is written at lines 413, 438, and 549. A module constant would keep the three sites in sync.
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/services/evaluations/prompt_improvement.py` around lines 543 - 552, Extract the repeated "instructions" key used in the prompt-improvement logic into a module-level constant, then replace the literals at the sites around lines 413, 438, and in _target_config_from_params with that constant. Keep the existing filtering and rendering behavior unchanged.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/api/docs/evaluation/improve_prompt_v2.md`:
- Around line 25-27: Update the improve-prompt error documentation near the
existing source_config_unavailable entry to also list the 422 variant returned
when the run has null config_id or config_version references, while preserving
the existing 409 deletion case.
In `@backend/app/services/evaluations/prompt_improvement.py`:
- Around line 704-735: Cap the evaluation traces embedded by the prompt-building
flow before constructing user_message_text, selecting only the N lowest-scoring
rows based on the prompt and ground-truth metrics while ignoring unscoreable
values. Serialize the capped collection instead of the full traces in the
`Evaluation traces` section, and preserve the existing task instructions and
scoring behavior.
---
Outside diff comments:
In `@backend/app/services/evaluations/prompt_improvement.py`:
- Around line 390-402: Initialize the payload-version state from durable
job/request data before the try block, rather than defaulting is_judge_run to
False. Update the early failure and timeout callback paths around the
prompt-improvement job so v2 requests consistently emit the v2 payload even when
validate_improve_prompt fails or run is not yet bound, while preserving the
existing assignment from the validated run when available.
- Around line 644-654: Update the exception handling around the LLM call to add
dedicated handlers before the catch-all `except Exception` in the
prompt-generation path. Re-raise both Celery `SoftTimeLimitExceeded` and the
relevant `Timeout` exception unchanged so the worker timeout branch receives
them; preserve the existing logging and `RuntimeError` conversion for other
unexpected exceptions.
---
Nitpick comments:
In `@backend/app/api/routes/evaluations/prompt_improvement_v2.py`:
- Around line 37-40: Update the exception handling around validate_callback_url
in the request flow to raise HTTPException from the caught ValueError using
explicit exception chaining, while preserving the existing 422 status and detail
message.
In `@backend/app/services/evaluations/prompt_improvement.py`:
- Around line 246-276: Update the return annotations for the affected
callback-builder function and execute_prompt_improvement to use dict[str, Any]
instead of bare dict, importing Any if needed. Keep the existing branch behavior
and response shapes unchanged.
- Around line 390-402: Persist the v1/v2 payload decision on the Job at enqueue
time and reuse it for callbacks. In
backend/app/services/evaluations/prompt_improvement.py lines 390-402, initialize
is_judge_run from the persisted job record before the try block, while retaining
re-validation updates when available; in lines 357-379, derive the redelivery
callback shape from that persisted flag instead of redelivery_run.is_judge_run,
so deleted or moved runs cannot change the payload type.
- Around line 543-552: Extract the repeated "instructions" key used in the
prompt-improvement logic into a module-level constant, then replace the literals
at the sites around lines 413, 438, and in _target_config_from_params with that
constant. Keep the existing filtering and rendering behavior unchanged.
In `@backend/app/tests/api/routes/test_improve_prompt_v2.py`:
- Line 26: Update the import in test_improve_prompt_v2.py to import Iterator
from collections.abc instead of typing, while retaining Any from typing.
- Around line 509-518: Add a TestV2Worker test covering a judge-run failure in
execute_prompt_improvement: make the drafting step raise, execute the worker
with a pending PROMPT_IMPROVEMENT job, and assert the failure callback uses the
expected v2 payload model and error details. Follow the existing success-path
test setup and assertions.
🪄 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: ced558cd-352c-4f1e-b980-5b315b938a1b
📒 Files selected for processing (7)
backend/app/api/docs/evaluation/improve_prompt_v2.mdbackend/app/api/main.pybackend/app/api/routes/evaluations/prompt_improvement_v2.pybackend/app/models/evaluation.pybackend/app/services/evaluations/prompt_improvement.pybackend/app/tests/api/routes/test_improve_prompt_v2.pydocs/wiki/modules/evaluations.md
Issue
Closes #1072
Summary
In this PR, adds a v2 prompt-iteration endpoint that generates prompt-improvement suggestions from the new three-metric judge results (Adherence to Ground Truth / Prompt / Knowledge Base — each a score + reasoning), instead of v1's cosine-similarity + correctness signal.
Built by reuse-and-branch on
EvaluationRun.is_judge_run: the existing job / Celery / config-version-minting machinery is shared, and only the recommendation-prompt builder and callback shape differ per run type. The v1 endpoint and its behavior are left unchanged.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.