[Feature] Pydantic validation for scheduler.py and structured_outputs.py

[vrdn-23]

Purpose

Part of #26366

Test Plan

Test Result

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results
• (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
• (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

[gemini-code-assist]

Code Review

This pull request refactors SchedulerConfig and StructuredOutputsConfig to use Pydantic's validation features more effectively. The changes are well-structured, replacing dataclasses features with Pydantic's Field and model_validator. This consolidation simplifies the validation logic and improves code clarity.

I've found one potential issue in scheduler.py where a None value could be assigned to a boolean field, leading to a runtime error. I've left a specific comment with a suggested fix. Otherwise, the changes look good.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

[chatgpt-codex-connector]

Static default for max_num_seqs disables usage-context tuning

max_num_seqs now has a hard default of 128 instead of None. EngineArgs.apply_usage_context sets different defaults depending on hardware only when self.max_num_seqs is None (see vllm/engine/arg_utils.py around lines 1829–1836). With the new default the field is never None, so the per-device logic never executes and every engine will default to 128 sequences regardless of GPU capability. This is a performance regression and contradicts the docstring that says “This config has no static default.”

Useful? React with 👍 / 👎.

[vrdn-23]

Okay this seems valid but maybe someone can weigh in is to how that code snippet is being invoked. Because in the original code it seems to be checking for None and immediately setting the value to 128

if self.max_num_seqs is None:
            self.max_num_seqs = 128

[hmellor]

The bot is right, the 128 limit was likely only ever seen in CI.

For now set it to Field(default=None, validate_default=True) (so we can remove the skip validation and type ignore) and leave

        if self.max_num_seqs is None:
            self.max_num_seqs = 128

in a before field validator

Then, in a follow up PR, we could look at moving the context aware default setting into the config classes instead of it living somewhere else.

[vrdn-23]

Sorry I'm late to this. I got caught up with some work today.

Shouldn't validate_default in Field(default=None, validate_default=True) be False because it will try to validate the default value against int and fail?

[vrdn-23]

I can try to triage the rest of the failing tests if you'd like to me keep working on this @hmellor

[hmellor]

I used validate_default=True so that the field validators would run when the user didn't pass those fields. Without this, the field will stay None and cause issues downstream.

Did moving the default overrides to the model validator fix any of the tests?

[vrdn-23]

I used validate_default=True so that the field validators would run when the user didn't pass those fields. Without this, the field will stay None and cause issues downstream.

I think I may be understanding this a bit wrong. validate_default is used to check whether the default value provided to the Field is correct with respect to the type annotation. So by setting the value to None and the type to int, Pydantic will always throw an error.

I moved it to the after model validator so that it replicates the same behavior we had previously. But now the issue is, the validation for checks such as ge and gt happen before the after model validator, and throw an error. I will move the validators to this back to before field validators and see if that fixes it, instead of allowing Optional to the types

[hmellor]

So by setting the value to None and the type to int, Pydantic will always throw an error.

Yes, however, by using a field_validator in before mode, I remove None before pydantic checks anything else and there is no error.

[hmellor]

Can we revert to 5984a27 to see what the actual errors were?

[hmellor]

The bot is right, the 128 limit was likely only ever seen in CI.

For now set it to Field(default=None, validate_default=True) (so we can remove the skip validation and type ignore) and leave

        if self.max_num_seqs is None:
            self.max_num_seqs = 128

in a before field validator

Then, in a follow up PR, we could look at moving the context aware default setting into the config classes instead of it living somewhere else.

[vrdn-23]

I'm back on this now. Will work to get this up and merged by this week
cc @hmellor

[vrdn-23]

@hmellor The tests that seemed to be failing are passing now! The existing failures seem to be unrelated. Let me know if this is good to merge. Also apologies for this taking so long (things were a bit hectic at my day job).

[DarkLight1337]

This PR broke Python 3.10 usage because of the import of typing.Self instead of typing_extensions.Self

[DarkLight1337]

Why didn't mypy catch this in pre-commit?

[hmellor]

Why didn't mypy catch this in pre-commit?

We are using a relatively old version of mypy. Updating it reveals new issues. There are quite a lot of them though so nobody has got round to updating it.