feat(validation)!: add spherical backend and paper workflow#509
Merged
Conversation
- Add SphericalPoint and SphericalMetric as the spherical coordinate/metric backend for intrinsic S^D points embedded in R^(D+1). - Add the bounded S^2/S^3 SphericalDelaunayBuilder, SphericalDelaunayTriangulation, SphericalSimplex, geodesic distances, and typed spherical construction/validation errors. - Implement spherical validation through Intrinsic PL Topology, Embedding Validity, and empty-cap/ambient-hull Geometric Predicates while keeping D as the manifold dimension. - Expose spherical construction, validation, and topology backend types through the root and focused preludes. - Rename the validation hierarchy across crate docs, README, AGENTS guidance, architecture docs, semgrep fixtures, and developer docs. - Document spherical support as a Level 4/5 backend extension, with full 2D-5D integration and mutable-triangulation support still called out as future work. - Add the validation paper sources, BibTeX, generated PNG figures, tracked reviewer PDF, and papers README. - Rename the validation notebook to notebooks/01_validation.ipynb and make it generate reproducible validation hierarchy and failure figures. - Add just papers and paper-* recipes plus a dedicated papers.yml CI workflow using Tectonic, tex-fmt, chktex, uv, and notebook-generated figures. - Add uv-managed paper support CLIs for PDF sanity checks, deterministic PDF metadata normalization, and TeX source-date extraction. - Include paper artifacts in the packaged crate and ignore LaTeX build intermediates. - Update local/CI cargo-nextest pins and setup/tooling guidance for the new paper toolchain. BREAKING CHANGE: Replace the legacy public SphericalSpace topology helper with SphericalPoint/SphericalMetric and the new spherical Delaunay types. Spherical Delaunay now treats D as the intrinsic dimension of S^D and stores the extra coordinate only in the spherical backend.
Contributor
|
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 ignored due to path filters (2)
📒 Files selected for processing (32)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughThis PR adds a bounded spherical Delaunay prototype, renames validation terminology across code and docs, refactors Pachner-stress execution to a mode-aware flow, and introduces a papers build/validation pipeline with CI, scripts, notebook support, and manuscript assets. ChangesSpherical Validation and Terminology
Pachner Workflow Refactor
Papers Publishing Infrastructure
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 269 |
🟢 Coverage 81.19% diff coverage · -0.18% coverage variation
Metric Results Coverage variation ✅ -0.18% coverage variation (-1.00%) Diff coverage ✅ 81.19% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (69f08d5) 78696 71783 91.22% Head commit (0c2293e) 79921 (+1225) 72759 (+976) 91.04% (-0.18%) 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 (#509) 1340 1088 81.19% 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.
coderabbitai
Bot
added
api
breaking change
enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #509 +/- ##
==========================================
- Coverage 91.19% 91.01% -0.18%
==========================================
Files 87 88 +1
Lines 78475 79700 +1225
==========================================
+ Hits 71564 72540 +976
- Misses 6911 7160 +249
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
.github/workflows/papers.yml (1)
99-100: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo drift check between rebuilt and committed paper artifacts.
The job rebuilds
papers/validation.pdfandpapers/generated/*.pngviajust papersbut never verifies the result matches the tracked copies, so a stale committed PDF/figure (e.g., someone editspapers/validation.texwithout re-running the build) would still pass CI.♻️ Proposed addition
- name: Build papers run: just papers + + - name: Verify tracked paper artifacts are up to date + run: git diff --exit-code -- papers/🤖 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 @.github/workflows/papers.yml around lines 99 - 100, The papers workflow currently rebuilds artifacts with just papers but never checks for drift against the committed outputs, so add a verification step after the build in the papers job that compares the regenerated papers/validation.pdf and papers/generated/*.png files with what is tracked in the repo and fails if they differ. Use the existing Build papers job in the workflow to locate the change, and make the new check explicit so CI catches stale paper artifacts whenever validation.tex or related inputs change without a corresponding rebuild.scripts/paper_source_date.py (1)
30-46: 📐 Maintainability & Code Quality | 🔵 TrivialRedundant re-parsing of the same date string.
parse_paper_source_dateparsesraw_datewithstrptime(Line 62) to buildinstant, and thenPaperSourceDate.__post_init__re-parses the exact same string again (Line 36) to validate agreement. Derivinginstantfromrawinside a single validated constructor (or a classmethod) would avoid duplicate parsing logic.Also applies to: 53-67
🤖 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 `@scripts/paper_source_date.py` around lines 30 - 46, `PaperSourceDate.__post_init__` is redundantly re-parsing the same `raw` date string already handled by `parse_paper_source_date`/`from_raw`, so move the shared parsing/validation into a single constructor path or classmethod and have `__post_init__` only enforce invariants on the already-derived `instant`. Keep the existing checks in `PaperSourceDate`, but avoid calling `datetime.strptime` twice for the same value by deriving `instant` once from `raw` and reusing that result throughout construction.semgrep.yaml (1)
1921-1923: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroaden the matcher to catch plain “Delaunay property” phrasing.
The updated rationale says stale four-level wording should be caught, but the matcher below still only covers Level-4-qualified variants. That leaves bare “Delaunay property” text undetected, so the rule can miss the rename in docs.
Suggested fix
- (?i)(?:\b4-level validation hierarchy\b|\bfour levels of validation\b|\blevel 4:\s*delaunay - property\b|\bdelaunay property\s*\(level 4\)|\bdelaunay level 4 validation\b) + (?i)(?:\b4-level validation hierarchy\b|\bfour levels of validation\b|\blevel 4:\s*delaunay + property\b|\bdelaunay property\s*\(level 4\)|\bdelaunay level 4 validation\b|\bdelaunay property\b)🤖 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 `@semgrep.yaml` around lines 1921 - 1923, The matcher for the validation-stack docs rule is too narrow because it only matches Level 4-qualified Delaunay wording and misses plain “Delaunay property” references. Update the semgrep rule in semgrep.yaml so the relevant pattern covers bare “Delaunay property” phrasing as well, while still catching the existing Level 4 variants; use the existing rule section around the validation hierarchy matcher to adjust the pattern set without changing unrelated matches.src/delaunay/spherical.rs (2)
257-268: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSpherical validation surface lacks
is_valid_*/*_reportparity with other layers.
SphericalDelaunayTriangulationonly exposesvalidate(),validate_topology(),validate_embedding(),validate_delaunay()(allResult-returning, cumulative). As per coding guidelines,src/**/*.rs: "each layer should expose the standard validation surface (is_valid,is_valid_*,*_diagnostic,*_report,validate(),validation_report())" — matching the pattern used byTds/Triangulation/DelaunayTriangulationelsewhere in the crate. Consider adding cheap booleanis_valid_topology/is_valid_embedding/is_valid_delaunaywrappers (and eventually a cumulative report) for API consistency, even in this prototype.Also applies to: 277-350, 465-489, 491-515
🤖 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 `@src/delaunay/spherical.rs` around lines 257 - 268, The SphericalDelaunayTriangulation validation API is missing the standard boolean/report surface used by other layers, so extend the existing validate_topology, validate_embedding, and validate_delaunay methods with cheap is_valid_topology, is_valid_embedding, and is_valid_delaunay wrappers, and add a cumulative validation_report/diagnostic path if appropriate. Keep the new methods aligned with the patterns already used by Tds, Triangulation, and DelaunayTriangulation so the spherical prototype exposes the same validation surface while reusing the existing validation logic.Source: Coding guidelines
517-610: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate origin/degenerate-hyperplane check between embedding and Delaunay validation.
validate_embedding_with_ambient(Lines 522-552) andvalidate_delaunay_with_ambient(Lines 558-586) both computeorigin_sideviasimplex_orientation_with_pointand reject onOrientation::DEGENERATEwith the identical error. Sincevalidate_delaunay()already callsself.validate_embedding()?first, this hyperplane-degeneracy check runs twice per simplex. Consider extracting a shared helper (e.g.,origin_side_for_simplex) thatvalidate_delaunay_with_ambientreuses instead of recomputing.♻️ Sketch of a shared helper
+ fn origin_side_for_simplex<const A: usize>( + &self, + simplex_index: usize, + simplex: &SphericalSimplex<D>, + ) -> Result<Orientation, SphericalDelaunayValidationError> { + let origin = Point::<A>::try_new([0.0; A]).map_err(|source| { + SphericalDelaunayValidationError::AmbientPointValidation { point_index: None, source } + })?; + let side = self.simplex_orientation_with_point::<A>(simplex_index, simplex, origin, None)?; + if side == Orientation::DEGENERATE { + return Err(SphericalDelaunayValidationError::SimplexHyperplaneContainsOrigin { + simplex_index, + simplex_vertices: simplex.vertex_indices().to_vec(), + }); + } + Ok(side) + }Both
validate_embedding_with_ambientandvalidate_delaunay_with_ambientwould then call this once.🤖 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 `@src/delaunay/spherical.rs` around lines 517 - 610, The origin-side/degenerate-hyperplane check is duplicated in both validate_embedding_with_ambient and validate_delaunay_with_ambient, causing the same simplex_orientation_with_point logic and Orientation::DEGENERATE rejection to run twice. Extract the shared origin-side computation into a small helper (for example, an origin_side_for_simplex-style method) and have both validation paths call it, with validate_delaunay_with_ambient reusing the result instead of recomputing it.
🤖 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 @.github/workflows/papers.yml:
- Around line 19-27: The papers workflow trigger is missing some build inputs
and does not verify generated artifacts after the build. Update the paths
filters in the workflow to include scripts/paper_pdf_normalize.py,
scripts/paper_source_date.py, and their tests, then add a post-build diff check
in the papers job that compares papers/validation.pdf and papers/generated/*.png
against the committed versions. Use the existing just papers step as the rebuild
source, and ensure the new diff step fails when regenerated outputs differ.
In `@docs/validation.md`:
- Around line 87-94: Clarify the default validation-policy behavior in the
incremental construction docs: the current wording makes it sound like
suspicion-based checking is the default for all insertions, but PLManifold
starts with ValidationPolicy::ExplicitOnly while only the Pseudomanifold path
uses OnSuspicion. Update the paragraph in the validation documentation to
explicitly distinguish these defaults and reference the incremental construction
path and ValidationPolicy names so readers can see which behavior applies to
each triangulation mode.
In `@docs/workflows.md`:
- Around line 492-493: The workflow text still uses the old “Delaunay property”
wording, so update the remaining Level 5 reference to the new Intrinsic PL
Topology terminology. In the workflow description that mentions validation and
optional repair/verify steps, replace the outdated phrase with the corresponding
Level 5 name so the section is consistent with the rest of the document and the
symbols to locate are the workflow bullet list and the “verify”/“repair”
wording.
In `@pyproject.toml`:
- Around line 59-61: The Ruff isort configuration is missing two new first-party
modules, so import grouping may treat them as third-party. Update the
`known-first-party` list in the `pyproject.toml` Ruff section to include
`paper_pdf_normalize` and `paper_source_date` alongside `paper_check`, keeping
the list aligned with the `py-modules` entries so `paper_pdf_normalize.py` and
`paper_source_date.py` are classified correctly.
In `@scripts/paper_source_date.py`:
- Around line 15-16: The paper-date parsing in scripts/paper_source_date.py
still depends on the process locale through PAPER_DATE_FORMAT and the parsing
logic that uses datetime.strptime, so adjust the date handling in the relevant
parsing function(s) to be locale-independent. Either replace the
locale-sensitive month parsing with an explicit month-name lookup or set LC_TIME
to a fixed English locale such as C or C.UTF-8 before parsing, and make sure the
behavior is consistent in the code paths that read DATE_COMMAND_RE matches.
---
Nitpick comments:
In @.github/workflows/papers.yml:
- Around line 99-100: The papers workflow currently rebuilds artifacts with just
papers but never checks for drift against the committed outputs, so add a
verification step after the build in the papers job that compares the
regenerated papers/validation.pdf and papers/generated/*.png files with what is
tracked in the repo and fails if they differ. Use the existing Build papers job
in the workflow to locate the change, and make the new check explicit so CI
catches stale paper artifacts whenever validation.tex or related inputs change
without a corresponding rebuild.
In `@scripts/paper_source_date.py`:
- Around line 30-46: `PaperSourceDate.__post_init__` is redundantly re-parsing
the same `raw` date string already handled by
`parse_paper_source_date`/`from_raw`, so move the shared parsing/validation into
a single constructor path or classmethod and have `__post_init__` only enforce
invariants on the already-derived `instant`. Keep the existing checks in
`PaperSourceDate`, but avoid calling `datetime.strptime` twice for the same
value by deriving `instant` once from `raw` and reusing that result throughout
construction.
In `@semgrep.yaml`:
- Around line 1921-1923: The matcher for the validation-stack docs rule is too
narrow because it only matches Level 4-qualified Delaunay wording and misses
plain “Delaunay property” references. Update the semgrep rule in semgrep.yaml so
the relevant pattern covers bare “Delaunay property” phrasing as well, while
still catching the existing Level 4 variants; use the existing rule section
around the validation hierarchy matcher to adjust the pattern set without
changing unrelated matches.
In `@src/delaunay/spherical.rs`:
- Around line 257-268: The SphericalDelaunayTriangulation validation API is
missing the standard boolean/report surface used by other layers, so extend the
existing validate_topology, validate_embedding, and validate_delaunay methods
with cheap is_valid_topology, is_valid_embedding, and is_valid_delaunay
wrappers, and add a cumulative validation_report/diagnostic path if appropriate.
Keep the new methods aligned with the patterns already used by Tds,
Triangulation, and DelaunayTriangulation so the spherical prototype exposes the
same validation surface while reusing the existing validation logic.
- Around line 517-610: The origin-side/degenerate-hyperplane check is duplicated
in both validate_embedding_with_ambient and validate_delaunay_with_ambient,
causing the same simplex_orientation_with_point logic and
Orientation::DEGENERATE rejection to run twice. Extract the shared origin-side
computation into a small helper (for example, an origin_side_for_simplex-style
method) and have both validation paths call it, with
validate_delaunay_with_ambient reusing the result instead of recomputing it.
🪄 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: 6f71ad19-490a-4956-8699-116888e51705
⛔ Files ignored due to path filters (4)
papers/generated/validation_hierarchy.pngis excluded by!**/*.png,!**/generated/**papers/generated/validation_model_failures.pngis excluded by!**/*.png,!**/generated/**papers/validation.pdfis excluded by!**/*.pdfuv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.github/workflows/ci.yml.github/workflows/codecov.yml.github/workflows/papers.yml.gitignoreAGENTS.mdCITATION.cffCargo.tomlREADME.mddocs/ORIENTATION_SPEC.mddocs/README.mddocs/api_design.mddocs/architecture/module_map.mddocs/architecture/prelude_reference.mddocs/architecture/project_structure.mddocs/code_organization.mddocs/dev/commands.mddocs/dev/docs.mddocs/dev/tooling-alignment.mddocs/invariants.mddocs/limitations.mddocs/topology.mddocs/validation.mddocs/workflows.mdjustfilenotebooks/01_validation.ipynbpapers/README.mdpapers/validation.bibpapers/validation.texpyproject.tomlscripts/paper_check.pyscripts/paper_pdf_normalize.pyscripts/paper_source_date.pyscripts/tests/test_paper_check.pyscripts/tests/test_paper_pdf_normalize.pyscripts/tests/test_paper_source_date.pysemgrep.yamlsrc/config.rssrc/core/tds/storage.rssrc/core/validation.rssrc/delaunay/spherical.rssrc/lib.rssrc/topology/spaces/spherical.rssrc/topology/traits/global_topology_model.rssrc/topology/traits/topological_space.rstests/prelude_exports.rstests/semgrep/docs/validation_levels.mdtests/spherical_delaunay.rs
- Make spherical topology own `S^d` point canonicalization, ambient `R^(d+1)` helpers, geodesic distances, and layered topology/embedding/Delaunay validation wrappers. - Simplify Pachner stress tooling around direct round-trip and random-walk workloads with stable benchmark fixtures. - Keep the validation manuscript as author-owned outline scaffolding and wire paper figure, PDF sanity, source-date, and CI freshness checks into the paper workflow. - Strengthen paper helper scripts and notebook generation with typed parsing for page counts, source dates, PDF normalization, and generated validation figures. BREAKING CHANGE: spherical topology helpers and Pachner stress workflows were reorganized around the canonical topology backend and direct `round-trip`/`random-walk` interfaces.
acgetchell
enabled auto-merge (squash)
July 7, 2026 03:29
acgetchell
disabled auto-merge
July 7, 2026 03:33
coderabbitai
Bot
removed
enhancement
acgetchell
enabled auto-merge (squash)
July 7, 2026 04:12
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
1 participant
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.
BREAKING CHANGE: Replace the legacy public SphericalSpace topology helper with SphericalPoint/SphericalMetric and the new spherical Delaunay types. Spherical Delaunay now treats D as the intrinsic dimension of S^D and stores the extra coordinate only in the spherical backend.