Skip to content

feat(player): admit and bind playable stem artifacts - #1160

Draft
seonghobae wants to merge 155 commits into
feat/playable-stem-audition-961from
feat/playable-stem-native-contract-961
Draft

feat(player): admit and bind playable stem artifacts#1160
seonghobae wants to merge 155 commits into
feat/playable-stem-audition-961from
feat/playable-stem-native-contract-961

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Stack role

Draft native-authority/UI child of #1159 on canonical #971 Active Player lineage:

#971 Active Player transport + revocable playback authority
→ #1159 real PCM16 stem publication + path-free process reference
→ #1160 strict native admission/binding + mounted source selector + admitted media switch/revocation + selector EN/KO copy/loading feedback
→ #970/#962 durable Project Persistence prerequisite + selected-source persistence/reload
→ #965 wider locale/a11y + rights-cleared desktop acceptance

It advances #961 without creating a second transport, playback-authority store, persistence owner, or global i18n owner. MIR quality remains #770/#828, Resource Admission & Decode remains #781/#866, canonical crash-safe/versioned project persistence is #970/#962, and broader shipped localization/accessibility convergence remains #965.

Exact current identity

Active Player boundary

run_analysis_engine admits strict status and strips native path/hash/artifact-set data from renderer status. A path-free four-stem reference is admitted only for canonical vocals, bass, drums, other PCM16 metadata. Native preflight derives only app-owned fixed stem locations and rejects containment/symlink/reparse/member/regular-file/byte/header/SHA-256/native-identity failures. #971's file-identity primitive and PlaybackAuthority remain the only native playback authority.

get_playback_source_availability, derivePlaybackSourceOptions, discoverPlaybackSourceOptions, and PlaybackSourceSession enforce same-project/all-or-none membership, canonical ordering, stale-request rejection, hostile-payload fail-closed behavior, exact issued-request completion, immutable snapshots and current-option-only selection.

The public RehearsalPlayer mounts the five-source set only when native availability is complete. Project rotation removes stale options synchronously; per-mount useId() isolates radio groups. A selected-stem media error immediately falls back to the existing full-mix authority and starts a new monotonic native discovery receipt.

Same-project source replacement uses PlaybackSourceSwitchSession: issue the exact receipt before audio.src mutation, pause and clear admitted duration, load(), admit only exact active session/plan + target + duration/loop/seek, restore seek/rate, resume only captured looping state, then consume with complete/abort. Plans/sessions are immutable and single-use; malformed loop timing and paused count-in fail closed; duplicate metadata is inert after settlement; old-resource play() Promise outcomes are retired before replacement.

Discovery waiting-state accessibility

Mounted review found that the selector correctly removes stale stem choices while get_playback_source_availability is pending but previously gave no visible or screen-reader explanation. RED c55e15d1884fcb8cfa750ed3434b498917858bbb holds discovery unresolved and requires a status with no premature stem group. Fix 1e2eb0b1654a368286e05b242680d14b0b147020 derives loading only from the current project's exact pendingRequest and adds EN/KO Checking playback sources… / 재생 소스를 확인하는 중… copy.

WAI-ARIA status is the non-interruptive advisory live-region semantic. Compatibility RED ff7b418633cdf053d6a9c46d00282590bb7876ec requires explicit aria-atomic="true"; fix b9592814a24969ff65176e45b545e660429323c2 adds it without focus movement or an assertive alert.

Exact-head CodeRabbit review then found the new test itself conflated this discovery status with RehearsalPlayerCore's separate persistent role=status. Repair 1fafe1fb391c8d31fa55538b3b8243804cbb0276 now targets only the discovery live region by exact loading text plus status role and, after settlement, asserts only that specific region is gone. Re-review of 6fa1bb4… found no new actionable source finding in that waiting-state slice.

A separate valid CodeRabbit finding showed the Korean selector regression overwrote process-global navigator.language without restoring it. Repair 6d414bdab6117c3fb2018112427fea483d79d169 captures the original own property descriptor and restores it in afterEach, deleting the temporary own property when no original existed. This is test isolation only; production locale or playback authority is unchanged.

docs/traceability/mounted-playback-source-selector.md records the waiting-state problem, RED/fix, review repair and WAI-ARIA 1.2 / WCAG 2.2 SC 4.1.3 / ARIA22 references. The loading text is presentation only: it never becomes playback authority, does not prolong a receipt, and disappears when the exact discovery settles.

Project Persistence prerequisite

The earlier #1160 compatibility repair (0926ab899… → 38b1328b…) proved the product shared RehearsalSong contains top-level collaboration and role-level harmonicExplanation, transpositionPlan, transcription, and practiceProgress, while strict native persistence had omitted them. That delta remains compatibility evidence here, but durable ownership is #970/#962.

Canonical #970 is now at 0185267ab819dd4b9ac1352f5fce1df8e2a7a782, ordinary-descended from protected develop@314ddeae… with behind_by=0. It has repaired the earlier structural schema mismatch, collaboration/progress domains, omission-vs-explicit-null semantics, and the previously stringly section/role/confidence/provenance/cue/export/manual-override domains while retaining deny_unknown_fields, finite-positive tempo validation, and projectFormatVersion: 1. Positive-domain tests cover every currently valid token, including kebab-case pre-chorus, cue-sheet, and chart-summary. A current review finding that project-format TRACEABILITY lacked the root AGENTS.md Security Notes contract was repaired by RED d7886876… and fix/current head 0185267…; the thread is resolved. #970 remains Draft because its exact-head hosted workflows are still queued/non-terminal and no qualifying current-head last-push approval is established.

