feat: add local analysis orchestration#44
Closed
seonghobae wants to merge 2 commits into
Closed
Conversation
Owner
Author
|
@coderabbitai review |
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough로컬 분석 오케스트레이션 시스템을 구현합니다. 타우리 IPC를 통해 Rust 백엔드가 Python 분석 엔진을 서브프로세스로 실행하고, 작업 시작 및 상태 조회 명령을 제공합니다. React 프론트엔드는 분석 작업 폴링 기능과 결과 렌더링을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant React as React 프론트엔드
participant Tauri as Tauri IPC
participant Rust as Rust 백엔드
participant Python as Python CLI
User->>React: 분석 시작 버튼 클릭
React->>Tauri: start_analysis_job(request)
Tauri->>Rust: IPC 호출
Rust->>Rust: 작업 생성 (Queued 상태)
Rust->>Python: subprocess 실행<br/>(stdin으로 JSON 전송)
Rust-->>Tauri: 초기 상태 반환
Tauri-->>React: AnalysisJobStatus {jobId, state: "queued"}
React->>React: 폴링 루프 시작
loop 폴링 (ANALYSIS_POLL_INTERVAL_MS)
React->>Tauri: get_analysis_job_status(jobId)
Tauri->>Rust: IPC 호출
Rust->>Rust: 작업 저장소에서 상태 조회
Rust-->>Tauri: 현재 상태 반환
Tauri-->>React: AnalysisJobStatus
React->>React: 상태 업데이트
end
Python->>Python: 분석 실행
Python->>Python: 결과 JSON 생성
Python-->>Rust: stdout으로 결과 전송
Rust->>Rust: 상태를 Running → Succeeded 전환
Rust->>Rust: 결과를 RehearsalSong으로 파싱
React->>Tauri: get_analysis_job_status(jobId)
Tauri->>Rust: IPC 호출
Rust-->>Tauri: {state: "succeeded", result: RehearsalSong}
Tauri-->>React: AnalysisJobStatus (성공)
React->>React: 폴링 중지
React->>User: 분석 결과 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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/src/main.rs`:
- Around line 362-378: The current code spawns an unbounded std::thread for
every request (in the block using app_state, std::thread::spawn, store_status,
run_analysis_engine), which allows unlimited concurrent Python processes;
introduce a global in-flight limit (e.g. a static tokio::sync::Semaphore or an
AtomicUsize with MAX_IN_FLIGHT) and attempt to acquire a permit before spawning:
if acquire fails, immediately call store_status with an AnalysisJobStatus
showing AnalysisJobState::Failed (or a new Rejected state), an appropriate error
message and return; if acquire succeeds, spawn the thread as before but ensure
the permit is released when the work completes (after run_analysis_engine and
the final store_status) and handle panics/errors to always release the permit.
Include references to the symbols app_state, store_status, run_analysis_engine,
AnalysisJobStatus, and AnalysisJobState when making the change.
- Around line 168-208: The code currently relies on build-time repo_root()
(env!("CARGO_MANIFEST_DIR")) inside analysis_command(), which breaks packaged
apps; replace repo_root()/env! usage with runtime resource detection: first
respect BANDSCOPE_ANALYSIS_PYTHON, then check for a bundled sidecar or
resource-relative python/engine by resolving paths at runtime (use
std::env::current_exe() or Tauri resource/sidecar locations instead of
repo_root()) and test existence before choosing it; if found, return that path
and args for "bandscope_analysis.cli", otherwise fall back to the uv run
invocation. Update the logic in analysis_command() and remove reliance on
repo_root() so packaged releases can find the engine.
In `@apps/desktop/src/App.tsx`:
- Around line 69-71: The current single-slot state
(jobStatus/jobResult/jobError) allows reentrant handleStartAnalysis() calls to
race and let stale responses overwrite the latest job; modify
handleStartAnalysis and the related start flow to prevent re-entry by either
disabling the start button when jobStatus is "starting" | "queued" | "running"
or by adding a per-request token/id (e.g., requestId) stored in a ref and
checked before updating setJobStatus/setJobResult/setJobError so only the latest
request may update state; ensure the token/ref is created at the start of
handleStartAnalysis, passed through async calls, and verified in all
response/error handlers (and apply the same guard where similar start logic
appears around the other referenced blocks).
- Around line 96-99: The hardcoded UI strings in the new analysis flow need to
be replaced with translations and progress keys: change usages of the literal
"Analysis could not start." and any direct "Start analysis" to use the i18n
function (t(...)) instead of hardcoded text by updating calls that
setJobError(...) and any button labels; stop sinking raw progressLabel text to
the UI and instead map the analysis status enum or a defined translation key
(e.g., the status enum used by progress/state handling and the progressLabel
consumer) to t(...) keys so all states render via the translation lookup (update
code paths around setJobError, progressLabel consumption, and the button label
renderers referenced in this diff and the related 117-129 region).
In `@docs/plans/2026-03-12-issue-32-analysis-orchestration-design.md`:
- Around line 54-57: Add and export the missing AnalysisJobSnapshot type in the
shared types module so it matches the other exported types (AnalysisJobRequest,
AnalysisJobStatus, AnalysisJobError); implement AnalysisJobSnapshot as an
interface/typedef that ties a job identifier to its current status and request
payload and includes timestamps and optional error/metadata (e.g., jobId:
string, status: AnalysisJobStatus, request: AnalysisJobRequest, startedAt?:
string, finishedAt?: string, error?: AnalysisJobError, metadata?: Record<string,
unknown>), then export it alongside
AnalysisJobRequest/AnalysisJobStatus/AnalysisJobError so the documented contract
is accurate.
In `@docs/plans/2026-03-12-issue-32-analysis-orchestration.md`:
- Line 3: 문서 내의 "**For Claude:** REQUIRED SUB-SKILL: Use
superpowers:executing-plans" 지시문은 구현 계획 텍스트의 노이즈이므로 해당 문구를 제거하고, 에이전트 실행 힌트나 권장
워크플로우(예: 이슈 템플릿 또는 별도 에이전트 운영 문서)로 옮기세요; 대상 문자열("For Claude")을 찾아 삭제하거나 주석으로
대체하고, 대신 참조할 별도 문서(에이전트 가이드/이슈 템플릿)에 동일한 실행 힌트만 기록해 유지보수성 및 독립성을 확보하십시오.
In `@packages/shared-types/src/index.ts`:
- Around line 270-289: The helper createAnalysisJobStatus currently accepts a
wide input that can produce states the validator validateAnalysisJobStatus
rejects (e.g., "succeeded" without result, "failed" without error); change the
input parameter to a discriminated union keyed by state (e.g., types for {
state: "succeeded"; result: RehearsalSong; ... }, { state: "failed"; error:
AnalysisJobError; ... }, and the other states without those required fields) so
the compiler enforces that "succeeded" always includes result and "failed"
always includes error, keep the function returning AnalysisJobStatus and
preserve the timestamp defaults (requestedAt/updatedAt using now) and optional
progressLabel for all variants; update any call sites if necessary to satisfy
the new narrowed types.
In `@services/analysis-engine/src/bandscope_analysis/api.py`:
- Around line 69-72: The error message for invalid elements in role_focus always
points to "roleFocus[0]" even when a later element is wrong; update the
validation to identify the exact failing index by iterating with an index (e.g.,
enumerate over role_focus) and when a non-str element is found raise ValueError
referencing that index (e.g., "roleFocus[{i}]"); adjust the existing isinstance
checks around role_focus and the element checks to use this indexed error
message so it accurately reports which element failed.
- Around line 34-43: The AnalysisJobStatus TypedDict currently uses total=False
which makes every field optional; change it to explicitly mark only
progressLabel, result, and error as optional using NotRequired and keep jobId,
state, requestedAt, and updatedAt required. Import NotRequired (and Required if
needed for your Python version) from typing or typing_extensions, then update
the AnalysisJobStatus definition to annotate jobId, state, requestedAt,
updatedAt as required and progressLabel, result, error as NotRequired so the
Python types match the TypeScript contract.
In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 14-20: Wrap the json.load(sys.stdin) call in a try/except that
catches json.JSONDecodeError and returns the structured failure envelope instead
of letting the exception propagate; specifically, if json.load fails create a
response object containing the required fields (e.g. {"state": "failed",
"error": "<parse error message>"}) and json.dump it to stdout and return 0,
otherwise proceed to compute response = run_analysis_job(str(job_id), request,
requested_at) as before; update the code paths around json.load(sys.stdin),
job_id/request extraction, and the final json.dump(response, sys.stdout) to
ensure malformed JSON yields the structured failure response.
🪄 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: fd27aca9-12d1-4b3b-8b6e-60714c1f3601
⛔ Files ignored due to path filters (5)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockapps/desktop/src-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/desktop-schema.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/macOS-schema.jsonis excluded by!**/gen/**
📒 Files selected for processing (21)
ARCHITECTURE.mdapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/build.rsapps/desktop/src-tauri/capabilities/main.jsonapps/desktop/src-tauri/permissions/autogenerated/get_analysis_job_status.tomlapps/desktop/src-tauri/permissions/autogenerated/start_analysis_job.tomlapps/desktop/src-tauri/src/main.rsapps/desktop/src-tauri/tauri.conf.jsonapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/lib/analysis.tsdocs/architecture/overview.mddocs/plans/2026-03-12-issue-32-analysis-orchestration-design.mddocs/plans/2026-03-12-issue-32-analysis-orchestration.mddocs/security/app-security.mdpackages/shared-types/src/index.tspackages/shared-types/test/index.test.tsservices/analysis-engine/src/bandscope_analysis/api.pyservices/analysis-engine/src/bandscope_analysis/cli.pyservices/analysis-engine/tests/test_api.pyservices/analysis-engine/tests/test_cli.py
Comment on lines
+168
to
+208
| fn repo_root() -> PathBuf { | ||
| Path::new(env!("CARGO_MANIFEST_DIR")) | ||
| .ancestors() | ||
| .nth(3) | ||
| .expect("repo root") | ||
| .to_path_buf() | ||
| } | ||
|
|
||
| fn analysis_command(repo_root: &Path) -> (String, Vec<String>) { | ||
| if let Ok(python_path) = std::env::var("BANDSCOPE_ANALYSIS_PYTHON") { | ||
| return ( | ||
| python_path, | ||
| vec!["-m".into(), "bandscope_analysis.cli".into()], | ||
| ); | ||
| } | ||
|
|
||
| let venv_python = repo_root | ||
| .join("services") | ||
| .join("analysis-engine") | ||
| .join(".venv") | ||
| .join("bin") | ||
| .join("python"); | ||
| if venv_python.exists() { | ||
| return ( | ||
| venv_python.to_string_lossy().into_owned(), | ||
| vec!["-m".into(), "bandscope_analysis.cli".into()], | ||
| ); | ||
| } | ||
|
|
||
| ( | ||
| "uv".into(), | ||
| vec![ | ||
| "run".into(), | ||
| "--project".into(), | ||
| "services/analysis-engine".into(), | ||
| "python".into(), | ||
| "-m".into(), | ||
| "bandscope_analysis.cli".into(), | ||
| ], | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 빌드 시점 저장소 경로 가정과 런타임 번들 구성을 확인합니다.
rg -n 'env!\("CARGO_MANIFEST_DIR"\)|services/analysis-engine|current_dir\(' apps/desktop/src-tauri/src/main.rs
fd 'tauri\.conf.*' apps/desktop/src-tauri -x sed -n '1,240p' {}
fd 'package_desktop_artifact.py' scripts -x sed -n '90,180p' {}
rg -n 'sidecar|resource_dir|resolve_resource|externalBin|analysis-engine' apps/desktop/src-tauri scriptsRepository: seonghobae/bandscope
Length of output: 3970
소스 트리 상대 경로를 런타임 기본값으로 쓰면 패키징된 앱에서 엔진을 찾을 수 없습니다.
env!("CARGO_MANIFEST_DIR")는 빌드 시점 경로를 고정하고, 패키징 스크립트(package_desktop_artifact.py)는 분석 엔진의 소스 디렉터리를 번들에 포함하지 않습니다. tauri.conf.json에도 sidecar나 resources 설정이 없으므로, repo_root()와 .current_dir()은 릴리스 환경에서 존재하지 않는 경로를 가리켜 spawn() 자체가 실패합니다. 런타임에서 실제로 사용 가능한 리소스(번들된 바이너리나 sidecar)를 기준으로 엔진 해석 경로를 변경해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src-tauri/src/main.rs` around lines 168 - 208, The code
currently relies on build-time repo_root() (env!("CARGO_MANIFEST_DIR")) inside
analysis_command(), which breaks packaged apps; replace repo_root()/env! usage
with runtime resource detection: first respect BANDSCOPE_ANALYSIS_PYTHON, then
check for a bundled sidecar or resource-relative python/engine by resolving
paths at runtime (use std::env::current_exe() or Tauri resource/sidecar
locations instead of repo_root()) and test existence before choosing it; if
found, return that path and args for "bandscope_analysis.cli", otherwise fall
back to the uv run invocation. Update the logic in analysis_command() and remove
reliance on repo_root() so packaged releases can find the engine.
Comment on lines
+362
to
+378
| let app_state = state.inner().clone(); | ||
| std::thread::spawn(move || { | ||
| store_status( | ||
| &app_state, | ||
| AnalysisJobStatus { | ||
| job_id: job_id.clone(), | ||
| state: AnalysisJobState::Running, | ||
| requested_at: requested_at.clone(), | ||
| updated_at: iso_timestamp_now(), | ||
| progress_label: Some("Running analysis".into()), | ||
| result: None, | ||
| error: None, | ||
| }, | ||
| ); | ||
| let finished = run_analysis_engine(job_id, parsed_request, requested_at); | ||
| store_status(&app_state, finished); | ||
| }); |
There was a problem hiding this comment.
허용된 명령 하나가 Python 프로세스를 무제한 생성할 수 있습니다.
호출마다 분리 스레드와 Python 프로세스를 즉시 띄우고 상한이 없어서, 반복 클릭이나 WebView 측 버그만으로도 로컬 CPU/RAM을 쉽게 고갈시킬 수 있습니다. 최소한 in-flight 제한(세마포어/큐)을 두고, 초과 시 typed failed status로 거절하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src-tauri/src/main.rs` around lines 362 - 378, The current code
spawns an unbounded std::thread for every request (in the block using app_state,
std::thread::spawn, store_status, run_analysis_engine), which allows unlimited
concurrent Python processes; introduce a global in-flight limit (e.g. a static
tokio::sync::Semaphore or an AtomicUsize with MAX_IN_FLIGHT) and attempt to
acquire a permit before spawning: if acquire fails, immediately call
store_status with an AnalysisJobStatus showing AnalysisJobState::Failed (or a
new Rejected state), an appropriate error message and return; if acquire
succeeds, spawn the thread as before but ensure the permit is released when the
work completes (after run_analysis_engine and the final store_status) and handle
panics/errors to always release the permit. Include references to the symbols
app_state, store_status, run_analysis_engine, AnalysisJobStatus, and
AnalysisJobState when making the change.
Comment on lines
+69
to
+71
| const [jobStatus, setJobStatus] = useState<AnalysisJobStatus | null>(null); | ||
| const [jobResult, setJobResult] = useState<RehearsalSong | null>(null); | ||
| const [jobError, setJobError] = useState<string | null>(null); |
There was a problem hiding this comment.
중복 시작 요청이 최신 작업 상태를 덮어쓰는 레이스를 만듭니다.
지금은 jobStatus/jobResult가 단일 슬롯인데 handleStartAnalysis() 재진입을 막지 않아서, 더 늦게 끝난 이전 호출이 최신 분석 요청의 상태를 다시 덮어쓸 수 있습니다. 시작 중이거나 queued/running일 때는 버튼을 막거나, stale 응답을 무시하는 토큰을 둬야 합니다.
🔒 제안된 수정
const [jobStatus, setJobStatus] = useState<AnalysisJobStatus | null>(null);
const [jobResult, setJobResult] = useState<RehearsalSong | null>(null);
const [jobError, setJobError] = useState<string | null>(null);
+ const [isStarting, setIsStarting] = useState(false);
@@
const handleStartAnalysis = async () => {
+ if (isStarting || jobStatus?.state === "queued" || jobStatus?.state === "running") {
+ return;
+ }
+ setIsStarting(true);
setJobError(null);
setJobResult(null);
setJobStatus(null);
try {
const nextStatus = await startAnalysisJob(defaultRequest);
setJobStatus(nextStatus);
if (nextStatus.state === "succeeded" && nextStatus.result) {
setJobResult(nextStatus.result);
}
} catch {
setJobError("Analysis could not start.");
+ } finally {
+ setIsStarting(false);
}
};
@@
- <button type="button" onClick={handleStartAnalysis}>Start analysis</button>
+ <button
+ type="button"
+ onClick={handleStartAnalysis}
+ disabled={isStarting || jobStatus?.state === "queued" || jobStatus?.state === "running"}
+ >
+ Start analysis
+ </button>Also applies to: 106-119, 128-128
🤖 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 69 - 71, The current single-slot state
(jobStatus/jobResult/jobError) allows reentrant handleStartAnalysis() calls to
race and let stale responses overwrite the latest job; modify
handleStartAnalysis and the related start flow to prevent re-entry by either
disabling the start button when jobStatus is "starting" | "queued" | "running"
or by adding a per-request token/id (e.g., requestId) stored in a ref and
checked before updating setJobStatus/setJobResult/setJobError so only the latest
request may update state; ensure the token/ref is created at the start of
handleStartAnalysis, passed through async calls, and verified in all
response/error handlers (and apply the same guard where similar start logic
appears around the other referenced blocks).
Comment on lines
+96
to
+99
| setJobError(nextStatus.error?.message ?? "Analysis could not start."); | ||
| } | ||
| } catch { | ||
| setJobError("Analysis could not start."); |
There was a problem hiding this comment.
새 분석 UI 문구가 기존 번역 경로를 벗어납니다.
"Start analysis"와 "Analysis could not start."는 여기서 직접 하드코딩되고, 진행 상태도 progressLabel 원문을 그대로 출력합니다. 지금 구조대로면 새 분석 플로우만 영문으로 남으니, 버튼/오류는 t(...)로 옮기고 진행 상태는 상태 enum 또는 번역 키를 프론트에서 매핑하는 편이 안전합니다.
Also applies to: 117-129
🤖 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 96 - 99, The hardcoded UI strings in
the new analysis flow need to be replaced with translations and progress keys:
change usages of the literal "Analysis could not start." and any direct "Start
analysis" to use the i18n function (t(...)) instead of hardcoded text by
updating calls that setJobError(...) and any button labels; stop sinking raw
progressLabel text to the UI and instead map the analysis status enum or a
defined translation key (e.g., the status enum used by progress/state handling
and the progressLabel consumer) to t(...) keys so all states render via the
translation lookup (update code paths around setJobError, progressLabel
consumption, and the button label renderers referenced in this diff and the
related 117-129 region).
Comment on lines
+21
to
+27
| function getInvoke(): TauriInvoke | null { | ||
| if (typeof window === "undefined") { | ||
| return null; | ||
| } | ||
|
|
||
| return window.__TAURI_INVOKE__ ?? null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 실제 Tauri 브리지 주입 코드와 fallback 가드를 찾습니다.
rg -n --glob '!**/dist/**' --glob '!**/node_modules/**' '__TAURI_INVOKE__|__TAURI_INTERNALS__|@tauri-apps/api/core|@tauri-apps/api' apps packages
rg -n 'function getInvoke|browserFallback|invokeAnalysis' apps/desktop/src/lib/analysis.tsRepository: seonghobae/bandscope
Length of output: 657
🏁 Script executed:
#!/bin/bash
# View the complete analysis.ts file to understand the fallback flow
cat -n apps/desktop/src/lib/analysis.ts | head -100Repository: seonghobae/bandscope
Length of output: 3544
🏁 Script executed:
#!/bin/bash
# Search for Tauri initialization and bridge setup in entry points
rg -n '__TAURI_INVOKE__|window\.__TAURI' --glob '!**/dist/**' --glob '!**/node_modules/**' -A 2 -B 2 apps/desktop/srcRepository: seonghobae/bandscope
Length of output: 556
🏁 Script executed:
#!/bin/bash
# Check main entry files for Tauri bridge initialization
find apps/desktop/src -type f \( -name 'main.ts' -o -name 'main.tsx' -o -name 'index.ts' -o -name 'index.tsx' -o -name 'App.ts' -o -name 'App.tsx' \) -exec cat -n {} +Repository: seonghobae/bandscope
Length of output: 7897
데스크톱 브리지 누락을 데모 성공으로 무음 처리하면 실제 Tauri 오작동이 가려집니다.
getInvoke()는 window.__TAURI_INVOKE__를 찾을 수 없으면 null을 반환하고, invokeAnalysis()는 이 경우 자동으로 browserFallback()으로 넘어갑니다. 문제는 데스크톱에서 Tauri 브리지 초기화가 실패해도 앱이 감지하지 못한 채 데모 데이터를 성공처럼 반환한다는 점입니다.
현재 코드에는:
- Tauri 브리지 주입 코드가 없음 (선언만 있음)
- 데스크톱 환경 감지 로직이 없음
- 브라우저/Storybook 테스트와 실제 데스크톱 실행을 구분할 방법이 없음
브라우저 환경에서만 fallback을 허용하고, 데스크톱에서는 명시적으로 브리지 초기화를 확인하거나 실패를 드러내는 것이 필요합니다. 예를 들어, 데스크톱 환경에서 브리지 없음을 즉시 에러로 throw하거나, Tauri가 정상 초기화된 후에만 앱을 시작하도록 해야 합니다.
| @@ -0,0 +1,238 @@ | |||
| # Issue 32 Analysis Orchestration Implementation Plan | |||
|
|
|||
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
특정 에이전트 전용 지시는 저장소 문서에서 분리하는 편이 좋습니다.
For Claude 문구는 구현 계획 자체와 무관해서 사람이나 다른 자동화 도구에게는 노이즈만 남깁니다. 실행 힌트가 필요하면 이슈 템플릿이나 별도 에이전트 문서로 분리하는 편이 유지보수성이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-12-issue-32-analysis-orchestration.md` at line 3, 문서 내의
"**For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans" 지시문은 구현 계획
텍스트의 노이즈이므로 해당 문구를 제거하고, 에이전트 실행 힌트나 권장 워크플로우(예: 이슈 템플릿 또는 별도 에이전트 운영 문서)로 옮기세요;
대상 문자열("For Claude")을 찾아 삭제하거나 주석으로 대체하고, 대신 참조할 별도 문서(에이전트 가이드/이슈 템플릿)에 동일한 실행
힌트만 기록해 유지보수성 및 독립성을 확보하십시오.
Comment on lines
+270
to
+289
| export function createAnalysisJobStatus(input: { | ||
| jobId: string; | ||
| state: AnalysisJobState; | ||
| result?: RehearsalSong; | ||
| error?: AnalysisJobError; | ||
| progressLabel?: string; | ||
| requestedAt?: string; | ||
| updatedAt?: string; | ||
| }): AnalysisJobStatus { | ||
| const now = new Date().toISOString(); | ||
| return { | ||
| jobId: input.jobId, | ||
| state: input.state, | ||
| requestedAt: input.requestedAt ?? now, | ||
| updatedAt: input.updatedAt ?? now, | ||
| progressLabel: input.progressLabel, | ||
| result: input.result, | ||
| error: input.error | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/shared-types/src/index.ts | sed -n '250,400p'Repository: seonghobae/bandscope
Length of output: 5440
이 helper는 같은 파일의 validator가 거부하는 status도 만들 수 있습니다.
예를 들어 createAnalysisJobStatus({ jobId: "x", state: "succeeded" })는 지금 그대로 반환되지만, validateAnalysisJobStatus()는 라인 373-378에서 state === "succeeded"일 때 result가 없으면, state === "failed"일 때 error가 없으면 invalid로 봅니다.
Public helper가 자기 validator보다 넓은 shape를 허용하면 IPC 경계에서 타입 안정성이 바로 깨집니다. 입력 타입을 상태별 discriminated union으로 좁혀서 컴파일 타임에 succeeded → result, failed → error를 강제하는 편이 안전합니다.
🔧 제안 수정안
+type CreateAnalysisJobStatusInput =
+ | {
+ jobId: string;
+ state: "queued" | "running";
+ progressLabel?: string;
+ requestedAt?: string;
+ updatedAt?: string;
+ }
+ | {
+ jobId: string;
+ state: "succeeded";
+ result: RehearsalSong;
+ progressLabel?: string;
+ requestedAt?: string;
+ updatedAt?: string;
+ }
+ | {
+ jobId: string;
+ state: "failed";
+ error: AnalysisJobError;
+ progressLabel?: string;
+ requestedAt?: string;
+ updatedAt?: string;
+ };
+
-export function createAnalysisJobStatus(input: {
- jobId: string;
- state: AnalysisJobState;
- result?: RehearsalSong;
- error?: AnalysisJobError;
- progressLabel?: string;
- requestedAt?: string;
- updatedAt?: string;
-}): AnalysisJobStatus {
+export function createAnalysisJobStatus(input: CreateAnalysisJobStatusInput): AnalysisJobStatus {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function createAnalysisJobStatus(input: { | |
| jobId: string; | |
| state: AnalysisJobState; | |
| result?: RehearsalSong; | |
| error?: AnalysisJobError; | |
| progressLabel?: string; | |
| requestedAt?: string; | |
| updatedAt?: string; | |
| }): AnalysisJobStatus { | |
| const now = new Date().toISOString(); | |
| return { | |
| jobId: input.jobId, | |
| state: input.state, | |
| requestedAt: input.requestedAt ?? now, | |
| updatedAt: input.updatedAt ?? now, | |
| progressLabel: input.progressLabel, | |
| result: input.result, | |
| error: input.error | |
| }; | |
| } | |
| type CreateAnalysisJobStatusInput = | |
| | { | |
| jobId: string; | |
| state: "queued" | "running"; | |
| progressLabel?: string; | |
| requestedAt?: string; | |
| updatedAt?: string; | |
| } | |
| | { | |
| jobId: string; | |
| state: "succeeded"; | |
| result: RehearsalSong; | |
| progressLabel?: string; | |
| requestedAt?: string; | |
| updatedAt?: string; | |
| } | |
| | { | |
| jobId: string; | |
| state: "failed"; | |
| error: AnalysisJobError; | |
| progressLabel?: string; | |
| requestedAt?: string; | |
| updatedAt?: string; | |
| }; | |
| export function createAnalysisJobStatus(input: CreateAnalysisJobStatusInput): AnalysisJobStatus { | |
| const now = new Date().toISOString(); | |
| return { | |
| jobId: input.jobId, | |
| state: input.state, | |
| requestedAt: input.requestedAt ?? now, | |
| updatedAt: input.updatedAt ?? now, | |
| progressLabel: input.progressLabel, | |
| result: input.result, | |
| error: input.error | |
| }; | |
| } |
🤖 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 270 - 289, The helper
createAnalysisJobStatus currently accepts a wide input that can produce states
the validator validateAnalysisJobStatus rejects (e.g., "succeeded" without
result, "failed" without error); change the input parameter to a discriminated
union keyed by state (e.g., types for { state: "succeeded"; result:
RehearsalSong; ... }, { state: "failed"; error: AnalysisJobError; ... }, and the
other states without those required fields) so the compiler enforces that
"succeeded" always includes result and "failed" always includes error, keep the
function returning AnalysisJobStatus and preserve the timestamp defaults
(requestedAt/updatedAt using now) and optional progressLabel for all variants;
update any call sites if necessary to satisfy the new narrowed types.
Comment on lines
+34
to
+43
| class AnalysisJobStatus(TypedDict, total=False): | ||
| """Typed analysis job snapshot shared with the desktop orchestrator.""" | ||
|
|
||
| jobId: str | ||
| state: Literal["queued", "running", "succeeded", "failed"] | ||
| requestedAt: str | ||
| updatedAt: str | ||
| progressLabel: str | ||
| result: RehearsalSong | ||
| error: AnalysisJobError |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "api.py" | grep -E "bandscope_analysis|analysis-engine"Repository: seonghobae/bandscope
Length of output: 120
🏁 Script executed:
find . -type f -name "index.ts" | grep -E "shared-types"Repository: seonghobae/bandscope
Length of output: 100
🏁 Script executed:
git ls-files services/analysis-engine/src/bandscope_analysis/ | head -20Repository: seonghobae/bandscope
Length of output: 496
🏁 Script executed:
git ls-files packages/shared-types/src/ | head -20Repository: seonghobae/bandscope
Length of output: 98
🏁 Script executed:
cat -n ./services/analysis-engine/src/bandscope_analysis/api.py | head -50Repository: seonghobae/bandscope
Length of output: 1712
🏁 Script executed:
cat -n ./packages/shared-types/src/index.tsRepository: seonghobae/bandscope
Length of output: 21541
🏁 Script executed:
grep -n "subprocess\|Popen\|call\|run" ./services/analysis-engine/src/bandscope_analysis/api.pyRepository: seonghobae/bandscope
Length of output: 225
🏁 Script executed:
python3 << 'EOF'
from typing import Literal, TypedDict
# Test the proposed inheritance pattern
class _AnalysisJobStatusRequired(TypedDict):
jobId: str
state: Literal["queued", "running", "succeeded", "failed"]
requestedAt: str
updatedAt: str
class AnalysisJobStatus(_AnalysisJobStatusRequired, total=False):
"""Typed analysis job snapshot shared with the desktop orchestrator."""
progressLabel: str
result: str # simplified for test
error: str # simplified for test
# Verify the type annotations
import typing
hints = typing.get_type_hints(AnalysisJobStatus)
print("AnalysisJobStatus type hints:")
for key, value in hints.items():
print(f" {key}: {value}")
# Check __required_keys__ and __optional_keys__
print(f"\nRequired keys: {AnalysisJobStatus.__required_keys__}")
print(f"Optional keys: {AnalysisJobStatus.__optional_keys__}")
EOFRepository: seonghobae/bandscope
Length of output: 455
AnalysisJobStatus의 모든 필드가 선택적으로 해석됩니다.
Line 34의 TypedDict(..., total=False)는 progressLabel, result, error만 선택적으로 만드는 것이 아니라 jobId, state, requestedAt, updatedAt도 모두 선택적으로 처리됩니다. 이는 packages/shared-types/src/index.ts의 TypeScript 계약(lines 95-99에서 네 필드 모두 필수)과 불일치하며, Python 타입 체커가 필수 필드의 누락을 감지하지 못합니다.
제안 수정안
+class _AnalysisJobStatusRequired(TypedDict):
+ jobId: str
+ state: Literal["queued", "running", "succeeded", "failed"]
+ requestedAt: str
+ updatedAt: str
+
-class AnalysisJobStatus(TypedDict, total=False):
+class AnalysisJobStatus(_AnalysisJobStatusRequired, total=False):
"""Typed analysis job snapshot shared with the desktop orchestrator."""
- jobId: str
- state: Literal["queued", "running", "succeeded", "failed"]
- requestedAt: str
- updatedAt: str
progressLabel: str
result: RehearsalSong
error: AnalysisJobError🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/api.py` around lines 34 - 43,
The AnalysisJobStatus TypedDict currently uses total=False which makes every
field optional; change it to explicitly mark only progressLabel, result, and
error as optional using NotRequired and keep jobId, state, requestedAt, and
updatedAt required. Import NotRequired (and Required if needed for your Python
version) from typing or typing_extensions, then update the AnalysisJobStatus
definition to annotate jobId, state, requestedAt, updatedAt as required and
progressLabel, result, error as NotRequired so the Python types match the
TypeScript contract.
Comment on lines
+69
to
+72
| if not isinstance(role_focus, list): | ||
| raise ValueError("Invalid analysis job request: invalid field 'roleFocus'") | ||
| if not all(isinstance(role, str) for role in role_focus): | ||
| raise ValueError("Invalid analysis job request: invalid field 'roleFocus[0]'") |
There was a problem hiding this comment.
잘못된 roleFocus 원소 위치가 항상 [0]로 고정됩니다.
Line 71-72는 리스트의 어느 항목이 잘못돼도 항상 roleFocus[0]를 반환합니다. 예를 들어 ["bass-guitar", 7]도 첫 번째 원소가 잘못된 것처럼 보고되어, 같은 계약을 검사하는 TypeScript 쪽 오류 메시지와 어긋날 수 있습니다.
🔧 제안 수정안
- if not all(isinstance(role, str) for role in role_focus):
- raise ValueError("Invalid analysis job request: invalid field 'roleFocus[0]'")
+ for index, role in enumerate(role_focus):
+ if not isinstance(role, str):
+ raise ValueError(
+ f"Invalid analysis job request: invalid field 'roleFocus[{index}]'"
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/api.py` around lines 69 - 72,
The error message for invalid elements in role_focus always points to
"roleFocus[0]" even when a later element is wrong; update the validation to
identify the exact failing index by iterating with an index (e.g., enumerate
over role_focus) and when a non-str element is found raise ValueError
referencing that index (e.g., "roleFocus[{i}]"); adjust the existing isinstance
checks around role_focus and the element checks to use this indexed error
message so it accurately reports which element failed.
Comment on lines
+14
to
+20
| payload = json.load(sys.stdin) | ||
| job_id = payload.get("jobId", "unknown-job") if isinstance(payload, dict) else "unknown-job" | ||
| request = payload.get("request") if isinstance(payload, dict) else payload | ||
| requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") | ||
| response = run_analysis_job(str(job_id), request, requested_at) | ||
| json.dump(response, sys.stdout) | ||
| return 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
fd "cli.py" -t f | grep -E "bandscope_analysis"Repository: seonghobae/bandscope
Length of output: 118
🏁 Script executed:
# Get the file content and examine lines around 14-20
cat -n services/analysis-engine/src/bandscope_analysis/cli.py | head -40Repository: seonghobae/bandscope
Length of output: 1013
🏁 Script executed:
# Check if there are subprocess invocations in this file
rg "subprocess|Popen|call|run" services/analysis-engine/src/bandscope_analysis/cli.py -A 2Repository: seonghobae/bandscope
Length of output: 237
🏁 Script executed:
# Check the overall structure and imports
head -30 services/analysis-engine/src/bandscope_analysis/cli.pyRepository: seonghobae/bandscope
Length of output: 845
🏁 Script executed:
# Look for test files to understand error handling expectations
fd "test.*cli|cli.*test" -t f | grep -E "bandscope|analysis"Repository: seonghobae/bandscope
Length of output: 106
🏁 Script executed:
# Examine the test file
cat -n services/analysis-engine/tests/test_cli.pyRepository: seonghobae/bandscope
Length of output: 4833
🏁 Script executed:
# Check if there are any error handling patterns elsewhere in the codebase
rg "json.load|JSONDecodeError" services/analysis-engine/src -A 3 -B 1Repository: seonghobae/bandscope
Length of output: 708
🏁 Script executed:
# Look for how this CLI is invoked - check for subprocess calls
rg "subprocess|Popen|run_analysis_job" services/analysis-engine -A 2 -B 1 | head -50Repository: seonghobae/bandscope
Length of output: 2754
🏁 Script executed:
# Check parent directory structure to understand the architecture
tree -L 3 services/analysis-engine/src -I "__pycache__"Repository: seonghobae/bandscope
Length of output: 328
JSON 파싱 오류 시 구조화된 실패 응답을 반환해야 합니다.
Line 14의 json.load(sys.stdin)은 malformed JSON 입력 시 JSONDecodeError를 발생시켜 프로세스를 종료합니다. 테스트 파일(test_cli.py 라인 91-102)에서 보듯이, 이 CLI는 모든 응답에 state와 error 필드가 포함된 구조화된 실패 envelope을 반환해야 합니다. JSON 구문 오류는 현재 예외로 처리되지 않아 호출자(subprocess, check=True)의 기대 계약을 위반합니다.
🔧 제안 수정안
def main() -> int:
"""Read a job payload from stdin and print a structured job response to stdout."""
+ requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z")
+ try:
- payload = json.load(sys.stdin)
+ payload = json.load(sys.stdin)
+ except json.JSONDecodeError as error:
+ json.dump(
+ {
+ "jobId": "unknown-job",
+ "state": "failed",
+ "requestedAt": requested_at,
+ "updatedAt": requested_at,
+ "error": {
+ "code": "invalid_request",
+ "message": f"Invalid analysis job request: {error.msg}",
+ },
+ },
+ sys.stdout,
+ )
+ return 0
+
job_id = payload.get("jobId", "unknown-job") if isinstance(payload, dict) else "unknown-job"
request = payload.get("request") if isinstance(payload, dict) else payload
- requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z")
response = run_analysis_job(str(job_id), request, requested_at)
json.dump(response, sys.stdout)
return 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/src/bandscope_analysis/cli.py` around lines 14 - 20,
Wrap the json.load(sys.stdin) call in a try/except that catches
json.JSONDecodeError and returns the structured failure envelope instead of
letting the exception propagate; specifically, if json.load fails create a
response object containing the required fields (e.g. {"state": "failed",
"error": "<parse error message>"}) and json.dump it to stdout and return 0,
otherwise proceed to compute response = run_analysis_job(str(job_id), request,
requested_at) as before; update the code paths around json.load(sys.stdin),
job_id/request extraction, and the final json.dump(response, sys.stdout) to
ensure malformed JSON yields the structured failure response.
Owner
Author
|
Superseded by #45 to clear CodeRabbit stale review state after the latest contract-hardening fixes and green verification. |
auto-merge was automatically disabled
March 12, 2026 01:52
Pull request was closed
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
Verification
./scripts/harness/quickcheck.shcargo check --manifest-path apps/desktop/src-tauri/Cargo.toml