fix(evals): reject non-positive max_concurrency

[VectorPeak]

Make Dataset.evaluate() fail fast for non-positive max_concurrency values instead of hanging on an impossible semaphore acquisition.

No issue; small focused bug fix.

Checklist

• Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
• No breaking changes in accordance with the version policy.
• PR title is fit for the release changelog.

What Problem This Solves

Dataset.evaluate() exposes max_concurrency as a user-facing limit for how many case evaluations may run at the same time. The valid meanings are:

• None: no explicit limit; run all cases concurrently as before.
• a positive integer: allow at most that many concurrent evaluations.

0 is neither of those meanings. It does not mean unbounded concurrency, because that is already represented by None; and it does not mean “run zero evaluations”, because Dataset.evaluate() still builds the case tasks and tries to execute them.

Today, max_concurrency=0 is passed directly into anyio.Semaphore(0). Once evaluation starts, every case task waits for a permit, but no permit can ever become available. The result is not a clear configuration error; the evaluation simply makes no progress until something external times out.

A minimal local reproduction before this change timed out:

TimeoutError

That failure mode is especially confusing because the apparent symptom is a hung evaluation, slow task, stuck evaluator, retry issue, or event-loop problem. The actual root cause is much simpler: a non-positive concurrency limit cannot schedule any work. Failing fast at the Dataset.evaluate() boundary gives the caller an immediate and actionable error before any tasks, evaluators, lifecycle hooks, or progress reporting are started.

This PR only rejects explicit non-positive integer limits. It does not change the existing unbounded max_concurrency=None behavior, positive concurrency limits such as 1, task execution, evaluator execution, retries, lifecycle handling, or result aggregation.

Change

Add an early validation guard in Dataset.evaluate():

max_concurrency must be >= 1

The existing None behavior remains unchanged and still means all cases can run concurrently. Positive integer limits, including 1, continue to work as before.

Evidence

Before this change, this call hung until wrapped by an external timeout:

await dataset.evaluate(task, max_concurrency=0, progress=False)

After this change, invalid values fail fast:

0: ValueError: max_concurrency must be >= 1, got 0
-1: ValueError: max_concurrency must be >= 1, got -1

Regression coverage was added for both 0 and -1.

Possible call chain / impact

User calls Dataset.evaluate(..., max_concurrency=0)
  -> Dataset.evaluate()
  -> anyio.Semaphore(0)
  -> per-case task waits for a permit
  -> evaluation never progresses

This PR only validates explicit non-positive integer limits. It does not change task execution, evaluator execution, retry behavior, lifecycle hooks, repeat, progress rendering, or the max_concurrency=None unbounded path.

Validation

• uv run pytest tests/evals/test_dataset.py::test_evaluate_with_concurrency tests/evals/test_dataset.py::test_evaluate_with_invalid_max_concurrency -p no:cacheprovider - passed, 3 tests
• uv run ruff check pydantic_evals/pydantic_evals/dataset.py tests/evals/test_dataset.py - passed
• git diff --check - passed
• Direct behavior proof for 0 and -1 - passed

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds validation to Dataset.evaluate so that a ValueError is raised when max_concurrency is provided and is less than 1, preventing invalid concurrency setup. A corresponding parameterized async test was added to verify this behavior for values 0 and -1, checking that the raised error message includes the invalid value.

Changes

File	Change Summary
pydantic_evals/pydantic_evals/dataset.py	Added validation in Dataset.evaluate to raise ValueError for max_concurrency < 1
tests/evals/test_dataset.py	Added parameterized test test_evaluate_with_invalid_max_concurrency covering invalid max_concurrency values

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Dataset as Dataset.evaluate
  Caller->>Dataset: evaluate(max_concurrency)
  alt max_concurrency < 1
    Dataset-->>Caller: raise ValueError
  else valid
    Dataset->>Dataset: proceed with concurrency setup
  end

Loading

Related Issues: None referenced.

Related PRs: None referenced.

Suggested labels: pydantic_evals

Suggested reviewers: None identified.

---

A rabbit hops through concurrency's gate,
Checks the number before it's too late,
Zero or negative? Not on my watch!
A ValueError raised, no need to botch,
Tests confirm it, hop hop, all's straight. 🐇

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: rejecting non-positive max_concurrency values in evals.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description is thorough and matches the template well, with checklist, problem, change, evidence, and validation covered.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/evals/test_dataset.py (1)

508-517: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing VCR-skip rationale.

This test pins a pre-request guard (validation short-circuits before the task ever runs), which is exactly the case the guideline calls out for requiring an explanation of why it's not a VCR test.

📝 Suggested docstring

`@pytest.mark.parametrize`('max_concurrency', [0, -1])
async def test_evaluate_with_invalid_max_concurrency(
    example_dataset: Dataset[TaskInput, TaskOutput, TaskMetadata], max_concurrency: int
):
+    """Not a VCR test: validation raises before the task/API call is ever made."""
    async def mock_task(inputs: TaskInput) -> TaskOutput:  # pragma: no cover
        return TaskOutput(answer=inputs.query)

As per coding guidelines, "Each unit test should explain why it is not (or cannot be) a VCR test, especially when it pins internal behavior, pre-request guards, or request payload shape that cassettes would not reliably catch."

🤖 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 `@tests/evals/test_dataset.py` around lines 508 - 517, Add a short docstring to
test_evaluate_with_invalid_max_concurrency explaining why this is not a VCR
test: it validates the pre-request guard in Dataset.evaluate, so the task never
runs and no HTTP interaction is recorded. Mention the key symbol names evaluate
and max_concurrency so future readers understand this test intentionally pins
internal validation behavior rather than cassette-backed I/O.

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.

Nitpick comments:
In `@tests/evals/test_dataset.py`:
- Around line 508-517: Add a short docstring to
test_evaluate_with_invalid_max_concurrency explaining why this is not a VCR
test: it validates the pre-request guard in Dataset.evaluate, so the task never
runs and no HTTP interaction is recorded. Mention the key symbol names evaluate
and max_concurrency so future readers understand this test intentionally pins
internal validation behavior rather than cassette-backed I/O.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f30ca1c-d2c6-43b7-9204-518c9ec06199

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa35af and 1d9f41c.

📒 Files selected for processing (2)

• pydantic_evals/pydantic_evals/dataset.py
• tests/evals/test_dataset.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• pydantic/logfire (auto-detected)
• pydantic/pydantic (auto-detected)