Skip to content

feat(ui): re-analyze CTA for legacy Session Musician runs - #29

Merged
slittycode merged 1 commit into
mainfrom
claude/reanalyze-legacy-cta
May 13, 2026
Merged

feat(ui): re-analyze CTA for legacy Session Musician runs#29
slittycode merged 1 commit into
mainfrom
claude/reanalyze-legacy-cta

Conversation

@slittycode

Copy link
Copy Markdown
Owner

Successor to #24 (auto-closed when its base branch claude/thirsty-easley-faeaea was deleted after PR #22 merged). Branch rebased onto main with PR #28's per-stem-confidence changes already present; no conflicts surfaced.

Why

When a cached run shows up with transcriptionMethod: 'basic-pitch' (or any non-torchcrepe method), the Stem note draft block enters its legacy render state and shows the existing notice "Re-analyze for current stem-aware quality. The notes below are kept for archival." But PR #22 left it at just words — this adds the actual button.

Behaviour

  • Block A renders a "Re-analyze with stem-aware pipeline" button only in the legacy render state AND only when App passes a callback (a source File is loaded and no analysis is currently in flight).
  • Clicking the button calls back through <SessionMusicianPanel><AnalysisResults>App.tsx, which fires handleStartAnalysis({ pitchNoteRequested: true }). The File state is retained in App across analyses, so no re-upload is required.
  • The STEM PITCH/NOTE TRANSLATION toggle is also updated to reflect the override so subsequent manual Run Analysis clicks behave consistently.

Investigation: does the backend support artifact reuse?

analysis_runtime.create_run stores the artifact bytes anew on every call — there's no API to reference a prior artifact ID. Because audioFile: File is retained in App state, the cleanest path is to re-POST the same File. No backend changes needed.

Tests

  • 5 new Vitest cases: legacy + callback → button renders, legacy + no callback → no button, stem-aware → no button, full-mix-fallback → no button, ran-with-no-result / requested-but-unavailable → no button.
  • Smoke test stubs transcriptionMethod: 'basic-pitch', clicks the CTA, asserts a second POST to /api/analysis-runs fires with pitch_note_mode=stem_notes.

47 smoke tests pass (1 skipped).

🤖 Generated with Claude Code

When a cached run shows up with `transcriptionMethod: 'basic-pitch'` (or any
non-torchcrepe method), the Stem note draft block enters its `legacy` render
state and shows the existing "Re-analyze for current stem-aware quality"
notice. PR #22 left the notice as just words — this adds the actual button.

Behaviour
- Block A renders a "Re-analyze with stem-aware pipeline" button only in the
  `legacy` render state AND only when App passes a callback (i.e. when a
  source File is loaded and no analysis is currently in flight).
- Clicking the button calls back up through SessionMusicianPanel →
  AnalysisResults → App, which fires `handleStartAnalysis` with an explicit
  `pitchNoteRequested: true` override. The File state (`audioFile`) is
  retained in App across analyses, so no re-upload is required; the existing
  bytes are re-posted to `POST /api/analysis-runs` with stem-aware mode on.
- The UI toggle "STEM PITCH/NOTE TRANSLATION" is also updated to reflect the
  override so subsequent manual Run Analysis clicks behave consistently.

Investigation
- `analysis_runtime.create_run` stores the artifact bytes anew on every run —
  there's no artifact-reuse API today. But because `audioFile: File` is
  retained in App state after analysis completes, re-running just means
  re-POSTing the same File, which is what the producer expects. No backend
  changes needed.

Wiring
- New optional `onReanalyzeWithStemAware?: () => void` prop on
  `SessionMusicianPanel` and `NoteDraftBlock`. AnalysisResults threads it
  through. App constructs the handler as
  `audioFile && !isAnalyzing ? () => handleStartAnalysis({ pitchNoteRequested: true }) : undefined`,
  so the button is only rendered (and clickable) when a re-run is actually
  viable.
- `handleStartAnalysis` gains a typed `overrides` param. The override is
  read in the call to `analyzeAudio`, and a matching
  `setPitchNoteTranslationRequested` keeps the toggle visually in sync.

Tests
- Vitest (`sessionMusicianPanel.test.ts`): added five static-markup cases
  covering button visibility — legacy + callback → button renders, legacy +
  no callback → no button, stem-aware → no button, full-mix-fallback → no
  button, ran-with-no-result / requested-but-unavailable → no button.
- Playwright smoke (`upload-phase1-midi.spec.ts`): stubs a run that returns
  `transcriptionMethod: 'basic-pitch'`. After the page renders the legacy
  state, asserts the button is visible, clicks it, and confirms a second
  POST to `/api/analysis-runs` fires with `pitch_note_mode=stem_notes`.

Targets PR #22 (claude/thirsty-easley-faeaea) as the base; will retarget to
main once that lands. Plan in
~/.claude/plans/re-evaluate-asa-s-session-musician-sharded-balloon.md
("Out-of-scope follow-ups" section).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@slittycode
slittycode merged commit 9961415 into main May 13, 2026
2 checks passed
@slittycode
slittycode deleted the claude/reanalyze-legacy-cta branch May 13, 2026 00:44

@slittycode slittycode left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

Adds a "Re-analyze with stem-aware pipeline" CTA button to NoteDraftBlock when it's in the legacy render state — wired through a callback chain from App.tsx down via AnalysisResults and SessionMusicianPanel. The handleStartAnalysis override pattern correctly captures the active pitch-note setting before the render cycle, then syncs the toggle state for consistency. Tests pass: 354/354 unit, 47/47 smoke (1 skipped per the description). No Phase 1 measurements are touched or re-derived anywhere in the diff.

Findings

None.

Test results

354 passed / 0 failed across 36 test files. The five new unit tests in sessionMusicianPanel.test.ts cover the expected render-state matrix cleanly (legacy+callback, legacy+no-callback, stem-aware, full-mix-fallback, ran-with-no-result/requested-but-unavailable). The smoke test correctly exercises the POST-fires-with-pitch_note_mode=stem_notes path.

One thing worth knowing: the smoke fixture at run_smoke_reanalyze_1 has both transcriptionMethod: 'basic-pitch' and fullMixFallback: true, which at first glance looks like it should hit full-mix-fallback render state and break the data-render-state="legacy" assertion. It doesn't — renderState.ts Rule 5 explicitly documents that legacy method wins over the fallback flag, and the unit test for full-mix-fallback uses a torchcrepe baseline to confirm the precedence. The fixture is fine.

Phase boundary check

Clean. No Phase 1 output is mutated or re-derived. Callback flows from App → AnalysisResults → SessionMusicianPanel → NoteDraftBlock are pure prop-threading with no access to measurement state.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant