Fix critical bugs: NumPy 1.24+ compatibility, division by zero, and alert detection

[Copilot]

Code review identified 5 bugs causing runtime failures, incorrect behavior, and version incompatibility.

Changes

NumPy 1.24+ compatibility (core/features.py)

• Replace np.math.factorial() (removed in NumPy 1.24) with math.factorial()

Division by zero guards (core/features.py)

• Spectral centroid: return 0.0 when PSD sum is zero instead of NaN
• Permutation entropy: validate signal length before embedding (return 0.0 with warning for insufficient data)

Numerical stability (core/preprocessing.py)

• Increase epsilon from 1e-10 to 1e-6 in quality check normalized std calculation

Alert detection fix (core/gate.py)

• Variable mutation bug: offset_idx reassignment in zip loop didn't propagate
• Use separate end_idx variable to correctly track adjusted alert window bounds

# Before: offset_idx mutation lost
for onset_idx, offset_idx in zip(onset_indices, offset_indices):
    if gate_series[offset_idx] == 0 and offset_idx > onset_idx:
        offset_idx -= 1  # Local reassignment, doesn't affect subsequent usage
    duration = timestamps[offset_idx] - timestamps[onset_idx]  # Uses original value

# After: explicit tracking variable
for onset_idx, offset_idx in zip(onset_indices, offset_indices):
    end_idx = offset_idx
    if gate_series[end_idx] == 0 and end_idx > onset_idx:
        end_idx -= 1
    duration = timestamps[end_idx] - timestamps[onset_idx]  # Uses corrected value

All 101 tests passing. Zero security vulnerabilities (CodeQL).

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

This PR fixes several edge-case/runtime issues in the signal feature extraction and alerting pipeline, including NumPy 1.24+ compatibility and guards against invalid numerical results.

Changes:

• Update permutation_entropy normalization to use math.factorial (NumPy 1.24+ compatibility) and add a “too short signal” guard.
• Prevent division-by-zero/NaN in compute_spectral_centroid when the PSD sum is zero.
• Increase the epsilon used in quality_check’s normalized std computation for improved numerical stability, and adjust alert-window indexing logic in detect_alert_events.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
core/features.py	Adds guards for spectral centroid / permutation entropy edge cases and replaces deprecated NumPy factorial usage.
core/preprocessing.py	Adjusts epsilon in quality normalization to reduce numerical instability.
core/gate.py	Refactors alert end-index handling to make the inclusive end bound explicit.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new “signal too short” early-return path (including warning emission) isn’t covered by tests. Consider adding a unit test that passes a too-short sig for a given order/delay, asserts the function returns 0.0, and uses pytest.warns to verify the warning is raised.

[Copilot]

min_length is computed as order + delay * (order - 1), which is stricter than what the embedding loop actually requires (range(n - delay*(order-1))). This will return 0.0 (and emit a warning) for signal lengths where the function could still compute permutation entropy (e.g., n=delay*(order-1)+1). Consider changing the guard to match the real minimum required for at least one embedded vector (or clearly justify/parameterize a stricter minimum).

Suggested change

	# Check if signal is too short for the embedding
	min_length = order + delay * (order - 1)
	# Check if signal is too short for the embedding:
	# need at least one embedded vector, i.e. n - delay * (order - 1) >= 1
	min_length = delay * (order - 1) + 1

[Copilot]

The new comment about “mutation issues with zip iteration” is misleading in Python: reassigning offset_idx inside the loop would affect subsequent uses of offset_idx within that iteration. If the intent is to make the inclusive end bound explicit (or to avoid implying mutation of offset_indices), consider rewording the comment to describe that instead.

Suggested change

	# Use separate variable to avoid mutation issues with zip iteration
	# Use a separate variable for the (potentially adjusted) inclusive end index

[Copilot]

New behavior returns 0.0 when np.sum(psd) is zero. There’s currently no test covering a silent/zero signal (PSD all zeros) to ensure this path is exercised and avoids NaNs/ZeroDivision. Consider adding a unit test for compute_spectral_centroid(np.zeros(...), fs) expecting 0.0.