feat(workspace): name tonight's first stop on the map - #1131
Conversation
Surface the first form-labeled stop so the room can cut together before the next entrance, without inventing a stop from groove or cue wording. Fail closed on missing or duplicate section identities.
|
Warning Review limit reachedNext included review available in 28 seconds. 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: Team Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough곡의 첫 번째 Changes첫 번째 스톱 안내
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The first-stop guidance can display "none" instead of the fallback ear-check message when a neighboring section has no name, and the associated test currently fails. Merge should wait until unnamed sections are handled correctly. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant firstStop
participant Locales
participant SectionRoadmap
Workspace->>firstStop: 곡의 첫 번째 stop 식별
firstStop-->>Workspace: stop 정보와 인접 label 반환
Workspace->>Locales: 조건별 안내 문구 조회
Locales-->>Workspace: 현지화된 문구 반환
SectionRoadmap->>firstStop: 섹션 카드가 stop 대상인지 확인
firstStop-->>SectionRoadmap: 대상 여부 반환
SectionRoadmap-->>Workspace: 해당 카드에 후속 작업 안내 표시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (8 skipped: 8 unsupported.) ✨ Finishing Touches 💡 1📝 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 |
![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.
![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.
|
|
||
| # Preserve the established package-level import while routing the analysis | ||
| # pipeline through the cross-role temporal coordination boundary. | ||
| RoleExtractor = CoordinatedRoleExtractor |
There was a problem hiding this comment.
🟡 Cached songs miss stop labels
Existing analysis caches bypass CoordinatedRoleExtractor because their schema remains valid. Previously analyzed songs omit stop labels until users clear the cache.
Prompt for agents
The package-level RoleExtractor now changes persisted RehearsalSong output by adding stop labels, but services/analysis-engine/src/bandscope_analysis/api.py still accepts ANALYSIS_CACHE_SCHEMA_VERSION 1 results created before this behavior existed. Increment the analysis-result cache schema/version and its cache directory or otherwise invalidate old result caches, while preserving the reusable stem feature cache. Add a cache test proving an old result is treated as a miss and reanalysis produces stop labels.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if section_start_time <= stop_start_time <= section_end_time: | ||
| section_candidate = section_candidates[section_index] | ||
| if isinstance(section_candidate, dict): | ||
| section_candidate["form_label"] = "stop" |
There was a problem hiding this comment.
🟡 Stop labels retain old identities
A detected stop changes only form_label, leaving IDs such as verse-1. The workspace then treats a stop as a mismatched section identity.
Prompt for agents
Stop-time coordination mutates form_label without reconciling the SectionCandidate id and sequence_index fields. The section model defines id as a slug derived from form_label and sequence_index, while downstream first-stop logic relies on section identities to target the correct card. Preserve the original structural label separately if needed, then emit a contract-consistent stop section identity and sequence numbering, with tests for multiple stops and collisions with existing section IDs.
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Preserve the established package-level import while routing the analysis | ||
| # pipeline through the cross-role temporal coordination boundary. | ||
| RoleExtractor = CoordinatedRoleExtractor |
There was a problem hiding this comment.
🔍 Import paths now differ semantically
Package-level RoleExtractor mutates section labels, while roles.extractor.RoleExtractor does not. Equivalent-looking imports now expose different behavioral contracts.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if not isinstance(section_boundaries, list) or len(section_boundaries) != len(section_candidates): | ||
| return | ||
|
|
||
| stop_time_moments = temporal_hits.detect_stop_time(audio_stems, sample_rate) |
There was a problem hiding this comment.
🔍 Role extraction adds another stem scan
Every package-level extract call runs detect_stop_time across all stems. Repeated callers cannot reuse the temporal result.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if section_start_time <= stop_start_time <= section_end_time: | ||
| section_candidate = section_candidates[section_index] | ||
| if isinstance(section_candidate, dict): | ||
| section_candidate["form_label"] = "stop" | ||
| break |
There was a problem hiding this comment.
📝 Info: Shared endpoints select the prior section
Inclusive ranges overlap at exact boundaries. Forward scanning deliberately assigns that stop to the preceding section, preserving the next section as re-entry.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Supersession evidence is now complete on canonical owner #934 rather than by title/number deduplication.
No #1131 review, check, approval, or predecessor-head evidence is transferred. #934 must earn fresh exact-head gates and independent approval on its own current head. |
1 participant
Product outcome
The ready workspace keeps tonight's first playable range as its own decision, then names tonight's first stop so the room can cut together before the next entrance.
Brand and architecture already treat stops as rehearsal blockers (
docs/brand-story.md,docs/architecture/rehearsal-domain-model.md). The analysis engine already detects stop-time, and the section contract already includes thestopform label. This PR is the buyer-visible map action: cut together here, then come back in on the next named section.Exact current identity
develop@749511c3ad4000090048718f685c6bee6b3d2c25.feat/workspace-first-stop.eac17a61823b2392940f865d95a14058ed95aa9f.Current exact scope
firstStopselects the first form-labeledstopin form order.Merge gate
Keep unmerged until this unchanged 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, and a qualifying independent non-author last-push approval under live protection.
Queued, pending, skipped-required, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.
Do not mix with #811, #828/#770, #897, or first-X rehearsal-map PRs already open. Canonical #783 remains protected
developshipped truth; this branch does not copy or suppress inherited npm HIGH findings.Do not self-approve.
Summary by CodeRabbit
새 기능
문서
테스트