Skip to content

[Snappy] Proximity spacing additional validator#1949

Open
piotrkluba wants to merge 8 commits intorelease-candidate/25.8from
piotr/extend-proximity-validator
Open

[Snappy] Proximity spacing additional validator#1949
piotrkluba wants to merge 8 commits intorelease-candidate/25.8from
piotr/extend-proximity-validator

Conversation

@piotrkluba
Copy link
Copy Markdown
Collaborator

@piotrkluba piotrkluba commented Apr 1, 2026

Note

Medium Risk
Tightens validation by turning an auto-correcting warning into hard ValueErrors, which can break existing configs that previously passed and were silently clamped. Changes are localized to snappy meshing parameter validation with low runtime risk but potential user-facing validation failures.

Overview
Enforces stricter proximity_spacing validation for Snappy refinements. SnappyEntityRefinement now raises a ValueError when proximity_spacing > min_spacing instead of logging a warning and mutating the value.

Adds default-aware validation for BodyRefinement. When min_spacing is omitted, SurfaceMeshingParams now rejects BodyRefinement.proximity_spacing values that exceed defaults.min_spacing, and new tests cover both failing and passing cases for these scenarios.

Written by Cursor Bugbot for commit a4a4adf. This will update automatically on new commits. Configure here.

if isinstance(refinement, BodyRefinement):
if refinement.min_spacing is None and refinement.proximity_spacing:
if refinement.proximity_spacing > self.defaults.min_spacing:
log.warning(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use add_validation_warning()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the user input in the validator is something I try to avoid, I cannot say exactly what is wrong with it but it feels strange to make SimulationParams mutable during the validation process.

I guess we cannot make this an error and ask user to fix it b.c. it breaks compatibility. Let's keep this as is for now and we will see how this holds up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh, we coul eaisly make this an error. Snappy does not have any real users really and its a small change so we can make this an error if you wiss

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it an error

Copy link
Copy Markdown

@cursor cursor bot left a 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 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants