Skip to content

feat(workspace): name tonight's first pending approval on the map - #998

Open
seonghobae wants to merge 12 commits into
developfrom
feat/workspace-first-pending-approval
Open

feat(workspace): name tonight's first pending approval on the map#998
seonghobae wants to merge 12 commits into
developfrom
feat/workspace-first-pending-approval

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

Name tonight's first pending approval so the room can read the waiting scope and open the uniquely named rendered map section.

Design authority: workspace next-action pattern plus docs/design-system/component-contract.md. Distinct from first-assignment (#996), first-open-comment (#997), first-count (#995), first-lyric (#913), and export-and-priority (#900). This does not invent a parallel MIR product; #828 remains the owner of #770. This does not replace #740 (handoff import).

Current exact scope

  • Protected target: develop@acdbea6344fe1231c39535b575f4de35e4c607c9.
  • Branch: feat/workspace-first-pending-approval.
  • FirstApprovalCallout names the owner, the owned scope, and the uniquely named labeled section and time.
  • Prefer changes_requested over pending. Already-approved scopes are not tonight's next action.
  • Open scrolls the renderer-owned song-structure section when the owned scope uniquely names a canonical form label. Missing or ambiguous targets stay guidance-only and do not claim navigation completed.
  • Do not invent a sign-off from assignments, comments, already-approved scopes, empty/whitespace scopes, Korean free-text tokens, or inherited runtime metadata.
  • Owned approval scope is interpolated once as copy and is never rescanned as template syntax.
  • Korean copy keeps particle-safe owner phrasing via {owner}님이.

Demo song: pending MD approval for Verse harmony pass at 0:10 on the unique verse.

Trust boundary

  • Untrusted runtime song, collaboration, approval, section, time-range, and form-label tokens are accepted only as own data properties.
  • Inherited properties, accessors, sparse arrays, and arrays masquerading as records cannot establish a pending approval or a named section.
  • Analysis section.id is never DOM-ID authority. Navigation uses renderer child index under the song-structure region.
  • The approval scope is bounded to 180 Unicode code points without splitting a surrogate pair.
  • Reduced-motion map navigation uses behavior: "auto" when the OS requests it.

Dependency ownership

Canonical #783 owns the protected-base JavaScript HIGH findings (pdfjs-dist / nanoid / undici). This branch has no JavaScript dependency, lockfile, or vulnerability-suppression delta. Inherited npm HIGH must not be suppressed here.

Merge gate

Queued, pending, skipped-required, failed, predecessor-head, protected-base, author-only, model-only, status-only, or synthetic evidence is non-passing.


Open in Devin Review

Name the owner and owned scope so the room can sign off the waiting
approval. Open moves to the uniquely named rendered map section.
Assignments, comments, and already-approved scopes stay guidance-only.
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 55 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: b92550d4-a01b-4cd5-9ed7-7e165ddbcb10

📥 Commits

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

📒 Files selected for processing (23)
  • .Jules/palette.md
  • .jules/palette.md
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/src/features/workspace/FirstApprovalCallout.particle.test.tsx
  • apps/desktop/src/features/workspace/FirstApprovalCallout.reduced-motion.test.tsx
  • apps/desktop/src/features/workspace/FirstApprovalCallout.review-regressions.test.tsx
  • apps/desktop/src/features/workspace/FirstApprovalCallout.test.tsx
  • apps/desktop/src/features/workspace/FirstApprovalCallout.tsx
  • apps/desktop/src/features/workspace/FirstApprovalCallout.workspace-scope.test.tsx
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstApproval.inherited-metadata.test.ts
  • apps/desktop/src/features/workspace/firstApproval.test.ts
  • apps/desktop/src/features/workspace/firstApproval.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
  • docs/design-system/component-contract.md
  • docs/doctoring/reduced-motion-first-approval-navigation.md

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.

chatgpt-codex-connector[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.

Band-wide approvals have no section to navigate to, so the callout renders
no Open action there and the key was never read in either locale. Drop the
dead copy instead of wiring a destination-less button.
devin-ai-integration[bot]

This comment was marked as resolved.

…scope

The changes copy appended {scope}를, which reads ungrammatically when the
owner-supplied scope ends in a consonant. Attach the particle to a fixed
noun ({scope} 부분을) so the sentence is grammatical for every scope ending,
and pin the safe shape in the i18n tests.
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 (FirstApprovalCallout + firstRangeSqueeze), unioned en/ko locale tails (JSON valid, en/ko symmetric). Also fixed a case-collision: merged legacy .Jules/palette.md learnings into canonical .jules/palette.md (10 unique sections appended) and dropped the stale alias. No conflict markers remain. Merge head: 69e5930.

devin-ai-integration[bot]

This comment was marked as resolved.

…ators

Free-text owner scopes like 'pre chorus' previously matched only
'chorus', silently pointing at the wrong section; canonical labels
use hyphens ('pre-chorus'). Normalize [-\s]+ runs to single spaces
for both scope and label keys before whole-token matching, keeping
occupied-span bookkeeping intact.

Evidence: firstApproval suite 12/12 incl. new 'pre chorus' case.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

…scope

- duplicated approval ids now vanish even when only one side is
  actionable (approved is not actionable, so status-first filtering
  let a pending duplicate present itself as the sole sign-off)
- label matching runs on the full trimmed scope; the 180-code-point
  bound applies only to reported copy, so truncation can no longer
  manufacture token boundaries ('verseline' \u2192 'verse')

Evidence: firstApproval suite 14/14 incl. both regression cases.

@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 thread apps/desktop/src/features/workspace/firstApproval.ts
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