RangedAggregateTransformer: bands is missing from _hash_message

[cboulay]

Summary

RangedAggregateTransformer._hash_message hashes only message metadata:

def _hash_message(self, message: AxisArray) -> int:
    axis = self.settings.axis or message.dims[0]
    target_axis = message.get_axis(axis)

    hash_components = (message.key,)
    if hasattr(target_axis, "data"):
        hash_components += (len(target_axis.data),)
    elif isinstance(target_axis, AxisArray.LinearAxis):
        hash_components += (target_axis.gain, target_axis.offset)
    return hash(hash_components)

But _reset_state derives self._state.slices and self._state.out_axis from self.settings.bands. Since bands is not in the hash, changing it at runtime without also changing the message metadata leaves the cached slices in place, and _process keeps aggregating the old bands while reporting the new ones in settings.

update_settings covers the common path — NONRESET_SETTINGS_FIELDS is empty on this class, so a bands change through it calls _request_reset(). The gap is direct assignment:

proc.settings = dataclasses.replace(proc.settings, bands=[(10.0, 20.0)])
# no reset queued; the next message is aggregated with the previous bands

Second case: the passthrough shortcut

def __call__(self, message: AxisArray) -> AxisArray:
    # Override for shortcut passthrough mode.
    if self.settings.bands is None:
        return message
    return super().__call__(message)

Setting bands = None returns before hashing, so _hash is frozen at its pre-passthrough value. Setting bands back to a different list than before resumes with the slices from the original list.

Unlike the EWMA/scaler case (see #195), this is not about accumulated signal state — slices, out_axis, and ax_vec are all derived, so there is no discontinuity to reason about, just a stale cache producing wrong output.

Suggested fix

Add bands to the hash:

hash_components = (message.key, self.settings.bands)

bands is a list[tuple[float, float]], so it needs tuple(...) (or None) to be hashable. Requesting a reset in the bands is None branch of __call__ would also close the passthrough case.

Test to add

Assert that assigning new bands directly (not via update_settings) changes the output, and that toggling bands to None and back to a different list does not reuse the first list's slices.

[cboulay]

Closing this — on a second look the premise doesn't hold up.

RangedAggregateTransformer never defines NONRESET_SETTINGS_FIELDS, so it inherits the empty frozenset() from BaseProcessor. Any bands change through update_settings therefore lands in changed - NONRESET_SETTINGS_FIELDS and calls _request_reset(). The issue notes this for the direct-assignment case but treats the passthrough shortcut as a second, separate hole — it isn't:

• update_settings(bands=None) sets _hash = -1
• passthrough messages return before _hash_message, so _hash stays -1
• update_settings(bands=[B]) sets _hash = -1 again
• the next real message resets and rebuilds the slices from [B]

So through the established interface there is no stale-slice bug, in either case.

The one reachable path is bare proc.settings = dataclasses.replace(...), which bypasses update_settings entirely. That's a caller mistake rather than a transformer bug, and defending against it would put a per-message tuple build (bands is a list[tuple[float, float]], and arrives as list[list[float]] from YAML) on _hash_message, which runs on every message. Not worth it for a case the supported API already covers.

Two parts of the proposed fix were worth keeping regardless, and are in #199:

• _request_reset() in the bands is None branch — free, and matches what BinnedAggregateTransformer already does.
• An __acall__ override, which turned out to matter for a different reason. The passthrough shortcut was only ever on __call__, but BaseTransformerUnit.on_signal awaits __acall__ — so RangedAggregate(bands=None) inside a graph skipped the shortcut, fell through to _reset_state, and raised TypeError: 'NoneType' object is not iterable. Every existing test passed because they all drive the transformer synchronously.

That last one is a real bug and is fixed with async coverage in #199.