feat(workspace): guide tonight's first pre-chorus on the map - #955
seonghobae wants to merge 19 commits into
Conversation
Name the earliest labeled pre-chorus so the room can play the lift into the chorus. Workspace Open scrolls the matching map section; the player Hear action exists only when playback can seek.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough첫 번째 유효한 Changes첫 프리코러스 안내
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workspace
participant FirstPreChorusCallout
participant firstPreChorus
participant SongStructureGrid
participant PlayerFeature
participant PlaybackSurface
Workspace->>FirstPreChorusCallout: 현재 song 전달
FirstPreChorusCallout->>firstPreChorus: 첫 pre-chorus 해석
firstPreChorus-->>FirstPreChorusCallout: 구간·역할·시작 시간 반환
FirstPreChorusCallout->>SongStructureGrid: Open 액션으로 대상 구간 스크롤
PlayerFeature->>FirstPreChorusCallout: callback-only 모드와 seek callback 전달
FirstPreChorusCallout->>PlaybackSurface: Hear 액션으로 시작 시간 전달
Merge Risk: 🔵 Low · up to The player experience adds copy that may bypass the English and Korean localization contract, which could produce inconsistent or untranslated text for some users. The change is mergeable with explicit owner awareness and follow-up on localization compliance. 🚥 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 |
![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.
|
Review finding verification on exact head |
opencode-agent
Bot
added
area: ui-ux
# Conflicts: # CHANGELOG.md
![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.
# 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
|
Merged current develop into this branch: resolved shared-file conflicts by including both this feature's first-pre-chorus lift callout and the newly landed #957 playable-range work (Workspace.tsx keeps both imports; en/ko locales unioned with symmetric keys; AGENTS/CLAUDE/CHANGELOG unioned). No behavior dropped. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
added
the
enhancement
ChatGPT Codex Connector
|
Admission-state correction for exact head Finding: live develop comparison is diverged (11 ahead / 2 behind) and 2 review threads remain unresolved. This PR remains Open and is moved to Draft/Proposed. Its commits, reviews, threads, and valid delta are preserved. Return it to Ready after causal repair/non-force reconciliation and fresh exact-head evidence. No bypass, synthetic status/approval, manual rerun, Force Push, review dismissal, or Close is used. |
…elop Adopt the current protected develop merge result as ordinary two-parent ancestry so the feature is no longer two commits behind protected product truth. Preserve the existing feature history and GitHub-resolved merge tree without force-push or destructive rebase. Signed-off-by: Seongho Bae <me@seonghobae.me>
…eature Restore protected develop blobs for AGENTS, architecture, changelog, Claude guidance, and the shared component contract. The first-pre-chorus feature keeps only feature-local source, tests, locale changes, and focused doctoring instead of becoming a second writer for repository-wide product/design authority. Signed-off-by: Seongho Bae <me@seonghobae.me>
Reproduce the review finding where a fresh runtime song object with the same stable song id clears already-completed pre-chorus guidance even though the selected lift is unchanged. Signed-off-by: Seongho Bae <me@seonghobae.me>
Preserve completed pre-chorus guidance when an unrelated edit replaces the RehearsalSong object but retains the same valid song id. Fall back to object identity when runtime id evidence is missing or accessor-backed so malformed replacement songs still fail closed. Signed-off-by: Seongho Bae <me@seonghobae.me>
Restore the protected PlayerFeature and remove branch-only player tests. No production caller supplies the new seek callback, so retaining a player Hear path would claim an interaction the running product cannot execute. Keep the workspace map action as the buyer-visible owner until Active Player wiring exists in its canonical lane. Signed-off-by: Seongho Bae <me@seonghobae.me>
Remove dead callback-only Active Player semantics from the first-pre-chorus callout after restoring the protected player surface. The buyer-visible action now only opens the renderer-owned map section, while the stable-song completion fix and fail-closed runtime identity handling remain intact. Signed-off-by: Seongho Bae <me@seonghobae.me>
Drop translation keys introduced only for the unwired Player/Hear surface after that capability was removed from this Workspace-owned lane. Retain the Workspace map-action and pre-chorus guidance copy. Signed-off-by: Seongho Bae <me@seonghobae.me>
Drop translation keys introduced only for the unwired Player/Hear surface after that capability was removed from this Workspace-owned lane. Retain the Workspace map-action and pre-chorus guidance copy. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
changed the title
1 participant
Workspace rehearsal decision owner
이 Draft는 분석 결과에 이미 존재하는 첫 번째 유효
pre-chorus구간을 찾아 Workspace에서 오늘의 다음 행동으로 표시하고, 실제 렌더러-owned Song Structure 위치로 Open/scroll하는 기능만 소유합니다. Active Player seek/playback은 이 PR의 권한이 아닙니다.Exact identity / topology
develop@314ddeae7b775a4957594b599358c8255617eb2efeat/workspace-first-prechorus-lift502b7c2f29036e3f1ec89dcd96ed2b2fa6285b56developAGENTS.md,ARCHITECTURE.md,CHANGELOG.md,CLAUDE.md, shared design-system contract, Player source/tests are no longer owned here.Current product contract
pre-chorus; do not invent one from another form label or an unlabeled section.pre-chorusas프리코러스; EN/KO contain only the Workspace keys owned by this lane.Protected-base reconciliation and single-writer repair
The previous live head
4058e5094bff94f9ed2df0f635313ff244f225a6was two commits behind current protecteddevelopand the PR body still named an older base/head. GitHub's clean merge result for4058e...+ protected314dde...had tree43e6370b644cc8be04e5346c71842d51d4c086de.Ordinary two-parent descendant
73410af56cdc62b53f225fb2220361ca22e9ffb1adopted that current protected merge result without force-push or destructive rebase.0f211b05b89c69a7ca196679c93d82fc794033acthen restored protected blobs for repository-wide agent/architecture/changelog/Claude/design-system authority, leaving this feature as a Workspace-local writer.Valid review finding: same-song update cleared completed guidance
The callout previously keyed completion state to the
songobject reference. An unrelated edit such as practice-progress update can replace the runtime object while retaining the same song and pre-chorus, incorrectly clearing the already-completed guidance.16003ee29da0ee9cfd7e187b57453bcf750068ba: after Open, rerender a fresh song object with the same stable song id and unchanged pre-chorus; completion must survive.dce5b8b8b91632e2d3192c9b857c17859cb3693f: use an own, nonblank data-property song id as stable identity. Missing/malformed/accessor-backed runtime ids fall back to object identity so replacement invalid-id songs still fail closed.The review thread is resolved against the current descendant.
Valid review finding: Player Hear had no production wiring
The prior feature branch added a Player
Hearaction guarded by anonPlayFromSecondsprop, but the running product had no production caller that supplied that callback. Tests alone therefore made the branch appear to own a buyer interaction that could not be executed.be7c42666d140dc6d8551eb87d5afe8431c9bd57restores protectedPlayerFeatureand removes branch-only Player tests.02a1167e117d0b38cae491f87c222b6d397e4c68removes callback-only/Hear semantics fromFirstPreChorusCallout; Workspace Open-to-map is the only action retained.11c68d3093a03d2d30075d5aff15b7a40acf1c62and current502b7c2f29036e3f1ec89dcd96ed2b2fa6285b56remove EN/KO keys that existed only for the unwired Player surface.The review thread is resolved. Active Player playback remains for its canonical owner rather than being simulated here.
Evidence boundary
Every predecessor check/review receipt is non-transferable after the source movement above. Only fresh exact-head repository CI, build-baseline, Security Scan, Semgrep, SBOM, CodeQL, relevant UI/browser evidence, and qualifying independent non-author current-head review count toward merge acceptance. Queued/pending/absent is not GREEN.
UI Delivery Gate: FAIL until current-head automated checks are terminal and packaged/browser evidence covers the actual Workspace action with pointer/touch/keyboard, visible focus, screen reader semantics, reduced-motion behavior, 400% zoom/reflow, responsive widths and required locale/text-expansion acceptance.
Commercial Release Gate: FAIL. This feature is not release evidence and does not alter signing/notarization, SBOM/provenance, reproducibility or updater rollback obligations.
No self-approval, force-push, destructive rebase, source-neutral retrigger, blind rerun, synthetic status, predecessor-evidence transfer, gate weakening, Player source duplication or repository-wide authority duplication.