ConformalLabpp/doc/reviewer/cgal-submission-readiness-audit-2026-05-31.md

# CGAL Submission-Readiness Audit — ConformalLabpp

**Date:** 2026-05-31
**Auditor:** External reviewer (Claude Opus 4.8)
**Scope:** Readiness of the `code/include/CGAL/` package for submission to the
CGAL project (editorial-board review + integration into the CGAL release).
**Reference:** CGAL Developer Manual — "Adding a new package", CGAL coding
conventions, and the CGAL package directory structure.

This document is self-contained. A new session can pick up any finding below and
act on it without prior context.

Status legend: ⛔ Blocker (submission rejected on sight) · 🔴 Critical · 🟡 Important · 🔵 Polish

> **🔄 Status (2026-05-31):** the whole package — **G0** (porting rights), and
> everything it gates (**G1–G12**) — is **blocked pending the original authors'
> reply**. The authors have been **contacted by email** asking to clarify
> porting/relicensing rights. No layout/license/header work proceeds until a
> written answer exists. Scheduled as **S6** (Opus, after G0/G1) in
> [`finding-orchestration.md`](finding-orchestration.md).

> **Companion documents:**
> - `external-audit-2026-05-30.md` — port-faithfulness bugs
> - `test-coverage-error-handling-audit-2026-05-31.md` — test gaps + error handling
> - `api-performance-audit-2026-05-31.md` — API consistency + performance
> - **this file** — CGAL submission readiness
>
> **Bottom line:** the *algorithmic* maturity is strong, but the package is **not
> submittable as-is**. There are **three** hard blockers — **provenance/porting
> rights (G0)**, licensing (G1), and directory layout (G2) — plus several
> CGAL-convention gaps. G0 is a legal precondition that gates everything else:
> until the right to port the original Varylab/TU-Berlin code is clarified with
> its authors, no license decision (G1) can be made and no public release or
> submission should happen. The remaining items are packaging and
> documentation-format work — substantial but mechanical, no research required.

---

## Verdict at a glance

| # | Sev | Title |
|---|-----|-------|
| G0 | ⛔ | **Porting rights unclear** — the original Varylab/TU-Berlin project has *no license* (all rights reserved); the C++ code is a documented *port* (derivative work). Must clarify with the authors whether porting + (re)licensing is permitted **before** any license/release decision |
| G1 | ⛔ | **License is MIT** — CGAL requires GPLv3+ (algorithmic) or LGPLv3+. Moot until G0 is resolved |
| G2 | ⛔ | **Directory layout is not CGAL package layout** — no `package_info/`, no `doc/<Pkg>/`, tests/examples not in CGAL harness |
| G3 | 🔴 | **CGAL is vendored** (`code/deps/CGAL-6.1.1`) — a CGAL package must not bundle CGAL |
| G4 | 🔴 | **Concept not in a Doxygen block** — `\cgalConcept` is in `//` comments, won't render; no `doc/<Pkg>/Concepts/` file |
| G5 | 🔴 | **No user manual** — CGAL requires `<Pkg>.txt` (\mainpage) + `PackageDescription.txt` |
| G6 | 🔴 | **Test suite uses GoogleTest + FetchContent** — CGAL uses its own test harness; external network deps are disallowed |
| G7 | 🟡 | **Primary template `Default_conformal_map_traits` is undefined** — only `Surface_mesh` works; concept claims `FaceGraph` genericity |
| G8 | 🟡 | **`.hpp` extension for implementation headers** — CGAL convention is `.h` |
| G9 | 🟡 | **Single-header monolith risk** — heavy `inline` free functions in headers; CGAL prefers `internal/` split + documented public surface |
| G10 | 🟡 | **No demo, no benchmark dir** — recommended for a numeric package |
| G11 | 🔵 | **Naming inconsistencies** carry into the public API (see api-performance-audit A1–A5) |
| G12 | 🔵 | **Copyright line** — CGAL files carry an `INRIA/GeometryFactory` style header + the CGAL license boilerplate, not a bare MIT line |

---

## G0 — ⛔ Porting rights: the original is unlicensed and this is a derivative work

> **This is the precondition for everything else in this audit (and for the license
> finding G1). It must be resolved with the original authors before any public
> release, relicensing, or CGAL submission.**
>
> *Disclaimer: the auditor is not a lawyer. This is a practical risk assessment,
> not legal advice. Confirm with the project owner / institutional legal counsel.*

