fix(release): output actual DMG and EXE installers#162
Merged
Conversation
… in zip This updates the CI to build Tauri properly so users get DMG/EXE/MSI files instead of zip files containing just the executable and raw frontend files.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough이 PR은 분석 엔진에 네 가지 새로운 분석 모듈(코드, 음역대, 시간적, 신호 분리)을 추가하고, 데스크톱 UI 기능을 확장하며, GitHub Actions 워크플로우 권한을 정리하고, JSDoc 문서화를 도입합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Desktop as 데스크톱 앱
participant Tauri as Tauri CLI
participant AnalysisEngine as 분석 엔진
participant Temporal as TemporalAnalyzer
participant Roles as RoleExtractor
participant Chords as ChordAnalyzer
participant Ranges as RangeAnalyzer
User->>Desktop: 로컬 오디오 선택
Desktop->>Tauri: 분석 요청 시작
Tauri->>AnalysisEngine: 오디오 경로 전달
AnalysisEngine->>Temporal: 시간적 분석 (BPM, 비트)
Temporal-->>AnalysisEngine: TemporalFeatures 반환
AnalysisEngine->>Roles: 역할 추출 (음역대, 화음)
Roles->>Chords: 코드 인식 (스템별)
Chords-->>Roles: 코드 데이터
Roles->>Ranges: 음역대 추출 (피치 추적)
Ranges-->>Roles: 음역대 데이터
Roles-->>AnalysisEngine: RoleExtractionResult
AnalysisEngine->>Ranges: RangeAnalyzer (음역대 중복)
Ranges-->>AnalysisEngine: 중복 경고
AnalysisEngine-->>Desktop: 완성된 RehearsalSong
Desktop->>Desktop: UI 렌더링 (기능별)
Desktop-->>User: 분석 결과 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai resolve |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
services/analysis-engine/tests/test_release_packaging.py (1)
121-167:⚠️ Potential issue | 🔴 Critical테스트 설정이 새 번들 구조와 불일치
main()테스트는target/.../release/bandscope-desktop바이너리 경로를 생성하지만, 실제 코드는 이제target/.../release/bundle/dmg/*.dmg또는bundle/nsis/*.exe구조에서 인스톨러를 찾습니다. 테스트가FileNotFoundError로 실패할 것입니다.🐛 테스트 설정 수정 제안
- binary_path = ( - repo_root - / "apps" - / "desktop" - / "src-tauri" - / "target" - / "aarch64-apple-darwin" - / "release" - / "bandscope-desktop" - ) - binary_path.parent.mkdir(parents=True) - binary_path.write_bytes(b"binary") + # Create bundle structure for DMG + dmg_path = ( + repo_root + / "apps" + / "desktop" + / "src-tauri" + / "target" + / "aarch64-apple-darwin" + / "release" + / "bundle" + / "dmg" + / "Bandscope_0.1.1_aarch64.dmg" + ) + dmg_path.parent.mkdir(parents=True) + dmg_path.write_bytes(b"dmg content")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/analysis-engine/tests/test_release_packaging.py` around lines 121 - 167, The test test_release_packaging_main_writes_arch_specific_manifest creates a raw binary at target/.../release/bandscope-desktop but the packaging.main() now looks for installers under bundle/dmg/*.dmg (mac) or bundle/nsis/*.exe (windows), so update the test setup to create the expected installer(s) instead of the old binary path: in the test function adjust the created artifact path using the BANDSCOPE_TARGET_TRIPLE/BANDSCOPE_ARTIFACT_OS to create repo_root/.../target/.../release/bundle/dmg/<something>.dmg for macos (and similarly bundle/nsis/<something>.exe for windows if needed), ensure the file contains some bytes, then keep the same env vars and assertions so packaging.main() finds the installer and writes the manifest.apps/desktop/src/App.tsx (1)
136-137:⚠️ Potential issue | 🟡 MinorYouTube 가져오기 실패 시 메시지 누락 케이스 fallback이 없습니다.
백엔드가
message없이 에러를 반환하면 사용자에게 빈 오류 상태가 표시될 수 있습니다. 로컬 오디오 처리와 동일하게 기본 문구 fallback을 넣어 주세요.🔧 제안 수정안
- } else { - setSelectionError(selection.error.message); + } else { + setSelectionError(selection.error.message || t("youtubeImportFailed")); }Based on learnings: Treat YouTube import as policy-constrained and fallback-friendly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.tsx` around lines 136 - 137, The call to setSelectionError currently uses selection.error.message directly and can set an empty/undefined message if the backend omits message; update the logic around setSelectionError (the code handling selection and selection.error.message) to provide a fallback string (e.g., "Failed to import YouTube video") when selection.error.message is falsy, mirroring the local audio handling so the UI never shows an empty error state.packages/shared-types/src/index.ts (1)
888-932:⚠️ Potential issue | 🟠 Major
partGraph참조 무결성 검증이 빠져 있습니다.Line [921] 이후 로직은 타입만 확인하고,
role_id/handoff_*가 실제roles[].id를 참조하는지와role_id중복 여부를 확인하지 않습니다. 잘못된 그래프가 파서를 통과해 후속 단계에서 깨집니다.변경 제안
function validateRehearsalSection(value: unknown, path: string): string | null { @@ for (const [index, role] of value.roles.entries()) { const roleError = validateRehearsalRole(role, `${path}.roles[${index}]`); if (roleError) { return roleError; } } + const roleIds = new Set((value.roles as RehearsalRole[]).map((role) => role.id)); if (!isDenseArray(value.partGraph)) { return invalidField(`${path}.partGraph`); } + const partGraphRoleIds = new Set<string>(); for (const [index, node] of value.partGraph.entries()) { const nodeError = validatePartGraphNode(node, `${path}.partGraph[${index}]`); if (nodeError) { return nodeError; } + const typedNode = node as PartGraphNode; + if (!roleIds.has(typedNode.role_id)) { + return invalidField(`${path}.partGraph[${index}].role_id`); + } + if (partGraphRoleIds.has(typedNode.role_id)) { + return invalidField(`${path}.partGraph[${index}].role_id`); + } + partGraphRoleIds.add(typedNode.role_id); + for (const [targetIndex, target] of typedNode.handoff_to.entries()) { + if (!roleIds.has(target)) { + return invalidField(`${path}.partGraph[${index}].handoff_to[${targetIndex}]`); + } + } + for (const [sourceIndex, source] of typedNode.handoff_from.entries()) { + if (!roleIds.has(source)) { + return invalidField(`${path}.partGraph[${index}].handoff_from[${sourceIndex}]`); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/src/index.ts` around lines 888 - 932, The validateRehearsalSection function currently validates shapes but not referential integrity between value.partGraph and value.roles: add checks after the existing partGraph validation loop to build a set/map of roles' ids from value.roles (ensure all roles have string id), verify every partGraph node's role_id and any handoff_* fields reference an existing role id, and ensure there are no duplicate role ids in roles[].id; report errors via invalidField using the same path format (e.g., `${path}.roles[${i}].id`, `${path}.partGraph[${j}].role_id`, `${path}.partGraph[${j}].handoff_*`) when a missing/duplicate reference is found so malformed graphs fail validation early.apps/desktop/src/lib/analysis.ts (2)
108-115:⚠️ Potential issue | 🟠 MajorIPC 명령/인자 타입이 너무 넓어서 허용 목록 보장이 깨집니다.
Line [108]의
command: string+args?: Record<string, unknown>는 로컬 IPC를 사실상 자유 입력으로 열어둡니다. 허용된 명령과 명령별 스키마를 타입으로 고정해 주세요.
As per coding guidelinesKeep local backend access on allowlisted IPC or \127.0.0.1` only, with strict schema validation`.변경 제안
+type AnalysisInvokePayload = + | { command: "select_local_audio_source" } + | { command: "start_analysis_job"; args: { request: AnalysisJobRequest } } + | { command: "get_analysis_job_status"; args: { jobId: string } } + | { command: "import_youtube_url"; args: { url: string } } + | { command: "save_project"; args: { payload: RehearsalSong } } + | { command: "load_project" }; + -async function invokeAnalysis(command: string, args?: Record<string, unknown>): Promise<unknown> { +async function invokeAnalysis(payload: AnalysisInvokePayload): Promise<unknown> { const invokeCommand = getInvoke(); if (invokeCommand) { - return invokeCommand(command, args); + return invokeCommand(payload.command, "args" in payload ? payload.args : undefined); } - - return browserFallback(command, args); + return browserFallback(payload.command, "args" in payload ? payload.args : undefined); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/analysis.ts` around lines 108 - 115, The invokeAnalysis function currently accepts open-ended command: string and args?: Record<string, unknown>, which defeats the allowlist and schema guarantees; change it to a discriminated union (or mapped type) of allowed commands and their exact arg shapes (e.g., type AnalysisCommand = { command: 'analyzeFile'; args: { path: string } } | { command: 'summarize'; args: { text: string } } ), update invokeAnalysis signature to accept that union and narrow the return type per command, and propagate the stricter types to getInvoke and browserFallback (or provide overloaded signatures) so only allowlisted command names are accepted; also add runtime validation for each command's args (using existing validator utilities or a small zod/io-ts schema per command) before calling the IPC to enforce schema at runtime.
179-193:⚠️ Potential issue | 🟠 MajorYouTube URL 입력/오류 메시지 처리가 신뢰 기반이라 정보 노출 위험이 있습니다.
Line [181]에서 URL 형식 검증 없이 IPC로 전달하고, Line [187]에서 내부 오류 메시지를 그대로 노출합니다. URL 스키마(https + youtube 도메인) 검증과 안전 메시지 allowlist를 적용해 주세요.
As per coding guidelinesTreat files, URLs, metadata, model artifacts, and project files as untrusted input.변경 제안
+const SAFE_YOUTUBE_IMPORT_MESSAGES = new Set([ + "YouTube import failed.", + "Unsupported YouTube URL." +]); + +function normalizeYoutubeUrl(input: string): string | null { + try { + const parsed = new URL(input); + const host = parsed.hostname.toLowerCase(); + const allowedHosts = new Set(["youtube.com", "www.youtube.com", "m.youtube.com", "youtu.be"]); + if (parsed.protocol !== "https:" || !allowedHosts.has(host)) return null; + return parsed.toString(); + } catch { + return null; + } +} + export async function importYoutubeUrl(url: string): Promise<LocalAudioSelectionResult> { + const normalizedUrl = normalizeYoutubeUrl(url); + if (!normalizedUrl) { + return { + ok: false, + error: { code: "invalid_request", message: "Unsupported YouTube URL." } + }; + } try { - const response = await invokeAnalysis("import_youtube_url", { url }); + const response = await invokeAnalysis("import_youtube_url", { url: normalizedUrl }); return { ok: true, bootstrap: parseProjectBootstrapSummary(response) }; } catch (error) { - const message = error instanceof Error ? error.message : (typeof error === 'string' ? error : "YouTube import failed."); + const raw = error instanceof Error ? error.message : (typeof error === "string" ? error : ""); + const message = SAFE_YOUTUBE_IMPORT_MESSAGES.has(raw) ? raw : "YouTube import failed."; return { ok: false, error: { code: "invalid_request", message } }; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/analysis.ts` around lines 179 - 193, Validate and sanitize the YouTube URL in importYoutubeUrl before calling invokeAnalysis: ensure the URL uses https and the hostname matches allowed YouTube domains (e.g., youtube.com, youtu.be) and reject/return a safe error for invalid inputs; also stop returning raw internal error text from the catch block of importYoutubeUrl — map known error types/messages from invokeAnalysis (or the caught error) to a small allowlist of user-facing messages and return a generic, non-sensitive message for all other failures while logging the original error internally for debugging; reference function names importYoutubeUrl and invokeAnalysis and the caught variable error when implementing these checks and mappings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src-tauri/.cargo/audit.toml`:
- Line 20: The audit entry "RUSTSEC-2024-0429" cannot be blindly ignored: check
the project's glib dependency version (Cargo.lock / Cargo.toml) and ensure it is
>= 0.20.0; if any crate pins glib < 0.20.0, update that dependency to a
compatible version (or update the dependent crate) to eliminate the
VariantStrIter unsoundness, and if an upgrade is impossible, add a comment next
to the "RUSTSEC-2024-0429" entry explaining why (with the blocking
crate/version) and open a tracked issue referencing this audit ID so the risk is
documented and tracked.
- Around line 2-21: 각 ignore 항목(RUSTSEC-2024-0413, RUSTSEC-2024-0416,
RUSTSEC-2025-0057, RUSTSEC-2024-0412, RUSTSEC-2024-0418, RUSTSEC-2024-0411,
RUSTSEC-2024-0417, RUSTSEC-2024-0414, RUSTSEC-2024-0415, RUSTSEC-2024-0420,
RUSTSEC-2024-0419, RUSTSEC-2024-0370, RUSTSEC-2025-0081, RUSTSEC-2025-0075,
RUSTSEC-2025-0080, RUSTSEC-2025-0100, RUSTSEC-2025-0098, RUSTSEC-2024-0429)마다 추적
이슈 번호, 만료일(또는 재검토 날짜), 담당자(이메일 또는 GitHub ID)와 해결 목표(예: 업그레이드 버전 또는 제거 예정일)를
메타데이터로 추가하고, GTK3 관련 집단 무시에 대해선 별도의 정책 문서 링크와 정기 재검토 주기(예: 분기별) 항목을 포함하세요; 특히
RUSTSEC-2024-0429는 메모리 안전성(unsoundness)이므로 별도 위험평가 요약과 완화 계획 및 확정된 해결 일정(또는 차단
이슈)를 명시하도록 업데이트하세요.
In `@apps/desktop/src-tauri/src/main.rs`:
- Around line 790-805: The code currently trusts metadata["filepath"] (the
filepath variable) and uses Path::new(filepath), metadata_fs, and extension
without validation; fix by treating filepath as untrusted: canonicalize the path
and ensure it is a descendant of the app cache/download directory (use
Path::canonicalize and check starts_with the canonicalized cache dir) to prevent
path traversal, validate the file exists after canonicalization (metadata_fs),
and enforce an allowlist for extension (check extension variable against
acceptable types like "m4a","mp3","wav") returning a clear error if checks fail;
apply these validations before using metadata_fs, path, or extension.
In `@apps/desktop/src/features/home/index.tsx`:
- Around line 28-33: The render accesses song.exportSummary directly which can
be undefined and cause a crash; update the component in
apps/desktop/src/features/home/index.tsx to defensively handle missing
exportSummary (use optional chaining like song.exportSummary?.format and
song.exportSummary?.headline or assign a default object before use) so the JSX
for format and headline only renders safe values or falls back to sensible
defaults.
In `@apps/desktop/src/features/settings/index.tsx`:
- Around line 12-45: Replace hardcoded UI strings in the settings component with
createTranslator keys: import and instantiate the translator (e.g., const t =
createTranslator('settings')) and replace "Supported Audio Formats", "Analysis
Pipeline", "About" and the list items ("Decode audio source", "Draft section and
role extraction", "Separate stems by category", "Persist analysis results") with
t('supportedAudioFormats'), t('analysisPipeline'), t('about') and appropriately
namespaced keys like t('analysisPipeline.decodeAudioSource'),
t('analysisPipeline.draftSectionExtraction'), etc.; also replace the hardcoded
paragraph text ("BandScope is a local-first rehearsal prep tool...") with a
t('about.description') key and add these keys to your locale files so
translations are available. Ensure SUPPORTED_AUDIO_FORMATS rendering remains the
same but use a translated heading key for its title.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Around line 12-13: Remove or replace the meaningless JSDoc "Documented."
comments with either a concise, useful description or delete them; specifically
update the JSDoc above the Workspace component (exported function Workspace) and
the other similar JSDoc blocks referenced (the comments at the other positions)
so they either provide a brief summary of the component/props or are removed to
avoid noise.
In `@apps/desktop/src/lib/export.ts`:
- Around line 17-21: The CSV formula-injection check currently uses /^[=+\-@]/
on value and misses cases where dangerous characters are preceded by whitespace;
update the detection to use /^\s*[=+\-@]/ to catch leading spaces/tabs and when
escaping preserve existing leading whitespace (e.g., extract leading whitespace
from value, then set escapedValue = leadingWhitespace + "'" + remainder) so
escapedValue and value handling in export.ts correctly neutralize formulas while
keeping original spacing.
In `@CHANGELOG.md`:
- Around line 1-13: 현재 파일의 헤딩 순서와 공백이 markdownlint(MD041, MD022)를 위반합니다: 상단에 "#
Changelog"를 파일 첫 줄로 올리고 그 아래 한 줄의 공백을 둔 뒤(예: "# Changelog\n\n"), 이어서 "##
[Unreleased]" 및 "## [0.1.1] - 2026-04-28" 같은 H2 섹션들을 각각 위아래로 한 줄의 공백을 두고 배치해 헤딩
계층과 주변 공백 규칙을 만족시키도록 수정하세요; 즉 "## [Unreleased]"와 "## [0.1.1] - 2026-04-28" 앞뒤에 빈
줄을 추가하고 파일 맨 위에 "# Changelog"만 H1으로 위치시켜 MD041/MD022 경고를 해소합니다.
In `@docs/plans/2026-03-28-ml-engine-integration.md`:
- Line 20: The "Track 3: Harmonic & Pitch Pipelines (`#107`) (COMPLETED)" line
incorrectly marks a future plan as completed; update the document by removing
the "(COMPLETED)" suffix from the heading or moving that status into a dedicated
implementation status section, and if Track 3 truly is complete, add a brief
status note in the summary section clarifying completion and linking to the
implementation/details (refer to the heading text "Track 3: Harmonic & Pitch
Pipelines (`#107`)" to locate the line).
- Around line 1-59: The Markdown file "ML Engine Integration Plan" has lint
warnings about missing blank lines around headings; update the document so every
heading token (e.g., "# ML Engine Integration Plan", "## Overview", "### Track
1: Temporal Foundation (`#105`)", etc.) is preceded and followed by a single blank
line, and ensure blank lines separate headings from adjacent paragraph text and
bullet lists (for example around the Execution Tracks, each "Track X" heading
and the Security Notes/Threat sections) so the file conforms to the
markdown-lint rules; review all headings and lists in the diff and insert or
adjust single blank lines consistently.
In `@docs/plans/2026-04-28-pr-159-rollout.md`:
- Around line 154-160: 수정할 문장은 "GitHub Releases serves as the trusted source of
artifacts."로, 이 문구를 과도하게 신뢰하는 표현에서 서명/체크섬 검증 후에만 신뢰 경계로 승격된다는 내용으로 바꿔주세요; 즉
GitHub Releases(또는 로컬 미디어 디렉토리)를 기본적으로 '비신뢰 입력'으로 분류하고, 문서의 Mitigations 섹션(예:
"Code signing for macOS and Windows applications", "Notarization process on
macOS")에 서명/체크섬 검증과 검증 실패 시 거부/알림 절차를 명시하여 아티팩트 승격 흐름을 추가하도록 변경하세요.
In `@docs/security/dependency-policy.md`:
- Line 105: Update the documented exception for "Cargo audit warnings for legacy
`gtk3`, `glib`, and `fxhash` ... ignored by default" to meet policy: replace
"ignored by default" and "e.g." with a precise list of RUSTSEC IDs (remove
"e.g."), state exactly where the exception is applied (the CI `security-audit`
workflow and any runtime scope), enumerate compensating controls and exposure
limits (e.g., BandScope does not parse untrusted runtime input, runtime/process
isolation, dependency usage scope), and add a documented remediation/review plan
specifying who reviews the exception and the cadence/process to remove it once
patched (e.g., triage owner, monthly review, and CI rule to fail when patched
versions available).
- Line 105: Update the sentence that claims "no alternative" for the listed
Cargo audit warnings: change the wording to state these are temporary exceptions
due to Tauri v2 still depending on GTK3/old crates (inherited via
wry/webkit2gtk) and that viable replacements exist (GTK4 as the GTK3 replacement
— migration planned in Tauri v3, see PR `#14684` — and rustc-hash as an
alternative to fxhash), and note that fixes are planned via migration rather
than asserting no alternative.
In `@eslint.config.js`:
- Around line 34-47: The require-jsdoc rule currently enables JSDoc for all node
types via the require object (ArrowFunctionExpression, ClassDeclaration,
ClassExpression, FunctionDeclaration, FunctionExpression, MethodDefinition),
which overrides the export-scoped contexts; to restrict JSDoc to exports either
remove the entire require block or keep it but add "publicOnly: true" inside the
rule so the existing contexts array (ExportNamedDeclaration > ...) is the
effective scope—update the require-jsdoc configuration by editing the require
object (or adding publicOnly: true) so only exported declarations defined in the
contexts array are required to have JSDoc.
In `@scripts/fix-version-format.sh`:
- Line 4: Replace the UUOC pipeline that reads VERSION_FILE into tr by
redirecting the file into tr: update the assignment to CURRENT_VERSION so it
uses tr -d '\r\n[:space:]' < "$VERSION_FILE" (keep the same variable names
CURRENT_VERSION and VERSION_FILE) to simplify the command and avoid unnecessary
use of cat.
In `@scripts/release/package_desktop_artifact.py`:
- Around line 114-117: The duplicate-prevention logic currently does nothing
because archive_name.replace(ext, f"{ext}") replaces the extension with the same
text; change it to append a unique identifier from the original installer
filename so multiple installers don't overwrite each other. In the block that
checks if len(installers) > 1 (variables: installers, installer_path, ext,
archive_name), build a new archive_name that inserts installer_path.stem (or
installer_path.name) into the archive base before the final extension (e.g.,
base + "_" + installer_path.stem + ext) so each installer produces a distinct
archive name.
- Around line 104-106: The FileNotFoundError raised when no installers are found
uses a long inline message; extract that message into a module-level constant
(e.g., INSTALLER_NOT_FOUND_MSG) or define a small custom exception class (e.g.,
InstallerNotFoundError) and raise it with the constant message. Update the code
that calls find_installer_packages and the raise site (reference: variable
installers and the raise of FileNotFoundError) to use the new constant or custom
exception so the message is centralized and reusable.
In `@SECURITY.md`:
- Line 10: Replace the placeholder contact email seonghobae@example.com in
SECURITY.md with a real, monitored security contact for the project (e.g.,
security@<your-project-domain> or security@bandscope.com) and update the
adjacent advisory link text if you change the domain to ensure reports reach
maintainers; ensure the new address uses the actual project domain and is
reachable for private vulnerability reports.
In `@services/analysis-engine/src/bandscope_analysis/chords/analyzer.py`:
- Around line 74-78: The "source" value pulled with harmony.get("source",
"model") must be allowlisted and normalized to only "model" or "user" before
being placed in the output dict; update the code that builds the chord dict (the
block that sets "chord": chord_name, "functionLabel": str(harmony.get(...)),
"source": ...) to read the raw value from harmony, coerce to str, check against
an allowlist {"model","user"} (optionally normalize known variants like
"Model"/"USER"), and set "source" to the validated value or fallback to "model"
if it isn't allowed; ensure the variable names harmony and chord_name are used
so the change localizes to the same dict construction.
In `@services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py`:
- Around line 85-87: Wrap the call to librosa.decompose.nn_filter(chromagram,
...) in a try/except that catches librosa.util.exceptions.ParameterError and
ValueError (and optionally Exception as a fallback), storing a copy of the
original chromagram before the call (e.g., original_chromagram = chromagram)
and, on any exception, assign chromagram = original_chromagram so processing
continues; also emit a warning via the module's logger (e.g., logger.warning or
self.logger.warning) including the exception message and context that nn_filter
failed.
In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 79-85: The code assumes request["localSource"] is a dict and calls
.get, which can raise AttributeError for malformed payloads; update the logic
around the block that computes audio_path (the request dict check and audio_path
assignment) to first validate the payload schema or at minimum assert
isinstance(request.get("localSource"), dict) before accessing
.get("sourcePath"), and reject/raise a clear validation error if it is not a
dict; also ensure this local backend access path follows the IPC/127.0.0.1
allowlist policy by validating the source address/transport before proceeding.
- Around line 34-36: 현재 CLI 코드 calls sys.stdin.read() into input_data with no
size cap, which risks OOM on large payloads; change the read to enforce a
reasonable maximum (e.g., N bytes) by reading in bounded chunks or using
sys.stdin.read(MAX_BYTES) and then validate length, and if the input exceeds the
limit, log/print an error and exit non‑zero (or return a failure response)
instead of processing; update references to input_data and the stdin read code
path to perform this check and fail fast.
In `@services/analysis-engine/src/bandscope_analysis/ranges/analyzer.py`:
- Around line 201-203: The loop over section_roles assumes each element is a
dict and calls role.get causing runtime errors; update the loop in the analyzer
(the section that iterates section_roles) to first verify that role is a dict
(e.g., isinstance(role, dict)) and skip non-dict entries before accessing
role.get("range"), so that role_range is only computed for dicts and non-dict
inputs are safely ignored (optionally log or warn when encountering invalid
items).
- Around line 59-67: The current left-scanning logic misparses negative octaves
(e.g., "C-1" becomes ("C-", 1)); replace the manual loop with a regex-based
parse that captures the note name and a signed integer octave to ensure "note"
like "C-1" yields name "C" and octave -1; update the code paths that set "name"
and "octave_str" (the parsing block in analyzer.py) to validate the regex groups
and convert the octave to int only after confirming it matches a signed integer
pattern so MIDI calculations use the correct negative octave.
In `@services/analysis-engine/src/bandscope_analysis/ranges/model.py`:
- Around line 8-14: RangeInfo has inconsistent field names: role_id and
role_name use snake_case while lowestNote and highestNote use camelCase; update
the TypedDict to use snake_case (lowest_note, highest_note) and then find and
update all usages/serializers/deserializers that reference
RangeInfo.lowestNote/RangeInfo.highestNote (e.g., any JSON parsing, pydantic
models, dict keys, or tests) to use the new names, preserving external API
compatibility where needed by mapping incoming/outgoing keys if the external
JSON must remain camelCase.
In `@services/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.py`:
- Around line 68-78: The current confidence logic sets confidence = "high" if
avg_prob > 0.6 else "low", which omits the "medium" level expected by
RangeOverlap.severity; update the logic around avg_prob and confidence in
pitch_tracker.py (the avg_prob calculation and the confidence assignment) so
that if avg_prob < 0.2 you still return the early unvoiced result with "low",
otherwise set confidence = "medium" for avg_prob in [0.2, 0.6] and "high" for
avg_prob > 0.6 (ensure the returned confidence values match the allowed union
"low" | "medium" | "high").
- Around line 41-44: The broad except Exception around the librosa.pyin call
should be narrowed to specific expected errors; update the try/except that wraps
librosa.pyin(y, fmin=fmin, fmax=fmax, sr=sr) to catch only known failure types
(for example librosa.util.exceptions.ParameterError, ValueError and RuntimeError
or other librosa-specific exceptions your codebase sees), log the caught
exception details, return the existing {"lowest_note": None, "highest_note":
None, "confidence": "low"} for those cases, and let unexpected exceptions
propagate (do not catch Exception or BaseException). Ensure you reference the
librosa.pyin call in pitch_tracker.py when making the change.
In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py`:
- Around line 57-95: The current single try/except around all stem processing
(wrapping PitchTracker/ChordRecognizer usage and the "vocals"/"bass"/"other"
blocks) causes one stem failure to abort processing of the rest; split the
exception handling so each stem is processed in its own try/except: keep
instantiation of PitchTracker and ChordRecognizer outside, then wrap the
"vocals" block, the "bass" block (including pitch_tracker.track and
chord_recognizer.recognize), and the "other" block (chord_recognizer.recognize)
each in their own try/except; in each except call logger.warning with a message
that includes the stem name and exception (use logger.warning("Failed to extract
%s features: %s", "vocals", e) etc.) so failures are scoped per stem and do not
prevent other stems from being extracted.
In `@services/analysis-engine/src/bandscope_analysis/separation/model.py`:
- Around line 23-26: Change the StemDescriptor.category annotation from plain
str to the StemCategory enum: update the dataclass/TypedDict/type definition to
use StemCategory, add/import StemCategory where it is defined, and adjust any
places that construct or serialize StemDescriptor (e.g., creation sites,
factories, JSON (de)serializers or pydantic models) to pass/convert enum members
instead of raw strings so type checks and runtime usage remain consistent.
In `@services/analysis-engine/src/bandscope_analysis/separation/separator.py`:
- Around line 35-41: The classification currently only special-cases role_type
== "vocal" and otherwise ignores role_type, so roles whose hint is in role_type
get misclassified; update the logic in separator.py (the block using role_type,
role_id, role_name, _ROLE_TO_STEM and StemCategory) to include role_type in the
matching input (e.g., normalize role_type and append it to search_text or check
role_type tokens against _ROLE_TO_STEM) before iterating over _ROLE_TO_STEM,
ensuring role_type-derived keywords can map to the correct StemCategory rather
than falling through to OTHER.
In `@services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py`:
- Around line 37-40: The current check uses Path(audio_path).exists() which
allows directories or special files; change the validation to ensure the path is
a regular file by using Path(audio_path).is_file() (after converting audio_path
to a Path once, e.g., Path(audio_path) assigned to a variable) and raise the
same FileNotFoundError (or a clearer error) with path_str if is_file() is False;
update the code around the variables audio_path, path_str and the
FileNotFoundError in analyzer.py to perform this stricter check before passing
the path to the decoder.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Line 24: Remove the debug print statement print("RESULT:", result) from the
test_chord_recognizer.py test (it pollutes CI logs); instead rely on existing
assertions or add a descriptive assertion message on the failing assert in the
test function that references result so failures convey needed info without
printing to stdout.
In `@services/analysis-engine/tests/test_cli.py`:
- Around line 251-260: Add a Path type annotation to the tmp_path parameter in
the failing test functions (e.g., test_cli_main_job_arg_invalid_file) so
signatures become tmp_path: Path; import Path from pathlib at the top of the
test module if not already present; update the other test functions referenced
(the ones at the same diff group) to match existing tests that already annotate
tmp_path for consistency.
In `@services/analysis-engine/tests/test_pitch_tracker.py`:
- Around line 100-107: Update the test docstring to describe the test's purpose
instead of repeating the function name: in test_pitch_tracker_none_f0 replace
the current docstring with a meaningful description such as "Test pitch tracking
when librosa.pyin returns None for the f0 array" so it's clear this verifies
PitchTracker.track behavior when librosa.pyin yields no f0 values; reference
PitchTracker and the librosa.pyin patch in the test to ensure context is
preserved.
In `@services/analysis-engine/tests/test_roles_ml.py`:
- Around line 42-47: The mock side_effect_recognize currently returns dicts with
keys "start" and "end" which do not match the TrackedChord schema; update
side_effect_recognize to return dicts using the actual schema keys "start_time"
and "end_time" (and keep "chord") so the mocked responses match TrackedChord;
locate and change the two branches in side_effect_recognize (the cases for
bass_stem and other_stem) to use "start_time" and "end_time" instead of "start"
and "end".
---
Outside diff comments:
In `@apps/desktop/src/App.tsx`:
- Around line 136-137: The call to setSelectionError currently uses
selection.error.message directly and can set an empty/undefined message if the
backend omits message; update the logic around setSelectionError (the code
handling selection and selection.error.message) to provide a fallback string
(e.g., "Failed to import YouTube video") when selection.error.message is falsy,
mirroring the local audio handling so the UI never shows an empty error state.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 108-115: The invokeAnalysis function currently accepts open-ended
command: string and args?: Record<string, unknown>, which defeats the allowlist
and schema guarantees; change it to a discriminated union (or mapped type) of
allowed commands and their exact arg shapes (e.g., type AnalysisCommand = {
command: 'analyzeFile'; args: { path: string } } | { command: 'summarize'; args:
{ text: string } } ), update invokeAnalysis signature to accept that union and
narrow the return type per command, and propagate the stricter types to
getInvoke and browserFallback (or provide overloaded signatures) so only
allowlisted command names are accepted; also add runtime validation for each
command's args (using existing validator utilities or a small zod/io-ts schema
per command) before calling the IPC to enforce schema at runtime.
- Around line 179-193: Validate and sanitize the YouTube URL in importYoutubeUrl
before calling invokeAnalysis: ensure the URL uses https and the hostname
matches allowed YouTube domains (e.g., youtube.com, youtu.be) and reject/return
a safe error for invalid inputs; also stop returning raw internal error text
from the catch block of importYoutubeUrl — map known error types/messages from
invokeAnalysis (or the caught error) to a small allowlist of user-facing
messages and return a generic, non-sensitive message for all other failures
while logging the original error internally for debugging; reference function
names importYoutubeUrl and invokeAnalysis and the caught variable error when
implementing these checks and mappings.
In `@packages/shared-types/src/index.ts`:
- Around line 888-932: The validateRehearsalSection function currently validates
shapes but not referential integrity between value.partGraph and value.roles:
add checks after the existing partGraph validation loop to build a set/map of
roles' ids from value.roles (ensure all roles have string id), verify every
partGraph node's role_id and any handoff_* fields reference an existing role id,
and ensure there are no duplicate role ids in roles[].id; report errors via
invalidField using the same path format (e.g., `${path}.roles[${i}].id`,
`${path}.partGraph[${j}].role_id`, `${path}.partGraph[${j}].handoff_*`) when a
missing/duplicate reference is found so malformed graphs fail validation early.
In `@services/analysis-engine/tests/test_release_packaging.py`:
- Around line 121-167: The test
test_release_packaging_main_writes_arch_specific_manifest creates a raw binary
at target/.../release/bandscope-desktop but the packaging.main() now looks for
installers under bundle/dmg/*.dmg (mac) or bundle/nsis/*.exe (windows), so
update the test setup to create the expected installer(s) instead of the old
binary path: in the test function adjust the created artifact path using the
BANDSCOPE_TARGET_TRIPLE/BANDSCOPE_ARTIFACT_OS to create
repo_root/.../target/.../release/bundle/dmg/<something>.dmg for macos (and
similarly bundle/nsis/<something>.exe for windows if needed), ensure the file
contains some bytes, then keep the same env vars and assertions so
packaging.main() finds the installer and writes the manifest.
🪄 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: d136a2a0-b9e2-4960-9645-5fc01ebf26a2
⛔ Files ignored due to path filters (3)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (78)
.github/workflows/bandit.yml.github/workflows/build-baseline.yml.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/ossf-scorecard.yml.github/workflows/sbom.yml.github/workflows/security-audit.yml.github/workflows/trivy.yml.gitignoreCHANGELOG.mdREADME.mdSECURITY.mdVERSIONapps/desktop/src-tauri/.cargo/audit.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/features/chords/index.tsxapps/desktop/src/features/home/index.tsxapps/desktop/src/features/player/index.tsxapps/desktop/src/features/ranges/index.tsxapps/desktop/src/features/settings/index.tsxapps/desktop/src/features/workspace/ConfidenceBadge.tsxapps/desktop/src/features/workspace/RoleSwitcher.tsxapps/desktop/src/features/workspace/SectionRoadmap.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/features/workspace/WorkspaceStates.tsxapps/desktop/src/i18n/index.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/export.test.tsapps/desktop/src/lib/export.tsdocs/plans/2026-03-28-ml-engine-integration.mddocs/plans/2026-04-28-pr-159-rollout.mddocs/security/dependency-policy.mdeslint.config.jspackage.jsonpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsscripts/checks/security_gates.pyscripts/checks/verify_docs.pyscripts/checks/verify_github_bootstrap_policy.pyscripts/checks/verify_security_notes.pyscripts/checks/verify_supply_chain.pyscripts/fix-version-format.shscripts/release/package_desktop_artifact.pyservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/chords/__init__.pyservices/analysis-engine/src/bandscope_analysis/chords/analyzer.pyservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/chords/model.pyservices/analysis-engine/src/bandscope_analysis/cli.pyservices/analysis-engine/src/bandscope_analysis/ranges/__init__.pyservices/analysis-engine/src/bandscope_analysis/ranges/analyzer.pyservices/analysis-engine/src/bandscope_analysis/ranges/model.pyservices/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.pyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/src/bandscope_analysis/separation/__init__.pyservices/analysis-engine/src/bandscope_analysis/separation/model.pyservices/analysis-engine/src/bandscope_analysis/separation/separator.pyservices/analysis-engine/src/bandscope_analysis/temporal/__init__.pyservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.pyservices/analysis-engine/src/bandscope_analysis/temporal/model.pyservices/analysis-engine/src/bandscope_analysis/youtube.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_chords.pyservices/analysis-engine/tests/test_cli.pyservices/analysis-engine/tests/test_pitch_tracker.pyservices/analysis-engine/tests/test_priority.pyservices/analysis-engine/tests/test_ranges.pyservices/analysis-engine/tests/test_release_packaging.pyservices/analysis-engine/tests/test_roles_ml.pyservices/analysis-engine/tests/test_sections.pyservices/analysis-engine/tests/test_separation.pyservices/analysis-engine/tests/test_supply_chain_policy.pyservices/analysis-engine/tests/test_temporal.pyservices/analysis-engine/tests/test_tuning.pyservices/analysis-engine/tests/test_youtube.py
💤 Files with no reviewable changes (3)
- scripts/checks/verify_security_notes.py
- scripts/checks/verify_github_bootstrap_policy.py
- scripts/checks/verify_docs.py
✅ Actions performedComments resolved and changes approved. |
20 tasks
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.
Users couldn't easily run the app from a zip file containing the raw binaries and frontend files separately. This runs the
tauri buildcommand correctly to generate macOS DMG and Windows MSI/EXE.