Skip to content

feat(workspace): name the next instrument check on Ranges and Player - #1052

Open
seonghobae wants to merge 18 commits into
developfrom
feat/ranges-player-next-instrument-check
Open

feat(workspace): name the next instrument check on Ranges and Player#1052
seonghobae wants to merge 18 commits into
developfrom
feat/ranges-player-next-instrument-check

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

Ranges now uses the same playable-span authority as the ready rehearsal map. A musician sees tonight's first named span and is told to check it on their instrument. Inverted or unnamed evidence is rejected instead of being shown as playable. The Player window names tonight's first map section to loop and states that this window does not play audio yet.

This completes the #957 playable-range contract on the Ranges board. It does not decode or play local audio; that remains #961. It does not extract OverlapWarningList or Storybook tokens; that remains #897.

Exact current identity

  • Protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 1f9a4ef75e563fe318f7761a400094b0d4ebff73.
  • Branch: feat/ranges-player-next-instrument-check.

Current exact scope

  • playableRange / firstRangeSqueeze decide which spans Ranges may name.
  • Clash copy tells the player to hear the overlap on their instrument before the section.
  • Missing or inverted spans ask for an ear check.
  • Written notes, when present, are an instrument check rather than an implementation dump.
  • Player empty and ready states name the next map action and do not claim playback.
  • Workspace exposes the Ranges and Player surfaces in the loaded view, and passes the selected role into the canonical range callout.
  • English and Korean copy is updated. AGENTS / ARCHITECTURE / CLAUDE / CHANGELOG stay current.

Verification

Post-push review repairs

  • Centralized runtime-object validation for nullable and malformed roots.

  • Preserved the first valid playable range; later clash warnings no longer replace the first-span cue.

  • Added singular English/Korean written-note copy.

  • Made range-card React keys and test ids unique within and across sections; overlap-warning keys remain unique within a role.

  • Rejects id-less roles instead of rendering a card that cannot be named by the canonical first-range resolver.

  • npm run test --workspace @bandscope/desktop: 226 tests passed; coverage thresholds held on the configured files.

  • npm run typecheck --workspace @bandscope/desktop

  • npm run lint --workspace @bandscope/desktop

  • npm run build --workspace @bandscope/desktop

Security Notes

  • Untrusted input: rehearsal song, section, role, range, overlap-warning, and transcription values from analysis results.
  • Trust boundary: lexical playable-span classification and named-section admission before buyer-visible copy. Malformed roots and collection members are isolated.
  • No new filesystem, URL, subprocess, IPC, WebView, model, update, export, or dependency path.
  • Canonical fix(security): establish canonical npm, PDF.js, Nanoid, and Undici baseline #783 remains protected develop shipped JavaScript dependency truth. This branch does not copy or suppress pdfjs-dist / nanoid / undici findings.

Merge gate


Devin Review

Summary by CodeRabbit

  • 새 기능

    • Ranges 화면에서 첫 번째 연주 가능 구간과 충돌·확인 안내를 제공합니다.
    • 구간별 연주 가능한 범위, 중첩 경고, 확인할 음과 악기 정보를 표시합니다.
    • Player 화면에서 오늘 밤 첫 번째 맵 섹션을 다음 반복 대상으로 안내합니다.
    • 곡이나 반복 가능한 섹션이 없을 때 안내 메시지를 표시합니다.
    • 현재는 오디오 재생을 제공하지 않음을 명확히 안내합니다.
  • 개선 사항

    • 잘못되거나 불완전한 구간 데이터가 있어도 화면이 중단되지 않습니다.
    • 영어와 한국어 안내 문구를 추가했습니다.

Latest exact-head verification

  • Protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25; current head: b737906.
  • Reused one owned-data accessor across the first-range authority, Ranges cards, and Player title/window reads; accessor and Proxy substitution regressions are covered.
  • Desktop verification: 229 tests passed; statements/branches/functions/lines 100%; desktop typecheck, lint, and production build passed.
  • Hosted deterministic checks for this new head are running.

Active-work guard

No runtime boundary or dependency was added. Malformed or accessor-backed analysis values fail closed before becoming buyer-visible range or loop authority.

Ranges now uses the same playable-span authority as the rehearsal map
and tells the player to check tonight's notes on their instrument.
The Player window names the first map section to loop and does not
claim local-audio playback.
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Ranges가 재생 가능한 범위와 악기 확인 문구를 표시합니다. Player가 첫 명명 섹션을 루프 대상으로 표시합니다. Workspace가 두 기능을 통합하고 영어·한국어 번역과 테스트를 추가합니다.

Changes

Ranges 화면

Layer / File(s) Summary
Ranges 검증 및 표시 로직
apps/desktop/src/features/ranges/index.tsx, apps/desktop/src/features/ranges/index.test.tsx
Ranges가 섹션과 역할 데이터를 검증합니다. 재생 가능한 첫 범위, 중첩 경고, 악기 확인 문구와 음역 카드를 표시합니다. 잘못된 데이터는 건너뜁니다.
Player 첫 루프 표시
apps/desktop/src/features/player/index.tsx, apps/desktop/src/features/player/index.test.tsx
firstNamedSection이 유효한 첫 명명 섹션을 선택합니다. PlayerFeature가 루프 대상과 오디오 미지원 상태를 표시합니다.

Workspace 화면 통합

Layer / File(s) Summary
Workspace 화면 통합 및 번역
apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/features/workspace/Workspace.rehearsalSurfaces.test.tsx, apps/desktop/src/locales/en/common.json, apps/desktop/src/locales/ko/common.json
Workspace가 Ranges와 Player를 함께 렌더링합니다. 새 상태 문구와 테스트를 추가합니다. 기존 범위 테스트 ID를 ranges-first-span으로 변경합니다.

동작 문서화

Layer / File(s) Summary
리허설 화면 동작 문서화
AGENTS.md, ARCHITECTURE.md, CLAUDE.md, CHANGELOG.md
Ranges의 재생 가능 범위와 Player의 첫 맵 섹션 루프 표시 동작을 문서와 변경 로그에 반영합니다.

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

Merge Risk: 🟡 Moderate · up to 644c3

The Ranges guidance can name a later playable span instead of tonight’s first playable span when a later span has an overlap warning, which may direct musicians to check the wrong section. This is a bounded correctness issue that should be fixed before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant RangesFeature
  participant PlayerFeature
  participant Translator
  Workspace->>RangesFeature: song과 activeRole 전달
  RangesFeature->>Translator: 범위 상태 문구 요청
  Translator-->>RangesFeature: 현재 로케일 문구 반환
  RangesFeature-->>Workspace: 첫 범위와 역할 카드 렌더링
  Workspace->>PlayerFeature: song 전달
  PlayerFeature->>Translator: Player 상태 문구 요청
  Translator-->>PlayerFeature: 현재 로케일 문구 반환
  PlayerFeature-->>Workspace: 첫 명명 섹션과 오디오 상태 렌더링
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (6 skipped: 6… 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 제목은 Ranges와 Player에서 다음 악기 점검을 명명하는 주요 변경 사항을 정확히 요약합니다. 간결하고 구체적입니다.
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ranges-player-next-instrument-check

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.

github-code-quality[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Please review exact current head 397b7e4a86503c5d649b2bb06c18a8adccfb6b29 only. The current required opencode-review validator failed because no authenticated current-head formal verdict exists; predecessor reviews are not evidence. Focus on the repaired Ranges authority boundary: blank/whitespace role IDs must fail closed consistently in both firstRangeSqueeze and per-role playable cards, while valid named spans, inversion rejection, repeated-section card identity, singular/plural transcription copy, Player honesty, and the protected #957 contract remain intact. Do not approve from deterministic/model-unavailable fallback evidence.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

github-code-quality[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Please perform the required independent formal review on exact current head 644c3a0f10e5af184644ed5849882b55c3a26589 against independently resolved develop@749511c3ad4000090048718f685c6bee6b3d2c25. Revalidate the buyer-visible Ranges/Player wiring, canonical playable-range authority, active-role callout behavior, malformed/Proxy/accessor fail-closed behavior, duplicate landmark avoidance, current local verification, and exact current security evidence. Post an authenticated APPROVED or CHANGES_REQUESTED verdict anchored to this exact head; do not reuse predecessor reviews or mutate the branch.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Please perform the required independent formal review on exact current head 1f9a4ef75e563fe318f7761a400094b0d4ebff73 against independently resolved develop@749511c3ad4000090048718f685c6bee6b3d2c25. Revalidate the buyer-visible Ranges/Player integration, canonical first playable span semantics, active-role callout, malformed runtime fail-closed behavior, unique selectors, current local verification, and exact current security evidence. Post an authenticated APPROVED or CHANGES_REQUESTED verdict anchored to this exact head; do not reuse predecessor reviews or mutate the branch.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Review exact current head b737906aba0b95251109e53434dd8860ff6664fd against independently resolved protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Required opencode-review job 99068427870 failed only because there is no authenticated APPROVED/CHANGES_REQUESTED OpenCode review bound to this SHA. Current formal reviews are COMMENTED and all current review threads are resolved; predecessor verdicts do not count.

Revalidate first-playable-span ordering, Ranges/Player loaded-product integration, own-data/Proxy fail-closed reads for song/section/range authority, unique role/card identity, no-playback honesty, desktop exact 100% statement/branch/function/line evidence, and current security/supply-chain state. Bind any formal verdict only to this exact head; do not inherit predecessor-head, deterministic-fallback, model-unavailable, status-only, or rate-limited evidence.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

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

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode reviewed the current-head product diff. Coverage is a separate gate.

Changed files

  • AGENTS.md — repository behavior
  • ARCHITECTURE.md — repository behavior
  • CHANGELOG.md — repository behavior
  • CLAUDE.md — repository behavior
  • apps/desktop/src/features/player/index.test.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/player/index.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/ranges/index.test.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/ranges/index.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/workspace/Workspace.rehearsalSurfaces.test.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/workspace/Workspace.test.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/workspace/Workspace.tsx — TypeScript or JavaScript runtime
  • apps/desktop/src/features/workspace/firstRangeSqueeze.test.ts — TypeScript or JavaScript runtime
  • apps/desktop/src/features/workspace/firstRangeSqueeze.ts — TypeScript or JavaScript runtime
  • apps/desktop/src/locales/en/common.json — repository behavior
  • apps/desktop/src/locales/ko/common.json — repository behavior

Changed behavior

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Repository file: AGENTS.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Repository file: AGENTS.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Repository file: ARCHITECTURE.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Repository file: ARCHITECTURE.md"]
  R2 --> V2["required checks"]
  Evidence --> S3["Repository file: CHANGELOG.md"]
  S3 --> I3["repository behavior"]
  I3 --> R3["Review risk: Repository file: CHANGELOG.md"]
  R3 --> V3["required checks"]
  Evidence --> S4["Repository file: CLAUDE.md"]
  S4 --> I4["repository behavior"]
  I4 --> R4["Review risk: Repository file: CLAUDE.md"]
  R4 --> V4["required checks"]
  Evidence --> S5["TypeScript/JavaScript: index.test.tsx (9 files)"]
  S5 --> I5["TypeScript or JavaScript runtime"]
  I5 --> R5["Review risk: TypeScript/JavaScript: index.test.tsx (9 files)"]
  R5 --> V5["package test plus coverage"]
  Evidence --> S6["Repository file: common.json"]
  S6 --> I6["repository behavior"]
  I6 --> R6["Review risk: Repository file: common.json"]
  R6 --> V6["required checks"]
  Evidence --> S7["Repository file: common.json"]
  S7 --> I7["repository behavior"]
  I7 --> R7["Review risk: Repository file: common.json"]
  R7 --> V7["required checks"]
Loading

Findings

No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.

  • Head SHA: b737906aba0b95251109e53434dd8860ff6664fd
  • Workflow run: 33246682101
  • Workflow attempt: 1
  • Coverage gate: failure

Review outcome

Coverage is a gate, not the review. This body reviews the changed product files.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Repository file: AGENTS.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Repository file: AGENTS.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Repository file: ARCHITECTURE.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Repository file: ARCHITECTURE.md"]
  R2 --> V2["required checks"]
  Evidence --> S3["Repository file: CHANGELOG.md"]
  S3 --> I3["repository behavior"]
  I3 --> R3["Review risk: Repository file: CHANGELOG.md"]
  R3 --> V3["required checks"]
  Evidence --> S4["Repository file: CLAUDE.md"]
  S4 --> I4["repository behavior"]
  I4 --> R4["Review risk: Repository file: CLAUDE.md"]
  R4 --> V4["required checks"]
  Evidence --> S5["TypeScript/JavaScript: index.test.tsx (9 files)"]
  S5 --> I5["TypeScript or JavaScript runtime"]
  I5 --> R5["Review risk: TypeScript/JavaScript: index.test.tsx (9 files)"]
  R5 --> V5["package test plus coverage"]
  Evidence --> S6["Repository file: common.json"]
  S6 --> I6["repository behavior"]
  I6 --> R6["Review risk: Repository file: common.json"]
  R6 --> V6["required checks"]
  Evidence --> S7["Repository file: common.json"]
  S7 --> I7["repository behavior"]
  I7 --> R7["Review risk: Repository file: common.json"]
  R7 --> V7["required checks"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Exact-head OpenCode dispatch note for b737906: the central coverage-evidence job failed before PR execution while building its trusted Python 3.14 tool image because base lock installation could not find antlr4-python3-runtime==4.9.3. This is central .github toolchain evidence, not a BandScope source verdict; no approval or merge evidence was produced.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant