Skip to content

feat: per-stem confidence in Session Musician note draft - #23

Closed
slittycode wants to merge 1 commit into
claude/thirsty-easley-faeaeafrom
claude/per-stem-confidence
Closed

feat: per-stem confidence in Session Musician note draft#23
slittycode wants to merge 1 commit into
claude/thirsty-easley-faeaeafrom
claude/per-stem-confidence

Conversation

@slittycode

Copy link
Copy Markdown
Owner

Why

Bass and lead stems often have very different transcription confidence — bass might be Solid (0.85) while the lead is Rough (0.4). PR #22 collapses both into a single combined band, which hides that. When a producer toggles the BASS or OTHER button in the Stem note draft block, the confidence pill should reflect that stem's mean confidence; the "All" state keeps showing the overall.

What changed

Backend

  • New pure helper _per_stem_average_confidence in analyze_transcription.py computes mean confidence per stemSource over the surviving post-dedup, post-cap notes.
  • TorchcrepeBackend.transcribe emits perStemAverageConfidence: Record<string, number> on both populated and empty result paths. Returns {} when fullMixFallback is true — no meaningful separation to report.
  • JSON_SCHEMA.md documents the new field.
  • No server.py or analysis_runtime.py changes needed — the field rides through the existing transcriptionDetail passthrough.

Frontend

  • Optional perStemAverageConfidence on the TranscriptionDetail type in measurement.ts — optional because legacy snapshots and the empty-notes path may omit it.
  • New pure helper selectNoteDraftBandConfidence in renderState.ts picks the right value based on (transcriptionDetail, stemFilter, renderState). Falls back to the overall when the filter is null, the field is missing, the key isn't keyed, the value is non-finite, or the render state is fallback/legacy.
  • NoteDraftBlock.tsx uses the helper to drive the band's confidence prop.
  • Bugfix discovered along the way: both analysisRunsClient.ts and backendPhase1Client.ts rebuild transcriptionDetail from an explicit field list and were silently dropping any new fields. Added matching parsePerStemAverageConfidence in each — clamps to 0–1 in the legacy path and skips non-finite entries in both.

Tests

  • test_analyze.py — pure-helper coverage: groups means by stem, handles empty notes, skips missing/empty/null stemSource, tolerates non-numeric confidence values.
  • test_server.py — end-to-end round-trip: subprocess emits perStemAverageConfidence, runtime persists it, snapshot returns it unchanged. Catches any future whitelist trimming in the worker path.
  • renderState.test.tsselectNoteDraftBandConfidence matrix (overall fallback, bass, other, missing field, missing key, non-finite value, plus the two override render states).
  • upload-phase1-midi.spec.ts — stubs perStemAverageConfidence: { bass: 0.92, other: 0.32 } and clicks BASS → OTHER → All, asserting the pill text changes Solid scaffold · 92%Rough sketch · 32%Solid scaffold · 83% (the overall).

364 backend tests pass. 46 smoke tests pass (1 skipped — live Gemini). Lint + build clean.

Base

This targets claude/thirsty-easley-faeaea (PR #22) since that's where the Session Musician redesign lives. Once #22 lands on main, GitHub can retarget this PR or I can rebase + force-push.

Plan

~/.claude/plans/re-evaluate-asa-s-session-musician-sharded-balloon.md — "Out-of-scope follow-ups" section.

🤖 Generated with Claude Code

Bass and lead stems often have very different transcription confidence — bass
might be Solid (0.85) while other is Rough (0.4). Collapsing them into one
combined band hides that. When the producer toggles the BASS / OTHER button
in Block A, the confidence pill now reflects that stem's mean confidence; the
"All" state keeps showing the overall.

Backend
- analyze_transcription.py: new pure helper `_per_stem_average_confidence`
  computes the mean confidence per `stemSource` over the surviving
  post-dedup, post-cap notes. `TorchcrepeBackend.transcribe` emits the new
  `perStemAverageConfidence: Record<string, number>` field on both the
  populated and empty result paths. Returns `{}` when `fullMixFallback` is
  true so the UI doesn't show a per-stem band where there's no meaningful
  separation to report.
- JSON_SCHEMA.md: documents the new field.
- analyze.py: re-exports the helper so test_analyze can reach it via the
  same module-loading pattern as the other transcription helpers.
- No changes to server.py or analysis_runtime — the field flows through the
  existing transcriptionDetail passthrough automatically (confirmed by the
  new round-trip server test).

Frontend
- measurement.ts: adds optional `perStemAverageConfidence` to
  TranscriptionDetail. Optional because legacy snapshots and the empty-notes
  path may omit it.
- renderState.ts: new pure helper `selectNoteDraftBandConfidence` picks the
  right confidence value based on (transcriptionDetail, stemFilter,
  renderState). Falls back to the overall when stem filter is null, the
  per-stem field is missing, the selected stem key isn't keyed, or the value
  is non-finite. Returns the overall in full-mix-fallback / legacy render
  states since those replace the band copy entirely.
- NoteDraftBlock.tsx: uses the helper to drive the band confidence prop.
- analysisRunsClient.ts and backendPhase1Client.ts: both rebuild
  transcriptionDetail from an explicit field list and were silently dropping
  the new field. Added `parsePerStemAverageConfidence` to each — clamps to
  0-1 in the legacy path and skips non-finite entries in both.

Tests
- test_analyze.py: pure-helper coverage for `_per_stem_average_confidence`
  (groups means by stem, handles empty notes, skips missing/empty/null
  stemSource, tolerates non-numeric confidence values).
- test_server.py: end-to-end round-trip — subprocess emits
  perStemAverageConfidence, runtime persists it, run snapshot returns it
  unchanged. Catches any future whitelisting in the worker path.
- renderState.test.ts: `selectNoteDraftBandConfidence` matrix (overall fallback,
  bass stem, other stem, missing field, missing key, non-finite value, plus
  the two override render states).
- upload-phase1-midi.spec.ts: stubs `perStemAverageConfidence: {bass: 0.92,
  other: 0.32}` and clicks BASS → OTHER → All, asserting the pill text
  changes "Solid scaffold · 92%" → "Rough sketch · 32%" → "Solid scaffold ·
  83%" (the overall).

Targets PR #22 (claude/thirsty-easley-faeaea) as the base; will retarget to
main after 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 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 perStemAverageConfidence as an optional, additive field on transcriptionDetail — computed at Phase 1 (post-dedup, post-cap notes in the transcription pipeline) and surfaced in the UI via a pure selector so the confidence band pill tracks the active stem filter. The change is purely additive on the Phase 1 schema; no existing fields touched. Test coverage is real and meaningful. One minor inconsistency worth cleaning up.

Findings

Worth considering

analysisRunsClient.ts:parsePerStemAverageConfidence (~line 619) accepts any finite float, but backendPhase1Client.ts:parsePerStemAverageConfidence (~line 992) applies clamp01. The PR author even left a comment in the legacy-client version explaining the clamp is "defensive against backend payload bugs." Both parsers are wired to the same field from the same backend, so they should behave identically. In practice the backend only ever emits values from np.mean of torchcrepe sigmoid outputs, so the range is always 0–1 and this won't bite. But the inconsistency is noticeable.

Observation (not a finding)

_per_stem_average_confidence uses note.get("confidence", 0.0) — a note with no confidence key contributes 0.0 to the average. Every note produced by analyze.py carries the key, so this is fine. Pre-existing pattern, not a regression.

Test results

Test infrastructure not installed in this review environment. PR reports: 364 backend tests pass, 46 smoke tests pass (1 skipped — live Gemini). _per_stem_average_confidence has four direct unit tests (groups by stem, empty input, missing/blank stemSource, invalid confidence types). selectNoteDraftBandConfidence has seven unit tests covering all documented fallback paths. Server round-trip test explicitly asserts the field survives the worker passthrough. Smoke test verifies pill text changes on BASS → OTHER → All. Coverage is thorough.

Phase boundary check

Clean. perStemAverageConfidence is computed inside transcribe() over the post-dedup, post-cap notes — solidly Phase 1. selectNoteDraftBandConfidence and NoteDraftBlock.tsx read Phase 1 values and choose which one to display; neither mutates nor re-derives Phase 1 data. No Phase 2/3 files changed.


Generated by Claude Code

@slittycode

Copy link
Copy Markdown
Owner Author

Superseded by #28 (rebased onto main and re-opened as a new PR after PR #22 merged caused this one to auto-close).

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