fix: release MLX resources between long diarized chunks - #399
Conversation
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGPU 오디오 큐레이션 아키텍처와 Rust CLI를 추가했습니다. macOS MLX 런타임 부트스트랩, segmentation benchmark, Python 패키지 연결, CI 검증 및 기존 기능 회귀 테스트를 보강했습니다. Changes오디오 큐레이션 스택
Estimated code review effort: 5 (Critical) | ~90+ minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/test_chapters.py (2)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value모듈 이름과 지역 변수 이름이 충돌합니다.
이 파일의 여러 테스트는
chapters라는 지역 변수를 정의합니다(예: Line 31, Line 47). 새 테스트는 같은 이름의 모듈을 참조합니다. 현재는 동작합니다. 그러나 이후에 이 테스트 안에chapters = ...할당을 추가하면UnboundLocalError가 발생합니다. 별칭을 사용하면 이 위험이 사라집니다.♻️ 별칭 도입 제안
-import chapters +from chapters import _boundaries_from_silencesself.assertEqual( - chapters._boundaries_from_silences( + _boundaries_from_silences( normalized_spans, total_duration=600.0, min_gap_seconds=3.0, ), [], )Also applies to: 62-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_chapters.py` at line 13, Rename the imported chapters module to a distinct alias and update the new tests’ module references accordingly, including the affected range also covered by this comment. Preserve existing local variables named chapters.
62-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value이 테스트는 도달 불가능한 입력을 검증합니다.
_boundaries_from_silences의 유일한 호출자인detect_chapters는_clamped_silences가[0, total_duration]으로 클램프한 spans만 전달합니다. 따라서(-10.0, 0.0)과(600.0, 610.0)같은 범위는 실제 경로에서 발생하지 않습니다. 방어 로직 테스트로서 가치는 있습니다. 클램프 이후 경계값인(0.0, 4.0)과(596.0, 600.0)을 추가하면 실제 경계 조건도 함께 고정됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_chapters.py` around lines 62 - 74, Update test_boundary_helper_drops_timeline_extremes to retain the existing out-of-range cases and also include post-clamping boundary spans (0.0, 4.0) and (596.0, 600.0), ensuring _boundaries_from_silences is verified for both defensive inputs and reachable timeline edges..coveragerc (1)
3-3: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCI에서 coverage gate를 실행하세요.
.coveragerc는audio_library와fail_under = 100을 설정하지만, CI는 unittest만 실행합니다.coverage run과coverage report가 없으므로 현재 설정은 CI 실패를 유발하지 않습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coveragerc at line 3, Update the CI test workflow to run the unittest suite through coverage and execute a coverage report gate afterward, ensuring the existing .coveragerc settings for audio_library and fail_under = 100 are enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/segmentation-reconciliation.md`:
- Around line 39-44: Update the flowchart around “TMK marker ranges” to add the
reliable chapter/marker metadata fallback before nominal checkpoint ranges,
including when TMK is pending, unavailable, or verified without usable markers.
Preserve the existing VAD/silence and fixed-range fallbacks after this metadata
path, and make the precedence explicit as TMK markers → reliable metadata →
VAD/silence → fixed ranges.
In `@scripts/benchmark_segmentation.py`:
- Around line 143-144: Validate the value loaded by the args.silence_json
parsing path before assigning or passing it onward: require a top-level list
whose every item is a two-element pair, and raise a clear input error when the
shape is invalid. Preserve valid silence data unchanged and keep the validation
localized to the --silence-json handling.
---
Nitpick comments:
In @.coveragerc:
- Line 3: Update the CI test workflow to run the unittest suite through coverage
and execute a coverage report gate afterward, ensuring the existing .coveragerc
settings for audio_library and fail_under = 100 are enforced.
In `@tests/test_chapters.py`:
- Line 13: Rename the imported chapters module to a distinct alias and update
the new tests’ module references accordingly, including the affected range also
covered by this comment. Preserve existing local variables named chapters.
- Around line 62-74: Update test_boundary_helper_drops_timeline_extremes to
retain the existing out-of-range cases and also include post-clamping boundary
spans (0.0, 4.0) and (596.0, 600.0), ensuring _boundaries_from_silences is
verified for both defensive inputs and reachable timeline edges.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 77d1eeb8-c686-40c2-8c5d-013f7b2a7d59
⛔ Files ignored due to path filters (3)
docs/papers/2212.04356-whisper.pdfis excluded by!**/*.pdfdocs/standards/NIST.FIPS.180-4.pdfis excluded by!**/*.pdfrust-core/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.coveragerc.github/workflows/ci.yml.gitignoreREADME.mdaudio_library.pydocs/architecture/gpu-transcription-rust-backend.mddocs/architecture/segmentation-reconciliation.mdpyproject.tomlrequirements-macos-mlx-lock.txtrust-core/Cargo.tomlrust-core/src/lib.rsrust-core/src/main.rsrust-toolchain.tomlscripts/benchmark_segmentation.pyscripts/bootstrap_macos_gpu_runtime.shtests/test_audio_library.pytests/test_chapters.pytests/test_ci_workflow.pytests/test_macos_gpu_bootstrap.pytests/test_mcp_driver.pytests/test_media_shrinker.pytests/test_saas_web.py
3f06be3 to
95821d3
Compare
1 participant
Summary
This keeps the existing TMK/VAD/fixed-duration provenance and checkpoint behavior unchanged while preventing long recordings from retaining completed GPU allocations.
Validation
python3 -m pytest -q(628 passed, 751 subtests)cargo test --manifest-path rust-core/Cargo.toml --release(21 passed)python3 -m ruff check audio_library.py tests/test_audio_library.pySummary by CodeRabbit