Skip to content

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

Open
seonghobae wants to merge 8 commits into
developfrom
feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti
Open

feat(workspace): name tonight's first leftover last-dropout remaining last-return tutti on the map#1113
seonghobae wants to merge 8 commits into
developfrom
feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti

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 last-return tutti after leftover last-dropout remaining last-return from existing partGraph evidence so the band can identify the later all-in and count it in from the top. 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 remaining, leftover last-dropout remaining last-return that is already all-in, leftover last-dropout return with nobody still out, tutti, handoff, Fine, last-line breath, a singleton leftover last-dropout, leftover last-dropout remaining last-return without a later tutti, a newly inactive role outside the original reduction cohort while a remaining return is being tracked, or a new MIR product.

Exact current identity

  • Protected target: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 19ae5e65468137c8c0f2bb5d8335e8ff6d3a821f.
  • Branch: feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti.

Current exact scope

  • firstLeftoverLastDropoutRemainingLastReturnTutti admits a named leftover sit-out, then a later named leftover return where at least one leftover part is active and at least one remains tacet, then a later leftover last-return, then a later last-dropout cohort of at least two named parts, then a partial return from that cohort, then a remaining last-return while at least one named part is still out, then a later section where every named graph role is active.
  • Blank labels, missing graph nodes, inherited or missing is_active, unnamed roles, incomplete transition chains, an already-all-in remaining last-return, singleton last-dropouts, and newly inactive roles outside the original reduction cohort while a remaining return is tracked fail closed or discard the current candidate and continue searching as appropriate.
  • A role from the original reduction cohort may become inactive again and later return; that transition alone does not invalidate the candidate. The selected-role regression distinguishes this from an outside-cohort interruption.
  • When a role is selected, only a completed sequence relevant to that named role is shown; an earlier completed sequence that excludes the selected role is discarded so later relevant candidates can still be found.
  • Ready workspace copy names the next action. When the cue is missing, it now tells the player to verify a last-return section where someone is still out and a later section where everyone is in, rather than asking them to confirm the opposite state.
  • English and Korean shipped copy describe the same order. Doctoring documents the executable original-cohort interruption rule.
  • AGENTS / CLAUDE / ARCHITECTURE / CHANGELOG and the component contract remain part of the PR scope.
  • Doctoring: docs/doctoring/first-leftover-last-dropout-remaining-last-return-tutti.md.

Distinct from adjacent first-X work

Verification

Current-head evidence is intentionally reset after the review repair. Predecessor-head successes do not transfer.

  • Desktop Vitest / coverage on exact 19ae5e65468137c8c0f2bb5d8335e8ff6d3a821f — hosted exact-head run pending.
  • npm run typecheck --workspace @bandscope/desktop on the exact current head — hosted exact-head run pending.
  • npm run lint --workspace @bandscope/desktop on the exact current head — hosted exact-head run pending.
  • ./scripts/harness/quickcheck.sh on the exact current head — required exact-head CI remains authoritative.
  • The existing Workspace regression expects the corrected English missing-cue sentence; the current English locale now supplies that exact sentence. Korean copy was updated to the same transition order.

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

The Workspace helper never opens files, URLs, IPC, WebView, subprocesses, model artifacts, or export paths. It only evaluates validated rehearsal graph evidence and returns a cue or safe failure.

Mitigations

  • Section labels and role names must be meaningful text; missing or inherited activity evidence is rejected.
  • A remaining last-return that is already all-in is not promoted into this cue; a later all-in section is required.
  • While a remaining return is tracked, a newly inactive role outside the original reduction cohort invalidates/reseeds the candidate. Original-cohort members may become inactive again and later return without being rejected solely for that transition.
  • Selected-role search discards a completed candidate that excludes the selected role and resumes from current evidence instead of pinning stale state.
  • Safe failure returns null or discards the invalid candidate; no filesystem/network/model authority is added.
  • Rejected or accepted cues are not logged. Copy interpolation keeps rehearsal values literal.

Test points

firstLeftoverLastDropoutRemainingLastReturnTutti.test.ts, selected-role search, and the Workspace callout cover the core sequence, selected-role scoping, inherited/missing flags, incomplete chains, already-all-in last-return, singleton last-dropouts, original-cohort re-drop/return, outside-cohort interruption, empty/malformed graphs, and literal copy filling.

Dependency and Supply Chain

i18n impact

  • English and Korean shipped locale values are updated to the same valid transition order.

Dependency / 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, synthetic, or administrative-bypass evidence is not success.
  • Never bypass protection or transfer predecessor evidence. Do not self-approve.
  • Not a MIR product and not a parallel [Product Gap] Add real-audio MIR accuracy acceptance benchmarks #770 owner. test(analysis): govern real YouTube known-stem benchmark #828 remains the known-stem vehicle.

Reviewer checklist

  • Gitflow target branch is develop.
  • Protected-branch rules were not weakened.
  • Required current-head checks and independent last-push approval are still required before merge.

Devin Review

… last-return tutti on the map

The ready rehearsal map names leftover last-dropout remaining last-return
tutti after leftover last-dropout remaining last-return so the band counts
that all-in from the top. Leftover last-dropout remaining last-return that
is already all-in stays leftover last-dropout remaining last-return.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 18 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: Team

Run ID: e6c07100-4a97-4697-88de-e4719d8df2b5

📥 Commits

Reviewing files that changed from the base of the PR and between fbeced2 and 19ae5e6.

📒 Files selected for processing (6)
  • apps/desktop/src/features/workspace/Workspace.test.tsx
  • apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemainingLastReturnTutti.selected-role.test.ts
  • apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemainingLastReturnTutti.ts
  • apps/desktop/src/locales/en/common.json
  • apps/desktop/src/locales/ko/common.json
  • docs/doctoring/first-leftover-last-dropout-remaining-last-return-tutti.md
📝 Walkthrough

Walkthrough

partGraph의 역할 상태를 분석해 첫 잔여·드롭아웃·라스트 리턴·투티 구간을 찾습니다. 결과를 Workspace 안내로 표시하고 영어·한국어 번역, 테스트, 프로젝트 문서를 추가합니다.

Changes

첫 잔여 투티 안내

Layer / File(s) Summary
투티 분석 상태 머신
apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemainingLastReturnTutti.ts, apps/desktop/src/features/workspace/*firstLeftoverLastDropoutRemainingLastReturnTutti*.test.ts
역할과 partGraph를 검증합니다. 잔여·드롭아웃·라스트 리턴·투티 단계를 상태 머신으로 추적합니다. 조건이 맞지 않으면 null을 반환합니다.
Workspace 안내 통합
apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/locales/*/common.json
분석 결과를 메모이제이션합니다. 역할별 안내와 누락 안내를 번역하고 장미색 Workspace 섹션으로 표시합니다. 관련 UI 테스트를 추가합니다.
기능 계약과 문서 갱신
AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, docs/design-system/component-contract.md, docs/doctoring/*
ready Workspace의 새 명칭과 동작을 프로젝트 문서, 컴포넌트 계약, CHANGELOG에 반영합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to fbece

The PR adds a localized rehearsal cue, but merge readiness is not yet established: the fallback message directs users to confirm the opposite state, the doctoring contract conflicts with an executable selected-role test, and the declared PR head does not match the reviewed revision. These bounded issues should be corrected or reconciled before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant TuttiAnalyzer
  participant Localization
  participant Player
  Workspace->>TuttiAnalyzer: song과 activeRole로 첫 투티 구간 분석
  TuttiAnalyzer-->>Workspace: 구간, remaining 역할 또는 null 반환
  Workspace->>Localization: 안내 키와 구간 정보 전달
  Localization-->>Workspace: 번역된 안내 문구 반환
  Workspace-->>Player: 투티 진입 안내 표시
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (8 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 워크스페이스 지도에서 첫 번째 leftover last-dropout remaining last-return tutti를 식별하고 이름을 표시하는 주요 변경을 정확하고 구체적으로 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (8 skipped: 8 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti

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.

Copy link
Copy Markdown
Collaborator Author

Exact-current-head ca3319b3046be9f34f22abfdef379d935f8c3364 names leftover last-dropout remaining last-return tutti after leftover last-dropout remaining last-return that is not already all-in. Leftover last-dropout remaining last-return that is already tutti stays leftover last-dropout remaining last-return.

Predecessor reviews do not transfer.

@opencode-agent review
@cwl-noema-review review

Read-only review of this unchanged SHA. Do not self-approve, update the branch, or merge. Inherited #783 npm HIGH findings stay #783-owned.

@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.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +280 to +287
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;
let pendingRemainingDropout: PendingRemainingDropout | null = null;
let pendingTutti: PendingTutti | 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.

🔍 Transition invariants need consolidation

Six independent pending variables duplicate advancement and reset rules across a 599-line search. Consolidating explicit states can prevent further inconsistent candidate invalidation.

Devin Review

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

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Become the sole writer for canonical BandScope branch feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti only while it still resolves to exact test-only head fbeced2aba38b74760ee0e5af26534ea1fb0c3dd and protected develop still resolves to 749511c3ad4000090048718f685c6bee6b3d2c25. Apply superpowers:using-superpowers, receiving-code-review, systematic-debugging, test-driven-development, and verification-before-completion. Refetch exact head/base plus apps/desktop/src/features/workspace/firstLeftoverLastDropoutRemainingLastReturnTutti.ts and firstLeftoverLastDropoutRemainingLastReturnTutti.selected-role.test.ts immediately before any write; abort/adapt if either moved. Do not create another branch/PR, force-push, destructively rebase, weaken gates, self-approve, or touch foreign repositories.

Fresh current-head Devin finding on the predecessor ca3319b3046be9f34f22abfdef379d935f8c3364 is verified against source: once pendingTutti holds a completed sequence, the branch if (activeRole && !selectedPartBelongs(pending, activeRole)) { continue; } retains that unrelated completed sequence forever, so a later valid sequence for the selected role cannot be discovered.

The exact test-only successor fbeced2... adds keeps searching after an earlier completed sequence excludes the selected part, using a fourth always-valid named observer role. The first complete sequence excludes observer; a second valid complete sequence includes observer. First prove RED by actually running this regression against fbeced2...; queued/pending CI is not RED evidence. If the test does not fail for the intended reason, stop and reassess rather than editing production.

Then make the narrowest causal repair at the pendingTutti selected-role mismatch boundary: discard the stale completed sequence and resume state-machine search. Preserve the current section as the next reduction baseline when it has inactive roles (reducedFrom = sectionLabel, sittingOutIds = new Set(...)); clear the baseline when all roles are active. Do not merely suppress the eventual return value, and do not alter song-wide identity validation or unrelated first-X semantics. Keep the existing negative selected-role contract.

Run the focused selected-role regression, the full helper tests, desktop typecheck/lint/coverage-relevant suite, and applicable repository quickcheck. Commit only the regression-compatible source repair to this same branch. Report the successor exact SHA and exact RED/GREEN evidence. Do not resolve the Devin thread until the successor exact-head regression is GREEN; predecessor approvals/checks do not transfer.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Exact-head repair request for fbeced2aba38b74760ee0e5af26534ea1fb0c3dd against protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Work only on the existing branch feat/workspace-first-leftover-last-dropout-remaining-last-return-tutti; refetch head/base/blob before writing and abort/adapt if the lane moves.

Apply receiving-code-review + systematic debugging + TDD. The current-head Devin findings are valid against the source: (1) the existing RED regression keeps searching after an earlier completed sequence excludes the selected part exposes pendingTutti retaining an excluded completed sequence forever; repair by discarding/reseeding that candidate so a later selected-role sequence can be discovered, without transferring predecessor evidence. (2) Add a smallest RED regression for a new dropout outside the tracked remainingDropoutIds after partial return; the pendingRemainingDropout stage currently inspects only tracked remaining IDs and can later publish a false all-in cue. Invalidate/reseed when activity continuity is broken, including a new dropout appearing at the partial-return transition itself. Do not weaken the state contract.

Also verify and fix the current CodeRabbit findings on this exact head: the English missing-copy currently tells the user to confirm the opposite state (last-return is all-in) even though a valid candidate requires that last-return not yet be all-in followed by a later all-in; make the customer-facing instruction describe the actual next action and keep Korean semantically equivalent. Reconcile doctoring/PR-body claims with the executable rule after the state-machine fix; source/tests define the intended repaired behavior, not stale prose.

Obtain focused RED→GREEN and full desktop/typecheck/lint evidence; then allow exact-head CI/security/coverage/review gates to re-materialize. Resolve only threads whose finding is actually addressed. Do not self-approve, force-push, weaken gates, mutate central .github, or create a competing PR.

@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.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines +307 to +315
if (activeRole && !selectedPartBelongs(pending, activeRole)) {
pendingTutti = null;
if (sittingOut.length === 0) {
reducedFrom = null;
sittingOutIds = null;
} else {
reducedFrom = sectionLabel;
sittingOutIds = new Set(sittingOut.map((node) => node.roleId));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Rejected candidates reuse current evidence

After pendingTutti excludes the selected role, fallthrough lets that section seed a new search. All-active sections instead clear the reduction baseline.

Devin Review

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

Comment on lines +482 to +487
const originalSitOutIds = new Set(pendingRemaining.originalSitOutIds);
if (sittingOut.some((node) => !originalSitOutIds.has(node.roleId))) {
pendingRemaining = null;
reducedFrom = sectionLabel;
sittingOutIds = new Set(sittingOut.map((node) => node.roleId));
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Interruption uses the original cohort

The originalSitOutIds check rejects inactivity outside the original reduction. Returning original members can drop again without falsely interrupting the sequence.

Devin Review

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

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Review the unchanged exact successor 19ae5e65468137c8c0f2bb5d8335e8ff6d3a821f against protected develop@749511c3ad4000090048718f685c6bee6b3d2c25 as an independent non-author reviewer. Do not write source/docs/refs or merge. Re-verify the repaired state-machine behavior plus the successor review repairs: the missing-cue copy must say the last-return still has someone out and a later section is all-in; English and Korean must preserve that order; doctoring must match the executable rule that only a newly inactive role outside the original reduction cohort invalidates/reseeds while an original-cohort role may re-drop and return. Also verify the current selected-role candidate reseeding/continuity regressions and current exact-head diff rather than predecessor prose. Submit formal APPROVED only if this exact head is review-clean, otherwise formal CHANGES_REQUESTED with current-head findings. Predecessor reviews and pending/queued/status-only evidence do not qualify.

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