Skip to content

feat(workspace): find tonight's first range on the roadmap - #1147

Closed
seonghobae wants to merge 5 commits into
feat/workspace-find-first-range-sectionfrom
feat/workspace-find-first-range-roadmap
Closed

feat(workspace): find tonight's first range on the roadmap#1147
seonghobae wants to merge 5 commits into
feat/workspace-find-first-range-sectionfrom
feat/workspace-find-first-range-roadmap

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Superseded by canonical first-range navigation owner #1143

This stacked PR is closed only after verifying that its valid product, test, and documentation requirements are preserved on the canonical owner branch. It is not being treated as merged, and no review/check evidence from this head transfers.

Verified source head before closure: 54d8ddfc0d76e5c70ae07d3b4a72663d762f99c5.
Verified successor branch after transfer/repair: feat/workspace-find-first-range-section@d657b8d17222e23e7acf95c43766016bb3f013fa in #1143, still based on protected develop@749511c3ad4000090048718f685c6bee6b3d2c25.

Succession evidence

#1143 now preserves and extends the valid roadmap behavior from this PR:

  • firstRangeSqueeze keeps the originating exact section/role IDs while playable-range presentation remains independent from navigation admission.
  • firstRangeRoadmap navigates by unique section/role identity; repeated display labels/names remain valid and duplicate/noncanonical IDs fail closed.
  • repeated Find activation is sequence-driven;
  • roadmap navigation honors reduced-motion preference;
  • the exact requested role card is scrolled, rather than only the section wrapper;
  • existing roadmap focus is revalidated against same-source song updates so removed/duplicated identities cannot retain stale aria-current state;
  • local-project and fallback-song identities are namespaced (project: / song:), repairing this PR's final unresolved collision finding;
  • successful saved-project replacement also remounts Workspace-local navigation state, covering analyzed results that reuse song.id = "analyzed-song";
  • English/Korean roadmap Find copy remains covered;
  • current canonical regression coverage includes repeated activation, project replacement, same-source ambiguity, project/song identity collision, repeated labels/display names with unique IDs, duplicate section/role IDs, noncanonical identity with preserved buyer-visible range evidence, exact-role scrolling, localization, and reduced motion.

The canonical branch also corrected two stale regression assertions to match the shipped role-card accessibility contract (aria-current="true") and consolidated the roadmap capability into CHANGELOG.md and docs/architecture/overview.md alongside timeline navigation.

Evidence boundary

#1143 exact-head workflows must run again on its own unchanged head. This closure does not import this PR's absent/stale checks, review state, or approvals. #961 remains the Active Player/transport owner; this navigation slice does not start playback.

The ready map already names tonight's first playable range. Players still
had to hunt the section roadmap for the chord, cue, and clash on that
part. The range card now offers Find {section} for {role} on the roadmap
and highlights the matching unique section and part without starting
playback.

Fail closed on duplicate section or part identity. Repeated Find clicks
scroll again. Replacing the loaded song cannot inherit a previous
highlight. Roadmap scrolling honors prefers-reduced-motion.
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 13d61baf-9654-4d44-97c3-785e7dd3fb21

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 9bed7714-c5b3-451d-85cc-a6f25e3c0e31

📥 Commits

Reviewing files that changed from the base of the PR and between 749511c and 2b8542e.

📒 Files selected for processing (13)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/src/features/workspace/SectionRoadmap.tsx
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx
  • apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts
  • apps/desktop/src/features/workspace/firstRangeSqueeze.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • docs/architecture/overview.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

ready workspace가 첫 연습 구간을 표시하고, 버튼으로 해당 역할과 섹션을 로드맵에서 찾습니다. 로드맵은 대상 카드를 스크롤하고 강조 표시합니다. 중복되거나 유효하지 않은 식별자는 기능을 비활성화합니다.

Changes

첫 연습 구간 로드맵 찾기

Layer / File(s) Summary
로드맵 대상 검증
apps/desktop/src/features/workspace/firstRangeSqueeze.ts, apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts
FirstRangeRoadmap 타입과 firstRangeRoadmap 함수를 추가했습니다. 함수는 섹션과 역할의 식별자 및 표시 이름을 검증하고, 중복 또는 잘못된 구조에서 null을 반환합니다.
워크스페이스 포커스 연결
apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/SectionRoadmap.tsx, apps/desktop/src/locales/*/common.json
워크스페이스가 첫 연습 구간 찾기 요청을 생성합니다. SectionRoadmap은 요청된 섹션과 역할을 스크롤하고 aria-current, 테스트 식별자, 강조 스타일을 적용합니다. 영어와 한국어 라벨을 추가했습니다.
동작 회귀 검증과 문서 반영
apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/features/workspace/firstRangeRoadmap.regression.test.tsx, AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, docs/architecture/overview.md
버튼 표시, 로케일, 반복 클릭, 곡 교체, 중복 식별자, 중복 역할, reduced-motion 동작을 검증했습니다. 프로젝트 문서와 변경 로그에 로드맵 찾기 동작을 반영했습니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 2b854

This UI-only change locates and highlights a uniquely matched roadmap item without starting playback, changing persistent state, or invoking external services. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant 연주자
  participant Workspace
  participant firstRangeRoadmap
  participant SectionRoadmap
  participant DOM
  연주자->>Workspace: 첫 연습 구간 로드맵 찾기 클릭
  Workspace->>firstRangeRoadmap: song과 firstRange 전달
  firstRangeRoadmap-->>Workspace: 검증된 sectionId와 roleId 반환
  Workspace->>SectionRoadmap: 포커스 요청 전달
  SectionRoadmap->>DOM: 대상 섹션으로 스크롤
  SectionRoadmap->>DOM: 섹션과 역할 강조
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (7 skipped: 7 u…
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 제목은 workspace에서 오늘 밤의 첫 연습 구간을 section roadmap에서 찾는 핵심 변경을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (7 skipped: 7 unsupported.)

✨ Finishing Touches
📝 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-roadmap

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 area: ui-ux Frontend, interaction, design, or user experience enhancement New feature or request priority: high High-priority or P1 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
@seonghobae
seonghobae changed the base branch from develop to feat/workspace-find-first-range-section September 2, 2026 15:38
@seonghobae
seonghobae changed the base branch from feat/workspace-find-first-range-section to develop September 2, 2026 15:38
@seonghobae
seonghobae changed the base branch from develop to feat/workspace-open-first-range-part September 2, 2026 15:38
@seonghobae
seonghobae changed the base branch from feat/workspace-open-first-range-part to develop September 2, 2026 15:38
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae changed the base branch from develop to feat/workspace-find-first-range-section September 2, 2026 15:42

Copy link
Copy Markdown
Collaborator Author

Preservation child of canonical first-range navigation owner #1143. The base is intentionally retargeted to feat/workspace-find-first-range-section@ce6c8fcf11b4191c2925324f7b8ac2a1b086484f so roadmap navigation does not remain another independently mergeable same-boundary writer.

Preserve the unique firstRangeRoadmap admission contract, SectionRoadmap section+role focus semantics, repeated activation sequencing, replacement-song scoping, duplicate section/role identity rejection, reduced-motion scrolling, EN/KO copy, and firstRangeRoadmap.regression.test.tsx until #1143 (or its verified successor) contains equivalent mounted behavior and regression evidence. Current overlapping-file conflicts are explicit repair work; do not resolve them by discarding the timeline or roadmap side, and do not transfer predecessor checks/reviews.

@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 on lines +195 to +249
const targetSectionId = meaningfulRangeText(rangeSqueeze.sectionId);
const targetRoleId = meaningfulRangeText(rangeSqueeze.roleId);
if (!targetSectionId || !targetRoleId) {
return null;
}

const runtimeSong: unknown = rehearsalSong;
if (!isRuntimeObject(runtimeSong) || !Array.isArray(runtimeSong.sections)) {
return null;
}

let sectionIdOccurrences = 0;
let targetSection: Record<string, unknown> | null = null;
for (const sectionValue of runtimeSong.sections) {
if (!isRuntimeObject(sectionValue)) {
continue;
}
if (meaningfulRangeText(sectionValue.id) === targetSectionId) {
sectionIdOccurrences += 1;
targetSection = sectionValue;
}
}

if (sectionIdOccurrences !== 1 || !targetSection || !Array.isArray(targetSection.roles)) {
return null;
}

let roleIdOccurrences = 0;
let targetRole: Record<string, unknown> | null = null;
for (const roleValue of targetSection.roles) {
if (!isRuntimeObject(roleValue)) {
continue;
}
if (meaningfulRangeText(roleValue.id) === targetRoleId) {
roleIdOccurrences += 1;
targetRole = roleValue;
}
}

if (roleIdOccurrences !== 1 || !targetRole) {
return null;
}

const sectionLabel = meaningfulRangeText(targetSection.label);
const roleName = meaningfulRangeText(targetRole.name);
if (!sectionLabel || !roleName) {
return null;
}

return {
sectionId: targetSectionId,
roleId: targetRoleId,
sectionLabel,
roleName
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Repeated labels remain unambiguous

firstRangeRoadmap uses the selected range's section and role IDs. Repeated display labels remain navigable, while duplicate IDs suppress the control.

Devin Review

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

Comment on lines +189 to +203
/** Request the first-range roadmap cell on every activation, even when it is already highlighted. */
const handleFindFirstRangeRoadmap = () => {
if (!firstRangeBoard) {
return;
}
setRoadmapFocusRequest((previousFocusRequest) => ({
rehearsalSongId: song.id,
sectionId: firstRangeBoard.sectionId,
roleId: firstRangeBoard.roleId,
requestSequence:
previousFocusRequest?.rehearsalSongId === song.id
? previousFocusRequest.requestSequence + 1
: 1
}));
};

@devin-ai-integration devin-ai-integration Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Repeated activation retriggers scrolling

Every click increments requestSequence. The roadmap effect observes that value, so an already-highlighted target scrolls again.

Devin Review

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

Comment on lines 172 to +175
}
)
: t("workspaceFirstRangeMissing");
const firstRangeBoard = firstRangeRoadmap(song, firstRange);

@devin-ai-integration devin-ai-integration Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Focused roles remain visible

firstRangeSqueeze follows the active role filter. The resulting focused role remains rendered when Find runs.

Devin Review

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

@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.

Devin Review found 1 new potential issue.

Devin Review

() => safeProjectBootstrapSummary(sourceBootstrap),
[sourceBootstrap]
);
const rehearsalSourceIdentity = parsedSourceBootstrap?.projectId ?? song.id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Loaded songs inherit stale roadmap focus

When a loaded song ID equals the prior project ID, rehearsalSourceIdentity treats both sources as identical. The new song retains the old roadmap highlight.

Suggested change
const rehearsalSourceIdentity = parsedSourceBootstrap?.projectId ?? song.id;
const rehearsalSourceIdentity = parsedSourceBootstrap
? `project:${parsedSourceBootstrap.projectId}`
: `song:${song.id}`;
Devin Review

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

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

Labels

area: ui-ux Frontend, interaction, design, or user experience enhancement New feature or request priority: high High-priority or P1 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