fix: fs-aware envelope band default and clean refusal on undeclared unit (v0.9.1)#33
Merged
Conversation
… undeclared unit (v0.9.1) Two robustness fixes of the same class, found while diagnosing crash reports from a pre-0.9.0 server: - generate_envelope_report resolved its default upper band edge to a fixed 5000 Hz, which exceeds Nyquist on any signal sampled below 10 kHz and made the report raise an invalid-band error. The default is now fs-aware (min(5000, Nyquist-1), matching the filter corner realized downstream and compute_envelope_spectrum's own default); an explicitly requested over-Nyquist band is still rejected, never clamped silently. - _convert_to_velocity_mm_s / assess_severity_raw crashed with 'NoneType' object has no attribute 'lower' when handed an undeclared (None) unit. The live diagnose_vibration / assess_severity paths already guard this; the pure conversion now also refuses None with an actionable message — hardening the latent crash for any direct caller while never guessing a unit (declared-unit discipline). Test-first: both reproduced (AttributeError; invalid-band ValueError at fs=8000) then fixed. Tool inventory fixture regenerated for the filter_high signature change. Full suite 887 passed / 17 skipped / 92.18%. Bump to 0.9.1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Patch release v0.9.1. Two robustness fixes of the same class, found by diagnosing crash reports that originated on a pre-0.9.0 server. Both reported symptoms were already resolved in v0.9.0 at the tool boundary; this release hardens the underlying functions so the same failures cannot resurface through any other caller.
Fixes
generate_envelope_report— fs-aware default bandThe default upper band edge was a fixed
5000 Hz, which exceeds Nyquist on any signal sampled below 10 kHz (e.g. 8 kHz), so a default envelope report on such a signal raised an invalid-band error. The default now resolves fs-aware tomin(5000, Nyquist-1)— matching the digital-filter corner realized downstream andcompute_envelope_spectrum's own default. An explicitly requested band above Nyquist is still rejected (never clamped silently). Reproduced at fs=8000 and fs=5000; fixed.ISO unit conversion — clean refusal on an undeclared unit
_convert_to_velocity_mm_s/assess_severity_rawcrashed with'NoneType' object has no attribute 'lower'when handed aNone(undeclared) unit. The livediagnose_vibration/assess_severitypaths already guard this (a structured refusal), so the crash was not reachable through the tools — but the pure conversion now refusesNonewith an actionable message too, hardening the latent crash for any direct caller. It refuses, it never guesses a unit (consistent with the v0.9.0 declared-unit discipline). Reproduced directly; fixed.Note: the reported "default to 'g'" idea was intentionally not taken — defaulting a unit reintroduces exactly the amplitude-based guessing that v0.9.0 removed.
Tests & verification
AttributeError; invalid-bandValueErrorat fs=8000) before the fix, then confirmed fixed. Added a regression for each, plus a guard that an explicit over-Nyquist band still raises.filter_highsignature change (float=5000→Optional[float]=None); the surface stays 33 tools + 3 prompts.Fixes the crashes tracked while running
/ce-debug.🤖 Generated with Claude Code