feat(workspace): start analysis from the first-run card - #974
Conversation
After a local or YouTube source is admitted, replace the empty-file prompt with a first-run card that names Analyze this song as the next action and records tonight's part before the job starts. This is a #964 first-run buyer slice. It does not add a mocked demo analysis path and does not invent a parallel MIR product. Security Notes: - Untrusted input: selected file names are reduced to a basename before any first-run attribute is emitted; role IDs are a closed allowlist. - Trust boundary: UI role choice maps onto existing analysis roleFocus values. Local paths never render. - Safe failure: analysis still starts only after an explicit click.
|
Warning Review limit reached
Next review available in: 45 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough오디오 선택 후 빈 상태 대신 첫 실행 분석 카드가 표시됩니다. 사용자는 전체 밴드 또는 특정 파트를 선택할 수 있습니다. 선택한 역할은 분석 요청의 Changes첫 실행 분석
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to The new first-run flow guides musicians from source selection into analysis, but the current change still has a lint-blocking declaration issue and does not show the selected song name on the card; merge should wait for those fixes, with the YouTube changelog wording updated as follow-up. Sequence Diagram(s)sequenceDiagram
participant App
participant FirstRunState
participant AnalysisJob
App->>FirstRunState: 선택한 파일과 기본 역할 표시
FirstRunState->>App: 사용자가 역할 선택
App->>AnalysisJob: 선택한 roleFocus로 분석 시작
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| onChooseDifferentFile={() => { | ||
| void handleChooseLocalAudio(); | ||
| }} |
There was a problem hiding this comment.
🟡 Cancelling 'Choose a different file' discards the selected song
Opening the file picker from the card's "Choose a different file" button and then cancelling runs handleChooseLocalAudio, whose failure branch calls setSelectedBootstrap(null) and shows a "choose a WAV/MP3..." error. The already-selected song is dropped and the user falls back to the empty prompt, since cancellation is indistinguishable from an unsupported file.
Prompt for agents
The first-run card's "Choose a different file" button calls handleChooseLocalAudio (apps/desktop/src/App.tsx). When the user cancels the native file dialog, the Rust command select_local_audio_source returns Err("Choose a WAV, MP3, FLAC, or M4A file to start analysis.") (apps/desktop/src-tauri/src/main.rs:644), and selectLocalAudioSource in apps/desktop/src/lib/analysis.ts collapses any non-safe error into that same unsupported message, so a user cancellation cannot be distinguished from a genuinely unsupported file. As a result handleChooseLocalAudio runs its failure branch: it clears the current selection (setSelectedBootstrap(null)) and shows the unsupported-file error, dropping the user from the first-run card back to the empty prompt even though they only cancelled. Consider surfacing a distinct cancellation signal (e.g. a dedicated error code/message from the Rust command and a check like the existing isUserCancellation helper) so that cancelling the picker leaves the previously selected source and first-run card intact instead of wiping it and showing a false error.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Revalidated on current live heads. This finding is still valid on #974 exact fd33104ca88030b5218e1c56a0cb683e2b50d7db, so I am leaving this thread unresolved. The cancellation protocol is already owned by canonical #976 current exact d07dc57347ce8187bc70b1d1f01b1ac5ec195887: its handleChooseLocalAudio explicitly checks isUserCancellation(...) and returns without clearing the admitted bootstrap or surfacing an unsupported-format error. #974 should inherit/restack that owner rather than duplicate a second native-cancellation authority. Acceptance for this thread is #974 on a head that contains that canonical cancellation behavior plus the first-run replacement-picker regression; predecessor/#976-only evidence is not sufficient to resolve it.
| onChooseDifferentFile={() => { | ||
| void handleChooseLocalAudio(); | ||
| }} | ||
| analysisDisabled={analysisInFlight || isStarting || isImporting} |
|
Fresh ownership routing for the unresolved picker-cancellation finding on exact head
Do not duplicate that authority in this leaf. Keep this first-run lane unready until the #976 cancellation boundary is integrated/restacked here, then add/retain the buyer-flow regression |
|
@opencode-agent Continue only the existing canonical branch Repair only the independent current static finding in |
|
@opencode-agent Continue only the existing |
Product outcome
A musician who has already chosen a local song no longer stares at the empty-file prompt. The workspace names Analyze this song as the next rehearsal action and asks only which part they are playing tonight.
Parent buyer gap: #964 (first-run journey). This slice does not add a mocked demo analysis, licensed demo audio, or a parallel MIR product. #828 still owns #770.
Exact current identity
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.feat/first-run-analyze-selected-song.822a44576223fd620c05c44498ea1353edaffc16.What landed
EmptyStateis replaced by a first-run card.roleFocusIDs (bass-guitar,keys-right,lead-vocal). Unknown IDs are not admitted.enandkoand enables the next action instead of describing the gap.Verification
Desktop
vitest --coverageon this head: 205 passed, App.tsx 100% statements/branches/functions/lines. No lockfile, dependency, model, or review-credential changes.Security Notes
AnalysisJobRequest.roleFocus. Basename sanitization before any first-run attribute.Dependency / merge gate
pdfjs-dist/nanoid/undici). This branch must not copy or suppress that baseline.APPROVE; and ordinary branch-protection acceptance.Summary by CodeRabbit
새 기능
다국어 지원
테스트