feat(workspace): name tonight's first breakdown plan on the map - #1039
feat(workspace): name tonight's first breakdown plan on the map#1039seonghobae wants to merge 17 commits into
Conversation
Name the earliest corroborated density drop so the staying part can hold the sparse texture until the drop. Engine copy is emitted only from real stem activity when the previous graph has at least three distinct sources and the current graph holds one or two of those same sources, with no new entrance. Heuristic topology, first sections, full stops, and mixed entrances stay unnamed. Open scrolls the matching rendered map section.
|
Warning Review limit reachedNext included review available in 16 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 (7)
📝 WalkthroughWalkthrough분석 엔진이 인접 섹션의 stem 밀도 감소를 기반으로 첫 Changes첫 breakdown plan 흐름
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The new breakdown callout behavior is covered by a fixture missing the explicit model provenance required by the expected post-open state. This can block or weaken CI validation of the provenance-gated UI behavior, so the fixture should be corrected before merge. Sequence Diagram(s)sequenceDiagram
participant AnalysisEngine
participant SharedTypes
participant Workspace
participant SongStructureGrid
AnalysisEngine->>SharedTypes: breakdownPlan 및 provenance 포함 역할 생성
SharedTypes->>Workspace: 검증된 RehearsalSong 전달
Workspace->>Workspace: 첫 breakdown plan 해석 및 현지화
Workspace->>SongStructureGrid: 대상 섹션으로 스크롤
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 70 functions across 32 files. (6 skipped: 6 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if ( | ||
| typeof sectionId !== "string" || | ||
| sectionId.trim().length === 0 || | ||
| typeof sectionLabel !== "string" || | ||
| !SECTION_FORM_LABEL_SET.has(sectionLabel) || | ||
| timeRange === null || | ||
| previousTimeRange === null || | ||
| previousTimeRange.end !== timeRange.start | ||
| ) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
📝 Info: UI resolver adds abutment gate the engine lacks
resolveSafeFirstBreakdownPlan requires the array-adjacent previous section to abut (previousTimeRange.end === timeRange.start), while the engine's _activity_breakdown_plan emits from activity alone with no timing constraint. Any non-contiguous section reaching the UI silently suppresses an engine-emitted breakdown. Section boundaries are contiguous today, so this holds.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@opencode-agent implement Work only on canonical branch Four current-head findings are verified BandScope-owned defects, not report-only notes:
Run focused tests first, then repository-pinned format/lint/typecheck, Rust tests, Python tests at exact 100% branch/statement coverage, canonical quickcheck, and leave all protection/security/release gates intact. Resolve only the four addressed threads after exact successor-head verification. Do not touch foreign repositories or unrelated feature lanes. |
![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.
| role = roles[role_key] | ||
| breakdown_plan = self._activity_breakdown_plan( | ||
| role_id, | ||
| roles, | ||
| role_activity, | ||
| previous_role_activity, | ||
| ) | ||
| if breakdown_plan is not None: | ||
| role = role.copy() | ||
| role["breakdownPlan"] = breakdown_plan | ||
| role["breakdownPlanSource"] = "model" | ||
| active_roles.append(role) |
There was a problem hiding this comment.
📝 Info: Two-source hold tags both staying parts
When a dense section drops to two named staying sources (bass + vocal), _build_activity_topology sets a model breakdownPlan on both: bass gets with Lead Vocal, vocal gets with Bass Guitar. The UI resolver picks one by priority, so display is fine, but the persisted song carries two roles each naming the other as the hold partner.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const holdingRole = pickHoldingRole( | ||
| rankedActiveRoles(section as RehearsalSection).flatMap((metadata) => { | ||
| if (!previousActiveIds.has(metadata.id)) { | ||
| return []; | ||
| } | ||
| const breakdownPlan = ownedBreakdownPlan(metadata.role); | ||
| return breakdownPlan === null | ||
| ? [] | ||
| : [ | ||
| { | ||
| ...metadata, | ||
| breakdownPlan: breakdownPlan.text, | ||
| breakdownPlanSource: breakdownPlan.source, | ||
| breakdownPlanGuidance: breakdownPlan.guidance | ||
| } | ||
| ]; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
📝 Info: Resolver allows accompaniment roles to hold a plan; engine never does
The engine's _activity_breakdown_plan refuses to assign a plan to keys-left, keys-right, or acoustic-guitar. The resolver's holding-role selection in resolveSafeFirstBreakdownPlan has no such exclusion, so any active role carrying own-data breakdownPlan can be named. Harmless for user-authored plans, but the two layers disagree on eligible holders.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** Preserve workspace-instance authority for immutable edits emitted by this workspace. */ | ||
| const commitSongUpdate = (nextSong: RehearsalSong) => { | ||
| if (!onSongUpdate) return; | ||
| localSongUpdateRef.current = nextSong; | ||
| onSongUpdate(nextSong); | ||
| }; |
There was a problem hiding this comment.
📝 Info: commitSongUpdate identity changes each render
commitSongUpdate is a fresh closure every render, now passed to SectionRoadmap in place of the previously-forwarded onSongUpdate. Prop identity changes each render, defeating any downstream memoization. Runtime behavior is unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (song !== previousSongRef.current) { | ||
| const isLocalWorkspaceUpdate = song === localSongUpdateRef.current; | ||
| if (!isLocalWorkspaceUpdate) { | ||
| workspaceInstanceRef.current = song; | ||
| } | ||
| localSongUpdateRef.current = null; | ||
| previousSongRef.current = song; | ||
| } |
There was a problem hiding this comment.
📝 Info: Opened-state persistence assumes parent echoes the exact song object
commitSongUpdate records the exact nextSong reference so a local edit keeps workspaceInstanceRef stable and the opened breakdown survives. A parent that clones or transforms the song before passing it back makes isLocalWorkspaceUpdate false, advancing the instance key and resetting the opened breakdown. Today all in-workspace edits route through it, but the contract is fragile for future callers.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Buyer-visible next action
This PR names tonight's first breakdown plan on the rehearsal map when an active part stays in after a density drop: previous graph ≥3 distinct sources, current graph 1–2 of those same staying sources, no new entrance, not a full stop. Open scrolls that rendered map section so the staying part can hold the sparse texture until the drop.
Breakdown is a corroborated density drop that keeps playing. Dropout is a leaving part. Cutoff is a stop-time. Pickup is rest-then-enter. Turnaround is shared continuation at section end. Heuristic-only topology stays unnamed.
Exact current identity
develop@749511c3ad4000090048718f685c6bee6b3d2c25(fix(security): establish canonical npm, PDF.js, Nanoid, and Undici baseline #783 integrated, feat(workspace): name tonight's first playable range on the map #957 playable range on develop).18acb454234939551a2ad2cdf72cef4d2b956001.feat/workspace-first-breakdown-plan.Current implemented boundary
breakdownPlancopy only from owned data properties and snapshots it once before ranking.Hold this breakdown with {target}; keep it sparse until the drop.(or the solo hold) only when previous graph source count is ≥3, current graph holds 1–2 of those same sources, and no new entrance arrives. Custom role-owned guidance is preserved verbatim unless model provenance is explicit.{at} {role} 브레이크다운 열기.data-section-index) and fails closed on ambiguous or missing targets. Reduced motion usesbehavior: "auto".Dependency / merge gate
Canonical #783 is protected
developshipped truth. This branch inherits that JavaScript baseline and does not duplicate or suppress it. Inherited npm HIGH must not be suppressed elsewhere.Keep unmerged until the unchanged then-current head has every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved findings, and a qualifying independent non-author last-push approval under live branch protection.
Queued, pending, skipped, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.
Summary by CodeRabbit