Skip to content

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

Open
seonghobae wants to merge 20 commits into
developfrom
feat/workspace-first-dynamics-plan
Open

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

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Buyer next action

The mounted rehearsal workspace names tonight's first dynamics plan so a part can lock the owned dynamics before rehearsal starts.

Demo: Bass Guitar still has Keep the verse under the vocal so the chorus still has somewhere to lift. Open is the next action: it moves to the matching rendered song-structure section.

Protected base: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
Exact current head: c7fa06aed32757aac5c94503582f39b07f9a36e5 on feat/workspace-first-dynamics-plan.

What the player sees

  • The callout names the owning part when an active graph node corroborates it, the owned dynamicsPlan copy, the labeled section, and the time.
  • Open scrolls the renderer-owned song-structure section and arms the next action: lock that dynamics before the room starts.
  • If no part owns rehearsal-facing dynamics copy, the card stays guidance-only. Stay on tonight's map.

Authority boundary and current-head repair

Security Notes

  • Project-file metadata is untrusted. The shared schema, buyer-visible resolver, and Rust loader now apply one explicit Unicode-safe blank and single-line dynamics-plan policy; malformed values fail closed without echoing rejected content. Tests cover TypeScript, UI, and Rust boundaries.

Exact-head verification state

Fresh repository workflows for c7fa06aed32757aac5c94503582f39b07f9a36e5 are queued/pending at the latest refetch. No non-terminal result and no predecessor-head evidence is counted as success.

Dependency / merge gate

  • fix(security): establish canonical npm, PDF.js, Nanoid, and Undici baseline #783 remains the canonical protected JavaScript dependency-security owner. Inherited pdfjs-dist / nanoid / undici HIGH findings are not copied or suppressed here. No lockfile change.
  • Keep unmerged until the unchanged current head has every applicable repository/central CI, cross-platform build, exact owned statement+branch coverage, docstring, SAST/security/SBOM/supply-chain/package/release gate terminal-success; zero valid unresolved findings; a qualifying independent non-author last-push approval; and ordinary branch-protection acceptance.
  • Queued, pending, skipped-required, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.
  • Never bypass protection or transfer predecessor evidence. Do not self-approve.

Open in Devin Review

The mounted rehearsal workspace names tonight's first dynamics plan so a
part can lock the owned dynamics before rehearsal starts. Open moves to
the matching rendered song-structure section.

Do not invent that copy from groove, cue, simplification, overlap, range,
chord labels, function labels, setup notes, transposition plans, confirmed
overrides, harmonic explanations, or confidence notes. Distinct from
first-setup-note, first-transposition-plan, and first-tuning-plan.

Security Notes:
- Attack surface: untrusted rehearsal-song runtime metadata (sections,
  roles, partGraph, identity accessors) plus DOM navigation against the
  mounted song-structure renderer.
- Trust boundary: only own data properties on dense collections are read.
  Inherited/accessor/sparse/hostile getters fail closed to null and keep
  the callout guidance-only.
- Navigation authority is the renderer-owned section index, not analysis
  ids.
- Logging/privacy: no path, filename, or rejected-metadata echo.
- Test points: inherited-metadata suite, particle-safe Korean copy,
  reduced-motion scroll, workspace-scope renderer selection, Workspace
  navigation. Shared-types 20/20 at 100%. Desktop targeted 68/68.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 33 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: 9c81c7af-c910-46a5-aff3-d8a735a190fb

📥 Commits

Reviewing files that changed from the base of the PR and between 749511c and c7fa06a.

📒 Files selected for processing (31)
  • .Jules/palette.md
  • .jules/palette.md
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/core/src/lib.rs
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.identity.test.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.memoization.test.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.particle.test.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.reduced-motion.test.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.tsx
  • apps/desktop/src/features/workspace/FirstDynamicsPlanCallout.workspace-scope.test.tsx
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/dynamicsCoverageContract.test.ts
  • apps/desktop/src/features/workspace/firstDynamicsPlan.inherited-metadata.test.ts
  • apps/desktop/src/features/workspace/firstDynamicsPlan.proxy-authority.test.ts
  • apps/desktop/src/features/workspace/firstDynamicsPlan.section-label.test.ts
  • apps/desktop/src/features/workspace/firstDynamicsPlan.test.ts
  • apps/desktop/src/features/workspace/firstDynamicsPlan.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-dynamics-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.

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.

# 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 (FirstDynamicsPlanCallout + 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: bb06076.

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

@devin-ai review this PR at current HEAD c7fa06a. Shared-types, the buyer-visible first-dynamics resolver, and Rust project loading now use the same explicit Unicode plan policy; tests cover NEL/LS/PS rejection 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 1 new potential issue.

Devin Review

Comment on lines +411 to +450
/** 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

Rust and TypeScript use identical whitespace and line-separator sets. BOM-padded text remains valid, while blank or line-separated plans fail at both boundaries.

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