We can't actually guarantee that Slab::insert will return a usize less than u32::MAX, right? Does the memory savings of u32 over u64 justify the silent truncation risk?

Also, what's the purpose of the debug_assert!? I don't see us testing such a scenario.

Debug asserts can be enabled in "release" builds too, they aren't necessarily tied to testing.
E.g. it's possible to produce a release build with debug assertions enabled to re-run particular scenarios and see what triggers.
In this specific case, it's better to make it a production assert, since the overhead is going to be negligible.

On one hand i dont think we need to crash the server in this case, because the user didnt do anything wrong
on the other hand, if handling it gracefully will add complexity to the code it doesn't worth it...
your call

is 0.0 the right value to return for leaf nodes when 0.0 is a valid split value for internal nodes?

Option is the right abstraction here, yes.

do we copy the entire inverted index of the range here?

Yes, we need to collect all its entries into a buffer.

dont we need to pass the range only if internal ranges are enabled?

At this stage, we keep it unconditionally, then it's trimmed in node_add based on the max_depth_range that was specified. I could also push the check further down, bringing max_depth_range into this context.

This is the actual default, right?
Consider using std::mem::take

Not sure if it matters much, but 0.0 is an arbitrary valid value.
Consider returning NaN, None, panic, or even extract the median instead

Option is the right abstraction here, yes.

makes rotations cheaper (index swaps instead of alloc/dealloc)

The C implementation doesn't require [de]allocations, so this is not a real improvement. The rest is true :)

I reworded it around pruning, which is when we see the difference (i.e. one realloc vs multiple deallocs).

Why do we need 2 * self.stats.num_leaves.saturating_sub(1) + 1? num_leaves should always be at least 1, no?

True, but we don't need this either, we know the exact node count thanks to the arena.

Why have this helper, and not simply define it at the NumericRange implementation of update_cardinality?

Later on, when have the GC code (#8293) we'll need to compute hashes of values outside the context of a NumericRange, and since f64 doesn't implement Hash, I used this helper as the source of truth for how we are hashing floats.
A different approach could be to make HyperLogLog strongly typed—generic over the type of values it's counting, and then create a thin wrapper around f64 which encodes the desired hashing behaviour.

Consider making node_add return an AddResult, unless it has performance panalties

It won't work unless we make node_add non-recursive. It needs to thread rv via function parameters so that each "layer" of the chain can update it in place.

I see. In practice, the child sets it inially and then it gets updated on the way up. Could be an idea for the future

Why do we collect the results, and not use the reader in the "Redistribute entries to children" for loop?

To avoid an annoying borrow-checker issue, but it can be worked around.