feat(workspace): guide tonight's first dropout on map and player - #914
feat(workspace): guide tonight's first dropout on map and player#914seonghobae wants to merge 40 commits into
Conversation
Surface the earliest part-graph handoff as a next action so the outgoing player can hear the last bar before the incoming part takes the section.
|
Warning Review limit reachedNext included review available in 21 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 (14)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough첫 dropout handoff 계산 모듈과 Changes첫 Dropout 기능
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The current changes enforce valid same-section handoffs and ensure navigation or playback actions match the owning surface. Merge readiness remains blocked because the required repository checks for the current PR head are still queued and have not reached terminal success. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant FirstDropoutCallout
participant PlayerFeature
participant PlaybackCallback
Workspace->>FirstDropoutCallout: song 전달
FirstDropoutCallout->>Workspace: 관련 섹션 스크롤 요청
PlayerFeature->>FirstDropoutCallout: onPlayFromSeconds 전달
FirstDropoutCallout->>PlaybackCallback: 30초 위치 재생 요청
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
seonghobae
changed the title
![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.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headf2a7b73ba7a00457e1b421c4b24fc08353d5de73. -
Head SHA:
f2a7b73ba7a00457e1b421c4b24fc08353d5de73 -
Workflow run: 32152920007
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (14 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (14 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: component-contract.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: component-contract.md"]
R2 --> V2["docs review"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (18 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (18 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: component-contract.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: component-contract.md"]
R2 --> V2["docs review"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current heada0b5fe92436113fcf57017f4a065e8901a471f9d. -
Head SHA:
a0b5fe92436113fcf57017f4a065e8901a471f9d -
Workflow run: 32216772558
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (18 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (18 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: component-contract.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: component-contract.md"]
R2 --> V2["docs review"]
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current heada0b5fe92436113fcf57017f4a065e8901a471f9d. -
Head SHA:
a0b5fe92436113fcf57017f4a065e8901a471f9d -
Workflow run: 32220378456
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (18 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (18 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: component-contract.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: component-contract.md"]
R2 --> V2["docs review"]
|
@opencode-agent review Review exact current head Repository |
|
Queued @cwl-noema-review and @opencode-agent for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
4 similar comments
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
1 similar comment
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
opencode-agent
Bot
added
area: ui-ux
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
2 similar comments
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #914 at head |
# Conflicts: # CHANGELOG.md
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - 신규 UI 요소에 대한 100% 테스트 커버리지를 보장하는 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). | ||
| - 신규 UI 요소에 대한 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). |
There was a problem hiding this comment.
🟡 Duplicate changelog bullet under 0.1.4
Two bullets now describe unit tests for the chords and ranges features, both citing the same two test files. The reworded '100% coverage' line was added alongside the original verbatim line instead of replacing it.
| - 신규 UI 요소에 대한 100% 테스트 커버리지를 보장하는 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). | |
| - 신규 UI 요소에 대한 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). | |
| - 신규 UI 요소에 대한 100% 테스트 커버리지를 보장하는 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). |
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (actionMode === "callback-only") { | ||
| onHearDropout!(handoff.endSeconds); | ||
| markDropoutActionComplete(); | ||
| return; |
There was a problem hiding this comment.
🔍 Hear-dropout seeks to the handoff instant
The callback-only action passes handoff.endSeconds to onHearDropout, the section end where the outgoing part has already dropped out. The armed copy (firstDropoutArmed) tells the user to start the last bar before the handoff, so playback beginning at the end point never plays the outgoing part's final bar. A test locks the value at 30 so this reads as intended, but the guidance copy and the seek target disagree.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const grid = document.querySelector('[data-testid="song-structure-grid"]'); | ||
| const target = handoffSectionIndex >= 0 ? grid?.children.item(handoffSectionIndex) : null; |
There was a problem hiding this comment.
📝 Info: Scroll navigation couples to grid child order
The workspace-scroll action selects grid.children.item(handoffSectionIndex) where the index comes from song.sections.indexOf(handoff.section). It is correct only because SongStructure renders song.sections in the same order and resolveFirstDropoutHandoff preserves references. Any future filtering, reordering, or virtualization of the grid would scroll to the wrong section with no error.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Re-review request for exact head |
# Conflicts: # AGENTS.md # CHANGELOG.md # CLAUDE.md # apps/desktop/src/features/workspace/Workspace.tsx # apps/desktop/src/locales/en/common.json # apps/desktop/src/locales/ko/common.json
|
Merged current develop into this branch: resolved shared-file conflicts by including both this feature's first-dropout handoff callout and the newly landed #957 playable-range work (Workspace.tsx keeps both imports; en/ko locales unioned with symmetric keys; AGENTS/CLAUDE/CHANGELOG unioned). No behavior dropped. |
|
|
||
| return ( | ||
| <aside | ||
| id="workspace-surface-dropout" |
There was a problem hiding this comment.
📝 Info: Fixed DOM id on a reused callout
The callout always renders aside id="workspace-surface-dropout", and the component now mounts in both Workspace and Player. Only one feature renders at a time today, so ids do not collide, but a fixed id on a component reused across surfaces is fragile if both ever mount together.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Product outcome
After analysis, the rehearsal map names tonight's first dropout: who gets out of the way, who takes the section, and when. Workspace opens the renderer-owned map position; Player exposes Hear only when its owning playback surface supplies a callback.
Exact current identity
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.a0b5fe92436113fcf57017f4a065e8901a471f9d.feat/workspace-first-dropout-handoff.Current boundary
partGraphhandoff with a strictly active (is_active === true), ranked outgoing part and a same-section reciprocal incoming edge.compute_handoffs()records its reciprocalhandoff_fromedge.Test-first repair lineage
Earlier TDD covers honest Workspace versus Player action authority, renderer-position navigation, same-section target resolution, inactive sources, reciprocal receiver evidence, strict Boolean activity, duplicate local identities, and runtime boundary containment.
Fresh exact-head failures on predecessor
745008c7d44d3d253ef4dac42d7f5eda63a50a75were inspected in CI run32207219130/ job95932669063and release run32207219097/ job95932669006before mutation. Three branch-owned failures were separated:scrollIntoView; production correctly refused success-shaped guidance.resolveFirstDropoutHandoff()rejected an entire section when one unrelated malformed runtime role/node existed, contradicting the existing safe-candidate regression.Repairs on the canonical branch:
64fbd94d002dcadea530e4ea86dd301d16a5bea8locks particle-free Korean dropout body/armed copy while preserving section/time/from/to semantics.c234b6c3d673fcdc9ba9d5c200bbb6c5636b601ecorrects the Workspace integration oracle by supplying the renderer-owned section and asserting the exactscrollIntoView({ block: "nearest", behavior: "smooth" })execution before armed guidance appears.73edbfebf8203be0859431260a390c9d923c925dfilters individually safe runtime roles/graph nodes while preserving duplicate usable-identity rejection and reciprocal-edge validation.a0b5fe92436113fcf57017f4a065e8901a471f9dremoves unsafe Korean fixed subject particles without adding heuristic morphology.The prior CodeRabbit action-mode and reciprocal-edge findings are resolved/outdated on current code. The reciprocal receiver intentionally does not require current-section
is_active === true, matchingservices/analysis-engine/.../activity.py::compute_handoffs()where a next-section activating receiver can already carry current-sectionhandoff_fromevidence.Exact-current-head verification state
Repository workflows were freshly dispatched for
a0b5fe92436113fcf57017f4a065e8901a471f9dand remain queued/pending at the latest refresh:ci32212165681;release32212165662;build-baseline32212165692;security-audit32212165515;Security Scan32212165581;sbom32212165525;32212165601;secret-scan-gate32212165567; and32212165666.Queued/pending evidence is non-passing. Exact-current-head GREEN is therefore not claimed yet. Any terminal failure must be inspected at job/log level before another edit or rerun.
Historical OpenCode
CHANGES_REQUESTEDsubmissions are bound to predecessorc711dbd70af0afbe6fdb1c9e7f3ef09256689258and central coverage-evidence failure; they are not current-head approval. Current inline threads are resolved/outdated, but there is no qualifying independent non-author last-push approval.Dependency / ownership routing
nanoid/pdfjs-dist/undicifailures; do not suppress or duplicate those here.ContextualWisdomLab/.github@mainisf16280a0aa215563d29200c5d0bab75c48af614a..github#1117is closed unmerged. The live read-only central owner is.github#1136@a1c3a195d24fd52ea4df4e0fa6b5c8fdd7dddd97, which fixes the pre-test uv 0.12.1 version-output verifier while preserving checksum/origin/platform controls. Do not add a BandScope leaf workaround or redispatch unchanged reviewer agents while that prerequisite is pending.Security / data-integrity notes
Merge gate
Keep unmerged until one unchanged exact head has every applicable repository and central gate terminal-success, exact required owned statement/branch/function/line coverage and docstring evidence where exposed, zero valid unresolved findings, a qualifying independent non-author last-push approval, dependency-root prerequisites integrated/revalidated, and ordinary protected-branch acceptance. Queue, pending, skipped-required, cancelled, failed, stale, predecessor, protected-base, self/author, model-only, rate-limited, or administrative-bypass evidence is non-passing.