feat(workspace): name tonight's first turnaround plan on the map - #1036
feat(workspace): name tonight's first turnaround plan on the map#1036seonghobae wants to merge 26 commits into
Conversation
Name the earliest corroborated shared continuation so the band can turn the last bars together and land the next downbeat. Engine copy is emitted only from mixed-source stem continuation into the next section; cutoff, heuristic topology, and a lone continuing part stay unnamed. Open scrolls the renderer-owned song-structure section.
|
Warning Review limit reachedNext included review available in 26 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 (4)
📝 WalkthroughWalkthrough분석 엔진이 공유 연속성에서 첫 turnaround plan을 생성합니다. 공유 타입과 검증을 확장합니다. 데스크톱 workspace가 계획을 현지화해 표시하고 대응 섹션으로 이동합니다. 관련 테스트와 문서를 추가합니다. ChangesTurnaround Plan 흐름
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The feature adds a first-turnaround callout and map navigation, but the current head contains a failing navigation-failure assertion that can block CI and should be fixed before merge; additionally, future engine wording changes could expose English text to Korean users. Sequence Diagram(s)sequenceDiagram
participant ActivityAnalysis
participant RoleExtractor
participant RehearsalSong
participant Workspace
participant FirstTurnaroundPlanCallout
participant SongStructureRenderer
ActivityAnalysis->>RoleExtractor: stem 활동과 섹션 데이터 제공
RoleExtractor->>RehearsalSong: turnaroundPlan과 model 출처 저장
Workspace->>FirstTurnaroundPlanCallout: song 전달
FirstTurnaroundPlanCallout->>RehearsalSong: 첫 계획 조회
FirstTurnaroundPlanCallout->>SongStructureRenderer: 대응 섹션 탐색
FirstTurnaroundPlanCallout->>SongStructureRenderer: scrollIntoView 실행
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 89.36% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 31 files. (2 skipped: 2 unsupported.) ✨ 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 |
seonghobae
added
area: ui-ux
Grok (by xAI)
![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.
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.
# Conflicts: # AGENTS.md # CHANGELOG.md # CLAUDE.md # apps/desktop/src/features/workspace/Workspace.tsx # apps/desktop/src/locales/en/common.json # apps/desktop/src/locales/ko/common.json
|
Resolved merge conflicts against origin/develop (749511c): union-merged AGENTS.md/CHANGELOG.md bullets, sentence-merged CLAUDE.md, kept both import sets in Workspace.tsx (FirstTurnaroundPlanCallout + firstRangeSqueeze), unioned en/ko locale tails (JSON valid, en/ko symmetric). Merged legacy .Jules/palette.md into canonical .jules/palette.md, dropped alias. No conflict markers remain. Merge head: b7577d7. |
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.
| simplification: "Stay on roots if the chorus entrance gets muddy.", | ||
| setupNote: "Keep the attack short so the verse breathes.", | ||
| transpositionPlan: "If the singer drops to B minor, keep the shape a whole step lower and let keys keep the color tones.", | ||
| turnaroundPlan: "Turn these last bars with Lead Vocal on the verse last beat; land the chorus downbeat together.", |
There was a problem hiding this comment.
🔴 Demo workspace never shows tonight's turnaround plan
The demo song's only section carries the turnaround plan (turnaroundPlan here), but resolveFirstTurnaroundPlan names a plan only for a section followed by another section. The browser-fallback/demo workspace therefore always renders "No turnaround plan is available" and the action button never appears.
Prompt for agents
The turnaround-plan resolver in apps/desktop/src/features/workspace/firstTurnaroundPlan.ts (resolveSafeFirstTurnaroundPlan) intentionally only names a turnaround plan for a section that continues into a following section, matching the Python engine (turnaround = shared continuation into the next section). However, the demo rehearsal song built by createDemoRehearsalSong (packages/shared-types/src/index.ts, demoRehearsalSongSeed) contains a single section (verse-1) and places turnaroundPlan on that single/final section. Because there is no following section, resolveFirstTurnaroundPlan returns null for the demo, and the browser-fallback workspace (served via createDemoRehearsalSong in apps/desktop/src/lib/analysis.ts and job_runner.ts) always shows the 'firstTurnaroundPlanUnavailable' message rather than the turnaround callout. This also contradicts the frontend tests that render the demo and expect the callout and action button. Fix by giving the demo song a second section (a labeled continuation after verse-1) in which the turnaround-carrying roles remain active, so the plan on verse-1 can be surfaced; keep the shared-types validation and other tests consistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Investigated — root cause is bigger than a seed patch: the resolver requires a successor section (sections[i+1]) before admitting any turnaround candidate, so the single-section demo seed can never surface its plan; meanwhile several committed suites (snapshot/provenance/particle) construct single-section songs expecting successful resolution, and fixture timing expectations split between 0:10 (App.test) and 0:30 (turnaround suites). Reconciling means deciding the continuation contract itself (relax resolver vs add demo successor section vs rewrite the contradicting suites). Leaving open for the author rather than guessing product intent in a review thread.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Repair the validated current-head demo-continuation defect on the existing Current review thread Existing RED regression already exists: Narrow root fix: make the demo song itself truthful by adding a realistic successor section after Then repair only test fixtures that currently manufacture a one-role/single-source positive turnaround while expecting successful resolution. In particular, Also preserve the exact Ruff E501 repair already committed at Verification: run the focused resolver/callout provenance/particle/demo/shared-types suites, repository-pinned Ruff check+format, then canonical |
|
Queued @opencode-agent for PR #1036 at head |
|
Exact-head CI repair on canonical branch I inspected the current terminal release failure before requesting any mutation.
Use this existing branch as the single writer. Apply only the exact output of the repository-pinned |
| const target = | ||
| renderer?.querySelector<HTMLElement>( | ||
| `[data-section-index="${named.sectionIndex}"]` | ||
| ) ?? null; |
There was a problem hiding this comment.
📝 Info: Callout depends on identical section arrays
The callout scrolls to [data-section-index=named.sectionIndex], and that index is the position in song.sections. It only lines up because SongStructure renders the same array with data-section-index={sectionIndex} (Workspace.tsx). A future caller passing a filtered section list to one but not the other would misnavigate.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@OpenCode Use the current exact head |
|
@opencode-agent Continue only on the existing canonical Use systematic-debugging + verification-before-completion. Current exact-head CI run Apply repository-pinned Ruff formatting to exactly those two analysis-engine files and commit the formatter-only delta on this same branch. Do not alter turnaround logic, provenance, test expectations, or the just-landed navigation-failure regression. Then run |
|
@opencode-agent Superseding the unacknowledged predecessor-head formatting dispatch only because the same canonical branch moved compatibly. Fresh exact head is now Re-check the lane before writing, then apply repository-pinned Ruff formatting to exactly |
1 participant
Buyer-visible next action
This PR names tonight's first turnaround plan on the rehearsal map when section-level activity shows a part remaining active into the next section with at least one other distinct source. Open scrolls that rendered map section so the band can turn the last bars together and land the next downbeat.
Cutoff is simultaneous deactivation. Turnaround is shared continuation. A lone continuing part, a last section with no next activity, or heuristic-only topology stays unnamed.
Exact current identity
develop@7ad56cf0065d068ec6463d92726de4855a6e201d(fix(security): establish canonical npm, PDF.js, Nanoid, and Undici baseline #783 integrated).a2d6dcfe30b416fed65036d7aec448ce171d58b6.feat/workspace-first-turnaround-plan.Current implemented boundary
turnaroundPlancopy only from owned data properties and snapshots it once before ranking.Turn these last bars with {target}; land the downbeat together.only from corroborated mixed-source continuation. Three or more sources name the rest of the band. Custom role-owned guidance is preserved verbatim unless model provenance is explicit.{at} {role} 턴어라운드 열기.data-section-index) and fails closed on ambiguous or missing targets. Reduced motion usesbehavior: "auto".Turn these last bars with Lead Vocal on the verse last beat; land the chorus downbeat together.Dependency / merge gate
Canonical #783 is protected
developshipped truth. This branch inherits that JavaScript baseline and does not duplicate or suppress it.Keep unmerged until the unchanged then-current head has every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved findings, and a qualifying independent non-author last-push approval under live branch protection.
Queued, pending, skipped, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.
Summary by CodeRabbit