Skip to content

feat(workspace): guide tonight's first entrance on map and player - #912

Open
seonghobae wants to merge 56 commits into
developfrom
feat/player-hear-first-entrance
Open

feat(workspace): guide tonight's first entrance on map and player#912
seonghobae wants to merge 56 commits into
developfrom
feat/player-hear-first-entrance

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

After analysis, the workspace names tonight's first entrance so the room can orient immediately instead of staring at a generic ready card. The first entrance is the earliest valid section that still has an active valid part, then the highest-priority active valid role in that section.

What changed

  • FirstEntranceCallout on the workspace map and player.
  • Workspace navigation is explicit: Open Bass Guitar entrance in the verse at 0:10 arms the cue and scrolls to the rendered section position; it no longer says Hear when no playback occurs.
  • Workspace navigation honors prefers-reduced-motion: smooth scroll remains the default, while reduced-motion users receive immediate auto scrolling to the same renderer-owned section target.
  • Player playback is explicit: Hear ... appears only when the owning player provides onPlayFromSeconds, and calls that callback with the exact entrance start time.
  • Malformed section starts, unknown rehearsal priorities, inactive roles, and non-Boolean activity evidence fail closed instead of influencing first-entrance selection.
  • Entrance armed state resets when entrance identity changes, including A → B → A transitions.
  • Analysis section.id remains domain data and never becomes DOM id authority for song-structure navigation.
  • English and Korean strings distinguish map navigation from playback; workspace-surface-cues remains the stable navigation anchor.
  • Docs: AGENTS, ARCHITECTURE, CLAUDE, CHANGELOG, docs/design-system/component-contract.md, and docs/doctoring/reduced-motion-first-entrance-navigation.md.

Test-first repair evidence

Earlier TDD on this lane established renderer-owned navigation and reduced-motion behavior: RED 03647cdd1f371aa419d31dae1e43d50477336038 → GREEN a51d2d83c050f7ea08fec5974ef25287eb0d90c5/aa466dd45a12599492bec4c175c7a8dc34aafa08, and reduced-motion RED 0262b8e80faeefba5e91d74345dcb679a3b00e91 → GREEN b754127d06d12ab95873fd7c547190230583d8f2. Predecessor-head workflow evidence does not transfer.

A prior buyer-visible truth repair established graph activity as entrance authority: RED a9b22e765c428c5f8ee7751d9d4822d9b28525bb reproduced a high-priority resting part outranking the actually active part; GREEN 791fa1112bd317fa4c21d3cc4a58f279fcea1fce restricted candidates to role IDs represented by active graph nodes. 6a749bd9261f6f36ef29524cc60c183249d8de17 aligned the property-based valid-metadata generator with that contract.

Fresh current-source review found that this boundary still used JavaScript truthiness (node.is_active). Runtime-corrupted metadata such as is_active: "false" is truthy and could therefore authorize a resting high-priority role despite the documented fail-closed contract. RED 960eb4921d44f9d4d819757f0665df6552d8ce05 adds a focused malformed-runtime regression requiring an active medium-priority bass to outrank a high-priority role whose activity value is the string "false". GREEN/current head 46a6829c90b8697d24e954a94d9d0fe33e2fc446 accepts activity authority only when is_active === true.

No predecessor-head workflow or review result transfers to the current head.

Exact-current-head verification

  • Protected base: develop@acdbea6344fe1231c39535b575f4de35e4c607c9.
  • Exact head: 46a6829c90b8697d24e954a94d9d0fe33e2fc446.
  • Fresh repository workflows are currently queued and therefore non-passing: ci 32164614599, security-audit 32164614684, aggregate Security Scan 32164614620, sbom 32164614556, release 32164614538, build-baseline 32164614468, SAST Semgrep 32164614477, bandit 32164614557, and secret-scan-gate 32164614634.
  • Any exact-head failure must be inspected at job/check-run/log level before another edit or rerun.

Ownership / merge gate

Security and accessibility notes

  • Untrusted input includes rehearsal song metadata, role priorities, runtime part-graph activity, and analysis-owned section identifiers.
  • Entrance attribution requires strict Boolean true section-local graph authority before priority ranking.
  • Workspace navigation uses renderer-owned section position rather than analysis-derived DOM IDs; Player playback remains explicit callback authority.
  • Reduced-motion handling reads only the user-agent/OS media preference and changes animation behavior, not data or capability authority.
  • W3C doctoring remains implementation rationale and does not claim WCAG certification or conformance from this single behavior.

Summary by CodeRabbit

  • 새로운 기능

    • 곡의 첫 진입 정보를 역할, 섹션, 시작 시간으로 표시합니다.
    • 워크스페이스에서 해당 위치를 열고, 플레이어에서는 가능한 경우 바로 청취할 수 있습니다.
    • 영어와 한국어 UI를 지원하며 섹션 명칭을 현지화합니다.
    • 사용자의 reduced-motion 설정에 맞춰 탐색 애니메이션을 조정합니다.
  • 개선 사항

    • 첫 진입 정보가 없거나 곡 분석이 필요한 경우 안내 메시지를 제공합니다.
    • 잘못된 데이터나 비활성 역할을 안정적으로 제외합니다.
  • 문서

    • 첫 진입 탐색 및 접근성 동작에 대한 안내를 추가했습니다.

Open in Devin Review

Name the first hearable part, section, and start time on the workspace
and player so the room can take the next rehearsal action instead of a
generic ready card.
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 20 seconds.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2359c0b4-e6f1-4e19-8653-283142001caf

📥 Commits

Reviewing files that changed from the base of the PR and between fd3d88f and a54ffbd.

📒 Files selected for processing (13)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/src/features/workspace/FirstEntranceCallout.reduced-motion.test.tsx
  • apps/desktop/src/features/workspace/FirstEntranceCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstEntranceCallout.tsx
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstEntrance.test.ts
  • apps/desktop/src/features/workspace/firstEntrance.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
📝 Walkthrough

Walkthrough

곡의 첫 진입을 계산하고 표시하는 FirstEntranceCallout을 추가했습니다. Workspace는 해당 섹션을 열고, Player는 seek 콜백이 있을 때만 청취 동작을 제공합니다. 번역, reduced-motion 처리, 테스트와 문서를 갱신했습니다.

Changes

첫 진입 기능

