fix(config): accept the documented 0 sentinel for per-attempt timeout override by Lesereingrape · Pull Request #3377 · confident-ai/deepeval

Lesereingrape · 2026-09-27T13:42:48Z

Description

DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE is declared as Optional[confloat(gt=0)], but the comment directly above it documents 0 as the "derive it for me" value:

If this is None or 0 the DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS is computed from either: ...

So the documented sentinel is rejected before a Settings object is ever built, and get_settings() raises an uncaught ValidationError:

pydantic_core._pydantic_core.ValidationError: 1 validation error for Settings
DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE
  Input should be greater than 0 [type=greater_than, input_value='0', input_type=str]

Anything that loads settings then fails — deepeval --help, deepeval.initialize(), and any evaluate() run. This is easy to hit from CI or a compose file that declares the variable with a neutral default of 0.

Two things indicate 0 is meant to be accepted:

both consumers already branch on it (DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS checks float(over) > 0, _calc_auto_outer_timeout checks timeout_seconds <= 0), so today those 0 paths are unreachable through the override;
the sibling outer override, documented with the same "If None or 0" wording, is declared confloat(ge=0).

This only widens the constraint to let the documented sentinel through; negative timeouts are still rejected.

How Has This Been Tested?

tests/test_core/test_config/test_settings.py: added test_per_attempt_timeout_override_zero_auto_derives, which fails on main with the ValidationError above, and now passes with =0 producing exactly the same derived budget as leaving the variable unset; added test_per_attempt_timeout_override_negative_still_rejected as a guard for the loosened constraint.
pytest tests/test_core/test_config/test_settings.py → 54 passed, 1 failed. The failure is test_env_dir_path_expanduser, which fails identically on a clean checkout of main in this Windows environment (52 passed, 1 failed before the change).
pytest tests/test_core/test_retry_policy.py → 37 passed, 1 skipped.
Manual, on the installed CLI: DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE=0 python -m deepeval.cli.main --help now prints usage instead of crashing; =5 still takes precedence (per-attempt 5.0s, outer budget 13.0s); =-1 still raises.
black --check and ruff check clean on both changed files.

… override DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE is declared confloat(gt=0), so setting it to 0 - which the block comment above it documents as the auto-derive sentinel - raises an uncaught ValidationError from get_settings() and takes down the CLI, deepeval.initialize() and any evaluate() call. Both consumers of the value already branch on <= 0, and the sibling outer override accepts ge=0.

vercel · 2026-09-27T13:42:54Z

@Lesereingrape is attempting to deploy a commit to the Confident AI Team on Vercel.

A member of the Team first needs to authorize it.

chrikrah

@Lesereingrape I would merge this. It brings the per-attempt override in line with DEEPEVAL_PER_TASK_TIMEOUT_SECONDS_OVERRIDE and DEEPEVAL_TASK_GATHER_BUFFER_SECONDS_OVERRIDE, which already take confloat(ge=0), and with the comment at deepeval/config/settings.py:1019. Reverting settings.py with your tests kept fails test_per_attempt_timeout_override_zero_auto_derives.

non-blocking: the timeout error at deepeval/models/retry_policy.py:509 tells users "None disables", and after this change neither None nor 0 does. DEEPEVAL_DISABLE_TIMEOUTS is the switch that turns the inner timeout off, per the first three runs below.

Evidence at head 3ea637d, base 385117c, Python 3.12, editable install:

$ P='from deepeval.models.retry_policy import resolve_effective_attempt_timeout as r; print(r())'
$ python -c "$P"
88.5
$ DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE=0 python -c "$P"
88.5
$ DEEPEVAL_DISABLE_TIMEOUTS=1 python -c "$P"
0
$ python -m pytest tests/test_core/test_config/test_settings.py -q
55 passed in 2.65s
$ git checkout 385117c -- deepeval/config/settings.py
$ python -m pytest tests/test_core/test_config/test_settings.py -q
FAILED tests/test_core/test_config/test_settings.py::test_per_attempt_timeout_override_zero_auto_derives
1 failed, 54 passed in 2.80s

@penguine-ip, should the message at :509 name DEEPEVAL_DISABLE_TIMEOUTS in this pull request, or separately?

The per-attempt timeout hint told users "(None disables)", but neither an unset nor a 0 DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE disables the inner timeout -- both derive it from the per-task budget. DEEPEVAL_DISABLE_TIMEOUTS is the opt-out, so the message now points there, with a regression test.

Lesereingrape · 2026-10-02T05:02:15Z

@chrikrah Thanks — I reproduced your matrix on this branch (Python 3.12, editable install): resolve_effective_attempt_timeout() returns 88.5 with nothing set, 88.5 with DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE=0, and 0 with DEEPEVAL_DISABLE_TIMEOUTS=1. So the hint at retry_policy.py:509 was wrong on both counts.

I folded the wording fix in rather than splitting it (ce8b918):

call timed out after 12.5s (per attempt). Raise DEEPEVAL_PER_ATTEMPT_TIMEOUT_SECONDS_OVERRIDE (unset or 0 derives it from the per-task budget), set DEEPEVAL_DISABLE_TIMEOUTS=1 to opt out of DeepEval timeouts, or reduce work per attempt.

and added test_timeout_error_names_the_switch_that_disables_timeouts to tests/test_core/test_retry_policy.py — it fails against the previous head and passes here.

Local results: pytest tests/test_core/test_retry_policy.py tests/test_core/test_config/test_settings.py → 92 passed, 1 skipped, 1 failed; black --check and ruff check clean on both files. The one failure is pre-existing and unrelated: test_env_dir_path_expanduser fails on this branch both before and after the commit on Windows, because ~ expands through USERPROFILE rather than the monkeypatched HOME.

@penguine-ip Say the word if you would rather keep this PR to the validator alone — the message change and its test are one commit, so moving them out is a rebase away.

Lesereingrape · 2026-10-02T05:31:05Z

Two test checks are red on this PR (Py Core Tests, Py Metrics Tests); both reproduce identically on main and on unrelated open PRs, so they are not caused by this change:

tests/test_core/test_sqlite_store.py::TestTestRunManagerIntegration::test_storage_failure_is_a_warning_not_an_error — asserts the bare substring 'disk I/O error', but the captured warning is line-wrapped by the runner's terminal width (...deepeval.db: disk\nI/O error\n), so the substring never matches.
tests/test_metrics/test_eval_mode_metrics.py::test_hybrid_jev_takes_the_decisions[sync-MultiTurnMCPUse] — AttributeError: 'CallToolResult' object has no attribute 'structuredContent', an MCP SDK attribute-shape mismatch independent of retry_policy.

Same two test IDs and the same two messages on the main push of 2026-10-01: run 36849620916 (Py Core Tests) and run 36849620912 (Py Metrics Tests). The other jobs on this head are green (Py Confident Tests, Py Integration, Py Lint, PR title check, and the integration matrices); the Vercel check reports "Authorization required to deploy", which looks like a preview-deployment permission on the repo side rather than anything this diff can affect.

chrikrah

@Lesereingrape I would merge ce8b918 with the message change kept in. Each production file fails only its own new test when reverted, so splitting it out later stays cheap.

# ce8b918 export, base 385117c, Python 3.12.3, pytest 9.1.1, DEEPEVAL_TELEMETRY_OPT_OUT=1
$ python -m pytest tests/test_core/test_config/test_settings.py tests/test_core/test_retry_policy.py -q
93 passed, 1 skipped in 12.51s    # ce8b918
90 passed, 1 skipped in 13.61s    # 385117c
# retry_policy.py from 385117c, tests kept
FAILED tests/test_core/test_retry_policy.py::test_timeout_error_names_the_switch_that_disables_timeouts
# settings.py from 385117c, tests kept
FAILED tests/test_core/test_config/test_settings.py::test_per_attempt_timeout_override_zero_auto_derives

@penguine-ip, ce8b918 puts the :509 fix you were asked about into this pull request. @kritinv, would you take a look if he is busy?

chrikrah approved these changes Oct 1, 2026

View reviewed changes

chrikrah approved these changes Oct 5, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(config): accept the documented 0 sentinel for per-attempt timeout override - #3377

fix(config): accept the documented 0 sentinel for per-attempt timeout override#3377
Lesereingrape wants to merge 2 commits into
confident-ai:mainfrom
Lesereingrape:fix/per-attempt-timeout-zero-sentinel

Lesereingrape commented Sep 27, 2026

Uh oh!

vercel Bot commented Sep 27, 2026

Uh oh!

chrikrah left a comment

Uh oh!

Lesereingrape commented Oct 2, 2026

Uh oh!

Lesereingrape commented Oct 2, 2026

Uh oh!

chrikrah left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Lesereingrape commented Sep 27, 2026

Description

How Has This Been Tested?

Uh oh!

vercel Bot commented Sep 27, 2026

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

Lesereingrape commented Oct 2, 2026

Uh oh!

Lesereingrape commented Oct 2, 2026

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants