feat(): remove deprecated remove_non_manifold_faces key from meshing defaults#1774
feat(): remove deprecated remove_non_manifold_faces key from meshing defaults#1774benflexcompute wants to merge 4 commits intomainfrom
remove_non_manifold_faces key from meshing defaults#1774Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d98fce700b
ℹ️ 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if "remove_non_manifold_faces" in value: | ||
| value.pop("remove_non_manifold_faces", None) | ||
| message = "`meshing.defaults.remove_non_manifold_faces` is no longer supported and has been ignored." |
There was a problem hiding this comment.
can we direct user to use remove_hidden_geometry in the shown message? or even better: if remove_non_manifold_faces = true, we show the warning, and translate that to remove_hidden_geometry = true with the default value of flooding_cell_size.
There was a problem hiding this comment.
Warning updated. Usually it is not good idea to silently change user inputs without explicit consent.
…g
Note
Medium Risk
Changes meshing parameter validation/serialization by silently ignoring a previously accepted field, which could affect users still passing it despite deprecation; impact is limited to a deprecated option and is covered by updated tests.
Overview
Removes
meshing.defaults.remove_non_manifold_facesfrom theMeshingDefaultsmodel and from the GAI surface-meshing translation whitelist, eliminating it from generated/filtered meshing JSON.Adds forward-compat cleanup for legacy payloads:
MeshingDefaultsnow strips the deprecated field on construction while emitting a validation warning, and a new_to_25_9_0updater step drops the key from serialized params (implemented but intentionally not wired intoVERSION_MILESTONESyet). Tests and reference JSON fixtures are updated accordingly, including new coverage for the warning behavior and updater removal.Written by Cursor Bugbot for commit 210828e. This will update automatically on new commits. Configure here.