Layer / File(s) Summary
첫 진입 해석
apps/desktop/src/features/workspace/firstEntrance.ts, apps/desktop/src/features/workspace/firstEntrance*.test.ts
유효한 시작 시간과 활성 역할을 기준으로 가장 이른 첫 진입을 선택합니다. 역할 우선순위를 적용하고, 잘못된 입력은 제외합니다.
첫 진입 콜아웃 동작
apps/desktop/src/features/workspace/FirstEntranceCallout.*, apps/desktop/src/i18n/index.ts, apps/desktop/src/locales/*/common.json
역할, 섹션, 시작 시간과 큐를 표시합니다. Workspace 스크롤과 Player 콜백 실행을 분리합니다. reduced-motion 설정과 한국어 조사 및 섹션 라벨 현지화를 지원합니다.
Workspace 및 Player 통합
apps/desktop/src/features/workspace/Workspace.*, apps/desktop/src/features/player/*
Workspace는 렌더러의 섹션 위치로 이동합니다. Player는 onPlayFromSeconds가 있을 때만 청취 버튼을 표시하고 시작 시간을 전달합니다.
문서 및 구현 계약 반영
AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, docs/design-system/component-contract.md, docs/doctoring/reduced-motion-first-entrance-navigation.md
첫 진입의 표시 형식, surface별 동작, reduced-motion 계약을 문서화합니다.

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

Merge Risk: 🟡 Moderate · up to fd3d8

Malformed song metadata can cause first-entrance guidance to crash instead of safely showing no cue, so the PR is not merge-ready until nested metadata validation and regression tests are added.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant PlayerFeature
  participant FirstEntranceCallout
  participant firstEntrance
  participant SongStructure
  Workspace->>FirstEntranceCallout: 곡 데이터 전달
  PlayerFeature->>FirstEntranceCallout: 곡 및 seek 콜백 전달
  FirstEntranceCallout->>firstEntrance: 첫 진입 계산
  firstEntrance-->>FirstEntranceCallout: 역할, 섹션, 시작 시간 반환
  FirstEntranceCallout->>SongStructure: Workspace 섹션 열기
  FirstEntranceCallout->>PlayerFeature: Player 시작 시간 재생
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
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 워크스페이스 맵과 플레이어에 첫 진입 안내를 추가하는 주요 변경 사항을 정확히 설명합니다.
✨ 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/player-hear-first-entrance

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.

coderabbitai[bot]

This comment was marked as resolved.

@seonghobae seonghobae changed the title feat(workspace): hear tonight's first entrance from the map feat(workspace): guide tonight's first entrance on map and player Aug 18, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@opencode-agent opencode-agent Bot added area: ui-ux Frontend, interaction, design, or user experience 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 Aug 22, 2026
@opencode-agent

Copy link
Copy Markdown
Contributor

Queued @cwl-noema-review and @opencode-agent for PR #912 at head b8f588ad6b0d017a4539ed6956fdcf53057292bc. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #912 at head b8f588ad6b0d017a4539ed6956fdcf53057292bc. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@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 2 new potential issues.

Open in Devin Review

Comment thread CHANGELOG.md Outdated
Comment thread apps/desktop/src/features/workspace/FirstEntranceCallout.tsx

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Exact-head owner repair on existing branch feat/player-hear-first-entrance@e5d3d0e38127c20ef4fe397e45dfd486df7209b0, protected develop@7ad56cf0065d068ec6463d92726de4855a6e201d. Refetch head/base/blob/review state immediately before writing; do not create a competing PR or force-push.

Current review finding PRRT_kwDORjvEXs6bIqkX is valid: production FirstEntranceCallout resolves the song-structure renderer through [data-testid="song-structure-grid"], so deleting/renaming a test-only marker silently disables buyer navigation. I strengthened the existing callout regression first on exact head e5d3d0e38127c20ef4fe397e45dfd486df7209b0: the realistic target helper now exposes only the intended production anchor id="workspace-song-structure-grid", with no data-testid; current production should therefore remain RED until the owning renderer/navigation contract changes. Do not count queued/predecessor evidence as RED/GREEN.

Implement the narrow root fix in this same PR: make SongStructure own a stable production navigation anchor (prefer a named/exported constant rather than duplicating magic strings), and make FirstEntranceCallout resolve that production anchor instead of any test-id. Keep section selection positional/renderer-owned; do not switch to analysis-controlled section IDs. Preserve fail-closed behavior when the renderer/section/scrollIntoView is absent and preserve reduced-motion semantics. Keep data-testid only if tests still need it; it must no longer be production authority.

Then run the focused FirstEntranceCallout/Workspace tests, desktop typecheck/lint/full suite and owned coverage/quickcheck. Resolve only PRRT_kwDORjvEXs6bIqkX after the successor exact head proves the production anchor and focused/full verification. The duplicate CHANGELOG bug thread was independently fixed/resolved on the predecessor lineage; do not reintroduce that conflict.

@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 2 new potential issues.

Open in Devin Review

Comment thread apps/desktop/src/features/workspace/FirstEntranceCallout.test.tsx
Comment thread apps/desktop/src/features/workspace/FirstEntranceCallout.tsx Outdated
@seonghobae

Copy link
Copy Markdown
Collaborator Author

Re-review request for exact head e5d3d0e38127c20ef4fe397e45dfd486df7209b0: all required status contexts are green on this head (including coverage-evidence and security scans) and all review threads are resolved. @opencode-agent please review current head e5d3d0e38127c20ef4fe397e45dfd486df7209b0 and update the review decision.

# 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
@seonghobae

Copy link
Copy Markdown
Collaborator Author

Merged current develop into this branch: resolved shared-file conflicts by including both this feature's first-entrance Hear 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.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent repair

Use systematic-debugging + verification-before-completion on exact current head ec1cc3bed544566f7b49df20f856c3380bd4cb21 only. Fresh writer-lease refetch shows the head is unchanged and no same-scope successor writer has moved this lane.

Exact hosted RED: CI run 32949135950, ci / build-and-test job 98132768479, actual checkout 6dc72fc363313fcb122a5a31e5f6c773ae762e94 (Merge ec1cc3b… into 749511c…). Lint/typecheck/Rust passed; desktop Vitest failed 4 tests in FirstEntranceCallout.test.tsx (237 passed / 4 failed). The first two failures have zero scrollIntoView calls; the latter failures cannot reach the armed Start on … copy because navigation never completes.

Verified first causal boundary: this branch’s main regression helper intentionally mounts a stable production anchor id="workspace-song-structure-grid", but FirstEntranceCallout.tsx still queries the test-only selector [data-testid="song-structure-grid"]. Workspace.tsx still gives SongStructure only data-testid="song-structure-grid". This also validates the still-unresolved Devin finding that production navigation depends on a test-only attribute. Do NOT take the suggested leaf workaround of changing the regression helper back to data-testid; that would make CI green while preserving the product reliability defect.

Root-cause repair on this same canonical branch:

  1. Preserve the existing RED regression that mounts id="workspace-song-structure-grid".
  2. Give the real SongStructure grid the stable production id="workspace-song-structure-grid" (you may retain data-testid for test tooling, but it must no longer be navigation authority).
  3. Change FirstEntranceCallout workspace navigation to resolve that stable production id, keeping section-index/array-order mapping, fail-closed missing target behavior, and reduced-motion semantics unchanged.
  4. Align FirstEntranceCallout.reduced-motion.test.tsx with the same production id; ensure manually appended grids are removed in afterEach so an assertion failure cannot leak duplicate anchors into later tests.
  5. Do not replace the armed-copy assertions: Start on Bass Guitar … is the post-success armed body and is currently absent only because the scroll action returns early.
  6. Do not use analysis-controlled section ids as DOM ids and do not broaden into the informational array-order note.

Verification: run the two FirstEntranceCallout test files first, then desktop tests/coverage, repository-pinned lint/Ruff/docstring checks and canonical quickcheck. Commit only to feat/player-hear-first-entrance. Refetch exact successor head/live base and report exact GREEN evidence. Resolve only the test-id production-dependency thread and wrong-helper thread after the successor exact head proves this repair; leave informational threads alone.

@opencode-agent

Copy link
Copy Markdown
Contributor

Queued @opencode-agent for PR #912 at head 9473742ed6c7d2adacd906f2e767c11751e5756e. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Queued @opencode-agent for PR #912 at head 9473742ed6c7d2adacd906f2e767c11751e5756e. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Queued @opencode-agent for PR #912 at head 9473742ed6c7d2adacd906f2e767c11751e5756e. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@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 2 new potential issues.

Open in Devin Review

Comment thread apps/desktop/src/features/workspace/FirstEntranceCallout.reduced-motion.test.tsx Outdated
Comment thread apps/desktop/src/features/workspace/FirstEntranceCallout.tsx

Copy link
Copy Markdown
Collaborator Author

@OpenCode Use superpowers:using-superpowers, receiving-code-review, systematic-debugging, test-driven-development, and verification-before-completion. Work ONLY canonical branch feat/player-hear-first-entrance at exact head 9473742ed6c7d2adacd906f2e767c11751e5756e against live protected develop@749511c3ad4000090048718f685c6bee6b3d2c25; refetch before write and adapt if either moves.

Fresh review/source verification confirms two linked current-head defects share one root production boundary:

  • PRRT_kwDORjvEXs6chvyR: resolveSongStructureGrid() prefers #workspace-song-structure-grid, but production SongStructure in Workspace.tsx still renders only data-testid="song-structure-grid", so production navigation relies on the fragile immediate-sibling fallback.
  • PRRT_kwDORjvEXs6chvw4: the reduced-motion fixture still uses the obsolete data-testid, so it no longer reaches the current stable-anchor resolver and its scroll assertion is RED.

Repair this SAME lane regression-first and narrowly:

  1. Preserve/add a focused Workspace regression requiring the renderer-owned grid to expose exactly the stable production id workspace-song-structure-grid; do not derive DOM id from analysis section metadata.
  2. Add id="workspace-song-structure-grid" to the production grid in SongStructure while retaining data-testid strictly for tests if existing tests still use it.
  3. Align FirstEntranceCallout.reduced-motion.test.tsx fixture and cleanup with that stable production id; do not reintroduce test-id as production navigation authority.
  4. Keep the current resolver's fail-closed duplicate-id behavior and callback-only Player behavior unchanged. Do not add another sibling/selector heuristic.
  5. Re-verify the informational partGraph fail-closed note but do not weaken is_active === true; analysis-engine topology already treats partGraph as active-role authority.

Verification: focused entrance + reduced-motion + Workspace navigation tests, repository-pinned lint/format/typecheck, then canonical ./scripts/harness/quickcheck.sh. If GREEN, push only the minimal repair to this branch, refetch exact new head/live base, and leave review threads unresolved until current-head hosted checks confirm them. No dependency/security/gate/foreign-repository changes.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Re-review exact current head a54ffbd6a9c56dc619ae659ed09f98ba81634179 against live protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Refetch both refs before judging and do not transfer predecessor-head review evidence.

The dependency-root navigation repair is now present on this exact head: SongStructure owns stable production anchor id="workspace-song-structure-grid"; the first-entrance navigation no longer depends on data-testid, and the reduced-motion fixture is aligned. All current review threads are resolved. Repository-local exact-head workflows ci, security-audit, aggregate Security Scan, release, sbom, build-baseline, SAST Semgrep, bandit, and secret-scan-gate are terminal-success. The same-head opencode-review check-run is also terminal-success; prior formal OpenCode CHANGES_REQUESTED reviews are on predecessor heads and are not current approval evidence.

Please perform the independent formal review on this unchanged exact head, using current required coverage/docstring/security/supply-chain evidence and current diff. Submit APPROVE only if the head independently satisfies review policy; otherwise submit precise current-head REQUEST_CHANGES findings. Do not self-merge, bypass protection, or reuse stale evidence.

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