feat(io): add validated mesh export schema#479
Conversation
- Add stable UUID-based visualization and mesh export DTOs with validated wrappers, typed topology schema values, and structured export/validation errors. - Expose `to_visualization_data` and `to_mesh_export` through crate-root and focused export prelude APIs, with DelaunayResult-compatible error conversions. - Document the mesh export JSON contract, validation boundary, and downstream attribute extension model. - Add reusable notebook hygiene tooling and flatten `just ci` into orthogonal validation buckets with one release-profile Rust nextest pass. Closes #446
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds stable mesh-export schemas, validation, re-exports, docs, and tests; adds a notebook-check CLI with notebook-specific validation commands and tests; updates unreleased changelog notes for 2D edge-query APIs. ChangesStable Mesh Export
Notebook Validation Workflow
Release Notes Update
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 427 |
🟢 Coverage 99.84% diff coverage · +0.08% coverage variation
Metric Results Coverage variation ✅ +0.08% coverage variation (-1.00%) Diff coverage ✅ 99.84% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (af2620b) 72969 66846 91.61% Head commit (a3482c5) 73579 (+610) 67460 (+614) 91.68% (+0.08%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#479) 610 609 99.84% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
tests/mesh_export.rs (1)
43-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a small 4D/5D sweep for the const-generic export path.
Most of this suite exercises
MeshExport<D>through the 2D helper, with only one 3D-specific check later. A lightweight validate/round-trip sweep across 2D through 5D would catch higher-dimensional arity and adjacency regressions in this new const-generic surface. As per coding guidelines,tests/**/*.rs: “dimension-generic tests should cover 2D through 5D whenever feasible.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/mesh_export.rs` around lines 43 - 65, Add a small dimension-generic test sweep to the const-generic mesh export path by extending the existing `sample_export`/`sample_delaunay_result_export` coverage beyond `MeshExport<2>` and the later 3D check. Update the test helpers or add new ones so `DelaunayTriangulationBuilder`, `to_mesh_export`, and the validate/round-trip assertions are exercised for 2D through 5D, keeping the test lightweight while verifying higher-dimensional arity and adjacency behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 31: The changelog entry was edited directly instead of being produced by
the project’s changelog generator. Revert the manual bullet changes in
CHANGELOG.md and regenerate the file using the changelog utility so the release
notes stay in sync with the generator’s source of truth; apply the same fix to
the affected changelog section referenced by the comment.
In `@docs/architecture/project_structure.md`:
- Line 71: The project structure snapshot is stale because the updated tree only
shows src/io/ and omits other newly added mesh-export artifacts. Refresh the
documentation in project_structure.md so the tree also includes
docs/mesh_export.md and tests/mesh_export.rs, keeping the snapshot aligned with
the current repo layout and any related mesh-export symbols.
In `@docs/dev/commands.md`:
- Around line 68-71: The Markdown validation guidance in the docs table is
outdated and should reference the focused handoff validator used for docs-only
changes. Update the entries in the relevant docs/dev markdown table, especially
the Markdown documentation row, to point iteration/final validation to just
markdown-ci instead of just markdown-check, and make sure the guidance is
consistent with the focused rules documented in docs/dev/README.md and the
related docs/dev/*.md pages.
In `@scripts/notebook_check.py`:
- Around line 520-535: The execute path in execute() is leaking uncaught
ModuleNotFoundError and nbclient runtime/timeout failures as full tracebacks.
Wrap the optional imports and NotebookClient.execute() call in execute() with
narrow exception handling, and convert those failures into the same concise
one-line error style used by main(), referencing execute() and the
NotebookClient creation/execution flow so the behavior stays consistent.
- Around line 351-370: The Ruff notebook checks are still using the caller’s
working directory instead of the configured repo root. Update
ruff_lint_diagnostics() and ruff_format_diagnostics() to accept and pass through
options.project_root (or an equivalent cwd=repo_root) when calling
run_safe_command, matching the behavior already used by ty_diagnostics(). This
should ensure Ruff resolves the same pyproject.toml and repo-local config when
notebook-check is run with --repo-root.
- Around line 23-24: The diagnostic parsing in notebook_check is only matching
Ruff’s arrow-style locations, so plain path:line:column messages still map to
cell 0. Update the location extraction in scripts/notebook_check.py by extending
the existing RUFF_LOCATION_RE / the diagnostic handling around parse_output to
also recognize Ruff’s inline file:line:column format, and make sure the cell
lookup uses the parsed line/column so Ruff errors resolve to the correct
notebook cell.
In `@src/delaunay/construction.rs`:
- Around line 311-325: Reduce the size of DelaunayError by changing the
VisualizationExport and VisualizationDataValidation variants to hold their
source errors indirectly (for example, boxed) so the enum no longer trips
Clippy’s result_large_err check. Update the DelaunayError definition in
construction.rs and keep the routing of mesh export failures through these
variants intact, rather than adding call-site workarounds in mesh_export.rs or
other DelaunayResult users.
In `@src/io/visualization.rs`:
- Around line 1125-1194: The reciprocity check in visualization validation
treats a self-neighbor as its own reciprocal, so an asymmetric self-gluing can
incorrectly pass. Update the adjacency validation in the logic around
neighbor_edges and the asymmetric adjacency pass to special-case simplex_id ==
neighbor_simplex_id, requiring a distinct reciprocal record (or explicit
reciprocal facet identity) instead of accepting the same adjacency slot. Add a
regression test covering a self-neighbor record that should fail unless a
separate reciprocal entry exists.
---
Nitpick comments:
In `@tests/mesh_export.rs`:
- Around line 43-65: Add a small dimension-generic test sweep to the
const-generic mesh export path by extending the existing
`sample_export`/`sample_delaunay_result_export` coverage beyond `MeshExport<2>`
and the later 3D check. Update the test helpers or add new ones so
`DelaunayTriangulationBuilder`, `to_mesh_export`, and the validate/round-trip
assertions are exercised for 2D through 5D, keeping the test lightweight while
verifying higher-dimensional arity and adjacency behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 133d31d7-3906-4981-b9c1-f55adf84fa5f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
AGENTS.mdCHANGELOG.mdREADME.mddocs/README.mddocs/architecture/module_map.mddocs/architecture/prelude_reference.mddocs/architecture/project_structure.mddocs/code_organization.mddocs/dev/commands.mddocs/dev/python.mddocs/dev/rust.mddocs/dev/testing.mddocs/mesh_export.mdjustfilepyproject.tomlscripts/README.mdscripts/notebook_check.pyscripts/tests/test_notebook_check.pysrc/delaunay/construction.rssrc/io/visualization.rssrc/lib.rstests/mesh_export.rstests/prelude_exports.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #479 +/- ##
==========================================
+ Coverage 91.58% 91.66% +0.07%
==========================================
Files 80 81 +1
Lines 72748 73358 +610
==========================================
+ Hits 66627 67241 +614
+ Misses 6121 6117 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
- Reject one-sided self-neighbor adjacency in mesh exports while preserving reciprocal self-neighbor records for valid pseudomanifold data. - Keep DelaunayResult small by boxing typed DelaunayError sources without erasing source-chain or variant information. - Make notebook validation resolve Ruff from the configured repo root and report execution dependency or runtime failures as concise CLI errors. - Align focused validation guidance so documentation, notebooks, and doctests run through the intended handoff workflows exactly once. BREAKING CHANGE: DelaunayError variants now store source errors as Box<T>; callers that destructure source fields should match through source.as_ref().
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/notebook_check.py`:
- Around line 622-627: The execute() notebook runner still allows a pre-set GUI
backend because it uses os.environ.setdefault for MPLBACKEND. Update execute()
in scripts/notebook_check.py to force the headless backend by assigning Agg
unconditionally before notebook execution, so callers of
load_notebook_execution_backend() and execute() cannot override it with TkAgg or
similar.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6b79f1ad-9ca4-40ca-a549-e17f36455bb7
📒 Files selected for processing (11)
docs/architecture/project_structure.mddocs/dev/commands.mddocs/dev/testing.mdjustfilescripts/notebook_check.pyscripts/tests/test_notebook_check.pysrc/delaunay/construction.rssrc/io/visualization.rstests/mesh_export.rstests/pachner_roundtrip.rstests/prelude_exports.rs
✅ Files skipped from review due to trivial changes (2)
- docs/dev/testing.md
- docs/architecture/project_structure.md
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/dev/commands.md
- tests/prelude_exports.rs
- src/delaunay/construction.rs
- src/io/visualization.rs
- tests/mesh_export.rs
- justfile
- Override ambient Matplotlib backends before notebook execution so notebook checks cannot inherit GUI-only settings. - Expand mesh-export invariant coverage for topology schema spellings, validated DTO conversion, adjacency export failures, and DelaunayError source routing.
to_visualization_dataandto_mesh_exportthrough crate-root and focused export prelude APIs, with DelaunayResult-compatible error conversions.just ciinto orthogonal validation buckets with one release-profile Rust nextest pass.Closes #446