feat(evaluation) 10/15: the EvalRunner Ray actor and its dispatcher - #821
feat(evaluation) 10/15: the EvalRunner Ray actor and its dispatcher#821Ahmath-Gadji wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 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 |
e0a85b2 to
f15c899
Compare
62e72a1 to
8319b05
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 56 minutes. |
f15c899 to
5400f80
Compare
8319b05 to
d32c76e
Compare
5400f80 to
ea0f369
Compare
d32c76e to
a475b79
Compare
ea0f369 to
43925aa
Compare
a475b79 to
6b45d33
Compare
`EvalRunner` executes one run: upload and time each corpus file, shell out to promptfoo twice, fold the outputs into metrics, persist, drop the throwaway partition. `RayEvaluationRunner` binds it to the `EvaluationRunner` port and keeps every Ray concern — actor lookup, `.remote()`, timeouts, cancellation — on the worker side of the boundary. The runner drives OpenRAG through its own HTTP API rather than in-process calls, for two reasons: it is the path a real user's documents take, so the indexing timings mean something, and it is the same surface promptfoo talks to, so an eval can never pass against a code path the API does not expose. Notes: - The actor handle is resolved on first use, not in `__init__`. `EvalRunner` is detached, so merely building the adapter must not be what spawns it — listing datasets should not start a worker process. - `run()` never raises. The caller dispatched it fire-and-forget and has nobody to catch for, so every outcome is written to the run row. - One bad corpus file is recorded as a failed sample rather than voiding the run; every file failing is an error worth surfacing. - promptfoo exits non-zero when assertions fail, which is a result, not an error — the presence of the output file is what decides. Both streams are captured, since promptfoo reports config errors on stdout. - Each run gets its own `PROMPTFOO_CONFIG_DIR`. promptfoo keeps a SQLite history under it, defaulting to `$HOME/.promptfoo`, which is not guaranteed writable in a container. - `max_concurrency=4` so `cancel()` and `is_busy()` still land while `run()` holds a slot.
43925aa to
cc3263c
Compare
6b45d33 to
6497847
Compare
Part 10 of 15 of the split of #811. Targets
eval/09-run-lifecycle(#820). This is where a run actually happens.What
Plus
RayEvaluationRunner, the adapter binding the actor to the part-8 port, and the container wiring that hands it toEvaluationService.Notable
__init__.EvalRunneris a detached actor, so merely building the adapter must not be what spawns it — listing datasets should not start a worker process. The container therefore buildsEvaluationServiceeagerly and the worker lazily.run()never raises. The caller dispatched it fire-and-forget and has nobody to catch for, so cancellation and failure are both written to the run row. Thefinallydrops the partition whatever happened.files_failed. Every file failing is an error worth surfacing, since the retrieval phase would otherwise grade an empty index.PROMPTFOO_CONFIG_DIR. promptfoo keeps a SQLite eval history under it, defaulting to$HOME/.promptfoo, which is not guaranteed writable in a container; a per-run temp dir is also never contended. WAL mode is disabled because some filesystems don't support it.max_concurrency=4socancel()andis_busy()still land whilerun()holds a slot — otherwise cancellation would deadlock behind the run it is trying to cancel.Testing
ruff, format check, the layer-import guard, and the full unit suite (2270 passed) including the DI wiring test, which now coversevaluation_service. The actor itself is exercised end to end on a real deployment rather than in unit tests — it is a thin shell over HTTP and a subprocess, both mocked out of meaning. The pure logic it calls is tested in parts 3–5.The one failure,
test_content_deduplication_can_be_disabled_by_env, reproduces ondevelop.