Repository navigation
fix(audio): apply warning limit in timeline order - #2637
rudycelekli wants to merge 2 commits into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
|
All contributors on this pull request have signed the VoiceStudio CLA. Thank you! |
|
[Medium risk] Changes how audio analysis truncates warnings. The reviewed changes appear safe to merge. SummaryThe PR retains the earliest 100 audio warnings across warning kinds and adds a regression test and documentation for that behavior. Reviews (1) · Last reviewed commit: "docs: link quality-order regression repo..." |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughAudio analysis now retains the earliest 100 warnings across warning kinds. It continues scanning after the limit is reached and marks the result as truncated. Documentation and regression coverage describe and test this behavior. ChangesAudio warning retention
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change addresses the reported warning-order regression and retains the earliest warnings across types. No actionable merge risk is evident in the supplied change context. 🚥 Pre-merge checks | ✅ 7 | ❌ 1 | ❓ 1❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (7 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 unsupported.) Full details: Cross-Platform Default ParityExplanation The PR changes default warning retention in
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
|
recheck |
|
Thanks for the report and the work on this, @rudycelekli. The problem in #2636 is fixed on |
Summary
The warning cap is applied while iterating warning types, before sorting by time. One hundred late clipping warnings can therefore remove an earlier silence warning from the result.
Changes
Type
Testing
Hosted CI audit at 2026-10-06T12:35:20.584679+00:00: no failing latest checks; required CLA passes. All latest reported checks completed successfully. No upstream merge performed.
A real SoundFile WAV contains early silence followed by more than one hundred clipping intervals. It reproduces the lost early warning before the fix; the repaired result contains the early silence and exactly one hundred warnings. The heavy API-import test was deselected.
Regression against unchanged main production source: 1 failed, 6 passed, 1 deselected. After the fix: 7 passed, 1 deselected; 20 with changelog gate.
Python 3.13 with
HF_HUB_OFFLINE=1and an empty Hugging Face cache. No model download or inference calls.Diff check and Python compilation pass.
Full backend/Electron suites were not run locally; focused tests alone do not establish hosted security or platform gates. Hosted results are reported separately below.
Checklist
smoke-matrixCI — not run locallyCloses #2636
analyze_audionow retains the earliest 100 warnings across warning types in timeline order and sets the existing truncation flag when it removes later warnings. This fixes cases where late clipping warnings hid earlier silence warnings; the docs and changelog describe the limit. Full backend, Electron, security, and platform validation were not run.