feat(score): wire score PDF viewer into app (PR 3/3, depends on #581 #583)#584
feat(score): wire score PDF viewer into app (PR 3/3, depends on #581 #583)#584seonghobae wants to merge 5 commits into
Conversation
…1/3)
Add three Tauri commands backing the upcoming score viewer:
- attach_score_pdf(project_id, song_id): OS file dialog (.pdf only),
validates %PDF- magic bytes and a 25MB size cap, copies the file
into the app-owned <app_local_data_dir>/<project_id>/scores/<uuid>.pdf
and returns {scoreId, fileName, fileSizeBytes}.
- read_score_pdf(project_id, score_id): strict lowercase-UUID allowlist,
path rebuilt locally, canonicalize + prefix guard against traversal,
returns file bytes.
- remove_score_pdf(project_id, score_id): same guards, idempotent delete.
Project ids are validated against the exact shape minted by
next_project_id (project-<nanos>-<counter>) before any path join.
Pure helpers are separated from the command wrappers and covered by
unit tests (uuid guard, traversal/symlink rejection, magic-byte and
size checks).
Drive-by fix: import_youtube_url, save_project, and load_project were
registered in the invoke handler but missing from the build.rs app
manifest and capabilities/main.json, so their permissions were never
generated or granted. Added them alongside the new score commands.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RjGVapDZ3k7V7zKYk16P4C
Add the React viewer for the score PDF feature (PR 2/3), independent of the PR 1 Tauri storage commands and composed with them in PR 3: - ScoreViewer renders validated in-memory PDF bytes on a canvas via pdfjs-dist, following the clearfolio contract: LOADING/FAILED/READY state machine with retry, and a validated-resource-only rule (bytes via props, no URL loading). - Rehearsal-friendly controls: large prev/next page targets, page x/y indicator, zoom in/out clamped to 0.5x-4x, and fit-width mode that re-renders on container resize. - The pdf.js worker is bundled locally by Vite via the pdfjs-dist/build/pdf.worker.min.mjs?url import and served same-origin, satisfying the Tauri script-src 'self' CSP (no CDN). - pdfjs-dist is pinned exactly to 6.1.200 (npm audit clean) per the dependency policy; admission evidence is recorded in the PR body. - New scoreViewer* strings added to both en and ko locales. - 12 vitest cases cover the state machine, nav bounds, zoom clamping, resize re-render, and teardown; ScoreViewer.tsx joins the measured coverage include list at 100%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RjGVapDZ3k7V7zKYk16P4C
…rigin/feat/score-pdf-viewer' into feat/score-pdf-wire
Enable the previously-disabled sidebar slot as the 악보/Score view and
compose the PR1 storage commands with the PR2 pdf.js viewer end-to-end.
- App: rename the disabled "Notes" nav slot to a routable Score view
(i18n navScore, ko+en), enabled once a song is loaded; the main pane
switches between the Workspace and Score views.
- ScoreView: per-song attachment list, "악보 추가" -> attach_score_pdf,
select -> read_score_pdf -> bytes -> <ScoreViewer>, remove with confirm
-> remove_score_pdf. Storage actions gate on an active project id.
- scoreStorage: typed Tauri bridge for attach/read/remove following the
analysis bridge's invoke-detection + fail-closed-in-browser pattern.
- Persist scoreAttachments [{id, fileName}] inside the song payload,
mirrored in shared-types (validator + deny-unknown-key allowlist) and
the Rust RehearsalSongPayload (deny_unknown_fields, optional + skipped
when empty) so .bscope save/load round-trips without breaking legacy
files.
Security Notes: no filesystem path crosses the IPC boundary from JS; the
Rust commands own the picker, magic-byte/size/symlink validation, and
UUID-keyed copies inside the app-owned scores root. Only allowlisted ids
are sent for read/remove.
Tests: ScoreView vitest suite (attach/list/select/remove + error/bridge
paths), shared-types + Rust payload round-trip/rejection tests, and App
score-nav tests. Desktop coverage 98.8% lines (gate 90%).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RjGVapDZ3k7V7zKYk16P4C
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RjGVapDZ3k7V7zKYk16P4C
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 head489a116cdfb0d8e9d8c107aedad02a34c5bef668. -
Head SHA:
489a116cdfb0d8e9d8c107aedad02a34c5bef668 -
Workflow run: 28844675930
-
Workflow attempt: 1
Coverage evidence
Coverage Evidence
- Head SHA:
489a116cdfb0d8e9d8c107aedad02a34c5bef668 - Required test evidence: supported repository test suites must pass.
- Required docstring evidence: repository-owned docstring gates must pass when configured; otherwise docstring coverage is advisory.
JavaScript/TypeScript dependencies (npm ci)
added 278 packages, and audited 281 packages in 8s
74 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
- Result: PASS
Repository docstring coverage
> bandscope@0.1.3 check:python-docstrings
> sh -c 'cd services/analysis-engine && uv run ruff check src tests ../../scripts --select D100,D101,D102,D103,D104,D105,D106,D107'
Using CPython 3.12.3 interpreter at: /usr/bin/python3.12
Creating virtual environment at: .venv
Building bandscope-analysis @ file:///home/runner/work/bandscope/bandscope/pr-head/services/analysis-engine
Downloading numpy (15.8MiB)
Downloading pygments (1.2MiB)
Downloading scipy (33.6MiB)
Downloading soundfile (1.3MiB)
Downloading scikit-learn (8.5MiB)
Downloading mypy (13.0MiB)
Downloading yt-dlp (3.0MiB)
Downloading llvmlite (53.7MiB)
Downloading ruff (10.7MiB)
Downloading numba (3.6MiB)
Downloaded soundfile
Downloaded pygments
Built bandscope-analysis @ file:///home/runner/work/bandscope/bandscope/pr-head/services/analysis-engine
Downloaded numba
Downloaded ruff
Downloaded scikit-learn
Downloaded yt-dlp
Downloaded numpy
Downloaded llvmlite
Downloaded scipy
Downloaded mypy
Installed 44 packages in 65ms
All checks passed!
- Result: PASS
JavaScript/TypeScript test coverage
> bandscope@0.1.3 test
> npm run test --workspaces --if-present && sh -c 'cd services/analysis-engine && uv run pytest tests --cov=src/bandscope_analysis --cov-report=term-missing --cov-fail-under=100' --coverage
> @bandscope/desktop@0.1.0 test
> node -e "require('node:fs').mkdirSync('coverage/.tmp', { recursive: true })" && vitest run --coverage
�[1m�[30m�[46m RUN �[49m�[39m�[22m �[36mv4.1.9 �[39m�[90m/home/runner/work/bandscope/bandscope/pr-head/apps/desktop�[39m
�[2mCoverage enabled with �[22m�[33mv8�[39m
�[32m✓�[39m src/features/score/ScoreViewer.test.tsx �[2m(�[22m�[2m12 tests�[22m�[2m)�[22m�[33m 819�[2mms�[22m�[39m
�[32m✓�[39m src/features/score/ScoreView.test.tsx �[2m(�[22m�[2m15 tests�[22m�[2m)�[22m�[33m 1132�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m renders the empty attachment list with an enabled attach button �[33m 348�[2mms�[22m�[39m
�[32m✓�[39m src/lib/export.test.ts �[2m(�[22m�[2m17 tests�[22m�[2m)�[22m�[32m 18�[2mms�[22m�[39m
�[32m✓�[39m src/lib/analysis.test.ts �[2m(�[22m�[2m14 tests�[22m�[2m)�[22m�[32m 22�[2mms�[22m�[39m
�[32m✓�[39m src/features/workspace/Workspace.test.tsx �[2m(�[22m�[2m11 tests�[22m�[2m)�[22m�[33m 1928�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m enables bass transcription from selected role metadata rather than role id text �[33m 444�[2mms�[22m�[39m
�[32m✓�[39m src/i18n/index.test.ts �[2m(�[22m�[2m9 tests�[22m�[2m)�[22m�[32m 8�[2mms�[22m�[39m
�[32m✓�[39m src/components/ui/ui-primitives.test.tsx �[2m(�[22m�[2m7 tests�[22m�[2m)�[22m�[32m 239�[2mms�[22m�[39m
�[32m✓�[39m src/features/workspace/RoleSwitcher.test.tsx �[2m(�[22m�[2m4 tests�[22m�[2m)�[22m�[33m 397�[2mms�[22m�[39m
�[32m✓�[39m src/features/workspace/SectionRoadmap.test.tsx �[2m(�[22m�[2m3 tests�[22m�[2m)�[22m�[33m 429�[2mms�[22m�[39m
�[32m✓�[39m src/App.test.tsx �[2m(�[22m�[2m58 tests�[22m�[2m)�[22m�[33m 16377�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m renders the rehearsal cockpit shell before analysis starts �[33m 814�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m renders the loaded song as a dark rehearsal command board �[33m 485�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m renders a rehearsal song structure timeline from real section ranges �[33m 466�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m does not show unavailable analysis metrics as detected facts �[33m 487�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m summarizes confidence from the lowest-confidence loaded section �[33m 329�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m selects a local audio source and starts a local-audio analysis job �[33m 372�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m starts an analysis job and renders the returned rehearsal result �[33m 843�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m shows the engine stage label and accessible progress value while analysis runs �[33m 365�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m animates rendered progress toward the running job target �[33m 443�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m uses translated progress labels when status payloads omit a progress label �[33m 613�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m falls back to failed progress copy when a pushed status has no error details �[33m 427�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m holds a terminal progress value immediately for pushed failed statuses �[33m 386�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m cleans up a late status subscription when the running view unmounts first �[33m 315�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m marks the active job failed when polling returns a malformed status �[33m 733�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m ignores malformed poll results after a pushed update changes the active job �[33m 667�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m ignores transport poll failures after a pushed update changes the active job �[33m 534�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m keeps handoff metadata tied to the source that produced the current result �[33m 750�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m shows a safe failed status when the job poll returns an error �[33m 484�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m falls back to a generic failure message when the engine omits details �[33m 475�[2mms�[22m�[39m
�[33m�[2m✓�[22m�[39m keeps polling the active job when one polling request rejects �[33m 759�[2mms�[22m�[39m
�[2m Test Files �[22m �[1m�[32m10 passed�[39m�[22m�[90m (10)�[39m
�[2m Tests �[22m �[1m�[32m150 passed�[39m�[22m�[90m (150)�[39m
�[2m Start at �[22m 12:17:39
�[2m Duration �[22m 19.39s�[2m (transform 1.10s, setup 1.59s, import 3.80s, tests 21.37s, environment 10.84s)�[22m
�[34m % �[39m�[2mCoverage report from �[22m�[33mv8�[39m
-------------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
-------------------|---------|----------|---------|---------|-------------------
All files | 98.88 | 96.71 | 99 | 98.84 |
src | 99.53 | 99.48 | 97.67 | 99.52 |
App.tsx | 99.53 | 99.48 | 97.67 | 99.52 | 604
...features/score | 97.94 | 90.82 | 100 | 97.82 |
ScoreView.tsx | 95.38 | 79.54 | 100 | 95.23 | 53,81,100
ScoreViewer.tsx | 100 | 100 | 100 | 100 |
scoreStorage.ts | 96.87 | 96.29 | 100 | 96.77 | 27
src/i18n | 100 | 100 | 100 | 100 |
index.ts | 100 | 100 | 100 | 100 |
src/lib | 100 | 100 | 100 | 100 |
export.ts | 100 | 100 | 100 | 100 |
-------------------|---------|----------|---------|---------|-------------------
> @bandscope/shared-types@0.1.0 test
> vitest run --coverage
�[1m�[30m�[46m RUN �[49m�[39m�[22m �[36mv4.1.9 �[39m�[90m/home/runner/work/bandscope/bandscope/pr-head/packages/shared-types�[39m
�[2mCoverage enabled with �[22m�[33mv8�[39m
�[32m✓�[39m test/index.test.ts �[2m(�[22m�[2m15 tests�[22m�[2m)�[22m�[32m 84�[2mms�[22m�[39m
�[2m Test Files �[22m �[1m�[32m1 passed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[1m�[32m15 passed�[39m�[22m�[90m (15)�[39m
�[2m Start at �[22m 12:17:59
�[2m Duration �[22m 496ms�[2m (transform 128ms, setup 0ms, import 162ms, tests 84ms, environment 0ms)�[22m
�[34m % �[39m�[2mCoverage report from �[22m�[33mv8�[39m
----------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files | 100 | 100 | 100 | 100 |
index.ts | 100 | 100 | 100 | 100 |
----------|---------|----------|---------|---------|-------------------
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-9.0.3, pluggy-1.6.0
rootdir: /home/runner/work/bandscope/bandscope/pr-head/services/analysis-engine
configfile: pyproject.toml
plugins: cov-7.0.0
collected 433 items
tests/test_activity.py ........ [ 1%]
tests/test_anchors.py .... [ 2%]
tests/test_api.py ......................... [ 8%]
tests/test_chord_recognizer.py .................... [ 13%]
tests/test_chords.py ......................... [ 18%]
tests/test_cli.py ................. [ 22%]
tests/test_extractor.py ...... [ 24%]
tests/test_health.py . [ 24%]
tests/test_pipeline_integration.py ......... [ 26%]
tests/test_pitch_tracker.py ............... [ 30%]
tests/test_priority.py ........... [ 32%]
tests/test_ranges.py ................... [ 36%]
tests/test_release_asset_selection.py ........ [ 38%]
tests/test_release_metadata.py ....... [ 40%]
tests/test_release_packaging.py ......... [ 42%]
tests/test_roles.py ....... [ 44%]
tests/test_roles_ml.py ... [ 44%]
tests/test_segmenter.py ..................... [ 49%]
tests/test_separation.py .................................. [ 57%]
tests/test_supply_chain_policy.py ...................................... [ 66%]
........................................................................ [ 82%]
......................................... [ 92%]
tests/test_temporal.py ......... [ 94%]
tests/test_transcription.py ... [ 95%]
tests/test_tuning.py ..... [ 96%]
tests/test_youtube.py ................ [100%]
=============================== warnings summary ===============================
tests/test_pipeline_integration.py::test_pipeline_without_detected_sections_falls_back
tests/test_roles.py::test_role_extractor_falls_back_when_activity_detection_fails
/home/runner/work/bandscope/bandscope/pr-head/services/analysis-engine/.venv/lib/python3.12/site-packages/librosa/core/pitch.py:103: UserWarning: Trying to estimate tuning from empty frequency set.
return pitch_tuning(
tests/test_roles.py::test_role_extractor_falls_back_when_activity_detection_fails
/home/runner/work/bandscope/bandscope/pr-head/services/analysis-engine/.venv/lib/python3.12/site-packages/librosa/core/spectrum.py:266: UserWarning: n_fft=2048 is too large for input signal of length=100
warnings.warn(
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.12.3-final-0 ________________
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------
src/bandscope_analysis/__init__.py 3 0 100%
src/bandscope_analysis/api.py 571 0 100%
src/bandscope_analysis/chords/__init__.py 5 0 100%
src/bandscope_analysis/chords/analyzer.py 116 0 100%
src/bandscope_analysis/chords/capo.py 10 0 100%
src/bandscope_analysis/chords/chord_recognizer.py 192 0 100%
src/bandscope_analysis/chords/model.py 15 0 100%
src/bandscope_analysis/cli.py 68 0 100%
src/bandscope_analysis/health.py 7 0 100%
src/bandscope_analysis/ranges/__init__.py 4 0 100%
src/bandscope_analysis/ranges/analyzer.py 77 0 100%
src/bandscope_analysis/ranges/model.py 19 0 100%
src/bandscope_analysis/ranges/pitch_tracker.py 54 0 100%
src/bandscope_analysis/roles/__init__.py 4 0 100%
src/bandscope_analysis/roles/activity.py 59 0 100%
src/bandscope_analysis/roles/extractor.py 118 0 100%
src/bandscope_analysis/roles/model.py 58 0 100%
src/bandscope_analysis/roles/priority.py 13 0 100%
src/bandscope_analysis/roles/tuning.py 11 0 100%
src/bandscope_analysis/sections/__init__.py 6 0 100%
src/bandscope_analysis/sections/anchors.py 5 0 100%
src/bandscope_analysis/sections/extractor.py 38 0 100%
src/bandscope_analysis/sections/model.py 35 0 100%
src/bandscope_analysis/sections/segmenter.py 140 0 100%
src/bandscope_analysis/sections/utils.py 8 0 100%
src/bandscope_analysis/separation/__init__.py 4 0 100%
src/bandscope_analysis/separation/audio_separator.py 145 0 100%
src/bandscope_analysis/separation/model.py 31 0 100%
src/bandscope_analysis/separation/separator.py 34 0 100%
src/bandscope_analysis/temporal/__init__.py 3 0 100%
src/bandscope_analysis/temporal/analyzer.py 49 0 100%
src/bandscope_analysis/temporal/model.py 9 0 100%
src/bandscope_analysis/transcription/__init__.py 2 0 100%
src/bandscope_analysis/transcription/api.py 11 0 100%
src/bandscope_analysis/youtube.py 81 0 100%
------------------------------------------------------------------------------------
TOTAL 2005 0 100%
Required test coverage of 100% reached. Total coverage: 100.00%
================== 433 passed, 3 warnings in 90.59s (0:01:30) ==================
- Result: PASS
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (30 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (30 files)"]
R1 --> V1["required checks"]
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
JavaScript/TypeScript dependencies (npm ci)
Repository docstring coverage
JavaScript/TypeScript test coverage
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (30 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (30 files)"]
R1 --> V1["required checks"]
|
What
PR 3/3 of the 악보 (Score PDF) feature. Composes the storage backend (#581,
feat/score-pdf-storage) and the pdf.js viewer (#583,feat/score-pdf-viewer) into a working, navigable Score view. Branched fromdevelopwith both PR branches merged in, so this depends on #581 + #583 landing first — GitHub will shrink the merge base automatically once they merge.Flows (end-to-end)
navScore, ko + en), enabled once a song is loaded. The main pane toggles between the Workspace and Score views; both the desktop sidebar and the compact mobile nav route to it.attach_score_pdf(projectId, songId)(native picker, validated copy) → metadata appended to the song'sscoreAttachmentsand the new PDF opens immediately.read_score_pdf(projectId, scoreId)→ bytes →<ScoreViewer data={bytes}>(PR2 pdf.js viewer with paging/zoom/fit-width).window.confirm→remove_score_pdf(projectId, scoreId), drops metadata and resets the viewer if the open score was removed..bscopefile (no live workspace) show a notice instead of failing.scoreAttachments: [{ id, fileName }]persists inside the song payload, mirrored in shared-types and the RustRehearsalSongPayload(bothdeny_unknown_fields; optional + skipped when empty) so.bscopesave/load round-trips and legacy files without the field still load.Per-section page bookmarks (work item 4) are deferred as a follow-up to keep this PR focused.
Security Notes
deny_unknown_fieldson both sides, so an unexpected key still fails closed onload_project.Tests / gates
ScoreView.test.tsx: attach / list / select / remove flows plus error and bridge-unavailable paths (mocks@tauri-apps/apiinvoke +ScoreViewer).scoreAttachments.lint,typecheck, desktoptest(150 passed) with coverage 98.8% lines / 96.7% branches (gate 90%), shared-types 100%,build; Rustcargo fmt --check,clippy -D warnings,cargo test(15 passed incl. 3 new).App.test.tsxanalysis-flow tests arewaitFor-timeout flaky under heavy machine load; they pass on an isolated single-fork run. All new tests are green.🤖 Generated with Claude Code