Skip to content

feat(workspace): consolidate first-range rehearsal navigation - #1143

Open
seonghobae wants to merge 29 commits into
developfrom
feat/workspace-find-first-range-section
Open

feat(workspace): consolidate first-range rehearsal navigation#1143
seonghobae wants to merge 29 commits into
developfrom
feat/workspace-find-first-range-section

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

BandScope’s first playable-range cue links a buyer-visible rehearsal decision back to two existing surfaces without starting playback: the structure timeline answers when the range occurs and Section Roadmap identifies the exact part. #961 remains the Active Player owner.

Exact current identity

  • protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25
  • branch: feat/workspace-find-first-range-section
  • exact current head: d657b8d17222e23e7acf95c43766016bb3f013fa
  • open, non-Draft, mergeable, unmerged at the latest fresh refetch

This PR is the canonical consolidation owner for the first-range Workspace navigation fan-out. #1147 has already been closed unmerged after its valid roadmap-navigation delta was preserved here; no predecessor check/review/approval evidence transfers.

Current behavior

  • firstRangeSqueeze owns playable-range presentation truth only: it preserves a complete ordered scientific-pitch span, role/section copy, overlap warning, and the source record’s exact string IDs even when those IDs are unsafe for DOM navigation. A noncanonical ID therefore cannot erase valid Bass Guitar C#2–E3-style rehearsal guidance.
  • firstRangeTimeline independently admits navigation only when the source section ID is exact/canonical and unique, and start/end clocks are finite, non-negative, and strictly increasing. Repeated form labels remain valid when IDs are unique.
  • firstRangeRoadmap independently admits navigation only when the exact section ID is unique across the map and the exact role ID is unique within that section. Repeated display labels/names remain valid.
  • Noncanonical or ambiguous IDs hide only their Find controls; they do not convert valid range evidence into a false missing-range state.
  • Every Find activation carries a monotonic request sequence, so reactivating an already-highlighted target scrolls again.
  • Existing focus requests are revalidated against the current song on every render. Same-source edits that remove or duplicate a previously focused section/role immediately remove aria-current and suppress stale focus scrolling.
  • Successful saved-project replacement advances a Workspace instance revision. Opening another .bscope result therefore clears UI-local navigation state even when both payloads reuse song.id = "analyzed-song"; ordinary immutable edits to the current project retain the same Workspace instance and valid focus.
  • Timeline and roadmap scrolling honor prefers-reduced-motion: reduce, and roadmap Find scrolls the exact requested role card rather than only the section wrapper.
  • English and Korean Find controls are localized. This slice does not claim the wider eight-locale i18n program complete.

TDD / repair history

Earlier RED/GREEN history on this branch covered repeated activation, replacement-song focus scoping, duplicate identities, exact-role scrolling, roadmap interaction coverage, and reduced-motion behavior. The repaired production path includes exact navigation admission, uniqueness helpers, strict positive-duration validation, current-song focus revalidation, saved-project Workspace instance revision, and decoupling of valid range presentation truth from unsafe navigation identity.

The six commits after previously documented head 740ad3b5d1311b309b70dffab0a043932665de32 are descendants without rebase; the latest current-head commit d657b8d17222e23e7acf95c43766016bb3f013fa keeps docs/architecture/overview.md code-current with the mounted timeline/roadmap navigation behavior.

Current verification boundary

Fresh exact-head inventory contains 31 check-runs for d657b8d17222e23e7acf95c43766016bb3f013fa. Windows/macOS build and OSV lanes remain queued, while publish-manual-pr-evidence-status is skipped; those are non-passing evidence. Hosted exact-head CI is authoritative and predecessor/protected-base evidence is not substituted.

Security / ownership boundary

No new filesystem, URL, subprocess, network, credential, database, MIR, or playback authority is introduced. IDs and clocks are treated as untrusted navigation evidence and fail closed before selecting a UI target, while independent buyer-visible range evidence remains intact. Project loading stays with the existing Project Persistence boundary; this PR only prevents UI-local focus state from crossing a successful project replacement.

Do not self-approve. Merge only on the unchanged exact head after every applicable required check is terminal-success, all valid review findings are resolved, and a qualifying independent non-author approval applies to the last push.

The ready map already names the first playable span. Players still had
to hunt the structure timeline for when that section happens. Offer
Find {section} at {clock} on the timeline, fail closed on duplicate
labels or unusable clocks, and highlight the matching cell.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

첫 연주 가능 범위를 고유 식별자로 검증하고, 타임라인과 로드맵에서 해당 섹션 또는 역할을 찾는 버튼을 추가했습니다. 포커스 요청은 반복 실행, 곡과 프로젝트 변경, 잘못된 시간 및 중복 식별자를 처리합니다.

Changes

첫 연주 가능 범위 탐색

Layer / File(s) Summary
첫 범위 데이터 생성 및 검증
apps/desktop/src/features/workspace/firstRangeSqueeze.ts, apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts
firstRangeTimelinefirstRangeRoadmap이 고유 섹션 ID와 역할 ID를 사용합니다. 유효하지 않은 식별자와 시간 범위에는 null을 반환합니다.
Workspace 포커스 동작
apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/SectionRoadmap.tsx
Workspace가 찾기 요청과 요청 시퀀스를 관리합니다. SongStructureSectionRoadmap은 대상 요소를 스크롤하고 aria-current와 강조 스타일을 적용합니다.
찾기 문구 및 통합 검증
apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/features/workspace/firstRangeTimeline.regression.test.tsx, apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx, apps/desktop/src/locales/en/common.json, apps/desktop/src/locales/ko/common.json, AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md
영어와 한국어 찾기 문구를 추가했습니다. 버튼 표시 조건, 반복 요청, 곡 교체, 프로젝트 변경, 중복 ID 및 reduced-motion 동작을 테스트했습니다. 관련 문서를 갱신했습니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🔵 Low · up to efc54

Replacing or updating a song can leave an earlier Find request highlighting the wrong timeline or roadmap item when identifiers overlap. This is a bounded presentation issue with no playback or data impact, so the change is mergeable with explicit owner awareness and follow-up to clear or revalidate focus on song changes.

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant Workspace
  participant firstRangeRoadmap
  participant SectionRoadmap
  Player->>Workspace: 로드맵 찾기 버튼 클릭
  Workspace->>firstRangeRoadmap: 섹션 및 역할 확인
  firstRangeRoadmap-->>Workspace: 검증된 sectionId와 roleId 반환
  Workspace->>SectionRoadmap: 포커스 요청 전달
  SectionRoadmap->>SectionRoadmap: 대상 역할 카드를 scrollIntoView
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 68.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 첫 번째 연주 가능 구간 탐색 기능을 통합한다는 PR의 주요 변경 사항을 명확하고 간결하게 설명합니다.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/workspace-find-first-range-section

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae added enhancement New feature or request priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: feature New or expanded product capability labels Sep 2, 2026 — with ChatGPT Codex Connector
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae changed the title feat(workspace): find tonight's first range on the timeline feat(workspace): consolidate first-range rehearsal navigation Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Canonical single-writer repair for the overlapping first-range UI boundary.

Fresh semantic comparison on the current protected base (develop@749511c3ad4000090048718f685c6bee6b3d2c25) shows #1142 (aa604734baebec68b25134397fd215b7a0b64e4d), this PR (ce6c8fcf11b4191c2925324f7b8ac2a1b086484f), and #1147 (2b8542ecec025ad489c1f5fbc43865b56390d80b) are divergent siblings editing the same Workspace / firstRangeSqueeze / locale/documentation surface. They are not duplicates: #1142 uniquely owns the Open-part/RoleSwitcher action; this head owns timeline identity/clock/focus/reduced-motion regressions; #1147 uniquely owns roadmap section+role focus, duplicate part/identity rejection, repeated-focus sequencing, reduced-motion roadmap scrolling, and its dedicated regressions.

Treat this PR as the consolidation owner. Do not merge it until those unique #1142/#1147 behaviors, tests, locale contracts, and still-valid documentation evidence are reconstructed here (or a verified successor) and fresh exact-head checks/reviews pass. The sibling PRs remain preservation children until complete semantic succession; no predecessor approval/check evidence transfers. Direct base retargeting currently exposes real overlapping-file conflicts, so conflict resolution must preserve both behaviors rather than dropping a side.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 3 new potential issues.

Devin Review

Comment thread apps/desktop/src/features/workspace/Workspace.tsx
Comment thread apps/desktop/src/features/workspace/Workspace.tsx
Comment thread apps/desktop/src/features/workspace/Workspace.tsx

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 5 new potential issues.

Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Focused tests need external confirmation

The workspace suites could not start without installed dependencies; Vite failed to resolve its React plugin. CI must provide the missing execution evidence.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread apps/desktop/src/features/workspace/Workspace.tsx Outdated
Comment thread apps/desktop/src/features/workspace/firstRangeSqueeze.ts Outdated
Comment thread apps/desktop/src/features/workspace/firstRangeSqueeze.ts
Comment thread apps/desktop/src/features/workspace/Workspace.tsx Outdated
coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant