feat(evaluation): Improve iteration loop efficiency - #1101
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
changed the title
…eration-loop # Conflicts: # backend/app/services/evaluations/fast.py # docs/wiki/modules/evaluations.md
OpenAPI changes 🟢 1 non-breaking changeTip Safe to merge from an API-contract perspective. Full changelog ·
|
| Method | Path | Change | |
|---|---|---|---|
| 🟢 | POST |
/api/v2/evaluations/iterations |
endpoint added |
main ↔ 3bce6f80 · generated by oasdiff
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| {"finalize_node": "finalize_node", "start_improve_node": "start_improve_node"}, | ||
| ) | ||
| graph.add_edge("start_improve_node", "wait_improve_node") | ||
| graph.add_edge("wait_improve_node", "start_eval_node") |
There was a problem hiding this comment.
wait_improve_node currently has an unconditional edge, so round_failed never reaches finalize_node and the webhook doesn't fire. Reuse route_after_eval here, mapping the continue branch to start_eval_node, so failures finalize correctly and both wait nodes follow the same pattern.
graph.add_conditional_edges(
"wait_improve_node",
route_after_eval,
{"finalize_node": "finalize_node", "start_improve_node": "start_eval_node"},
)
| ) | ||
|
|
||
|
|
||
| def _mark_iteration_run_failed( |
There was a problem hiding this comment.
here mark the row FAILED but never hit the callback_url, caller just waits forever. row already has the url, can we POST a failure report from here too?
| so a caller passing an oversized value degrades to the safety cap instead of | ||
| failing outright. | ||
| """ | ||
| resolved_max_rounds = min( |
There was a problem hiding this comment.
Silently capping max_rounds at 25 is surprising when the caller asks for 100. Since each round costs money, we should reject invalid values instead. Add le=EVAL_ITERATION_MAX_ROUNDS_HARD_CAP to EvaluationIterationCreateRequest so FastAPI returns a 422 and the limit is reflected in OpenAPI.
just nitpick
| ["project_id"], | ||
| unique=False, | ||
| ) | ||
|
|
There was a problem hiding this comment.
create index on status too, since cron is going to check status on every tick, so having index might improve the query performance.
| checkpointer = get_evaluation_iteration_checkpointer() | ||
| graph = build_evaluation_iteration_graph(checkpointer) | ||
| thread_config = {"configurable": {"thread_id": str(iteration_run_id)}} | ||
|
|
There was a problem hiding this comment.
If thread_config is None, skip the eval iteration since LangGraph runs it with an empty state.
2 participants
Issue
Closes #PLEASE_TYPE_ISSUE_NUMBER
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.Notes
Please add here if any other information is required for the reviewer.
Original PR description
Issue
Closes #PLEASE_TYPE_ISSUE_NUMBER
Summary
Explain the motivation for making this change. What existing problem does the pull request solve?
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.Notes
Please add here if any other information is required for the reviewer.