### Evidence
The original project (`gitlab.discretization.de:varylab/conformallab.git`,
mirrored to `git.eulernest.eu`) is the **Varylab / TU-Berlin** discrete-conformal
codebase (DFG SFB/Transregio "Discretization in Geometry and Dynamics"; author
circle around Stefan Sechelmann, Boris Springborn).

```
$ find /Users/tarikmoussa/Desktop/conformallab -maxdepth 2 \
       \( -iname 'license*' -o -iname 'copying*' \)
# → (nothing)
# Java sources carry NO license header; only build.xml + .project exist.
```

- The original has **no LICENSE file and no per-file license headers** → under
  default copyright law this means **all rights reserved** by the TU-Berlin/Varylab
  authors. "Publicly visible on an academic GitLab" is **not** a license grant.
- `ConformalLabpp` describes itself as a **port**: source comments say
  *"Ported from de.varylab.discreteconformal.functional.EuclideanCyclicFunctional"*,
  and the test suite asserts "Java golden-oracle parity". That makes the C++ code a
  **derivative work of the original source code**, not merely an independent
  implementation of the published papers.

### Why it blocks
A derivative work of all-rights-reserved code **cannot be licensed by the porter
alone** — neither as MIT (the current state) nor as GPL (the CGAL requirement) —
without permission from the original copyright holders. The existing MIT header on
`ConformalLabpp` is therefore on shaky ground today, independent of CGAL.

Crucial distinction:

| What was actually reproduced | Legal status |
|---|---|
| The **published algorithms** (Springborn 2020, Bobenko-Pinkall-Springborn 2010, Luo 2004) | Free to reimplement — mathematical methods are not copyrightable, only their concrete code expression |
| The **source code** of the Varylab Java project, line-by-line | Derivative work → requires the authors' permission to port and (re)license |

The "Ported from …" comments currently document a code derivation in writing, which
is the principal risk.

### Action (owner-level, before any code/license change)
1. **Determine provenance honestly:** is `ConformalLabpp` a clean-room
   reimplementation *from the papers*, or a translation *of the Varylab code*?
   The current comments assert the latter.
2. **Contact the original authors** (Sechelmann / Springborn, TU Berlin / Varylab)
   to either (a) obtain written permission to port and relicense (ideally under
   GPLv3+ so CGAL submission stays open), or (b) confirm the original is intended to
   be freely reusable and get that in writing / as an added LICENSE upstream.
3. **Pause public releases and CGAL submission** until 1–2 are settled.
4. If permission cannot be obtained, the only clean path is a genuine clean-room
   reimplementation from the published papers, with the "Ported from …" provenance
   comments removed and replaced by paper citations.

### Done when
- Written clarity exists on the right to port + (re)license the original code;
  the provenance comments in the source match that reality.

---

## G1 — ⛔ License: MIT is incompatible with CGAL submission

> Blocked by **G0** — do not act on G1 until porting/relicensing rights are clear.

### Evidence
All 41 headers carry:
```cpp
// SPDX-License-Identifier: MIT
```
(`grep -rh "SPDX-License-Identifier" code/include/` → 41× MIT, 0× anything else.)

### Why it blocks
CGAL is dual-licensed: the **algorithmic** packages are **GPLv3+**, the foundational
ones **LGPLv3+**, with a commercial license sold by GeometryFactory. A new package
must be contributed under the **same dual GPL/LGPL scheme**, with copyright assigned
or licensed appropriately. A package licensed MIT cannot be merged into the CGAL
tree — the editorial board will reject it at intake.

### Action
This is a **licensing/legal decision for the project owner**, not a code change:
1. Decide whether to relicense the contributed package as GPLv3+ (the normal choice
   for a new algorithmic CGAL package).
2. Replace every header's `SPDX-License-Identifier: MIT` with the CGAL license
   header boilerplate (see `Installation/LICENSE*` in the CGAL tree for the exact
   text and the `$URL$ $Id$ SPDX-License-Identifier: GPL-3.0-or-later` form).
3. Keep a separate MIT-licensed standalone build if a permissive standalone
   distribution is also desired (dual-distribution is possible but must be explicit).

> ⚠️ Do not auto-rewrite license headers without the owner's explicit decision —
> relicensing is irreversible for contributed copies and may involve other authors.

### Done when
- Owner has chosen the license; headers carry the CGAL-conformant boilerplate.

---

## G2 — ⛔ Directory layout is not a CGAL package

