-
Notifications
You must be signed in to change notification settings - Fork 70
refactor(tidy3d): FXC-5286-pydantic-v-2-refactor-validator-sprawl #3265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
|
07ee26c to
e86fd90
Compare
e86fd90 to
c5b752a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
c5b752a to
7fbc731
Compare
7fbc731 to
dbd7e5a
Compare
yaugenst-flex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcorudolphflex thanks, this looks good directionally. But as evidenced by this thread i think we need to improve the process design here too. At the very least there should be some docstrings, and also a contributor doc (could add this to AGENTS.md too) that documents the correct way to add validators in this style.
dbd7e5a to
806c113
Compare
That makes definitely sense! Added some notes to |
57e250f to
ee771f8
Compare
Refactors validator hotspots by consolidating multiple @model_validator(mode="after") hooks into a single orchestrator validator that invokes ordered helper methods. This preserves existing warning/error ordering and messages while improving readability and maintainability. Tests are added/updated to lock in validation order where needed.
Note
Medium Risk
Touches core model validation paths for mediums and simulations, so regressions could change warning/error timing or behavior even though the checks themselves are largely unchanged.
Overview
Refactors Pydantic v2 post-init validation for the
MediumandSimulationclass families by replacing many scattered@model_validator(mode="after")methods with a single_run_after_validators()orchestrator per class that calls helper checks in an explicit order.Adds
call_wrapped_validator(...)to invoke existing validator factories deterministically, updates several simulation variants (Simulation,ModeSimulation,EMESimulation, TCAD heat/charge) and multiple medium subclasses to follow the new pattern, and renames/privatizes various validator helpers for consistency.Tests and policy enforcement: adds new tests to lock in validator warning/error ordering (e.g.,
CustomMedium,Simulation) and introducestest_validator_policy.pyto prevent newmode="after"validators from being added outside_run_after_validators()in the medium/simulation hierarchies.Written by Cursor Bugbot for commit ee771f8. This will update automatically on new commits. Configure here.