feat: implement local ML stem separation with chunking#111
Open
seonghobae wants to merge 2 commits into
Open
Conversation
This commit introduces the AudioStemSeparator class to perform local audio separation using PyTorch and demucs (or torchaudio implementations). It handles chunked inference to prevent OOM errors, includes extensive mock testing to maintain 100% test coverage, and integrates with the CLI's main analyze command. It also updates the supply chain inventory for the newly added dependencies (torch, torchaudio, torchvision).
📝 WalkthroughSummary by CodeRabbit릴리스 노트
전체 설명이 변경사항은 분석 엔진에 Demucs 기반 오디오 스템 분리 기능을 통합합니다. 새로운 변경 사항
시퀀스 다이어그램sequenceDiagram
participant CLI as CLI Handler
participant Librosa as Librosa
participant Separator as AudioStemSeparator
participant Demucs as Demucs Model
participant Torch as Torch/Device
CLI->>Librosa: load(audio_path, sr=44100, mono=False)
Librosa-->>CLI: audio_data, sr
CLI->>Separator: separate_audio(audio_data, sr, segment_seconds=2.0)
Separator->>Demucs: _load_model("htdemucs")
Demucs-->>Separator: model instance
Separator->>Separator: eval mode, shape normalization
Separator->>Torch: check CUDA/MPS availability
Torch-->>Separator: device selection (cuda/mps/cpu)
Separator->>Demucs: apply_model(mix, split=True, overlap=0.25, segment=2.0)
Demucs->>Torch: inference under no_grad()
Torch-->>Demucs: separated stems tensor
Demucs-->>Separator: stems (batch, channels, samples)
Separator->>Torch: convert to CPU numpy
Torch-->>Separator: dict[stem_name, np.ndarray]
Separator-->>CLI: {vocals, bass, drums, other}
CLI->>CLI: log stems, continue analysis
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~25 분 시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-baseline.yml:
- Around line 224-226: The workflow step named "Sync Python dependencies"
currently skips macOS x86_64 by using the if-condition; revert that skip so the
uv sync --project services/analysis-engine --group dev --frozen runs on macOS
x86_64 (remove or adjust the if guard) and instead address the PyTorch wheel
issue by adding a CI-friendly workaround: either pin a compatible PyTorch wheel
in the analysis-engine dependency config (pyproject/requirements) or add
commands in the workflow to install/build a macOS x86_64-compatible PyTorch
(e.g., install a compatible wheel or build from source) before running uv sync;
do not document this as a policy exception — keep the sync step enforced per the
cross-platform build policy.
In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 94-106: The stem separation block must be executed only after
request validation and only when the explicit opt-in flag is set; move the
librosa.load / AudioStemSeparator logic so it runs after your validator returns
success and behind a check for the CLI/handler flag (e.g. --separate-stems or a
boolean parameter separate_stems), and keep the existing try/except around the
heavy work (librosa.load, AudioStemSeparator(), separator.separate_audio) so
malformed payloads can't trigger file access or model inference before
validation; specifically, gate the use of audio_path, librosa.load,
AudioStemSeparator, and separator.separate_audio on validation success and
separate_stems == True.
- Around line 102-106: The stem-separation step currently swallows all
exceptions and never uses the produced stems (stems is only logged), so
separation can fail silently or be wasted on success; update the flow so that
AudioStemSeparator().separate_audio(...) returns are passed into the downstream
call (e.g., include stems in the payload or call run_analysis_job(request,
stems=stems)) when successful, and do not convert all errors to warnings — catch
only expected exceptions or log the full error and propagate/fail the job (raise
or return an error status) instead of continuing as success; update the
try/except around separate_audio to call run_analysis_job with the stems
variable on success and to surface failures (or use a controlled mock fallback
with an explicit flag) rather than a silent logging.warning.
In
`@services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py`:
- Around line 38-46: 현재 _load_model 호출은
demucs.pretrained.get_model(self.model_name)를 직접 사용해 런타임에 원격 다운로드와 무결성 검증을
우회하므로, 로컬에 사전 프로비저닝된 weight와 체크섬을 우선 검증한 후 없으면 명시적으로 실패하도록 수정하세요: update
_load_model to first resolve a project-local artifact path for self.model_name
(or a configured model directory), verify the file exists and validate its
checksum/signature, only then load the model (and call .eval()); remove or gate
any fallback to demucs.pretrained.get_model that would perform a network fetch
so that cold-cache runs fail fast with a clear error rather than downloading
remotely.
In `@services/analysis-engine/tests/test_audio_separator.py`:
- Around line 42-64: The test currently only verifies apply_model was called but
not that it received the OOM-mitigation parameters; update the test in
test_audio_separator.py to assert apply_model was called with the expected
kwargs by inspecting mock_apply_model.call_args.kwargs after calling
separator.separate_audio (use the existing mock_apply_model and
separator.separate_audio/segment_seconds), and assert kwargs["split"] is True,
kwargs["segment"] == 2.0 (or segment_seconds), kwargs["overlap"] == 0.25,
kwargs["shifts"] == 1, and kwargs["progress"] is False; keep the existing checks
for call counts and returned stems.
In `@services/analysis-engine/tests/test_cli.py`:
- Around line 377-443: The test
test_cli_main_temporal_analyzer_and_separator_mock_success currently only checks
jobId and can pass without calling librosa.load or
AudioStemSeparator.separate_audio; update the test to assert those functions are
actually invoked with expected args: replace or wrap the current monkeypatch for
librosa.load and
bandscope_analysis.separation.audio_separator.AudioStemSeparator with
spies/mocks that record call counts and parameters, then after cli.main() assert
librosa.load was called with the expected path/sr/mono/duration and that
AudioStemSeparator.separate_audio was called with the expected audio,
sample_rate and segment_seconds; also add a complementary test (or extend this
one) exercising the --separate-stems gate to confirm separation is skipped when
the flag is absent and that separate_audio is not called.
In `@supply-chain/supplemental-component-inventory.json`:
- Around line 14-23: The htdemucs modelArtifacts entry is missing checksum
verification data; add a checksum field (and algorithm if your schema supports
it, e.g., checksumAlgorithm or checksumType) to the htdemucs object so releases
can validate cached weights; locate the "modelArtifacts" array and the entry
with "name": "htdemucs" (and "version": "demucs-v4.0", "sourceUrl": "...") and
include the checksum value and algorithm consistent with other artifacts in this
inventory.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: d034c4c0-df4d-4382-ac78-7aa825c1dea5
⛔ Files ignored due to path filters (1)
services/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.github/workflows/build-baseline.ymlservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/cli.pyservices/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyservices/analysis-engine/tests/test_audio_separator.pyservices/analysis-engine/tests/test_cli.pysupply-chain/supplemental-component-inventory.json
Comment on lines
224
to
226
| - name: Sync Python dependencies | ||
| if: runner.os != 'macOS' || runner.arch != 'X64' # PyTorch lacks Python 3.12 wheel for macOS x86_64 | ||
| run: uv sync --project services/analysis-engine --group dev --frozen |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if there's existing documentation about platform limitations
# and verify Python version compatibility with PyTorch on macOS x86_64
# Check for existing platform limitation documentation
echo "=== Checking for platform limitation docs ==="
rg -i "macos.*x86|x86_64.*mac|intel.*mac" --type md -C 2
# Check pyproject.toml for Python version constraints
echo "=== Checking Python version constraints ==="
fd -t f "pyproject.toml" --exec cat {} \; 2>/dev/null | grep -A5 "python"
# Check if there's a cross-platform build policy
echo "=== Checking cross-platform build policy ==="
fd -t f "cross-platform-build-policy.md" --exec cat {} \; 2>/dev/null | head -50Repository: seonghobae/bandscope
Length of output: 3626
macOS x86_64 빌드에서 의존성 동기화를 건너뛰면 필수 보안 정책을 위반합니다.
docs/security/cross-platform-build-policy.md에서 명시하고 있듯이, macOS Intel (x86_64) 빌드는 protected-branch 변경에 대한 필수 보안 제어이며 선택사항이 아닙니다. 해당 정책은 빌드 작업이 "real dependency install, frontend build, native shell build, analysis engine packaging sanity, and artifact upload"을 실행해야 한다고 요구합니다.
이 변경으로 인해 macOS x86_64에서 uv sync 단계를 건너뛰면 analysis-engine의 의존성이 제대로 설치되지 않아 필수 플랫폼에서 analysis engine packaging sanity 검증이 불가능해집니다.
조치 필요:
PyTorch 휠 부재 문제를 해결하는 대안을 찾아야 합니다. 예를 들어, 다른 의존성 버전 조합을 검토하거나 이 플랫폼에 대한 CI 검증 차외 불일치를 공식적으로 문서화하고 정책 업데이트를 통해 예외를 정당화해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-baseline.yml around lines 224 - 226, The workflow
step named "Sync Python dependencies" currently skips macOS x86_64 by using the
if-condition; revert that skip so the uv sync --project services/analysis-engine
--group dev --frozen runs on macOS x86_64 (remove or adjust the if guard) and
instead address the PyTorch wheel issue by adding a CI-friendly workaround:
either pin a compatible PyTorch wheel in the analysis-engine dependency config
(pyproject/requirements) or add commands in the workflow to install/build a
macOS x86_64-compatible PyTorch (e.g., install a compatible wheel or build from
source) before running uv sync; do not document this as a policy exception —
keep the sync step enforced per the cross-platform build policy.
Comment on lines
+94
to
+106
| logging.info(f"Performing stem separation on {audio_path}...") | ||
| try: | ||
| import librosa | ||
|
|
||
| from bandscope_analysis.separation.audio_separator import AudioStemSeparator | ||
|
|
||
| # Load only the first 10 seconds for the CLI proof to prevent hanging | ||
| y, sr = librosa.load(audio_path, sr=44100, mono=False, duration=10.0) | ||
| separator = AudioStemSeparator() | ||
| stems = separator.separate_audio(y, sample_rate=int(sr), segment_seconds=2.0) | ||
| logging.info(f"Successfully extracted {len(stems)} stems: {list(stems.keys())}") | ||
| except Exception as e: | ||
| logging.warning(f"Stem separation failed, continuing with mock: {e}") |
There was a problem hiding this comment.
검증된 opt-in 요청에서만 stem 분리를 실행해야 합니다.
현재는 local_audio와 localSource만 맞으면 request validation 전에 바로 librosa.load()와 모델 추론으로 들어갑니다. malformed payload도 추가 파일 접근과 무거운 연산을 유도할 수 있고, PR 목표의 --separate-stems opt-in도 지켜지지 않습니다. validator를 먼저 통과시킨 뒤, 명시적 플래그가 있을 때만 이 블록을 실행하세요. As per coding guidelines, "**/*.{ts,tsx,js,jsx,py}: Treat files, URLs, metadata, model artifacts, and project files as untrusted input`."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/cli.py` around lines 94 -
106, The stem separation block must be executed only after request validation
and only when the explicit opt-in flag is set; move the librosa.load /
AudioStemSeparator logic so it runs after your validator returns success and
behind a check for the CLI/handler flag (e.g. --separate-stems or a boolean
parameter separate_stems), and keep the existing try/except around the heavy
work (librosa.load, AudioStemSeparator(), separator.separate_audio) so malformed
payloads can't trigger file access or model inference before validation;
specifically, gate the use of audio_path, librosa.load, AudioStemSeparator, and
separator.separate_audio on validation success and separate_stems == True.
Comment on lines
+102
to
+106
| separator = AudioStemSeparator() | ||
| stems = separator.separate_audio(y, sample_rate=int(sr), segment_seconds=2.0) | ||
| logging.info(f"Successfully extracted {len(stems)} stems: {list(stems.keys())}") | ||
| except Exception as e: | ||
| logging.warning(f"Stem separation failed, continuing with mock: {e}") |
There was a problem hiding this comment.
이 분리 단계는 실패해도 성공으로 끝나고, 성공해도 결과를 버립니다.
stems는 로그에만 쓰이고 실제 응답 생성은 여전히 원래 request로 run_analysis_job()를 호출합니다. 게다가 예외를 모두 warning으로 삼켜서, 분리가 완전히 실패해도 job은 성공으로 보일 수 있습니다. 지금 상태로는 비용만 추가되고 기능 계약은 충족되지 않습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/cli.py` around lines 102 -
106, The stem-separation step currently swallows all exceptions and never uses
the produced stems (stems is only logged), so separation can fail silently or be
wasted on success; update the flow so that
AudioStemSeparator().separate_audio(...) returns are passed into the downstream
call (e.g., include stems in the payload or call run_analysis_job(request,
stems=stems)) when successful, and do not convert all errors to warnings — catch
only expected exceptions or log the full error and propagate/fail the job (raise
or return an error status) instead of continuing as success; update the
try/except around separate_audio to call run_analysis_job with the stems
variable on success and to surface failures (or use a controlled mock fallback
with an explicit flag) rather than a silent logging.warning.
Comment on lines
+38
to
+46
| def _load_model(self) -> Any: | ||
| from demucs.pretrained import get_model | ||
|
|
||
| if self._model is None: | ||
| logger.info("Loading demucs model '%s'...", self.model_name) | ||
| self._model = get_model(self.model_name) | ||
| if self._model: | ||
| self._model.eval() | ||
| return self._model |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full file to understand the context
cat -n services/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyRepository: seonghobae/bandscope
Length of output: 4642
🏁 Script executed:
# Check for any model configuration or provisioning files
find . -type f -name "*.json" -o -name "*.yaml" -o -name "*.yml" | grep -E "(model|config|provision)" | head -20Repository: seonghobae/bandscope
Length of output: 345
🏁 Script executed:
# Search for demucs usage patterns across the codebase
rg "demucs" --type py -B 2 -A 2Repository: seonghobae/bandscope
Length of output: 5509
🏁 Script executed:
# Check if there's any model preloading, caching, or verification logic
rg "model" services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py -A 3 -B 1Repository: seonghobae/bandscope
Length of output: 1922
🏁 Script executed:
# Check for any model provisioning, validation, or configuration mechanisms elsewhere
rg "model" services/analysis-engine/src/bandscope_analysis/ --type py | grep -E "(cache|checksum|verify|hash|provision|local|offline)"Repository: seonghobae/bandscope
Length of output: 236
🏁 Script executed:
# Check if there are any environment variables or config files that enforce local-only model loading
find services/analysis-engine -type f \( -name "*.py" -o -name "*.yaml" -o -name "*.env*" \) -exec grep -l "model.*path\|model.*dir\|model.*local\|DEMUCS" {} \;Repository: seonghobae/bandscope
Length of output: 141
🏁 Script executed:
# Look at demucs library documentation/behavior to understand get_model() default behavior
rg "get_model" services/analysis-engine -B 5 -A 5Repository: seonghobae/bandscope
Length of output: 7920
모델 로딩이 오프라인/무결성 보장을 우회합니다.
코드는 모델명만 전달하여 demucs.pretrained.get_model()을 직접 호출하므로, 사전 프로비저닝된 로컬 artifact와 checksum 검증을 거치지 않습니다. 도큐먼트 주석(25-26줄)에서 "첫 실행 시 네트워크로 다운로드하며, 이후는 오프라인이어야 함"이라고 명시하지만, 실제로는 프로젝트 차원에서 강제하지 않습니다. Cold cache에서는 런타임 원격 다운로드가 발생하고, warm cache에서도 프로젝트 기준 무결성 검증이 없습니다.
로컬 분석 환경에서는 검증된 로컬 weight만 허용하고, 없으면 명시적으로 실패해야 합니다. 코딩 가이드라인 상 "**/*.{ts,tsx,js,jsx,py}: Do not add network-dependent runtime paths for local analysis" 및 "**/*.{ts,tsx,js,jsx,py}: Treat files, URLs, metadata, model artifacts, and project files as untrusted input"을 준수해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py`
around lines 38 - 46, 현재 _load_model 호출은
demucs.pretrained.get_model(self.model_name)를 직접 사용해 런타임에 원격 다운로드와 무결성 검증을
우회하므로, 로컬에 사전 프로비저닝된 weight와 체크섬을 우선 검증한 후 없으면 명시적으로 실패하도록 수정하세요: update
_load_model to first resolve a project-local artifact path for self.model_name
(or a configured model directory), verify the file exists and validate its
checksum/signature, only then load the model (and call .eval()); remove or gate
any fallback to demucs.pretrained.get_model that would perform a network fetch
so that cold-cache runs fail fast with a clear error rather than downloading
remotely.
Comment on lines
+42
to
+64
| # fake apply_model output (batch, sources, channels, samples) | ||
| mock_apply_model.return_value = torch.ones((1, 4, 2, 100)) | ||
|
|
||
| separator = AudioStemSeparator(model_name="fake_model") | ||
|
|
||
| # Test mono audio | ||
| audio_data = np.zeros((100,)) | ||
| result = separator.separate_audio(audio_data, sample_rate=22050, segment_seconds=2.0) | ||
|
|
||
| # Assertions | ||
| mock_get_model.assert_called_once_with("fake_model") | ||
| mock_apply_model.assert_called_once() | ||
|
|
||
| # Verify the results match the model sources | ||
| assert set(result.keys()) == {"drums", "bass", "other", "vocals"} | ||
| for stem_name in ["drums", "bass", "other", "vocals"]: | ||
| assert result[stem_name].shape == (2, 100) | ||
| assert np.all(result[stem_name] == 1.0) | ||
|
|
||
| # Check that model gets loaded only once | ||
| separator.separate_audio(audio_data, sample_rate=22050, segment_seconds=2.0) | ||
| assert mock_get_model.call_count == 1 | ||
| assert mock_apply_model.call_count == 2 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
fd -t f "test_audio_separator.py" --exec wc -l {} +Repository: seonghobae/bandscope
Length of output: 186
🏁 Script executed:
fd -t f "audio_separator.py" --exec wc -l {} +Repository: seonghobae/bandscope
Length of output: 315
🏁 Script executed:
find . -type f -name "test_audio_separator.py" -o -name "audio_separator.py" 2>/dev/null | head -20Repository: seonghobae/bandscope
Length of output: 200
🏁 Script executed:
sed -n '42,64p' ./services/analysis-engine/tests/test_audio_separator.pyRepository: seonghobae/bandscope
Length of output: 1008
🏁 Script executed:
cat -n ./services/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyRepository: seonghobae/bandscope
Length of output: 4642
apply_model() 호출 파라미터를 테스트로 고정하세요.
현재 테스트는 apply_model()이 호출되었는지만 확인하므로, split=True, segment=segment_seconds, overlap=0.25가 빠져도 테스트가 통과합니다. 이 PR의 핵심인 OOM 완화 메커니즘이 테스트로 보호되지 않습니다.
프로덕션 코드(line 103-111)는 다음 파라미터로 apply_model을 호출합니다:
apply_model(
model,
mix,
shifts=1,
split=True,
overlap=0.25,
segment=segment_seconds,
progress=False,
)다음과 같이 파라미터 검증을 추가하세요:
mock_apply_model.assert_called_once()
kwargs = mock_apply_model.call_args.kwargs
assert kwargs["split"] is True
assert kwargs["segment"] == 2.0
assert kwargs["overlap"] == 0.25
assert kwargs["shifts"] == 1
assert kwargs["progress"] is False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/tests/test_audio_separator.py` around lines 42 - 64,
The test currently only verifies apply_model was called but not that it received
the OOM-mitigation parameters; update the test in test_audio_separator.py to
assert apply_model was called with the expected kwargs by inspecting
mock_apply_model.call_args.kwargs after calling separator.separate_audio (use
the existing mock_apply_model and separator.separate_audio/segment_seconds), and
assert kwargs["split"] is True, kwargs["segment"] == 2.0 (or segment_seconds),
kwargs["overlap"] == 0.25, kwargs["shifts"] == 1, and kwargs["progress"] is
False; keep the existing checks for call counts and returned stems.
Comment on lines
+377
to
+443
| def test_cli_main_temporal_analyzer_and_separator_mock_success(monkeypatch) -> None: | ||
| """Ensure the temporal analyzer and stem separator injection block succeeds.""" | ||
| import io | ||
| import json | ||
|
|
||
| from bandscope_analysis import cli | ||
|
|
||
| stdin = io.StringIO( | ||
| json.dumps( | ||
| { | ||
| "jobId": "job-audio-success-sep", | ||
| "request": { | ||
| "sourceKind": "local_audio", | ||
| "projectId": "p1", | ||
| "sourceLabel": "test.wav", | ||
| "roleFocus": [], | ||
| "localSource": { | ||
| "sourcePath": "/valid/path.wav", | ||
| "fileName": "test.wav", | ||
| "extension": "wav", | ||
| "fileSizeBytes": 100, | ||
| }, | ||
| }, | ||
| } | ||
| ) | ||
| ) | ||
| stdout = io.StringIO() | ||
|
|
||
| class FakeAnalyzerSuccess: | ||
| def analyze(self, path): | ||
| return {"bpm": 120.0, "beats": []} | ||
|
|
||
| class FakeAudioStemSeparator: | ||
| def separate_audio(self, audio, sample_rate, segment_seconds=2.0): | ||
| import numpy as np | ||
|
|
||
| return { | ||
| "vocals": np.zeros((2, 100), dtype=np.float32), | ||
| "drums": np.zeros((2, 100), dtype=np.float32), | ||
| "bass": np.zeros((2, 100), dtype=np.float32), | ||
| "other": np.zeros((2, 100), dtype=np.float32), | ||
| } | ||
|
|
||
| def fake_librosa_load(path, sr, mono, duration): | ||
| import numpy as np | ||
|
|
||
| return np.zeros((2, 100), dtype=np.float32), sr | ||
|
|
||
| import librosa | ||
|
|
||
| monkeypatch.setattr(librosa, "load", fake_librosa_load) | ||
| import bandscope_analysis.separation.audio_separator | ||
|
|
||
| monkeypatch.setattr( | ||
| bandscope_analysis.separation.audio_separator, | ||
| "AudioStemSeparator", | ||
| FakeAudioStemSeparator, | ||
| ) | ||
|
|
||
| monkeypatch.setattr(cli, "TemporalAnalyzer", FakeAnalyzerSuccess) | ||
| monkeypatch.setattr(cli.sys, "stdin", stdin) | ||
| monkeypatch.setattr(cli.sys, "stdout", stdout) | ||
| monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) | ||
|
|
||
| assert cli.main() == 0 | ||
| res = json.loads(stdout.getvalue()) | ||
| assert res["jobId"] == "job-audio-success-sep" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
새 CLI 테스트가 분리 경로를 실제로 고정하지 못합니다.
이 테스트는 최종 jobId만 확인해서 librosa.load()나 AudioStemSeparator.separate_audio()가 더 이상 호출되지 않아도 그대로 통과합니다. 새 기능 회귀를 잡으려면 호출 횟수와 인자까지 assert해야 합니다. --separate-stems 게이트를 추가하면 비활성 케이스도 같이 고정해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/tests/test_cli.py` around lines 377 - 443, The test
test_cli_main_temporal_analyzer_and_separator_mock_success currently only checks
jobId and can pass without calling librosa.load or
AudioStemSeparator.separate_audio; update the test to assert those functions are
actually invoked with expected args: replace or wrap the current monkeypatch for
librosa.load and
bandscope_analysis.separation.audio_separator.AudioStemSeparator with
spies/mocks that record call counts and parameters, then after cli.main() assert
librosa.load was called with the expected path/sr/mono/duration and that
AudioStemSeparator.separate_audio was called with the expected audio,
sample_rate and segment_seconds; also add a complementary test (or extend this
one) exercising the --separate-stems gate to confirm separation is skipped when
the flag is absent and that separate_audio is not called.
Comment on lines
+14
to
+23
| "modelArtifacts": [ | ||
| { | ||
| "name": "htdemucs", | ||
| "version": "demucs-v4.0", | ||
| "sourceUrl": "https://dl.fbaipublicfiles.com/demucs/hybrid_transformer/f7e0c4bc-ba3fe64a.th", | ||
| "license": "MIT", | ||
| "storagePath": "~/.cache/torch/hub/checkpoints/", | ||
| "releaseUsage": "Used by analysis-engine for spectral source separation (stems)." | ||
| } | ||
| ], |
There was a problem hiding this comment.
체크섬이 없어 model artifact 검증이 비어 있습니다.
notes와 linked issue의 DoD가 checksum 추적/검증을 요구하는데, 새 htdemucs 엔트리에는 그 값이 없습니다. 이 상태로는 릴리즈 시점에 캐시된 가중치가 기대한 artifact인지 확인할 수 없습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supply-chain/supplemental-component-inventory.json` around lines 14 - 23, The
htdemucs modelArtifacts entry is missing checksum verification data; add a
checksum field (and algorithm if your schema supports it, e.g.,
checksumAlgorithm or checksumType) to the htdemucs object so releases can
validate cached weights; locate the "modelArtifacts" array and the entry with
"name": "htdemucs" (and "version": "demucs-v4.0", "sourceUrl": "...") and
include the checksum value and algorithm consistent with other artifacts in this
inventory.
This was referenced Apr 29, 2026
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.
Description
This PR integrates local-first source separation into the
bandscope-analysisengine. It utilizestorchandtorchaudio(demucs) to split ingested audio into discrete instrumental/vocal stems.Key Changes:
AudioStemSeparatorclass with device management (CUDA/MPS/CPU).cli.pyto support the--separate-stemsflag.test_audio_separator.pyandtest_cli.py.Resolves #106
Security Notes
gc.collect()and clearing cache.