### Evidence
Current layout:
```
code/include/CGAL/...        ← headers (good location)
code/examples/*.cpp          ← flat, not per-package
code/tests/cgal/*.cpp        ← GoogleTest, not CGAL harness
code/deps/CGAL-6.1.1         ← vendored CGAL (see G3)
(no package_info/ anywhere)
(no doc/<Pkg>/ with PackageDescription.txt)
```

CGAL expects a package rooted at a top-level package directory mirroring the CGAL
source tree:
```
Discrete_conformal_map/
  include/CGAL/Discrete_conformal_map.h
  include/CGAL/Conformal_map/...
  doc/Discrete_conformal_map/
      Discrete_conformal_map.txt          (user manual, \mainpage)
      PackageDescription.txt              (\package_listing entries)
      Concepts/ConformalMapTraits.h       (the concept, doxygen-only)
      examples.txt, fig/
  examples/Discrete_conformal_map/*.cpp   (+ CMakeLists using CGAL macros)
  test/Discrete_conformal_map/*.cpp       (CGAL test harness)
  benchmark/Discrete_conformal_map/       (optional but recommended)
  package_info/Discrete_conformal_map/
      copyright  description.txt  dependencies  license.txt  maintainer
```

### Action
Restructure into the CGAL package layout. This is mechanical but touches every file
location and the build. Recommended approach:
1. Create the `package_info/Discrete_conformal_map/` metadata files first (small).
2. Move headers under the package's `include/CGAL/` (already close).
3. Re-home examples → `examples/Discrete_conformal_map/`, tests → `test/Discrete_conformal_map/`.
4. Build the `doc/Discrete_conformal_map/` tree (G5).

### Done when
- Layout matches an existing CGAL package (use e.g. `Surface_mesh_parameterization`
  or `Heat_method_3` as a template — both are close in spirit).

---

## G3 — 🔴 CGAL is vendored inside the repo

### Evidence
```
code/deps/CGAL-6.1.1     ← full CGAL 6.1.1 source tree bundled
```

### Why it matters
A package being submitted *into* CGAL must build against the surrounding CGAL tree,
not bundle its own copy. The vendored `CGAL-6.1.1` (and the standalone build wiring
around it) is appropriate for the current standalone distribution, but must be
removed from the submitted package — the package's `dependencies` file declares CGAL
package dependencies instead.

### Action
- For the CGAL-submission branch: drop `code/deps/CGAL-*`; rely on the host CGAL tree.
- Keep the vendored copy only on the standalone-distribution branch.
- List actual dependencies (`Surface_mesh`, `BGL`, `Property_map`, `Solver_interface`,
  `Number_types`, plus the external `Eigen`) in `package_info/.../dependencies`.

### Done when
- The submission branch contains no bundled CGAL; it builds inside a CGAL checkout.

---

## G4 — 🔴 The concept is not a documented CGAL concept

### Evidence
`code/include/CGAL/Conformal_map_traits.h:36-76` declares the concept with
`\cgalConcept`, `\concept ConformalMapTraits`, `\cgalHasModelsBegin/End` — but the
whole block uses **plain `//` comments**, not a Doxygen `/*! ... */` block:

```cpp
// ════════════════════════════════════════════════════════════════════════════
// \cgalConcept
//
// \concept ConformalMapTraits
...
```

Doxygen only parses `/*! */` (or `///`) blocks, so none of this renders. There is
also no `doc/<Pkg>/Concepts/ConformalMapTraits.h` — in CGAL, concepts live as
doxygen-only header files under the package `doc/.../Concepts/` directory, separate
from the model.

### Action
1. Create `doc/Discrete_conformal_map/Concepts/ConformalMapTraits.h` containing the
   concept as a `/*! \cgalConcept ... */`-documented (empty) class, following the
   CGAL concept-file pattern.
2. Reference it via `\cgalHasModels` from `Default_conformal_map_traits`.
3. Convert the in-source `//` concept block to either a proper `/*! */` block or
   remove it in favor of the doc/Concepts file.

### Done when
- `ConformalMapTraits` appears in the generated reference manual under
  `PkgConformalMapConcepts` with its model linked.

---

## G5 — 🔴 No CGAL user manual / PackageDescription

### Evidence
`find . -name PackageDescription.txt -o -path '*doc*' -name '*.txt'` → none (outside `deps/`).
The project has rich Markdown docs under `doc/` (api/, math/, architecture/), but
CGAL requires its own doxygen-driven manual files:
- `doc/<Pkg>/<Pkg>.txt` — the user manual with `\mainpage`-style narrative,
  `\cgalExample` references, figures.