Selected source must persist through #970/#962 as a stable semantic (full_mix | vocals | bass | drums | other, or the canonical equivalent), never as a revocable bandscope-playback URL/authority. Reload must re-resolve the semantic through current native availability and fail closed to Full mix if unavailable. #1160 must not create a second project store while that owner work remains unmerged.

Localization boundary

The selector no longer owns component-local English. EN/KO visible/accessibility copy, including discovery waiting state, lives under the existing locale tree and uses canonical Locale / detectPreferredLocale(); translated copy is never playback authority. Native fieldset/legend/labeled-radio semantics remain. JA/ZH/VI/ES/DE/FR, CJK/text expansion/font fallback, DB-backed/versioned translation-ledger lifecycle and shipped accessibility evidence remain #965/product-level work.

Current buyer gap

Source selection, same-project continuity, project/generation reset, stale previous-resource Promise retirement, error-driven stem revocation/fallback, selector EN/KO copy, and a programmatically determinable discovery waiting state are implemented at source level. Remaining buyer-facing work is durable selected-source persistence/reload through #970/#962; distinct discovery failure/empty-state copy only if it gives an actionable distinction beyond safe full-mix fallback; broader localization/accessibility; current-head browser/screen-reader evidence; and rights-cleared audible Windows/macOS acceptance.

Verification / merge gate

Keep Draft. Source/test/TRACEABILITY evidence is not repository GREEN. Current-head hosted check-runs must be materialized and terminal-success, and the unchanged head needs all protected repository/central CI, native Windows/macOS build, dependency/SBOM/security, coverage, qualifying independent last-push approval and zero unresolved actionable threads. Absent, queued, skipped, cancelled, stale, predecessor, author/self, model-only or administrative-bypass evidence is non-passing.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

@seonghobae seonghobae added area: api API, protocol, event, or external contract area: security Security boundary, hardening, or vulnerability prevention priority: medium Normal-priority or P2 work scope: product-gap Customer-visible product gap type: feature New or expanded product capability labels Sep 4, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review the exact current Draft head with emphasis on Rust compilation, custom Deserialize behavior, numeric overflow/non-finite guards, canonical WAV-size assumptions, path-free authority, and whether the tests exercise the intended branches. Draft status means incomplete integration, not permission to skip substantive source findings.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== revision =='
git rev-parse HEAD
git status --short
git show -s --format='%H%n%P%n%s' HEAD

printf '%s\n' '== candidate files =='
fd -t f -e rs . apps/desktop/core | sort
fd -t f 'Cargo\.toml' apps/desktop/core | sort

printf '%s\n' '== contract outline =='
ast-grep outline apps/desktop/core/src/playable_stem_contract.rs --items all --view expanded || true

printf '%s\n' '== module and symbol references =='
rg -n -C 3 --type rust \
  '(playable_stem_contract|PlaybackStemKind|PlayableStemArtifactSet|PlayableStemArtifact|deserialize|Deserialize)' \
  apps/desktop/core

printf '%s\n' '== Cargo configuration =='
cat -n apps/desktop/core/Cargo.toml

printf '%s\n' '== contract source =='
cat -n apps/desktop/core/src/playable_stem_contract.rs

printf '%s\n' '== test sources mentioning contract =='
rg -n -C 5 --type rust \
  '(playable_stem_contract|PlaybackStemKind|PlayableStemArtifactSet|PlayableStemArtifact)' \
  apps/desktop/core

Length of output: 72208


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== complete contract integration test =='
cat -n apps/desktop/core/tests/playable_stem_artifact_reference.rs

printf '%s\n' '== exact-head diff =='
git diff --no-ext-diff --unified=80 \
  c5cfac357240f7126a33bf84b4d1798cee9eb0c2..2f4892008cfd13681feb4dc658f6c073312f3079 \
  -- apps/desktop/core/src/playable_stem_contract.rs \
     apps/desktop/core/tests/playable_stem_artifact_reference.rs \
     apps/desktop/core/src/lib.rs \
     apps/desktop/core/Cargo.toml

printf '%s\n' '== PR cumulative changed paths =='
git diff --name-status 5da57dfedf699fc158f98588d9e54306e992f8ea..HEAD
git diff --stat 5da57dfedf699fc158f98588d9e54306e992f8ea..HEAD

printf '%s\n' '== export, workspace, toolchain, and lock context =='
rg -n -C 4 --glob '!*target*' \
  '(\bmod\s+playable_stem_contract\b|\bpub\s+mod\s+playable_stem_contract\b|bandscope-desktop-core|rust-toolchain|rust-version|name = "serde")' \
  Cargo.toml Cargo.lock apps README.md .github 2>/dev/null || true
fd -H -t f -a 'rust-toolchain(\.toml)?|Cargo\.lock|Cargo\.toml' . | sort

printf '%s\n' '== WAV/RIFF contract references =='
rg -n -i -C 4 \
  '(canonical.{0,40}(wav|wave)|riff|wave header|pcm16|fileSizeBytes|sampleCount)' \
  . -g '!**/node_modules/**' -g '!**/target/**'

Length of output: 132793


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Python artifact writer and validation =='
sed -n '100,210p' services/analysis-engine/src/bandscope_analysis/separation/playback_artifacts.py
sed -n '220,350p' services/analysis-engine/src/bandscope_analysis/separation/playback_artifacts.py
sed -n '350,440p' services/analysis-engine/src/bandscope_analysis/separation/playback_artifacts.py

printf '%s\n' '== ADR format and native-contract sections =='
rg -n -i -C 8 \
  '(riff|wav|pcm16|44|u32|file size|sample count|path-free|native contract|reference)' \
  docs/adr/0001-playable-stem-delivery-and-audition.md \
  docs/superpowers/plans/2026-09-04-playable-stem-audition.md

printf '%s\n' '== classic RIFF/WAV boundary calculation =='
python3 - <<'PY'
U32_MAX = 2**32 - 1
HEADER = 44
BYTES_PER_MONO_PCM16_SAMPLE = 2
# RIFF chunk size stores total file bytes after the first 8 bytes.
max_file_bytes = U32_MAX + 8
max_data_bytes = max_file_bytes - HEADER
max_samples = max_data_bytes // BYTES_PER_MONO_PCM16_SAMPLE
print(f"max_file_bytes={max_file_bytes}")
print(f"max_data_bytes={max_data_bytes}")
print(f"max_samples={max_samples}")
print(f"max_canonical_file_size={HEADER + BYTES_PER_MONO_PCM16_SAMPLE * max_samples}")
print(f"first_invalid_samples={max_samples + 1}")
print(f"first_invalid_canonical_file_size={HEADER + BYTES_PER_MONO_PCM16_SAMPLE * (max_samples + 1)}")
PY

printf '%s\n' '== Rust module inclusion and current compile targets =='
sed -n '1,80p' apps/desktop/core/src/lib.rs
find apps/desktop/core -maxdepth 3 -type f \( -name '*.rs' -o -name 'Cargo.toml' \) -print | sort

Length of output: 41541


@seonghobae I reviewed exact head 2f4892008cfd13681feb4dc658f6c073312f3079.

Finding: canonical WAV size limit is incomplete.
apps/desktop/core/src/playable_stem_contract.rs, Lines 268-272, reject only u64 arithmetic overflow. A classic 44-byte RIFF/WAV file stores its RIFF and data chunk sizes as u32. The largest valid mono PCM16 sample count is 2_147_483_629, with a file size of 4_294_967_302 bytes.

The contract accepts sampleCount = 2_147_483_630 when all metadata matches. It derives fileSizeBytes = 4_294_967_304. That value cannot be represented by a classic RIFF/WAV header. The Python wave publisher cannot produce this artifact, but an untrusted subprocess reference can claim it.

Add an explicit classic RIFF/WAV maximum before accepting the reference. Add boundary tests for the maximum accepted count and the next count rejected. The existing u64::MAX test covers arithmetic overflow only. It does not cover this format-limit branch.

The custom Deserialize design otherwise fails closed for unknown set and artifact fields. The raw structs use deny_unknown_fields. The path-free derived location uses only the native root, a validated lowercase SHA-256 identifier, and fixed filenames. The tests exercise the main intended rejection paths, including ordering, identifiers, alignment, and the four fixed filenames.

Rust format, build, test, Clippy, and coverage evidence remains unavailable. Source inspection does not establish those gates.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

Verified and repaired the RIFF format-limit finding rather than treating u64 safety as media-format safety.

TDD/source lineage:

  • RED-oriented e362c43768b5d5f4b337270562458ee3249e484e imports MAX_CLASSIC_RIFF_PCM16_SAMPLE_COUNT, requires sample count 2,147,483,629 / file size 4,294,967,302 to remain admissible, and requires 2,147,483,630 to fail.
  • GREEN df2a305b3b12b4d02c3ee0b670186218b626b50a derives the maximum from the classic RIFF 32-bit chunk-size field and the fixed canonical header: (u32::MAX - (44 - 8)) / 2. The custom deserializer now rejects larger sample counts before duration or per-artifact admission. Once bounded, canonical size calculation no longer needs an unreachable u64 overflow branch.

The prior u64::MAX arithmetic test was replaced by the actual format boundary. No build/test success is claimed until Rust 1.97.1 execution becomes available. Current exact head is df2a305b3b12b4d02c3ee0b670186218b626b50a; predecessor inspection does not transfer.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Please execute the exact current head if your review environment permits it: cargo +1.97.1 fmt --manifest-path apps/desktop/core/Cargo.toml --check and cargo +1.97.1 test --manifest-path apps/desktop/core/Cargo.toml --locked --test playable_stem_artifact_reference. Report the complete first compiler/test failure rather than inferring success from source inspection. This is diagnostic evidence only, not a substitute for protected hosted gates.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -uo pipefail

expected_head='df2a305b3b12b4d02c3ee0b670186218b626b50a'
actual_head="$(git rev-parse HEAD)"

printf '%s\n' '== exact head =='
printf 'expected: %s\nactual:   %s\n' "$expected_head" "$actual_head"
if [ "$actual_head" != "$expected_head" ]; then
  printf '%s\n' 'HEAD_MISMATCH: Requested commands were not run.'
  exit 2
fi

run_command() {
  local name="$1"
  shift
  local output status

  printf '\n== %s ==\n' "$name"
  output="$(mktemp)"
  "$@" >"$output" 2>&1
  status=$?
  cat "$output"
  rm -f "$output"
  printf '\n== %s exit status: %s ==\n' "$name" "$status"
  return "$status"
}

run_command 'cargo fmt check' \
  cargo +1.97.1 fmt --manifest-path apps/desktop/core/Cargo.toml --check
fmt_status=$?

run_command 'targeted contract test' \
  cargo +1.97.1 test --manifest-path apps/desktop/core/Cargo.toml --locked \
    --test playable_stem_artifact_reference
test_status=$?

printf '\n== summary ==\n'
printf 'cargo fmt check: %s\n' "$fmt_status"
printf 'targeted contract test: %s\n' "$test_status"

if [ "$fmt_status" -ne 0 ]; then
  exit "$fmt_status"
fi
exit "$test_status"

Length of output: 6580


@seonghobae Exact HEAD matched df2a305b3b12b4d02c3ee0b670186218b626b50a.

The first nonzero result was the format command. It exited with status 101 before formatting checks ran:

error: no such command: `fmt`

help: a command with a similar name exists: `fix`

help: view all installed commands with `cargo --list`
help: find a package to install `fmt` with `cargo search cargo-fmt`

This is a Rust toolchain-component failure. It is not a source-format failure. The environment installed Rust 1.97.1, but it did not install rustfmt.

The targeted test command completed successfully:

Finished `test` profile [unoptimized + debuginfo] target(s) in 37.58s

running 10 tests
...
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

This run compiled bandscope-desktop-core and found no compiler or test failure in playable_stem_artifact_reference. The format gate remains unverified until the Rust 1.97.1 rustfmt component is available. This diagnostic result does not replace the protected hosted gates.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai Please run one exact-head integration diagnostic on ace6947c6efd27fd4939c77654a9c2eda4d86cb3 without committing. First insert only these two declarations immediately before the first use serde in apps/desktop/core/src/lib.rs:

pub mod analysis_process_status;
pub mod playable_stem_contract;

Then execute:

cargo +1.97.1 test --manifest-path apps/desktop/core/Cargo.toml --locked --test playable_stem_artifact_reference --test analysis_process_status
cargo +1.97.1 test --manifest-path apps/desktop/core/Cargo.toml --locked --lib
cargo +1.97.1 clippy --manifest-path apps/desktop/core/Cargo.toml --all-targets --locked -- -D warnings

If rustfmt is absent, attempt rustup component add rustfmt --toolchain 1.97.1 and then run cargo +1.97.1 fmt --manifest-path apps/desktop/core/Cargo.toml --check. Report exact command outputs and the first source failure. The temporary edit is diagnostic only; no success should be inferred from a modified working tree unless every listed source gate passes.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 9 minutes and 57 seconds before sending another message.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review exact current Draft head f22d1a17a2236603dbae6b1f24985dc93fc3f4d8, especially the new playback-source localization slice: canonical Locale ownership, JSON resource typing, Korean accessible names, preservation of opaque authority values, native fieldset/legend/radio semantics, and whether the new mounted regression or copy adapter creates any coverage/type/test-isolation gap. Draft status means the wider product vertical is incomplete; substantive findings are still useful.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='f22d1a17a2236603dbae6b1f24985dc93fc3f4d8'
actual_head="$(git rev-parse HEAD)"
printf 'expected HEAD: %s\nactual HEAD:   %s\n' "$expected_head" "$actual_head"
if [ "$actual_head" != "$expected_head" ]; then
  printf '%s\n' 'HEAD_MISMATCH: stop review; the requested revision is not checked out.'
  exit 2
fi

printf '\n== head ==\n'
git show -s --format='%H%n%P%n%s' HEAD

printf '\n== localization-related changed paths (parent..HEAD) ==\n'
git diff --name-status HEAD^..HEAD
git diff --name-status c27f3781ddcbcc013dce07a26c0baf6080e4b2ac..HEAD | \
  rg -i '(locale|localization|playback.*source|rehearsal.*player|discover|session|test)' || true

printf '\n== candidate file discovery ==\n'
fd -t f . apps/desktop/src | rg -i '(locale|playback.*source|rehearsal.*player|discover|session|test)' | sort

printf '\n== source structure ==\n'
ast-grep outline apps/desktop/src --items all --type function,class,interface,type --match 'Locale|PlaybackSource|RehearsalPlayer|discoverPlaybackSourceOptions|detectPreferredLocale' || true

printf '\n== localization and selector references ==\n'
rg -n -C 5 --glob '*.{ts,tsx,json}' \
  '(detectPreferredLocale|type Locale|interface Locale|PlaybackSource|playbackSource|fieldset|legend|radio|accessible|aria-|sourceOptions)' \
  apps/desktop/src

printf '\n== exact localization-focused diff ==\n'
git diff --no-ext-diff --unified=100 HEAD^..HEAD -- apps/desktop/src

