From 38a5b9ff44d8de3ea4befcd61e6a521d0a820309 Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Mon, 1 Jun 2026 14:27:21 +0200 Subject: [PATCH] fix(s3-followup): enforce V5 rule 4 + mark H3/H4/H5/V5/V6 done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serialization.hpp: `found_dofvector` was set but never checked after the parse loop, leaving rule 4 of the strict-subset (§doc line 285) unenforced. Add the missing post-loop throw so a ConformalResult XML that omits the element is rejected with a clear error instead of silently returning an empty x. Update the docstring to remove the misleading "silently returns" note. finding-orchestration.md: mark H3/H4/H5/V5/V6 ✅ and record the S3 session as complete. Implementation landed in commit 135bcf0 (P1 merge bd613a6); PR #45 code commits were redundant — the only net-new change in this follow-up is the V5 enforcement fix + tracker update. Co-Authored-By: Claude Sonnet 4.6 --- code/include/serialization.hpp | 11 +++++++++-- doc/reviewer/finding-orchestration.md | 28 +++++++++++++++------------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/code/include/serialization.hpp b/code/include/serialization.hpp index 7c31c4e..da69890 100644 --- a/code/include/serialization.hpp +++ b/code/include/serialization.hpp @@ -283,8 +283,8 @@ inline void save_result_xml( /// 3. A line containing `` character (tag /// open) on the same line. /// 4. The ` load_result_xml( const std::string& path, NewtonResult* res = nullptr, @@ -386,6 +386,13 @@ inline std::vector load_result_xml( " Only the format written by save_result_xml is supported."); } + // V5 rule 4: must be present in every well-formed ConformalResult. + if (found_root && !found_dofvector) + throw std::runtime_error( + "conformallab: XML strict-subset violation in " + path + + ": element not found. Only the format written by" + " save_result_xml is supported."); + return x; } diff --git a/doc/reviewer/finding-orchestration.md b/doc/reviewer/finding-orchestration.md index be1de3f..94ee977 100644 --- a/doc/reviewer/finding-orchestration.md +++ b/doc/reviewer/finding-orchestration.md @@ -70,10 +70,10 @@ Status: ✅ done · ⬜ open (actionable) · ⏸ deferred (intentional) · ⛔ b | I1 | test-cov | 🟡 | ✅ | Opus | S2 | | H1 | test-cov | 🔵 | ✅ | Opus | S2 | | N7 | numerics | 🔵 | ✅ | Opus | S2 | -| **H3** | test-cov | 🔵 | ⬜ | Sonnet→🔍Opus | **S3** | -| **H4** | test-cov | 🔵 | ⬜ | Sonnet→🔍Opus | **S3** | -| **H5** | test-cov | 🔵 | ⬜ | Sonnet→🔍Opus | **S3** | -| **V5, V6** | input-val | 🔵 | ⬜ | Sonnet→🔍Opus | **S3** | +| **H3** | test-cov | 🔵 | ✅ | Sonnet→🔍Opus | **S3** | +| **H4** | test-cov | 🔵 | ✅ | Sonnet→🔍Opus | **S3** | +| **H5** | test-cov | 🔵 | ✅ | Sonnet→🔍Opus | **S3** | +| **V5, V6** | input-val | 🔵 | ✅ | Sonnet→🔍Opus | **S3** | | **N2** | numerics | 🟡 | ⬜ | Haiku→🔍Opus | **S4** | | **thread-safety doc** | thread-safety | 🟡 | ⬜ | Haiku→🔍Opus | **S4** | | **M3** | math-cite | 🟡 | ⬜ | Haiku→🔍Opus | **S4** | @@ -126,14 +126,18 @@ CGAL result types (`Conformal_map_result`, `Hyper_ideal_map_result`, -### ⬜ S3 — Robustness & test-gap closure (Sonnet → 🔍 Opus) -- **H3** — `enforce_gauss_bonnet` reports the magnitude of the applied correction. -- **H4** — test `reduce_to_fundamental_domain` boundary `Im(τ)==0.0`. -- **H5** — integration test for the degenerate-triangle path in the Newton solve. -- **V5** — make the hand-rolled XML reader *reject* reformatted-but-valid XML - instead of silently mis-reading it into zeros. -- **V6** — DOF-vector-vs-mesh size check on load. -- **🔍 Opus review:** verify the new rejections don't break valid round-trips. +### ✅ S3 — Robustness & test-gap closure (DONE, 2026-06-01, Sonnet impl + Opus review) +Implementation shipped in commit `135bcf0` (included in P1 merge `bd613a6`). +Follow-up commit closes the doc-tracker gap and fixes dead `found_dofvector` variable +(V5 rule 4: `` missing now throws instead of silently returning empty `x`). +- **H3** — `enforce_gauss_bonnet` returns `|deficit|` (both overloads); 3 new tests. +- **H4** — `ReduceToFD_ThrowsForRealAxisBoundary` covers `Im(τ)==0.0` exact boundary. +- **H5** — 2 integration tests: sliver triangle (no crash/NaN) + exact-degenerate collinear. +- **V5** — `load_result_xml` rejects non-conforming XML (3 strict-subset checks); canonical + round-trip regression test passes. V5 rule 4 (`` must be present) now enforced. +- **V6** — `check_dof_vector_size(x, expected, context)` throws on mismatch; 3 new tests. +- **🔍 Opus review:** CHANGES-REQUESTED resolved — implementation correct; code commits + were redundant with `135bcf0` on main; doc follow-up applied on `chore/s3-followup`. ### ⬜ S4 — Documentation & citations (Haiku → 🔍 Opus) - **N2** — `doc/math/tolerances.md`: every numerical threshold, its role, and