feat(workspace): name tonight's first ear check on the map - #1001
feat(workspace): name tonight's first ear check on the map#1001seonghobae wants to merge 30 commits into
Conversation
Surface the earliest low or medium confidence section as a rehearsal next action so uncertain analysis is confirmed by ear before the room starts.
|
Warning Review limit reachedNext included review available in 33 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 (16)
📝 WalkthroughWalkthroughChanges첫 ear check 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to A malformed or hostile song record can still crash the workspace while rendering the ear-check guidance, and the Korean callout may communicate the wrong state. The PR should not merge until the guarded access and localized wording are corrected or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Workspace
participant FirstEarCheckCallout
participant resolveFirstEarCheck
participant SongStructureRenderer
Workspace->>FirstEarCheckCallout: 현재 song 전달
FirstEarCheckCallout->>resolveFirstEarCheck: 첫 ear check 해석
resolveFirstEarCheck-->>FirstEarCheckCallout: ear check 정보 반환
FirstEarCheckCallout->>SongStructureRenderer: 대상 섹션 탐색
SongStructureRenderer-->>FirstEarCheckCallout: reduced-motion 옵션으로 스크롤
🚥 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…te semantics - key armed guidance on owned song id plus content fingerprint so distinct songs sharing an id stop sharing armed state while immutable copies keep it - resolve map navigation through accessibility regions owning rendered section cells first, keeping the data-testid hook only as a nested identification of the same map; collapse nested surfaces to the outermost one and fail closed on disjoint local or global ambiguity - memoize locale detection, translator, and per-song ear-check resolution once per mount/song to match the surrounding workspace pattern - reserve band-wide ear-check copy for section-owned uncertainty: sections whose only uncertainty sits on inactive or unprovable parts no longer claim tonight's ear check
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Repair only canonical BandScope PR #1001 branch Use systematic debugging + TDD. Two current-head BandScope-owned defects are now proven:
For defect 2, add the smallest realistic regression first that lets resolver validation succeed but makes a later root Run focused ear-check tests, full desktop tests/coverage, typecheck/lint, then repository quickcheck/current-head CI. Resolve only |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Queued @opencode-agent for PR #1001 at head |
![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 headc8804b7662b9a9d6bcabc920fcab73330bb89f4f. -
Head SHA:
c8804b7662b9a9d6bcabc920fcab73330bb89f4f -
Workflow run: 32766926114
-
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 (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"]
|
|
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
|
Resolved merge conflicts against origin/develop (749511c): union-merged AGENTS.md/CHANGELOG.md bullets, sentence-merged CLAUDE.md, kept both import sets in Workspace.tsx (FirstEarCheckCallout + firstRangeSqueeze), unioned en/ko locale tails (JSON valid, en/ko symmetric). Merged legacy .Jules/palette.md into canonical .jules/palette.md, dropped alias. No conflict markers remain. Merge head: d3bbd82. |
| function stableEarCheckSongIdentity(song: RehearsalSong): unknown { | ||
| const songId = stableEarCheckSongId(song); | ||
| if (songId === null) { | ||
| return song; | ||
| } | ||
| const fingerprint = earCheckSongFingerprint(song); | ||
| return fingerprint === null ? song : `${songId}\u0000${fingerprint}`; | ||
| } |
There was a problem hiding this comment.
📝 Info: Armed state keyed on id plus bounded fingerprint
stableEarCheckSongIdentity (FirstEarCheckCallout.tsx) keys armed guidance on song.id plus a bounded fingerprint of title and per-section id/start/end. Two distinct songs sharing all of those would share armed state, but the opened guard at FirstEarCheckCallout.tsx additionally compares section id, index, holding role, and time, so a different ear check still resets. Behavior holds.
Was this helpful? React with 👍 or 👎 to provide feedback.
| useEffect(() => { | ||
| setOpenedEarCheck(null); | ||
| }, [ | ||
| songIdentity, | ||
| earCheckSectionIndex, | ||
| earCheck?.section.id, | ||
| earCheck?.holdingRole?.id, | ||
| earCheck?.atSeconds | ||
| ]); |
There was a problem hiding this comment.
📝 Info: Redundant reset effect
The effect at FirstEarCheckCallout.tsx clears openedEarCheck when identity, section, role, or time changes. The opened computation at lines 223-229 already returns false whenever any of those differ, so the effect only triggers an extra render and never alters what the user sees.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Product outcome
Uncertain analysis is no longer a badge-only fact. After a song is analyzed, the mounted rehearsal workspace names tonight's first ear check — the earliest canonical labeled section whose owned confidence is
lowormedium— and the Open action moves to that uniquely rendered map section so the room can confirm it by ear before rehearsal starts.Protected target:
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.Exact current head:
c8804b7662b9a9d6bcabc920fcab73330bb89f4f.Branch:
feat/workspace-first-ear-check.Buyer-visible next action
confidence.notescome only from that same part; another uncertain part's note cannot appear under the named holder.confidence.notesstay visible as a text node and are never rescanned as template syntax.high, the callout stays guidance-only: stay on tonight's map until a part is marked uncertain.피아노 파트를).Current-head repairs
aa610108d76e1604ffe1ffffcdad99ed4e1323b7adds a regression requiring concurrent ear-check callouts to avoid duplicate DOM ids. No hosted RED run completed before the successor push, so this is regression-first source evidence rather than terminal hosted RED evidence.da9c1a70041073421b798ca795f0239540e5681euses ReactuseId()so concurrently mounted callouts receive unique landmark ids while preserving theworkspace-surface-ear-check-prefix.c5d3a746e3f5dd73f98045e5ca55a5c83553dc8adds a regression requiring a runtimeverse-customsection label to fail closed rather than surface untranslated buyer copy. No hosted RED run completed before the successor push.c8804b7662b9a9d6bcabc920fcab73330bb89f4faccepts section labels only from sharedSECTION_FORM_LABELSbefore they can become ear-check/navigation/copy authority.Trust boundary
SECTION_FORM_LABELS; unknown nonempty strings are not buyer-visible localization authority.song.idonly as an owned plain-string data property and pairs it with a bounded content fingerprint; accessor/descriptor-trap identity or oversized/hostile songs fall back to object identity.data-section-index; analysissection.idis never DOM-ID authority.highconfidence cannot invent an ear check.#783remains the canonical protected JavaScript dependency-security owner. Inherited dependency findings are not copied or suppressed here.Security and accessibility notes
autoscroll, and unique callout landmark ids.Exact-head verification state
Fresh repository workflows were dispatched for exact head
c8804b7662b9a9d6bcabc920fcab73330bb89f4f:32761528172;32761528089;32761528225;32761528299;32761528371;32761528361;32761528220;32761528448;32761528344.At the latest refetch these runs were queued, so none is counted as success and no predecessor-head evidence transfers. Addressed review threads remain subject to exact-head verification before merge readiness.
Merge gate
Keep unmerged until one unchanged exact head has every applicable repository and central CI/security/SAST/SBOM/coverage/review gate terminal-success, exact required coverage/docstring evidence, zero valid unresolved current-head findings, a qualifying independent non-author last-push approval, and ordinary branch-protection acceptance.
Queued, pending, skipped-required, cancelled, failed, stale/predecessor/protected-base, status-only/model-only, self/author, or administrative-bypass evidence is not success. Never bypass branch protection, suppress inherited dependency findings, or transfer predecessor evidence.