Skip to content

feat(workspace): name tonight's first hook plan on the map - #1017

Open
seonghobae wants to merge 11 commits into
developfrom
feat/workspace-first-hook-plan
Open

feat(workspace): name tonight's first hook plan on the map#1017
seonghobae wants to merge 11 commits into
developfrom
feat/workspace-first-hook-plan

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

The mounted rehearsal workspace names tonight's first hook so a part can lock the melody before the room starts. Open moves to the matching rendered map section.

Customer-facing next action: Open Lead Vocal hook at 0:10 (demo). If no corroborated hook exists, stay on tonight's map for the next rehearsal cue.

Exact current identity

  • Protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact head: f1003c639229880b22808527894df8a3ad2eef5d.
  • Branch: feat/workspace-first-hook-plan.

Scope and trust boundary

  • Owned hookPlan copy on an active, corroborated part plus labeled section and time.
  • Do not invent hook copy from groove, cue, simplification, overlap, range, chord labels, function labels, setup notes, transposition plans, fill plans, tuning plans, dynamics plans, articulation plans, confirmed overrides, harmonic explanations, or confidence notes.
  • Runtime hook-plan authority is the snapshotted own data-property descriptor value. A Proxy get trap cannot substitute different buyer-visible hook copy after the own-data check.
  • The shared exported RehearsalRole.hookPlan declaration documents that the field is rehearsal-facing hook guidance owned by the role only when runtime graph evidence corroborates it.
  • Distinct from first-fill-plan, first-setup-note, first-transposition-plan, first-tuning-plan, first-dynamics-plan, and first-articulation-plan.
  • Reduced-motion Open uses behavior: auto.
  • Korean next-action copy stays particle-safe ().

Exact-head verification state

Current review finding PRRT_kwDORjvEXs6bsI4m was verified against the previous head, repaired on the canonical branch with descriptive shared-type JSDoc, and resolved only after the exact new blob was refetched.

Fresh repository-native workflows for exact head f1003c639229880b22808527894df8a3ad2eef5d are dispatched. At the latest refetch build-baseline is in progress and ci, release, security-audit, aggregate Security Scan, sbom, SAST Semgrep, bandit, and secret-scan-gate are queued. None of these non-terminal results is counted as success, and no predecessor-head evidence transfers.

Security Notes

  • Project-file metadata is untrusted. Shared-types, the buyer-visible first-hook resolver, and Rust project loading now apply one explicit Unicode-safe blank and single-line hook-plan policy; malformed values fail closed without echoing rejected content. Tests cover TypeScript, UI, and Rust boundaries.

Dependency / merge gate


Open in Devin Review

Summary by CodeRabbit

  • 새로운 기능
    • 워크스페이스에서 오늘의 첫 훅 계획을 확인하고 해당 곡 구조 구간으로 바로 이동할 수 있습니다.
    • 훅 계획이 없을 때는 안내 메시지가 표시됩니다.
  • 개선 사항
    • 영어와 한국어 UI 및 섹션 라벨을 지원합니다.
    • 동작 줄이기 설정에서는 즉시 이동합니다.
    • 여러 워크스페이스에서도 올바른 곡 구조로 이동하며, 잘못된 정보는 안전하게 표시하지 않습니다.
  • 문서
    • 첫 훅 계획 안내와 탐색 동작에 대한 사용 지침을 추가했습니다.

Name the owning part, owned hookPlan copy, labeled section, and time so
the next rehearsal action is obvious. Open moves to the matching map
section. Do not invent hook copy from other rehearsal metadata.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Rehearsal 역할에 hookPlan을 추가했습니다. 첫 번째 유효한 계획을 해석하는 resolver와 FirstHookPlanCallout을 구현했습니다. Workspace는 해당 섹션으로 이동하며, 영어·한국어와 reduced-motion 동작을 지원합니다.

Changes

첫 훅 계획 기능

Layer / File(s) Summary
hookPlan 계약 및 해석
packages/shared-types/src/index.ts, packages/shared-types/test/index.test.ts, apps/desktop/core/src/lib.rs, apps/desktop/src/features/workspace/firstHookPlan.ts, apps/desktop/src/features/workspace/firstHookPlan*.test.ts
RehearsalRole과 Rust payload에 선택적 hookPlan을 추가했습니다. 런타임 입력, 활성 graph 역할, 시간 범위, 중복 ID를 검증하고 첫 계획을 결정합니다.
Workspace 콜아웃 및 섹션 탐색
apps/desktop/src/features/workspace/FirstHookPlanCallout.tsx, apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/FirstHookPlanCallout*.test.tsx, apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/i18n/*, apps/desktop/src/locales/*
지역화된 콜아웃과 Open 동작을 추가했습니다. Open 동작은 유일한 renderer-owned 섹션으로 이동하며, reduced-motion 설정에서는 즉시 스크롤합니다.
커버리지 계약 및 기능 문서
apps/desktop/vite.config.ts, apps/desktop/src/features/workspace/coverageContract.test.ts, docs/design-system/component-contract.md, docs/doctoring/reduced-motion-first-hook-plan-navigation.md, AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, .jules/palette.md
새 프로덕션 파일을 커버리지 목록에 추가했습니다. 첫 훅 계획의 데이터 출처, 탐색 규칙, reduced-motion 동작과 접근성 지침을 문서화했습니다.

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

Merge Risk: 🔵 Low · up to d60de

The PR adds an optional corroborated hook plan to Workspace with navigation to the matching map section. The remaining issues are limited to contradictory contributor guidance and an outdated architecture description, so the change is mergeable with explicit owner follow-up to align the documentation.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant FirstHookPlanCallout
  participant resolveFirstHookPlan
  participant SongStructureRenderer
  Workspace->>FirstHookPlanCallout: song 전달
  FirstHookPlanCallout->>resolveFirstHookPlan: 첫 hookPlan 해석
  FirstHookPlanCallout->>SongStructureRenderer: 대상 section으로 scrollIntoView
  SongStructureRenderer-->>FirstHookPlanCallout: 탐색 완료 상태 반환
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 10 files. (7 skipped: 7…
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 제목은 워크스페이스 지도에 오늘 밤의 첫 훅 계획을 표시하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

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

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 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-first-hook-plan

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.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent

Repair only canonical BandScope PR #1017 branch feat/workspace-first-hook-plan at exact head 6d92ed24f5695d6c130c1df9f6353f641c49a47b if that head is still current; if it moved, refetch the intervening delta first and adapt instead of overwriting another writer.

Current-head review finding PRRT_kwDORjvEXs6bsI4m is valid and narrowly scoped: packages/shared-types/src/index.ts extends the exported RehearsalRole contract with hookPlan?: string but leaves that new field undocumented. Add a beginner-readable JSDoc immediately above hookPlan that states this copy is owned by the rehearsable part and represents its corroborated hook plan. Do not change the type, runtime behavior, dependency files, coverage thresholds, navigation logic, or #783-owned security state.

Run the shared-types lint/typecheck/tests and the applicable repository quickcheck, commit only this JSDoc repair to the same branch, and report the exact successor head plus verification evidence. Do not create another PR, merge, force-push, self-approve, or weaken protection.

devin-ai-integration[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.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Re-review request for exact head 90732b7fed0523f5ee5817b76151e7fb6a2bffc7: 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 90732b7fed0523f5ee5817b76151e7fb6a2bffc7 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

Resolved merge conflicts against origin/develop (749511c): union-merged AGENTS.md/CHANGELOG.md bullets, sentence-merged CLAUDE.md, kept both import sets in Workspace.tsx (FirstHookPlanCallout + firstRangeSqueeze), unioned en/ko locale tails (JSON valid, en/ko symmetric). Merged legacy .Jules/palette.md into canonical .jules/palette.md, dropped alias. No conflict markers remain. Merge head: ffa1766.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@devin-ai review this PR at current HEAD f1003c6. Shared-types, the buyer-visible first-hook resolver, and Rust project loading now use the same explicit Unicode-safe plan policy; tests cover blank, NEL, LS, PS, and BOM-padded text.

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

Devin Review

Comment on lines +412 to +451
/** Return whether a code point is in the cross-language plan whitespace set. */
function isPlanWhitespaceCodePoint(codePoint: number): boolean {
return (
(codePoint >= 0x0009 && codePoint <= 0x000d) ||
codePoint === 0x0020 ||
codePoint === 0x0085 ||
codePoint === 0x00a0 ||
codePoint === 0x1680 ||
(codePoint >= 0x2000 && codePoint <= 0x200a) ||
codePoint === 0x2028 ||
codePoint === 0x2029 ||
codePoint === 0x202f ||
codePoint === 0x205f ||
codePoint === 0x3000 ||
codePoint === 0xfeff
);
}

/** Apply one explicit cross-language Unicode whitespace policy to plan text. */
export function isNonEmptySingleLineText(value: unknown): value is string {
if (typeof value !== "string") {
return false;
}
let hasNonWhitespace = false;
for (const character of value) {
const codePoint = character.codePointAt(0)!;
if (
codePoint === 0x000a ||
codePoint === 0x000d ||
codePoint === 0x0085 ||
codePoint === 0x2028 ||
codePoint === 0x2029
) {
return false;
}
if (!isPlanWhitespaceCodePoint(codePoint)) {
hasNonWhitespace = true;
}
}
return hasNonWhitespace;

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: Cross-language validation stays aligned

TypeScript and Rust use identical whitespace sets and reject the same line separators. Omitted plans remain valid; present blank or multiline plans fail consistently.

Devin Review

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

Comment on lines +121 to +125
if (!isNonEmptySingleLineText(hookPlan)) {
return null;
}
const trimmed = hookPlan.trim();
return truncateCodePoints(trimmed, MAX_HOOK_PLAN_CHARACTERS);

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: BOM padding remains display-safe

The validator accepts BOM padding, then trim() removes it before truncation and display. Invisible edge padding cannot enter the rendered hook guidance.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant