Skip to content

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

Open
seonghobae wants to merge 5 commits into
developfrom
feat/workspace-first-voicing-plan
Open

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

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

After analysis, the room can see tonight's first owned voicing plan and take one next action: open that part on the map.

Cites docs/design-system/component-contract.md (First Voicing Plan Callout).

This is a setup-cue next action after first-setup-note, first-transposition-plan, first-tuning-plan, first-dynamics-plan, and first-articulation-plan. It is not a parallel MIR product and does not invent voicing from groove, cue, simplification, overlap, range, chords, function labels, setup notes, transposition plans, tuning plans, dynamics plans, articulation plans, confirmed overrides, harmonic explanations, or confidence notes.

Exact current identity

  • Protected base: develop@acdbea6344fe1231c39535b575f4de35e4c607c9.
  • Branch: feat/workspace-first-voicing-plan.
  • Head: b3e8922bc9507593698d86a6eb57fd73259391e1.

Buyer-facing copy

  • Named: {role} still has a voicing plan in the {section} at {at} plus the owned voicingPlan text.
  • Open: Open {role} voicing at {at} scrolls the renderer-owned [data-testid=song-structure-grid] [data-section-index=N].
  • Armed: Lock that voicing on {role} at {at} before the room starts.
  • Unavailable: stay on tonight's map until a part owns rehearsal-facing voicing copy. That unavailable sentence is the next action.
  • Korean copy stays particle-safe with 파트.

Safety and current-head repair

  • Fail closed on inherited metadata, own accessors, sparse arrays, duplicate role/graph identities, inactive graph nodes, and unbounded time ranges.
  • Runtime voicing-plan authority is now the snapshotted own data-property descriptor value. A Proxy get trap cannot substitute different buyer-visible voicing copy after the own-data check.
  • Regression commit 7f79b6d7d445da5e10bad5f5e8ec30d5873613d1 locks the Proxy get-trap case; production repair b3e8922bc9507593698d86a6eb57fd73259391e1 consumes the descriptor value directly.
  • Open is renderer-owned map navigation only. Player Hear remains [Product] Add an active rehearsal player with precise looping and role controls #961.
  • Reduced motion uses scroll-behavior: auto.
  • Ambiguous workspace mounts do not steal another surface's section.

Exact-head verification state

Fresh repository workflows for b3e8922bc9507593698d86a6eb57fd73259391e1 have been dispatched and were queued at the latest refetch. No non-terminal result and no predecessor-head evidence is counted as success.

Out of scope / merge gate


Open in Devin Review

Surface the owned voicing plan as the next rehearsal action and open
the matching rendered map section. Do not invent voicing from groove,
cues, simplification, overlap, range, chords, function labels, setup
notes, transposition, tuning, dynamics, articulation, overrides,
harmonic explanations, or confidence notes.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 27 minutes.

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: b2460f61-7b5c-44d2-8afb-efebc0e74db4

📥 Commits

Reviewing files that changed from the base of the PR and between acdbea6 and a0285ef.

📒 Files selected for processing (28)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.identity.test.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.particle.test.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.reduced-motion.test.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.unavailable-copy.test.tsx
  • apps/desktop/src/features/workspace/FirstVoicingPlanCallout.workspace-scope.test.tsx
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstVoicingPlan.inherited-metadata.test.ts
  • apps/desktop/src/features/workspace/firstVoicingPlan.proxy-authority.test.ts
  • apps/desktop/src/features/workspace/firstVoicingPlan.section-label.test.ts
  • apps/desktop/src/features/workspace/firstVoicingPlan.test.ts
  • apps/desktop/src/features/workspace/firstVoicingPlan.ts
  • apps/desktop/src/features/workspace/voicingCoverageContract.test.ts
  • apps/desktop/src/i18n/index.test.ts
  • apps/desktop/src/i18n/index.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • apps/desktop/vite.config.ts
  • docs/design-system/component-contract.md
  • docs/doctoring/reduced-motion-first-voicing-plan-navigation.md
  • packages/shared-types/src/index.ts
  • packages/shared-types/test/index.test.ts

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.

@seonghobae seonghobae 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 scope: product-gap Customer-visible product gap labels Aug 23, 2026 — with Grok (by xAI)
devin-ai-integration[bot]

This comment was marked as resolved.

@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/FirstVoicingPlanCallout.tsx
Comment thread apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx

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

Open in Devin Review

Comment on lines +81 to +85
const locale = detectPreferredLocale();
const t = createTranslator(locale);
const songIdentity = stableVoicingPlanSongIdentity(song);
const runtimeSong = song as unknown as Partial<RehearsalSong> | null;
const named = resolveFirstVoicingPlan(song);

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: Voicing resolver recomputed on every render

The callout calls resolveFirstVoicingPlan(song) plus createTranslator on every render (apps/desktop/src/features/workspace/FirstVoicingPlanCallout.tsx:81-85), while the surrounding Workspace memoizes its translator and role map. The resolver sorts roles and sections each call. Correct, but inconsistent with the local memoization pattern.

Open in Devin Review

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

Copy link
Copy Markdown
Collaborator Author

@opencode-agent

Repair only canonical BandScope PR #1013 branch feat/workspace-first-voicing-plan at exact head a0285ef59d84f27cf109b1f4b26cd92d45f13c70 if that head is still current; if it moved, refetch and adapt to the intervening delta rather than overwriting another writer.

Current-head review finding PRRT_kwDORjvEXs6brkMM is valid: FirstVoicingPlanCallout currently calls detectPreferredLocale(), createTranslator(locale), and resolveFirstVoicingPlan(song) on every render, while the resolver scans/sorts rehearsal metadata. Use test-driven development: first add a deterministic regression showing an unchanged song rerender does not recompute the voicing-plan resolution/translator work, then memoize only the derived locale/translator/resolution at the narrowest React boundary with correct dependencies. Preserve stable identity reset, renderer fail-closed behavior, copy, reduced-motion navigation, coverage thresholds, dependency files, and #783-owned security state.

Run the focused callout tests, desktop typecheck/lint/coverage, and applicable repository quickcheck. Commit only this performance repair to the same branch and report the exact successor head plus GREEN evidence. Do not create another PR, merge, force-push, self-approve, or weaken protection.

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 scope: product-gap Customer-visible product gap 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