feat(workspace): name tonight's first score check on the map - #1092
feat(workspace): name tonight's first score check on the map#1092seonghobae wants to merge 13 commits into
Conversation
Tell the player to open a trusted attached score in Score and check tonight's first range on the page, or add a score and check the span by ear. Display names stay fail-closed; PDF bytes stay on the Score bridge.
📝 WalkthroughWalkthroughready Workspace가 신뢰할 수 있는 첫 번째 PDF 악보와 선택된 역할의 음역을 계산하고, Score에서 확인할 안내를 영어와 한국어로 표시합니다. 관련 테스트와 프로젝트 문서도 갱신합니다. Changes첫 번째 악보 점검
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The workspace guidance is safely limited to validated score metadata and falls back when no usable score is available. The PR is mergeable with owner follow-up to align the architecture, contributor, changelog, and agent documentation with that behavior. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant safeProjectBootstrapSummary
participant firstScoreCheck
participant RehearsalSong
Workspace->>safeProjectBootstrapSummary: sourceBootstrap 검증
safeProjectBootstrapSummary-->>Workspace: scoreWorkspaceAvailable 반환
Workspace->>firstScoreCheck: song, activeRole, workspace 상태 전달
firstScoreCheck->>RehearsalSong: scoreAttachments와 역할 음역 조회
RehearsalSong-->>firstScoreCheck: 첫 신뢰 가능한 악보와 범위 반환
firstScoreCheck-->>Workspace: 점검 데이터 또는 null 반환
Workspace-->>Workspace: 번역된 first-score-check 섹션 렌더링
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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 5 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
seonghobae
added
area: ui-ux
Grok (by xAI)
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const firstScoreCopy = firstScore | ||
| ? fillRangeCopy( | ||
| t( | ||
| firstScore.sectionLabel | ||
| ? "workspaceFirstScoreCheck" | ||
| : "workspaceFirstScoreCheckNoRange" | ||
| ), | ||
| { | ||
| fileName: firstScore.fileName, | ||
| roleName: firstScore.roleName ?? "", | ||
| lowestNote: firstScore.lowestNote ?? "", | ||
| highestNote: firstScore.highestNote ?? "", | ||
| sectionLabel: firstScore.sectionLabel ?? "" | ||
| } | ||
| ) |
There was a problem hiding this comment.
📝 Info: Filename interpolation remains literal
fillRangeCopy performs one callback-based pass. Braces and replacement patterns inside filenames remain visible text rather than becoming template syntax.
Was this helpful? React with 👍 or 👎 to provide feedback.
| * Native attachment owns filesystem validation and can legitimately persist | ||
| * long names, surrounding spaces, repeated dots, and platform-reserved stems. | ||
| * This UI helper therefore avoids imposing a second basename policy. It only | ||
| * rejects values that cannot represent a PDF display name safely in this | ||
| * copy surface; the value is never used to rebuild a filesystem path. |
There was a problem hiding this comment.
📝 Info: Relaxed names remain display-only
Long names, repeated dots, reserved stems, and leading spaces cannot alter storage paths. Native attachment stores each PDF under a generated UUID.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const scoreWorkspaceAvailable = useMemo( | ||
| () => safeProjectBootstrapSummary(sourceBootstrap) !== null, | ||
| [sourceBootstrap] | ||
| ); | ||
| const firstScore = useMemo( | ||
| () => firstScoreCheck(song, activeRole, scoreWorkspaceAvailable), | ||
| [activeRole, scoreWorkspaceAvailable, song] |
There was a problem hiding this comment.
📝 Info: Availability matches Score actions
scoreWorkspaceAvailable follows the bootstrap state that enables Score actions. Live results retain it; reopened project files clear it and receive fallback guidance.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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.
| if (scoreFileName === null) { | ||
| return null; | ||
| } | ||
| return { scoreId: attachmentCandidate.id, scoreFileName }; |
There was a problem hiding this comment.
📝 Info: Compatibility translation stays local
Only firstScoreCheck consumes trustedScoreAttachment, and it uses the semantic fields. Persisted projects retain their established id and fileName keys.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
The ready rehearsal map names tonight's first attached score and the next Score action only while the current session still has a validated live project workspace. With a trusted attachment plus a named span, the player is told to open that score in Score and check the selected role's range against the page. If there is no live project context, no trusted score attachment, or no usable attachment metadata, the map fails closed to the existing add-a-score / ear-check guidance.
This slice connects the rehearsal map to Score using existing
scoreAttachments,sourceBootstrap, andfirstRangeSqueezecontracts. It does not open, read, or parse PDF bytes, rebuild a filesystem path from display metadata, or invent page-to-note mapping/MIR.Figma/component contract:
docs/design-system/component-contract.mdready Workspace on page34 Workspace State Matrix.Exact identity
develop@749511c3ad4000090048718f685c6bee6b3d2c25.feat/workspace-first-score-check.8099e3b2525723474aca09db4d669167035263b3.Buyer-visible behavior
.bscopeproject with no live project bootstrap: Add a score in Score, or check tonight's first range by ear before the first section.Review-driven repairs on the current branch
Preserve native-valid display names
The original UI helper imposed an unrelated secondary basename policy and could hide scores the native attach boundary had already accepted. Current
trustedScoreFileNametherefore preserves native-valid literal display metadata including long names, surrounding spaces, repeated dots, and platform-reserved stems. It only rejects values that cannot be represented safely as PDF display metadata in this copy surface: empty/non-string values, control characters, path separators, and non-PDF names. The value is never used to reconstruct a filesystem path.Do not advertise impossible score opens after reopen
Persisted
scoreAttachmentsmetadata is not proof that the native Score workspace still exists.loadProject()intentionally clears the live analysis/project bootstrap, soWorkspacederivesscoreWorkspaceAvailablefrom a validatedsourceBootstrapand passes that authority tofirstScoreCheck. Reopened metadata-only projects therefore fall back instead of promising an action thatScoreViewcannot execute.Isolate generic persisted score keys at an anti-corruption boundary
The existing project-format score attachment shape uses the established wire keys
idandfileName. Those persisted keys remain unchanged in this PR to avoid a silent project-format break.trustedScoreAttachmentis now the explicit compatibility boundary: it validates the old wire shape and immediately translates it to the workspace-owned semantic projection{ scoreId, scoreFileName }. Workspace logic no longer carries the generic persisted names beyond that boundary.TDD sequence for this naming repair:
35dc521f03711d749771751ecf39b904f193057dchanged the focused regression to require{ scoreId, scoreFileName }while production still returned{ id, fileName }.8cd6756ef242d99fc323181b21b58f96fe24c731introducedTrustedScoreAttachment, translated the persisted wire keys attrustedScoreAttachment, and renamed touched organization-owned locals to bounded score/range vocabulary.Keep product guidance truthful
Current review found that
ARCHITECTURE.md,AGENTS.md,CHANGELOG.md, andCLAUDE.mdcould be read as promising that any persisted attachment was immediately openable. Commits5af64f5c3ddc85b237a4426678de0233ee4f5fdf,5a2abb1aa404eb0df133cbaeade44439621e56d6,893b87a53faaa08f3f972a4dc264c47ff9c83511, and8099e3b2525723474aca09db4d669167035263b3now state the same runtime invariant as production: score-open guidance requires both validated attachment metadata and a live Score workspace; reopened metadata-only projects or untrusted metadata use the add-score / ear-check fallback.Ownership boundary
Exact-head verification
Fresh verification must be evaluated on exact head
8099e3b2525723474aca09db4d669167035263b3. Every earlier success receipt, including those fordd17febbbb3b35d9e322ac50a200c0fae51a0cbcand the intermediate naming head8cd6756ef242d99fc323181b21b58f96fe24c731, is predecessor evidence only. Queued, pending, absent, or stale evidence is non-passing.Security notes
Attack surface
scoreAttachmentsmetadata with compatibility wire keysidandfileNamerendered as customer-facing copy only after validation and semantic translation.Trust boundary and mitigations
scoreIdandscoreFileName.fillRangeCopyperforms one-shot literal replacement so attached names are not rescanned as template syntax.Merge gate
Keep unmerged until this unchanged exact head has every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved findings, a qualifying independent non-author last-push approval, and ordinary branch-protection acceptance. Never self-approve or bypass protection. Queued, pending, skipped-required, cancelled, failed, predecessor-head, protected-base, author, model-only, or administrative-bypass evidence is non-passing.