printf '\n== resource files ==\n'
fd -t f -e json . apps/desktop/src/locales | sort | while IFS= read -r f; do
  echo "--- $f"
  cat -n "$f"
done

Length of output: 198548


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='f22d1a17a2236603dbae6b1f24985dc93fc3f4d8'
test "$(git rev-parse HEAD)" = "$expected_head"

printf '%s\n' '== exact last-commit changed paths and diff =='
git diff-tree --no-commit-id --name-status -r HEAD
git show --no-ext-diff --format=fuller --find-renames --find-copies --unified=120 HEAD -- \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/locales/en/playback-source.json \
  apps/desktop/src/locales/ko/playback-source.json

printf '%s\n' '== player wrapper outline and source-selection-specific references =='
ast-grep outline apps/desktop/src/features/workspace/RehearsalPlayer.tsx --items all --view expanded
rg -n -C 12 --glob '*.{ts,tsx}' \
  '(createPlaybackSourceCopy|playbackSourceCopy|fieldset|legend|type="radio"|name=|value=|PlaybackSourceOption|selectedAuthority)' \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayerCore.tsx

printf '%s\n' '== localization adapter, test, and i18n files =='
for f in \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/i18n/index.ts \
  apps/desktop/src/i18n.ts \
  apps/desktop/tsconfig.json \
  apps/desktop/vitest.config.ts \
  apps/desktop/src/test/setup.ts \
  apps/desktop/src/setupTests.ts
do
  if [ -f "$f" ]; then
    echo "--- $f"
    cat -n "$f"
  fi
done

printf '%s\n' '== locale definition locations =='
rg -n -C 8 --glob '*.{ts,tsx}' \
  '(export type Locale|type Locale|detectPreferredLocale|createTranslator)' \
  apps/desktop/src

printf '%s\n' '== resource key parity =='
python3 - <<'PY'
import json
from pathlib import Path
en = json.loads(Path("apps/desktop/src/locales/en/playback-source.json").read_text())
ko = json.loads(Path("apps/desktop/src/locales/ko/playback-source.json").read_text())
print("only EN:", sorted(set(en) - set(ko)))
print("only KO:", sorted(set(ko) - set(en)))
print("non-string EN:", [k for k,v in en.items() if not isinstance(v, str)])
print("non-string KO:", [k for k,v in ko.items() if not isinstance(v, str)])
PY

printf '%s\n' '== relevant test names and locale mutation cleanup =='
rg -n -C 6 --glob '*.{test,spec}.{ts,tsx}' \
  '(playback source|Playback source|재생 소스|Full mix|전체 믹스|navigator.*language|defineProperty\(window\.navigator)' \
  apps/desktop/src

Length of output: 144043


@seonghobae I reviewed exact head f22d1a17a2236603dbae6b1f24985dc93fc3f4d8.

Finding: the Korean mounted regression does not restore navigator.language.
apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx, Lines 25-35, changes the process-global navigator.language value from en-US to ko-KR. The test file has no afterEach restoration.

A later test can then call detectPreferredLocale() and receive "ko" unexpectedly. This makes test results depend on execution order. Other locale-mutating test files preserve and restore the original descriptor.

Save the original navigator.language property descriptor before the suite. Restore that descriptor in afterEach. This keeps the Korean accessibility-name regression isolated.

The reviewed implementation otherwise uses the canonical Locale type. The JSON resources have matching string keys. The selector preserves opaque authority values in each radio value. It retains native fieldset, legend, label, and radio semantics. The mounted regression verifies the Korean legend and all five Korean accessible names.

I did not run tests or type checks. Source inspection does not establish hosted gate status.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Please review exact Draft head ae2549621efd72973f751ecba47524f652ebb47e, especially the new mounted discovery waiting-state slice: pending receipt → visible/programmatic status, EN/KO resource parity, explicit aria-atomic="true", disappearance only after exact discovery settlement, and whether any stale project/revocation race can surface the status or options for the wrong authority. Treat Draft as incomplete integration, not permission to skip source findings.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='ae2549621efd72973f751ecba47524f652ebb47e'
actual_head="$(git rev-parse HEAD)"
printf 'expected HEAD: %s\nactual HEAD:   %s\n' "$expected_head" "$actual_head"
if [ "$actual_head" != "$expected_head" ]; then
  printf '%s\n' 'HEAD_MISMATCH: stop review; requested revision is not checked out.'
  exit 2
fi

printf '\n== head ==\n'
git show -s --format='%H%n%P%n%s' HEAD

printf '\n== current commit changed paths ==\n'
git diff-tree --no-commit-id --name-status -r HEAD

printf '\n== cumulative candidate paths ==\n'
git diff --name-status c27f3781ddcbcc013dce07a26c0baf6080e4b2ac..HEAD | \
  rg -i '(playback.*source|rehearsal.*player|locale|i18n|discover|session|test|traceability)' || true

printf '\n== candidate file discovery ==\n'
fd -t f . apps/desktop/src docs/traceability | \
  rg -i '(playback.*source|rehearsal.*player|locale|i18n|discover|session)' | sort

printf '\n== focused source outlines ==\n'
for f in \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/playbackSourceSession.ts \
  apps/desktop/src/features/workspace/discoverPlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/derivePlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts
do
  if [ -f "$f" ]; then
    echo "--- $f"
    ast-grep outline "$f" --items all --view expanded || true
  fi
done

printf '\n== waiting-state and identity references ==\n'
rg -n -C 10 --glob '*.{ts,tsx,json}' \
  '(pendingRequest|Checking playback sources|재생 소스를 확인하는 중|aria-atomic|role="status"|role=.status.|get_playback_source_availability|discoverPlaybackSourceOptions|PlaybackSourceSession|projectId|authority|receipt|requestId)' \
  apps/desktop/src

printf '\n== current commit focused diff ==\n'
git diff --no-ext-diff --unified=120 HEAD^..HEAD -- apps/desktop/src docs/traceability

printf '\n== relevant test locations ==\n'
rg -n -C 8 --glob '*.{test,spec}.{ts,tsx}' \
  '(Checking playback sources|재생 소스를 확인하는 중|aria-atomic|pendingRequest|PlaybackSourceSession|discoverPlaybackSourceOptions|source selection)' \
  apps/desktop/src

printf '\n== resource parity and values ==\n'
python3 - <<'PY'
import json
from pathlib import Path
paths = [
    Path("apps/desktop/src/locales/en/playback-source.json"),
    Path("apps/desktop/src/locales/ko/playback-source.json"),
]
for path in paths:
    if path.exists():
        data = json.loads(path.read_text())
        print(f"--- {path}")
        for key, value in data.items():
            print(f"{key}: {value!r}")
if all(p.exists() for p in paths):
    en = json.loads(paths[0].read_text())
    ko = json.loads(paths[1].read_text())
    print("only EN:", sorted(set(en) - set(ko)))
    print("only KO:", sorted(set(ko) - set(en)))
    print("non-string EN:", sorted(k for k,v in en.items() if not isinstance(v, str)))
    print("non-string KO:", sorted(k for k,v in ko.items() if not isinstance(v, str)))
PY

Length of output: 203427


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='ae2549621efd72973f751ecba47524f652ebb47e'
test "$(git rev-parse HEAD)" = "$expected_head"

printf '%s\n' '== wrapper: complete source =='
cat -n apps/desktop/src/features/workspace/RehearsalPlayer.tsx

printf '%s\n' '== session: complete source =='
cat -n apps/desktop/src/features/workspace/playbackSourceSession.ts

printf '%s\n' '== waiting-state mounted test =='
sed -n '1,180p' apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx

printf '%s\n' '== project-rotation and revocation test sections =='
rg -n -A90 -B12 --glob 'RehearsalPlayer.sourceSelection.test.tsx' \
  '(rotation|stale|revocation|error|pending|discovery|status)' \
  apps/desktop/src/features/workspace

printf '%s\n' '== exact waiting-state commits and focused patches =='
git show --no-ext-diff --format=fuller --stat \
  1e2eb0b1654a368286e05b242680d14b0b147020 \
  b9592814a24969ff65176e45b545e660429323c2
git diff --no-ext-diff --unified=100 \
  1e2eb0b1654a368286e05b242680d14b0b147020^..1e2eb0b1654a368286e05b242680d14b0b147020 \
  -- apps/desktop/src
git diff --no-ext-diff --unified=100 \
  b9592814a24969ff65176e45b545e660429323c2^..b9592814a24969ff65176e45b545e660429323c2 \
  -- apps/desktop/src

printf '%s\n' '== copy adapter and locale type boundary =='
cat -n apps/desktop/src/features/workspace/playbackSourceCopy.ts
rg -n -C 6 --glob '*.{ts,tsx}' \
  '(export type Locale|type Locale|detectPreferredLocale|createPlaybackSourceCopy)' \
  apps/desktop/src

Length of output: 101981


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='ae2549621efd72973f751ecba47524f652ebb47e'
test "$(git rev-parse HEAD)" = "$expected_head"

printf '%s\n' '== core status render context =='
sed -n '1080,1150p' apps/desktop/src/features/workspace/RehearsalPlayerCore.tsx

printf '%s\n' '== all mounted source-selection tests after Line 150 =='
sed -n '150,340p' apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx

printf '%s\n' '== all loading-copy use and assertions =='
rg -n -C 8 --glob '*.{ts,tsx,json}' \
  '(playbackSourceCopy\("loading"\)|"loading": "Checking playback sources|"loading": "재생 소스를 확인하는 중|Checking playback sources|재생 소스를 확인하는 중)' \
  apps/desktop/src

printf '%s\n' '== role=status counts in wrapper/core render sources =='
rg -n -C 4 --glob 'RehearsalPlayer*.tsx' 'role="status"' \
  apps/desktop/src/features/workspace

Length of output: 13350


@seonghobae I reviewed exact head ae2549621efd72973f751ecba47524f652ebb47e.

Finding: the new mounted waiting-state regression cannot identify or dismiss the discovery status.

apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx, Line 58, calls screen.findByRole("status"). RehearsalPlayerCore always renders its loop-action status at apps/desktop/src/features/workspace/RehearsalPlayerCore.tsx, Lines 1117-1123.

Before the discovery effect commits, findByRole("status") can return the core status. After the discovery status renders, the query has two matches. The assertion at Line 66 also cannot pass because the core status remains mounted.

Select the new status by its loading text or add a specific test identifier. After settlement, assert that this specific discovery status is absent. Do not assert that all status elements are absent.

The implementation uses the required aria-atomic="true". It derives the waiting state from the mounted authority plus the exact pending receipt. It also prevents stale project state through authority matching, generation invalidation, and exact receipt completion. The EN and KO resources have matching string keys, including loading.

I did not run tests or hosted gates. Source inspection shows that the waiting-state test needs correction.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Exact head advanced to 6fa1bb4cd77442dfb86e41954197a3fa09fe92b6 after accepting the previous finding. 1fafe1fb391c8d31fa55538b3b8243804cbb0276 now selects only the discovery live region by exact loading copy plus role=status and asserts only that specific region disappears; the persistent RehearsalPlayerCore status is no longer conflated with discovery evidence. Please review this unchanged head for the waiting-state slice and any stale project/revocation race.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='6fa1bb4cd77442dfb86e41954197a3fa09fe92b6'
actual_head="$(git rev-parse HEAD)"
printf 'expected HEAD: %s\nactual HEAD:   %s\n' "$expected_head" "$actual_head"
if [ "$actual_head" != "$expected_head" ]; then
  printf '%s\n' 'HEAD_MISMATCH: stop review; requested revision is not checked out.'
  exit 2
fi

printf '\n== exact head ==\n'
git show -s --format='%H%n%P%n%s' HEAD

printf '\n== repaired commit and cumulative changed paths ==\n'
git show --no-ext-diff --format=fuller --stat \
  1fafe1fb391c8d31fa55538b3b8243804cbb0276
git diff --name-status \
  c27f3781ddcbcc013dce07a26c0baf6080e4b2ac..HEAD | \
  rg -i '(RehearsalPlayer|playback.*source|discover|session|locale|traceability|test)' || true

printf '\n== repair diff ==\n'
git diff --no-ext-diff --unified=120 \
  1fafe1fb391c8d31fa55538b3b8243804cbb0276^..1fafe1fb391c8d31fa55538b3b8243804cbb0276 \
  -- apps/desktop/src/features/workspace \
     apps/desktop/src/locales \
     docs/traceability

printf '\n== focused structure ==\n'
for f in \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/features/workspace/playbackSourceSession.ts \
  apps/desktop/src/features/workspace/discoverPlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/derivePlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/features/workspace/RehearsalPlayerCore.tsx
do
  if [ -f "$f" ]; then
    printf '\n--- outline: %s ---\n' "$f"
    ast-grep outline "$f" --items all --view expanded || true
  fi
done

printf '\n== waiting state, receipt, authority, and revocation references ==\n'
rg -n -C 12 --glob '*.{ts,tsx,json}' \
  '(Checking playback sources|재생 소스를 확인하는 중|role="status"|aria-atomic|pendingRequest|requestId|receipt|projectId|authority|revocation|stale|discoverPlaybackSourceOptions|PlaybackSourceSession)' \
  apps/desktop/src/features/workspace apps/desktop/src/locales

printf '\n== relevant current sources ==\n'
for f in \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/features/workspace/playbackSourceSession.ts \
  apps/desktop/src/features/workspace/discoverPlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/derivePlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts
do
  if [ -f "$f" ]; then
    printf '\n--- source: %s ---\n' "$f"
    cat -n "$f"
  fi
done

Length of output: 203131


@seonghobae I reviewed exact head 6fa1bb4cd77442dfb86e41954197a3fa09fe92b6.

I found no new actionable source finding in the waiting-state slice.

apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx now selects only the discovery region. It uses the exact loading text and [role="status"]. It checks aria-atomic="true". It waits for that same region to disappear after discovery settles. The persistent RehearsalPlayerCore status no longer affects this test.

apps/desktop/src/features/workspace/RehearsalPlayer.tsx shows the discovery status only when both conditions are true:

  • sourceSession.fullMixAuthority === audioSourcePath
  • sourceSession.pendingRequest !== null

A project rotation hides old options and the old waiting status immediately. The effect cleanup invalidates the prior generation. A late completion also must pass the generation check. completePlaybackSourceDiscovery() then requires the exact issued request object and matching full-mix authority. A stale completion cannot populate options or settle the pending request for another project.

On selected-stem media error, beginPlaybackSourceDiscovery() synchronously resets the option set to Full mix and resets selection to the full-mix authority. The wrapper starts a new discovery only after that reset. This prevents stale stem choices from remaining selectable during revocation handling.

The mounted tests cover the repaired status query, stale prior-project options, and immediate stem revocation. I did not execute tests or hosted gates.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

Exact head is now 6d414bdab6117c3fb2018112427fea483d79d169. I also accepted the earlier locale-test isolation finding: RehearsalPlayer.sourceSelection.i18n.test.tsx captures the original own navigator.language descriptor and restores it in afterEach (or deletes the temporary own property when none existed), so the Korean regression cannot leak locale into later tests. Please review this unchanged head for remaining actionable findings in the mounted source-selector/loading/i18n slice.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='6d414bdab6117c3fb2018112427fea483d79d169'
actual_head="$(git rev-parse HEAD)"
printf 'expected HEAD: %s\nactual HEAD:   %s\n' "$expected_head" "$actual_head"
if [ "$actual_head" != "$expected_head" ]; then
  printf '%s\n' 'HEAD_MISMATCH: stop review; requested revision is not checked out.'
  exit 2
fi

printf '\n== head and repair commit ==\n'
git show -s --format='%H%n%P%n%s' HEAD
git show --no-ext-diff --format=fuller --stat HEAD
git diff --no-ext-diff --unified=120 HEAD^..HEAD -- \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/locales

printf '\n== focused source structure ==\n'
for f in \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/features/workspace/playbackSourceSession.ts \
  apps/desktop/src/features/workspace/discoverPlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/derivePlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts
do
  if [ -f "$f" ]; then
    printf '\n--- outline: %s ---\n' "$f"
    ast-grep outline "$f" --items all --view expanded || true
  fi
done

printf '\n== full focused tests and wrapper ==\n'
for f in \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx \
  apps/desktop/src/features/workspace/RehearsalPlayer.tsx \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/features/workspace/playbackSourceSession.ts \
  apps/desktop/src/features/workspace/discoverPlaybackSourceOptions.ts \
  apps/desktop/src/features/workspace/derivePlaybackSourceOptions.ts
do
  if [ -f "$f" ]; then
    printf '\n--- source: %s ---\n' "$f"
    cat -n "$f"
  fi
done

printf '\n== navigator locale mutation and test setup ==\n'
rg -n -C 10 --glob '*.{test,spec}.{ts,tsx}' \
  '(navigator\.language|defineProperty\(navigator|defineProperty\(window\.navigator|detectPreferredLocale|afterEach|beforeEach)' \
  apps/desktop/src
for f in apps/desktop/src/test/setup.ts apps/desktop/src/setupTests.ts apps/desktop/vitest.config.ts; do
  if [ -f "$f" ]; then
    printf '\n--- source: %s ---\n' "$f"
    cat -n "$f"
  fi
done

printf '\n== copy resources and authority/status references ==\n'
for f in \
  apps/desktop/src/locales/en/playback-source.json \
  apps/desktop/src/locales/ko/playback-source.json
do
  if [ -f "$f" ]; then
    printf '\n--- source: %s ---\n' "$f"
    cat -n "$f"
  fi
done
rg -n -C 8 --glob '*.{ts,tsx,json}' \
  '(pendingRequest|role="status"|aria-atomic|Checking playback sources|재생 소스를 확인하는 중|fullMixAuthority|selectedAuthority|beginPlaybackSourceDiscovery|completePlaybackSourceDiscovery|value=\{.*authority)' \
  apps/desktop/src/features/workspace apps/desktop/src/locales

Length of output: 203569


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='6d414bdab6117c3fb2018112427fea483d79d169'
test "$(git rev-parse HEAD)" = "$expected_head"

printf '%s\n' '== locale test: complete =='
cat -n apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx

printf '%s\n' '== wrapper callbacks, effects, and render: complete =='
cat -n apps/desktop/src/features/workspace/RehearsalPlayer.tsx

printf '%s\n' '== source-selection test: project rotation through end =='
sed -n '160,310p' apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.test.tsx

printf '%s\n' '== discovery adapter and selection derivation: complete =='
for f in \
  apps/desktop/src/features/workspace/playbackSourceDiscovery.ts \
  apps/desktop/src/features/workspace/playbackSourceSelection.ts \
  apps/desktop/src/features/workspace/playbackSourceCopy.ts \
  apps/desktop/src/i18n.ts
do
  if [ -f "$f" ]; then
    echo "--- $f"
    cat -n "$f"
  fi
done

printf '%s\n' '== selector i18n changes and file-level test isolation =='
git diff --no-ext-diff --unified=100 \
  6fa1bb4cd77442dfb86e41954197a3fa09fe92b6..HEAD -- \
  apps/desktop/src/features/workspace/RehearsalPlayer.sourceSelection.i18n.test.tsx
rg -n -C 5 --glob 'RehearsalPlayer.sourceSelection.i18n.test.tsx' \
  '(beforeEach|afterEach|cleanup|restoreAllMocks|navigator)' \
  apps/desktop/src/features/workspace

Length of output: 31592


@seonghobae I reviewed exact head 6d414bdab6117c3fb2018112427fea483d79d169.

I found no new actionable source finding in the mounted source-selector, discovery-status, and localization slice.

RehearsalPlayer.sourceSelection.i18n.test.tsx now restores the original own navigator.language descriptor in afterEach. When no own descriptor existed, it deletes only the temporary property. This fixes the prior test isolation finding.

The mounted selector still limits visible options and the loading status to the session that matches the mounted full-mix authority. Effect cleanup advances the discovery generation. Late completions cannot update the current session. completePlaybackSourceDiscovery() also requires the exact pending request object and matching authority.

The EN and KO resources have matching keys. The selector keeps native fieldset, legend, label, and radio semantics. It keeps each opaque authority value in the radio value. The discovery test targets only the new loading status and does not conflate it with the persistent core status.

I did not run automated tests or hosted gates. This review does not establish those gates.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: api API, protocol, event, or external contract area: security Security boundary, hardening, or vulnerability prevention priority: medium Normal-priority or P2 work scope: product-gap Customer-visible product gap type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant