Skip to content

feat(evaluation) 8/15: define the EvaluationRunner port - #819

Open
Ahmath-Gadji wants to merge 1 commit into
eval/07-dataset-servicefrom
eval/08-runner-port
Open

feat(evaluation) 8/15: define the EvaluationRunner port#819
Ahmath-Gadji wants to merge 1 commit into
eval/07-dataset-servicefrom
eval/08-runner-port

Conversation

@Ahmath-Gadji

@Ahmath-Gadji Ahmath-Gadji commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Part 8 of 15 of the split of #811. Targets eval/07-dataset-service (#818). One file, no behaviour — but it is the boundary the next three parts are shaped by, so it is worth reading on its own.

What

core/evaluation/runner.py: the contract between the evaluation orchestrator and the worker layer — is_busy, dispatch, cancel.

Notable

  • Why a port at all. EvaluationService (parts 7 and 9) owns the orchestration around a run: provisioning the throwaway partition and the service-user token, enforcing one-run-at-a-time, recording the queued row. The run itself executes inside a Ray actor (part 10). Declaring the three operations the orchestrator needs on a dedicated port keeps it Ray-free, per the Phase 9 rule that all Ray code lives under services/workers/ (docs/refactoring/REFACTORING_STRATEGY_v1.md). Same port/adapter shape as IndexingDispatcher.
  • No Ray types cross it — only strings, mappings and bools. That is what lets part 9's tests bind an in-memory fake and run with no actor, no worker process and no Ray import.
  • Two docstring contracts are load-bearing, not decoration:
    • is_busy doubles as the liveness probe, so an implementation must raise rather than hang when the worker is unreachable. Part 9 depends on this to fail a start fast instead of stranding a run in QUEUED.
    • dispatch is fire-and-forget — the worker owns the run once accepted and writes its own terminal status. Consequently cancel returning False means no worker owns this run, which is the orchestrator's signal to reap the row itself rather than wait for a status that will never arrive.

Testing

ruff and the layer-import guard pass. Behaviour is exercised in part 9 (against a fake) and part 10 (the Ray adapter).

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@EnjoyBacon7, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4611e220-1ee5-4db7-b223-a8127d38a427

📥 Commits

Reviewing files that changed from the base of the PR and between 54e7a03 and 5acd338.

📒 Files selected for processing (1)
  • openrag/core/evaluation/runner.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch eval/08-runner-port

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Ahmath-Gadji

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
✅ Action performed

Full 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 57 minutes.

@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/07-dataset-service branch from 5ce2f07 to 4759352 Compare July 27, 2026 14:32
@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/08-runner-port branch from 03026f3 to 224a71c Compare July 27, 2026 14:32
@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/07-dataset-service branch from 4759352 to 64dc0d3 Compare July 27, 2026 15:14
@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/08-runner-port branch from 224a71c to b0f87a9 Compare July 27, 2026 15:14
@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/07-dataset-service branch from 64dc0d3 to c09367d Compare July 27, 2026 15:31
@Ahmath-Gadji
Ahmath-Gadji force-pushed the eval/08-runner-port branch from b0f87a9 to 58ad1d0 Compare July 27, 2026 15:31
The contract between the evaluation orchestrator and the worker layer:
`is_busy`, `dispatch`, `cancel`.

`EvaluationService` owns the orchestration around a run — provisioning the
throwaway partition and the service-user token, enforcing one run at a
time, recording the queued row. The run itself executes inside a Ray actor.
Declaring the three operations the orchestrator needs on a dedicated port
keeps it Ray-free, per the Phase 9 rule that all Ray code lives under
`services/workers/`. The adapter binding this to the actor follows; tests
bind it to an in-memory fake.

No Ray types cross the boundary — only strings, mappings and bools.

Two details the docstrings pin down, because both are load-bearing for the
implementations:

- `is_busy` doubles as the orchestrator's liveness probe, so it must raise
  rather than hang when the worker cannot be reached.
- `dispatch` is fire-and-forget: the worker owns the run once accepted and
  records its own terminal status. `cancel` returning False means no worker
  owns that run, which is the orchestrator's signal to reap the row itself.
@EnjoyBacon7
EnjoyBacon7 force-pushed the eval/07-dataset-service branch from c09367d to 54e7a03 Compare July 28, 2026 10:25
@EnjoyBacon7
EnjoyBacon7 force-pushed the eval/08-runner-port branch from 58ad1d0 to 5acd338 Compare July 28, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants