Skip to content

feat(workspace): name tonight's first leftover last-dropout remaining on the map - #1110

Open
seonghobae wants to merge 1 commit into
developfrom
feat/workspace-first-leftover-last-dropout-remaining
Open

feat(workspace): name tonight's first leftover last-dropout remaining on the map#1110
seonghobae wants to merge 1 commit into
developfrom
feat/workspace-first-leftover-last-dropout-remaining

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

The ready rehearsal map names tonight's first leftover last-dropout remaining after leftover last-dropout from existing partGraph evidence so the leftover last-dropout remaining stays out, and the band counts that leftover last-dropout remaining out. This is not a come-in, tacet, leftover sit-out, leftover return, remaining leftover at leftover return, leftover last-return, leftover last-dropout, leftover last-dropout return with nobody still out, tutti, handoff, Fine, last-line breath, a singleton leftover last-dropout, a leftover last-dropout without a later remaining return, a new dropout after remaining leftover, or a new MIR product.

Exact current identity

  • Protected target: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 04e70a0b28e1bffd06924fad085dfe519b1aa975.
  • Branch: feat/workspace-first-leftover-last-dropout-remaining.

Current exact scope

  • firstLeftoverLastDropoutRemaining admits a named leftover sit-out (at least one previously sitting-out named part is own-property active and at least one remains own-property tacet), then a later named leftover return where at least one leftover named part is own-property active and at least one leftover remains own-property tacet, then a later named leftover last-return where every remaining leftover is own-property active, then a later named leftover last-dropout where at least two named parts are own-property tacet, then a later named leftover last-dropout remaining where at least one leftover last-dropout named part is own-property active and at least one leftover last-dropout remains own-property tacet.
  • Blank labels, missing graph nodes, inherited is_active, missing is_active, unnamed roles, leftover sit-outs without leftover return, leftover last-dropouts without a later remaining return, leftover last-dropout returns with nobody still out, singleton leftover last-dropouts, come-ins without a leftover, all-active later sections after leftover last-return (tuttis), remaining leftovers without last-return, new dropouts after remaining leftover, and malformed roots fail closed.
  • When a role is selected, only leftover last-dropout remaining after a leftover last-dropout that includes that named part, or leftover last-dropout remaining of that named part, is shown. The leftover last-dropout remaining gets stay-out copy; included returning or other parts get count-them-out copy.
  • Ready workspace copy names the next action: stay out from the top of the named leftover last-dropout remaining, count the leftover last-dropout remaining out from the top, or confirm who stays out after leftover last-dropout before the first section when leftover last-dropout remaining is missing.
  • AGENTS / CLAUDE / ARCHITECTURE / CHANGELOG and the component contract stay current.
  • Doctoring: docs/doctoring/first-leftover-last-dropout-remaining.md.

Distinct from adjacent first-X work

Verification

  • Desktop Vitest on this head: 248 tests passed; coverage thresholds held on the configured files.
  • npm run typecheck --workspace @bandscope/desktop
  • npm run lint --workspace @bandscope/desktop
  • ./scripts/harness/quickcheck.sh (full harness left to required CI on this exact head)

Security Notes

Attack surface

Untrusted RehearsalSong JSON, section labels, partGraph nodes, is_active, role ids, and role names from analysis or a reopened project.

Trust boundary

Workspace helper never opens files, URLs, IPC, WebView, subprocesses, model artifacts, or export paths. It only admits an own-property leftover sit-out, leftover return with remaining leftover, leftover last-return, leftover last-dropout cohort of at least two named parts, and a later named leftover last-dropout remaining where at least one leftover last-dropout named part is own-property active and at least one leftover last-dropout remains own-property tacet.

Mitigations

Allowlist: section labels and role names must be meaningful text. A missing graph node is not leftover last-dropout remaining. Inherited is_active is isolated. A leftover last-dropout is leftover last-dropout, not leftover last-dropout remaining. A leftover last-dropout return with nobody still out is leftover last-dropout return, not leftover last-dropout remaining. A leftover last-return is leftover last-return, not leftover last-dropout remaining. Remaining leftover at leftover return is remaining leftover, not leftover last-dropout remaining. A singleton leftover last-dropout cannot leave leftover last-dropout remaining. A new dropout after remaining leftover is a dropout, not leftover last-dropout remaining. All-active later sections after leftover last-return are tuttis, not leftover last-dropout remaining, until a leftover last-dropout cohort exists and later leaves someone out. When a role is selected, only leftover last-dropout remaining after a leftover last-dropout that includes that named part, or leftover last-dropout remaining of that named part, is shown. Safe failure returns null so the workspace shows the missing-copy next action.

Test points

firstLeftoverLastDropoutRemaining.test.ts, selected-role search, and the Workspace callout cover the demo all-active case, an explicit bass leftover last-dropout remaining after leftover last-dropout, selected-role scoping, inherited flags, missing is_active, leftover last-dropouts without later remaining return, leftover last-dropout returns, singleton leftover last-dropouts, tuttis, come-ins, leftover returns with nobody still out, remaining leftovers without last-return, new dropouts after remaining leftover, unnamed roles, empty graphs, and literal copy filling.

Dependency and Supply Chain

i18n impact

  • Korean and English locale impact was updated

Dependency / merge gate

Reviewer checklist

  • Gitflow target branch is develop
  • protected-branch rules were not weakened
  • required checks are expected to stay green (do not treat queue/pending/skipped/cancelled as passing; do not merge without independent non-author APPROVE + exact-head required-gate success)

Devin Review

… on the map

Name who still stays out after leftover last-dropout: leftover sit-out, leftover return with remaining leftover, leftover last-return, leftover last-dropout as a cohort of at least two named parts, then leftover last-dropout remaining when at least one leftover last-dropout comes back and at least one remains tacet.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 33 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: ff6116aa-8773-462a-945d-2e719f297828

📥 Commits

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

📒 Files selected for processing (13)
  • AGENTS.md
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/Workspace.tsx
  • apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemaining.selected-role.test.ts
  • apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemaining.test.ts
  • apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemaining.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • docs/design-system/component-contract.md
  • docs/doctoring/first-leftover-last-dropout-remaining.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 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 2 potential issues.

Devin Review

Comment on lines +117 to +120
const roleId = meaningfulRangeText(nodeValue.role_id);
if (!roleId || !namedRoles.has(roleId) || seenRoleIds.has(roleId)) {
return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Permanently inactive parts suppress valid cues

namedGraphNodes rejects every section when a graph part never appears in active-only role lists. Valid analyses with a permanently silent part always show missing guidance.

Prompt for agents
The production engine's RoleExtractor._build_activity_topology emits all five identities in partGraph but includes only active identities in section roles. firstLeftoverLastDropoutRemaining builds names only from those active role lists, then namedGraphNodes rejects any graph identity absent from that catalog. A role that remains inactive for the whole song therefore disables detection even when the named roles contain a valid sequence. Adjust catalog/graph admission so permanently inactive unnamed graph members are excluded from cue candidates without invalidating complete, consistent activity evidence for named roles. Preserve duplicate, malformed, and missing-node fail-closed behavior for identities that participate in detection, and add a production-shaped regression test.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +234 to +239
let reducedFrom: string | null = null;
let sittingOutIds: Set<string> | null = null;
let pendingSitOut: PendingLeftoverSitOut | null = null;
let pendingRemaining: PendingRemainingLeftover | null = null;
let pendingLastReturn: PendingLastReturn | null = null;
let pendingDropout: PendingDropout | null = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 State machine needs stronger invariants

Five temporal stages share mutable state across many early exits. Add transition-table or property tests before extending this detector further.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent You are the sole writer for canonical BandScope branch feat/workspace-first-leftover-last-dropout-remaining, exact head 04e70a0b28e1bffd06924fad085dfe519b1aa975, against protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Apply receiving-code-review, systematic-debugging, test-driven-development, then verification-before-completion; do not create another PR, force-push, rebase destructively, weaken gates, or touch foreign repositories.

The current Devin finding at thread PRRT_kwDORjvEXs6dor4J is VALID against current production contracts. Protected services/analysis-engine/src/bandscope_analysis/roles/extractor.py::_build_activity_topology always emits all five identities in part_graph, but active_roles contains only identities whose is_active is true. Current namedSongRoles() derives its catalog only from section roles, while namedGraphNodes() rejects any graph identity absent from that catalog (!namedRoles.has(roleId)) and also requires seenRoleIds.size === namedRoles.size. Therefore one role that is inactive for the entire song appears in every partGraph but never in any section roles, causing every section to fail closed and suppressing an otherwise valid leftover-last-dropout-remaining cue.

Repair this owning helper in this existing branch. First add the smallest production-shaped RED regression: use active-only section role lists, keep a fifth identity (for example acoustic-guitar) inactive in every section and therefore absent from every section roles, while the named participating roles still form the existing valid leftover -> remaining -> last-return -> 2-role last-dropout -> remaining sequence. Expected result must remain the same valid cue, not null. Prove the regression fails on this exact predecessor before the source fix.

Then make the narrowest causal repair. The graph may contain well-formed, own-boolean identities that never appear in the song-wide active-role name catalog; those permanently inactive identities must be ignored as non-candidate graph members rather than invalidating the section. Preserve fail-closed behavior for malformed graph nodes, duplicate role_ids, inherited/missing/non-boolean is_active, duplicate/conflicting names, and missing graph evidence for every named participating role. Do NOT synthesize display names for never-active identities and do NOT allow an unknown graph identity to become a cue candidate. The final per-section completeness invariant should still prove that every catalogued named candidate has exactly one trustworthy graph node.

Run the focused desktop test first, then the full desktop suite/typecheck/lint and repository-exposed coverage; keep exact 100% configured statement/branch/function/line coverage. Refetch exact new head/base after writing. Resolve only the validated bug thread after the repaired exact head has focused GREEN; leave the advisory state-machine-property-test thread unresolved unless you actually address it. Then let exact-head CI/security/SBOM/release/review gates run; do not claim predecessor evidence.

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