test(analysis): govern real YouTube known-stem benchmark - #828
test(analysis): govern real YouTube known-stem benchmark#828seonghobae wants to merge 16 commits into
Conversation
📝 WalkthroughWalkthroughBandScope에 검증된 Demucs ChangesKnown-stem 검증과 문서 계약
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant YouTubeDownloader
participant AudioSeparator
participant Benchmark
Operator->>YouTubeDownloader: 옵트인 YouTube 입력 다운로드
YouTubeDownloader->>AudioSeparator: 검증된 오디오 전달
AudioSeparator->>Benchmark: shifts=0 htdemucs stem 전달
Benchmark-->>Operator: 정렬 및 품질 게이트 결과 반환
🚥 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (3)
supply-chain/supplemental-component-inventory.json (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
yt-dlp버전을 범위가 아닌 정확한 값으로 기록하는 방안을 검토하십시오.
">=2026.7.4"는 상한이 없는 범위입니다. 43행 notes는 모든 아티팩트에 대해 version 추적을 요구합니다. 재현 가능한 빌드와 SBOM 대조에는 정확한 해석 버전이 더 유용합니다.uv.lock에서 해석된 정확한 버전을 기록하고, 하한 요구사항은 별도 필드로 두는 방안을 검토하십시오.🤖 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 `@supply-chain/supplemental-component-inventory.json` at line 7, Update the yt-dlp component entry in supplemental-component-inventory.json to record the exact resolved version from uv.lock instead of the open-ended “>=2026.7.4” range. Preserve the minimum-version requirement in a separate field if the inventory schema supports it, while keeping the artifact’s version field exact for reproducible SBOM comparison.services/analysis-engine/tests/test_youtube_stem_e2e.py (1)
319-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win러너 검사가 정확한 공백과 줄바꿈에 의존합니다.
324행은
scripts/checks/run_root_tests.mjs의 원본 텍스트에서'"-m",\n "not youtube_stem_e2e"'를 찾습니다. 이 검사는 2칸 들여쓰기와 특정 줄바꿈 위치를 전제합니다. 포매터가 배열 요소를 한 줄로 합치거나 들여쓰기를 바꾸면, 정책이 그대로여도 테스트가 실패합니다.공백에 둔감한 검사로 바꾸십시오.
♻️ 공백에 둔감한 검사로 변경하는 예시
- assert '"-m",\n "not youtube_stem_e2e"' in runner + normalized = " ".join(runner.split()) + assert '"-m", "not youtube_stem_e2e"' in normalized🤖 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 `@services/analysis-engine/tests/test_youtube_stem_e2e.py` around lines 319 - 324, Update test_required_root_suite_explicitly_excludes_live_youtube_marker to validate the runner’s exclusion policy without relying on exact whitespace or line breaks; normalize the runner text or use a whitespace-tolerant pattern while still requiring the "-m" option and "not youtube_stem_e2e" marker.services/analysis-engine/tests/known_stem_benchmark.py (1)
250-261: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value거친 정렬과 정밀 정렬의 상관 부호 처리가 다릅니다.
250-260행의 거친 단계는
coarse_correlation[valid_coarse]의 최대값을 부호 그대로 선택합니다. 287행의 정밀 단계는np.abs(...)의 최대값을 선택합니다. 290행_normalized_correlation도 절댓값을 반환합니다.이 차이 때문에 정밀 단계는 위상이 반전된 정렬 위치를 최적으로 선택할 수 있습니다. 그 결과
identity_correlation이 높게 나와도 신호가 반전 상태일 수 있습니다. 정체성 검증 용도에서는 부호를 무시하는 것이 의도인지 확인하십시오.의도가 "동일 녹음 확인"이라면 정밀 단계도 부호 있는 최대값을 사용하는 편이 일관됩니다.
♻️ 부호 처리를 일치시키는 변경 예시
- best_refined_index = int(valid_refined[np.argmax(np.abs(refined_correlation[valid_refined]))]) + best_refined_index = int(valid_refined[np.argmax(refined_correlation[valid_refined])])Also applies to: 287-290
🤖 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 `@services/analysis-engine/tests/known_stem_benchmark.py` around lines 250 - 261, Align the correlation sign handling in the coarse and fine alignment stages: update the fine-stage selection around _normalized_correlation and the logic at lines 287-290 to choose the maximum signed correlation rather than the maximum absolute value. Preserve the existing lag search and ensure identity validation cannot prefer phase-inverted matches.
🤖 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 `@ARCHITECTURE.md`:
- Around line 96-102: AudioStemSeparator._load_model에서
demucs.pretrained.get_model() 호출 전에 지정된 모델 캐시/아티팩트의 바이트 크기와 전체 SHA-256을 검증하고, 누락
또는 불일치 시 torch 역직렬화 전에 closed 상태로 실패하게 하십시오. 검증용 다운로드 경로를 제공하거나 htdemucs 캐시가 없으면
로드를 거부하고, supply-chain/supplemental-component-inventory.json의 메타데이터와 실제 검증값을
일치시키십시오. ARCHITECTURE.md 96-102, CLAUDE.md 61-64, docs/TRD.md 78-84,
docs/engineering/youtube-known-stem-validation.md 73-82,
docs/operations/deploy-runbook.md 35-36의 설명을 이 실제 로드 경계 동작 및 release blocker 정책에
맞게 갱신하십시오.
In `@docs/architecture/diagrams.md`:
- Around line 144-148: Update the Mermaid relationship between REFERENCE_ARCHIVE
and REFERENCE_STEM to use one-to-many cardinality, changing the current ||--||
association to ||--|{ so an archive can contain multiple extracted members while
preserving the existing diagram entities.
- Around line 130-134: Update the deployment diagram around the YouTube,
archive, master, model, App, and Evidence nodes by adding a KnownStemBenchmark
opt-in validation boundary. Route YouTube, Pinned creator archive, Pinned
creator master, and bounded numeric evidence through KnownStemBenchmark instead
of connecting them directly to App; keep App as the local runtime, and show
model provisioning separately without adding network dependencies to the
React/Tauri, Rust, or Python runtime layers.
In `@docs/architecture/overview.md`:
- Around line 48-50: Update the known-stem validation description in the
architecture overview to replace “proves” with wording such as “defines and
exercises,” and explicitly state that no live passing run currently validates
the complete YouTube intake through SI-SDR scoring path. Preserve the
distinction between offline deterministic-component coverage and unverified
production behavior.
In `@docs/documentation-coverage-matrix.md`:
- Around line 56-57: Clarify the “13 passed” entry in the documentation-coverage
matrix by identifying it as a historical partial suite and specifying which
deterministic known-stem contract cases were missing or expected failures
relative to the documented 16-test baseline. Add a reference to the execution
command or case manifest used so the result is traceable, while preserving the
separate 16-test standard.
In `@docs/engineering/acceptance-criteria.md`:
- Around line 60-61: Update the release-blocker criteria in the acceptance
criteria entry to explicitly require closure of the model-rights/legal decision
before a release can be blocked or passed. Add this requirement alongside the
existing ADR-0001/0002 blockers without removing the authorization,
verification, candidate, calibration, or platform-evidence conditions.
In `@docs/PRD.md`:
- Around line 13-15: Update the BandScope proof-obligation text in PRD.md to
state the requirement directly, without referring to the current conversation or
other external context. Describe that production YouTube intake and the
production separator must demonstrate improvement on a real, known source rather
than merely returning plausible arrays or synthetic output, and reference the
relevant repository Issue or ADR.
In `@docs/repository/bootstrap-plan.md`:
- Around line 20-21: Align the protected-check lists in
docs/repository/bootstrap-plan.md lines 20-21 and
docs/workflow/github-bootstrap-execution-policy.md lines 90-92 with the
canonical list in docs/security/github-required-checks.md by adding
trivy-fs-scan or directly referencing the canonical document at both sites; do
not weaken the required CI and security gates.
In `@docs/TRD.md`:
- Around line 86-88: Define an allowlisted executable policy in docs/TRD.md:
release evidence must use a verified absolute ffmpeg path rather than an
operator-provided PATH resolution, record its file hash or trusted package
source, and reject evidence when identity verification fails. Update
docs/engineering/youtube-known-stem-validation.md lines 73-75 so the benchmark
invokes that verified absolute path. Update docs/operations/deploy-runbook.md
lines 31-32 to record the executable path and file identity alongside ffmpeg
-version; yt-dlp remains a locked Python package and neither dependency should
be described as bundled.
In `@scripts/checks/verify_docs.py`:
- Around line 99-110: Update documentation_violations to recursively inspect
every Markdown file under docs/plans, independently of REQUIRED_REFERENCES, and
append a violation for each file lacking a “Security Notes” section while
preserving existing checks. Add a regression test covering a newly discovered
plan document without that section and asserting the violation is reported.
In `@scripts/checks/verify_supply_chain.py`:
- Around line 81-123: Strengthen the inventory validation around the function
handling supplemental inventory: first require the top-level inventory to be a
dictionary before calling get, then strictly validate required model-artifact
fields for presence, non-empty values, and expected types. Reject booleans as
sizeBytes by checking for an actual integer, while preserving positive-size
validation; also add regression tests covering an empty modelArtifacts list,
array-valued inventories, sizeBytes=true, and invalid or empty required-field
values.
In `@services/analysis-engine/src/bandscope_analysis/youtube.py`:
- Around line 146-148: Ensure the YouTube download configuration using
compat_opts explicitly supports environments with unavailable or unconfigured
system CA stores. Preserve the valid no-certifi option for OS-managed trust,
while documenting or enforcing CA availability for target CI/container
environments and validating a certifi fallback path where needed.
In `@services/analysis-engine/tests/known_stem_benchmark.py`:
- Around line 237-246: 추가된 align_active_reference_window 검증 분기를 직접 호출하는 테스트를
작성하십시오. sample_rate, alignment durations, alignment resolution, 참조 신호보다 긴
scoring window, 허용 lag 없음, 전체 scoring window 생성 불가 조합마다 ValueError를 검증하고, 각 오류
메시지와 해당 입력 조건을 명시적으로 확인하여 100% 분기 커버리지를 확보하십시오.
In `@services/analysis-engine/tests/test_youtube_stem_e2e.py`:
- Around line 327-422: known_stem_benchmark.py의 align_active_reference_window()
입력 검증 및 lag 탐색 분기가 커버되지 않았습니다.
services/analysis-engine/tests/known_stem_benchmark.py:237-286에 sample_rate <=
0, 잘못된 duration/refinement, scoring window보다 짧은 reference, 허용된 coarse lag 부재,
refinement 전체 조회에서 유효한 refined lag 부재를 각각 검증하는 테스트를 추가하거나 해당 테스트 모듈을 coverage
source에 포함하십시오. services/analysis-engine/tests/test_youtube_stem_e2e.py:327-422는
이 변경으로 직접 수정할 필요가 없습니다.
---
Nitpick comments:
In `@services/analysis-engine/tests/known_stem_benchmark.py`:
- Around line 250-261: Align the correlation sign handling in the coarse and
fine alignment stages: update the fine-stage selection around
_normalized_correlation and the logic at lines 287-290 to choose the maximum
signed correlation rather than the maximum absolute value. Preserve the existing
lag search and ensure identity validation cannot prefer phase-inverted matches.
In `@services/analysis-engine/tests/test_youtube_stem_e2e.py`:
- Around line 319-324: Update
test_required_root_suite_explicitly_excludes_live_youtube_marker to validate the
runner’s exclusion policy without relying on exact whitespace or line breaks;
normalize the runner text or use a whitespace-tolerant pattern while still
requiring the "-m" option and "not youtube_stem_e2e" marker.
In `@supply-chain/supplemental-component-inventory.json`:
- Line 7: Update the yt-dlp component entry in
supplemental-component-inventory.json to record the exact resolved version from
uv.lock instead of the open-ended “>=2026.7.4” range. Preserve the
minimum-version requirement in a separate field if the inventory schema supports
it, while keeping the artifact’s version field exact for reproducible SBOM
comparison.
🪄 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: 0f8ca180-cf33-4f83-90f4-acbf593cccb3
📒 Files selected for processing (43)
AGENTS.mdARCHITECTURE.mdCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/PRD.mddocs/README.mddocs/TRD.mddocs/adr/0001-source-separation-runtime-and-model-delivery.mddocs/adr/0002-known-stem-youtube-quality-gate.mddocs/adr/0003-ephemeral-benchmark-evidence-model.mddocs/adr/README.mddocs/architecture/diagrams.mddocs/architecture/overview.mddocs/doctoring/real-audio-accuracy-acceptance.mddocs/documentation-coverage-matrix.mddocs/engineering/acceptance-criteria.mddocs/engineering/harness-engineering.mddocs/engineering/youtube-known-stem-validation.mddocs/operations/deploy-runbook.mddocs/plans/2026-03-28-ml-engine-integration.mddocs/release/release-policy.mddocs/repository/bootstrap-plan.mddocs/repository/gitflow.mddocs/repository/governance.mddocs/security/dependency-policy.mddocs/security/github-required-checks.mddocs/workflow/github-bootstrap-execution-policy.mdscripts/checks/run_root_tests.mjsscripts/checks/verify_docs.pyscripts/checks/verify_supply_chain.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyservices/analysis-engine/src/bandscope_analysis/separation/model_weights/bandsplit-v1.jsonservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/known_stem_benchmark.pyservices/analysis-engine/tests/test_documentation_policy.pyservices/analysis-engine/tests/test_separation.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_youtube.pyservices/analysis-engine/tests/test_youtube_stem_e2e.pysupply-chain/supplemental-component-inventory.json
💤 Files with no reviewable changes (1)
- services/analysis-engine/src/bandscope_analysis/separation/model_weights/bandsplit-v1.json
|
@coderabbitai review Please review the current exact head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
scripts/checks/verify_supply_chain.py (1)
80-140: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win인벤토리와 런타임 모델 식별자를 비교하십시오.
Line 80-83은 런타임 모델 이름만 읽습니다. Line 120-139은
checksum과sizeBytes의 형식만 확인합니다. 따라서 다른 유효 SHA-256과 양수 크기를 가진htdemucs인벤토리도 통과합니다.인벤토리의 SHA-256 및 바이트 수를
audio_separator.py의_HTDEMUCS_MODEL_SHA256및_HTDEMUCS_MODEL_BYTES와 비교하십시오. 스키마에 파일명이 있으면_HTDEMUCS_MODEL_FILENAME도 비교하십시오. 유효한 형식이지만 값이 다른 인벤토리를 거부하는 테스트를 추가하십시오.이 문제는 이전 모델 인벤토리와 런타임 검증 계약 지적과 같은 근본 원인입니다.
As per coding guidelines, "Treat files, URLs, metadata, model artifacts, and project files as untrusted input" 및 "strict schema validation" 요구를 적용했습니다.
🤖 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 `@scripts/checks/verify_supply_chain.py` around lines 80 - 140, Update the supplemental inventory validation around the runtime artifact checks to compare checksum and sizeBytes against audio_separator.py’s _HTDEMUCS_MODEL_SHA256 and _HTDEMUCS_MODEL_BYTES, rejecting values that are valid in format but do not match. If the artifact schema includes a filename field, also require it to match _HTDEMUCS_MODEL_FILENAME. Add tests covering mismatched checksum and size values for an otherwise valid htdemucs artifact.Source: Coding guidelines
🤖 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 `@apps/desktop/package.json`:
- Line 23: Update the pull request description for the pdfjs-dist 6.2.108 change
by adding a Security Notes section covering the desktop PDF attack surface,
trust boundary, mitigations, verification or test points, dependency and
supply-chain impact, and any i18n impact.
In `@CHANGELOG.md`:
- Around line 21-28: CHANGELOG.md must document the required security evidence
for these changes. Add a Security Notes section covering attack surfaces, trust
boundaries, mitigations, test points, dependency and supply-chain impact, and
i18n impact; also include the result of running ./scripts/harness/quickcheck.sh
before claiming completion.
- Around line 25-28: Update AudioStemSeparator failure handling and the
_stem_separation_failure() mapping so missing or invalid local htdemucs
artifacts that raise ValueError are classified as FAILED, not BLOCKED. Preserve
BLOCKED for operator authorization, authentication, or network-related failures,
and return the appropriate failed classification/API response for both
missing-file and invalid-artifact cases.
In
`@services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py`:
- Around line 180-203: Update the model artifact validation flow around
configured and artifact_path so expanduser() runs before the symlink check,
ensuring paths such as ~/model-link are rejected with ValueError before
resolve(). Add a regression test covering a tilde-based path that points
directly to a symlink, while preserving the existing artifact filename, size,
and SHA-256 validation.
In `@services/analysis-engine/tests/test_separation.py`:
- Around line 373-396: Update AudioStemSeparator._verified_model_artifact_path()
to call expanduser() before checking is_symlink(), ensuring user-relative paths
cannot bypass symlink rejection before resolve(). Extend
test_audio_stem_separator_rejects_untrusted_model_path_shapes with a regression
case using a ~/... configured path that resolves to a symlink, and verify it
raises the existing “model artifact” ValueError.
---
Duplicate comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 80-140: Update the supplemental inventory validation around the
runtime artifact checks to compare checksum and sizeBytes against
audio_separator.py’s _HTDEMUCS_MODEL_SHA256 and _HTDEMUCS_MODEL_BYTES, rejecting
values that are valid in format but do not match. If the artifact schema
includes a filename field, also require it to match _HTDEMUCS_MODEL_FILENAME.
Add tests covering mismatched checksum and size values for an otherwise valid
htdemucs artifact.
🪄 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: dc829a52-8d24-4146-923e-44d6c970c772
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
ARCHITECTURE.mdCHANGELOG.mdCLAUDE.mdapps/desktop/package.jsondocs/PRD.mddocs/TRD.mddocs/adr/0001-source-separation-runtime-and-model-delivery.mddocs/adr/README.mddocs/architecture/diagrams.mddocs/architecture/overview.mddocs/documentation-coverage-matrix.mddocs/engineering/acceptance-criteria.mddocs/engineering/youtube-known-stem-validation.mddocs/operations/deploy-runbook.mddocs/plans/2026-03-28-ml-engine-integration.mddocs/repository/bootstrap-plan.mddocs/workflow/github-bootstrap-execution-policy.mdscripts/checks/verify_docs.pyscripts/checks/verify_supply_chain.pyservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/known_stem_benchmark.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_documentation_policy.pyservices/analysis-engine/tests/test_separation.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_youtube.pyservices/analysis-engine/tests/test_youtube_stem_e2e.pysupply-chain/supplemental-component-inventory.json
🚧 Files skipped from review as they are similar to previous changes (17)
- services/analysis-engine/tests/test_documentation_policy.py
- docs/workflow/github-bootstrap-execution-policy.md
- docs/adr/README.md
- docs/repository/bootstrap-plan.md
- scripts/checks/verify_docs.py
- docs/operations/deploy-runbook.md
- docs/PRD.md
- docs/documentation-coverage-matrix.md
- docs/architecture/overview.md
- docs/plans/2026-03-28-ml-engine-integration.md
- docs/engineering/acceptance-criteria.md
- supply-chain/supplemental-component-inventory.json
- docs/adr/0001-source-separation-runtime-and-model-delivery.md
- CLAUDE.md
- docs/TRD.md
- services/analysis-engine/tests/test_youtube_stem_e2e.py
- services/analysis-engine/tests/known_stem_benchmark.py
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Exact-head handoff — 2026-08-10PR #828 is published at:
The remote tree is byte-identical to the frozen locally audited tree. OutcomeThe bounded known-vocal YouTube sentinel is now design-sufficient, not release-ready. The canonical graph includes 11 PRD requirements, 13 TRD requirements, three ADRs, root/discoverable Architecture, sequence/state/deployment/class UML, a schema-v1 logical run/evidence aggregate, explicit physical-ERD non-applicability, and 13 complete machine-checked requirement-to-evidence rows. The evidence contract now resolves the prior contradictions:
Local exact-tree verification
Honest live boundaryHistorical production YouTube intake failed closed with HTTP 502 after 65.49 s, before separation. It produced no identity correlation or SI-SDR score. Creator-master-only calibration (+1.752 dB SI-SDRi, +7.631 dB margin) is not a YouTube pass. Still blocking release/mergeProduct release remains blocked by formal authorization, model rights/delivery, exact-checkpoint approved-pickle risk acceptance or non-pickle replacement, authorized YouTube calibration/pass, accepted retention controls plus schema-v1 emitter/artifact, per-advertised-platform evidence, planned failure UX, and broader #770 coverage. Merge additionally waits for unchanged-head hosted workflows and qualifying exact-head independent review. The PR intentionally remains open and unmerged. |
|
@coderabbitai review Please review the current exact head |
|
I will focus on the restricted ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
CHANGELOG.md (1)
24-25: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win패치된 버전의 변경 내역을 고정된 출처로 기록하세요.
changeset의 현재 변경 내용에는pdfjs-dist,nanoid,undici의 advisory ID, 이전·이후 버전, lockfile/SBOM 근거가 없습니다.CHANGELOG.md항목에GHSA-/CVE-ID와 해당 패키지의 고정된 패치 버전 또는 lockfile/SBOM 감사 출력을 추가하세요.🤖 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 `@CHANGELOG.md` around lines 24 - 25, Update the changelog entry describing the patched pdfjs-dist, nanoid, and undici versions to include each applicable GHSA or CVE advisory ID, the previous and fixed package versions, and immutable lockfile or SBOM audit evidence. Keep the existing security-update summary while recording these concrete provenance details in CHANGELOG.md.Source: Coding guidelines
services/analysis-engine/tests/test_youtube.py (1)
339-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value전역
os.access대신 대상 모듈 속성을 패치하십시오.현재 패치는 stdlib
os모듈 전역을 교체합니다. 같은 테스트 실행 중 pytest 내부나 다른 헬퍼가os.access를 호출하면 예기치 않은 결과를 받습니다. 검증 경계인bandscope_analysis.youtube.os.access로 범위를 좁히십시오.♻️ 제안 수정
ffmpeg = tmp_path / "ffmpeg" ffmpeg.write_bytes(b"not executable") - monkeypatch.setattr(os, "access", lambda *_args: False) + monkeypatch.setattr( + "bandscope_analysis.youtube.os.access", + lambda *_args, **_kwargs: False, + )🤖 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 `@services/analysis-engine/tests/test_youtube.py` around lines 339 - 342, Update the monkeypatch in the test’s non-executable ffmpeg branch to target bandscope_analysis.youtube.os.access instead of the global os.access attribute. Keep the lambda behavior returning False unchanged, while limiting the patch to the module under test.services/analysis-engine/tests/test_supply_chain_policy.py (1)
285-293: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value저장소 전체 순회 테스트의 실행 비용을 확인하세요.
security_pattern_violations(repo_root)는repo_root.rglob("*")로 모든 항목을 순회합니다.EXCLUDED_PARTS는 스캔 대상만 걸러내고 디렉터리 순회 자체는 막지 않습니다.node_modules,target,.venv가 존재하는 개발 환경에서는 이 테스트 하나가 수만 개 경로를 방문합니다.
security_pattern_violations에서 제외 디렉터리를 순회 단계에서 잘라내면 이 테스트와 실제 게이트 실행 모두 빨라집니다.🤖 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 `@services/analysis-engine/tests/test_supply_chain_policy.py` around lines 285 - 293, Update security_pattern_violations to prune directories listed in EXCLUDED_PARTS during traversal, rather than filtering them only after repo_root.rglob("*") yields paths. Preserve scanning of all non-excluded files and directories while preventing descent into excluded trees such as node_modules, target, and .venv.scripts/checks/verify_supply_chain.py (1)
115-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
analysis_lock_path기본값이inventory_path위치에 암묵적으로 결속됩니다.기본값은
inventory_path.resolve().parent.parent를 저장소 루트로 가정합니다. 이 가정은 인벤토리가supply-chain/바로 아래에 있을 때만 성립합니다. 호출자가 다른 위치의 인벤토리를 전달하면 잘못된 경로를 읽고analysis lock is unreadable위반이 발생합니다.services/analysis-engine/tests/test_supply_chain_policy.py의tmp_path기반 테스트가 이미 이 상태에 해당하며, 해당 테스트는 특정 문자열만 확인하므로 잉여 위반이 조용히 섞입니다.저장소 루트를 파일 위치에서 직접 유도하세요.
♻️ 제안 수정
if analysis_lock_path is None: - analysis_lock_path = ( - inventory_path.resolve().parent.parent / ANALYSIS_LOCK_PATH - ) + analysis_lock_path = REPO_ROOT / ANALYSIS_LOCK_PATH
REPO_ROOT = Path(__file__).resolve().parents[2]를 모듈 상수로 추가하세요.🤖 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 `@scripts/checks/verify_supply_chain.py` around lines 115 - 118, Update the default analysis_lock_path resolution in the surrounding verification flow to derive the repository root from the script location, not inventory_path; add the REPO_ROOT module constant using Path(__file__).resolve().parents[2] and use it when analysis_lock_path is None.scripts/checks/security_gates.py (1)
42-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value예외 규칙이 로더 소스의 정확한 문자열 스냅샷에 결속되어 있습니다.
VERIFIED_MODEL_SAFE_GLOBALS_DEFINITION은audio_separator.py의 함수 본문을 공백과 줄바꿈까지 포함해 복제합니다.VERIFIED_TORCH_LOAD_CALL도 주석 문구와 줄 배치를 고정합니다. 포매터 설정, 줄 길이, 주석 문구가 바뀌면 예외가 사라지고 게이트는 무해한 변경에도 실패합니다.이 결속은 의도된 fail-closed 동작이므로 지금은 안전합니다. 다만 실패 메시지가 원인을 설명하지 않습니다. 스냅샷 불일치 시 별도 진단 메시지를 추가하면 유지보수 비용이 줄어듭니다.
♻️ 진단 메시지 추가 예시
def _content_for_pattern_scan(relative_path: Path, content: str) -> str: """Remove only the one fully constrained checkpoint-deserialization call.""" if relative_path != VERIFIED_MODEL_LOADER_PATH: return content
security_pattern_violations()에서 이 파일이 위반으로 보고될 때, 스냅샷 상수 중 어떤 항목이 불일치했는지 함께 출력하도록 확장하세요.🤖 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 `@scripts/checks/security_gates.py` around lines 42 - 77, Update security_pattern_violations() to include a diagnostic identifying which snapshot validation failed when this file is reported as a violation. Distinguish mismatches for VERIFIED_MODEL_SAFE_GLOBALS_DEFINITION, VERIFIED_TORCH_LOAD_CALL, and VERIFIED_MODEL_LOADER_PREREQUISITES while preserving the existing fail-closed validation behavior.
🤖 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 `@CHANGELOG.md`:
- Around line 20-33: Under [Unreleased] in CHANGELOG.md, add a Security Notes
section documenting the security evidence for the changes listed in the Fixed
section: untrusted YouTube inputs and model artifacts, TLS and ffmpeg/ffprobe
trust boundaries and allowlists, fail-closed validation behavior, logging and
privacy impact, security verification tests, and dependency/supply-chain
verification scope.
In `@scripts/checks/verify_security_notes.py`:
- Around line 18-31: Update security_notes_section to locate the heading with
the same SECURITY_NOTES_PATTERN contract used by documentation_violations,
allowing trailing whitespace, and return an empty string when absent. Track
fenced code blocks with FENCE_PATTERN while scanning, and only apply
HEADING_PATTERN as the section terminator outside fences; preserve the existing
section extraction and lowercasing behavior.
In
`@services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py`:
- Around line 38-40: Update the _numpy_scalar import in audio_separator.py to
use the NumPy 2.x-compatible numpy._core.multiarray path instead of
numpy.core.multiarray, while preserving the existing scalar alias and behavior.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 24-25: Update the changelog entry describing the patched
pdfjs-dist, nanoid, and undici versions to include each applicable GHSA or CVE
advisory ID, the previous and fixed package versions, and immutable lockfile or
SBOM audit evidence. Keep the existing security-update summary while recording
these concrete provenance details in CHANGELOG.md.
In `@scripts/checks/security_gates.py`:
- Around line 42-77: Update security_pattern_violations() to include a
diagnostic identifying which snapshot validation failed when this file is
reported as a violation. Distinguish mismatches for
VERIFIED_MODEL_SAFE_GLOBALS_DEFINITION, VERIFIED_TORCH_LOAD_CALL, and
VERIFIED_MODEL_LOADER_PREREQUISITES while preserving the existing fail-closed
validation behavior.
In `@scripts/checks/verify_supply_chain.py`:
- Around line 115-118: Update the default analysis_lock_path resolution in the
surrounding verification flow to derive the repository root from the script
location, not inventory_path; add the REPO_ROOT module constant using
Path(__file__).resolve().parents[2] and use it when analysis_lock_path is None.
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 285-293: Update security_pattern_violations to prune directories
listed in EXCLUDED_PARTS during traversal, rather than filtering them only after
repo_root.rglob("*") yields paths. Preserve scanning of all non-excluded files
and directories while preventing descent into excluded trees such as
node_modules, target, and .venv.
In `@services/analysis-engine/tests/test_youtube.py`:
- Around line 339-342: Update the monkeypatch in the test’s non-executable
ffmpeg branch to target bandscope_analysis.youtube.os.access instead of the
global os.access attribute. Keep the lambda behavior returning False unchanged,
while limiting the patch to the module under test.
🪄 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: cbed67a7-5530-4015-b6c8-7569e46dce82
📒 Files selected for processing (43)
ARCHITECTURE.mdCHANGELOG.mdCLAUDE.mddocs/PRD.mddocs/TRD.mddocs/adr/0001-source-separation-runtime-and-model-delivery.mddocs/adr/0002-known-stem-youtube-quality-gate.mddocs/adr/README.mddocs/architecture/diagrams.mddocs/architecture/overview.mddocs/doctoring/real-audio-accuracy-acceptance.mddocs/documentation-coverage-matrix.mddocs/engineering/acceptance-criteria.mddocs/engineering/youtube-known-stem-validation.mddocs/operations/deploy-runbook.mddocs/plans/2026-03-10-bandscope-cross-platform-build.mddocs/plans/2026-03-10-bandscope-harness.mddocs/plans/2026-03-10-bandscope-supply-chain-design.mddocs/plans/2026-03-10-bandscope-supply-chain.mddocs/plans/2026-03-28-ml-engine-integration.mddocs/release/release-policy.mddocs/repository/bootstrap-plan.mddocs/security/dependency-policy.mddocs/security/sbom-policy.mddocs/workflow/github-bootstrap-execution-policy.mdscripts/checks/run_analysis_command.pyscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/separation/__init__.pyservices/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/known_stem_benchmark.pyservices/analysis-engine/tests/test_analysis_command.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_documentation_policy.pyservices/analysis-engine/tests/test_separation.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_youtube.pyservices/analysis-engine/tests/test_youtube_stem_e2e.pysupply-chain/supplemental-component-inventory.json
🚧 Files skipped from review as they are similar to previous changes (18)
- docs/adr/README.md
- services/analysis-engine/tests/test_api.py
- services/analysis-engine/src/bandscope_analysis/api.py
- docs/adr/0002-known-stem-youtube-quality-gate.md
- docs/repository/bootstrap-plan.md
- docs/PRD.md
- scripts/checks/verify_docs.py
- ARCHITECTURE.md
- docs/doctoring/real-audio-accuracy-acceptance.md
- docs/architecture/overview.md
- docs/operations/deploy-runbook.md
- docs/engineering/youtube-known-stem-validation.md
- services/analysis-engine/tests/known_stem_benchmark.py
- docs/plans/2026-03-28-ml-engine-integration.md
- docs/engineering/acceptance-criteria.md
- docs/documentation-coverage-matrix.md
- docs/adr/0001-source-separation-runtime-and-model-delivery.md
- services/analysis-engine/tests/test_separation.py
|
@coderabbitai review Please review the current exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='e37b456f55ab3fb117bbaab306ac5846207ad89d'
git rev-parse --verify "${head_sha}^{commit}"
git status --short
git diff --name-status "${head_sha}^" "${head_sha}"
printf '\n--- focused symbols ---\n'
rg -n -C 3 \
-e 'Security Notes' \
-e 'numpy\._core|numpy\.core|_core' \
-e 'safe_globals|torch\.load|weights_only|pickle' \
-e 'security_pattern_violations|default.*lock|lock.*default' \
-e 'exclude|excluded|rglob|os\.walk|glob' \
scripts services --glob '*.py' || trueLength of output: 100318 🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='e37b456f55ab3fb117bbaab306ac5846207ad89d'
printf '%s\n' '--- commit diff: focused files ---'
git diff --unified=80 "${head_sha}^" "${head_sha}" -- \
scripts/checks/verify_security_notes.py \
scripts/checks/security_gates.py \
scripts/checks/verify_supply_chain.py \
services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py \
services/analysis-engine/tests/test_documentation_policy.py \
services/analysis-engine/tests/test_supply_chain_policy.py
printf '%s\n' '--- security notes checker ---'
cat -n scripts/checks/verify_security_notes.py
printf '%s\n' '--- security gate ---'
sed -n '1,170p' scripts/checks/security_gates.py | cat -n
printf '%s\n' '--- supplemental inventory function and constants ---'
rg -n -C 8 'def supplemental_inventory_violations|ANALYSIS_LOCK_PATH|analysis_lock_path|default.*lock|lock_path' scripts/checks/verify_supply_chain.py
printf '%s\n' '--- focused test ranges ---'
sed -n '1,130p' services/analysis-engine/tests/test_documentation_policy.py | cat -n
sed -n '70,125p' services/analysis-engine/tests/test_supply_chain_policy.py | cat -n
sed -n '300,590p' services/analysis-engine/tests/test_supply_chain_policy.py | cat -nLength of output: 88114 🤖 web_search executed:
Length of output: 11497 🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='e37b456f55ab3fb117bbaab306ac5846207ad89d'
printf '%s\n' '--- NumPy lock entries ---'
rg -n -C 4 'name = "numpy"|name = "torch"|name = "demucs"' services/analysis-engine/uv.lock
printf '%s\n' '--- release-risk traceability changes ---'
git diff --unified=20 "${head_sha}^" "${head_sha}" -- \
CHANGELOG.md \
docs/TRD.md \
docs/adr/0001-source-separation-runtime-and-model-delivery.md
printf '%s\n' '--- fence parser call sites and parser tests ---'
rg -n -C 3 '\bsecurity_notes_section\s*\(' . --glob '*.py'
rg -n -C 4 'FENCE_PATTERN|fenced|fence|```|~~~' services/analysis-engine/tests --glob '*.py'
printf '%s\n' '--- static diff hygiene ---'
git diff --check "${head_sha}^" "${head_sha}"Length of output: 26306
Security Notes fence parsing —
A plan can therefore put Restrict the fence prefix to zero through three literal spaces. Add a regression case that confirms a four-space-indented ````` line does not suppress the next peer heading. The requested NumPy compatibility alias, checkpoint security-gate binding, excluded-tree pruning, and repository-root default lock behavior otherwise appear correctly constrained by the inspected source and tests.
|
|
@coderabbitai The exact-head Security Notes parser finding is fixed in |
|
Tip For best results, initiate chat on the files or code changes.
I will treat If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='189d722c6a1fc6ec8e23363d38eb97da5680f3eb'
git rev-parse --verify "${head_sha}^{commit}"
git show -s --format='commit=%H%nparent=%P%ntree=%T%nsubject=%s' "$head_sha"
printf '%s\n' '--- changed paths ---'
git diff --name-status "${head_sha}^" "$head_sha"
printf '%s\n' '--- changed Python and Markdown outlines ---'
changed_files=$(git diff --name-only "${head_sha}^" "$head_sha")
while IFS= read -r path; do
case "$path" in
*.py)
printf '\n--- outline: %s ---\n' "$path"
ast-grep outline "$path" --items all || true
;;
esac
done <<< "$changed_files"
printf '%s\n' '--- focused symbols and schema/policy references ---'
rg -n -C 4 \
-e 'schema-v1|schema_v1|schemaVersion|evidence' \
-e 'rendered|Markdown|markdown|fence|heading' \
-e 'authority|authoritative|PRD|TRD|ADR' \
scripts services docs \
--glob '*.py' --glob '*.md' --glob '*.json' || true
printf '%s\n' '--- focused commit diff ---'
git diff --unified=70 "${head_sha}^" "$head_sha" -- \
scripts/checks \
services/analysis-engine/tests \
docs \
AGENTS.md ARCHITECTURE.md README.md CLAUDE.md CONTRIBUTING.md CHANGELOG.md \
supply-chain || true
printf '%s\n' '--- diff whitespace check ---'
git diff --check "${head_sha}^" "$head_sha"Length of output: 91305 🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='189d722c6a1fc6ec8e23363d38eb97da5680f3eb'
printf '%s\n' '--- commit metadata and changed paths ---'
git show -s --format='commit=%H%nparent=%P%ntree=%T%nsubject=%s' "$head_sha"
git diff --name-status "${head_sha}^" "$head_sha"
printf '%s\n' '--- parser and checkers ---'
cat -n scripts/checks/markdown_sections.py
sed -n '1,470p' scripts/checks/verify_docs.py | cat -n
cat -n scripts/checks/verify_security_notes.py
printf '%s\n' '--- Markdown parser dependency and check invocations ---'
rg -n -C 4 'markdown-it|markdown_it' services/analysis-engine/pyproject.toml services/analysis-engine/uv.lock package.json scripts services --glob '!**/test/**' || true
rg -n -C 3 'verify_docs\.py|verify_security_notes\.py|run_analysis_command' package.json scripts .github --glob '*.json' --glob '*.yml' --glob '*.yaml' --glob '*.mjs' --glob '*.py' || true
printf '%s\n' '--- authority and evidence-contract content ---'
for path in \
docs/README.md docs/PRD.md docs/TRD.md docs/documentation-coverage-matrix.md \
docs/adr/0003-ephemeral-benchmark-evidence-model.md \
docs/release/release-policy.md \
docs/engineering/youtube-known-stem-validation.md; do
if [ -f "$path" ]; then
printf '\n--- %s ---\n' "$path"
rg -n -C 5 'authority|authoritative|schema-v1|schema v1|evidence|retention|upload|path|environment' "$path" || true
fi
done
printf '%s\n' '--- targeted test inventory ---'
rg -n -C 2 \
-e 'table|traceability|canonical_outer_pipe|source_headers|source_rows' \
-e 'html|comment|heading|Setext|fence|link|image|escape' \
services/analysis-engine/tests/test_documentation_policy.py \
services/analysis-engine/tests/test_*security* \
|| trueLength of output: 142761
I found no actionable issue. The rendered-Markdown checks use the directly pinned The schema-v1 contract consistently keeps artifact upload and retention disabled until governance accepts the required store, access, TTL, deletion-verification, and incident-owner controls. The documents correctly separate historical failures and creator-master calibration from current-head production-path evidence. This review was static. The sandbox cannot run the repository test suite. You are interacting with an AI system. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
scripts/checks/verify_docs.py (1)
361-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick wincanonical
## Security Notesheading 판별 로직이 두 스크립트에 중복됩니다.이 블록은
scripts/checks/verify_security_notes.py의_security_notes_contract(27-33행)과 동일한 판정을 수행합니다. 두 구현은 같은 정책 계약을 표현합니다. 한쪽만 수정하면 두 검사기의 판정이 갈라집니다.공유 헬퍼를
scripts/checks/markdown_sections.py에 두고 두 스크립트가 호출하도록 정리하십시오.♻️ 제안 방향
+# scripts/checks/markdown_sections.py +def canonical_h2_headings( + document: MarkdownDocument, + heading_text: str, +) -> list[MarkdownHeading]: + """Return exact column-zero canonical H2 headings from a scanned document.""" + return [ + heading + for heading in document.headings + if heading.level == 2 + and heading.text == heading_text + and document.lines[heading.start].rstrip(" \t") == f"## {heading_text}" + ]
verify_docs.py의_canonical_h2_headings와verify_security_notes.py의 heading 필터를 이 헬퍼로 대체하십시오.🤖 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 `@scripts/checks/verify_docs.py` around lines 361 - 372, Extract the shared canonical “Security Notes” heading detection into a helper in markdown_sections.py, preserving the existing heading-level, exact-text, regex, and unsafe-HTML checks. Update verify_docs.py’s _canonical_h2_headings and verify_security_notes.py’s _security_notes_contract to call this helper so both checks use one policy implementation.services/analysis-engine/tests/test_documentation_policy.py (2)
680-690: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value기대값을 파라미터와 함께 선언하십시오.
680-689행은 공통 기대값을 만든 다음
hidden_kind로 다시 덮어씁니다. 1265-1271행도 같은 패턴입니다. 파라미터마다 기대 결과가 다르므로,pytest.mark.parametrize에 입력과 기대값을 쌍으로 넣으면 각 케이스의 계약이 한 곳에서 읽힙니다. 이 방식은 케이스 추가 시 분기 조건을 갱신하는 실수를 막습니다.🤖 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 `@services/analysis-engine/tests/test_documentation_policy.py` around lines 680 - 690, Update the parameterized tests around security_notes_violations and the analogous case near the other duplicated expectation to pass each hidden_kind together with its expected result through pytest.mark.parametrize. Remove the in-test expected-value reassignment branches, and assert directly against the case-specific expected value so each parameter’s contract is declared in one place.
786-788: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value후행 공백에 의존하는 픽스처를 명시적으로 표현하십시오.
787행은 ``` 뒤의 후행 공백 한 칸에 테스트 의도가 걸려 있습니다. 편집기 설정이나
trailing-whitespace계열 포매터가 이 공백을 제거하면 픽스처가 유효한 GFM 펜스 종료로 바뀝니다. 그러면 테스트 의도가 사라집니다.공백을 문자열 연결로 명시하십시오.
♻️ 제안 수정
plan_path.write_text( - """# Unsafe plan + """# Unsafe plan ## Security Notes ```text -``` +```""" + + " " + + """ ### Attack surface또는 픽스처 본문을
"\n".join([...])로 구성하고 해당 줄만"``` "로 두십시오.🤖 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 `@services/analysis-engine/tests/test_documentation_policy.py` around lines 786 - 788, Make the Markdown fixture around the closing ``` before “### Attack surface” explicitly include its trailing space through string concatenation or a joined-line representation. Ensure the resulting fixture still contains exactly “``` ” rather than relying on source-line trailing whitespace.scripts/checks/verify_security_notes.py (1)
25-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueraw HTML 실패와 heading 누락 실패를 구분해 보고하십시오.
25-26행은
has_unsafe_html이 참일 때 빈 계약을 반환합니다.security_notes_violations는 이 상태를missing section: ## Security Notes로 보고합니다. 문서에 canonical heading이 실제로 존재해도 같은 메시지가 나옵니다. 작성자는 원인을 찾기 어렵습니다.별도 상태를 반환해
contains unsupported raw HTML위반을 보고하십시오.verify_docs.py의requirement_traceability_violations는 이미 같은 방식으로 구분합니다. 이 변경은 관련 테스트 기대값 갱신을 동반합니다.🤖 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 `@scripts/checks/verify_security_notes.py` around lines 25 - 26, Update the unsafe-HTML branch in security_notes_violations so it returns a distinct violation state instead of the empty contract currently produced when document.has_unsafe_html is true. Make the reporting path emit “contains unsupported raw HTML” while preserving “missing section: ## Security Notes” for genuinely absent headings, following the distinction used by requirement_traceability_violations. Update related test expectations.
🤖 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/engineering/youtube-known-stem-validation.md`:
- Around line 106-110: Update the youtube-known-stem-v1 documentation to define
authorization_ref as a required non-null evidence input, describe its validation
before live execution proceeds, and document the preflight authorization_missing
termination path when it is absent. Keep the existing execution-input and
retained-evidence rules unchanged.
In `@package.json`:
- Around line 16-17: Update the check:docs and check:security-notes npm scripts,
along with the Python launcher contract used by scripts/harness/quickcheck.sh,
to invoke the cross-platform Python executable guaranteed on Windows instead of
python3. Preserve the existing run_analysis_command.py arguments so its
uv/local-venv fallback remains reachable.
In `@services/analysis-engine/tests/test_youtube.py`:
- Around line 342-345: Update the symlink test around _has_execute_permission to
create an actual symbolic link pointing to an executable regular file,
preserving the target’s execute permission, then assert
runtime_dependency_invalid for that artifact. Keep the existing non-executable
regular-file validation as a separate test case rather than using it for the
symlink scenario.
---
Nitpick comments:
In `@scripts/checks/verify_docs.py`:
- Around line 361-372: Extract the shared canonical “Security Notes” heading
detection into a helper in markdown_sections.py, preserving the existing
heading-level, exact-text, regex, and unsafe-HTML checks. Update
verify_docs.py’s _canonical_h2_headings and verify_security_notes.py’s
_security_notes_contract to call this helper so both checks use one policy
implementation.
In `@scripts/checks/verify_security_notes.py`:
- Around line 25-26: Update the unsafe-HTML branch in security_notes_violations
so it returns a distinct violation state instead of the empty contract currently
produced when document.has_unsafe_html is true. Make the reporting path emit
“contains unsupported raw HTML” while preserving “missing section: ## Security
Notes” for genuinely absent headings, following the distinction used by
requirement_traceability_violations. Update related test expectations.
In `@services/analysis-engine/tests/test_documentation_policy.py`:
- Around line 680-690: Update the parameterized tests around
security_notes_violations and the analogous case near the other duplicated
expectation to pass each hidden_kind together with its expected result through
pytest.mark.parametrize. Remove the in-test expected-value reassignment
branches, and assert directly against the case-specific expected value so each
parameter’s contract is declared in one place.
- Around line 786-788: Make the Markdown fixture around the closing ``` before
“### Attack surface” explicitly include its trailing space through string
concatenation or a joined-line representation. Ensure the resulting fixture
still contains exactly “``` ” rather than relying on source-line trailing
whitespace.
🪄 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: fd7a46a2-643c-4af1-9812-32081f880358
⛔ Files ignored due to path filters (1)
services/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
ARCHITECTURE.mdCHANGELOG.mddocs/PRD.mddocs/TRD.mddocs/adr/0001-source-separation-runtime-and-model-delivery.mddocs/adr/0002-known-stem-youtube-quality-gate.mddocs/adr/0003-ephemeral-benchmark-evidence-model.mddocs/adr/README.mddocs/architecture/diagrams.mddocs/architecture/overview.mddocs/doctoring/real-audio-accuracy-acceptance.mddocs/documentation-coverage-matrix.mddocs/engineering/acceptance-criteria.mddocs/engineering/youtube-known-stem-validation.mddocs/operations/deploy-runbook.mddocs/plans/2026-03-10-bandscope-harness.mddocs/plans/2026-03-12-issue-32-analysis-orchestration-design.mddocs/release/release-policy.mdpackage.jsonscripts/checks/markdown_sections.pyscripts/checks/run_analysis_command.pyscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/harness/quickcheck.shservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/separation/audio_separator.pyservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/conftest.pyservices/analysis-engine/tests/test_analysis_command.pyservices/analysis-engine/tests/test_documentation_policy.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_youtube.py
🚧 Files skipped from review as they are similar to previous changes (17)
- services/analysis-engine/pyproject.toml
- docs/adr/README.md
- docs/plans/2026-03-10-bandscope-harness.md
- docs/architecture/overview.md
- docs/doctoring/real-audio-accuracy-acceptance.md
- docs/adr/0003-ephemeral-benchmark-evidence-model.md
- ARCHITECTURE.md
- docs/PRD.md
- docs/engineering/acceptance-criteria.md
- scripts/checks/security_gates.py
- docs/adr/0001-source-separation-runtime-and-model-delivery.md
- docs/operations/deploy-runbook.md
- docs/architecture/diagrams.md
- docs/release/release-policy.md
- services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py
- docs/documentation-coverage-matrix.md
- services/analysis-engine/src/bandscope_analysis/youtube.py
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='8e34c0e563fafa08909307109b358cbbebf5537f'
printf '%s\n' '--- exact commit ---'
git rev-parse --verify "${head_sha}^{commit}"
git show -s --format='commit=%H%nparent=%P%ntree=%T%nsubject=%s' "$head_sha"
printf '%s\n' '--- changed paths ---'
git diff --name-status "${head_sha}^" "$head_sha"
printf '%s\n' '--- JavaScript outlines for changed files ---'
git diff --name-only "${head_sha}^" "$head_sha" | while IFS= read -r path; do
case "$path" in
*.js|*.mjs|*.cjs)
printf '\n--- %s ---\n' "$path"
ast-grep outline "$path" --items all || true
;;
esac
done
printf '%s\n' '--- focused diff ---'
git diff --unified=100 "${head_sha}^" "$head_sha" -- \
package.json \
scripts \
services/analysis-engine/tests \
| sed -n '1,2400p'
printf '%s\n' '--- candidate launcher files ---'
fd -t f . scripts services | rg '(python|launcher|command|root_tests|quickcheck|test).*\.(mjs|cjs|js|sh|py)$' || true
printf '%s\n' '--- invocation and implementation references ---'
rg -n -C 5 -i \
-e 'spawnSync|spawn\(|execFile|exec\(|shell\s*:' \
-e 'python3|python|py\s' \
-e 'ENOENT|EACCES|exitCode|status|signal' \
scripts package.json services/analysis-engine/tests \
--glob '*.{js,mjs,cjs,sh,py,json}' || true
printf '%s\n' '--- package scripts and integration files ---'
cat -n package.json
sed -n '1,260p' scripts/checks/run_root_tests.mjs 2>/dev/null || true
sed -n '1,260p' scripts/harness/quickcheck.sh 2>/dev/null || true
printf '%s\n' '--- diff whitespace check ---'
git diff --check "${head_sha}^" "$head_sha"Length of output: 247581 🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='8e34c0e563fafa08909307109b358cbbebf5537f'
printf '%s\n' '--- complete changed launcher and tests ---'
cat -n scripts/checks/python_launcher.mjs
cat -n scripts/checks/run_python.mjs
sed -n '1,380p' services/analysis-engine/tests/test_analysis_command.py | cat -n
printf '%s\n' '--- all current launcher and analysis-command references ---'
rg -n -C 3 \
-e 'run_python\.mjs' \
-e 'run_analysis_command\.py' \
-e '(^|[[:space:]])python3?[[:space:]]+scripts/checks/' \
. \
-g '!package-lock.json' -g '!services/analysis-engine/uv.lock' \
|| true
printf '%s\n' '--- changed documentation diff ---'
git diff --unified=50 "${head_sha}^" "$head_sha" -- \
CHANGELOG.md \
docs/agents/skills/bandscope-supply-chain-warning-remediation/SKILL.md \
docs/engineering/acceptance-criteria.md \
docs/engineering/harness-engineering.md
printf '%s\n' '--- command parsing and root-check policy references ---'
rg -n -C 5 \
-e 'run_analysis_command|run_python|check:docs|quickcheck|root test' \
scripts/checks services/analysis-engine/tests \
--glob '*.py' --glob '*.mjs' --glob '*.sh' || true
printf '%s\n' '--- shell-free launcher check ---'
rg -n -C 2 'spawnSync\(|shell\s*:' scripts/checks/python_launcher.mjs scripts/checks/run_python.mjs scripts/checks/run_root_tests.mjs
printf '%s\n' '--- static whitespace and merge-base summary ---'
git diff --check "${head_sha}^" "$head_sha"
git status --shortLength of output: 80722
I found no actionable issue.
The launcher retries only after The regression tests cover candidate order, This was a static review. I did not run repository code or tests in the sandbox. You are interacting with an AI system. |
|
Exact-head handoff for
No merge was attempted: formal independent exact-head approval remains 0, and the documented authorization/rights/live-calibration/evidence-retention/platform/failure-UX/#770 product gates remain open. |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head8e34c0e563fafa08909307109b358cbbebf5537f. -
Head SHA:
8e34c0e563fafa08909307109b358cbbebf5537f -
Workflow run: 31388648662
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (29 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (29 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (30 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (30 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (7 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (7 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (29 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (29 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (30 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (30 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (7 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (7 files)"]
R3 --> V3["targeted test run"]
|
2 participants
Product outcome
This PR adds an explicit opt-in, fail-closed production-path sentinel for authorized public YouTube intake → independently pinned creator-master identity → composed global alignment → deterministic htdemucs → zero-mean SI-SDR improvement and named-stem assignment.
It is one known-vocal slice related to #770. It does not close the broader multi-fixture/four-stem MIR acceptance program, and it has no passing live exact-candidate score.
Exact published state
8e34c0e563fafa08909307109b358cbbebf5537fa4e77ef4998ac7b30fea38aa02edb8a74b5c348993fdbe70e605b0546df077b784f5570a2a2859e8develop@acdbea6344fe1231c39535b575f4de35e4c607c9(ancestor of head)The published tree is byte-identical to the locally verified tree.
Delivered boundary
BANDSCOPE_HTDEMUCS_MODEL_PATHwithout weakening the same-byte loader.py -3 → python → python3, POSIXpython3 → python; it falls through only on ENOENT and preserves spawn/interpreter exit failures.markdown-it-py 4.0.0; fenced, nested, commented, raw-HTML-hidden, malformed, or markup-only lookalikes cannot satisfy Security Notes or traceability.pdfjs-dist 6.2.108,nanoid 3.3.18, andundici 7.29.0, with a mutation-sensitive lockfile floor test.Verification
The exact candidate tree
a4e77ef4998ac7b30fea38aa02edb8a74b5c3489was constructed before publication and the GitHub tree produced from the predecessor tree matched it byte-for-byte.Current-tree pre-publication evidence:
python_launcher.mjs,run_python.mjs, andrun_root_tests.mjs;git diff --checkpassed;A pre-cleanup superset candidate with the same runtime/model/input/launcher implementation also passed full quickcheck, exact owned-code coverage, type/lint/format/docstring/security/supply-chain/bootstrap checks, npm audit, build, and the exact provisioned htdemucs load smoke. That predecessor evidence is supportive, not a substitute for exact-head proof.
All nine hosted workflows for
8e34c0e563fafa08909307109b358cbbebf5537fcompleted successfully: CI, release, build-baseline, security-audit, Security Scan, SAST Semgrep, Bandit, secret-scan-gate, and SBOM. CodeRabbit's exact-head combined status is success and its command review reported no actionable issue; all 29 review threads are resolved with zero non-outdated unresolved threads. A qualifying independent formal exact-head approval is still absent and remains a merge blocker.Live evidence boundary
A historical authorized predecessor attempt authenticated the pinned archive, extracted vocal, creator master, and pre-provisioned htdemucs full hash. Production YouTube intake then failed closed with HTTP 502 after 65.49 seconds, before separation. It produced no identity correlation or SI-SDR score. This is historical failure evidence, not a live pass and not current-head validation.
Creator-master-only deterministic calibration produced +1.752 dB SI-SDR improvement and +7.631 dB assignment margin. That supports provisional sentinels only; it is not exact YouTube-candidate success.
Documentation fitness
The branch is DESIGN-SUFFICIENT for this bounded known-vocal slice:
BenchmarkRunbinds fixture, candidate, model, and sanitized toolchain even for early failures;It is not release-ready. Product/release blockers are:
Security Notes
Attack surface
Public media, pinned reference assets, yt-dlp, ffmpeg/ffprobe, ZIP/audio decoders, Demucs/torch, model paths/cache, temporary media, Markdown policy evidence, dependency locks, and any future release evidence.
Trust boundary
Every remote response, redirect, archive member, decoded signal, executable, model byte, filesystem identity, Markdown-rendered authority, and provider outcome is untrusted until its owning structure, allowlist, size, type, provenance, and integrity checks pass.
Mitigations
Exact HTTPS hosts, bounded media, full hashes, non-symlink/O_NOFOLLOW model loading, same-byte restricted weights-only deserialization, an exact minimal allowlist, serialized one-time model load, no unrestricted fallback or runtime model retrieval, exact four-part media-runtime preflight, TLS verification, deterministic
shifts=0, ephemeral raw media, redacted errors, rendered-Markdown authority checks, schema-v1 field invariants, disabled retention, and explicit live opt-in outside required CI.Test points
The exact published tree passed full quickcheck, 100% owned-code coverage, npm audit with zero vulnerabilities, Bandit, type/lint/documentation/supply-chain gates, restricted-loader mutation tests, rendered-Markdown adversarial tests, executable-identity rejection tests, and the production build. All nine hosted exact-head workflows passed and no actionable thread remains. Qualifying independent formal exact-head approval is still absent.
Dependency and supply-chain impact
No production dependency was added. Documentation checks directly pin
markdown-it-py 4.0.0as a development dependency. htdemucs remains externally provisioned and non-bundled; ffmpeg/ffprobe remain operator-provided; yt-dlp remains exactly lock-managed.i18n impact
No translatable UI surface or locale resource changed. PRD-KS-011/TRD-KS-013 explicitly keep distinct recovery copy/state work planned rather than claiming it is shipped.
Rollback
Revert this documentation-policy hardening commit while retaining the validated runtime/input/model boundaries. Disable the opt-in live invocation if necessary, but do not restore the FFT pseudo-separator, remote model loading, weakened dependency floors, PATH-only release evidence, or retained raw media.
Reviewer checklist
develop