feat(workspace): guide tonight's first stop on map and player - #934
feat(workspace): guide tonight's first stop on map and player#934seonghobae wants to merge 34 commits into
Conversation
Name the earliest labeled cut so the room can hold it. Workspace opens the matching map section; the player exposes Hear only when the owning surface supplies a seek callback. Fail closed on malformed role ids and non-boolean activity flags.
|
Warning Review limit reachedNext included review available in 16 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 (29)
✨ 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 |
![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 head1f6d98f63667cafbadb1ba535cc938c799f78181. -
Head SHA:
1f6d98f63667cafbadb1ba535cc938c799f78181 -
Workflow run: 32193271993
-
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 (25 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (25 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
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 (27 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (27 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
|
Stale predecessor-head review: this submission is explicitly bound to 1f6d98f, while the PR now points at 68a5927 after a test-first zero-length stop-window repair. Dismissal does not count as approval; current-head coverage/OpenCode/Noema/Strix evidence must be regenerated and evaluated independently.
|
Queued @cwl-noema-review and @opencode-agent for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
4 similar comments
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
1 similar comment
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const grid = document.querySelector('[data-testid="song-structure-grid"]'); | ||
| const target = stopSectionIndex >= 0 ? grid?.children.item(stopSectionIndex) : null; | ||
| if (typeof target?.scrollIntoView !== "function") { | ||
| return; | ||
| } | ||
| target.scrollIntoView({ | ||
| block: "nearest", | ||
| behavior: preferredStopScrollBehavior() | ||
| }); | ||
| markStopActionComplete(); |
There was a problem hiding this comment.
📝 Info: Scroll target index matches rendered grid order
resolveFirstStopHandoff picks the earliest stop by time, but navigation uses the section's array index via indexOf and grid.children.item(stopSectionIndex). The song-structure grid renders sections in array order and the tick row is a sibling, not a child, so the index resolves the correct element regardless of time order.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return ( | ||
| <section style={{ padding: "24px" }}> | ||
| <h2>{title}</h2> | ||
| <FirstStopCallout song={song} actionMode="callback-only" onHearStop={onPlayFromSeconds} /> |
There was a problem hiding this comment.
🔍 Player Hear action has no live caller
PlayerFeature renders the Hear action only when onPlayFromSeconds is supplied, but no production code renders PlayerFeature or passes that callback — only tests do. The new callback-only playback path is currently unreachable in the running app.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
opencode-agent
Bot
added
area: ui-ux
|
Queued @cwl-noema-review and @opencode-agent for PR #934 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
1 similar comment
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #934 at head |
|
@opencode-agent Repair only canonical branch |
| const grid = document.querySelector('[data-testid="song-structure-grid"]'); | ||
| const target = stopSectionIndex >= 0 ? grid?.children.item(stopSectionIndex) : null; |
There was a problem hiding this comment.
📝 Info: Scroll navigation couples callout index to grid child order
The callout scrolls to grid.children.item(stopSectionIndex) where stopSectionIndex is the position in song.sections. This stays correct only while SongStructure renders one direct grid child per section in the same order. Any future header row, reorder, or filtered subset silently scrolls to the wrong section.
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-stop 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. |
1 participant
Product outcome
After analysis, the room can see tonight's first explicit stop and take one honest next action: open that cut on the renderer-owned map, or hear it from Player only when playback authority is supplied.
Exact current identity
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.7bb7ba61d3cfea4993454450592558ef1140f7b2.feat/workspace-first-stop-handoff.Product / trust boundary
stopsection whose runtime window is an integer, non-negative, positive-duration interval bounded by sharedMAX_SECTION_TIME_SECONDS.Test-first repair lineage
Earlier branch work established fail-closed collection/object/id/name/priority/activity/timing boundaries, renderer-owned navigation, callback-only playback, reduced-motion behavior, duplicate-identity rejection, and shared time-ceiling use.
Fresh exact-head verification exposed a branch-owned runtime-root defect on predecessor
2b1d1899cff7885d2477eec162255690b763da67:32208030870, job95934940141, checked out PR merge5db6670caa660c3009e505a052c33debcf380af4;FirstStopCallout.localization.test.tsxreproducedTypeError: Cannot read properties of null (reading 'sections')inresolveFirstStopHandoff();null as RehearsalSong), so no synthetic duplicate test was added;7bb7ba61d3cfea4993454450592558ef1140f7b2adds the narrow owning guard!isRuntimeObject(song) || !Array.isArray(song.sections)before property use. No unrelated action, network, filesystem, IPC, package, or dependency authority changed.The same predecessor release log also showed default checkout credential persistence; that is separately owned by canonical #894 and is not duplicated in this feature lane.
Exact-current-head verification state
A fresh repository cycle is dispatched for
7bb7ba61d3cfea4993454450592558ef1140f7b2and is currently queued:ci32212352297;release32212352268;build-baseline32212352307;security-audit32212352251;Security Scan32212352344;sbom32212352315;32212352267;secret-scan-gate32212352221; and32212352359.Queued evidence is non-passing, so exact-current-head GREEN is not claimed yet. Current inline review threads are empty. The only formal OpenCode review is dismissed predecessor-head
CHANGES_REQUESTEDfrom1f6d98f63667cafbadb1ba535cc938c799f78181; dismissal is not approval and there is no qualifying independent non-author current-head approval.Dependency routing
nanoid/pdfjs-dist/undicidependency-security baseline; do not suppress or duplicate it here.ContextualWisdomLab/.github@mainisf16280a0aa215563d29200c5d0bab75c48af614a..github#1117is closed unmerged. The live read-only central owner is.github#1136@a1c3a195d24fd52ea4df4e0fa6b5c8fdd7dddd97; after it integrates, require genuinely fresh exact-current-head coverage/OpenCode/Noema/Strix evidence rather than transferring predecessor results or adding a BandScope workaround.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, canonical dependency prerequisites integrated/revalidated, a qualifying independent non-author last-push approval, and ordinary protected-branch acceptance. Queue/pending/skipped/cancelled/failed/stale/predecessor/protected-base/dismissed/self/author/model-only/rate-limited evidence is not success.
Security Notes
Untrusted rehearsal metadata remains data only. Runtime roots and collections are validated before access; section time windows are bounded; section-local identities and ranked activity are corroborated; renderer positions own navigation; Player playback remains explicit callback authority; and this repair adds no filesystem, network, subprocess, WebView, model, export, persistence, dependency, or review-credential authority.