Skip to content

fix(audio): apply warning limit in timeline order - #2637

Closed
rudycelekli wants to merge 2 commits into
debpalash:mainfrom
rudycelekli:fix/voice-quality-warning-order-20261006
Closed

rudycelekli wants to merge 2 commits into
debpalash:mainfrom
rudycelekli:fix/voice-quality-warning-order-20261006

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Keep the earliest one hundred warnings across all warning types in timeline order, with bounded storage and the existing truncation flag.
  • Add focused regressions and update the relevant documentation and changelog.

Type

  • 🐛 Bug fix

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=1 and 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

  • I've tested this locally
  • Every commit author has signed the CLA — registered signature and required CLA status verified
  • I've updated relevant documentation
  • No local machine paths, logs, or personal env details in this PR
  • Maintained version files are in sync — no version bump
  • Runtime regression fixture still loads green on smoke-matrix CI — not run locally

Closes #2636

analyze_audio now 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.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

All contributors on this pull request have signed the VoiceStudio CLA. Thank you!

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Changes how audio analysis truncates warnings.

The reviewed changes appear safe to merge.

Summary

The 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..."

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: debpalash/VoiceStudio/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 12532ee6-adbc-45cb-87f8-487a409c48fe
📥 Commits

Reviewing files that changed from the base of the PR and between 286f46b and 453ee30.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • backend/services/audio_quality.py
  • docs/audio-quality.md
  • tests/test_audio_quality.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

Audio 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.

Changes

Audio warning retention

Layer / File(s) Summary
Ordered warning cap and validation
backend/services/audio_quality.py, tests/test_audio_quality.py, docs/audio-quality.md, CHANGELOG.md
Warnings are inserted in start-time and kind order. If the count exceeds 100, the last warning is removed and the result is marked truncated. Scanning continues. The test checks that an early silence warning remains ahead of later clipping warnings. The docs and changelog describe the limit.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to 453ee

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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… Write docstrings for the functions missing them to satisfy the coverage threshold.
Cross-Platform Default Parity ❓ Inconclusive The PR changes default warning retention in backend/services/audio_quality.py (lines 74–78). The insertion and ordering logic has no platform-specific branch, but the available evidence does not ver… Run the focused audio-quality regression on macOS, Windows, and Linux and confirm the same warning results on each platform, or move the changed default behavior behind an explicit opt-in.
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed #2636 requires the earliest 100 warnings across warning types. analyze_audio inserts warnings in (start, kind) order, removes the latest entry above the cap, and sets truncated; the regression t…
Out of Scope Changes check ✅ Passed All changed code, regression coverage, documentation, and changelog content support the warning-order fix in #2636. No unrelated change appears in the whole-PR diff.
I18n Completeness (21 Locales) ✅ Passed No Electron files changed in the reviewed diff. The changes are limited to backend audio analysis, tests, documentation, and the changelog. Therefore, the PR adds or changes no Electron UI translation…
Local-First Guarantee ✅ Passed The PR adds no outbound calls, accounts, or API keys. The changed analysis only reads a local audio file and sorts warnings in memory; the regression test also uses local temporary WAV data. No analyt…
Backward Compatibility ✅ Passed The PR changes only audio warning ordering, its test, documentation, and the changelog. analyze_audio reads audio and returns an AudioQuality result; the diff contains no voice, project, settings,…
Title check ✅ Passed The title follows the required conventional-commit format with the audio scope. The issue reference appears in the description as Closes #2636.
Description check ✅ Passed The description includes the required Summary, Changes, Type, Testing, and Checklist sections. It explains the fix, reports focused test results, notes validation limits, and identifies issue #2636.
Full details: Docstring Coverage

Explanation

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 Parity

Explanation

The PR changes default warning retention in backend/services/audio_quality.py (lines 74–78). The insertion and ordering logic has no platform-specific branch, but the available evidence does not verify the new behavior on all three platforms: .github/workflows/ci.yml runs the full tests/ suite on Linux, while its macOS and Windows smoke jobs do not include tests/test_audio_quality.py. No platform divergence is established.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rudycelekli

Copy link
Copy Markdown
Contributor Author

recheck

@debpalash

Copy link
Copy Markdown
Owner

Thanks for the report and the work on this, @rudycelekli. The problem in #2636 is fixed on main by 156ea81e (merged in #2673), so I'm closing this pull request as already fixed. It will ship in v0.5.7. Reporter credit stays in the changelog.

@debpalash debpalash closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Late clipping warnings hide earlier silence at the warning limit

2 participants