@MridulS asked Claude for a code review and here is the result.
We should check each point to see if it is valid and if so, fix it.

Scipp Codebase Deep Audit Report

Date: 2026-04-09
Scope: ~411 C++ files, ~91 Python source files, ~93 test files
Areas: Memory management, edge cases/UB, Python security, pybind11 bindings, concurrency, build hygiene

Executive Summary

The codebase is generally well-engineered with strong RAII discipline, no raw new/delete in production, and good defensive patterns (cycle detection, iterator invalidation guards). However, several significant issues were found.

Totals: 3 Critical, 10 High, 16 Medium, 20+ Low

CRITICAL (3)

• C1. atan variance formula is mathematically wrong (Incorrect error propagation in atan #3876)
• C2. sqrt variance propagation divides by zero when value=0 (sqrt variance propagation divides by zero when value=0 #3878)
• C3. SubbinSizes::exclusive_scan out-of-bounds read with negative index (SubbinSizes::exclusive_scan out-of-bounds read with negative index #3879)

HIGH (10)

• H5. ElementArrayView.__getitem__ -- dangling reference risk (ElementArrayView lifetime errors in Python #3894)
• H6. ElementArrayView.__iter__ -- missing keep_alive (ElementArrayView lifetime errors in Python #3894)

H1. asin/acos variance divide-by-zero at domain boundary

lib/core/include/scipp/core/value_and_variance.h:132

At |x| == 1, arc_trig_variance divides by 1 - 1 = 0, producing inf variance. The NaN guard doesn't catch this since asin(1) = pi/2 is valid.

H2. Negative exponent integer overflow in numeric::pow

lib/common/include/scipp/common/numeric.h:106

-exponent is UB when exponent == INT64_MIN. No guard exists at the C++ level.

H3. floor_divide silently returns 0 for integer div-by-zero

lib/common/include/scipp/common/numeric.h:67

b == 0 ? 0 : ... masks a mathematical error. Python raises ZeroDivisionError for this.

H4. Rebin division by zero with zero-width bins

lib/core/include/scipp/core/element/rebin.h:49

delta / owidth where owidth=0 for duplicate bin edges.

H7. HDF5 deserialization: no size validation on shape from file

src/scipp/io/hdf5.py:314-327

shape from untrusted HDF5 files is passed directly to empty(). A crafted file could trigger OOM via extreme shapes like [2^30, 2^30].

H8. Phantom histogram in __all__, deleted reduction still listed

src/scipp/__init__.py:350,403

histogram is exported but never defined. reduction is deleted at line 177 but still in __all__. Both break from scipp import * introspection.

H9. psutil is an unused mandatory dependency

requirements/base.in:2

Listed as a core dependency, never imported anywhere in the codebase.

H10. CI supply-chain: most GitHub Actions pinned by moving tag, not SHA

.github/workflows/

Actions like actions/checkout@v4 can be force-pushed upstream. Only 2 of ~15 third-party actions are SHA-pinned. No top-level permissions: {} on most workflows.

MEDIUM (16)

• M1. Dim registry TOCTOU race (Dim registry TOCTOU race #3881)
• ~ M8. time_point default constructor leaves m_duration uninitialized (time_point default constructor leaves m_duration uninitialized #3883)~ Won't do, this would break empty.

M2. Unit aliases not thread-safe

lib/units/unit.cpp:227-231

add_unit_alias/clear_unit_aliases modify global state with no synchronization (documented but hazardous).

JLW: This is correct but difficult to fix. We don't control reading from the user-defined units directly. So to fix this, we would have to put a read lock around all unit creation. Not sure that is worth the added cost.

M3. VariableFactory global singleton has no mutation guard

lib/variable/variable_factory.cpp:89

Mutable singleton with no enforcement preventing runtime registration after init.

JLW: Technically correct, but the factory is only modified during static initialisation in C++. So there is no actual bug here.

M4. PyObject move-assignment may decrement refcount without GIL

lib/python/py_object.h:24

Defaulted move-assignment replaces an existing py::object, decrementing its refcount without acquiring the GIL.

M5. py::mod_gil_not_used() -- free-threaded Python risk

lib/python/scipp.cpp:77

Many binding functions lack explicit GIL acquisition, problematic for free-threaded Python (PEP 703).

M6. reinterpret_cast of function pointer from Python integer

lib/python/transform.cpp:17

No validation that the address points to a valid function with the expected signature.

M7. operator/ variance with zero denominator

lib/core/include/scipp/core/value_and_variance.h:276

b.value == 0 produces inf/NaN in variance with no guard.

M9. volume() and stride computation can overflow int64_t

lib/core/include/scipp/core/dimensions.h:59, lib/core/strides.cpp:20

Signed integer overflow is UB in C++. No overflow check exists for large dimension products.

M10. isclose integer subtraction overflow

lib/core/include/scipp/core/element/comparison.h:40

x - y can overflow for extreme int64_t values. abs(INT64_MIN) is also UB.

M11. XSS in HTML table rendering

src/scipp/visualization/table.py:35

Variable names and units interpolated into HTML without escaping. formatting_html.py escapes consistently; table.py does not.

M12. getattr(DType, d[key]) from untrusted dict

src/scipp/compat/dict.py:166

Should use an allowlist of valid dtype names instead of unrestricted getattr.

M13. subspan_view creates spans with no lifetime tie

lib/variable/subspan_view.cpp:18-33

Returned Variable contains std::span<T> pointing into source data. No ownership link in the C++ API.

M14. Dict::end() const passes wrong value iterator

lib/core/include/scipp/core/dict.h:370

Passes m_values.cbegin() instead of cend(). Works by accident since end iterators are never dereferenced.

M15. CMake minimum version inconsistency

CMakeLists.txt requires 3.28, CMakePresets.json claims 3.16, pyproject.toml says 3.21.

M16. Numpy constraint mismatch

pyproject.toml says >= 2 (open-ended), requirements/base.in says >=2,<2.4 (bounded).

LOW (selected highlights)