feat(workspace): guide tonight's first outro on the rehearsal map - #986
feat(workspace): guide tonight's first outro on the rehearsal map#986seonghobae wants to merge 25 commits into
Conversation
Name the earliest labeled outro and the holding part so the room can finish together. Map Open scrolls the renderer-owned section; player Hear only appears when a seek callback exists. Do not invent an ending from intro, verse, chorus, tag, pickup, stop, handoff, or unlabeled sections.
|
Warning Review limit reachedNext included review available in 29 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthrough첫 번째 유효한 아웃트로를 식별하고 holding role과 시작 시간을 표시합니다. 워크스페이스는 해당 구간으로 이동합니다. 플레이어는 seek 콜백이 있을 때만 Hear 동작을 제공합니다. 영어와 한국어 문구 및 런타임 검증 테스트를 추가했습니다. Changes첫 번째 아웃트로 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The new outro guidance may display playback times with fractional seconds instead of the documented m:ss format. The PR is otherwise mergeable with owner awareness and a follow-up to normalize or reject fractional inputs. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant FirstOutroCallout
participant resolveFirstOutro
participant SongStructureGrid
Workspace->>FirstOutroCallout: song 전달
FirstOutroCallout->>resolveFirstOutro: 첫 아웃트로 해석
resolveFirstOutro-->>FirstOutroCallout: section, holdingRole, atSeconds 반환
FirstOutroCallout->>SongStructureGrid: 대상 구간으로 스크롤
sequenceDiagram
participant PlayerFeature
participant FirstOutroCallout
participant PlaybackSurface
PlayerFeature->>FirstOutroCallout: song과 선택적 seek callback 전달
FirstOutroCallout->>PlaybackSurface: Hear 동작 표시
PlaybackSurface-->>PlayerFeature: atSeconds로 seek callback 실행
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Re-review request for exact head |
# Conflicts: # AGENTS.md # CLAUDE.md # apps/desktop/src/features/workspace/Workspace.tsx # apps/desktop/src/locales/en/common.json # apps/desktop/src/locales/ko/common.json
|
Merged current develop into this branch: resolved shared-file conflicts by including both this feature's first-outro callout and the newly landed #957 playable-range work (Workspace.tsx keeps both imports; en/ko locales unioned with symmetric keys; AGENTS/CLAUDE unioned). No behavior dropped. |
1 participant
Product outcome
After analysis, the mounted rehearsal workspace names tonight's first labeled outro so the room can finish together, then Open moves to the matching rendered map section.
The existing
PlayerFeatureplaceholder is not mounted byApp.tsxand therefore is not playback authority. It may summarize the analyzed outro, but it must not accept a seek callback or expose a buyer-facing Hear action until a real mounted playback owner exists. This keeps the slice honest instead of manufacturing an unreachable control.Exact current identity
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.4222d99c025723329e522f98b7088486a4c3b5ca.feat/workspace-first-outro.Current exact scope
resolveFirstOutropicks the earliest labeledoutrowith a bounded rehearsal window and the highest-priority unique active part.intro,verse,chorus,tag,pickup,stop,handoff, or the last unlabeled section.Openscrolls the renderer-owned song-structure section; analysissection.idis never DOM-ID authority.PlayerFeatureremains guidance-only and does not acceptonPlayFromSeconds; a type-level regression prevents restoring that false authority accidentally.behavior: auto.Review repair and verification
Current-head review correctly found that the previous
PlayerFeaturecallback contract was unreachable in production. The repair was test-first:4eea95amade the unreachable action fail the regression,ae63713removed seek authority from the placeholder, and6a1de2dpinned the type/runtime boundary. Canonical docs and CHANGELOG were then made code-current.On unchanged head
4222d99c025723329e522f98b7088486a4c3b5ca, repositorycirun32609617359is terminal-success, includinggate / ci / rust-check;release, SBOM, Semgrep, Bandit, and secret-scan are also terminal-success. The exact security-audit and aggregate Security Scan failures are inherited protected-base npm findings (nanoid,pdfjs-dist,undici, includingpdfjs-distCVE-2026-16633) and remain canonical #783 ownership; no suppression or leaf dependency workaround is added here.build-baselineremains non-terminal and is not counted as success.The unreachable-Hear review finding is now resolved after the exact-head CI proof. A later informational note observes that the current
[data-section-index]navigation selector is document-scoped; today only the song-structure grid owns that attribute, so it is not treated as a present misnavigation defect.Merge gate
Keep unmerged until this unchanged exact head has every applicable repository and central CI/security/SAST/SBOM/coverage/review gate terminal-success, zero valid unresolved defects, canonical dependency prerequisites integrated and revalidated, a qualifying independent non-author last-push
APPROVE, and ordinary protected-branch acceptance. Never self-approve or transfer predecessor evidence.Queued, pending, skipped-required, cancelled, failed, predecessor-head, protected-base, model-only, author, or administrative-bypass evidence is non-passing. Inherited #783 npm HIGH findings must not be suppressed here.