feat: add local analysis orchestration#45
Closed
seonghobae wants to merge 5 commits into
Closed
Conversation
Owner
Author
|
@coderabbitai review |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Cache: Disabled due to Reviews > Disable Cache setting Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
Walkthrough데스크톱에 로컬 분석 오케스트레이션을 도입합니다: React ↔ Tauri IPC 명령, Rust 오케스트레이터(작업 큐·상태·동시성 제한), Python 엔진과 stdin/stdout 서브프로세스, 프런트엔드 폴링 UI, 공유 타입·검증·테스트·문서·권한 추가. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant React as React Frontend
participant Tauri as Tauri Bridge
participant Rust as Rust Orchestrator
participant Python as Python Engine
User->>React: Start Analysis 클릭
React->>Tauri: invoke start_analysis_job(request)
Tauri->>Rust: IPC start_analysis_job
Rust->>Rust: validate & enqueue (state: queued)
Rust-->>Tauri: AnalysisJobStatus{jobId, state: queued}
Tauri-->>React: queued 응답
loop Polling
React->>Tauri: invoke get_analysis_job_status(jobId)
Tauri->>Rust: IPC get_analysis_job_status
Rust->>Rust: 조회 및 상태 반환
Rust-->>Tauri: AnalysisJobStatus{state, progress/result?}
Tauri-->>React: 상태 업데이트
end
Note over Rust,Python: 백그라운드 워커
Rust->>Python: spawn subprocess (stdin: request JSON)
Python->>Python: stdin 읽기, validate, run
Python-->>Rust: stdout에 AnalysisJobStatus JSON
Rust->>Rust: job map 갱신 (succeeded/failed)
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: 10
🤖 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 391-404: The current match on process.wait_with_output() can block
indefinitely if the child hangs; add a timeout-based wait path that enforces a
deadline (e.g., using a Duration constant) and if the timeout elapses, send a
kill to the child process (call child.kill() or equivalent), wait for it to exit
(child.wait() / wait_with_output), and then return the failed_status (using
AnalysisJobErrorCode::EngineUnavailable or a new timeout code) so the job slot
is released; update the block around process.wait_with_output() to use a
timed-wait approach and ensure any errors from kill/wait are logged and still
result in calling failed_status for the same jobId so release_job_slot() can
proceed.
In `@apps/desktop/src/App.tsx`:
- Around line 123-149: The Start button stays enabled until jobStatus is set,
allowing multiple clicks before the first await completes; add a local
"isStarting" state (e.g., useState<boolean>) and update handleStartAnalysis to
set isStarting = true immediately on click, then set it back to false in a
finally block after the await startAnalysisJob(...) completes (or errors), and
update the button disabled prop to disabled={analysisInFlight || isStarting};
reference handleStartAnalysis, startAnalysisJob, analysisInFlight, and
setJobStatus when making these changes.
- Around line 104-117: The catch block only sets jobError which leaves jobStatus
stuck in "queued"/"running" and prevents retry; update the error handler to also
move the jobStatus into a terminal/failable state so the UI can unlock and
polling can be retried — e.g., call setJobStatus to a failed state (or null) in
the catch, and keep the existing setJobError(t("analysisCouldNotStart")) so the
UI shows the error; adjust use of jobStatus/setJobStatus in that async timeout
callback to ensure a terminal state is written on exception.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 31-38: The current browser fallback builds a deterministic jobId
from request.sourceLabel (see parseAnalysisJobRequest and the jobId assignment)
which causes collisions and overwrites in browserJobStore when the same demo
runs again; change jobId generation to include a unique suffix (e.g., timestamp,
random nonce, or UUID) so each call creates a distinct id before calling
createAnalysisJobStatus and browserJobStore.set; update any places that
reference jobId in this scope so they use the new unique id.
- Around line 82-89: startAnalysisJob currently lets parseAnalysisJobRequest
throw, causing a rejected promise; instead catch parse errors and return a
proper AnalysisJobStatus representing a failed/invalid_request so callers get
the typed failure. Change startAnalysisJob to try { const parsed =
parseAnalysisJobRequest(request); ... invokeAnalysis(...) } catch (err) { return
a resolved AnalysisJobStatus with failure status, error message, and the same
invalid_request envelope/identifier used elsewhere }, and keep the existing
isAnalysisJobStatus check for invokeAnalysis responses. Reference:
startAnalysisJob, parseAnalysisJobRequest, invokeAnalysis, and
isAnalysisJobStatus.
- Around line 21-27: The getInvoke() helper currently always returns null
because no Tauri invoke is wired; update the desktop bootstrap so getInvoke()
can return the real Tauri invoke by importing and using the Tauri core provider
or by ensuring window.__TAURI_INVOKE__ is injected at startup; specifically,
either import the invoke provider from "@tauri-apps/api/core" (or use the tauri
init routine) and assign it to window.__TAURI_INVOKE__, or modify getInvoke() to
call the official provider (referencing the getInvoke helper and
browserFallback() usage) so desktop builds receive the real invoke instead of
falling back to browserFallback(). Ensure the chosen approach unblocks local
Rust orchestration and that getInvoke() returns a TauriInvoke when running in
the Tauri desktop environment.
In `@apps/desktop/src/locales/en/common.json`:
- Around line 15-19: The two JSON keys "analysisCouldNotStart" and
"analysisStateFailed" currently use the same English string, which prevents
users from distinguishing a failure to start vs a failure that occurred while
running; update "analysisStateFailed" to a distinct message (e.g., "Analysis
failed during execution" or similar) in apps/desktop/src/locales/en/common.json
and make the equivalent wording change in
apps/desktop/src/locales/ko/common.json so both locales clearly differentiate
start-fail vs runtime-fail states.
In `@packages/shared-types/src/index.ts`:
- Around line 354-406: The current validators (validateAnalysisJobStatus and
validateAnalysisJobError) allow unexpected fields and mixed state payloads;
update them to enforce a strict schema: in validateAnalysisJobStatus, first
ensure value is a plain record, then compute its keys and compare against
state-specific allowed key sets (for "succeeded" allow only
["jobId","state","requestedAt","updatedAt","progressLabel","result"], for
"failed" allow only
["jobId","state","requestedAt","updatedAt","progressLabel","error"], for
"queued"/"running" allow only
["jobId","state","requestedAt","updatedAt","progressLabel"]); if any extra or
missing required keys are present return invalidField with the offending key.
Also ensure that "succeeded" requires result and disallows error, and "failed"
requires error and disallows result. In validateAnalysisJobError, enforce that
the record has exactly the allowed keys (e.g., ["code","message"]) and reject
extra fields via invalidField(`${path}.<key>`). Likewise update
validateRehearsalSong (used for value.result) to reject unexpected fields so
nested results are strict. Use Object.keys and set membership checks to identify
offending symbols and return appropriate invalidField messages; reference
functions validateAnalysisJobStatus, validateAnalysisJobError, and
validateRehearsalSong for where to add these checks.
In `@services/analysis-engine/src/bandscope_analysis/api.py`:
- Around line 25-31: RehearsalSong currently uses loose dict[str, object] for
sections and exportSummary which prevents static checking of the nested
contract; define explicit TypedDicts (e.g., SectionTypedDict and
ExportSummaryTypedDict) matching the shared Rust/TS contract, replace sections:
list[dict[str, object]] with sections: list[SectionTypedDict] and exportSummary:
dict[str, object] with exportSummary: ExportSummaryTypedDict, and update any
usages/tests to the new types so static checkers can catch drift (refer to the
RehearsalSong, sections, and exportSummary symbols when locating the code to
change).
In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 34-37: The payload handling in
services/analysis-engine/src/bandscope_analysis/cli.py currently coerces any
jobId into a string before calling run_analysis_job, which can break
correlation; update the logic that builds job_id/request so it first ensures
payload is a dict and payload.get("jobId") is a non-empty str (e.g.,
isinstance(payload, dict) and isinstance(payload.get("jobId"), str) and
payload["jobId"].strip()), and if that check fails return an invalid_request
response (instead of calling run_analysis_job) — keep using the same symbols
(job_id, request, run_analysis_job, requested_at) so callers remain unchanged
but invalid jobId paths early-return an error.
🪄 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: 97d4fadf-58dd-4feb-84c4-07bc187b3a3d
⛔ 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 (24)
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/i18n/index.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsondocs/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
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 74-168: The parsed AnalysisJobStatus (from
serde_json::from_slice::<AnalysisJobStatus>) is being accepted without enforcing
valid combinations of state/result/error and allowed enum-like values; add a
small validator (e.g., validate_analysis_job_status) that inspects
AnalysisJobStatus.state (AnalysisJobState) and ensures required fields are
present for each state (e.g., state==Succeeded requires result:
RehearsalSongPayload populated and valid nested fields; state==Failed requires
error: AnalysisJobError; InProgress/Pending allow progress_label but no
result/error), and also validate nested enum-like strings (e.g., role_type,
cue.kind, confidence.level, export_summary.format) against the allowed set; call
this validator immediately after serde deserialization where you currently call
serde_json::from_slice::<AnalysisJobStatus>, and if validation fails map to
failed_status(original_job_id, "Analysis engine returned an invalid response.")
before persisting.
- Around line 380-393: The child process's stdin is being written to but not
closed, so the Python CLI waiting on EOF may hang; replace the current as_mut()
usage with taking ownership of the stdin handle (use process.stdin.take()),
write_all to that owned ChildStdin, then let it drop (or explicitly drop it)
immediately after writing to send EOF; keep the existing error path (call
process.kill() and return failed_status with payload/jobId, requested_at, and
AnalysisJobErrorCode::EngineUnavailable) if write_all fails.
In `@apps/desktop/src/App.tsx`:
- Around line 113-115: The code currently surfaces backend error.message
directly to users (see use of setJobError(nextStatus.error?.message ??
t("analysisCouldNotStart")) in App.tsx), which can leak validation/contract
text; instead map backend error.code to a user-facing translation key (use
nextStatus.error?.code to select t(...) fallbacking to a generic
t("analysisCouldNotStart")), setJobError to that translated message, and send
the raw error.message and stack to developer logs/diagnostics (console.error or
telemetry) for troubleshooting; apply the same change for the similar occurrence
around the 136-137 area so all user-visible error UI uses translated codes
rather than raw messages.
In `@apps/desktop/src/lib/analysis.ts`:
- Around line 22-28: The getInvoke function currently returns the imported
invoke whenever window exists, causing non-Tauri environments (browsers, Vitest)
to use Tauri invoke and throw; update getInvoke to import and call isTauri()
from "@tauri-apps/api/core" and only return window.__TAURI_INVOKE__ ?? invoke
when isTauri() is true, otherwise return the browserFallback; apply the same
isTauri() check and fallback replacement in the similar logic around the code
referenced at lines 74-80 so both call sites use isTauri() before selecting
invoke.
In `@services/analysis-engine/tests/test_api.py`:
- Around line 60-67: The test
test_build_demo_rehearsal_song_matches_expected_fixture relies on a hard-coded
role index (song["sections"][0]["roles"][2]) which is brittle; update the test
to locate the role by id instead of by index: within the song returned by
build_demo_rehearsal_song(), find the section (song["sections"][0]) then search
its "roles" list for the role with id "lead-vocal" (or the expected id), assert
that the role exists, and then assert that its
manualOverrides[0]["value"]["source"] == "user"; keep the title assertion as-is
and add a clear failure message if the role is missing to aid debugging.
🪄 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: d8e68f48-e60d-4cc1-ba94-686d76f145c6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
apps/desktop/package.jsonapps/desktop/src-tauri/src/main.rsapps/desktop/src/App.test.tsxapps/desktop/src/App.tsxapps/desktop/src/lib/analysis.tsapps/desktop/src/locales/en/common.jsonapps/desktop/src/locales/ko/common.jsonpackages/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
+74
to
+168
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct RehearsalSongPayload { | ||
| id: String, | ||
| title: String, | ||
| sections: Vec<RehearsalSectionPayload>, | ||
| export_summary: ExportSummaryPayload, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct ConfidencePayload { | ||
| level: String, | ||
| source: String, | ||
| notes: String, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct CuePayload { | ||
| kind: String, | ||
| value: String, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct RangePayload { | ||
| lowest_note: String, | ||
| highest_note: String, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct HarmonyPayload { | ||
| chord: String, | ||
| function_label: String, | ||
| source: String, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct ManualOverridePayload { | ||
| field: String, | ||
| value: HarmonyPayload, | ||
| source: String, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct RehearsalRolePayload { | ||
| id: String, | ||
| name: String, | ||
| role_type: String, | ||
| harmony: HarmonyPayload, | ||
| cue: CuePayload, | ||
| range: RangePayload, | ||
| confidence: ConfidencePayload, | ||
| rehearsal_priority: String, | ||
| simplification: String, | ||
| setup_note: String, | ||
| manual_overrides: Vec<ManualOverridePayload>, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct RehearsalSectionPayload { | ||
| id: String, | ||
| label: String, | ||
| groove: String, | ||
| confidence: ConfidencePayload, | ||
| roles: Vec<RehearsalRolePayload>, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct ExportSummaryPayload { | ||
| format: String, | ||
| headline: String, | ||
| focus_sections: Vec<String>, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
| struct AnalysisJobStatus { | ||
| job_id: String, | ||
| state: AnalysisJobState, | ||
| requested_at: String, | ||
| updated_at: String, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| progress_label: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| result: Option<RehearsalSongPayload>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| error: Option<AnalysisJobError>, | ||
| } |
There was a problem hiding this comment.
엔진 stdout contract 검증이 아직 느슨합니다.
지금은 serde_json::from_slice::<AnalysisJobStatus>만 통과하면 그대로 저장되는데, 이 타입은 state/result/error 조합과 nested enum-like 값들을 엄격히 막지 못합니다. 그래서 child가 shared-types contract와 어긋난 0-exit envelope를 내보내도 Rust에서는 저장되고, 프론트가 poll 시점에 첫 strict validator가 되어 예외로 끝나게 됩니다. 저장 전에 상태별 필수 필드와 허용 값까지 검증하고, 어긋난 envelope는 failed_status(..., "Analysis engine returned an invalid response.")로 매핑하는 편이 안전합니다.
Also applies to: 458-468
🤖 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 74 - 168, The parsed
AnalysisJobStatus (from serde_json::from_slice::<AnalysisJobStatus>) is being
accepted without enforcing valid combinations of state/result/error and allowed
enum-like values; add a small validator (e.g., validate_analysis_job_status)
that inspects AnalysisJobStatus.state (AnalysisJobState) and ensures required
fields are present for each state (e.g., state==Succeeded requires result:
RehearsalSongPayload populated and valid nested fields; state==Failed requires
error: AnalysisJobError; InProgress/Pending allow progress_label but no
result/error), and also validate nested enum-like strings (e.g., role_type,
cue.kind, confidence.level, export_summary.format) against the allowed set; call
this validator immediately after serde deserialization where you currently call
serde_json::from_slice::<AnalysisJobStatus>, and if validation fails map to
failed_status(original_job_id, "Analysis engine returned an invalid response.")
before persisting.
Comment on lines
+113
to
+115
| if (nextStatus.state === "failed") { | ||
| setJobError(nextStatus.error?.message ?? t("analysisCouldNotStart")); | ||
| } |
There was a problem hiding this comment.
백엔드 진단 문구를 그대로 UI에 노출하지 않는 편이 좋겠습니다.
여기서는 error.message를 바로 보여 줘서 "Invalid analysis job request: invalid field 'sourceKind'" 같은 계약/검증 문구가 사용자에게 그대로 노출됩니다. 사용자용 문구는 error.code를 번역 키로 매핑하고, 상세 원인은 로그나 개발자 진단으로 분리해 주세요.
As per coding guidelines, "Prefer practical, friendly, rehearsal-first wording over academic or authority-heavy language in UX copy and UI text".
Also applies to: 136-137
🤖 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 113 - 115, The code currently surfaces
backend error.message directly to users (see use of
setJobError(nextStatus.error?.message ?? t("analysisCouldNotStart")) in
App.tsx), which can leak validation/contract text; instead map backend
error.code to a user-facing translation key (use nextStatus.error?.code to
select t(...) fallbacking to a generic t("analysisCouldNotStart")), setJobError
to that translated message, and send the raw error.message and stack to
developer logs/diagnostics (console.error or telemetry) for troubleshooting;
apply the same change for the similar occurrence around the 136-137 area so all
user-visible error UI uses translated codes rather than raw messages.
Comment on lines
+22
to
+28
| function getInvoke(): TauriInvoke | null { | ||
| if (typeof window === "undefined") { | ||
| return null; | ||
| } | ||
|
|
||
| return window.__TAURI_INVOKE__ ?? invoke; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Tauri v2 @tauri-apps/api/core, what is the official way to detect whether code is running inside a Tauri runtime before calling invoke, and what happens if invoke is called in a normal browser?
💡 Result:
Official runtime check (Tauri v2)
Use isTauri() from @tauri-apps/api/core before calling invoke:
import { isTauri, invoke } from "@tauri-apps/api/core";
if (isTauri()) {
const result = await invoke("my_command");
}isTauri(): boolean is the documented API for detecting whether you’re running inside the Tauri runtime. [1]
What happens in a normal browser if you call invoke
In Tauri v2, invoke() calls window.__TAURI_INTERNALS__.invoke(...). [2] In a normal browser, window.__TAURI_INTERNALS__ is not present, so the call fails and the returned promise rejects with a JavaScript TypeError (due to trying to access .invoke on undefined). [2]
Sources:
[1] Tauri JS API reference – core.isTauri() docs. (v2.tauri.app)
[2] Tauri @tauri-apps/api implementation – core.invoke() uses window.__TAURI_INTERNALS__.invoke. (github.com)
Citations:
- 1: https://v2.tauri.app/reference/javascript/api/namespacecore/
- 2: https://github.com/tauri-apps/tauri/blob/dev/packages/api/src/core.ts
isTauri() 체크를 추가하여 Tauri 런타임만에서 invoke를 사용해야 합니다.
현재 코드는 window가 존재하는 모든 환경(일반 브라우저, Vitest 포함)에서 import된 invoke를 반환하므로, browserFallback()로 내려가지 않습니다. 일반 브라우저에서 invoke를 호출하면 window.__TAURI_INTERNALS__가 없어 TypeError가 발생합니다. 대신 @tauri-apps/api/core의 isTauri() API를 사용하여 Tauri 런타임인지 먼저 확인한 후에만 import된 invoke를 선택하세요.
import { isTauri, invoke } from "@tauri-apps/api/core";
function getInvoke(): TauriInvoke | null {
if (typeof window === "undefined") {
return null;
}
if (isTauri()) {
return window.__TAURI_INVOKE__ ?? invoke;
}
return browserFallback;
}Also applies to: 74-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/analysis.ts` around lines 22 - 28, The getInvoke
function currently returns the imported invoke whenever window exists, causing
non-Tauri environments (browsers, Vitest) to use Tauri invoke and throw; update
getInvoke to import and call isTauri() from "@tauri-apps/api/core" and only
return window.__TAURI_INVOKE__ ?? invoke when isTauri() is true, otherwise
return the browserFallback; apply the same isTauri() check and fallback
replacement in the similar logic around the code referenced at lines 74-80 so
both call sites use isTauri() before selecting invoke.
Comment on lines
+60
to
+67
| def test_build_demo_rehearsal_song_matches_expected_fixture() -> None: | ||
| """Ensure the bootstrap demo result is present and player-relevant.""" | ||
| song = build_demo_rehearsal_song() | ||
|
|
||
| assert song["title"] == "Late Night Set" | ||
| assert song["sections"][0]["roles"][0]["id"] == "bass-guitar" | ||
| assert song["sections"][0]["roles"][2]["manualOverrides"][0]["value"]["source"] == "user" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
데모 픽스처 테스트가 특정 인덱스에 의존합니다.
song["sections"][0]["roles"][2]가 lead-vocal이고 manualOverrides가 있다고 가정합니다. build_demo_rehearsal_song()의 역할 순서가 변경되면 이 테스트가 실패할 수 있습니다.
♻️ 인덱스 대신 id로 역할 찾기 제안
def test_build_demo_rehearsal_song_matches_expected_fixture() -> None:
"""Ensure the bootstrap demo result is present and player-relevant."""
song = build_demo_rehearsal_song()
assert song["title"] == "Late Night Set"
assert song["sections"][0]["roles"][0]["id"] == "bass-guitar"
- assert song["sections"][0]["roles"][2]["manualOverrides"][0]["value"]["source"] == "user"
+ lead_vocal = next(r for r in song["sections"][0]["roles"] if r["id"] == "lead-vocal")
+ assert lead_vocal["manualOverrides"][0]["value"]["source"] == "user"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/analysis-engine/tests/test_api.py` around lines 60 - 67, The test
test_build_demo_rehearsal_song_matches_expected_fixture relies on a hard-coded
role index (song["sections"][0]["roles"][2]) which is brittle; update the test
to locate the role by id instead of by index: within the song returned by
build_demo_rehearsal_song(), find the section (song["sections"][0]) then search
its "roles" list for the role with id "lead-vocal" (or the expected id), assert
that the role exists, and then assert that its
manualOverrides[0]["value"]["source"] == "user"; keep the title assertion as-is
and add a clear failure message if the role is missing to aid debugging.
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
Owner
Author
|
Superseded by #46 to clear stale CodeRabbit review state after the latest orchestration hardening fixes and green verification. |
auto-merge was automatically disabled
March 12, 2026 03:06
Pull request was closed
✅ Actions performedReview triggered.
|
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📝 Walkthrough
Walkthrough
새로운 분석 작업 오케스트레이션 계층을 구현합니다. Tauri IPC 명령어, 공유 타입 계약, Python 서브프로세스 통신, 데스크톱 폴링 UI, Rust 기반 작업 상태 관리, 그리고 관련 테스트 및 문서를 추가합니다.
Changes
ARCHITECTURE.md,docs/architecture/overview.md,docs/plans/2026-03-12-issue-32-analysis-orchestration-design.md,docs/plans/2026-03-12-issue-32-analysis-orchestration.md,docs/security/app-security.mdapps/desktop/src-tauri/Cargo.toml,apps/desktop/src-tauri/build.rs,apps/desktop/src-tauri/tauri.conf.json,apps/desktop/src-tauri/capabilities/main.json,apps/desktop/src-tauri/permissions/autogenerated/*apps/desktop/src-tauri/src/main.rsapps/desktop/src/App.tsx,apps/desktop/src/App.test.tsxapps/desktop/src/lib/analysis.ts,apps/desktop/src/i18n/index.tsapps/desktop/src/locales/en/common.json,apps/desktop/src/locales/ko/common.jsonpackages/shared-types/src/index.ts,packages/shared-types/test/index.test.tsservices/analysis-engine/src/bandscope_analysis/api.py,services/analysis-engine/src/bandscope_analysis/cli.py,services/analysis-engine/tests/test_api.py,services/analysis-engine/tests/test_cli.pySequence Diagram(s)
sequenceDiagram actor User participant React as React Frontend participant Tauri as Tauri Bridge participant Rust as Rust Orchestrator participant Python as Python Engine User->>React: Click Start Analysis React->>Tauri: startAnalysisJob(request) Tauri->>Rust: IPC invoke Rust->>Rust: Validate & enqueue (queued state) Rust->>Rust: Spawn worker thread Rust-->>Tauri: Return jobId + queued status Tauri-->>React: AnalysisJobStatus{jobId, state:queued} React->>React: Poll get_analysis_job_status loop While Queued/Running React->>Tauri: getAnalysisJobStatus(jobId) Tauri->>Rust: IPC invoke Rust->>Rust: Lookup in job map Rust-->>Tauri: Return current status Tauri-->>React: AnalysisJobStatus{state} React->>React: Update UI progress end Note over Rust,Python: Background worker thread Rust->>Python: Spawn subprocess with args Python->>Python: Read stdin (AnalysisJobRequest) Python->>Python: Validate & run analysis Python->>Python: Build demo result Python-->>Rust: Output JSON (AnalysisJobStatus) Rust->>Rust: Update job map (succeeded/failed) React->>Tauri: getAnalysisJobStatus(jobId) Tauri->>Rust: IPC invoke Rust-->>Tauri: Return succeeded status + result Tauri-->>React: AnalysisJobStatus{state:succeeded, result} React->>React: Render analysis result React-->>User: Display rehearsal songEstimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem