From d3f22b2af8ddfcedfa82b72fe518e8bd0f2ba5e6 Mon Sep 17 00:00:00 2001 From: slitty-codes Date: Wed, 13 May 2026 08:28:13 +1200 Subject: [PATCH] feat(ui): re-analyze CTA for legacy Session Musician runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/ui/src/App.tsx | 25 ++- apps/ui/src/components/AnalysisResults.tsx | 9 + .../src/components/SessionMusicianPanel.tsx | 8 + .../sessionMusician/NoteDraftBlock.tsx | 20 ++ .../services/sessionMusicianPanel.test.ts | 88 +++++++++ .../ui/tests/smoke/upload-phase1-midi.spec.ts | 181 ++++++++++++++++++ 6 files changed, 329 insertions(+), 2 deletions(-) diff --git a/apps/ui/src/App.tsx b/apps/ui/src/App.tsx index 244c8af1..99cdbd25 100644 --- a/apps/ui/src/App.tsx +++ b/apps/ui/src/App.tsx @@ -473,7 +473,15 @@ export default function App() { return currentLogs; }, []); - const handleStartAnalysis = async () => { + // Overrides let the Session Musician panel re-trigger analysis for a legacy + // (non-torchcrepe) snapshot with the stem-aware pipeline forced on, without + // waiting for the React state of the toggle to flush before kicking off + // the run. The current toggle is also updated so the UI stays consistent. + type StartAnalysisOverrides = { + pitchNoteRequested?: boolean; + }; + + const handleStartAnalysis = async (overrides: StartAnalysisOverrides = {}) => { if (!audioFile) return; const activeFile = audioFile; @@ -481,6 +489,14 @@ export default function App() { const activeEstimate = analysisEstimate; const activeTimeoutMs = deriveAnalyzeTimeoutMs(activeEstimate?.totalHighMs); const audioMetadata = buildAudioMetadata(activeFile); + const activePitchNoteRequested = + overrides.pitchNoteRequested ?? pitchNoteTranslationRequested; + if ( + overrides.pitchNoteRequested !== undefined && + overrides.pitchNoteRequested !== pitchNoteTranslationRequested + ) { + setPitchNoteTranslationRequested(overrides.pitchNoteRequested); + } startRenderBenchmarkCycle(window); ignoredRunIdsRef.current.clear(); @@ -609,7 +625,7 @@ export default function App() { }, { analysisMode, - pitchNoteRequested: pitchNoteTranslationRequested, + pitchNoteRequested: activePitchNoteRequested, timeoutMs: activeTimeoutMs, signal: ac.signal, interpretationRequested, @@ -1231,6 +1247,11 @@ export default function App() { apiBaseUrl={appConfig.apiBaseUrl} runId={activeRunId ?? undefined} pitchNoteMode={analysisRun?.requestedStages.pitchNoteMode ?? null} + onReanalyzeWithStemAware={ + audioFile && !isAnalyzing + ? () => handleStartAnalysis({ pitchNoteRequested: true }) + : undefined + } /> ) : null} diff --git a/apps/ui/src/components/AnalysisResults.tsx b/apps/ui/src/components/AnalysisResults.tsx index 47f9ecb9..a9c86450 100644 --- a/apps/ui/src/components/AnalysisResults.tsx +++ b/apps/ui/src/components/AnalysisResults.tsx @@ -65,6 +65,13 @@ export interface AnalysisResultsProps { apiBaseUrl?: string; runId?: string; pitchNoteMode?: 'stem_notes' | 'off' | null; + /** + * Click handler for the "Re-analyze with stem-aware pipeline" button that + * Block A renders in its legacy render state. App owns the run-creation + * primitives; pass `undefined` to hide the button (e.g. while an analysis + * is already in flight or no source File is loaded). + */ + onReanalyzeWithStemAware?: () => void; } const LOW_CHORD_CONFIDENCE_THRESHOLD = 0.5; @@ -396,6 +403,7 @@ export function AnalysisResults({ apiBaseUrl, runId, pitchNoteMode = null, + onReanalyzeWithStemAware, }: AnalysisResultsProps) { const [openArrangement, setOpenArrangement] = useState>({}); const [openSonic, setOpenSonic] = useState>(new Set()); @@ -1551,6 +1559,7 @@ export function AnalysisResults({ sourceFileName={sourceFileName} pitchNoteMode={pitchNoteMode} hasStemListeningNotes={hasStemSummaryContent} + onReanalyzeWithStemAware={onReanalyzeWithStemAware} /> diff --git a/apps/ui/src/components/SessionMusicianPanel.tsx b/apps/ui/src/components/SessionMusicianPanel.tsx index 7a07cbb2..ebbc7444 100644 --- a/apps/ui/src/components/SessionMusicianPanel.tsx +++ b/apps/ui/src/components/SessionMusicianPanel.tsx @@ -99,12 +99,19 @@ interface SessionMusicianPanelProps { pitchNoteMode?: PitchNoteMode; /** Set by AnalysisResults when the Gemini stem listening notes section will render. */ hasStemListeningNotes?: boolean; + /** + * Optional callback for the "Re-analyze with stem-aware pipeline" button + * that NoteDraftBlock renders only in the `legacy` render state. App owns + * the run-creation primitives; passing `undefined` hides the button. + */ + onReanalyzeWithStemAware?: () => void; } export function SessionMusicianPanel({ phase1, pitchNoteMode = null, hasStemListeningNotes = false, + onReanalyzeWithStemAware, }: SessionMusicianPanelProps) { const melodyDetail = phase1.melodyDetail ?? null; const transcriptionDetail = phase1.transcriptionDetail ?? null; @@ -207,6 +214,7 @@ export function SessionMusicianPanel({ controller={controller} bpm={phase1.bpm} trackDurationSeconds={phase1.durationSeconds} + onReanalyzeWithStemAware={onReanalyzeWithStemAware} /> )} {showMelodyBlock && ( diff --git a/apps/ui/src/components/sessionMusician/NoteDraftBlock.tsx b/apps/ui/src/components/sessionMusician/NoteDraftBlock.tsx index 4d0df783..9c188d03 100644 --- a/apps/ui/src/components/sessionMusician/NoteDraftBlock.tsx +++ b/apps/ui/src/components/sessionMusician/NoteDraftBlock.tsx @@ -9,6 +9,7 @@ // precedence rules. import React, { useMemo, useState } from 'react'; +import { RotateCw } from 'lucide-react'; import type { TranscriptionDetail } from '../../types'; import type { QuantizeOptions } from '../../services/midi/types'; import { quantizeNotes } from '../../services/midi/quantization'; @@ -84,6 +85,12 @@ interface NoteDraftBlockProps { bpm: number; /** Used to scale the piano-roll x-axis; defaults to last-note-end when missing. */ trackDurationSeconds: number; + /** + * When set AND the render state is `legacy`, the block renders a button + * that re-runs analysis on the current source file with the stem-aware + * torchcrepe pipeline forced on. Hidden in all other render states. + */ + onReanalyzeWithStemAware?: () => void; } export function NoteDraftBlock({ @@ -92,6 +99,7 @@ export function NoteDraftBlock({ controller, bpm, trackDurationSeconds, + onReanalyzeWithStemAware, }: NoteDraftBlockProps) { const renderState = useMemo( () => deriveNoteDraftRenderState(transcriptionDetail, pitchNoteMode), @@ -216,6 +224,18 @@ export function NoteDraftBlock({ testId="note-draft-band" /> + {renderState === 'legacy' && onReanalyzeWithStemAware && ( + + )} +
{countLabel} | diff --git a/apps/ui/tests/services/sessionMusicianPanel.test.ts b/apps/ui/tests/services/sessionMusicianPanel.test.ts index 7b6bc104..1535674e 100644 --- a/apps/ui/tests/services/sessionMusicianPanel.test.ts +++ b/apps/ui/tests/services/sessionMusicianPanel.test.ts @@ -224,6 +224,94 @@ describe('SessionMusicianPanel — Block A render states', () => { expect(deriveNoteDraftRenderState(transcriptionDetail, 'stem_notes')).toBe('legacy'); }); + it('renders the Re-analyze CTA in legacy state when the callback is provided', () => { + const transcriptionDetail = stemAwareTranscription({ + transcriptionMethod: 'basic-pitch', + }); + const html = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { ...baseMeasurement, transcriptionDetail }, + pitchNoteMode: 'stem_notes', + onReanalyzeWithStemAware: () => undefined, + }), + ); + expect(html).toContain('data-render-state="legacy"'); + expect(html).toContain('data-testid="note-draft-reanalyze"'); + expect(html).toContain('Re-analyze with stem-aware pipeline'); + }); + + it('omits the Re-analyze CTA in legacy state when the callback is absent', () => { + const transcriptionDetail = stemAwareTranscription({ + transcriptionMethod: 'basic-pitch', + }); + const html = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { ...baseMeasurement, transcriptionDetail }, + pitchNoteMode: 'stem_notes', + }), + ); + expect(html).toContain('data-render-state="legacy"'); + expect(html).not.toContain('data-testid="note-draft-reanalyze"'); + }); + + it('does not render the Re-analyze CTA in the stem-aware render state', () => { + const html = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { ...baseMeasurement, transcriptionDetail: stemAwareTranscription() }, + pitchNoteMode: 'stem_notes', + onReanalyzeWithStemAware: () => undefined, + }), + ); + expect(html).toContain('data-render-state="stem-aware"'); + expect(html).not.toContain('data-testid="note-draft-reanalyze"'); + }); + + it('does not render the Re-analyze CTA in the full-mix-fallback render state', () => { + const transcriptionDetail = stemAwareTranscription({ + stemSeparationUsed: false, + fullMixFallback: true, + stemsTranscribed: ['full_mix'], + notes: stemAwareTranscription().notes.map((note) => ({ + ...note, + stemSource: 'full_mix', + })), + }); + const html = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { ...baseMeasurement, transcriptionDetail }, + pitchNoteMode: 'stem_notes', + onReanalyzeWithStemAware: () => undefined, + }), + ); + expect(html).toContain('data-render-state="full-mix-fallback"'); + expect(html).not.toContain('data-testid="note-draft-reanalyze"'); + }); + + it('does not render the Re-analyze CTA in ran-with-no-result or requested-but-unavailable states', () => { + const ranHtml = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { + ...baseMeasurement, + transcriptionDetail: stemAwareTranscription({ notes: [], noteCount: 0 }), + }, + pitchNoteMode: 'stem_notes', + onReanalyzeWithStemAware: () => undefined, + }), + ); + expect(ranHtml).toContain('data-render-state="ran-with-no-result"'); + expect(ranHtml).not.toContain('data-testid="note-draft-reanalyze"'); + + const requestedHtml = renderToStaticMarkup( + React.createElement(SessionMusicianPanel, { + phase1: { ...baseMeasurement }, + pitchNoteMode: 'stem_notes', + onReanalyzeWithStemAware: () => undefined, + }), + ); + expect(requestedHtml).toContain('data-render-state="requested-but-unavailable"'); + expect(requestedHtml).not.toContain('data-testid="note-draft-reanalyze"'); + }); + it('shows requested-but-unavailable notice when stem_notes is on but transcription is missing', () => { const html = renderToStaticMarkup( React.createElement(SessionMusicianPanel, { diff --git a/apps/ui/tests/smoke/upload-phase1-midi.spec.ts b/apps/ui/tests/smoke/upload-phase1-midi.spec.ts index 29798f44..d4d3f23f 100644 --- a/apps/ui/tests/smoke/upload-phase1-midi.spec.ts +++ b/apps/ui/tests/smoke/upload-phase1-midi.spec.ts @@ -824,3 +824,184 @@ test('pitch/note off with melody present shows opted-out state', async ({ page } // Block B owns its own quantize controls. await expect(melodyContour.getByRole('button', { name: '1/16 note' })).toBeVisible(); }); + +test('legacy run shows Re-analyze CTA that fires a new POST with pitch_note_mode=stem_notes', async ({ page }) => { + await stubGeminiPhase2(page); + + const postedPitchNoteModes: string[] = []; + + await page.route('**/api/analysis-runs/estimate', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + requestId: 'req_estimate_smoke_reanalyze_001', + estimate: { + durationSeconds: 12, + totalLowMs: 4000, + totalHighMs: 6000, + stages: [{ key: 'local_dsp', label: 'Local DSP analysis', lowMs: 4000, highMs: 6000 }], + }, + }), + }); + }); + + await page.route('**/api/analysis-runs', async (route) => { + if (route.request().method() !== 'POST') { + await route.fallback(); + return; + } + const body = route.request().postData() ?? ''; + const match = body.match(/name="pitch_note_mode"\r?\n\r?\n([^\r\n]+)/); + postedPitchNoteModes.push(match?.[1] ?? ''); + const runIndex = postedPitchNoteModes.length; + const runId = `run_smoke_reanalyze_${runIndex}`; + // Return a minimal snapshot — the GET handler below fills in the data + // the page actually renders from. + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + runId, + requestedStages: { + pitchNoteMode: 'stem_notes', + pitchNoteBackend: 'auto', + interpretationMode: 'off', + interpretationProfile: 'producer_summary', + interpretationModel: null, + }, + artifacts: { + sourceAudio: { + artifactId: `artifact_smoke_reanalyze_${runIndex}`, + filename: 'silence.wav', + mimeType: 'audio/wav', + sizeBytes: 2048, + contentSha256: 'abc123', + path: '/tmp/silence.wav', + }, + }, + stages: { + measurement: { status: 'queued', authoritative: true, result: null, provenance: null, diagnostics: null, error: null }, + pitchNoteTranslation: { status: 'queued', authoritative: false, preferredAttemptId: null, attemptsSummary: [], result: null, provenance: null, diagnostics: null, error: null }, + interpretation: { status: 'not_requested', authoritative: false, preferredAttemptId: null, attemptsSummary: [], result: null, provenance: null, diagnostics: null, error: null }, + }, + }), + }); + }); + + // Only the first run completes with a legacy basic-pitch transcription. + // The second run created by the Re-analyze CTA can stay queued — the + // assertion is just that the POST fired with pitch_note_mode=stem_notes. + await page.route('**/api/analysis-runs/run_smoke_reanalyze_1', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + runId: 'run_smoke_reanalyze_1', + requestedStages: { + pitchNoteMode: 'stem_notes', + pitchNoteBackend: 'auto', + interpretationMode: 'off', + interpretationProfile: 'producer_summary', + interpretationModel: null, + }, + artifacts: { + sourceAudio: { + artifactId: 'artifact_smoke_reanalyze_1', + filename: 'silence.wav', + mimeType: 'audio/wav', + sizeBytes: 2048, + contentSha256: 'abc123', + path: '/tmp/silence.wav', + }, + }, + stages: { + measurement: { + status: 'completed', + authoritative: true, + result: { + bpm: 120, + bpmConfidence: 0.9, + key: 'C major', + keyConfidence: 0.85, + timeSignature: '4/4', + durationSeconds: 12, + lufsIntegrated: -10, + truePeak: -0.3, + stereoWidth: 0.5, + stereoCorrelation: 0.8, + spectralBalance: { subBass: 0, lowBass: 0, lowMids: 0, mids: 0, upperMids: 0, highs: 0, brilliance: 0 }, + }, + provenance: null, + diagnostics: null, + error: null, + }, + pitchNoteTranslation: { + status: 'completed', + authoritative: false, + preferredAttemptId: 'sym_smoke_reanalyze_1', + attemptsSummary: [ + { attemptId: 'sym_smoke_reanalyze_1', backendId: 'basic-pitch', mode: 'stem_notes', status: 'completed' }, + ], + result: { + transcriptionMethod: 'basic-pitch', + noteCount: 1, + averageConfidence: 0.5, + stemSeparationUsed: false, + fullMixFallback: true, + stemsTranscribed: ['full_mix'], + dominantPitches: [{ pitchMidi: 60, pitchName: 'C4', count: 1 }], + pitchRange: { minMidi: 60, maxMidi: 60, minName: 'C4', maxName: 'C4' }, + notes: [ + { + pitchMidi: 60, + pitchName: 'C4', + onsetSeconds: 0, + durationSeconds: 0.5, + confidence: 0.5, + stemSource: 'full_mix', + }, + ], + }, + provenance: null, + diagnostics: null, + error: null, + }, + interpretation: { + status: 'not_requested', + authoritative: false, + preferredAttemptId: null, + attemptsSummary: [], + result: null, + provenance: null, + diagnostics: null, + error: null, + }, + }, + }), + }); + }); + + await page.goto('/', { waitUntil: 'networkidle' }); + const fixturePath = path.resolve(testDir, './fixtures/silence.wav'); + await page.setInputFiles('#audio-upload', fixturePath); + // Uncheck AI interpretation so the test doesn't need a Phase 2 stub. + await page.getByLabel('AI INTERPRETATION').uncheck(); + await page.getByRole('button', { name: /Run Analysis/i }).click(); + + const panel = page.locator('section').filter({ hasText: /SESSION MUSICIAN/i }).first(); + const noteDraft = panel.getByTestId('note-draft-block'); + await expect(noteDraft).toBeVisible(); + await expect(noteDraft).toHaveAttribute('data-render-state', 'legacy'); + // The Re-analyze CTA is visible in the legacy state. + const reanalyzeButton = noteDraft.getByTestId('note-draft-reanalyze'); + await expect(reanalyzeButton).toBeVisible(); + await expect(reanalyzeButton).toContainText(/Re-analyze with stem-aware pipeline/i); + + // Two POSTs so far: the original run created during Run Analysis. Clicking + // the CTA should fire a second POST with pitch_note_mode=stem_notes. + expect(postedPitchNoteModes).toHaveLength(1); + await reanalyzeButton.click(); + await expect.poll(() => postedPitchNoteModes.length, { timeout: 5000 }).toBe(2); + expect(postedPitchNoteModes[1]).toBe('stem_notes'); +});