feat(Prompt Iteration): Move processing to Celery#1023
Conversation
OpenAPI changes 🔴 2 breaking changesCaution Downstream consumers may need an update before merging. Breaking changes ·
|
| Method | Path | Change | |
|---|---|---|---|
| 🔴 | POST |
/api/v1/evaluations/{evaluation_id}/improve-prompt |
added required request body |
| 🔴 | POST |
/api/v1/evaluations/{evaluation_id}/improve-prompt |
removed the success response with the status 201 |
Full changelog · 3
| Method | Path | Change | |
|---|---|---|---|
| 🔴 | POST |
/api/v1/evaluations/{evaluation_id}/improve-prompt |
added required request body |
| 🔴 | POST |
/api/v1/evaluations/{evaluation_id}/improve-prompt |
removed the success response with the status 201 |
| 🟢 | POST |
/api/v1/evaluations/{evaluation_id}/improve-prompt |
added the success response with the status 202 |
main ↔ d114980a · generated by oasdiff
📝 WalkthroughWalkthroughPrompt improvement now validates completed evaluations, returns ChangesAsynchronous prompt improvement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant EvaluationAPI
participant PromptImprovementService
participant Celery
participant ObjectStorage
participant Claude
participant Database
participant CallbackURL
Client->>EvaluationAPI: POST improve-prompt with callback_url
EvaluationAPI->>PromptImprovementService: validate and start job
PromptImprovementService->>Celery: enqueue run_prompt_improvement
EvaluationAPI-->>Client: return 202 with job_id
Celery->>ObjectStorage: load score traces
Celery->>Claude: draft improved instructions
Celery->>Database: persist config version and job status
Celery->>CallbackURL: POST success or failure APIResponse
Possibly related issues
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/routes/evaluations/evaluation.py`:
- Around line 269-289: Update the job lookup and successful-result resolution
around JobCrud.get and get_evaluation_run_by_id to validate the job belongs to
the requested evaluation_id and has type PROMPT_IMPROVEMENT, persisting those
fields when the job is created. Resolve the config version from the persisted
config_version_id in job.meta instead of combining the caller evaluation_id with
a metadata version number, while preserving the existing not-found and
successful response behavior.
In `@backend/app/celery/tasks/job_execution.py`:
- Around line 249-253: Update run_prompt_improvement by adding explicit type
hints for self, **kwargs, and a dict return value, consistent with the task’s
existing conventions. Replace the literal priority=6 with a descriptive
fast-evaluation priority constant or the appropriate settings value, preserving
the task’s current queue and priority behavior.
In `@backend/app/celery/utils.py`:
- Around line 183-186: Update start_prompt_improvement to annotate kwargs with
an appropriate mapping value type such as Any, and replace the raw trace_id
default "N/A" with a named module-level constant such as DEFAULT_TRACE_ID.
Preserve the existing function behavior and use the constant in the signature.
In `@backend/app/services/evaluations/prompt_improvement.py`:
- Around line 211-218: In backend/app/services/evaluations/prompt_improvement.py
lines 211-218, update execute_prompt_improvement to replace **kwargs with an
explicitly typed task_id parameter and return the appropriate typed result
model. In backend/app/tests/api/routes/test_improve_prompt.py lines 101-107,
replace broad Any and bare-container annotations throughout the changed helpers
and test cases with concrete fixture, domain, and sentinel types; ensure every
affected parameter and return value has a narrowed type hint.
- Around line 187-199: Update the failure handling in the prompt-improvement job
enqueue paths, including the block around JobCrud.update, so job.error_message
never contains raw exc text. Keep exc and exc_info=True in logger.error for
diagnostics, but persist a stable sanitized client-facing error code or message
instead of interpolating exc; apply the same change to the other referenced
failure path.
- Around line 241-250: The execute_prompt_improvement flow currently checks
SUCCESS non-atomically, allowing concurrent deliveries or retries after a crash
to create duplicate versions. Replace the check/update sequence around
job_crud.get and job_crud.update with an atomic job claim or lease, and make
version creation plus the SUCCESS completion update transactional or keyed by
the job for idempotency; apply the same coordination to the later
create_or_raise flow referenced around lines 301-322.
In `@backend/app/tests/api/routes/test_improve_prompt.py`:
- Line 24: Update the typing import in test_improve_prompt.py to import Iterator
from collections.abc instead of typing, while keeping Any sourced from typing.
🪄 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: 0336024b-43c0-44b2-a8db-6376ff94356e
📒 Files selected for processing (13)
backend/app/alembic/versions/073_add_prompt_improvement_jobtype.pybackend/app/api/docs/evaluation/get_improve_prompt_status.mdbackend/app/api/docs/evaluation/improve_prompt.mdbackend/app/api/routes/evaluations/evaluation.pybackend/app/celery/tasks/job_execution.pybackend/app/celery/utils.pybackend/app/models/evaluation.pybackend/app/models/job.pybackend/app/services/evaluations/__init__.pybackend/app/services/evaluations/prompt_improvement.pybackend/app/tests/api/routes/test_improve_prompt.pydocs/wiki/domain-map.mddocs/wiki/modules/evaluations.md
| # Priority 6 (fast-eval tier): user-blocking interactive prompt iteration in the | ||
| # evaluation domain, above default batch work but below core LLM call/chain jobs. | ||
| @celery_app.task(bind=True, queue="default", priority=6) | ||
| @gevent_timeout(settings.CELERY_TASK_SOFT_TIME_LIMIT, "run_prompt_improvement") | ||
| def run_prompt_improvement(self, project_id: int, job_id: str, trace_id: str, **kwargs): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add missing type hints and extract magic values.
As per coding guidelines, every function parameter and return value must have a type hint, and magic values must be extracted to constants.
- The
run_prompt_improvementfunction is missing type hints forself,**kwargs, and its return value (-> dict). priority=6is a magic number. Since the comment identifies it as the "fast-eval tier", it should be extracted to a descriptive constant or loaded from settings.
💡 Proposed fix (example)
# Priority 6 (fast-eval tier): user-blocking interactive prompt iteration in the
# evaluation domain, above default batch work but below core LLM call/chain jobs.
-@celery_app.task(bind=True, queue="default", priority=6)
+FAST_EVAL_PRIORITY = 6
+
+@celery_app.task(bind=True, queue="default", priority=FAST_EVAL_PRIORITY)
`@gevent_timeout`(settings.CELERY_TASK_SOFT_TIME_LIMIT, "run_prompt_improvement")
-def run_prompt_improvement(self, project_id: int, job_id: str, trace_id: str, **kwargs):
+def run_prompt_improvement(self: Any, project_id: int, job_id: str, trace_id: str, **kwargs: Any) -> dict:
from app.services.evaluations.prompt_improvement import execute_prompt_improvement📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Priority 6 (fast-eval tier): user-blocking interactive prompt iteration in the | |
| # evaluation domain, above default batch work but below core LLM call/chain jobs. | |
| @celery_app.task(bind=True, queue="default", priority=6) | |
| @gevent_timeout(settings.CELERY_TASK_SOFT_TIME_LIMIT, "run_prompt_improvement") | |
| def run_prompt_improvement(self, project_id: int, job_id: str, trace_id: str, **kwargs): | |
| # Priority 6 (fast-eval tier): user-blocking interactive prompt iteration in the | |
| # evaluation domain, above default batch work but below core LLM call/chain jobs. | |
| FAST_EVAL_PRIORITY = 6 | |
| `@celery_app.task`(bind=True, queue="default", priority=FAST_EVAL_PRIORITY) | |
| `@gevent_timeout`(settings.CELERY_TASK_SOFT_TIME_LIMIT, "run_prompt_improvement") | |
| def run_prompt_improvement(self: Any, project_id: int, job_id: str, trace_id: str, **kwargs: Any) -> dict: |
🤖 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/celery/tasks/job_execution.py` around lines 249 - 253, Update
run_prompt_improvement by adding explicit type hints for self, **kwargs, and a
dict return value, consistent with the task’s existing conventions. Replace the
literal priority=6 with a descriptive fast-evaluation priority constant or the
appropriate settings value, preserving the task’s current queue and priority
behavior.
Source: Coding guidelines
| def start_prompt_improvement( | ||
| project_id: int, job_id: str, trace_id: str = "N/A", **kwargs | ||
| ) -> str: | ||
| from app.celery.tasks.job_execution import run_prompt_improvement |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add type hints for kwargs and avoid magic values.
As per coding guidelines, every function parameter must have a type hint, and magic values should be extracted into constants.
**kwargsis missing a type hint (e.g.,**kwargs: Any).- The default value
"N/A"is a magic string. Consider defining a constant (e.g.,DEFAULT_TRACE_ID = "N/A") instead of using a raw literal.
💡 Proposed fix (example)
+DEFAULT_TRACE_ID = "N/A"
+
def start_prompt_improvement(
- project_id: int, job_id: str, trace_id: str = "N/A", **kwargs
+ project_id: int, job_id: str, trace_id: str = DEFAULT_TRACE_ID, **kwargs: Any
) -> str:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def start_prompt_improvement( | |
| project_id: int, job_id: str, trace_id: str = "N/A", **kwargs | |
| ) -> str: | |
| from app.celery.tasks.job_execution import run_prompt_improvement | |
| DEFAULT_TRACE_ID = "N/A" | |
| def start_prompt_improvement( | |
| project_id: int, job_id: str, trace_id: str = DEFAULT_TRACE_ID, **kwargs: Any | |
| ) -> str: | |
| from app.celery.tasks.job_execution import run_prompt_improvement |
🤖 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/celery/utils.py` around lines 183 - 186, Update
start_prompt_improvement to annotate kwargs with an appropriate mapping value
type such as Any, and replace the raw trace_id default "N/A" with a named
module-level constant such as DEFAULT_TRACE_ID. Preserve the existing function
behavior and use the constant in the signature.
Source: Coding guidelines
| except Exception as exc: | ||
| logger.error( | ||
| f"[start_prompt_improvement_job] Failed to enqueue | job_id={job.id} " | ||
| f"evaluation_id={evaluation_id} | {exc}", | ||
| exc_info=True, | ||
| ) | ||
| JobCrud(session=session).update( | ||
| job.id, | ||
| JobUpdate( | ||
| status=JobStatus.FAILED, | ||
| error_message=f"Failed to queue prompt improvement: {exc}", | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not persist raw exception text in client-visible job errors.
Both paths store str(exc), while the polling endpoint returns job.error_message verbatim. Database, broker, storage, or SDK exceptions can therefore expose internal details. Log the original exception, but persist a stable sanitized error code/message.
Also applies to: 348-360
🤖 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 187 -
199, Update the failure handling in the prompt-improvement job enqueue paths,
including the block around JobCrud.update, so job.error_message never contains
raw exc text. Keep exc and exc_info=True in logger.error for diagnostics, but
persist a stable sanitized client-facing error code or message instead of
interpolating exc; apply the same change to the other referenced failure path.
| def execute_prompt_improvement( | ||
| *, | ||
| project_id: int, | ||
| job_id: str, | ||
| organization_id: int, | ||
| evaluation_id: int, | ||
| **kwargs, | ||
| ) -> dict: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Narrow the new async-flow type contracts.
backend/app/services/evaluations/prompt_improvement.py#L211-L218: replace**kwargswith an explicit typedtask_idand return a typed result model.backend/app/tests/api/routes/test_improve_prompt.py#L101-L107: replace new broadAny/bare-container annotations throughout the changed helpers and test cases with concrete fixture, domain, and sentinel types.
As per coding guidelines, every function parameter and return value must have narrowed type hints.
📍 Affects 2 files
backend/app/services/evaluations/prompt_improvement.py#L211-L218(this comment)backend/app/tests/api/routes/test_improve_prompt.py#L101-L107
🤖 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 211 -
218, In backend/app/services/evaluations/prompt_improvement.py lines 211-218,
update execute_prompt_improvement to replace **kwargs with an explicitly typed
task_id parameter and return the appropriate typed result model. In
backend/app/tests/api/routes/test_improve_prompt.py lines 101-107, replace broad
Any and bare-container annotations throughout the changed helpers and test cases
with concrete fixture, domain, and sentinel types; ensure every affected
parameter and return value has a narrowed type hint.
Source: Coding guidelines
| existing = job_crud.get(job_id=job_uuid, project_id=project_id) | ||
| if existing and existing.status == JobStatus.SUCCESS: | ||
| logger.info( | ||
| f"[execute_prompt_improvement] Redelivery of completed job, skipping | " | ||
| f"job_id={job_id}" | ||
| ) | ||
| return {"success": True, **(existing.meta or {})} | ||
|
|
||
| job_crud.update( | ||
| job_uuid, JobUpdate(status=JobStatus.PROCESSING, task_id=task_id) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make version creation and job completion idempotent atomically.
The SUCCESS check only protects sequential redeliveries. Concurrent deliveries can both pass it, and a crash after create_or_raise() but before the SUCCESS update causes a retry to mint another version. Use an atomic job claim/lease plus a transaction or job-based idempotency key for version creation.
Also applies to: 301-322
🤖 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 241 -
250, The execute_prompt_improvement flow currently checks SUCCESS
non-atomically, allowing concurrent deliveries or retries after a crash to
create duplicate versions. Replace the check/update sequence around job_crud.get
and job_crud.update with an atomic job claim or lease, and make version creation
plus the SUCCESS completion update transactional or keyed by the job for
idempotency; apply the same coordination to the later create_or_raise flow
referenced around lines 301-322.
| import contextlib | ||
| import json | ||
| from contextlib import ExitStack | ||
| from typing import Any, Iterator |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ruff check backend/app/tests/api/routes/test_improve_prompt.py --select UP035Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 794
Import Iterator from collections.abc.
Proposed fix
+from collections.abc import Iterator
-from typing import Any, Iterator
+from typing import Any📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import Any, Iterator | |
| from collections.abc import Iterator | |
| from typing import Any |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 24-24: Import from collections.abc instead: Iterator
Import from collections.abc
(UP035)
🤖 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.py` at line 24, Update the
typing import in test_improve_prompt.py to import Iterator from collections.abc
instead of typing, while keeping Any sourced from typing.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/tests/api/routes/test_improve_prompt.py (1)
144-145: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExercise the signed-callback path.
_worker_envalways returnsNonefor the webhook secret, so these assertions cannot detect regressions that make callbacks unsigned. Use a non-empty test secret and verify thatsend_callbackreceives the signing input or resulting signature metadata.Also applies to: 636-659
🤖 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.py` around lines 144 - 145, Update the test setup around _SERVICE.get_webhook_secret to return a non-empty secret, then assert that send_callback receives the expected signing input or signature metadata. Apply the same signed-callback assertions to the related coverage in the additional range, while preserving the existing callback behavior checks.
🤖 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.md`:
- Line 17: Update the callback contract in
backend/app/api/docs/evaluation/improve_prompt.md:17-17 and
docs/wiki/modules/evaluations.md:31-31 to describe at-least-once delivery rather
than a single best-effort attempt, explicitly noting that Celery redelivery may
produce duplicate callbacks and clients must deduplicate by job_id. Preserve the
existing statement that config_version remains persisted and recoverable
regardless of callback outcome.
---
Nitpick comments:
In `@backend/app/tests/api/routes/test_improve_prompt.py`:
- Around line 144-145: Update the test setup around _SERVICE.get_webhook_secret
to return a non-empty secret, then assert that send_callback receives the
expected signing input or signature metadata. Apply the same signed-callback
assertions to the related coverage in the additional range, while preserving the
existing callback behavior checks.
🪄 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: cdd50c8f-f07d-4ebc-82ed-0bf26360ae6f
📒 Files selected for processing (7)
backend/app/alembic/versions/074_add_prompt_improvement_jobtype.pybackend/app/api/docs/evaluation/improve_prompt.mdbackend/app/api/routes/evaluations/evaluation.pybackend/app/models/evaluation.pybackend/app/services/evaluations/prompt_improvement.pybackend/app/tests/api/routes/test_improve_prompt.pydocs/wiki/modules/evaluations.md
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/models/evaluation.py
- backend/app/services/evaluations/prompt_improvement.py
|
|
||
| **Callback delivery:** | ||
|
|
||
| Once the worker finishes (success or failure), it POSTs a single JSON body to `callback_url`. Delivery is **best-effort, single-attempt** — there is no retry. The `config_version` is persisted regardless of callback outcome and remains recoverable from the configuration's version list, so a failed callback loses the notification, not the work. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align both documents with the worker’s at-least-once callback behavior.
Celery redelivery can cause the same successful job callback to be sent again, so clients must tolerate duplicates and deduplicate by job_id.
backend/app/api/docs/evaluation/improve_prompt.md#L17-L17: replace “single-attempt”/“no retry” with explicit at-least-once semantics and duplicate-callback guidance.docs/wiki/modules/evaluations.md#L31-L31: replace “single best-effort callback” with the same at-least-once contract.
📍 Affects 2 files
backend/app/api/docs/evaluation/improve_prompt.md#L17-L17(this comment)docs/wiki/modules/evaluations.md#L31-L31
🤖 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/docs/evaluation/improve_prompt.md` at line 17, Update the
callback contract in backend/app/api/docs/evaluation/improve_prompt.md:17-17 and
docs/wiki/modules/evaluations.md:31-31 to describe at-least-once delivery rather
than a single best-effort attempt, explicitly noting that Celery redelivery may
produce duplicate callbacks and clients must deduplicate by job_id. Preserve the
existing statement that config_version remains persisted and recoverable
regardless of callback outcome.
|
🎉 This PR is included in version 1.4.0-main.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Issue
Closes #1018
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.