Repository navigation
Recompute output integrals during stored-report verification - #4
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecd4baf9d0
ℹ️ 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".
| require(normalize([s.geometry() for s in states])==normalize(d['operands']),'final reduced geometry/ancestry replay mismatch') | ||
| reduced=d['operands'] | ||
| final=failure_signature([check(m['vertices'],m['faces']) for m in reduced],d['final_outputs']['FLOAT']) | ||
| validate_mesh(d['final_outputs']['MANIFOLD']);output_integral(d['final_outputs']['MANIFOLD']) |
There was a problem hiding this comment.
When reduction reaches geometry for which MANIFOLD returns invalid topology, reduce_pair() still emits a valid witness report because solidrift/engine.py validates the comparator only at baseline and during repair quality checks, not for final_outputs['MANIFOLD']. This new validate_mesh call therefore makes verify reject reports produced by the analyzer itself; recompute the retained integral without imposing a topology invariant that generation does not guarantee.
Useful? React with 👍 / 👎.
| vv=[tuple(Fraction(x) for x in p) for p in vertices];origin=vv[0] | ||
| value=sum((dot(sub(vv[f[0]],origin),cross(sub(vv[f[1]],origin),sub(vv[f[2]],origin)))/6 for f in faces),Fraction(0)) |
There was a problem hiding this comment.
When FLOAT returns an open or otherwise non-closed surface—which this helper explicitly permits—the signed face sum depends on the tetrahedron origin. native.mesh_data() stores BMesh.calc_volume(signed=True), whose triangle-volume primitive uses the world origin (Blender source), but this implementation translates every face by vertex 0. An authentic output containing a boundary triangle away from that vertex therefore gets a different recomputed scalar and is rejected; use the same world-origin integral as the native serializer.
Useful? React with 👍 / 👎.
| # Blender accumulates a floating integral. Permit a narrowly declared | ||
| # relative accumulation difference, with no absolute floor at small scales. | ||
| require(math.isclose(actual,declared,rel_tol=1e-9,abs_tol=0.0),'serialized output volume differs from recomputed surface integral') |
There was a problem hiding this comment.
For valid geometry translated far from the origin, Blender's world-origin double accumulation can differ from the exact rational result by more than this fixed 1e-9 relative tolerance because large tetrahedron terms cancel; Blender performs that accumulation as shown in its BMesh implementation. For example, a valid binary32 tetrahedron near coordinate 960,000 produces native volume 13332660.249348959 but exact volume 13332660.271769205 (a 1.68e-9 relative difference), causing an authentic report to fail here; the identical tolerance in bind_signature also needs an error bound that accounts for coordinate magnitude and accumulation.
Useful? React with 👍 / 👎.
Address the GitHub Codex review: validate retained output mesh scalars against exact-coordinate signed-integral recomputation, use recomputed values in failure tests, and bind mirrored signature fields to checked inputs/outputs. Native accumulation comparison uses1e-9 relative tolerance with no absolute floor. Before the fix, baseline/witness/undo geometry/scalar corruptions all passed; now all reject while the authentic native report verifies60 steps. Independent review also checked six mirror corruptions and schema1 compatibility.26 Python tests include the recorded native compatibility case, scalar corruption, tiny-scale tolerance and malformed values. Native analysis is unchanged; version0.1.2 corrects the verifier without claiming report authenticity or ideal-union proof.