- `doc/<Pkg>/PackageDescription.txt` — the `\cgalPkgDescriptionBegin` listing
  (authors, intro, license, dependencies, demo/example links) that populates the
  CGAL "Packages" overview.

### Action
- Author `Discrete_conformal_map.txt` (user manual). Much of the content can be
  adapted from the existing `doc/api/pipeline.md` and `doc/math/*.md`.
- Author `PackageDescription.txt` with the standard CGAL macros.
- Wire both into a package `doc/Doxyfile.in` consistent with CGAL's doc build.

### Done when
- `cgal_create_package` doc build produces a reference + user manual without warnings.

---

## G6 — 🔴 Test suite is GoogleTest + FetchContent

### Evidence
`code/tests/cgal/CMakeLists.txt`:
```cmake
target_link_libraries(conformallab_cgal_tests PRIVATE GTest::gtest_main)
gtest_discover_tests(conformallab_cgal_tests ...)
```
(plus FetchContent for GoogleTest elsewhere in the build).

### Why it matters
CGAL's test infrastructure uses its own harness (`create_single_source_cgal_program`,
the `test/` programs return 0/non-0, driven by the CGAL testsuite scripts). External
network dependencies (FetchContent pulling GoogleTest) are not allowed in the CGAL
tree, and the GoogleTest dependency would need vendoring or removal.

### Action
This is the largest mechanical task. Options:
- **(a)** Port the assertions to CGAL's plain-`assert` + `return EXIT_FAILURE` test
  style, one program per test file under `test/Discrete_conformal_map/`. This is the
  canonical CGAL approach and removes the GoogleTest dependency entirely.
- **(b)** Keep GoogleTest for the standalone branch; maintain a parallel CGAL-style
  `test/` tree for the submission branch.

The 272 existing tests are an asset — the assertions translate directly; only the
framework scaffolding changes.

### Done when
- `test/Discrete_conformal_map/` runs under the CGAL testsuite with no GoogleTest.

---

## G7 — 🟡 Genericity claimed but not delivered

### Evidence
`Conformal_map_traits.h:80-92`: the primary template
`Default_conformal_map_traits<TriangleMesh, Kernel_>` is **declared but undefined**;
only the `Surface_mesh` partial specialization (`:113`) is implemented. The concept
docstring (`:13-21`) advertises *"can run on any CGAL halfedge mesh — Surface_mesh,
Polyhedron_3, OpenMesh-adapter, pmp — without changes"*, and the API takes a
`TriangleMesh` template parameter, implying BGL-generic support.

### Why it matters
CGAL reviewers scrutinize generic-programming claims. Shipping a `TriangleMesh`
template that compiles only for `Surface_mesh` is a documentation/implementation
mismatch: a user passing a `Polyhedron_3` gets a hard template error against an
undefined primary template, not a clean diagnostic.

### Action
Either:
- **(a)** Implement a generic `FaceGraph`/`HalfedgeGraph` path using `boost::graph_traits`
  + dynamic property maps (CGAL's `dynamic_vertex_property_t`), removing the
  `Surface_mesh`-only restriction — this is the "Phase 8a.2" already on the roadmap; or
- **(b)** Until then, narrow the documented contract: state explicitly that only
  `Surface_mesh` is supported, and `static_assert` a friendly message in the primary
  template instead of leaving it undefined.

### Done when
- The advertised mesh-type support matches what compiles; unsupported types yield a
  `static_assert` message, not a raw template error.

---

## G8 — 🟡 Header extension `.hpp` vs CGAL `.h`

### Evidence
The implementation headers use `.hpp` (`euclidean_functional.hpp`, `newton_solver.hpp`,
…), while the CGAL-facing headers correctly use `.h` (`Discrete_conformal_map.h`).
CGAL's convention is `.h` for all headers.

### Action
Rename the implementation headers to `.h` when they move under the package
`include/CGAL/Conformal_map/internal/` tree (G9), updating `#include` directives.
Low-risk, mechanical; do it as part of the G2 restructure.

### Done when
- All package headers use `.h`.

---

## G9 — 🟡 Public/internal header surface is undivided

### Evidence
The 26 `code/include/*.hpp` headers are all effectively public (flat in `include/`),
mixing the user-facing CGAL entry points with low-level functional/geometry/solver
internals (`hyper_ideal_geometry.hpp`, `projective_math.hpp`, `clausen.hpp`, …).

### Why it matters
CGAL packages keep a small documented public API and move implementation into
`include/CGAL/<Pkg>/internal/`. Exposing every helper as a top-level header makes
the supported API surface ambiguous and the package harder to evolve without
breaking users.

### Action
- Designate the public surface (the `CGAL/Discrete_*` entry points, the traits, the
  layout/result types).
- Move everything else under `include/CGAL/Conformal_map/internal/`.
- Mark internal headers `\internal` for Doxygen (the parameters header already does
  this correctly — `Conformal_map/internal/parameters.h` is the right pattern).

### Done when
- Only the intended public headers are documented; the rest are under `internal/`.

---

## G10 — 🟡 No demo / benchmark directory

### Evidence
No `demo/` and no `benchmark/` directory. For a numerical package, CGAL reviewers
appreciate (and for some packages expect) a benchmark establishing performance
characteristics, and a small demo is a strong plus.

### Action
- Add `benchmark/Discrete_conformal_map/` measuring solve time / iteration counts on
  the existing smoke meshes (cathead/brezel/brezel2) — this also dovetails with the
  performance findings in `api-performance-audit-2026-05-31.md` (B1/B4).
- A demo is optional; skip if time-constrained.

### Done when
- A benchmark program exists and its numbers are referenced in the user manual.

---

## G11 — 🔵 Public-API naming inconsistencies (cross-ref)

The naming inconsistencies documented in `api-performance-audit-2026-05-31.md`
(findings A1–A5: `assign_*`, `compute_*`, `gradient_check`, the `_map` suffix, and
the split result types) become **public-API-stability concerns** once the package is
in CGAL, because CGAL APIs are expected to be stable across releases. Resolve A1–A5
**before** submission, not after — renaming a shipped CGAL API requires a deprecation
cycle.

### Done when
- A1–A5 resolved; the public names are the ones intended to be permanent.

---

## G12 — 🔵 File header boilerplate

### Evidence
Headers begin with `// Copyright (c) 2024-2026 Tarik Moussa.` + a bare SPDX line.
CGAL files use a standardized header including the `$URL$`/`$Id$` SVN/git keywords,
the SPDX line in CGAL's form, and the copyright holder convention used in the tree.

### Action
Adopt the CGAL file-header template (tied to the G1 license decision). Mechanical
once G1 is settled.

### Done when
- Headers match the CGAL file-header template.

---

## What is already strong (genuine assets for submission)

- **Named-parameter API** is idiomatic: `CGAL::parameters::gradient_tolerance(...)
  .max_iterations(...)`, with the tag machinery correctly placed in
  `Conformal_map/internal/parameters.h` and marked `\internal`. This is the hard part
  done right.
- **Doxygen group hierarchy** (`PkgConformalMap` / `Ref` / `Concepts` /
  `NamedParameters`) is set up correctly in `doxygen_groups.h`.
- **The `\internal` discipline** on the parameters header shows the author knows the
  public/internal distinction — it just needs to be applied to the other helpers (G9).
- **Result types carry observability** (`sparse_qr_fallback_used`, iteration counts).
- **Algorithmic breadth + validation**: five DCE models with Java golden-oracle
  parity and FD-gradient checks — a strong scientific basis, which is ultimately what
  the editorial board cares most about.
- **Traits-based design intent** is correct CGAL architecture; it only needs the
  genericity actually implemented (G7) and the concept properly documented (G4).

---

## Suggested order of work (submission track)

0. **G0 (porting rights)** — owner-level legal precondition; contact the
   Varylab/TU-Berlin authors. **Nothing below should ship publicly until this is
   resolved.**
1. **G1 (license decision)** — follows directly from G0; pick GPLv3+ if submission stays the goal.
2. **G2 + G3 + G8 (restructure)** — do the layout move and header rename together;
   drop vendored CGAL on the submission branch.
3. **G9 (public/internal split)** — naturally part of the restructure.
4. **G11 / A1–A5 (lock the public names)** — before any docs are written against them.
   *(A1–A3 are internal-API renames and are safe to do independently of G0; A4–A5
   touch the public CGAL surface and should wait for the G0/G1 outcome.)*
5. **G4 + G5 (concept file + user manual + PackageDescription)** — the doc deliverables.
6. **G6 (CGAL test harness)** — port the 272 tests off GoogleTest.
7. **G7 (deliver or narrow genericity)**.
8. **G10 (benchmark)** + **G12 (headers)** — polish.

> Realistic framing for the owner: the **science is submission-grade**; the gap is
> **packaging + licensing + doc-format**. None of items G2–G12 require new research,
> but collectively they are a significant, mostly-mechanical effort. G1 is a decision,
> not work.