fix: resolve CI failures for linting and vulnerabilities#149
Merged
Conversation
- Added ChordRecognizer using librosa's chromagrams to extract chords from stems. - Added PitchTracker using librosa's pYIN/YIN to determine pitch ranges. - Updated RoleExtractor to use real stems and these new DSP components to inject actual chords and ranges into roles. - Reached 100% test coverage with extensive mocking of librosa functions to simulate error conditions. - Resolved type hinting and formatting issues.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTauri 폴링 기반 분석 상태를 워크스페이스 모델로 전환하고, workspace 타입·검증과 Job Runner 브리지, Pitch/Chord 분석기·통합, 관련 테스트·UI(Stem Player) 및 테스트·빌드 설정을 추가·수정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as Desktop App
participant Bridge as Job Runner
participant Backend as Tauri Backend
participant Mock as Browser Mock
participant Workspace as Workspace State
User->>App: 로컬 오디오 / YouTube / 데모 요청
App->>Bridge: enqueueSong(request)
alt isTauri()
Bridge->>Backend: invoke('enqueue_song')
Backend->>Workspace: 워크스페이스 업데이트 (queued→analyzing→ready)
Backend->>Bridge: emit "workspace-updated"
else 브라우저 폴백
Bridge->>Mock: 업데이트 in-memory workspace (queued→analyzing→ready)
Mock->>Bridge: notify subscribers
end
Bridge->>App: 구독 콜백으로 워크스페이스 전달
App->>App: selectedPackId 업데이트 → UI 재렌더(팩 리스트/상태/팩 상세)
App->>User: 상태 및 결과 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
seonghobae
force-pushed
the
fix/issue-148-ci-pipeline
branch
from
eec66e8 to
1eb6225
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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/App.test.tsx`:
- Around line 236-239: The test uses an invalid packState value "unknown" which
is not part of the PackState union, so update the test data in App.test.tsx (the
array containing { id: "p3", packState: "unknown", sourceLabel: "Song 3" }) to
either use a valid PackState (e.g., "queued") if you meant a real state, or if
you intentionally need an out-of-type value to exercise the progressMessage
default branch, make that explicit by casting it to the type (e.g., force a
PackState cast) or change the test to simulate the default branch another way;
locate the test data array in App.test.tsx and apply the change to remove the
type mismatch.
- Around line 393-402: The test "covers saveProject early return when no ready
pack" uses a fragile pause via await new Promise(r => setTimeout(r, 0)); replace
that with an explicit async wait that asserts the expected post-click state —
e.g., use waitFor to wait for a UI change or absence of side effects after
fireEvent.click(screen.getByRole("button", { name: /Save Project/i }));
specifically remove the setTimeout promise and call waitFor(() => { /* assert
the expected condition: no save modal, no network call, or a toast message
absent/present */ }) targeting the elements/functions exercised in this test so
the test reliably waits for the asynchronous behavior to finish.
- Around line 299-307: The test "covers loadProject User cancelled exception" is
a duplicate of the existing "covers handles loadProject cancellation" case;
remove the redundant test block (the one that uses
mockLoadProject.mockRejectedValueOnce(new Error("User cancelled")) and renders
<App /> then clicks the "open project" button) so only one cancellation scenario
remains, or consolidate both into a single test; locate the duplicates by the
test titles and the use of mockLoadProject and ensure only one test asserting
that "Failed to load project" is not shown remains.
In `@apps/desktop/src/App.tsx`:
- Around line 132-145: The code is using a hardcoded workspace because
loadProject still returns the old shape; update the loadProject adapter to
return a RehearsalWorkspace and remove the temporary hardcoded object used with
setWorkspace, or if that cannot be done now, create a tracked issue and add a
clear TODO comment referencing loadProject and RehearsalWorkspace so this
migration isn't lost; specifically, change the loadProject implementation (and
its call site in App.tsx where setWorkspace is invoked) to produce a
RehearsalWorkspace (including songs/packState/sourceLabel fields) or add an
issue/PR link and a TODO near the current hardcoded block to ensure the work is
picked up later.
- Around line 60-75: subscribeToWorkspaceUpdates의 Promise가 resolve되기 전에 컴포넌트가
언마운트되면 unlisten이 undefined인 상태로 cleanup이 실행되는 경쟁 상태가 있으니 이를 방지하세요: useEffect
내부에서 unlisten을 지역 변수로 두지 말고 useRef로 참조(unlistenRef)하거나 unlisten을 반환하는
Promise(unlistenPromise)를 저장하고, subscribeToWorkspaceUpdates(...).then(u => { if
(!unmounted) { unlistenRef.current = u; } }); 형태로 마운트 상태(unmounted 플래그 또는
isMounted ref)를 체크해 setWorkspace와 unlisten 할당을 안전하게 처리하고, cleanup 함수에서는
unmounted를 설정한 뒤 unlistenRef.current가 있으면 호출하고 없다면 unlistenPromise.then(u => u
&& u())로 Promise가 나중에 resolve되어도 정리되게 하세요; 관련 식별자: useEffect,
subscribeToWorkspaceUpdates, getWorkspaceState, unlisten (또는 unlistenRef /
unlistenPromise).
- Around line 84-89: The call to enqueueSong is fire-and-forget and lacks error
handling; update the enqueueSong(...) invocation to handle promise rejections
(e.g., add .catch(...)) so failures are logged and the user receives feedback;
in the .catch, log the error (console.error or your logger) including context
(selection.bootstrap.projectId, selection.bootstrap.source.fileName,
defaultRequest.roleFocus) and surface a user-facing notification (e.g., show a
toast/snackbar or set an error state) so IPC/enqueue failures are visible to the
user.
In `@apps/desktop/src/features/workspace/Workspace.tsx`:
- Around line 89-96: The placeholder stem-player buttons rendered when
activeRole is set (the Play button and the two icon-only buttons labeled "▶
Play", "🔁 Loop Section", "🔇 Mute Others (Solo)" in Workspace.tsx) lack
accessibility hints and affordance for unimplemented behavior; update these
button elements to include descriptive aria-label attributes (e.g.,
aria-label="Play stem", "Loop section", "Solo/mute others") and mark them as
disabled (disabled={true}) or add a visible "Coming soon"
tooltip/aria-describedby so keyboard and screen-reader users know they are
non-functional, and ensure the Play button and the icon-only buttons use the
same accessible attributes when activeRole is present.
In `@apps/desktop/src/lib/job_runner.ts`:
- Around line 84-92: The current retry handler (command === "retry_song") only
resets pack.packState/engineState/error and calls triggerMockUpdate, so the pack
never progresses to analyzing/ready; change it to call the same
simulation/enqueue path used by the existing enqueue logic (reuse the
enqueue_song flow or extract and invoke a shared helper, e.g., the function that
enqueues or simulates processing) instead of only flipping states—locate the
retry block that finds pack via mockWorkspace.songs.find(p => p.id === jobId)
and replace the state-only fallback with a call to the enqueue/simulation helper
(ensuring triggerMockUpdate still runs) so the retried pack is re-scheduled and
progresses to analyzing/ready.
- Around line 23-28: getInvoke currently always returns the imported invoke
which causes Tauri code paths to run in the browser; import isTauri from
'@tauri-apps/api/core' and change getInvoke to return invoke only when isTauri()
is true (otherwise return null or undefined), then update
subscribeToWorkspaceUpdates (and any call sites like invokeRunner(),
browserFallback(), mockListeners) to check isTauri() (or the getInvoke result)
and take the Tauri branch only when isTauri() is true so the pure-browser
fallback and mockListeners become reachable.
In `@docs/plans/2026-03-28-ml-engine-integration.md`:
- Around line 20-21: 문서의 제목 줄 "### Track 3: Harmonic & Pitch Pipelines (`#107`)
(COMPLETED)" 바로 아래에 빈 줄이 빠져 있어 markdownlint 규칙(MD022) 위반이 발생합니다; 해당 제목과 다음 목록(예:
"- **Goal**: Replace hardcoded `C#m7` strings with DSP-derived chord and pitch
arrays.") 사이에 한 줄 공백을 추가하여 제목 아래 빈 줄을 삽입해 주세요.
In `@packages/shared-types/src/index.ts`:
- Around line 119-126: Replace the loose SongRehearsalPack object type with a
discriminated union keyed on packState so each state enforces required/forbidden
fields (e.g., {packState: "ready", song: RehearsalSong, engineState?: ... ,
error?: never}, {packState: "failed", error: AnalysisJobError, song?: never},
{packState: "pending" | "processing", engineState: AnalysisJobState, song?:
never, error?: never}, etc.), update any related exported types that mirror this
shape (the other occurrences you flagged) and add a runtime invariant/validator
used by apps/desktop/src/lib/job_runner.ts IPC handler to assert the
discriminant and field presence/absence (throw or return validation error)
before accepting the payload. Ensure references to AnalysisJobState, PackState,
RehearsalSong, and AnalysisJobError are used in the union branches and exported
so static and runtime checks both enforce the state invariants.
In `@services/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.py`:
- Around line 41-44: The try/except around librosa.pyin is too broad; replace
the bare except Exception with catching the specific
librosa.util.exceptions.ParameterError (import it), and log the exception
details (e.g., using the module logger) before returning the fallback
{"lowest_note": None, "highest_note": None, "confidence": "low"} so failures due
to bad fmin/fmax or input are visible; keep the same return behavior for that
specific error but do not swallow other unexpected exceptions.
In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py`:
- Around line 65-79: PitchTracker.track() can return a dict with None notes, so
currently vocal_range and bass_range get overwritten with empty strings; change
the assignments in the branches using p_res (for "vocals" and "bass") to only
override vocal_range/bass_range when both p_res["lowest_note"] and
p_res["highest_note"] are present (not None/empty). In other words, after p_res
truthiness check, add a guard that both notes are non-None (or truthy) before
setting the dict (variables: pitch_tracker.track, p_res, vocal_range,
bass_range, stems).
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Around line 21-26: Remove the debug print that emits test output to CI logs:
delete the print("RESULT:", result) call in the test_chord_recognizer test so
recognizer.recognize(y, sr=22050) runs silently; if you need evidence on
failure, replace it with a more specific assertion message on the existing
assert or use the test framework's logging/capture facilities to record `result`
rather than printing directly from the test.
In `@services/analysis-engine/tests/test_pitch_tracker.py`:
- Around line 75-77: 테스트 독스트링이 함수 이름을 그대로 반복하고 있어 의도를 알기 어려우니
test_pitch_tracker_pyin_exception, test_pitch_tracker_few_frames,
test_pitch_tracker_none_f0의 도큐스트링을 각 테스트의 목적과 기대 동작(예: 어떤 예외를 기대하는지, 어떤 입력 상황을
시뮬레이션하는지, 반환값이나 상태 변화의 기대치)을 한두 문장으로 명확히 설명하도록 수정하세요; 관련 식별자로는 해당 함수 이름들과
PitchTracker 인스턴스 생성 코드(PitchTracker())를 참고하고, 독스트링에 실패 시 예상되는 예외 타입/메시지 또는 검증할
조건을 포함해 유지보수성과 테스트 판독성을 높이세요.
🪄 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: f414b5e5-163f-4b06-8613-e6be1072cb06
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.gitignoreapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/lib/job_runner.tsapps/desktop/vite.config.tsdocs/plans/2026-03-28-ml-engine-integration.mdpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsservices/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.pyservices/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.pyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_pitch_tracker.pyservices/analysis-engine/tests/test_roles_ml.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
services/analysis-engine/src/bandscope_analysis/roles/extractor.py (1)
67-79:⚠️ Potential issue | 🟠 Major
track()저신뢰 결과가 기본 음역대를 빈 문자열로 덮어씁니다.Line 67/75의 현재 분기는
p_res가 dict이기만 하면 통과해서,Nonenote가""로 치환되어 기본 fallback range가 깨집니다. 두 note가 모두 유효할 때만 override 하세요.🔧 제안 수정
if "vocals" in stems: p_res = pitch_tracker.track(stems["vocals"], sr=sr) - if p_res: + if p_res and p_res.get("lowest_note") and p_res.get("highest_note"): vocal_range = { - "lowestNote": p_res["lowest_note"] or "", - "highestNote": p_res["highest_note"] or "", + "lowestNote": p_res["lowest_note"], + "highestNote": p_res["highest_note"], } if "bass" in stems: p_res = pitch_tracker.track(stems["bass"], sr=sr) - if p_res: + if p_res and p_res.get("lowest_note") and p_res.get("highest_note"): bass_range = { - "lowestNote": p_res["lowest_note"] or "", - "highestNote": p_res["highest_note"] or "", + "lowestNote": p_res["lowest_note"], + "highestNote": p_res["highest_note"], }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py` around lines 67 - 79, The current logic overwrites default vocal_range and bass_range whenever pitch_tracker.track(...) returns a dict even if its notes are None, replacing valid defaults with empty strings; change the condition to only update vocal_range and bass_range when p_res is truthy AND both p_res["lowest_note"] and p_res["highest_note"] are non-empty/non-None values (e.g., check both keys explicitly) so that track() low-confidence results do not clobber the fallback ranges; apply this fix for the blocks that call pitch_tracker.track(...) for the vocal stem and the "bass" stem.services/analysis-engine/tests/test_chord_recognizer.py (1)
24-24:⚠️ Potential issue | 🟡 Minor테스트의 디버그 출력은 제거하세요.
Line 24의
🔧 제안 수정
- print("RESULT:", result)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/analysis-engine/tests/test_chord_recognizer.py` at line 24, Remove the debug print statement in the test by deleting the line that calls print("RESULT:", result) in test_chord_recognizer.py (the stray print inside the test function that outputs `result`), ensuring the test no longer emits debug output to CI logs; if the output was intended for verification, replace it with an appropriate assertion against `result` instead.services/analysis-engine/tests/test_pitch_tracker.py (1)
75-102: 🧹 Nitpick | 🔵 Trivial독스트링이 함수명을 반복하고 있어 의도 전달력이 낮습니다.
Line 76, Line 87, Line 101의 독스트링을 테스트 목적/기대 동작 중심으로 바꾸면 가독성이 좋아집니다.
📝 제안 수정
def test_pitch_tracker_pyin_exception(): - """Test for test_pitch_tracker_pyin_exception.""" + """librosa.pyin 예외 발생 시 low-confidence 기본값으로 안전하게 fallback 되는지 검증한다.""" @@ def test_pitch_tracker_few_frames(): - """Test for test_pitch_tracker_few_frames.""" + """voiced frame이 매우 적을 때(min/max 분기)도 note 추적이 가능한지 검증한다.""" @@ def test_pitch_tracker_none_f0(): - """Test for test_pitch_tracker_none_f0.""" + """pyin이 f0=None을 반환할 때 None note 결과를 반환하는지 검증한다."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/analysis-engine/tests/test_pitch_tracker.py` around lines 75 - 102, The docstrings for the three tests (test_pitch_tracker_pyin_exception, test_pitch_tracker_few_frames, test_pitch_tracker_none_f0) simply repeat the function names and should be replaced with descriptive statements of the test intent and expected behavior; update each docstring to briefly describe what is being tested and the expected outcome (e.g., "When librosa.pyin raises an exception, tracker.track should return None for lowest_note and highest_note" for test_pitch_tracker_pyin_exception, "With very short audio (~0.1s) the tracker should still return a lowest_note when voiced frames are few" for test_pitch_tracker_few_frames, and a similar expectation for test_pitch_tracker_none_f0), leaving the test code unchanged.apps/desktop/src/App.tsx (2)
84-89:⚠️ Potential issue | 🟠 Major모든 enqueue 경로가 실패를 삼키고 있습니다.
세 경로 모두
enqueueSong()을 fire-and-forget으로 호출해서 IPC 실패나 fallback 오류가 나도 UI에는 아무 표시가 없습니다. 공통 helper로 묶어서await/.catch()후selectionError나workspaceError를 세팅해야 합니다.🔧 제안 수정
+ const enqueueWithFeedback = async ( + request: Parameters<typeof enqueueSong>[0], + fallbackMessage: string, + ) => { + try { + await enqueueSong(request); + } catch (error) { + setSelectionError( + error instanceof Error && error.message ? error.message : fallbackMessage, + ); + } + }; @@ - enqueueSong({ + await enqueueWithFeedback({ sourceKind: "local_audio", projectId: selection.bootstrap.projectId, sourceLabel: selection.bootstrap.source.fileName, roleFocus: defaultRequest.roleFocus - }); + }, t("unsupportedLocalAudio")); return; @@ - enqueueSong({ + await enqueueWithFeedback({ sourceKind: "local_audio", projectId: selection.bootstrap.projectId, sourceLabel: "YouTube Import", roleFocus: defaultRequest.roleFocus - }); + }, t("youtubeImportFailed")); @@ - enqueueSong(defaultRequest); + void enqueueWithFeedback(defaultRequest, t("analysisStateFailed")); };Also applies to: 104-109, 124-126
🤖 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 84 - 89, All calls to enqueueSong are being invoked fire-and-forget and swallowing IPC/fallback failures; wrap these calls in a shared async helper that awaits enqueueSong and catches errors, then set the appropriate state (selectionError or workspaceError) with the caught error; update every place that calls enqueueSong (the blocks around enqueueSong with selection.bootstrap and other enqueue paths) to call this helper and ensure UI-visible error state is set on failure.
60-75:⚠️ Potential issue | 🟠 Major비동기 구독 setup이 언마운트와 경합합니다.
subscribeToWorkspaceUpdates()가 resolve 되기 전에 cleanup이 먼저 돌면unlisten을 잃어버리고, 이후 등록된 listener가 남습니다. 같은 이유로getWorkspaceState()도 언마운트 뒤에setWorkspace를 호출할 수 있으니 mounted 플래그와 late-resolve cleanup이 필요합니다.🔧 제안 수정
useEffect(() => { - let unlisten: (() => void) | undefined; - subscribeToWorkspaceUpdates((ws) => { - setWorkspace(ws); - }).then(u => { - unlisten = u; - }); - - getWorkspaceState().then(ws => { - if (ws) setWorkspace(ws); - }); + let isMounted = true; + let unlisten: (() => void) | undefined; + + const unlistenPromise = subscribeToWorkspaceUpdates((ws) => { + if (isMounted) { + setWorkspace(ws); + } + }).then((u) => { + if (isMounted) { + unlisten = u; + } else { + u(); + } + return u; + }); + + getWorkspaceState().then((ws) => { + if (ws && isMounted) { + setWorkspace(ws); + } + }); return () => { - if (unlisten) unlisten(); + isMounted = false; + if (unlisten) { + unlisten(); + } else { + void unlistenPromise.then((u) => u()); + } }; }, []);🤖 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 60 - 75, subscribeToWorkspaceUpdates and getWorkspaceState resolve asynchronously and can race with the effect cleanup, losing the unlisten callback or calling setWorkspace after unmount; fix by adding a mounted boolean flag and handling late-resolve cleanup: when calling subscribeToWorkspaceUpdates(), capture its returned promise (and when it resolves to an unlisten function, if mounted assign it to unlisten else call it immediately), and for getWorkspaceState() only call setWorkspace if mounted; in the effect cleanup set mounted = false and if unlisten is already set call it; also handle promise rejections to avoid unhandled errors.apps/desktop/src/lib/job_runner.ts (2)
84-92:⚠️ Potential issue | 🟡 Minor
retry_song이 mock 재처리를 다시 시작하지 않습니다.여기서는 상태만
queued로 되돌리고 끝나서, 브라우저 fallback에서는 재시도한 pack이 다시analyzing/ready로 진행되지 않습니다.enqueue_song과 같은 시뮬레이션 helper를 재사용하도록 묶어야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/job_runner.ts` around lines 84 - 92, The retry_song branch only resets pack fields (pack.packState, pack.engineState, pack.error) but doesn't start the simulated processing pipeline, so packs never move to analyzing/ready; update the "retry_song" handler to call the same simulation helper used by enqueue_song (reuse the enqueue_song or its underlying simulation function) for the found pack after resetting fields, ensuring you still call triggerMockUpdate() and any timers/promises the enqueue helper uses so the mock pack progresses through analyzing -> ready as expected.
23-28:⚠️ Potential issue | 🟠 Major브라우저 fallback이 여전히 도달 불가능합니다.
typeof window !== "undefined"만으로는 Tauri 런타임을 판별할 수 없습니다. 지금 구현은 순수 브라우저에서도invoke/listen경로를 타서browserFallback()과mockListeners가 사실상 죽어 있습니다.isTauri()로 Tauri 경로를 명시적으로 gate 해야 합니다.🔧 제안 수정
-import { invoke } from "@tauri-apps/api/core"; +import { invoke, isTauri } from "@tauri-apps/api/core"; import { listen, type UnlistenFn } from "@tauri-apps/api/event"; @@ function getInvoke(): TauriInvoke | null { - if (typeof window === "undefined") { + if (typeof window === "undefined" || !isTauri()) { return null; } return window.__TAURI_INVOKE__ ?? invoke; }In `@tauri-apps/api` v2, what is the recommended runtime check before calling invoke() or listen() so code can safely fall back in a plain browser environment?Also applies to: 107-113, 131-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/job_runner.ts` around lines 23 - 28, The runtime check is wrong: replace the window existence check in getInvoke() with an explicit Tauri runtime guard (use isTauri() from `@tauri-apps/api/tauri`) so plain browsers use the browserFallback and mockListeners; specifically, change getInvoke() to return invoke only when isTauri() is true, otherwise return null, and apply the same pattern for the listen-related code paths (the code that currently calls listen/returns Tauri listeners) so browserFallback() and mockListeners are reachable.apps/desktop/src/App.test.tsx (3)
249-256:⚠️ Potential issue | 🟡 Minor동일한 취소 시나리오를 두 번 테스트하고 있습니다.
두 케이스 모두
loadProject가"User cancelled"를 던질 때 에러 메시지가 보이지 않는지만 확인합니다. 하나로 합치지 않으면 이후 변경 시 두 테스트를 같이 고쳐야 합니다.Also applies to: 299-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` around lines 249 - 256, Two identical tests check that when mockLoadProject.rejects with "User cancelled" no error is shown; consolidate them into one to avoid duplicated maintenance. Keep the test that stubs mockLoadProject.mockRejectedValueOnce(new Error("User cancelled")), renders <App /> and clicks the "open project" button (fireEvent.click(screen.getByRole("button", { name: /open project/i }))) and retains the assertion await waitFor(() => expect(screen.queryByText(/Failed to load project/i)).toBeNull()); remove the duplicate test (the other block referencing the same mock behavior) or merge any unique setup into this single test to cover the cancellation scenario.
8-11:⚠️ Potential issue | 🟡 Minor
any때문에 불가능한packState를 정상 케이스처럼 테스트하고 있습니다.
mockWorkspaceStore: any가 있어서"unknown"같은 계약 밖 값을 그대로 밀어 넣고 있습니다. 이 테스트는 실제SongRehearsalPack스키마를 검증하지 못하므로, mock store/subscriber를 타입으로 고정하고 비정상 payload를 시험해야 한다면 그 케이스만 명시적으로 분리하세요.#!/bin/bash # Verify the shared pack-state contract and the out-of-contract fixture used here. rg -n --type=ts 'type .*PackState|packState|SongRehearsalPack' packages/shared-types/src apps/desktop/src/App.test.tsxAlso applies to: 230-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` around lines 8 - 11, The test uses untyped mocks (mockWorkspaceStore and workspaceSubscribers) allowing out-of-contract values like "unknown" to masquerade as valid packState; fix by importing the shared SongRehearsalPack / PackState types and give mockWorkspaceStore and workspaceSubscribers explicit types matching the store/subscriber signatures (replace any with the correct PackState/Subscriber types), then update the fixture to use a valid SongRehearsalPack instance for the normal test case and move any out-of-contract payload checks into a separate negative test that asserts validation errors; update references to packState in App.test.tsx and ensure assertions validate the real schema instead of relying on any.
400-401:⚠️ Potential issue | 🟡 Minor
setTimeout(0)로 비동기 완료를 가정하면 테스트가 쉽게 헛돌 수 있습니다.이 세 케이스는 클릭 후 한 틱만 기다리고 끝나서, 실제로
saveProject가 호출되지 않았는지/오류가 표시되지 않았는지 같은 결과를 검증하지 못합니다.waitFor로 명시적인 불변식 하나를 기다리도록 바꾸는 편이 안정적입니다.Also applies to: 419-420, 427-428
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` around lines 400 - 401, Replace the fragile "await new Promise(r => setTimeout(r, 0))" tick-wait with an explicit waitFor assertion so the test waits for a concrete invariant; after firing the click on the Save Project button (fireEvent.click(screen.getByRole(...))), use waitFor to assert the expected outcome (e.g., waitFor that the mocked saveProject function has been called via expect(saveProject).toHaveBeenCalled() or that an error/success message appears via expect(screen.getByText(...)).toBeInTheDocument()); make the same replacement for the other two occurrences referenced (the other clicks at the noted locations) so each test waits for a specific condition rather than a single event loop tick.
🤖 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/App.tsx`:
- Around line 157-162: The Save Project button currently only sets aria-disabled
and doesn't reflect actual saveability; compute a boolean like hasReadyPack =
Boolean(workspace && workspace.songs.find(s => s.packState === "ready" &&
s.song)) and use that to set the button's disabled prop (and keep aria-disabled
in sync) and guard the handler (handleSaveProject) early using the same
condition (instead of only checking workspace). Update any duplicate UI logic
around the other Save Project instance (the similar block at the 210-224 region)
to use the same hasReadyPack check so the button is both visually and
functionally disabled when no readyPack exists.
In `@services/analysis-engine/tests/test_roles_ml.py`:
- Around line 88-99: The test currently mocks PitchTracker.track() to return
None but misses the real low-confidence return shape (e.g., {"lowest_note":
None, "highest_note": None, ...}), so add an additional mock case for
PitchTracker.track in test_roles_ml.py that returns the low-confidence dict
shape and assert extractor.extract(sections, audio_features) still produces the
expected fallback values (inspect usage in extract() and assertions around
intro_topology/active_roles to mirror existing checks for "lead-vocal" range and
harmony); update the mock setup for mock_track (and reuse mock_recognize) to
include both None and the dict-return scenario to cover the fallback branch
invoked by the extractor.
---
Duplicate comments:
In `@apps/desktop/src/App.test.tsx`:
- Around line 249-256: Two identical tests check that when
mockLoadProject.rejects with "User cancelled" no error is shown; consolidate
them into one to avoid duplicated maintenance. Keep the test that stubs
mockLoadProject.mockRejectedValueOnce(new Error("User cancelled")), renders <App
/> and clicks the "open project" button
(fireEvent.click(screen.getByRole("button", { name: /open project/i }))) and
retains the assertion await waitFor(() => expect(screen.queryByText(/Failed to
load project/i)).toBeNull()); remove the duplicate test (the other block
referencing the same mock behavior) or merge any unique setup into this single
test to cover the cancellation scenario.
- Around line 8-11: The test uses untyped mocks (mockWorkspaceStore and
workspaceSubscribers) allowing out-of-contract values like "unknown" to
masquerade as valid packState; fix by importing the shared SongRehearsalPack /
PackState types and give mockWorkspaceStore and workspaceSubscribers explicit
types matching the store/subscriber signatures (replace any with the correct
PackState/Subscriber types), then update the fixture to use a valid
SongRehearsalPack instance for the normal test case and move any out-of-contract
payload checks into a separate negative test that asserts validation errors;
update references to packState in App.test.tsx and ensure assertions validate
the real schema instead of relying on any.
- Around line 400-401: Replace the fragile "await new Promise(r => setTimeout(r,
0))" tick-wait with an explicit waitFor assertion so the test waits for a
concrete invariant; after firing the click on the Save Project button
(fireEvent.click(screen.getByRole(...))), use waitFor to assert the expected
outcome (e.g., waitFor that the mocked saveProject function has been called via
expect(saveProject).toHaveBeenCalled() or that an error/success message appears
via expect(screen.getByText(...)).toBeInTheDocument()); make the same
replacement for the other two occurrences referenced (the other clicks at the
noted locations) so each test waits for a specific condition rather than a
single event loop tick.
In `@apps/desktop/src/App.tsx`:
- Around line 84-89: All calls to enqueueSong are being invoked fire-and-forget
and swallowing IPC/fallback failures; wrap these calls in a shared async helper
that awaits enqueueSong and catches errors, then set the appropriate state
(selectionError or workspaceError) with the caught error; update every place
that calls enqueueSong (the blocks around enqueueSong with selection.bootstrap
and other enqueue paths) to call this helper and ensure UI-visible error state
is set on failure.
- Around line 60-75: subscribeToWorkspaceUpdates and getWorkspaceState resolve
asynchronously and can race with the effect cleanup, losing the unlisten
callback or calling setWorkspace after unmount; fix by adding a mounted boolean
flag and handling late-resolve cleanup: when calling
subscribeToWorkspaceUpdates(), capture its returned promise (and when it
resolves to an unlisten function, if mounted assign it to unlisten else call it
immediately), and for getWorkspaceState() only call setWorkspace if mounted; in
the effect cleanup set mounted = false and if unlisten is already set call it;
also handle promise rejections to avoid unhandled errors.
In `@apps/desktop/src/lib/job_runner.ts`:
- Around line 84-92: The retry_song branch only resets pack fields
(pack.packState, pack.engineState, pack.error) but doesn't start the simulated
processing pipeline, so packs never move to analyzing/ready; update the
"retry_song" handler to call the same simulation helper used by enqueue_song
(reuse the enqueue_song or its underlying simulation function) for the found
pack after resetting fields, ensuring you still call triggerMockUpdate() and any
timers/promises the enqueue helper uses so the mock pack progresses through
analyzing -> ready as expected.
- Around line 23-28: The runtime check is wrong: replace the window existence
check in getInvoke() with an explicit Tauri runtime guard (use isTauri() from
`@tauri-apps/api/tauri`) so plain browsers use the browserFallback and
mockListeners; specifically, change getInvoke() to return invoke only when
isTauri() is true, otherwise return null, and apply the same pattern for the
listen-related code paths (the code that currently calls listen/returns Tauri
listeners) so browserFallback() and mockListeners are reachable.
In `@services/analysis-engine/src/bandscope_analysis/roles/extractor.py`:
- Around line 67-79: The current logic overwrites default vocal_range and
bass_range whenever pitch_tracker.track(...) returns a dict even if its notes
are None, replacing valid defaults with empty strings; change the condition to
only update vocal_range and bass_range when p_res is truthy AND both
p_res["lowest_note"] and p_res["highest_note"] are non-empty/non-None values
(e.g., check both keys explicitly) so that track() low-confidence results do not
clobber the fallback ranges; apply this fix for the blocks that call
pitch_tracker.track(...) for the vocal stem and the "bass" stem.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Line 24: Remove the debug print statement in the test by deleting the line
that calls print("RESULT:", result) in test_chord_recognizer.py (the stray print
inside the test function that outputs `result`), ensuring the test no longer
emits debug output to CI logs; if the output was intended for verification,
replace it with an appropriate assertion against `result` instead.
In `@services/analysis-engine/tests/test_pitch_tracker.py`:
- Around line 75-102: The docstrings for the three tests
(test_pitch_tracker_pyin_exception, test_pitch_tracker_few_frames,
test_pitch_tracker_none_f0) simply repeat the function names and should be
replaced with descriptive statements of the test intent and expected behavior;
update each docstring to briefly describe what is being tested and the expected
outcome (e.g., "When librosa.pyin raises an exception, tracker.track should
return None for lowest_note and highest_note" for
test_pitch_tracker_pyin_exception, "With very short audio (~0.1s) the tracker
should still return a lowest_note when voiced frames are few" for
test_pitch_tracker_few_frames, and a similar expectation for
test_pitch_tracker_none_f0), leaving the test code unchanged.
🪄 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: e32b56be-0a35-4f0f-9b42-b88094d1b621
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
apps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/lib/job_runner.tsservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_pitch_tracker.pyservices/analysis-engine/tests/test_roles_ml.py
seonghobae
force-pushed
the
fix/issue-148-ci-pipeline
branch
from
1eb6225 to
6448595
Compare
seonghobae
force-pushed
the
fix/issue-148-ci-pipeline
branch
from
a4c7f76 to
8bc1eb0
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (6)
services/analysis-engine/tests/test_roles_ml.py (1)
88-89: 🧹 Nitpick | 🔵 Trivial저신뢰 반환 형태 테스트 케이스 누락
mock_track.return_value = None은PitchTracker.track()이 완전히 실패하는 경우만 테스트합니다. 실제 구현에서 저신뢰 결과({"lowest_note": None, "highest_note": None, "confidence": "low"})를 반환하는 경로도 테스트해야 fallback 로직의 회귀를 방지할 수 있습니다.🧪 추가 테스트 케이스 제안
+def test_role_extractor_with_low_confidence_pitch(): + """저신뢰 피치 추적 결과에 대한 fallback 동작 검증.""" + extractor = RoleExtractor() + sections = [{"id": "intro"}] + audio_features = { + "stems": {"vocals": np.zeros(1024), "bass": np.zeros(1024)}, + "sr": 22050, + } + + with ( + patch("bandscope_analysis.ranges.pitch_tracker.PitchTracker.track") as mock_track, + patch("bandscope_analysis.chords.chord_recognizer.ChordRecognizer.recognize") as mock_recognize, + ): + mock_track.return_value = { + "lowest_note": None, + "highest_note": None, + "confidence": "low", + } + mock_recognize.return_value = [] + + result = extractor.extract(sections, audio_features) + # Assert fallback values are used🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/analysis-engine/tests/test_roles_ml.py` around lines 88 - 89, 현재 테스트 only covers a complete failure (mock_track.return_value = None) but misses the low-confidence return path; add a test case where mock_track (the PitchTracker.track() mock) returns {"lowest_note": None, "highest_note": None, "confidence": "low"} and verify the same fallback behavior/assertions as for the None case (and keep/mock mock_recognize as appropriate) so the fallback logic for low-confidence results is exercised and prevents regressions.apps/desktop/src/App.test.tsx (2)
384-393:⚠️ Potential issue | 🟡 Minor이 세 테스트는 아직도 플래키하고 회귀를 거의 잡지 못합니다.
Line 392, Line 411, Line 419의
setTimeout(0)대기만으로는 조기-return 분기를 검증하지 못합니다. 지금은saveProject가 잘못 호출돼도 타이밍만 맞으면 그대로 통과할 수 있으니,waitFor(() => expect(mockSaveProject).not.toHaveBeenCalled())처럼 관찰 가능한 결과를 확인하도록 바꾸는 편이 안전합니다.Also applies to: 395-412, 414-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` around lines 384 - 393, The test currently uses an empty setTimeout to wait for the "early return when no ready pack" branch but that is flaky; update the test to observe a measurable outcome by asserting the saveProject mock was not invoked (use waitFor(() => expect(mockSaveProject).not.toHaveBeenCalled())). Locate the test that renders <App /> and fires the "Save Project" button (the block referencing saveProject/mockSaveProject in App.test.tsx) and replace the final new Promise/setTimeout(0) with a waitFor assertion that mockSaveProject is not called, ensuring the early-return branch is reliably verified.
230-246:⚠️ Potential issue | 🟠 Major
packState: "unknown" as any가 CI 실패를 다시 만들고 있습니다.Line 238은 이번 PR이 없애려는
@typescript-eslint/no-explicit-any오류를 그대로 재도입합니다. 게다가PackState계약 밖의 값을 테스트 데이터에 넣어서, 실제 앱에서 도달하지 않는 상태를 검증하게 됩니다. 이 케이스는 유효한 상태값으로 바꾸거나, 정말 fallback 분기를 확인해야 한다면 타입을 우회하지 않는 별도 단위 테스트로 분리하는 편이 맞습니다.#!/bin/bash rg -n '"unknown"\s+as any' apps/desktop/src/App.test.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` around lines 230 - 246, The test reintroduces an explicit any cast via mockWorkspaceStore.songs[x].packState = "unknown" as any; replace that with a valid PackState value (e.g., "queued" or another real state your app handles) so the test uses only contract-approved values, or if you really need to exercise the fallback branch create a separate unit test that intentionally supplies an out-of-contract value without using `as any` (e.g., build the raw props object via JSON.parse or a fixture file) and assert the fallback behavior; update the "covers missing progressMessage branches" test to remove the `"unknown" as any` usage and reference mockWorkspaceStore and the songs[].packState field when making the change.apps/desktop/src/App.tsx (2)
107-128:⚠️ Potential issue | 🟠 MajorYouTube import 성공 뒤의 enqueue 실패가 사용자에게 보이지 않습니다.
여기서는
importYoutubeUrl()만try/catch에 들어가 있고, 성공 후enqueueSong()은 기다리지도/catch하지도 않아서 실패 시 unhandled rejection으로 끝납니다. 그러면 URL 입력은 비워지는데 실제로는 곡이 추가되지 않은 상태가 됩니다.enqueueSong()까지await해서 같은 에러 경로로 보내는 편이 안전합니다.🔧 수정 예시
try { const selection = await importYoutubeUrl(youtubeUrl); if (selection.ok) { - enqueueSong({ + await enqueueSong({ sourceKind: "local_audio", projectId: selection.bootstrap.projectId, sourceLabel: "YouTube Import", roleFocus: defaultRequest.roleFocus }); setYoutubeUrl(""); } else { setSelectionError(selection.error.message); } - } catch { - setSelectionError(t("youtubeImportFailed")); + } catch (error) { + setSelectionError(error instanceof Error ? error.message : t("youtubeImportFailed")); } finally { setIsImporting(false); }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 107 - 128, handleImportYoutube currently only awaits importYoutubeUrl and calls enqueueSong without awaiting or catching errors, which can cause unhandled rejections and clear the youtubeUrl even if enqueue fails; change the flow so that after importYoutubeUrl resolves you await enqueueSong(...) and let any error bubble into the existing try/catch so setSelectionError(t("youtubeImportFailed")) runs on failure, and only clear youtubeUrl (setYoutubeUrl("")) after enqueueSong completes successfully; update references in this fix: handleImportYoutube, importYoutubeUrl, enqueueSong, setYoutubeUrl, setSelectionError, setIsImporting.
166-171:⚠️ Potential issue | 🟠 Major저장할 pack이 없어도
Save Project가 계속 클릭 가능하게 보입니다.
handleSaveProject()는 ready pack이 없으면 바로 return하는데, 버튼 쪽은 여전히workspace존재 여부만 보고aria-disabled만 바꾸고 있습니다. 그래서 저장 불가 상태에서도 버튼이 포커스/클릭 가능하고, 눌러도 아무 일도 안 일어나는 접근성/UX 불일치가 남습니다.hasReadyPack을 한 번 계산해서 handler와disabled/aria-disabled/스타일에 같이 쓰는 편이 맞습니다.🔧 수정 예시
- const selectedPack = workspace?.songs.find(s => s.id === selectedPackId); + const selectedPack = workspace?.songs.find((s) => s.id === selectedPackId); + const hasReadyPack = Boolean( + workspace?.songs.find((s) => s.packState === "ready" && s.song) + ); @@ - onClick={handleSaveProject} - aria-disabled={!workspace} + onClick={handleSaveProject} + disabled={!hasReadyPack} + aria-disabled={!hasReadyPack} style={{ padding: "8px 16px", - cursor: workspace ? "pointer" : "not-allowed", + cursor: hasReadyPack ? "pointer" : "not-allowed", borderRadius: "4px", - backgroundColor: workspace ? "#fff" : "#f5f5f5", + backgroundColor: hasReadyPack ? "#fff" : "#f5f5f5", border: "1px solid `#ccc`", - opacity: workspace ? 1 : 0.5 + opacity: hasReadyPack ? 1 : 0.5 }}Also applies to: 210-230
🤖 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 166 - 171, The save button's enabled state is inconsistent with handleSaveProject: compute a boolean like hasReadyPack by checking workspace.songs for a pack with packState === "ready" and song present, use that value both in handleSaveProject (early return) and in the button props (disabled, aria-disabled and styling) so the button is truly disabled/unfocusable when no ready pack exists; update references around handleSaveProject, readyPack and the Save Project button to use hasReadyPack for consistency (also apply same change to the other occurrence noted around lines 210-230).packages/shared-types/src/index.ts (1)
119-139:⚠️ Potential issue | 🟠 Major
packState/engineState불변식이 아직 너무 느슨합니다.지금 정의로는
packState: "ready"인데engineState: "queued"이거나,packState: "failed"인데engineState: "running"같은 모순된 payload가 타입과 validator 둘 다 통과합니다. 이 값은apps/desktop/src/lib/job_runner.ts의 IPC/event 검증 경계에서 바로 소비되므로, 상태별 허용engineState를 같이 좁혀야 합니다.🔧 수정 방향 예시
export type SongRehearsalPack = | { id: string; - packState: "queued" | "analyzing"; - engineState: AnalysisJobState; + packState: "queued"; + engineState: "queued"; + sourceLabel: string; + } + | { + id: string; + packState: "analyzing"; + engineState: "running"; sourceLabel: string; } | { id: string; packState: "ready"; - engineState?: AnalysisJobState; + engineState?: "succeeded"; song: RehearsalSong; sourceLabel: string; } | { id: string; packState: "failed"; - engineState?: AnalysisJobState; + engineState?: "failed"; error: AnalysisJobError; sourceLabel: string; };- if (value.packState === "queued" || value.packState === "analyzing") { + if (value.packState === "queued") { const extraKey = unexpectedKey(value, ["id", "packState", "engineState", "sourceLabel"], path); if (extraKey) return extraKey; - if (!isOneOf(ANALYSIS_JOB_STATES, value.engineState)) return invalidField(`${path}.engineState`); + if (value.engineState !== "queued") return invalidField(`${path}.engineState`); + } else if (value.packState === "analyzing") { + const extraKey = unexpectedKey(value, ["id", "packState", "engineState", "sourceLabel"], path); + if (extraKey) return extraKey; + if (value.engineState !== "running") return invalidField(`${path}.engineState`); } else if (value.packState === "ready") { const extraKey = unexpectedKey(value, ["id", "packState", "engineState", "sourceLabel", "song"], path); if (extraKey) return extraKey; + if (value.engineState !== undefined && value.engineState !== "succeeded") { + return invalidField(`${path}.engineState`); + } if (value.song === undefined) return invalidField(`${path}.song`); } else if (value.packState === "failed") { const extraKey = unexpectedKey(value, ["id", "packState", "engineState", "sourceLabel", "error"], path); if (extraKey) return extraKey; + if (value.engineState !== undefined && value.engineState !== "failed") { + return invalidField(`${path}.engineState`); + } if (value.error === undefined) return invalidField(`${path}.error`); }As per coding guidelines, "Treat files, URLs, metadata, model artifacts, and project files as untrusted input" and "Keep local backend access on allowlisted IPC or
127.0.0.1only, with strict schema validation".Also applies to: 1042-1068
🤖 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 119 - 139, The SongRehearsalPack discriminated union allows contradictory combinations of packState and engineState; fix it by replacing the current broad variants with one explicit union member per packState value (e.g., separate variants for "queued", "analyzing", "ready", "failed") and narrow the engineState field in each member using TypeScript utility types (Extract/Exclude) against AnalysisJobState so only the allowed engine states can appear for that packState; update any checks in job_runner.ts consumers to the new narrowed types (refer to SongRehearsalPack, packState, engineState, and AnalysisJobState) so the IPC/schema boundary enforces the invariant.
🤖 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/lib/job_runner.ts`:
- Around line 66-77: The browser fallback is mutating a pack to "ready" without
providing the required SongRehearsalPack fields (e.g., song), which violates the
contract and breaks isRehearsalWorkspace/parseRehearsalWorkspace and
getWorkspaceState; locate the timeout block that finds pack by packId in
mockWorkspace.songs and, when promoting to pack.packState = "ready" and
pack.engineState = "succeeded", replace the pack with a new fully populated
SongRehearsalPack object (including song and any other required fields) rather
than only updating packState/engineState in place, then call
triggerMockUpdate(); also apply the same fix to the similar block around the
94-104 region.
In `@packages/shared-types/test/index.test.ts`:
- Line 912: The destructuring "const { song, ...packWithoutSong } = validPack;"
creates an unused variable song causing an ESLint unused-vars error; either
remove song from the destructuring and just write "const packWithoutSong = (({
song, ...rest }) => rest)(validPack)" equivalent by directly omitting song, or
rename it to a deliberately ignored variable like "_song" (e.g., "const { song:
_song, ...packWithoutSong } = validPack;") so ESLint recognizes it as
intentionally unused; update the line in the test accordingly, keeping the rest
of the test assertions unchanged.
- Line 924: The test uses an ESLint-forbidden "as any" cast; update the
assertion to avoid any by casting the invalid song object to unknown instead
(e.g., replace "song: { id: 123 } as any" with "song: { id: 123 } as unknown")
so parseSongRehearsalPack still receives a wrong-type song for the test while
complying with lint rules; locate the test referencing parseSongRehearsalPack,
validPack, packState and song to make the change.
In `@packages/shared-types/vitest.config.ts`:
- Around line 10-13: The coverage thresholds for keys "lines", "functions",
"branches", and "statements" currently set to 90 need an inline rationale and
traceability: add a concise comment immediately adjacent to the coverage config
(the object containing lines/functions/branches/statements = 90) explaining why
the threshold was lowered (e.g., practical coverage limits, alignment with
standards, or temporary CI workaround), include the date/author and whether a
follow-up issue was created, and if this is a temporary change open a tracking
issue and reference its ID in the comment so reviewers can follow up; apply the
same comment to the corresponding config that mirrors this threshold so both
configs stay documented and consistent.
In `@services/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.py`:
- Around line 73-82: The avg_prob calculation can become NaN when all entries in
voiced_probs are NaN; after filtering NaNs you must re-check the resulting array
length and treat an empty result as 0.0. Change the logic around
voiced_probs/avg_prob (the block that defines avg_prob, confidence, and the
subsequent avg_prob < 0.2 check) to compute filtered =
voiced_probs[~np.isnan(voiced_probs)], set avg_prob = np.mean(filtered) if
filtered.size > 0 else 0.0, then derive confidence = "high" if avg_prob > 0.6
else "low" and keep the existing unvoiced return when avg_prob < 0.2 so that
fallback behavior is correct when all values were NaN.
In `@services/analysis-engine/tests/test_chord_recognizer.py`:
- Around line 140-143: The test currently asserts presence of "C" and "G" in
identified_chords (variables: test_chord_recognizer_changing_chords, result,
identified_chords) but not their order or timing; update the test to assert that
the detected "C" occurs before "G" by either checking the index positions in
result (e.g., index of first entry with r["chord"] == "C" < index of first with
r["chord"] == "G") or, if result entries include a timestamp/start field (e.g.,
r["start_time"] or r["time"]), assert that the start time for "C" is less than
the start time for "G".
- Around line 47-48: The docstring for test_chord_recognizer_hpss_exception
simply repeats the function name; update it to a concise description of the test
intent and expected behavior (for example: that the chord recognizer should
raise or handle an HPSS-related exception when HPSS fails or returns malformed
data). Edit the docstring in the test_chord_recognizer_hpss_exception function
to state the scenario being simulated, the expected outcome (exception type or
error handling behavior), and any important preconditions or mocks used so
future readers understand the purpose immediately.
In `@services/analysis-engine/tests/test_roles_ml.py`:
- Around line 10-11: The test docstring for
test_role_extractor_with_audio_features repeats the function name; update it to
a descriptive sentence summarizing the test's purpose and expected behavior
(e.g., what inputs/fixtures are used and which outputs or assertions are
verified). Edit the docstring inside the test_role_extractor_with_audio_features
function to state the scenario being tested (audio feature handling by the role
extractor) and the specific validation (e.g., expected roles, error handling, or
shape/type checks) so future readers understand intent without reading the test
body.
---
Duplicate comments:
In `@apps/desktop/src/App.test.tsx`:
- Around line 384-393: The test currently uses an empty setTimeout to wait for
the "early return when no ready pack" branch but that is flaky; update the test
to observe a measurable outcome by asserting the saveProject mock was not
invoked (use waitFor(() => expect(mockSaveProject).not.toHaveBeenCalled())).
Locate the test that renders <App /> and fires the "Save Project" button (the
block referencing saveProject/mockSaveProject in App.test.tsx) and replace the
final new Promise/setTimeout(0) with a waitFor assertion that mockSaveProject is
not called, ensuring the early-return branch is reliably verified.
- Around line 230-246: The test reintroduces an explicit any cast via
mockWorkspaceStore.songs[x].packState = "unknown" as any; replace that with a
valid PackState value (e.g., "queued" or another real state your app handles) so
the test uses only contract-approved values, or if you really need to exercise
the fallback branch create a separate unit test that intentionally supplies an
out-of-contract value without using `as any` (e.g., build the raw props object
via JSON.parse or a fixture file) and assert the fallback behavior; update the
"covers missing progressMessage branches" test to remove the `"unknown" as any`
usage and reference mockWorkspaceStore and the songs[].packState field when
making the change.
In `@apps/desktop/src/App.tsx`:
- Around line 107-128: handleImportYoutube currently only awaits
importYoutubeUrl and calls enqueueSong without awaiting or catching errors,
which can cause unhandled rejections and clear the youtubeUrl even if enqueue
fails; change the flow so that after importYoutubeUrl resolves you await
enqueueSong(...) and let any error bubble into the existing try/catch so
setSelectionError(t("youtubeImportFailed")) runs on failure, and only clear
youtubeUrl (setYoutubeUrl("")) after enqueueSong completes successfully; update
references in this fix: handleImportYoutube, importYoutubeUrl, enqueueSong,
setYoutubeUrl, setSelectionError, setIsImporting.
- Around line 166-171: The save button's enabled state is inconsistent with
handleSaveProject: compute a boolean like hasReadyPack by checking
workspace.songs for a pack with packState === "ready" and song present, use that
value both in handleSaveProject (early return) and in the button props
(disabled, aria-disabled and styling) so the button is truly
disabled/unfocusable when no ready pack exists; update references around
handleSaveProject, readyPack and the Save Project button to use hasReadyPack for
consistency (also apply same change to the other occurrence noted around lines
210-230).
In `@packages/shared-types/src/index.ts`:
- Around line 119-139: The SongRehearsalPack discriminated union allows
contradictory combinations of packState and engineState; fix it by replacing the
current broad variants with one explicit union member per packState value (e.g.,
separate variants for "queued", "analyzing", "ready", "failed") and narrow the
engineState field in each member using TypeScript utility types
(Extract/Exclude) against AnalysisJobState so only the allowed engine states can
appear for that packState; update any checks in job_runner.ts consumers to the
new narrowed types (refer to SongRehearsalPack, packState, engineState, and
AnalysisJobState) so the IPC/schema boundary enforces the invariant.
In `@services/analysis-engine/tests/test_roles_ml.py`:
- Around line 88-89: 현재 테스트 only covers a complete failure
(mock_track.return_value = None) but misses the low-confidence return path; add
a test case where mock_track (the PitchTracker.track() mock) returns
{"lowest_note": None, "highest_note": None, "confidence": "low"} and verify the
same fallback behavior/assertions as for the None case (and keep/mock
mock_recognize as appropriate) so the fallback logic for low-confidence results
is exercised and prevents regressions.
🪄 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: 9d167af1-5728-4b1d-b2bc-214fb9295871
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonservices/analysis-engine/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
apps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/features/workspace/Workspace.tsxapps/desktop/src/lib/job_runner.tsapps/desktop/vite.config.tsdocs/plans/2026-03-28-ml-engine-integration.mdpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tspackages/shared-types/vitest.config.tsservices/analysis-engine/pyproject.tomlservices/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.pyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pyservices/analysis-engine/tests/test_chord_recognizer.pyservices/analysis-engine/tests/test_pitch_tracker.pyservices/analysis-engine/tests/test_roles_ml.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/App.tsx (1)
219-233:⚠️ Potential issue | 🟡 MinorSave Project 버튼의 접근성 및 활성화 조건 개선 필요
현재
aria-disabled만 설정되어 있어 버튼이 실제로 비활성화되지 않습니다. 또한workspace존재 여부만 확인하고readypack 유무는 확인하지 않아, 저장 가능한 콘텐츠가 없어도 버튼이 활성화된 것처럼 보입니다.🔧 접근성 및 조건 수정
+ const hasReadyPack = workspace?.songs.some(s => s.packState === "ready" && s.song); <button type="button" onClick={handleSaveProject} - aria-disabled={!workspace} + disabled={!hasReadyPack} + aria-disabled={!hasReadyPack} style={{ padding: "8px 16px", - cursor: workspace ? "pointer" : "not-allowed", + cursor: hasReadyPack ? "pointer" : "not-allowed", borderRadius: "4px", - backgroundColor: workspace ? "#fff" : "#f5f5f5", + backgroundColor: hasReadyPack ? "#fff" : "#f5f5f5", border: "1px solid `#ccc`", - opacity: workspace ? 1 : 0.5 + opacity: hasReadyPack ? 1 : 0.5 }} >🤖 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 219 - 233, The Save Project button only uses aria-disabled and checks only workspace, so make it truly disabled and reflect the correct enablement by adding the native disabled attribute and gating the onClick; update the JSX for the button (the element with onClick={handleSaveProject}) to set disabled={!workspace || !readyPack} (or the actual boolean that indicates a ready pack) and also short-circuit handleSaveProject so it returns early if !workspace || !readyPack; keep aria-disabled for accessibility parity and adjust styles/cursor based on the same combined condition so the button is visually and functionally disabled when saving isn't possible.
♻️ Duplicate comments (4)
apps/desktop/src/lib/job_runner.ts (2)
94-107:⚠️ Potential issue | 🟠 Major
retry_song에서 동일한 계약 위반 발생
enqueue_song과 마찬가지로retry_song에서도ready상태 전환 시song필드 없이 상태만 변경하고 있습니다. 동일한 문제로 인해 재시도된 pack이 UI에서ready상태로 표시되지 않습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/job_runner.ts` around lines 94 - 107, The retry_song flow is mirroring enqueue_song's contract violation by updating packState/engineState to "ready"/"succeeded" without attaching the updated song object, so the UI doesn't see the retried pack as ready; modify the retry_song branch that finds the song via mockWorkspace.songs.find(s => s.id === jobId) to update the entire song object (including the song field/content) rather than only packState/engineState, and then call triggerMockUpdate() with the updated song data or ensure mockWorkspace.songs is replaced/updated atomically so the UI receives the full song object (reference functions/vars: retry_song, enqueue_song, mockWorkspace.songs, p, packState, engineState, triggerMockUpdate, jobId).
66-77:⚠️ Potential issue | 🟠 Major
ready상태 전환 시SongRehearsalPack계약 위반
packState를"ready"로 변경하면서song필드를 설정하지 않고 있습니다.SongRehearsalPack타입 정의(context snippet 4)에 따르면ready상태의 pack은 반드시song필드가 있어야 합니다.이로 인해
subscribeToWorkspaceUpdates의isRehearsalWorkspace검증이 실패하고, 콜백이 호출되지 않아 UI가ready상태로 업데이트되지 않습니다.🔧 계약을 준수하는 수정 제안
+import { createDemoRehearsalSong } from "@bandscope/shared-types"; setTimeout(() => { - pack.packState = "ready"; - pack.engineState = "succeeded"; + const packIndex = mockWorkspace.songs.findIndex(p => p.id === packId); + if (packIndex !== -1) { + mockWorkspace.songs[packIndex] = { + id: packId, + packState: "ready", + engineState: "succeeded", + sourceLabel: mockWorkspace.songs[packIndex].sourceLabel, + song: createDemoRehearsalSong() + }; + } triggerMockUpdate(); }, 2000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/job_runner.ts` around lines 66 - 77, When transitioning a mock pack to ready in the timeout, ensure you satisfy the SongRehearsalPack contract by setting the required song field; update the block that sets pack.packState = "ready" and pack.engineState = "succeeded" (found in the mock timeout that reads mockWorkspace.songs.find) to also assign pack.song with a valid Song object (or clone from an existing sample) before calling triggerMockUpdate(); this will make subscribeToWorkspaceUpdates' isRehearsalWorkspace check succeed and allow the UI to see the ready state.apps/desktop/src/App.test.tsx (1)
393-393: 🧹 Nitpick | 🔵 Trivial
setTimeout대신 명시적인 비동기 대기 사용 권장
await new Promise(r => setTimeout(r, 0))는 비동기 작업 완료를 보장하지 않으며 테스트 안정성을 저하시킬 수 있습니다.waitFor를 사용하여 예상되는 상태 변경이나 부수 효과의 부재를 명시적으로 확인하는 것이 좋습니다.♻️ 안정적인 비동기 대기로 변경
fireEvent.click(screen.getByRole("button", { name: /Save Project/i })); - await new Promise(r => setTimeout(r, 0)); + // Verify save was not called since there's no ready pack + await waitFor(() => { + expect(mockSaveProject).not.toHaveBeenCalled(); + });412번, 420번 줄에서도 동일하게 적용하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/App.test.tsx` at line 393, Replace the three uses of "await new Promise(r => setTimeout(r, 0))" with explicit waits using waitFor from `@testing-library/react`: import waitFor if not already present and change each instance to await waitFor(() => /* assertion or condition that reflects the expected state change, e.g. expect(someElement).toBeInTheDocument() or expect(mockFn).toHaveBeenCalled() */ ), ensuring you wait for a concrete observable outcome rather than an arbitrary tick; target the specific DOM/query or mock used in the surrounding test where "await new Promise(r => setTimeout(r, 0))" appears so the test asserts the desired state before proceeding.packages/shared-types/test/index.test.ts (1)
912-912:⚠️ Potential issue | 🔴 CriticalESLint 오류: 미사용 변수
_song이 CI 파이프라인 실패를 일으키고 있습니다.
_song접두사 컨벤션은 ESLint의@typescript-eslint/no-unused-vars규칙에서 기본적으로 허용되지 않습니다. 파이프라인 로그에서 이 줄이 빌드 실패의 원인임을 확인할 수 있습니다.🔧 ESLint 오류 수정을 위한 제안
ESLint 설정에서
argsIgnorePattern이나varsIgnorePattern이_접두사를 허용하도록 설정되어 있지 않다면, 아래와 같이 수정하세요:- const { song: _song, ...packWithoutSong } = validPack; + // eslint-disable-next-line `@typescript-eslint/no-unused-vars` -- Intentionally omitting song from pack + const { song: _song, ...packWithoutSong } = validPack;또는 함수형 접근 방식으로 변경:
- const { song: _song, ...packWithoutSong } = validPack; + const packWithoutSong = (({ song, ...rest }) => rest)(validPack);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/test/index.test.ts` at line 912, The test destructuring currently binds an unused variable as "song: _song" which trigers ESLint; change the destructuring to avoid an unused named binding (e.g. use a single underscore binding like "song: _" in the same statement) or update ESLint to allow underscore-only ignored vars (add ^_$ or ^_ to varsIgnorePattern/argsIgnorePattern), referencing the destructuring that produces packWithoutSong from validPack (the const that currently reads "const { song: _song, ...packWithoutSong } = validPack").
🤖 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/App.tsx`:
- Around line 113-118: handleImportYoutube currently calls enqueueSong({...})
without error handling while handleChooseLocalAudio and handleDemoSong use
.catch(); update handleImportYoutube to mirror their behavior by adding promise
rejection handling (either append .catch(...) to the enqueueSong(...) call or
convert to async/await with try/catch), log the error and surface user feedback
(toast/dialog) on IPC/enqueue failures, and ensure you reference the same error
message style used in handleChooseLocalAudio/handleDemoSong so users receive
consistent feedback.
In `@services/analysis-engine/.python-version`:
- Line 1: `.python-version`(현재 "3.12")과 pyproject.toml의 `requires-python` 제약이
일치하지 않아 빌드/CI에서 불일치가 발생합니다; 수정 방법은 pyproject.toml의 `requires-python`을 명시적으로 상한을
추가해 ">=3.12,<3.14"로 변경하거나(권장) `.python-version`을 pyproject.toml의 범위 내의 구체적 패치
버전으로 맞추는 것입니다; 구체적으로 수정 대상은 pyproject.toml의 requires-python 설정과 루트의
.python-version 파일(또는 문서화 선택 사유)을 동기화하도록 업데이트하세요.
---
Outside diff comments:
In `@apps/desktop/src/App.tsx`:
- Around line 219-233: The Save Project button only uses aria-disabled and
checks only workspace, so make it truly disabled and reflect the correct
enablement by adding the native disabled attribute and gating the onClick;
update the JSX for the button (the element with onClick={handleSaveProject}) to
set disabled={!workspace || !readyPack} (or the actual boolean that indicates a
ready pack) and also short-circuit handleSaveProject so it returns early if
!workspace || !readyPack; keep aria-disabled for accessibility parity and adjust
styles/cursor based on the same combined condition so the button is visually and
functionally disabled when saving isn't possible.
---
Duplicate comments:
In `@apps/desktop/src/App.test.tsx`:
- Line 393: Replace the three uses of "await new Promise(r => setTimeout(r, 0))"
with explicit waits using waitFor from `@testing-library/react`: import waitFor if
not already present and change each instance to await waitFor(() => /* assertion
or condition that reflects the expected state change, e.g.
expect(someElement).toBeInTheDocument() or expect(mockFn).toHaveBeenCalled() */
), ensuring you wait for a concrete observable outcome rather than an arbitrary
tick; target the specific DOM/query or mock used in the surrounding test where
"await new Promise(r => setTimeout(r, 0))" appears so the test asserts the
desired state before proceeding.
In `@apps/desktop/src/lib/job_runner.ts`:
- Around line 94-107: The retry_song flow is mirroring enqueue_song's contract
violation by updating packState/engineState to "ready"/"succeeded" without
attaching the updated song object, so the UI doesn't see the retried pack as
ready; modify the retry_song branch that finds the song via
mockWorkspace.songs.find(s => s.id === jobId) to update the entire song object
(including the song field/content) rather than only packState/engineState, and
then call triggerMockUpdate() with the updated song data or ensure
mockWorkspace.songs is replaced/updated atomically so the UI receives the full
song object (reference functions/vars: retry_song, enqueue_song,
mockWorkspace.songs, p, packState, engineState, triggerMockUpdate, jobId).
- Around line 66-77: When transitioning a mock pack to ready in the timeout,
ensure you satisfy the SongRehearsalPack contract by setting the required song
field; update the block that sets pack.packState = "ready" and pack.engineState
= "succeeded" (found in the mock timeout that reads mockWorkspace.songs.find) to
also assign pack.song with a valid Song object (or clone from an existing
sample) before calling triggerMockUpdate(); this will make
subscribeToWorkspaceUpdates' isRehearsalWorkspace check succeed and allow the UI
to see the ready state.
In `@packages/shared-types/test/index.test.ts`:
- Line 912: The test destructuring currently binds an unused variable as "song:
_song" which trigers ESLint; change the destructuring to avoid an unused named
binding (e.g. use a single underscore binding like "song: _" in the same
statement) or update ESLint to allow underscore-only ignored vars (add ^_$ or ^_
to varsIgnorePattern/argsIgnorePattern), referencing the destructuring that
produces packWithoutSong from validPack (the const that currently reads "const {
song: _song, ...packWithoutSong } = validPack").
🪄 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: 82f2431b-8b79-4878-9a71-97aeedb12f19
📒 Files selected for processing (5)
apps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/lib/job_runner.tspackages/shared-types/test/index.test.tsservices/analysis-engine/.python-version
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/shared-types/test/index.test.ts (1)
925-926:⚠️ Potential issue | 🟠 Major
any우회(eslint-disable)를 제거하고unknown으로 바꾸세요.Line 925에서 규칙을 비활성화하고 Line 926에서
as any를 사용해 린트 정책을 우회하고 있습니다. PR 목표(린트 오류/any제거)와도 충돌합니다.🔧 제안 수정
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - expect(() => parseSongRehearsalPack({ ...validPack, packState: "ready", song: { id: 123 } as any })).toThrow("id"); + expect(() => + parseSongRehearsalPack({ ...validPack, packState: "ready", song: { id: 123 } as unknown }) + ).toThrow("id");#!/bin/bash # 검증 목적: 테스트 코드에 남아 있는 any 우회 패턴 확인 rg -n --type=ts -e 'as any' -e 'eslint-disable-next-line `@typescript-eslint/no-explicit-any`' packages/shared-types/test/index.test.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared-types/test/index.test.ts` around lines 925 - 926, Remove the eslint-disable and stop using "as any": in the test invoking parseSongRehearsalPack with validPack and a bad song, delete the "// eslint-disable-next-line `@typescript-eslint/no-explicit-any`" and replace "song: { id: 123 } as any" with a safe assertion like "song: { id: 123 } as unknown as Song" (or cast to the appropriate Song type used by parseSongRehearsalPack) so you avoid the any bypass while preserving the intent of the test; keep the expect(() => parseSongRehearsalPack(...)).toThrow("id") assertion and reference parseSongRehearsalPack and validPack when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/shared-types/test/index.test.ts`:
- Around line 925-926: Remove the eslint-disable and stop using "as any": in the
test invoking parseSongRehearsalPack with validPack and a bad song, delete the
"// eslint-disable-next-line `@typescript-eslint/no-explicit-any`" and replace
"song: { id: 123 } as any" with a safe assertion like "song: { id: 123 } as
unknown as Song" (or cast to the appropriate Song type used by
parseSongRehearsalPack) so you avoid the any bypass while preserving the intent
of the test; keep the expect(() => parseSongRehearsalPack(...)).toThrow("id")
assertion and reference parseSongRehearsalPack and validPack when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 460ba970-c07e-4c48-8a02-1c4fafefdb37
📒 Files selected for processing (1)
packages/shared-types/test/index.test.ts
seonghobae
force-pushed
the
fix/issue-148-ci-pipeline
branch
from
1306844 to
7ad4f03
Compare
Owner
Author
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
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.
Summary
Resolves #148. This PR fixes the CI pipeline failures that blocked merging:
anytypes, unused vars, console statements) in desktop app.viteandpostcss.Verification
npm run lint:workspacespasses.npm audit --workspacesreturns 0 vulnerabilities../scripts/harness/quickcheck.shpasses successfully locally.