feat(validation)!: add certified realization intersection fast paths - #531
Merged
Conversation
- Add orientation, shared-face, and exact linear-program certificates before the conservative active-set fallback. - Dispatch finite floating-point systems through la-stack's exact Bareiss solver while preserving rational fallbacks and typed witnesses. - Bound the 4D and 5D release regressions and document the broader evidence deferred to #482 and #483. - Align release documentation and tooling with Rust 1.97.1. - Refresh pinned development tools and override Semgrep's vulnerable MCP dependency with the patched release. - Harden Tectonic dependency discovery across supported development environments. BREAKING CHANGE: Rust 1.97.1 is now the minimum supported Rust version. Closes #506
acgetchell
enabled auto-merge (squash)
July 26, 2026 05:49
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 selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR standardizes the Rust 1.97.1 baseline, locks Semgrep execution, updates Tectonic and paper artifact tooling, adds exact Level-4 simplex intersection validation, and bounds high-dimensional regression tests. ChangesRelease validation and tooling
Geometry validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Validator as simplex intersection validator
participant Confinement as shared-face confinement
participant LP as revised-simplex LP solver
participant ExactSolver as exact runtime solver
Validator->>Confinement: test shared-face confinement
Validator->>LP: classify intersection constraints
LP->>ExactSolver: solve exact basis systems
ExactSolver-->>LP: return rational solution or error
LP-->>Validator: return classification or witness
Possibly related issues
Possibly related PRs
Suggested labels: 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 | 312 |
🟢 Coverage 83.73% diff coverage · -0.19% coverage variation
Metric Results Coverage variation ✅ -0.19% coverage variation (-1.00%) Diff coverage ✅ 83.73% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (91efa35) 80351 73552 91.54% Head commit (75ea22f) 81860 (+1509) 74775 (+1223) 91.34% (-0.19%) 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 (#531) 1727 1446 83.73% 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
enhancement
![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: 2
🧹 Nitpick comments (2)
src/geometry/realization.rs (2)
1025-1152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider moving the LP machinery into its own module.
realization.rsnow carries a complete revised-simplex implementation (exact + provisionalf64phases, artificial-variable handling, dual certificates, matrix helpers) alongside realization validation. Asrc/geometry/simplex_lp.rs(or similar) submodule with a narrowintersection_via_linear_programsurface would keep the validator readable and let the solver be unit-tested independently.Also applies to: 1154-1432
🤖 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/geometry/realization.rs` around lines 1025 - 1152, Move the revised-simplex LP implementation and its supporting exact/provisional phases, artificial-variable handling, dual certificates, and matrix helpers from realization.rs into a dedicated geometry submodule such as simplex_lp.rs. Keep intersection_via_linear_program as the narrow public-facing surface used by realization validation, update imports and visibility accordingly, and preserve the existing LP behavior while making the solver independently testable.
2622-2660: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the new LP tests beyond D=2.
All three new
intersection_via_linear_programcases are 2D triangles, so the Phase I/II paths, the dual certificate, andexact_axis_through_shared_faceare only exercised at the smallest dimension where most fast paths short-circuit early. Adding a 3D shared-facet case and a 3D crossing case (and ideally 4D/5D) would cover the code that this PR is actually optimizing.As per coding guidelines, "dimension-generic tests should cover dimensions 2 through 5 whenever feasible".
Also applies to: 2780-2800
🤖 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/geometry/realization.rs` around lines 2622 - 2660, Extend the tests for intersection_via_linear_program beyond 2D triangles to cover dimensions 3 through 5 where feasible. Add explicit 3D shared-facet and crossing cases, and include analogous higher-dimensional cases when practical, while preserving assertions for valid intersections and exercising Phase I/II, dual-certificate, and exact_axis_through_shared_face paths.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 `@justfile`:
- Around line 1143-1214: Update ensure_tectonic_build_dependencies and its
required_pkg_config_packages logic in justfile (1143-1214) to apply Tectonic’s
platform-specific prerequisites: do not require fontconfig on macOS, and require
openssl on non-Apple platforms. Update the corresponding prerequisite
documentation in docs/dev/commands.md (635-645) to describe the same conditional
requirements.
In `@papers/validation.tex`:
- Around line 48-64: Resolve the authorship conflict in the introduction around
the paragraphs beginning “Applications such as Causal Dynamical Triangulation”
and “Delaunay is a Rust library”: if the prose is not authored by Adam, replace
both narrative paragraphs with concise \AuthorTodo{...} prompts, preserving the
intended topics and outline structure; otherwise leave the prose unchanged.
---
Nitpick comments:
In `@src/geometry/realization.rs`:
- Around line 1025-1152: Move the revised-simplex LP implementation and its
supporting exact/provisional phases, artificial-variable handling, dual
certificates, and matrix helpers from realization.rs into a dedicated geometry
submodule such as simplex_lp.rs. Keep intersection_via_linear_program as the
narrow public-facing surface used by realization validation, update imports and
visibility accordingly, and preserve the existing LP behavior while making the
solver independently testable.
- Around line 2622-2660: Extend the tests for intersection_via_linear_program
beyond 2D triangles to cover dimensions 3 through 5 where feasible. Add explicit
3D shared-facet and crossing cases, and include analogous higher-dimensional
cases when practical, while preserving assertions for valid intersections and
exercising Phase I/II, dual-certificate, and exact_axis_through_shared_face
paths.
🪄 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: c3b197b9-c823-40b7-98b3-ed326b485db1
⛔ Files ignored due to path filters (2)
papers/validation.pdfis excluded by!**/*.pdfuv.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.config/nextest.toml.github/workflows/semgrep-sarif.ymlAGENTS.mdCargo.tomlREADME.mdclippy.tomldocs/dev/commands.mddocs/dev/tooling-alignment.mddocs/limitations.mddocs/roadmap.mdjustfilepapers/ARTIFACT.mdpapers/validation.texpyproject.tomlrust-toolchain.tomlscripts/tests/test_benchmark_models.pysrc/geometry/matrix.rssrc/geometry/realization.rstests/large_scale_debug.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #531 +/- ##
==========================================
- Coverage 91.51% 91.32% -0.20%
==========================================
Files 87 88 +1
Lines 80140 81649 +1509
==========================================
+ Hits 73343 74566 +1223
- Misses 6797 7083 +286
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
- Move exact revised-simplex intersection solving behind a private geometry utility while preserving fallback and certificate behavior. - Harden Level 4 shared-facet and crossing classification through 5D, including singular and high-range barycentric systems. - Compare paper artifacts structurally across platforms and align Tectonic prerequisites without pkgx. - Refresh validation artifacts and uv notebook dependencies, and enforce native Python 3.14 annotation semantics. Closes #506
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/geometry/realization.rs`:
- Around line 695-727: Update intersection_is_confined_by_orientation to first
require coordinate identity for all shared labels, reusing the existing
coordinates_are_identical check and matching the simplex_lp.rs behavior. Return
false when shared-label coordinates differ, before applying the orientation
shortcut; preserve the current orientation logic for identical coordinates.
In `@src/geometry/util/simplex_lp.rs`:
- Around line 1370-1400: Guard all non-finite intermediates before calling
rational_from_f64 in the shared-coordinate confinement path: validate each axis
value after 1024 scaling and validate the dot-product-derived provisional affine
values before conversion, returning None on failure. Mirror the post-scaling
finiteness validation used by filtered_single_shared_vertex_confinement, while
preserving the existing successful affine computation.
🪄 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: a7b3c88e-9894-45c1-ad16-344f6405ac2f
⛔ Files ignored due to path filters (8)
docs/assets/validation/validation_hierarchy.pngis excluded by!**/*.pngdocs/assets/validation/validation_level_1_element_validity.pngis excluded by!**/*.pngdocs/assets/validation/validation_level_2_combinatorial_consistency.pngis excluded by!**/*.pngdocs/assets/validation/validation_level_3_intrinsic_pl_topology.pngis excluded by!**/*.pngdocs/assets/validation/validation_level_4_valid_realization.pngis excluded by!**/*.pngdocs/assets/validation/validation_level_5_geometric_predicates.pngis excluded by!**/*.pngpapers/validation.pdfis excluded by!**/*.pdfuv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/papers.ymldocs/architecture/module_map.mddocs/dev/commands.mddocs/dev/tooling-alignment.mdjustfilepyproject.tomlscripts/check_docs_version_sync.pyscripts/paper_check.pyscripts/tag_release.pyscripts/tests/test_check_docs_version_sync.pyscripts/tests/test_paper_check.pysemgrep.yamlsrc/geometry/matrix.rssrc/geometry/realization.rssrc/geometry/util/simplex_lp.rssrc/lib.rstests/semgrep/scripts/tests/python_style.py
💤 Files with no reviewable changes (3)
- scripts/tag_release.py
- scripts/check_docs_version_sync.py
- scripts/tests/test_check_docs_version_sync.py
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- docs/dev/tooling-alignment.md
- Require coordinate-identical shared vertices before using orientation confinement. - Reject non-finite scaled axes and affine offsets before exact conversion.
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: Rust 1.97.1 is now the minimum supported Rust version.
Closes #506