refactor: migrate config to Pydantic v2 field_validator - #36
Merged
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
lezama
force-pushed
the
wpbench/11-artifact-modes
branch
from
July 10, 2026 16:15
542ec74 to
18e5ecc
Compare
The project requires pydantic>=2.8 but still used the deprecated v1 @validator, which warned on every run and breaks in Pydantic v3. - @validator('temperature') -> @field_validator + @classmethod. - Added @field_validator for top_p (0..1 or None). - Tightened numeric fields with Field constraints: request_timeout, timeout_seconds, setup_timeout_seconds, concurrency (gt=0) and limit (gt=0 when set). - Test suite now runs with zero Pydantic deprecation warnings; a regression test constructs every config model under warnings.simplefilter('error', DeprecationWarning). Verified: pytest python (131 passed, 9 new validation tests; the long-standing PydanticDeprecatedSince20 warning is gone), ruff clean, mypy (2 pre-existing trunk errors only).
lezama
force-pushed
the
wpbench/12-pydantic-v2
branch
from
July 10, 2026 16:19
57696d3 to
f5b965b
Compare
Contributor
|
rebased on trunk, green (133). the PydanticDeprecated warning that's been in every run is now gone, no v1 validators left, and the regression guard fails CI if one comes back 👍 |
lezama
pushed a commit
that referenced
this pull request
Jul 10, 2026
#37) ## Why this matters A benchmark repo needs unusually high change discipline, because its failure modes are silent: a dataset edit can shift every model's score, a runtime change can break grading without any test noticing locally, and a scoring tweak can quietly invalidate comparisons with published history. Until now none of the documented checks ran automatically — trust depended on every contributor remembering to run everything by hand. This puts the quality gates in the merge path and, just as importantly, makes *score-breaking changes explicit*: anyone touching datasets, runtime, or scoring must declare whether results remain comparable, so the leaderboard's history stays interpretable. ## Changes **CI (four jobs on PRs and trunk pushes)** - **python** — ruff, mypy, pytest against an editable install with dev extras - **dataset** — dataset shape/metadata tests plus a full Parquet export build (currently 470 tests), so export breakage is caught before publish time - **runtime** — `php -l` over every runtime PHP file - **docker** — grader image build, catching Dockerfile/package rot **mypy is now enforced and fully clean** — this PR fixes the last two long-standing errors (missing PyYAML stubs in dev deps; a targeted ignore for the `datasets` library's dynamically-generated exports). The codebase goes from 3 mypy errors on trunk to 0, gated in CI. **Process** - PR template requiring verification commands and an explicit score-impact declaration: either "does not change scoring" or a version bump (`scoring_version` / suite / `RESULT_SCHEMA_VERSION`) plus a comparability note. - `RELEASE_CHECKLIST.md`: version bumps, quality gates, full reference-solution verification against a live runtime, dataset export validation with row-count checks, and Docker image publication *by digest* so official runs pin an immutable grader. ## Verification - `pytest python`: **131 passed** - `ruff check python`: clean - `mypy python`: **clean — 0 errors** (was 3 on trunk) - `python datasets/export_dataset.py`: 470 tests exported with all expected columns - Workflow YAML validated ## Notes - Stacked on #36 — final PR in the series. - The full reference-solution runtime smoke test lives in the release checklist rather than per-PR CI; running wp-env inside Actions is worth doing but deserves its own cost/flakiness evaluation first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this matters
The config layer is the first thing every contributor and every run touches, and right now it greets them with a deprecation warning on literally every invocation — the project pins
pydantic>=2.8but still uses the v1@validatorAPI, which Pydantic removes entirely in v3. Beyond the noise (which trains people to ignore warnings, exactly what a benchmark repo can't afford), it's a time bomb: the first dependency bump to Pydantic v3 would break config loading outright. This clears the debt and uses the opportunity to tighten validation where invalid values previously slipped through to fail confusingly mid-run.Changes
@validator('temperature')→@field_validator+@classmethod(v2 idiom).top_pvalidator (must be 0–1 or unset) — previously any float was accepted and failed provider-side with a cryptic API error.Fieldconstraints on numerics that must be positive:request_timeout,timeout_seconds,setup_timeout_seconds,concurrency, andlimit— alimit: 0or zero timeout now fails at config load with a clear message instead of producing a silently empty or hanging run.warnings.simplefilter('error', DeprecationWarning), so any future deprecated-API usage in the config layer fails CI rather than accumulating.Verification
pytest python: 131 passed (9 new validation tests) — and thePydanticDeprecatedSince20warning that appeared in every previous test run is goneruff check python: cleanmypy python: 2 pre-existing trunk errors onlyvalidatorimports remain anywhere in the packageNotes