feat(workspace): name tonight's first new dropout on the map - #1105
feat(workspace): name tonight's first new dropout on the map#1105seonghobae wants to merge 1 commit into
Conversation
After leftover parts return, the ready rehearsal map names who newly sits out so the room can count that part out, or the newly sitting-out part can stay out, from the renderer-owned landing.
|
Warning Review limit reachedNext included review available in 53 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 (15)
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}
| if (returning.length === baselineIds.size && leftovers.length === 0) { | ||
| const newDropouts = sittingOut.filter((node) => !baselineIds.has(node.roleId)); | ||
| const found = namedDropout( |
There was a problem hiding this comment.
🟡 Continued sit-outs become false new dropouts
When another part sits out before leftovers fully return, newDropouts labels its continued absence as new. The map points to the wrong section.
Prompt for agents
In apps/desktop/src/features/workspace/firstNewDropout.ts, firstNewDropout only remembers the original sittingOutIds. During a partial return, a different role can become inactive; when the final original role returns, lines 308-310 classify that already-inactive role as a same-section new dropout. Track activity transitions across each named section, and only accept a role as a new dropout if it changes from active to inactive at or after the completed leftover return. A role that became inactive before the return completed and remains inactive is a continued sit-out and must not produce a cue. Update the tests that currently expect this continued sit-out to be named, and add coverage for both same-section active-to-inactive transitions and genuine later transitions.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** | ||
| * Collect one complete, unique activity record for every song-wide named role. | ||
| * | ||
| * Missing, unknown, duplicate, inherited, accessor, Proxy, sparse, or | ||
| * non-boolean graph evidence fails closed so a leftover part cannot also | ||
| * count as tonight's new dropout in the same section. | ||
| */ | ||
| function namedGraphNodes( | ||
| sectionValue: Record<string, unknown>, | ||
| namedRoles: NamedRoleCatalog | ||
| ): NamedGraphNode[] | null { | ||
| const partGraph = denseOwnArray(sectionValue.partGraph); | ||
| if (!partGraph) { | ||
| return null; | ||
| } | ||
|
|
||
| const nodes: NamedGraphNode[] = []; | ||
| const seenRoleIds = new Set<string>(); | ||
| for (const nodeValue of partGraph) { | ||
| if ( | ||
| !isRuntimeObject(nodeValue) || | ||
| !Object.prototype.hasOwnProperty.call(nodeValue, "role_id") | ||
| ) { | ||
| return null; | ||
| } | ||
|
|
||
| const roleId = meaningfulRangeText(nodeValue.role_id); | ||
| if (!roleId || !namedRoles.has(roleId) || seenRoleIds.has(roleId)) { | ||
| return null; | ||
| } | ||
|
|
||
| const active = ownActiveFlag(nodeValue); | ||
| if (active === null) { | ||
| return null; | ||
| } | ||
|
|
||
| seenRoleIds.add(roleId); | ||
| nodes.push({ roleId, active }); | ||
| } | ||
|
|
||
| return seenRoleIds.size === namedRoles.size ? nodes : null; |
There was a problem hiding this comment.
📝 Info: Strict graph admission matches production
Production emits complete activity graphs and active-only role lists. The song-wide catalog can therefore name inactive parts without rejecting normal analysis results.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const grid = document.querySelector("[data-testid='song-structure-grid']"); | ||
| if (!(grid instanceof HTMLElement)) { | ||
| return; | ||
| } | ||
| const landing = grid.querySelector(`[data-section-index="${namedNewDropout.sectionIndex}"]`); | ||
| if (!(landing instanceof HTMLElement)) { | ||
| return; | ||
| } | ||
| const reduceMotion = window.matchMedia("(prefers-reduced-motion: reduce)").matches; | ||
| landing.scrollIntoView({ | ||
| behavior: reduceMotion ? "auto" : "smooth", | ||
| block: "nearest", | ||
| inline: "nearest" | ||
| }); |
There was a problem hiding this comment.
📝 Info: Scrolling remains renderer-owned
Open targets a renderer-created numeric index. Repeated labels remain distinct, and analysis section IDs never control DOM identifiers.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@opencode-agent You are the sole writer for canonical BandScope branch Fresh source verification confirms Devin thread Repair this existing branch TDD-first. Add the smallest RED timeline where original leftovers reduce, one original role partially returns while a previously active non-baseline role drops out, then the final original leftover returns while that non-baseline role stays inactive. Expected: the continued sit-out is not a new-dropout cue at the completed-return section. Add positive coverage for (a) a genuine active→inactive transition in the same section that completes the original leftover return and (b) a genuine later active→inactive transition after the return is complete. Prove the predecessor failure before source repair. Then make the narrowest causal state fix: carry trustworthy previous-section activity for named candidates (or equivalent transition provenance) and accept a new dropout only on a real active→inactive transition at or after completed leftover return. A role already inactive before completion and still inactive is continued absence, never a new transition. Preserve current own-property boolean admission, duplicate/malformed graph fail-closed behavior, active-role scoping, deterministic section ordering, and renderer-owned Open targeting; do not infer activity from section role-list presence alone. Run focused firstNewDropout + selected-role + Workspace tests first, then full desktop tests/typecheck/lint/docs/security checks and configured statement/branch/function/line coverage at exact 100% where exposed. Refetch exact new head/base after writing. Resolve only the validated bug thread after repaired exact-head focused/full GREEN; the two informational threads may be resolved only if the current code still matches their stated contracts. Let exact-head CI/security/SBOM/release/review gates run; do not transfer predecessor evidence. |
1 participant
Product outcome
After analysis, the ready rehearsal map names tonight's first new dropout so the room can take one next action: Open that landing on the renderer-owned map section. This is who newly sits out after leftover parts return. It is not a leftover sit-out (#1100), leftover return (#1101), remaining leftover (#1102), come-in, tacet, tutti, or a continued sit-out with nobody returning.
Gould (2011) treats a later rest or tacet after a return as a new sit-out for that part, not leftover of the earlier rest. MusicXML 4.0 records the same activity on each part at each measure; a later rest after every earlier silent part has resumed is new dropout evidence (MakeMusic & W3C Music Notation Community Group, 2021).
Next action
[data-testid=song-structure-grid] [data-section-index=N]. Analysissection.idis never DOM-ID authority. Reduced-motion Open usesbehavior: auto.Trust boundary
Own data-property
is_activeonly. Inherited members, own accessors, Proxy get-traps, sparse arrays, duplicate role/graph identities, unnamed roles, blank labels, and malformed roots fail closed. Local-audio fallback does not fabricate a new dropout.Exact current identity
develop@749511c3ad4000090048718f685c6bee6b3d2c25.f35dcfconfeat/workspace-first-new-dropout.Merge gate
Keep unmerged until the unchanged then-current head has repository CI/coverage/security/SBOM/build-baseline terminal-success, exact owned statement+branch coverage on changed files, a qualifying independent non-author last-push APPROVE, and ordinary protected-branch acceptance. Queued, pending, skipped, cancelled, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success. Inherited #783 npm HIGH must not be suppressed here.