feat(workspace): name tonight's first vamp plan on the map - #1024
feat(workspace): name tonight's first vamp plan on the map#1024seonghobae wants to merge 27 commits into
Conversation
Name the owning part, owned vampPlan copy, labeled section, and time so the next rehearsal action is Open Bass Guitar vamp at 0:10. Do not invent that copy from groove, cue, pad, solo, or other neighboring metadata.
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough분석 엔진이 유효한 stem 진입에서 Changes첫 번째 vamp plan 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to This PR adds a first-vamp callout and activity-derived rehearsal guidance, but the current head is not merge-ready because it can show multiple holding plans in one transition and required formatting and dependency-security checks are failing. The correctness fix and successor-head validation must pass before merging. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant FirstVampPlanCallout
participant resolveFirstVampPlan
participant SongStructureRenderer
Workspace->>FirstVampPlanCallout: song 전달
FirstVampPlanCallout->>resolveFirstVampPlan: 첫 vamp plan 해석
resolveFirstVampPlan-->>FirstVampPlanCallout: 역할, 섹션, 계획, 시간 반환
FirstVampPlanCallout->>SongStructureRenderer: 대상 섹션 탐색
FirstVampPlanCallout->>SongStructureRenderer: scrollIntoView 호출
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 24 files. (9 skipped: 9 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Continue only on the existing canonical Systematic-debugging evidence: exact CI run
Run the repository-pinned formatter on exactly those files and commit only its formatter-equivalent output; do not change vamp semantics while fixing this gate. Then run pinned Ruff check + format-check and the focused Review note: current |
|
Superseded duplicate dispatch note: same-head canonical repair was already active in comment #5406905818 for exact |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Continue only on the existing canonical Systematic-debugging evidence from exact CI run Apply only the formatter-equivalent output produced by the repository-pinned Ruff formatter to that one file on this same branch. Do not hand-edit behavior while satisfying the formatter. Then verify, on the resulting exact successor head, focused |
|
@opencode-agent Repair the exact current-head BandScope-owned Ruff formatter failure on the existing |
|
Queued @opencode-agent for PR #1024 at head |
|
Already queued @opencode-agent on this exact request for PR #1024 at head |
|
Queued @opencode-agent for PR #1024 at head |
|
Already queued @opencode-agent on this exact request for PR #1024 at head |
|
Already queued @opencode-agent on this exact request for PR #1024 at head |
|
Queued @opencode-agent for PR #1024 at head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Continue only on the existing canonical Apply Apply only the exact output of |
# 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
|
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 (FirstVampPlanCallout + 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: 95d59b4. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin review flagged extractor.py as failing `ruff format --check`. Run the pinned formatter; no behavioral changes. Full pytest suite passes (682 passed, 24 skipped).
![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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent Continue only on canonical |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| function resolveVampPlanRenderer(origin: HTMLElement): HTMLElement | null { | ||
| const selector = "#workspace-song-structure-grid"; | ||
| const localScope = origin.closest("aside")?.parentElement ?? null; | ||
| const localRenderers = localScope?.querySelectorAll<HTMLElement>(selector) ?? []; | ||
| if (localRenderers.length === 1) { | ||
| return localRenderers[0] ?? null; | ||
| } | ||
| if (localRenderers.length > 1) { | ||
| return null; | ||
| } | ||
|
|
||
| const globalRenderers = document.querySelectorAll<HTMLElement>(selector); | ||
| return globalRenderers.length === 1 ? (globalRenderers[0] ?? null) : null; |
There was a problem hiding this comment.
🔴 Vamp-plan Open action never navigates
resolveVampPlanRenderer looks up the grid by the id selector #workspace-song-structure-grid, but the rendered grid in SongStructure only sets data-testid="song-structure-grid" with no matching id. The Open button finds no section target and returns early, so it never scrolls to the section or shows the armed copy.
Prompt for agents
The vamp-plan callout resolves the song-structure grid via the CSS id selector #workspace-song-structure-grid (resolveVampPlanRenderer in FirstVampPlanCallout.tsx), then queries [data-section-index] inside it. However, the production grid rendered by the SongStructure component in Workspace.tsx only sets data-testid="song-structure-grid" and never sets id="workspace-song-structure-grid". As a result, in the real app the id lookup matches nothing, resolveVampPlanRenderer returns null, and the Open button's onClick returns early without scrolling to the section or arming the follow-up copy. Add id="workspace-song-structure-grid" to the grid div in SongStructure (the div currently marked data-testid="song-structure-grid" that holds the [data-section-index] children), or change the callout to resolve by the existing data-testid. Note this also breaks the author's own Workspace.test.tsx and the data-testid-based callout tests, which expect scrollIntoView to fire.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** Return a bounded snapshotted own vamp plan, or null when it cannot be shown. */ | ||
| function ownedVampPlan(role: unknown): string | null { | ||
| if (!isRuntimeObject(role)) { | ||
| return null; | ||
| } | ||
| const vampPlan = ownDataValue(role, "vampPlan"); | ||
| if (typeof vampPlan !== "string") { | ||
| return null; | ||
| } | ||
| const trimmed = vampPlan.trim(); | ||
| if (trimmed.length === 0 || trimmed.includes("\n") || trimmed.includes("\r")) { | ||
| return null; | ||
| } | ||
| return ( | ||
| boundedGeneratedActivityVampPlan(trimmed) ?? | ||
| truncateCodePoints(trimmed, MAX_VAMP_PLAN_CHARACTERS) | ||
| ); | ||
| } |
There was a problem hiding this comment.
📝 Info: Vamp copy limited to the owned vampPlan property
resolveFirstVampPlan reads vampPlan only through an own-data descriptor and never synthesizes it from groove, cue, harmony, or other fields, matching the repo rule against inventing vamp copy. Worth a confirming read given how much guarding depends on ownDataValue/hasOwnData behaving as intended across untrusted runtime data.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Product outcome
The mounted rehearsal workspace names tonight's first vamp so a part can hold the groove until the next entrance. Open moves to the matching rendered map section.
Customer-facing next action: Open Bass Guitar vamp at 0:10 (demo). If no corroborated vamp exists, stay on tonight's map for the next rehearsal cue.
Exact current identity
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.49be3fe568ddabca6b0fbcb9c015f0e2b37804c8.7bacf1dfb17df682ea436b88c81a7a4cf3872d4f.feat/workspace-first-vamp-plan.Scope and trust boundary
vampPlancopy on an active, corroborated part plus labeled section and time.gettrap cannot substitute different buyer-visible vamp copy after the own-data check.RehearsalRole.vampPlandeclaration documents that the field is rehearsal-facing vamp guidance owned by the role only when runtime graph evidence corroborates it.vampPlantext stays literal.behavior: autoand Korean next-action copy remains particle-safe.Exact-current-head failure contract
All predecessor-head evidence is historical. Exact current CI run
32836332440checked out merge result7bacf1dfb17df682ea436b88c81a7a4cf3872d4f.gate / ci / rust-checkjob97765915171is terminal-success.ci / build-and-testjob97765915469is terminal-failure at canonical./scripts/harness/quickcheck.sh.uv run ruff check src testsall pass.format --check: exactlyservices/analysis-engine/src/bandscope_analysis/roles/extractor.pywould be reformatted; 86 files are already formatted.A same-exact-head OpenCode owner-control repair is already queued/acknowledged in this PR conversation. It is constrained to applying only the repository-pinned formatter output to
roles/extractor.py, preserving vamp behavior, then rerunning focused role tests, Ruff check/format, and canonical quickcheck on the successor exact head. Do not send another same-head dispatch while that repair remains active.Exact current repository workflow conclusions at this head are:
build-baseline,sbom,SAST Semgrep,bandit,secret-scan-gate;ci,release,security-audit, aggregateSecurity Scan.The
security-auditcurrent-head job97765915405completes setup/install and fails first atAudit npm dependencies; Python/Rust audit stages are skipped and are not counted as success. Canonical #783 remains the sole protected-base JavaScript dependency-security owner; this vamp-plan lane must not copy, suppress, or independently regenerate that baseline.Merge gate
Keep unmerged until one unchanged successor 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. Queued, pending, skipped-required, cancelled, failed, stale, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is non-passing. Never force-push, weaken a gate, suppress dependency findings, or transfer predecessor evidence.
Summary by CodeRabbit
새로운 기능
개선 사항