Fable review of Boost.Histogram

I did a sweep with Fable for issues. It independency found two existing issues that already have pending fixes, along with some others. My intent is to slowly fix these a few at a time, unless you'd rather something else. Is there anything you'd like me to prioritize or skip?

:robot: _AI text below_ :robot:

All verification is done. Here's the consolidated review.

# Boost.Histogram project review

Seven parallel review agents swept the codebase (core, axes, detail, accumulators, algorithms/fill, performance, modernization); I verified every finding below either by reproduction (ASan/UBSan/compiled repros, sources in the session scratchpad) or by reading the code myself. Two rediscovered known issues are excluded: #426 (broadcast-scalar fill wipes a chunk when a non-inclusive axis drops the first entry — found independently by two agents) and #430 (rank-0 `indexed()`).

## Bugs — memory safety (all reproduced under ASan/UBSan)

1. **`multi_index.hpp:94` — stack-buffer overflow in dynamic-rank `multi_index`.** The static-size specialization (line 50) throws `std::invalid_argument` on a size mismatch; the dynamic specialization copies into `data_[32]` with no check. `h.at(std::vector(40, 0))` on a dynamic-axes histogram overflows the stack (ASan-confirmed) instead of throwing.

2. **`detail/reduce_command.hpp:50` — stack-buffer overflow in `reduce` with too many positional commands.** The bounds check only guards commands with an explicit axis index. `reduce(h, rebin(2), rebin(2))` on a rank-1 tuple-axes histogram writes past the stack command buffer (ASan-confirmed); the explicit form `rebin(1, 2)` correctly throws.

3. **`storage_adaptor.hpp:85` — buffer overflow in `array_impl` converting constructor.** `operator=` and `reset` both throw `std::length_error` when the source exceeds `max_size()`; the converting constructor doesn't check. Reachable purely through the public API by converting a histogram with more cells than the target `std::array` storage holds (ASan-confirmed).

## Bugs — wrong results (reproduced)

4. #438 **`axis/variable.hpp:214` — `std::modf` outputs swapped in circular `value()`.** `modf` *returns* the fraction and stores the integral part in `z`, but the code uses the return value as bin index `k` and `z` as interpolation weight. Every fractional index and every integer index ≥ 2 on a non-equidistant circular variable axis returns a wrong edge/center (e.g. size-3 axis `{0,1,3,4}`: `value(2)` returns 2, should be 3). The existing test only uses a size-2 axis where the error cancels. Related to but distinct from #380.

5. **`axis/regular.hpp:312` — circular axis puts finite values in the overflow bin.** For a value an ulp below a period boundary, `z -= std::floor(z)` rounds to exactly 1.0 and the index becomes `size()`. `axis::circular<>{4, 0, 1}.index(-1e-17)` returns 4; the count silently lands in overflow (or is dropped without the overflow bin).

6. **`unlimited_storage.hpp:347` — cell self-assignment zeroes the value.** `reference::operator=` zeroes the destination before reading through the visited source pointer, so `h.at(0) = h.at(0)` sets the cell to 0. The comment claims self-assignment safety but only covers reallocation, not aliasing.

7. **Float→int conversion UB on extreme finite inputs (UBSan-confirmed):** `axis/regular.hpp:340,351` (`update()` casts `1e300 * size()` to `int`), `axis/integer.hpp:109,115` (`x - min_` signed overflow for `index(INT_MAX)` with negative `min_`; also misbins as underflow on wrap), `axis/integer.hpp:133` (cast of large finite double to `long` in `update()`). All reachable from a plain fill on growth axes; the project's own `histogram_ubasan` CI variant would flag these given a test input.

8. **`utility/binomial_proportion_interval.hpp:123` — `std::terminate` at ≥ ~8.3σ.** The `noexcept` conversion `deviation → confidence_level` computes a cl that rounds to exactly 1.0, and the `confidence_level` ctor throws inside `noexcept` → terminate (reproduced). Same pattern in the `noexcept` `operator*` with a negative factor.

## Bugs — lower severity

9. **`ostream.hpp:84`** — `tabular_ostream_wrapper` swaps in its own `rdbuf` and restores it only in `complete()`; a throwing user `operator<<` leaves the caller's stream pointing at a destroyed streambuf (use-after-scope on the next write). Needs a restoring destructor/RAII guard.
10. **`storage_adaptor.hpp:195`** — map-backed `operator*=` skips absent cells, so `h *= NaN`/`inf` leaves untouched cells at 0 while dense storage yields NaN everywhere; the sibling `operator/=` handles exactly this case.
11. **`detail/accumulator_traits.hpp:59-65`** — two overloads at the same `priority<2>` are ambiguous for an accumulator with both `operator()` and `operator+=(weight_type)`; hard compile error (confirmed by compilation).
12. **`detail/chunk_vector.hpp:79-83`** — `insert()` has an inverted size check (throws on the *valid* multiple-of-chunk case) and doesn't even compile when reached via `collector::operator+=`; experimental type, only test coverage never calls it.
13. **`detail/term_info.hpp:80`** — `width()` returns `min(col, w)` when the ioctl succeeded but `COLUMNS` is unset (`col == 0`), discarding the detected width → garbled terminal plots below 78 columns.
14. **`detail/static_vector.hpp:54,59`** — `at()` is `noexcept` yet throws `std::out_of_range` → terminate; currently latent (no out-of-range caller).
15. **`detail/priority.hpp:10`** — uses `std::size_t` with only `` included; fails standalone on libc++ (public headers unaffected).

## Performance

- #439 **`algorithm/reduce.hpp:421-425`** — the per-cell loop recomputes the loop-invariant `(end - begin) / merge` division for every axis of every cell and divides by `merge` even when it's 1; hoisting into the precomputed `opts` buffer removes O(ncells × rank) integer divisions. Verified in code; the clearest win.
- **`histogram.hpp:453` (and `-=`, `*=`, `/=` siblings)** — histogram-histogram arithmetic with `unlimited_storage` does two buffer-type dispatches per cell; a storage-level visit-once loop (like the existing scalar `*=` path at line 562) would make the common merge-after-parallel-fill pattern vectorizable.
- **`algorithm/sum.hpp:55`** — same per-cell dispatch through `const_reference::operator double()`; a single typed visit would remove it.
- (Henry: tested this, not measurable) ~~**`detail/fill_n.hpp:139`** — `std::vector>` histograms always take the growth path if the variant merely *lists* a growing axis type (acknowledged by the TODO there); a cheap runtime "does any held axis actually grow" check like the existing `all_inclusive` check would skip the per-element shift bookkeeping.~~
- **`indexed.hpp:296`** — dynamic-axes iterators embed a 32-entry `index_data` array (~1 KB each); iterating many small histograms pays multi-KB copies per `indexed()` call.

## Simplification / modernization (all grep-verified)

Dead code in `detail/detect.hpp`: the `void_t` alias (line 31), `detect_base::val()`, `has_method_push_back` (69), `requires_storage` (184), `requires_sequence_of_axis` / `requires_sequence_of_axis_variant` (204-208) are unreferenced; `is_indexable` (71) and `is_complete` (129) are used only by their own unit test. The `ignore_deprecation_warning_{begin,end}.hpp` pair is included only by the ODR test with no deprecation markers anywhere to suppress, and `detail/debug.hpp` has zero includes (likely intentional dev tool). Optional: the detect macros could collapse onto `mp11::mp_valid`, and `option.hpp`'s `(B + 0)` gcc-6 workaround is obsolete for every CI-tested compiler — though the README still claims gcc ≥ 5.5 support, so that one is a policy call.

The rejected-candidates lists (things examined and cleared — Neumaier/Welford math, `large_int` arithmetic, `safe_comparison`, interval formulas against Brown/Cai/DasGupta, linearize/growth paths, etc.) and all repro programs are in the scratchpad if you want to look. `/henry-generic-review-followup` can pick these up; I'd start with the three ASan overflows (items 1–3) and the two circular-axis result bugs (4–5), each of which needs a regression test first per your workflow.

Choose one finding from the review before starting; the named entry points include multi_index.hpp, detail/reduce_command.hpp, storage_adaptor.hpp, axis/variable.hpp, and axis/regular.hpp. Begin by reproducing the selected case with the relevant ASan/UBSan command or existing tests, then add a regression test. Done means the reproduction no longer fails and the focused test passes.

Written by the indexing model from the issue text.