Drop the unreachable guards, and split the STI chain - #507
Merged
Conversation
Three of the four issues SonarCloud still reports on main, plus the dead code the previous branch uncovered but left alone. `_validated_task_levels` re-checked what `Task.__post_init__` already rejects. `Task` is a frozen dataclass that refuses an empty sample set and a non-positive duration at construction, so no invalid task can reach the function and neither guard could ever fire. They are gone and the function is `_task_levels` now, since it no longer validates anything. This is what the `pytest.raises` work found: the test that appeared to cover those guards was in fact exercising the constructor, one frame earlier. `_with_advisory` had a single caller and existed only to name one call to `dataclasses.replace`. Inlining it removes the declared return type that a static analyser cannot reconcile with `replace`'s own signature, and nothing about the behaviour changes. `_sti_from_mtf` was at cognitive complexity 23 against a limit of 15. Three helpers come out of it, each one a step the standard names: `_truncated_mtf` (the A.5.3 NOTE 1 validity check and the truncation to 1,0), `_snr_vector` (the per-band signal-to-noise ratios) and `_corrected_mtf` (the A.5.3 noise, masking and reception-threshold corrections). Validation order is unchanged, so which error fires first is unchanged, and the truncation warning gained a stack level to keep pointing at the caller rather than at library code. The fourth issue is deliberately left open. `OctaveFilterBank.filter` returns two or three values depending on `sigbands`, which is what the rule objects to, but that arity is the documented public contract: four `@overload` stubs type it, 18 call sites unpack two values, 16 unpack three, and 20 documentation pages show both. Making the length uniform would break every one of them to satisfy a rule that is describing the API correctly.
Reviewer's GuideRefactors STI computation into smaller helpers, removes unreachable validation in occupational exposure task handling, and inlines a trivial helper to satisfy static analysis findings without changing behaviour. Sequence diagram for occupational exposure advisory and task level computationsequenceDiagram
actor User
participant task_based_exposure
participant _task_levels
participant job_based_exposure
participant dataclasses_replace as replace
User->>task_based_exposure: task_based_exposure(tasks, ...)
task_based_exposure->>_task_levels: _task_levels(tasks)
_task_levels-->>task_based_exposure: levels, durations
task_based_exposure-->>User: ExposureResult
User->>job_based_exposure: job_based_exposure(tasks, ...)
job_based_exposure->>job_based_exposure: [sampling issue detected]
job_based_exposure->>replace: replace(result, sampling_advisory=True)
replace-->>job_based_exposure: ExposureResult
job_based_exposure-->>User: ExposureResult
Flow diagram for refactored STI computation chainflowchart TD
A["_sti_from_mtf(mtf, snr, level, ambient)_"]
B["_truncated_mtf(mtf)_"]
C["_snr_vector(snr)_"]
D["_corrected_mtf(m, snr_arr, level, ambient)_"]
A --> B
B -->|validated & truncated m| D
A --> C
C -->|snr_arr| D
D -->|corrected m, band_levels| E["downstream STI steps (effective SNR, TI, weighted STI)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
the
area: hearing
|
You've reached the fair usage limit for the Bito Code Review Agent trial. Upgrade to a paid plan or contact support@bito.in for assistance. Learn more |
github-actions
Bot
added
the
area: speech
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR simplifies occupational exposure task processing and advisory updates. It also refactors STI MTF validation, SNR normalization, and correction calculations into private helpers while preserving the existing calculation pipeline. ChangesOccupational exposure cleanup
Speech transmission index refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
validation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #507 +/- ##
=======================================
Coverage 97.04% 97.05%
=======================================
Files 295 295
Lines 39148 39146 -2
=======================================
Hits 37992 37992
+ Misses 1156 1154 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Numerical conformance report✅ 533/533 conformance checks pass across 57 domains and 362 standards - filters class 1 - weightings within IEC 61672-1 class 1. Each row pins a standard clause to its expected normative value and the value the library computes. Every section below is collapsible and stays collapsed while all of its rows pass; a section with any failing row opens automatically. ✅ Numerical validation - filters & weightings: class showcase (IEC 61260-1 · IEC 61672-1 · ISO 7196)IEC 61260-1:2014 class per filter architecture (order 6, one-third-octave, 100 Hz-10 kHz, fs = 48 kHz). For each architecture the table shows, at its binding band, the measured relative attenuation and the class-1 limit it must clear, so the number and the range it must sit in are both visible. A positive margin means the acceptance limits are met with that much room.
Only Butterworth (the library default) and Chebyshev-II are class-compliant architectures. Chebyshev-I and elliptic trade the mask for passband ripple, and Bessel for a maximally-flat group delay (soft rolloff); they cannot satisfy the IEC 61260-1 Class 1/2 attenuation mask by construction, so they are labelled By design - this is expected, not a failure or regression. Frequency-weighting conformance (A/C: IEC 61672-1 Table 3; G: ISO 7196 A.3). The max deviation from nominal is informational (it falls at a frequency extreme where the tolerance is widest and asymmetric); compliance is judged at the binding frequency - the one with the least headroom - where the deviation, the applicable tolerance band and the headroom are shown together.
✅ Filters & weightings: 100% (10/10)
✅ Levels & dosimetry: 100% (9/9)
✅ Room & building acoustics: 100% (72/72)
✅ Room acoustics: 100% (16/16)
✅ Psychoacoustics: 100% (14/14)
✅ Speech transmission (IEC 60268-16): 100% (10/10)
✅ System measurement (Golay / Kirkeby / Mueller-Massarani): 100% (5/5)
✅ Intensity & sound power: 100% (10/10)
✅ Building prediction & uncertainty: 100% (15/15)
✅ Outdoor propagation & occupational exposure: 100% (10/10)
✅ Materials: absorption, airflow & impedance: 100% (6/6)
✅ Scattering & diffusion (ISO 17497): 100% (14/14)
✅ In-situ road absorption (ISO 13472): 100% (3/3)
✅ Precision sound power (ISO 3745 / 9614-3): 100% (4/4)
✅ Human vibration (ISO 8041 / 2631 / 5349): 100% (15/15)
✅ Speech intelligibility (ANSI S3.5-1997): 100% (24/24)
✅ Objective intelligibility (STOI / ESTOI): 100% (3/3)
✅ Impulsive-sound prominence (NT ACOU 112): 100% (2/2)
✅ Impulsive-sound prominence (ISO/PAS 1996-3): 100% (2/2)
✅ Room noise (ANSI S12.2-2019): 100% (3/3)
✅ Hearing threshold (ISO 7029 / ISO 389-7): 100% (3/3)
✅ Measurement uncertainty (GUM / Supplement 1): 100% (7/7)
✅ Noise-induced hearing loss (ISO 1999): 100% (6/6)
✅ Multiple-shock whole-body vibration (ISO 2631-5): 100% (6/6)
✅ Sound absorption in enclosed spaces (EN 12354-6): 100% (2/2)
✅ Prominent discrete tones (ECMA-418-1): 100% (2/2)
✅ Tonal audibility (ISO/PAS 20065): 100% (11/11)
✅ Psychoacoustic annoyance & fluctuation strength (Fastl & Zwicker): 100% (3/3)
✅ Electroacoustics: distortion & frequency response: 100% (20/20)
✅ Calibrated spectral analysis (Bendat & Piersol): 100% (12/12)
✅ Multiple-input coherence (Bendat & Piersol): 100% (5/5)
✅ Time-frequency analysis (Bendat & Piersol): 100% (3/3)
✅ Correlation, time delay and envelope (B&P / Knapp & Carter): 100% (7/7)
✅ Cepstrum, liftering and envelope spectrum (Havelock / B&P): 100% (3/3)
✅ Time synchronous averaging (McFadden 1987): 100% (5/5)
✅ Data qualification and Rice statistics (Bendat & Piersol): 100% (8/8)
✅ Underwater acoustics (ISO 18405/17208/18406): 100% (6/6)
✅ Underwater sound propagation (transmission loss): 100% (16/16)
✅ Underwater propagation regimes (Weston flux theory): 100% (3/3)
✅ Marine-mammal auditory weighting (NMFS / Southall): 100% (4/4)
✅ Underwater numerical propagation (modes / rays / PE): 100% (4/4)
✅ Aircraft noise (ICAO Annex 16 / IEC 61265): 100% (15/15)
✅ Rotorcraft noise (ECAC Doc 32 / NORAH2): 100% (12/12)
✅ CNOSSOS-EU road source (Directive 2002/49/EC Annex II): 100% (6/6)
✅ Wind-turbine noise (IEC 61400-11): 100% (3/3)
✅ Porous & multilayer absorbers (Mechel / Bies / Cox & D'Antonio): 100% (20/20)
✅ Slow-sound perfect absorbers (Jimenez et al. Appl. Sci. 2017): 100% (3/3)
✅ Program loudness (ITU-R BS.1770 / EBU R 128): 100% (8/8)
✅ 2D FDTD wave simulation (Attenborough & Van Renterghem 2021, Ch. 4): 100% (4/4)
✅ Swept-sine distortion & phase utilities (Farina / Novak): 100% (7/7)
✅ Spherical ground & barriers (Attenborough / Salomons / Bies): 100% (7/7)
✅ Panel & aperture sound insulation (Bies / Hopkins / Cremer): 100% (17/17)
✅ Bending-wave plate-junction transmission (Cremer / Craik / Hopkins): 100% (6/6)
✅ Atmospheric refraction (Salomons rays / GFPE): 100% (3/3)
✅ Electroacoustics: 100% (9/9)
✅ Industrial noise control: 100% (22/22)
✅ CNOSSOS-EU railway source (Directive 2002/49/EC Annex II): 100% (8/8)
Tests & coverage — 48204 tests, 0 failures (✅ all green)
Conformance harness: |
|
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
1 participant
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.
Three of the four issues SonarCloud still reports on
main, plus the dead codethe previous branch uncovered but deliberately left for a decision.
The unreachable guards
_validated_task_levelsre-checked whatTask.__post_init__already rejects.Taskis a frozen dataclass that refuses an empty sample set and anon-positive duration at construction, so no invalid task can reach the
function and neither guard could ever fire. Both are gone, and the function is
_task_levelsnow, since it no longer validates anything.This is what the
pytest.raiseswork found on the previous branch: the testthat appeared to cover those guards was in fact exercising the constructor, one
frame earlier. The guards had no coverage and could not have any.
S5886, the return type_with_advisoryhad a single caller and existed only to name one call todataclasses.replace. Inlining it removes the declared return type that astatic analyser cannot reconcile with
replace's own signature. Nothing aboutthe behaviour changes.
S3776, the STI chain_sti_from_mtfsat at cognitive complexity 23 against a limit of 15. Threehelpers come out of it, each one a step the standard itself names:
_truncated_mtf— the A.5.3 NOTE 1 validity check and the truncation to 1,0_snr_vector— the per-band signal-to-noise ratios, broadcast from a scalar_corrected_mtf— the A.5.3 noise, masking and reception-threshold correctionsValidation order is unchanged, so which error fires first is unchanged. The
truncation warning gained a stack level to keep pointing at the caller rather
than at library code, which is verified rather than assumed: a controlled call
chain confirms the warning is still attributed to the caller's line.
The fourth issue is left open on purpose
OctaveFilterBank.filterreturns two or three values depending onsigbands,which is what
S8495objects to. That arity is the documented public contract:four
@overloadstubs type it, 18 call sites unpack two values, 16 unpackthree, and 20 documentation pages show both forms. Making the length uniform
would break every one of them to satisfy a rule that is describing the API
correctly rather than finding a defect in it.
No suppression is added here, per the convention that exclusions get their own
branch and their own argument.
One issue that cannot be fixed from the code
SonarCloud also lists an
S1940onsrc/phonometry/iso2631_5.py. That filedoes not exist: the taxonomy work moved it, and the API confirms it
(
Component key ... not found). The issue has been open since 2026-07-09 withno update since, so it is orphaned in SonarCloud rather than present in the
repository, and closing it needs the web interface.
Checks
8041 tests,
ruff,mypyover 419 files, and 533/533 conformance checks withdocs/CONFORMANCE.mdunchanged.Summary by CodeRabbit