diff --git a/CLAUDE.md b/CLAUDE.md index e1d958d8..323847c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,8 +71,32 @@ npm run test:smoke -- tests/smoke/upload-phase1.spec.ts TEST_FLAC_PATH=/path/to/track.flac GEMINI_API_KEY=… VITE_ENABLE_PHASE2_GEMINI=true ./scripts/test-e2e.sh # Full live Gemini run ``` +### Scripts at a glance + +Operational and one-shot scripts live in two places. They are not on the request-path; reach for them when the situation calls for it. + +`scripts/` (repo root): +1. `dev.sh` — full-stack dev launcher (covered above). +2. `test-e2e.sh` / `test-e2e-integration.sh` — e2e harnesses (covered above). +3. `calibrate_confidence.py` — threshold-sweep harness for the pitch / chord / sidechain detectors. Research-only. + +`apps/backend/scripts/`: +1. `bootstrap.sh` — recreate the Python 3.11 venv (covered above). +2. `render_upload_limit_contract.py` — regenerate the operator-facing upload-limit contract text from `upload_limits.py`. Run after changing the canonical limits. +3. `evaluate_phase1.py`, `evaluate_polyphonic.py`, `evaluate_structure_sweep.py` — offline evaluation harnesses for the Phase 1 detector battery, the research polyphonic transcriber, and structure-segmentation parameter sweeps. Wired to `phase1_evaluation.py` / `polyphonic_evaluation.py`. +4. `audit_pass1.py`, `genre_check.py`, `replay_catalog_validation.py` — corpus auditing and Live 12 device-catalog validation. `genre_corpus.md` is the corpus manifest. + ## Architecture +### Repo Layout — what's on the product path + +`apps/backend/`, `apps/ui/`, and `scripts/` are the product. The remaining top-level directories are off-path and safe to skip unless the task explicitly names them: + +1. `advisory/` — auditor notes and one-shot reviews (e.g. `phase1_audit/`). Not imported by either app. +2. `experiments/` — sandbox scratch space (e.g. `crepe_test/` carries only a venv). Throwaway by intent; do not wire production code here. +3. `docs/` — long-form rationale (`ARCHITECTURE_STRATEGY.md`, `history/`). Read these *before* structural changes; do not treat them as living API docs. +4. `tests/ground_truth/` — labeled-corpus fixtures consumed by `scripts/calibrate_confidence.py` (see `tests/ground_truth/README.md`). Not a test suite — each app owns its own (`apps/backend/tests/`, `apps/ui/tests/`). + ### Three-Layer Model ASA's hybrid architecture splits work into three layers. Read `docs/ARCHITECTURE_STRATEGY.md` before proposing changes to this structure — it records *why* the design is shaped this way. @@ -112,7 +136,7 @@ The backend supports staged execution via `analysis_runtime.py`, which persists Run-oriented endpoints (`/api/analysis-runs*`) are the canonical interface for staged execution. Legacy `POST /api/analyze`, `POST /api/analyze/estimate`, and `POST /api/phase2` remain only as temporary compatibility wrappers — do not build new functionality on them. -Phase 3 audition-sample generation (`POST/GET /api/analysis-runs/{run_id}/samples`) is **on-demand**, not a queue stage — the UI explicitly requests it after interpretation completes. See `server_samples.py` and the `sample_*.py` modules. +Phase 3 audition-sample generation (`POST/GET /api/analysis-runs/{run_id}/samples`) is **on-demand**, not a queue stage — the UI explicitly requests it after interpretation completes. See `server_samples.py` and the `sample_*.py` modules. On the frontend, `apps/ui/src/components/SamplePlayback.tsx` is the entry point — mounted from `AnalysisResults.tsx` and driven by `src/services/sampleGenerationClient.ts` (`generateSamples` posts, `fetchSamples` retrieves). Frontend polling: `src/services/analysisRunsClient.ts` creates runs and polls stage snapshots. `src/services/analyzer.ts` orchestrates the create-run + poll loop and projects display payloads. @@ -140,7 +164,7 @@ Artifact access goes through `artifact_storage.py` rather than direct disk paths 9. **`upload_limits.py`**: Canonical 100 MiB raw-audio / 101 MiB request-envelope limits. Regenerate the operator contract via `scripts/render_upload_limit_contract.py` if numbers change. 10. **`spectral_viz.py`**: Librosa-based spectrogram/time-series artifacts. Called after measurement; failures are non-critical. 11. **`url_ingest.py`**: SSRF-guarded URL-mode ingestion for `POST /api/analysis-runs` — fetches a public `http`/`https` audio URL and feeds it through the same downstream pipeline as a multipart upload. -12. **`csv_export.py`**: CSV exporters for Phase 1 time-series fields, backing `GET /api/analysis-runs/{run_id}/export/csv/{field_path}`. Keeps the route handler a thin lookup-and-serve. +12. **`csv_export.py`**: CSV exporters for Phase 1 time-series fields, backing `GET /api/analysis-runs/{run_id}/export/csv/{field_path}`. Keeps the route handler a thin lookup-and-serve. Field paths are an explicit allowlist (`csv_export.list_supported_fields()` — currently `lufsCurve.shortTerm`, `lufsCurve.momentary`, `rhythmDetail.tempoCurve`, `spectralBalanceTimeSeries`); arbitrary descent into the payload is not supported. Example: `curl http://127.0.0.1:8100/api/analysis-runs//export/csv/rhythmDetail.tempoCurve -o tempo.csv`. 13. **`stage_status.py`**: Collapses the eight internal stage statuses into the additive client-facing `publicStatus` field on every stage snapshot. 14. **`server_samples.py` + `sample_generation.py`, `sample_theory.py`, `sample_synthesis.py`, `sample_drums.py`**: Phase 3 audition-sample generation. `sample_theory.py` builds the PyTheory musical plan, `sample_synthesis.py` renders audio (FluidSynth with sine-additive fallback), `sample_drums.py` synthesizes drum one-shots, `sample_generation.py` orchestrates and emits the citation manifest. On-demand only. 15. **`dsp_bandbank.py` + `dsp_utils.py`**: Shared DSP primitives — `BatchedBandpass` (4th-order Butterworth bandpass bank) and cross-module utility functions. diff --git a/apps/ui/src/components/AnalysisResults.tsx b/apps/ui/src/components/AnalysisResults.tsx index 47ce64af..7ade7407 100644 --- a/apps/ui/src/components/AnalysisResults.tsx +++ b/apps/ui/src/components/AnalysisResults.tsx @@ -40,7 +40,9 @@ import { } from './MeasurementPrimitives'; import { PhaseSourceBadge } from './PhaseSourceBadge'; import { StickyNav, type StickyNavSection } from './StickyNav'; -import { CitationBlock } from './CitationBlock'; +import { CitationBlock, CitationHeadline } from './CitationBlock'; +import { ConfidenceBandBadge } from './sessionMusician/ConfidenceBandBadge'; +import { toConfidenceBand } from '../services/sessionMusician/confidenceBand'; import { loadAppliedIds, toggleAppliedId } from '../services/appliedRecommendations'; import { buildArrangementViewModel, @@ -256,11 +258,9 @@ function SourcesToggle({ sources, showSources, onToggle }: { sources?: string[]; ); } -function confidenceClass(level: string): string { - if (level === 'High') return 'text-success bg-success/10 border-success/20'; - if (level === 'Moderate') return 'text-warning bg-warning/10 border-warning/20'; - return 'text-error bg-error/10 border-error/20'; -} +// Audit Finding #4: `confidenceClass` was the tone mapper for the legacy +// three-level Confidence Notes chips. Retired — chips now route through +// `ConfidenceBandBadge` with the canonical four-band ladder. function shortenCharacteristicName(name: string): string { return name.trim().split(/\s+/).slice(0, 2).join(' '); @@ -409,6 +409,98 @@ function MetaBadgeList({ items }: { items: MetaBadgeItem[] }) { // orange-accent pills. Track Layout — its only call site — now uses // CitationBlock with the segmentIndexes routed through the `extraRows` prop. +// Audit Finding #1C: the Interpretation Caution panel used to render the +// backend's `originalValue` / `coercedValue` strings verbatim inside small +// badges. For dropped Phase 2 recommendations, the backend JSON-dumps the +// whole AbletonRecommendation object into `originalValue` (see +// `_stringify_warning_value` / `_build_phase2_validation_warning` in +// `apps/backend/server_phase2.py`). The producer would see a literal +// `{"advancedTip":"…","device":"$Saturator","phase1Fields":[...],…}` string +// inside the panel — engine output leaking through. +// +// `formatDroppedValue` keeps the backend contract intact and renders a +// compact human summary for JSON-shaped values: parse, pick a few headline +// keys (device, parameter, value, …), join as "k: v · k: v". Non-JSON +// strings pass through. Invalid JSON falls back to a truncated raw string. +const FORMAT_DROPPED_VALUE_HEADLINE_KEYS = [ + 'device', + 'parameter', + 'value', + 'category', + 'name', + 'field', +] as const; +const FORMAT_DROPPED_VALUE_MAX_CHARS = 80; + +function formatDroppedValue(raw: unknown): string { + if (raw === null || raw === undefined) return '—'; + const str = String(raw).trim(); + if (str.length === 0) return '—'; + const looksLikeJson = str.startsWith('{') || str.startsWith('['); + if (!looksLikeJson) { + return str.length > FORMAT_DROPPED_VALUE_MAX_CHARS + ? `${str.slice(0, FORMAT_DROPPED_VALUE_MAX_CHARS - 1)}…` + : str; + } + + let parsed: unknown; + try { + parsed = JSON.parse(str); + } catch { + return str.length > FORMAT_DROPPED_VALUE_MAX_CHARS + ? `${str.slice(0, FORMAT_DROPPED_VALUE_MAX_CHARS - 1)}…` + : str; + } + + if (Array.isArray(parsed)) { + const n = parsed.length; + const noun = `${n} item${n === 1 ? '' : 's'}`; + const first = parsed[0]; + if (first && typeof first === 'object' && !Array.isArray(first)) { + const firstRec = first as Record; + const label = + (typeof firstRec.device === 'string' && firstRec.device) || + (typeof firstRec.name === 'string' && firstRec.name) || + null; + if (label) return `${noun} (${label}${n > 1 ? ', …' : ''})`; + } + return noun; + } + + if (parsed && typeof parsed === 'object') { + const record = parsed as Record; + const parts: string[] = []; + for (const key of FORMAT_DROPPED_VALUE_HEADLINE_KEYS) { + if (parts.length >= 3) break; + const value = record[key]; + if (value === null || value === undefined || value === '') continue; + const valueStr = typeof value === 'string' ? value : JSON.stringify(value); + if (!valueStr) continue; + parts.push(`${key}: ${valueStr}`); + } + if (parts.length === 0) { + const entries = Object.entries(record).filter( + ([, v]) => v !== null && v !== undefined && v !== '', + ); + for (const [k, v] of entries.slice(0, 2)) { + const valueStr = typeof v === 'string' ? v : JSON.stringify(v); + parts.push(`${k}: ${valueStr}`); + } + } + if (parts.length === 0) return '—'; + const joined = parts.join(' · '); + return joined.length > FORMAT_DROPPED_VALUE_MAX_CHARS + ? `${joined.slice(0, FORMAT_DROPPED_VALUE_MAX_CHARS - 1)}…` + : joined; + } + + // Primitive that happened to start with `{` / `[` (very unlikely after the + // JSON.parse succeeded into an object, but defensive). + return str.length > FORMAT_DROPPED_VALUE_MAX_CHARS + ? `${str.slice(0, FORMAT_DROPPED_VALUE_MAX_CHARS - 1)}…` + : str; +} + function describeInterpretationWarning( warning: InterpretationValidationWarning, ): Pick { @@ -505,9 +597,9 @@ function meterStatusLabel(phase1: Phase1Result): string { return isAssumedMeter(phase1) ? 'ASSUMED' : 'DETECTED'; } -function formatBpmScore(value: number): string { - return `SCORE ${value.toFixed(2).replace(/\.?0+$/, '')}`; -} +// Audit Finding #4: `formatBpmScore` retired — the BPM card now renders +// the canonical band pill via ConfidenceBandBadge, same vocabulary as +// every other confidence surface. export function AnalysisResults({ phase1, @@ -803,11 +895,10 @@ export function AnalysisResults({ headerRight={} footer={
- + {/* Audit Finding #4: `SCORE 0.86` badge retired in favor of the + canonical band pill — same vocabulary as Key, Character, and + every other confidence surface. */} + {phase1.bpmSource && ( {phase1.bpmSource.replace(/_/g, ' ')} @@ -839,9 +930,9 @@ export function AnalysisResults({ color="var(--color-accent)" glow /> - - CONF {(phase1.keyConfidence * 100).toFixed(0)}% - + {/* Audit Finding #4: `CONF 62%` text replaced with the canonical + band pill so every confidence reads in the same vocabulary. */} +
} /> @@ -884,9 +975,12 @@ export function AnalysisResults({ color="var(--color-accent)" glow /> - - CONF {Math.round(phase1.genreDetail.confidence * 100)}% - + {/* Audit Finding #4: `CONF X%` replaced with the canonical + band pill — same vocabulary across every confidence. */} + } /> @@ -1058,12 +1152,16 @@ export function AnalysisResults({ > {(m.originalValue || m.coercedValue) && ( <> + {/* Audit Finding #1C: render compact human summary + for JSON-shaped values (dropped recommendations) + instead of dumping the raw object. See + `formatDroppedValue` near the top of the file. */} - {m.originalValue ?? '—'} + {formatDroppedValue(m.originalValue)} → - {m.coercedValue ?? '—'} + {formatDroppedValue(m.coercedValue)} )} @@ -1090,14 +1188,22 @@ export function AnalysisResults({ {confidenceBadges.length > 0 && (
- {confidenceBadges.map((badge, idx) => ( - - {badge.label}: {badge.level} - - ))} + {/* Audit Finding #4: chips used to render "{label}: High|Moderate|Low" + with bespoke success/warning/error tones. Now route through the + canonical band ladder so the same vocabulary (Solid / Workable / + Rough / Unreliable) appears across every confidence surface. + Filter null bands (unparseable values) rather than render a + misleading default. */} + {confidenceBadges.map((badge, idx) => + badge.band ? ( + + + {badge.label}: + + + + ) : null, + )}
)} @@ -1552,17 +1658,21 @@ export function AnalysisResults({ > {item.name} - - {item.confidence} - + {/* Audit Finding #4: Detected Characteristics cards used + to render a HIGH/MED/LOW string pill with bespoke + success/warning/error tones. Replaced with the canonical + ConfidenceBandBadge so the same vocabulary (Solid / + Workable / Rough / Unreliable) reads across every + confidence surface in the UI. toConfidenceBand maps + Gemini's HIGH→solid (0.9), MED→workable (0.6), + LOW→rough (0.3) — middle of each band so the percent + label reads as an honest hedge. */} + {(() => { + const band = toConfidenceBand(item.confidence); + return band ? ( + + ) : null; + })()}

{truncateAtSentenceBoundary(item.explanation, 600)} @@ -1814,6 +1924,18 @@ export function AnalysisResults({ )} + {/* Audit Finding #3: primary citation visible in the + collapsed header. Mirrors the Mix Chain / Patch + placement so all three card types feel parallel. */} + {card.phase1Fields.length > 0 && ( +

+ +
+ )}

{card.summary}

@@ -1958,6 +2080,20 @@ export function AnalysisResults({ {card.category} + {/* Audit Finding #3: primary citation visible in + the collapsed header so the chain-of-custody + evidence isn't gated behind expansion. The + expanded CitationBlock below still carries the + full multi-row list. */} + {card.phase1Fields.length > 0 && ( +
+ +
+ )}

{card.role}

@@ -2103,9 +2239,25 @@ export function AnalysisResults({ {patch.category} -

- {patch.patchRole} -

+ {/* Audit Finding #3: primary citation in the + collapsed header so the chain-of-custody + evidence is visible without expanding. */} + {patch.phase1Fields.length > 0 && ( +
+ +
+ )} + {/* Audit Finding #1B: the per-card patchRole + paragraph used to render a duplicated + category-keyed placeholder ("Primary tone + generator" on every SYNTHESIS card). It has been + removed; the category chip above carries the + bucket and `whyThisWorks` (inside the expanded + card body) carries the actionable explanation. */}
); } + +/** + * Audit Finding #3: surfaces a one-line primary citation inside the collapsed + * card header so the chain-of-custody evidence is visible without expanding + * the card. The audit's literal example: + * + * Crest factor 8.2 dB → Glue Compressor + * + * Reads as `{humanized label} {formatted value} → {device h4 to the right}`. + * The arrow at the end of the headline points into the device name that + * follows in the card title row, making the implicit "measurement justified + * device" statement visible at scan-time. + * + * Returns `null` when the cited field doesn't resolve in Phase 1 — the caller + * is expected to guard with `phase1Fields.length > 0`, but the component is + * defensive against unmapped paths and empty Phase 1 payloads. The expanded + * card body still renders the full CitationBlock with up to 4 rows; this + * primitive is the collapsed-state companion, not a replacement. + */ +interface CitationHeadlineProps { + phase1: Phase1Result; + /** Single primary path (typically `card.phase1Fields[0]`). */ + field: string; + /** When false, the confidence pill is hidden even if a value is available. */ + showConfidenceBadge?: boolean; + className?: string; + testId?: string; +} + +export function CitationHeadline({ + phase1, + field, + showConfidenceBadge = true, + className, + testId = 'citation-headline', +}: CitationHeadlineProps) { + const raw = pickPhase1Value(phase1, field); + const value = formatCitedValue(field, raw); + if (value === '') return null; + + const label = humanizeFieldPath(field); + const confidence = pickPhase1Confidence(phase1, field); + const band = + showConfidenceBadge && confidence !== null + ? getConfidenceBand(confidence) + : null; + const pillClass = band ? CONFIDENCE_PILL_CLASSES[band.id] : null; + + return ( + + + {label} + + + {value} + + {band && pillClass && confidence !== null && ( + + {formatBandPillLabel(band, confidence)} + + )} + + + ); +} diff --git a/apps/ui/src/components/MeasurementDashboard.tsx b/apps/ui/src/components/MeasurementDashboard.tsx index a62ff06c..d14ae14c 100644 --- a/apps/ui/src/components/MeasurementDashboard.tsx +++ b/apps/ui/src/components/MeasurementDashboard.tsx @@ -22,6 +22,7 @@ import { SpectrogramViewer } from './SpectrogramViewer'; import { SpectralEvolutionChart } from './SpectralEvolutionChart'; import { ChromaHeatmap } from './ChromaHeatmap'; import { MiniHeatmap } from './MiniHeatmap'; +import { ConfidenceBandBadge } from './sessionMusician/ConfidenceBandBadge'; import { MixDoctorPanel } from './MixDoctorPanel'; import { AccentMetricCard, @@ -58,7 +59,8 @@ const formatDuration = (seconds: number): string => { return `${mins}:${secs.toString().padStart(2, '0')}`; }; -const formatBpmScore = (value: number): string => `SCORE ${formatNumber(value, 2)}`; +// Audit Finding #4: `formatBpmScore` retired — Tempo card renders the +// canonical band pill, same vocabulary as every other confidence surface. const isAssumedMeter = (phase1: Phase1Result): boolean => phase1.timeSignatureSource === 'assumed_four_four' || (phase1.timeSignatureConfidence ?? 1) <= 0; @@ -1326,7 +1328,11 @@ export function MeasurementDashboard({ footer={
- + {/* Audit Finding #4: `SCORE 0.86` badge retired — band pill + shows the same hedge in the canonical vocabulary. + Cross-Check is an agreement signal (do multiple BPM + detectors agree?), orthogonal to confidence — left as-is. */} + {phase1.bpmAgreement !== undefined && phase1.bpmAgreement !== null && ( )} - - CONF {Math.round(phase1.keyConfidence * 100)}% - + {/* Audit Finding #4: `CONF X%` retired in favor of the canonical + band pill. Same vocabulary as the AnalysisResults Key card. */} +
} /> diff --git a/apps/ui/src/components/Phase2ConsistencyReport.tsx b/apps/ui/src/components/Phase2ConsistencyReport.tsx index e37ab134..2256c7e2 100644 --- a/apps/ui/src/components/Phase2ConsistencyReport.tsx +++ b/apps/ui/src/components/Phase2ConsistencyReport.tsx @@ -21,7 +21,16 @@ function severityClass(severity: ValidationViolation['severity']): string { } export function Phase2ConsistencyReport({ report }: Phase2ConsistencyReportProps) { - if (report.passed && report.violations.length === 0) { + // Audit Finding #1E: dev-audience violations (currently NEW_FIELD_UNCITED + // coverage signals) stay in `report.violations` and `report.summary` so + // tests and offline analysis see them, but they are suppressed from the + // user-facing System Diagnostics surface. Header counts are computed from + // `userVisible` to prevent a "5 warnings shown" header above 0 rows. + const userVisible = report.violations.filter((v) => v.audience !== 'dev'); + const userErrorCount = userVisible.filter((v) => v.severity === 'ERROR').length; + const userWarningCount = userVisible.filter((v) => v.severity === 'WARNING').length; + + if (report.passed && userVisible.length === 0) { return (
CONSISTENCY OK @@ -29,14 +38,14 @@ export function Phase2ConsistencyReport({ report }: Phase2ConsistencyReportProps ); } - if (report.violations.length === 0) { + if (userVisible.length === 0) { return null; } return (
- {report.summary.errorCount} error(s), {report.summary.warningCount} warning(s) across{' '} + {userErrorCount} error(s), {userWarningCount} warning(s) across{' '} {report.summary.checkedFields} checked fields
@@ -55,7 +64,7 @@ export function Phase2ConsistencyReport({ report }: Phase2ConsistencyReportProps - {report.violations.map((violation, rowIndex) => ( + {userVisible.map((violation, rowIndex) => ( 14 ? `${compact.slice(0, 14)}...` : compact; } -function parseConfidenceScalar(raw: string): number | null { - const normalized = raw.trim().toLowerCase(); - if (!normalized) return null; - - if (normalized.includes("%")) { - const value = Number.parseFloat(normalized.replace("%", "")); - return Number.isFinite(value) ? value / 100 : null; - } - - const value = Number.parseFloat(normalized); - if (!Number.isFinite(value)) return null; - if (value > 1) return value / 100; - return value; -} - -function normalizeConfidenceLevel(raw: string): ConfidenceLevel { - const normalized = raw.trim().toLowerCase(); - - if (normalized.includes("high")) return "High"; - if (normalized.includes("moderate") || normalized.includes("medium") || normalized === "med") { - return "Moderate"; - } - if (normalized.includes("low")) return "Low"; - - const scalar = parseConfidenceScalar(raw); - if (scalar === null) return "Moderate"; - if (scalar >= 0.8) return "High"; - if (scalar >= 0.5) return "Moderate"; - return "Low"; -} +// Audit Finding #4: `parseConfidenceScalar` and `normalizeConfidenceLevel` +// were retired in favor of `toConfidenceBand` in +// `services/sessionMusician/confidenceBand.ts`, which subsumes both: it +// parses numeric and string inputs (including the percent strings +// parseConfidenceScalar handled) and lands on the canonical four-band +// ladder instead of the legacy three-level High/Moderate/Low enum. export function toConfidenceBadges( notes: Phase2Result["confidenceNotes"] | undefined | null, @@ -331,7 +327,7 @@ export function toConfidenceBadges( return notes.map((note) => ({ label: normalizeConfidenceFieldLabel(note.field), - level: normalizeConfidenceLevel(note.value), + band: toConfidenceBand(note.value), })); } @@ -870,21 +866,24 @@ function buildDerivedChainParameters( } } +// Audit Finding #1A: render Gemini's per-card `reason` verbatim (capitalized, +// punctuated) instead of fabricating a "{stage phrase} by {verb}" splice. The +// section header already labels each card's processing group; the old prefix +// concatenation produced ungrammatical output like "Controls bass energy by +// ensures the extreme low-end mono…" because `item.reason` is a present-tense +// clause, not a gerund. The HIGH-END "(for …)" suffix survives as an optional +// trailing parenthetical when cues are present. function buildRoleSentence(reason: string, group: ProcessingGroup, highEndCues: string[] = []): string { const base = sanitizeText(reason, 220); - const sentence = base.split(SENTENCE_BREAK_REGEX)[0] || base; - const prefixMap: Record = { - "DRUM PROCESSING": "Shapes drum impact", - "BASS PROCESSING": "Controls bass energy", - "SYNTH / MELODIC": "Supports melodic clarity", - "MID PROCESSING": "Balances the center band", - "HIGH-END DETAIL": "Refines high-end articulation", - "MASTER BUS": "Finalizes the master bus", - }; - const cueSuffix = group === "HIGH-END DETAIL" && highEndCues.length > 0 - ? ` for ${summarizeHighEndFocus(highEndCues)}` + const firstSentence = base.split(SENTENCE_BREAK_REGEX)[0] || base; + const trimmed = firstSentence.replace(/[.\s]+$/, ""); + const capitalized = trimmed.length > 0 + ? trimmed.charAt(0).toUpperCase() + trimmed.slice(1) : ""; - return `${prefixMap[group]}${cueSuffix} by ${sentence.charAt(0).toLowerCase()}${sentence.slice(1)}`; + const suffix = group === "HIGH-END DETAIL" && highEndCues.length > 0 + ? ` (for ${summarizeHighEndFocus(highEndCues)})` + : ""; + return capitalized.length > 0 ? `${capitalized}${suffix}.` : ""; } function buildProTip(group: ProcessingGroup): string { @@ -1065,16 +1064,10 @@ export function buildMixChainGroups( return compactMixChainGroups(displayGroups); } -function mapPatchRole(category: string): string { - const key = category.toLowerCase(); - if (key.includes("synth")) return "Primary tone generator"; - if (key.includes("eq")) return "Tone shaper"; - if (key.includes("dynamic")) return "Dynamic control stage"; - if (key.includes("stereo")) return "Stereo placement stage"; - if (key.includes("midi")) return "Pattern driver"; - if (key.includes("routing")) return "Signal routing stage"; - return "Texture and movement stage"; -} +// Audit Finding #1B: `mapPatchRole` was deleted. It was a 7-key category-keyed +// fallback that produced duplicated placeholder strings ("Primary tone +// generator" stamped on every SYNTHESIS card). The per-device `reason` already +// renders in `whyThisWorks`, and the category chip carries the bucket label. function groupRecommendationsByDevice(recommendations: AbletonRecommendation[]): Array<{ device: string; @@ -1140,7 +1133,6 @@ function buildStereoWidthPatchCard( id: `patch-${order}-stereo-width`, device: "Utility / Stereo Imager", category: "UTILITY", - patchRole: "Stereo image management", whyThisWorks: truncateAtSentenceBoundary( `${widthNarrative} This maps directly to measured width ${phase1.stereoWidth.toFixed(2)} and correlation ${phase1.stereoCorrelation.toFixed(2)}.`, 320, @@ -1174,11 +1166,29 @@ export function buildPatchCards( const melodyInsights = buildMelodyInsights(phase1); const recommendations = Array.isArray(phase2.abletonRecommendations) ? phase2.abletonRecommendations : []; const grouped = groupRecommendationsByDevice(recommendations); + + // Audit Finding #1B (overlap): Gemini sometimes emits the same device in + // both `mixAndMasterChain` (processing) and `abletonRecommendations` + // (patches). When that happens the Patches section visibly re-lists Mix + // Chain devices (Glue Compressor, Auto Filter, Wavetable, Utility, + // Limiter), which makes the two sections feel redundant. Filter + // case-insensitively against the chain so Patches stays distinct. + // Synthetic fallbacks (stereo width, MIDI Clip Guide) are built from + // Phase 1 only and bypass this filter intentionally. + const chainDeviceNames = new Set( + (Array.isArray(phase2.mixAndMasterChain) ? phase2.mixAndMasterChain : []) + .map((item) => item?.device?.trim?.().toLowerCase()) + .filter((name): name is string => typeof name === "string" && name.length > 0), + ); + const filteredGroups = grouped.filter( + (group) => !chainDeviceNames.has(group.device.trim().toLowerCase()), + ); + const characteristicContext = Array.isArray(phase2.detectedCharacteristics) ? phase2.detectedCharacteristics.slice(0, 2).map((item) => item.name).join(" + ") : "the detected track profile"; - const cards: PatchCardViewModel[] = grouped.map((group, index) => { + const cards: PatchCardViewModel[] = filteredGroups.map((group, index) => { const normalizedCategory = String(group.category || "EFFECTS").toUpperCase(); const midiFocused = !!melodyInsights && isMidiFocusedGroup(group.device, normalizedCategory); const primaryItem = group.items[0]; @@ -1228,7 +1238,6 @@ export function buildPatchCards( id: `patch-${index}-${group.device}`, device: group.device, category: normalizedCategory, - patchRole: midiFocused ? `${mapPatchRole(normalizedCategory)} • Transcription-driven` : mapPatchRole(normalizedCategory), whyThisWorks, deviceFamily: primaryItem?.deviceFamily, trackContext: primaryItem?.trackContext, @@ -1249,7 +1258,6 @@ export function buildPatchCards( id: `patch-${cards.length}-midi-clip-guide`, device: "MIDI Clip Guide", category: "MIDI", - patchRole: "Transcription-derived melodic scaffold", whyThisWorks: truncateAtSentenceBoundary( `Transcription-derived: ${melodyInsights.noteCount} notes covering ${melodyInsights.rangeLabel} with dominant notes ${melodyInsights.dominantNotes.slice(0, 3).join(", ") || "n/a"}. ${ melodyInsights.isDraft @@ -1315,8 +1323,12 @@ export function buildPatchGroups( // when no keyword matches; cards are already roughly ordered by intent // (drum/bass first → master last) so the position-based fallback lands // in a sensible bucket. + // Audit Finding #1B: previously included `card.patchRole`, which was a + // category-keyed fallback string that has been removed. `whyThisWorks` is + // already a per-card sentence built from Gemini's `reason`, so the + // keyword surface for `inferProcessingGroup` stays rich. const text = - `${card.device} ${card.category} ${card.patchRole} ${card.whyThisWorks}`.toLowerCase(); + `${card.device} ${card.category} ${card.whyThisWorks}`.toLowerCase(); const positionRatio = cards.length <= 1 ? 1 : index / (cards.length - 1); const group = inferProcessingGroup(text, positionRatio); grouped.get(group)?.push(card); diff --git a/apps/ui/src/components/sessionMusician/ConfidenceBandBadge.tsx b/apps/ui/src/components/sessionMusician/ConfidenceBandBadge.tsx index 64c0831a..e7a56635 100644 --- a/apps/ui/src/components/sessionMusician/ConfidenceBandBadge.tsx +++ b/apps/ui/src/components/sessionMusician/ConfidenceBandBadge.tsx @@ -26,7 +26,26 @@ const PILL_CLASSES: Record = { }; interface ConfidenceBandBadgeProps { - confidence: number; + /** + * Numeric 0-1 confidence. Optional when `band` is supplied (some callers + * have a pre-converted band from a string enum and no original scalar). + * When provided, drives the pill percent via `formatBandPillLabel`. + */ + confidence?: number; + /** + * Pre-converted band. When supplied, skips the `getConfidenceBand` round- + * trip and uses this for tone + copy. Required when `confidence` is absent. + * Useful when the source vocabulary is a string enum (HIGH/MED/LOW) that's + * been normalized via `toConfidenceBand`. + */ + band?: ConfidenceBand; + /** + * Audit Finding #4: 'compact' renders just the pill (no copy paragraph) + * for use in card corners, metric-card footers, and chip rows where the + * full variant's paragraph would blow up the layout. 'full' is the + * Session Musician panel default — pill plus hedging copy beneath. + */ + variant?: 'full' | 'compact'; /** When set, replaces the pill text entirely (used for fallback / legacy states). */ overrideLabel?: string | null; /** When set, replaces the band copy entirely. */ @@ -39,24 +58,55 @@ interface ConfidenceBandBadgeProps { export function ConfidenceBandBadge({ confidence, + band, + variant = 'full', overrideLabel, overrideCopy, overrideTone, testId, }: ConfidenceBandBadgeProps) { - const band = getConfidenceBand(confidence); - const pillText = overrideLabel ?? formatBandPillLabel(band, confidence); - const copyText = overrideCopy ?? band.copy; - const toneId = overrideTone ?? band.id; + // Dev-time guard against the both-missing case. In production we still + // default to the unreliable band on undefined confidence so we never crash + // a results render — but the guard surfaces the misuse during development. + if (confidence === undefined && band === undefined) { + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line no-console + console.warn( + '[ConfidenceBandBadge] Pass `confidence` (0-1 number), `band` (pre-converted), or both.', + ); + } + } + + const bandToUse = band ?? getConfidenceBand(confidence ?? 0); + const pillText = + overrideLabel + ?? (confidence !== undefined ? formatBandPillLabel(bandToUse, confidence) : bandToUse.label); + const copyText = overrideCopy ?? bandToUse.copy; + const toneId = overrideTone ?? bandToUse.id; const pillClass = PILL_CLASSES[toneId]; + const pill = ( + + {pillText} + + ); + + if (variant === 'compact') { + // Wrap in a span so the testid hook still has a stable anchor; consumers + // place the badge inline in card headers / footers and don't want the + // full variant's block-level div breaking their layout. + return ( + + {pill} + + ); + } + return (
- - {pillText} - + {pill}

{copyText}

diff --git a/apps/ui/src/services/phase2Validator.ts b/apps/ui/src/services/phase2Validator.ts index c357d523..144b7633 100644 --- a/apps/ui/src/services/phase2Validator.ts +++ b/apps/ui/src/services/phase2Validator.ts @@ -17,6 +17,16 @@ export interface ValidationViolation { phase2Value?: any; severity: 'ERROR' | 'WARNING'; message: string; + /** + * Audit Finding #1E: 'dev' marks an engine-coverage signal (e.g. + * NEW_FIELD_UNCITED) that helps research/tests confirm citation coverage + * but does NOT carry actionable meaning for producers. The + * `Phase2ConsistencyReport` renderer suppresses these from the user-facing + * System Diagnostics panel while keeping them in the underlying + * `ValidationReport` for tests, offline analysis, and future tooling. + * Omit the field (or set 'user') for violations that should surface. + */ + audience?: 'dev' | 'user'; } export interface ValidationReport { @@ -968,6 +978,11 @@ function validateNewFieldCoverage( type: 'NEW_FIELD_UNCITED', field: path, severity: 'WARNING', + // Audit Finding #1E: this is a coverage signal for the engine team, + // not actionable advice for the producer ("warning is benign"). Tag + // it dev-audience so the user-facing Phase2ConsistencyReport skips it + // while tests/research still see the violation in the report. + audience: 'dev', message: `Phase 1 field "${path}" is present in the measurement payload but no Phase 2 ` + `recommendation cites it. If the field is relevant to this track, Gemini should ` + diff --git a/apps/ui/src/services/sessionMusician/confidenceBand.ts b/apps/ui/src/services/sessionMusician/confidenceBand.ts index 8d2ea0ec..4ad3ce42 100644 --- a/apps/ui/src/services/sessionMusician/confidenceBand.ts +++ b/apps/ui/src/services/sessionMusician/confidenceBand.ts @@ -58,3 +58,44 @@ export function formatBandPillLabel(band: ConfidenceBand, confidence: number): s const percent = Math.max(0, Math.min(100, Math.round(safe * 100))); return `${band.label} · ${percent}%`; } + +// Audit Finding #4: every site that today expresses confidence as a string +// enum (HIGH/MED/LOW), a 0-100 percent, or a plain 0-1 scalar can route +// through this normalizer to land on the canonical four-band ladder. Each +// string enum maps to the middle of its target band so `formatBandPillLabel` +// reads as an honest percentage (HIGH → 0.9 → "Solid scaffold · 90%", +// not a band-boundary value like 0.8 → "Solid scaffold · 80%" which would +// imply a precision the string enum doesn't carry). +// +// Returns `null` when input is unparseable so callers can choose a fallback +// rather than render a misleading "Unreliable" band on bad data. +export function toConfidenceBand( + value: number | string | null | undefined, +): ConfidenceBand | null { + if (value === null || value === undefined) return null; + + if (typeof value === 'number') { + if (!Number.isFinite(value)) return null; + const clamped = + value > 1 && value <= 100 + ? value / 100 + : Math.max(0, Math.min(1, value)); + return getConfidenceBand(clamped); + } + + const trimmed = value.trim(); + if (trimmed.length === 0) return null; + const lower = trimmed.toLowerCase(); + + if (lower.includes('high')) return getConfidenceBand(0.9); + if (lower === 'med' || lower.includes('medium') || lower.includes('moderate')) { + return getConfidenceBand(0.6); + } + if (lower.includes('low')) return getConfidenceBand(0.3); + + // Fall through: try to parse as a scalar (handles "62%", "0.62", "62"). + const cleaned = trimmed.replace(/%\s*$/, ''); + const parsed = Number.parseFloat(cleaned); + if (!Number.isFinite(parsed)) return null; + return toConfidenceBand(parsed); +} diff --git a/apps/ui/tests/services/analysisResultsUi.test.ts b/apps/ui/tests/services/analysisResultsUi.test.ts index e4cbaeff..8b50cb9d 100644 --- a/apps/ui/tests/services/analysisResultsUi.test.ts +++ b/apps/ui/tests/services/analysisResultsUi.test.ts @@ -472,7 +472,11 @@ describe('AnalysisResults UI wiring', () => { expect(html).toContain('data-text-role="eyebrow"'); expect(html).toContain('>SUB BASS'); expect(html).toContain('data-text-role="body"'); - expect(html).toContain('Shapes drum impact by adds punch to drums.'); + // Audit Finding #1A: Mix Chain role text now renders Gemini's reason verbatim + // (capitalized + period) instead of fabricating a "{stage phrase} by {verb}" + // splice. The section header carries the group label. + expect(html).toContain('Adds punch to drums.'); + expect(html).not.toContain('Shapes drum impact by'); }); it('renders character pills from the first four detected characteristics with shortened names', () => { @@ -714,6 +718,98 @@ describe('AnalysisResults UI wiring', () => { expect(html).toContain('data-testid="interpretation-warnings"'); }); + // Audit Finding #1C: dropped recommendations land in `originalValue` as + // JSON-stringified AbletonRecommendation objects (see + // `_stringify_warning_value` in `server_phase2.py`). Before this fix the + // panel rendered `{"advancedTip":"…","device":"$Saturator",…}` literally. + // `formatDroppedValue` (in AnalysisResults.tsx) now parses these and shows + // a compact `device: X · parameter: Y · value: Z` summary. + it('formats dropped AbletonRecommendation JSON as device + parameter summary', () => { + const droppedRec = { + advancedTip: 'Modulate the drive macro slowly across phrases.', + category: 'EFFECTS', + device: '$Saturator', + parameter: 'drive', + phase1Fields: ['lufsIntegrated', 'truePeak'], + reason: 'Adds harmonic warmth to the master bus.', + value: '8.0', + }; + const html = renderToStaticMarkup( + React.createElement(AnalysisResults as React.ComponentType>, { + phase1: baseMeasurement, + phase2: phase2V2, + phase2SchemaVersion: 'interpretation.v2', + phase2ValidationWarnings: [ + { + code: 'DROPPED_UNKNOWN_DEVICE', + path: 'abletonRecommendations[3]', + message: 'Dropped recommendation because the device is not in the Live 12 catalog.', + originalValue: JSON.stringify(droppedRec), + }, + ], + sourceFileName: 'example.wav', + }), + ); + + // Human summary surfaces device, parameter, value (in headline-key order). + expect(html).toContain('device: $Saturator'); + expect(html).toContain('parameter: drive'); + expect(html).toContain('value: 8.0'); + // Noisy keys from the raw JSON do NOT leak into the rendered badge. + expect(html).not.toContain('advancedTip'); + expect(html).not.toContain('phase1Fields'); + // The original `{"…":"…"}` outline must not be visible to producers. + expect(html).not.toContain('"$Saturator"'); + }); + + it('leaves non-JSON originalValue strings untouched', () => { + const html = renderToStaticMarkup( + React.createElement(AnalysisResults as React.ComponentType>, { + phase1: baseMeasurement, + phase2: phase2V2, + phase2SchemaVersion: 'interpretation.v2', + phase2ValidationWarnings: [ + { + code: 'COERCED_TRACK_CONTEXT', + path: 'abletonRecommendations[0].trackContext', + message: "Coerced trackContext 'Slap Back' to 'Return:Slap Back' to match the required Return: format.", + originalValue: 'Slap Back', + coercedValue: 'Return:Slap Back', + }, + ], + sourceFileName: 'example.wav', + }), + ); + + expect(html).toContain('Slap Back'); + expect(html).toContain('Return:Slap Back'); + }); + + it('falls back to truncated raw string when JSON parse fails', () => { + const truncated = '{not valid json but starts with a brace and has more text'; + const html = renderToStaticMarkup( + React.createElement(AnalysisResults as React.ComponentType>, { + phase1: baseMeasurement, + phase2: phase2V2, + phase2SchemaVersion: 'interpretation.v2', + phase2ValidationWarnings: [ + { + code: 'DROPPED_UNKNOWN_DEVICE', + path: 'abletonRecommendations[2]', + message: 'Dropped recommendation due to malformed payload.', + originalValue: truncated, + }, + ], + sourceFileName: 'example.wav', + }), + ); + + // The function must not throw on invalid JSON; the raw input renders + // (truncated to <=80 chars) inside the small badge. This input is 56 + // characters long so it renders verbatim. + expect(html).toContain('{not valid json but starts with a brace'); + }); + it('keeps the interpretation intro flat so it matches the surrounding AI sections', () => { const html = renderToStaticMarkup( React.createElement(AnalysisResults as React.ComponentType>, { @@ -1605,9 +1701,14 @@ describe('AnalysisResults UI wiring', () => { expect(html).toContain('Track Character'); expect(html).toContain('Measured summary with explicit genre and dynamic language.'); expect(html).toContain('ASSUMED'); - expect(html).toContain('SCORE 1.88'); - expect(html).toContain('rhythm extractor confirmed'); + // Audit Finding #4: `SCORE 1.88` retired in favor of the canonical band + // pill. bpmConfidence=1.88 (> 1) clamps to 100% in formatBandPillLabel + // so the pill reads "Solid scaffold · 100%" — no CONF/SCORE leakage. + expect(html).toContain('Solid scaffold'); + expect(html).not.toContain('SCORE 1.88'); expect(html).not.toContain('CONF 188%'); + expect(html).not.toContain('188%'); + expect(html).toContain('rhythm extractor confirmed'); expect(html).toContain('257 BARS'); expect(html).not.toContain('110 BARS'); expect(html).toContain('C Major (Bridge)'); @@ -1749,6 +1850,163 @@ describe('AnalysisResults UI wiring', () => { expect(segmentRowCount).toBe(1); }); + // ───────────────────────────────────────────────────────────────────── + // Audit Finding #3: chain-of-custody citation also surfaces in the + // collapsed card header via CitationHeadline so producers see the + // measurement evidence without expanding the card. The CitationBlock + // above stays in the expanded body unchanged. + // ───────────────────────────────────────────────────────────────────── + it('renders CitationHeadline in collapsed Mix Chain card header with the primary cited field', () => { + const phase2WithCitations: Phase2Result = { + ...basePhase2, + mixAndMasterChain: [ + { + order: 1, + device: 'Drum Buss', + deviceFamily: 'NATIVE', + trackContext: 'Drum Group', + workflowStage: 'MIX', + parameter: 'Drive', + value: '25%', + reason: 'Adds punch to drums.', + phase1Fields: ['bpm', 'lufsIntegrated'], + }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: phase2WithCitations, + sourceFileName: 'example.wav', + }), + ); + + // Headline mount appears in the collapsed header (the button is part of + // renderToStaticMarkup output regardless of isOpen). + expect(html).toMatch(/data-testid="mix-chain-headline-/); + // Primary cited field is phase1Fields[0] → bpm → "Tempo" label + BPM value. + expect(html).toContain('Tempo'); + expect(html).toMatch(/\d+ BPM/); + // Arrow leads into the device h4 that follows in the title row. + expect(html).toContain('→'); + }); + + it('renders CitationHeadline in collapsed Patch card header with the primary cited field', () => { + const phase2WithCitations: Phase2Result = { + ...basePhase2, + abletonRecommendations: [ + { + device: 'Drift', + category: 'SYNTHESIS', + deviceFamily: 'NATIVE', + trackContext: 'Synth Group', + workflowStage: 'SOUND_DESIGN', + parameter: 'Position', + value: '0.42', + reason: 'Builds the supersaw lead character.', + phase1Fields: ['key', 'spectralBalance.subBass'], + }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: phase2WithCitations, + sourceFileName: 'example.wav', + }), + ); + + expect(html).toMatch(/data-testid="patch-headline-/); + expect(html).toContain('Key'); + }); + + it('renders CitationHeadline in collapsed Sonic Element card header above the summary', () => { + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: basePhase2, + sourceFileName: 'example.wav', + }), + ); + + // At least one Sonic Element card surfaces a headline (the SONIC_ELEMENT_FIELD_PATHS + // map populates phase1Fields[] for cards whose fixture fields resolve). + expect(html).toMatch(/data-testid="sonic-headline-/); + }); + + it('does not render CitationHeadline when phase1Fields is empty on a Mix Chain card', () => { + const phase2WithEmpty: Phase2Result = { + ...basePhase2, + mixAndMasterChain: [ + { + order: 1, + device: 'Drum Buss', + deviceFamily: 'NATIVE', + trackContext: 'Drum Group', + workflowStage: 'MIX', + parameter: 'Drive', + value: '25%', + reason: 'Adds punch to drums.', + // phase1Fields intentionally omitted — view-model defaults to []. + }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: phase2WithEmpty, + sourceFileName: 'example.wav', + }), + ); + + // No headline rendered for the Drum Buss card; the title row + role + // paragraph + meta badges layout stays intact. + expect(html).not.toMatch(/data-testid="mix-chain-headline-1-Drum Buss-/); + // Sanity: the card itself still renders (device name visible). + expect(html).toContain('Drum Buss'); + }); + + it('renders the confidence pill in the headline when the primary cited field has a low-confidence sibling (Rough or Unreliable)', () => { + // bpm has confidence sibling bpmConfidence; baseMeasurement sets it to + // 0.62 (Workable). To exercise the hedging path, use a fixture override. + const lowConfidencePhase1 = { + ...baseMeasurement, + bpmConfidence: 0.18, // < 0.25 → Unreliable + }; + const phase2WithCitations: Phase2Result = { + ...basePhase2, + mixAndMasterChain: [ + { + order: 1, + device: 'Drum Buss', + deviceFamily: 'NATIVE', + trackContext: 'Drum Group', + workflowStage: 'MIX', + parameter: 'Drive', + value: '25%', + reason: 'Adds punch.', + phase1Fields: ['bpm'], + }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: lowConfidencePhase1, + phase2: phase2WithCitations, + sourceFileName: 'example.wav', + }), + ); + + // Headline carries the Unreliable band pill so producers see the hedge + // without expanding the card — preserves the chain-of-custody invariant. + expect(html).toMatch(/data-testid="mix-chain-headline-.+-pill"/); + expect(html).toContain('Unreliable'); + }); + // ───────────────────────────────────────────────────────────────────── // Audit Finding #14 + #15: applied-recommendation checkbox affordance on // Mix Chain / Patches cards. The checkbox renders only when a content hash @@ -1880,4 +2138,124 @@ describe('AnalysisResults UI wiring', () => { // The card itself still renders. expect(html).toContain('Drum Buss'); }); + + // ───────────────────────────────────────────────────────────────────── + // Audit Finding #4: every confidence surface now reads in the canonical + // four-band vocabulary (Solid scaffold / Workable draft / Rough sketch / + // Unreliable). The legacy vocabularies (HIGH/MED/LOW pills, High/ + // Moderate/Low chips, CONF X% text, SCORE X.XX badges) are retired. + // ───────────────────────────────────────────────────────────────────── + it('Detected Characteristics cards render band pills instead of HIGH/MED/LOW chips', () => { + const phase2WithChars: Phase2Result = { + ...basePhase2, + detectedCharacteristics: [ + { name: 'Wide Stereo Discipline', confidence: 'HIGH', explanation: 'Controlled width.' }, + { name: 'Bass Weight', confidence: 'MED', explanation: 'Sub support moderate.' }, + { name: 'Top End Texture', confidence: 'LOW', explanation: 'Light sparkle only.' }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: phase2WithChars, + sourceFileName: 'example.wav', + }), + ); + + // Band labels surface in the rendered pill. + expect(html).toContain('Solid scaffold'); + expect(html).toContain('Workable draft'); + expect(html).toContain('Rough sketch'); + // The bespoke HIGH/MED/LOW chip styling is no longer at this site. + // (The string "HIGH" may still appear elsewhere — e.g., in + // characteristic-name chips on the Character metric card, which is a + // separate site out of scope for this PR.) + expect(html).not.toMatch(/text-success bg-success\/10 border-success\/20[^"]*"[\s>]*HIGH[<\s]/); + }); + + it('Confidence Notes chips render band pills instead of High/Moderate/Low text', () => { + const phase2WithNotes: Phase2Result = { + ...basePhase2, + confidenceNotes: [ + { field: 'Key Signature', value: '0.62', reason: 'Workable confidence' }, + { field: 'True Peak', value: 'HIGH', reason: 'Stable result' }, + ], + }; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: baseMeasurement, + phase2: phase2WithNotes, + sourceFileName: 'example.wav', + }), + ); + + // Field labels still render alongside their bands. + expect(html).toContain('Key:'); + expect(html).toContain('Peak:'); + // Band pill text replaces the legacy "High"/"Moderate"/"Low" chip text. + expect(html).toContain('Workable draft'); + expect(html).toContain('Solid scaffold'); + // The "label: Level" composed string from the legacy renderer should + // no longer appear. + expect(html).not.toMatch(/Key: Moderate { + const phase1WithKeyConf = { ...baseMeasurement, keyConfidence: 0.62 }; + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: phase1WithKeyConf, + phase2: basePhase2, + sourceFileName: 'example.wav', + }), + ); + + expect(html).toContain('Workable draft'); + expect(html).not.toMatch(/CONF\s+\d+%/); + }); + + it('Character card footer renders band pill instead of CONF X% text', () => { + const phase1WithGenre = { + ...baseMeasurement, + genreDetail: { + genre: 'Techno', + genreFamily: 'Electronic', + confidence: 0.4, // rough band + secondaryGenre: undefined, + }, + } as typeof baseMeasurement; + + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: phase1WithGenre, + phase2: basePhase2, + sourceFileName: 'example.wav', + }), + ); + + expect(html).toContain('Rough sketch'); + // Inline `CONF X%` text under the Character card metric bar must be gone. + // (The Character card characteristicPills chips are a different vocabulary + // out of scope for this PR; they don't render `CONF` text.) + expect(html).not.toMatch(/CONF\s+\d+%/); + }); + + it('BPM (Tempo) card footer renders band pill instead of SCORE X.XX badge', () => { + const phase1WithBpmConf = { ...baseMeasurement, bpmConfidence: 0.94 }; + const html = renderToStaticMarkup( + React.createElement(AnalysisResults, { + phase1: phase1WithBpmConf, + phase2: basePhase2, + sourceFileName: 'example.wav', + }), + ); + + // 0.94 → solid band ("Solid scaffold · 94%"). + expect(html).toContain('Solid scaffold'); + expect(html).toContain('94%'); + expect(html).not.toMatch(/SCORE\s+0?\.\d+/); + }); }); diff --git a/apps/ui/tests/services/analysisResultsViewModel.test.ts b/apps/ui/tests/services/analysisResultsViewModel.test.ts index f9d94942..80c9ad54 100644 --- a/apps/ui/tests/services/analysisResultsViewModel.test.ts +++ b/apps/ui/tests/services/analysisResultsViewModel.test.ts @@ -104,18 +104,38 @@ describe('analysisResultsViewModel helpers', () => { expect(output).toBe('One sentence. Two sentence. Three sentence....'); }); - it('normalizes confidence badges to friendly labels and levels', () => { + // Audit Finding #4: confidence badges now return a canonical ConfidenceBand + // (Solid / Workable / Rough / Unreliable) instead of the legacy three-level + // enum. The 3-level → 4-band mismatch surfaces here: scalar 0.5-0.79 maps + // to "workable" (used to be "Moderate"), scalar 0.25-0.49 maps to "rough" + // (used to be "Low") — an intentional refinement. + it('normalizes confidence badges to friendly labels and canonical bands', () => { const badges = toConfidenceBadges([ { field: 'Key Signature', value: '0.62', reason: 'Measured confidence' }, { field: 'Melody Transcription', value: 'LOW', reason: 'Weak melodic signal' }, { field: 'True Peak', value: 'HIGH', reason: 'Stable result' }, ]); - expect(badges).toEqual([ - { label: 'Key', level: 'Moderate' }, - { label: 'Melody', level: 'Low' }, - { label: 'Peak', level: 'High' }, + expect(badges).toHaveLength(3); + expect(badges[0].label).toBe('Key'); + expect(badges[0].band?.id).toBe('workable'); + expect(badges[1].label).toBe('Melody'); + expect(badges[1].band?.id).toBe('rough'); + expect(badges[2].label).toBe('Peak'); + expect(badges[2].band?.id).toBe('solid'); + }); + + // Audit Finding #4: unparseable values produce `band: null` so the render + // site can filter them rather than show a misleading default band. + it('returns band: null entries for unparseable confidence values', () => { + const badges = toConfidenceBadges([ + { field: 'Key Signature', value: 'completely unparseable', reason: 'whatever' }, + { field: 'True Peak', value: '0.95', reason: 'real value' }, ]); + + expect(badges).toHaveLength(2); + expect(badges[0].band).toBeNull(); + expect(badges[1].band?.id).toBe('solid'); }); it('builds arrangement timeline segments and novelty markers', () => { @@ -287,6 +307,99 @@ describe('analysisResultsViewModel helpers', () => { expect(groups.every((group) => group.name.split(' / ').length <= 3)).toBe(true); }); + // Audit Finding #1A: buildRoleSentence used to fabricate "{stage phrase} by + // {verb}" by lowercasing the first letter of Gemini's reason and prepending a + // category label. Because Gemini's `reason` is a present-tense clause, this + // produced ungrammatical splices ("Controls bass energy by ensures..."). The + // current behavior renders the reason verbatim with a capitalized first letter + // and trailing period; only the HIGH-END cue suffix survives. + it('capitalizes Gemini reason without prepending a category label', () => { + const groups = buildMixChainGroups( + measurement, + [ + { + order: 1, + device: 'EQ Eight', + trackContext: 'Bass Group', + workflowStage: 'MIX', + parameter: 'Low Cut', + value: '30 Hz', + reason: 'ensures the extreme low-end mono envelope stays tight under the kick.', + }, + ], + { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + ); + + const card = groups.flatMap((g) => g.cards).find((c) => c.device === 'EQ Eight'); + expect(card).toBeDefined(); + expect(card?.role).toBe('Ensures the extreme low-end mono envelope stays tight under the kick.'); + expect(card?.role).not.toContain(' by '); + // The old prefix labels — none of them should appear inside the role text now. + expect(card?.role).not.toContain('Controls bass energy'); + expect(card?.role).not.toContain('Shapes drum impact'); + expect(card?.role).not.toContain('Supports melodic clarity'); + }); + + it('appends high-end cue suffix when group is HIGH-END DETAIL and cues are present', () => { + const groups = buildMixChainGroups( + measurement, + [ + { + order: 1, + device: 'Auto Filter', + parameter: 'High Shelf', + value: '+2.0 dB @ 10 kHz', + reason: 'Adds sparkle to the hi-hats and synth sweeps in the top end.', + }, + ], + { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'Top-end details from hi-hats and synth sweeps.', + }, + ); + + const card = groups.find((g) => g.name === 'HIGH-END DETAIL')?.cards[0]; + expect(card).toBeDefined(); + expect(card?.role).toMatch(/^Adds sparkle to the hi-hats and synth sweeps in the top end \(for .+\)\.$/); + expect(card?.role).not.toContain(' by '); + }); + + it('omits high-end cue suffix when no cues are present', () => { + const groups = buildMixChainGroups( + measurement, + [ + { + order: 1, + device: 'Drum Buss', + parameter: 'Drive', + value: '6 dB', + reason: 'Adds bite and transient character to the drum bus.', + }, + ], + { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + ); + + const card = groups.find((g) => g.name === 'DRUM PROCESSING')?.cards[0]; + expect(card).toBeDefined(); + expect(card?.role).toBe('Adds bite and transient character to the drum bus.'); + expect(card?.role).not.toMatch(/\(for /); + }); + it('builds expanded patch cards with at least three parameters', () => { const phase2 = { trackCharacter: 'Character sentence.', @@ -412,6 +525,218 @@ describe('analysisResultsViewModel helpers', () => { expect(flatFromGroups.length).toBe(flatFromCards.length); }); + // Audit Finding #1B: PatchCardViewModel.patchRole was deleted in favor of + // letting the category chip + per-card whyThisWorks carry the bucket label + // and the actionable explanation. The category chip lives in the rendered + // card; whyThisWorks now provides the per-card uniqueness that mapPatchRole + // (a 7-key fallback) couldn't. + it('PatchCardViewModel no longer carries a patchRole field', () => { + const phase2: Phase2Result = { + trackCharacter: 'Character.', + detectedCharacteristics: [], + arrangementOverview: { summary: 'Summary', segments: [] }, + sonicElements: { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + mixAndMasterChain: [], + secretSauce: { title: 'Sauce', explanation: 'Explain', implementationSteps: ['Step'] }, + confidenceNotes: [], + abletonRecommendations: [ + { + device: 'Wavetable', + deviceFamily: 'NATIVE', + trackContext: 'Synth Group', + workflowStage: 'SOUND_DESIGN', + category: 'SYNTHESIS', + parameter: 'Position', + value: '0.42', + reason: 'Dialed for the supersaw character with high-pass at 120 Hz.', + }, + ], + } as Phase2Result; + + const cards = buildPatchCards(measurement, phase2); + expect(cards.length).toBeGreaterThan(0); + for (const card of cards) { + expect('patchRole' in card).toBe(false); + } + }); + + // Audit Finding #1B: removing card.patchRole from the buildPatchGroups + // text-concat shrinks the keyword surface for inferProcessingGroup. This + // test guards the SYNTH / MELODIC routing for a representative synth card. + it('buildPatchGroups still routes a synth card into SYNTH / MELODIC after dropping patchRole', () => { + const phase2: Phase2Result = { + trackCharacter: 'Character.', + detectedCharacteristics: [], + arrangementOverview: { summary: 'Summary', segments: [] }, + sonicElements: { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + mixAndMasterChain: [], + secretSauce: { title: 'Sauce', explanation: 'Explain', implementationSteps: ['Step'] }, + confidenceNotes: [], + abletonRecommendations: [ + { + device: 'Wavetable', + deviceFamily: 'NATIVE', + trackContext: 'Synth Group', + workflowStage: 'SOUND_DESIGN', + category: 'SYNTHESIS', + parameter: 'Position', + value: '0.42', + reason: 'Builds the supersaw lead character with detune across voices.', + }, + ], + } as Phase2Result; + + const groups = buildPatchGroups(measurement, phase2); + const synthGroup = groups.find((g) => g.name === 'SYNTH / MELODIC'); + expect(synthGroup).toBeDefined(); + expect(synthGroup?.cards.some((c) => c.device === 'Wavetable')).toBe(true); + }); + + // Audit Finding #1B (overlap): when Gemini emits the same device in both + // mixAndMasterChain and abletonRecommendations, the Patches section used to + // re-list Mix Chain devices verbatim. buildPatchCards now filters case- + // insensitively against the chain. Synthetic fallbacks (stereo width, MIDI + // Clip Guide) are built from Phase 1 only and bypass the filter. + it('buildPatchCards drops recommendations whose device also appears in mixAndMasterChain', () => { + const phase2: Phase2Result = { + trackCharacter: 'Character.', + detectedCharacteristics: [], + arrangementOverview: { summary: 'Summary', segments: [] }, + sonicElements: { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + mixAndMasterChain: [ + { + order: 1, + device: 'Glue Compressor', + deviceFamily: 'NATIVE', + trackContext: 'Master', + workflowStage: 'MASTER', + parameter: 'Threshold', + value: '-2 dB', + reason: 'Cohesive master bus glue.', + }, + ], + secretSauce: { title: 'Sauce', explanation: 'Explain', implementationSteps: ['Step'] }, + confidenceNotes: [], + abletonRecommendations: [ + { + device: 'Glue Compressor', + deviceFamily: 'NATIVE', + trackContext: 'Master', + workflowStage: 'MASTER', + category: 'DYNAMICS', + parameter: 'Ratio', + value: '2:1', + reason: 'Redundant with the chain — should not surface as a patch.', + }, + { + device: 'Wavetable', + deviceFamily: 'NATIVE', + trackContext: 'Synth Group', + workflowStage: 'SOUND_DESIGN', + category: 'SYNTHESIS', + parameter: 'Position', + value: '0.5', + reason: 'Builds the lead supersaw character.', + }, + ], + } as Phase2Result; + + const cards = buildPatchCards(measurement, phase2); + expect(cards.some((c) => c.device === 'Glue Compressor')).toBe(false); + expect(cards.some((c) => c.device === 'Wavetable')).toBe(true); + }); + + it('buildPatchCards is case-insensitive against chain device names', () => { + const phase2: Phase2Result = { + trackCharacter: 'Character.', + detectedCharacteristics: [], + arrangementOverview: { summary: 'Summary', segments: [] }, + sonicElements: { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + }, + mixAndMasterChain: [ + { + order: 1, + device: 'AUTO FILTER', + parameter: 'Cutoff', + value: '2 kHz', + reason: 'Mixchain filter sweep.', + }, + ], + secretSauce: { title: 'Sauce', explanation: 'Explain', implementationSteps: ['Step'] }, + confidenceNotes: [], + abletonRecommendations: [ + { + device: 'auto filter', + category: 'EFFECTS', + parameter: 'Resonance', + value: '0.4', + reason: 'Lowercased device name — still a duplicate.', + }, + ], + } as Phase2Result; + + const cards = buildPatchCards(measurement, phase2); + expect(cards.some((c) => /auto filter/i.test(c.device))).toBe(false); + }); + + it('synthetic stereo-width fallback card still appears even if Utility is in the chain', () => { + const phase2: Phase2Result = { + trackCharacter: 'Character.', + detectedCharacteristics: [], + arrangementOverview: { summary: 'Summary', segments: [] }, + sonicElements: { + kick: 'Kick.', + bass: 'Bass.', + melodicArp: 'Arp.', + grooveAndTiming: 'Groove.', + effectsAndTexture: 'FX.', + widthAndStereo: 'Tight low end, controlled high-end width on hi-hats.', + }, + mixAndMasterChain: [ + { + order: 1, + device: 'Utility', + parameter: 'Bass mono', + value: '120 Hz', + reason: 'Stereo discipline on the low end.', + }, + ], + secretSauce: { title: 'Sauce', explanation: 'Explain', implementationSteps: ['Step'] }, + confidenceNotes: [], + abletonRecommendations: [], + } as Phase2Result; + + const cards = buildPatchCards(measurement, phase2); + // The synthetic Utility / Stereo Imager card is built from Phase 1 only + // and must survive the chain dedup. Its device label intentionally + // includes both names so a strict equality with chain "Utility" doesn't + // match — that's the point of bypassing the filter. + expect(cards.some((c) => c.id.endsWith('stereo-width'))).toBe(true); + }); + it('builds melody insights from phase1 transcription payload', () => { const insights = buildMelodyInsights({ ...measurement, diff --git a/apps/ui/tests/services/citationHeadline.test.ts b/apps/ui/tests/services/citationHeadline.test.ts new file mode 100644 index 00000000..1cacec42 --- /dev/null +++ b/apps/ui/tests/services/citationHeadline.test.ts @@ -0,0 +1,188 @@ +/** + * Locks in the CitationHeadline primitive (audit Finding #3): the one-line + * "{label} {value} →" companion that mounts inside every collapsed Mix Chain + * / Patches / Sonic Element card header so the chain-of-custody evidence is + * visible without expanding the card. CitationBlock (the multi-row evidence + * block) keeps rendering in the expanded body — this primitive is the + * collapsed-state companion, not a replacement. + * + * Mirrors the structure of `citationBlock.test.ts` so the two primitives are + * easy to keep in sync. + */ +import { describe, expect, it } from 'vitest'; +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { CitationHeadline } from '../../src/components/CitationBlock'; +import type { Phase1Result } from '../../src/types'; + +const phase1 = { + bpm: 156.6, + bpmConfidence: 0.86, + key: 'F minor', + keyConfidence: 0.62, + timeSignature: '4/4', + lufsIntegrated: -9.3, + truePeak: -0.2, + crestFactor: 11.6, + stereoWidth: 0.42, + stereoCorrelation: 0.84, + spectralBalance: { + subBass: -0.7, + lowBass: 1.2, + highs: 1.05, + }, + kickDetail: { + fundamentalHz: 64.3, + crestFactor: 8.2, + thd: 0.29, + }, + sidechainDetail: { + pumpingRate: 4, + pumpingStrength: 0.71, + pumpingConfidence: 0.18, // < 0.25 → Unreliable band + }, +} as unknown as Phase1Result; + +describe('CitationHeadline', () => { + it('renders label + formatted value + arrow for a resolvable field', () => { + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'kickDetail.crestFactor', + showConfidenceBadge: false, + }), + ); + + expect(html).toContain('citation-headline'); + expect(html).toContain('Kick crest factor'); + expect(html).toContain('8.2 dB'); + // Arrow points into the device h4 that follows in the card title row. + expect(html).toContain('→'); + }); + + it('returns null when the cited field does not resolve in Phase 1', () => { + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'missing.field.path', + showConfidenceBadge: false, + }), + ); + + expect(html).toBe(''); + }); + + it('returns null when the cited field is an empty string', () => { + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: '', + showConfidenceBadge: false, + }), + ); + + expect(html).toBe(''); + }); + + it('renders the confidence pill when the cited field has a paired sibling', () => { + // bpm has a paired sibling `bpmConfidence` (0.86 → Solid band). + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'bpm', + }), + ); + + expect(html).toContain('citation-headline-pill'); + expect(html).toContain('Solid scaffold'); + }); + + it('omits the confidence pill when no paired sibling exists', () => { + // truePeak has no `truePeakConfidence` sibling in CONFIDENCE_PAIRS. + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'truePeak', + }), + ); + + expect(html).toContain('citation-headline'); + expect(html).not.toContain('citation-headline-pill'); + }); + + it('suppresses the confidence pill when showConfidenceBadge=false', () => { + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'bpm', + showConfidenceBadge: false, + }), + ); + + expect(html).toContain('Tempo'); + expect(html).not.toContain('citation-headline-pill'); + }); + + it('uses the unreliable band for confidence < 0.25 with hedging copy in the title attribute', () => { + // sidechainDetail.pumpingRate ↔ pumpingConfidence 0.18 → Unreliable. + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'sidechainDetail.pumpingRate', + }), + ); + + expect(html).toContain('citation-headline-pill'); + expect(html).toContain('Unreliable'); + // The hedging copy in the band's `.copy` field lands in the title attribute + // — verify the pill carries it so producers see "this is shaky" on hover. + expect(html).toMatch(/title="[^"]+"/); + }); + + it('formats the value using the same formatter as CitationBlock (spectralBalance signed dB)', () => { + // spectralBalance.highs = 1.05 → CitationBlock renders "+1.1 dB" (signed, + // 1 decimal). Headline must use the same formatCitedValue path. + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'spectralBalance.highs', + showConfidenceBadge: false, + }), + ); + + expect(html).toContain('Highs balance'); + expect(html).toContain('+1.1 dB'); + }); + + it('humanizes the label using the FIELD_LABELS map (kickDetail.crestFactor → "Kick crest factor")', () => { + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'kickDetail.crestFactor', + showConfidenceBadge: false, + }), + ); + + expect(html).toContain('Kick crest factor'); + // Not the raw dotted path. + expect(html).not.toContain('kickDetail.crestFactor'); + }); + + it('renders compact-header class hooks (text-[9px] eyebrow, text-xs tabular-nums value)', () => { + // Lightweight assertion on typography classes — the headline must read as + // "small uppercase label + larger weighted value" to signal authority + // alongside the device h4 in the card title row. + const html = renderToStaticMarkup( + React.createElement(CitationHeadline, { + phase1, + field: 'bpm', + showConfidenceBadge: false, + }), + ); + + expect(html).toContain('text-[9px]'); + expect(html).toContain('text-xs'); + expect(html).toContain('tabular-nums'); + expect(html).toContain('font-semibold'); + }); +}); diff --git a/apps/ui/tests/services/confidenceBand.test.ts b/apps/ui/tests/services/confidenceBand.test.ts index 0a735f96..9dc0e3b7 100644 --- a/apps/ui/tests/services/confidenceBand.test.ts +++ b/apps/ui/tests/services/confidenceBand.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { formatBandPillLabel, getConfidenceBand, + toConfidenceBand, } from '../../src/services/sessionMusician/confidenceBand'; describe('getConfidenceBand', () => { @@ -93,3 +94,78 @@ describe('formatBandPillLabel', () => { expect(formatBandPillLabel(band, Number.NaN)).toBe('Unreliable · 0%'); }); }); + +// Audit Finding #4: `toConfidenceBand` is the normalizer that lets every +// confidence-rendering site (Detected Characteristics HIGH/MED/LOW pills, +// Confidence Notes High/Moderate/Low chips, Key/Character/Tempo CONF and +// SCORE displays) route through the same four-band ladder. It accepts +// numeric 0-1, numeric 0-100, the string enums emitted by Gemini, and +// percent strings like "62%". Returns null for unparseable input so +// callers can choose a fallback instead of rendering a misleading band. +describe('toConfidenceBand', () => { + it('returns null for null/undefined/empty string', () => { + expect(toConfidenceBand(null)).toBeNull(); + expect(toConfidenceBand(undefined)).toBeNull(); + expect(toConfidenceBand('')).toBeNull(); + expect(toConfidenceBand(' ')).toBeNull(); + }); + + it('maps 0-1 floats via getConfidenceBand at the band boundaries', () => { + expect(toConfidenceBand(0.25)?.id).toBe('rough'); + expect(toConfidenceBand(0.5)?.id).toBe('workable'); + expect(toConfidenceBand(0.8)?.id).toBe('solid'); + expect(toConfidenceBand(0.249)?.id).toBe('unreliable'); + }); + + it('maps 0-100 integers by dividing', () => { + expect(toConfidenceBand(95)?.id).toBe('solid'); + expect(toConfidenceBand(62)?.id).toBe('workable'); + expect(toConfidenceBand(30)?.id).toBe('rough'); + expect(toConfidenceBand(10)?.id).toBe('unreliable'); + }); + + it('maps HIGH/High/high to the solid band (0.9 mid-band)', () => { + expect(toConfidenceBand('HIGH')?.id).toBe('solid'); + expect(toConfidenceBand('High')?.id).toBe('solid'); + expect(toConfidenceBand('high')?.id).toBe('solid'); + // 0.9 → 90% — solidly inside the band so the pill reads as an honest hedge. + const band = toConfidenceBand('HIGH'); + expect(band && formatBandPillLabel(band, 0.9)).toBe('Solid scaffold · 90%'); + }); + + it('maps MED/Medium/Moderate/moderate to the workable band (0.6 mid-band)', () => { + expect(toConfidenceBand('MED')?.id).toBe('workable'); + expect(toConfidenceBand('Medium')?.id).toBe('workable'); + expect(toConfidenceBand('Moderate')?.id).toBe('workable'); + expect(toConfidenceBand('moderate')?.id).toBe('workable'); + expect(toConfidenceBand('medium')?.id).toBe('workable'); + }); + + it('maps LOW/Low/low to the rough band (0.3 mid-band)', () => { + expect(toConfidenceBand('LOW')?.id).toBe('rough'); + expect(toConfidenceBand('Low')?.id).toBe('rough'); + expect(toConfidenceBand('low')?.id).toBe('rough'); + }); + + it('parses percent strings to the matching band', () => { + expect(toConfidenceBand('62%')?.id).toBe('workable'); + expect(toConfidenceBand('30%')?.id).toBe('rough'); + expect(toConfidenceBand('95%')?.id).toBe('solid'); + // Also accepts bare numeric strings. + expect(toConfidenceBand('0.62')?.id).toBe('workable'); + expect(toConfidenceBand('62')?.id).toBe('workable'); + }); + + it('returns null for NaN/Infinity and unparseable strings', () => { + expect(toConfidenceBand(Number.NaN)).toBeNull(); + expect(toConfidenceBand(Number.POSITIVE_INFINITY)).toBeNull(); + expect(toConfidenceBand(Number.NEGATIVE_INFINITY)).toBeNull(); + expect(toConfidenceBand('not a confidence')).toBeNull(); + expect(toConfidenceBand('???')).toBeNull(); + }); + + it('clamps negative numbers to the unreliable band', () => { + expect(toConfidenceBand(-0.4)?.id).toBe('unreliable'); + expect(toConfidenceBand(-50)?.id).toBe('unreliable'); + }); +}); diff --git a/apps/ui/tests/services/confidenceBandBadge.test.ts b/apps/ui/tests/services/confidenceBandBadge.test.ts index bb01e4a8..31b0966d 100644 --- a/apps/ui/tests/services/confidenceBandBadge.test.ts +++ b/apps/ui/tests/services/confidenceBandBadge.test.ts @@ -10,6 +10,10 @@ import { renderToStaticMarkup } from 'react-dom/server'; import { describe, expect, it } from 'vitest'; import { ConfidenceBandBadge } from '../../src/components/sessionMusician/ConfidenceBandBadge'; +import { + getConfidenceBand, + toConfidenceBand, +} from '../../src/services/sessionMusician/confidenceBand'; // Confidence values that fall squarely inside each band per the thresholds // in services/sessionMusician/confidenceBand.ts (≥0.80 solid, ≥0.50 workable, @@ -137,3 +141,66 @@ describe('ConfidenceBandBadge — override path used by full-mix-fallback / lega expect(html).not.toContain("Notes look reliable. Expect light cleanup in Ableton's piano roll."); }); }); + +// Audit Finding #4: compact variant omits the copy paragraph so the badge +// can sit inline in card corners and metric-card footers without breaking +// the surrounding layout. The pill itself stays identical to the full +// variant — same tone, same label, same percent. +describe('ConfidenceBandBadge — compact variant', () => { + it('compact variant omits the copy paragraph', () => { + const html = render(BAND_CONFIDENCE.solid, { variant: 'compact' }); + expect(html).toContain('Solid scaffold'); + expect(html).toContain('90%'); + // The full variant's hedging copy must NOT render in compact mode. + expect(html).not.toContain("Notes look reliable. Expect light cleanup in Ableton's piano roll."); + // The full variant wraps in `
`; compact wraps in + // an inline-flex span so it composes cleanly inline. + expect(html).not.toMatch(/]*space-y-2/); + }); + + it('compact variant with explicit band prop and no confidence renders label-only (no percent)', () => { + const band = getConfidenceBand(0.9); // solid + const html = renderToStaticMarkup( + React.createElement(ConfidenceBandBadge, { band, variant: 'compact' }), + ); + expect(html).toContain('Solid scaffold'); + // No percent rendered because no confidence was passed. + expect(html).not.toContain('%'); + }); + + it('band override prop wins over confidence-derived band for tone', () => { + // confidence=0.95 alone would yield solid (success tone). Override the + // band to the unreliable (error tone) variant via the `band` prop. + const unreliableBand = getConfidenceBand(0.1); + const html = renderToStaticMarkup( + React.createElement(ConfidenceBandBadge, { + confidence: 0.95, + band: unreliableBand, + variant: 'compact', + }), + ); + const tokens = extractColorTokens(html); + expect(tokens.text).toBe('text-error'); + // Label uses the override band; percent still comes from the + // confidence prop ("Unreliable · 95%") — exotic combination but the + // primitive needs to handle it without crashing. + expect(html).toContain('Unreliable'); + expect(html).toContain('95%'); + }); + + it('routes Gemini HIGH/MED/LOW strings through toConfidenceBand into the badge', () => { + // The real-world wiring at the Detected Characteristics site: caller + // converts the string enum, passes the resulting band. + const band = toConfidenceBand('MED'); + expect(band).not.toBeNull(); + const html = renderToStaticMarkup( + React.createElement(ConfidenceBandBadge, { + band: band ?? undefined, + variant: 'compact', + }), + ); + expect(html).toContain('Workable draft'); + const tokens = extractColorTokens(html); + expect(tokens.text).toBe('text-accent'); + }); +}); diff --git a/apps/ui/tests/services/phase2ConsistencyReport.test.ts b/apps/ui/tests/services/phase2ConsistencyReport.test.ts index 9fba4fae..13cf9df2 100644 --- a/apps/ui/tests/services/phase2ConsistencyReport.test.ts +++ b/apps/ui/tests/services/phase2ConsistencyReport.test.ts @@ -88,6 +88,94 @@ describe('Phase2ConsistencyReport', () => { expect(html).toContain('text-warning'); }); + // Audit Finding #1E: dev-audience violations (currently + // NEW_FIELD_UNCITED coverage signals) stay in the underlying + // ValidationReport for tests/research, but the user-facing System + // Diagnostics panel suppresses them from both the rendered table and the + // header counts to prevent "5 warnings shown" above an empty table. + it('suppresses dev-audience violations from the rendered table and header counts', () => { + const html = renderToStaticMarkup( + React.createElement(Phase2ConsistencyReport, { + report: buildReport({ + passed: true, + violations: [ + { + type: 'BOUNDS_VIOLATION', + field: 'segmentLufs', + severity: 'WARNING', + message: 'User-visible bounds violation worth surfacing.', + }, + { + type: 'NEW_FIELD_UNCITED', + field: 'lufsCurve.shortTerm', + severity: 'WARNING', + audience: 'dev', + message: 'Phase 1 field present but not cited — engine coverage signal.', + }, + { + type: 'NEW_FIELD_UNCITED', + field: 'rhythmDetail.tempoCurve', + severity: 'WARNING', + audience: 'dev', + message: 'Phase 1 field present but not cited — engine coverage signal.', + }, + { + type: 'NEW_FIELD_UNCITED', + field: 'chordDetail.chordTimeline', + severity: 'WARNING', + audience: 'dev', + message: 'Phase 1 field present but not cited — engine coverage signal.', + }, + ], + summary: { + errorCount: 0, + warningCount: 4, + checkedFields: 12, + }, + }), + }), + ); + + // Header counts derive from userVisible, not the summary object. + expect(html).toContain('0 error(s), 1 warning(s) across 12 checked fields'); + // Only the user-visible row renders. + expect(html).toContain('User-visible bounds violation worth surfacing.'); + // None of the dev-audience NEW_FIELD_UNCITED messages leak into the + // user-facing surface. + expect(html).not.toContain('engine coverage signal'); + expect(html).not.toContain('lufsCurve.shortTerm'); + expect(html).not.toContain('rhythmDetail.tempoCurve'); + expect(html).not.toContain('chordDetail.chordTimeline'); + }); + + it('hides the report entirely when every violation is dev-audience', () => { + const html = renderToStaticMarkup( + React.createElement(Phase2ConsistencyReport, { + report: buildReport({ + passed: true, + violations: [ + { + type: 'NEW_FIELD_UNCITED', + field: 'lufsCurve.shortTerm', + severity: 'WARNING', + audience: 'dev', + message: 'Engine coverage signal.', + }, + ], + summary: { + errorCount: 0, + warningCount: 1, + checkedFields: 5, + }, + }), + }), + ); + + // Falls through to the success rail because no user-visible violations exist. + expect(html).toContain('CONSISTENCY OK'); + expect(html).not.toContain(' { const longMessage = 'This detail message is intentionally much longer than one hundred and twenty characters so the table cell must truncate it cleanly.'; diff --git a/apps/ui/tests/services/phase2Validator.test.ts b/apps/ui/tests/services/phase2Validator.test.ts index bbb522b9..e4ebe0b4 100644 --- a/apps/ui/tests/services/phase2Validator.test.ts +++ b/apps/ui/tests/services/phase2Validator.test.ts @@ -872,6 +872,66 @@ describe('validatePhase2Consistency', () => { ); expect(uncited).toHaveLength(0); }); + + // Audit Finding #1E: NEW_FIELD_UNCITED is a coverage signal for the + // engine team ("warning is benign" copy in the message). Tag it + // dev-audience so the user-facing System Diagnostics panel can filter + // it out while tests/research keep seeing the violations. + it('NEW_FIELD_UNCITED violations are marked audience=dev', () => { + const phase1 = createBasePhase1({ + lufsCurve: { + shortTerm: [ + { t: 0.0, lufs: -16.2 }, + { t: 3.0, lufs: -15.8 }, + { t: 6.0, lufs: -14.9 }, + { t: 9.0, lufs: -13.5 }, + { t: 12.0, lufs: -11.2 }, + { t: 15.0, lufs: -9.4 }, + ], + momentary: [{ t: 0.0, lufs: -15.8 }], + }, + } as Partial); + const phase2 = createBasePhase2({ + abletonRecommendations: [ + { + device: 'EQ Eight', + category: 'EQ', + parameter: 'Low Cut', + value: '30 Hz', + reason: 'Removes rumble.', + phase1Fields: ['spectralBalance.subBass'], + }, + ], + }); + + const result = validatePhase2Consistency(phase1, phase2); + const uncited = result.violations.filter(v => v.type === 'NEW_FIELD_UNCITED'); + expect(uncited.length).toBeGreaterThan(0); + expect(uncited.every(v => v.audience === 'dev')).toBe(true); + }); + + it('non-NEW_FIELD_UNCITED violations default to user audience (audience undefined)', () => { + // Mirror the existing "EQ cutoff above spectral centroid" + // BOUNDS_VIOLATION fixture from the Numeric bounds suite so we have a + // reliable non-coverage violation to assert against. + const phase1 = createBasePhase1({ spectralDetail: { spectralCentroidMean: 2000 } }); + const phase2 = createBasePhase2({ + abletonRecommendations: [ + { + device: 'EQ Eight', + category: 'EQ', + parameter: 'High Cut', + value: '8000 Hz', + reason: 'Roll off highs', + }, + ], + }); + + const result = validatePhase2Consistency(phase1, phase2); + const nonCoverage = result.violations.filter(v => v.type !== 'NEW_FIELD_UNCITED'); + expect(nonCoverage.length).toBeGreaterThan(0); + expect(nonCoverage.every(v => v.audience === undefined)).toBe(true); + }); }); describe('pathCoversTracked helper', () => { diff --git a/docs/LAYER2_EVALUATION.md b/docs/LAYER2_EVALUATION.md new file mode 100644 index 00000000..807be94e --- /dev/null +++ b/docs/LAYER2_EVALUATION.md @@ -0,0 +1,110 @@ +# Layer 2 (Torchcrepe) Evaluation + +**Status:** Local opt-in. Synthetic self-test runs on demand; real corpus is user-side. +**Updated:** 2026-05-16 + +## What This Is + +ASA's three-layer architecture (`docs/ARCHITECTURE_STRATEGY.md`) splits work into: + +1. **Layer 1** — deterministic measurement (Essentia / DSP). +2. **Layer 2** — pitch/note translation (torchcrepe), on Demucs-separated stems. +3. **Layer 3** — interpretation (Gemini), grounded in Layer 1. + +Layer 1 had a real-track gate (`tests/fixtures/bench_tracks/`). Layer 2 did not. This doc closes that asymmetry: a dedicated bench under `tests/fixtures/transcription_tracks/`, a harness extension to `phase1_evaluation.py`, and an interactive workflow for turning DAW MIDI into ground-truth notes. + +This is **not** the same thing as `docs/POLYPHONIC_TRANSCRIPTION_SPIKE.md`. That spike covers research candidates (basic-pitch, MT3) on **polyphonic** material and is explicitly product-gated by manual usefulness gates. Layer 2 here is the **shipped** product backend (`apps/backend/analyze_transcription.py:TorchcrepeBackend`), and its target corpus is monophonic-friendly material the product is expected to handle. + +## Target Material + +8–15 short clips (10–40s each) across these categories: + +1. Monophonic vocal leads (isolated vocal stems, vocal-only renders) +2. Isolated bass lines (DI bass, bass synth stems, bass-only renders) +3. Simple monophonic synth leads (saw / square / sine, light vibrato welcome) +4. Whistled / hummed lines +5. Acoustic monophonic instruments (flute, sax, trumpet) +6. Spare slots for failure modes discovered during audit (rapid runs, glissandi, voice cracks) + +Out of scope: dense polyphony, piano stacks, full mixes. Those go in `tests/fixtures/polyphonic_tracks/` and are evaluated by the polyphonic research harness, not by Layer 2. + +## Sourcing Workflow + +1. Use audio you own or license. Nothing copyrighted is committed; the fixtures directory contains only a `README.md`. +2. Export a reference MIDI file from your DAW (Logic / Ableton / Reaper / Cubase / etc.). Quantization is acceptable — the harness's onset tolerance is ±50 ms. +3. Convert the MIDI into the `groundTruthNotes` JSON fragment: + +```bash +cd apps/backend +./venv/bin/python scripts/import_midi_to_ground_truth.py /path/to/reference.mid +``` + +4. Paste the emitted JSON array into `tests/fixtures/phase1_eval_manifest.json` under your `transcriptionTracks[].groundTruthNotes` field. + +Useful flags on the importer: + +- `--monophonic-collapse highest` — when MIDI has overlapping notes, keep the highest pitch and drop the rest. Use to flatten chordal MIDI to a melodic line. +- `--monophonic-collapse reject` (default) — exit non-zero on any overlap. Useful to catch accidental polyphony before it lands in the bench. +- `--offset-seconds N` — add `N` seconds to every onset. Use when DAW MIDI export starts before/after the corresponding audio (e.g. negative track delay, head silence trimmed after MIDI export). + +## Manifest Entry Shape + +```json +{ + "id": "monophonic_synth_lead_01", + "audioPath": "lead_01.flac", + "category": "monophonic_lead", + "description": "Saw lead, monophonic, ~120 BPM", + "analyzeFlags": ["--separate", "--transcribe"], + "groundTruthNotes": [ + { "pitchMidi": 60, "onsetSeconds": 0.50, "durationSeconds": 0.25 } + ], + "thresholds": { + "noteMetrics.f1": { "target": 0.75, "tolerance": 0.0, "direction": "min" }, + "noteMetrics.meanPitchCentsError": { "target": 0.0, "tolerance": 50.0 }, + "transcriptionDetail.averageConfidence": { "target": 0.65, "tolerance": 0.0, "direction": "min" } + } +} +``` + +Fields under `noteMetrics.*` are computed by the harness (precision / recall / f1 / meanPitchCentsError / matchedCount / missedCount / falsePositiveCount) and matched against `_evaluate_threshold`. Every other dotted path resolves against the raw analyzer payload — i.e. it can target anything `analyze.py` emits. + +The threshold-direction extension is new: + +- `{"direction": "min"}` — pass when `actual >= target - tolerance`. +- `{"direction": "max"}` — pass when `actual <= target + tolerance`. +- No `direction` key — symmetric `|actual - target| <= tolerance` (original behavior). + +## Threshold Interpretation + +| Metric | Meaning | Starting target | +|---|---|---| +| `noteMetrics.f1` | Harmonic mean of precision and recall under ±50 ms onset + ±1 semitone matching | ≥ 0.75 for monophonic; tighten per-category | +| `noteMetrics.meanPitchCentsError` | Signed mean cents error across matched pairs (multiples of 100 because pitchMidi is an integer in the analyze.py contract) | within ±50 cents (i.e. within one semitone bias) | +| `transcriptionDetail.averageConfidence` | torchcrepe periodicity averaged across notes | ≥ 0.65 as a floor sanity check | + +The matcher tolerances (`onset_window_s=0.05`, `pitch_tolerance_semitones=1`) are baked into `_match_notes` rather than per-track. If a clip needs different tolerances, add a per-track parameter later — for now, choose a clip that fits these constants or hand-edit ground truth to absorb the offset. + +The harness writes one synthetic self-test (`stepped_sine_synthetic`) every time `--include-transcription` is set, so you get at least one F1 row even before populating the corpus. The self-test uses a 0.5 target so it does not fail on torchcrepe's onset jitter for short tones. + +## Running the Bench + +```bash +# Default — synthetic-fixture gate, no transcription, always runnable +./venv/bin/python scripts/evaluate_phase1.py + +# Layer 2 opt-in — stepped-sine self-test + any registered transcription tracks +./venv/bin/python scripts/evaluate_phase1.py --include-transcription + +# Custom transcription tracks directory +./venv/bin/python scripts/evaluate_phase1.py --include-transcription \ + --transcription-tracks-dir /path/to/tracks +``` + +When `--include-transcription` is set but no audio files match the manifest entries, each missing track is reported as skipped (not failed) with a clear notice. The overall run still passes if synthetic + present-real + present-transcription checks pass. + +## What This Doesn't Cover + +- **Polyphonic accuracy** — out of scope; that's the polyphonic spike's job. +- **Sub-semitone pitch accuracy** — `pitchMidi` is integer in the analyzer contract. If finer reporting becomes a requirement, switch the matcher to use the underlying CREPE pitch-Hz output rather than the quantized MIDI integer. +- **Stem-separation quality** — the harness assumes whatever stems Demucs produced. Stem quality regressions surface as transcription regressions but the harness does not isolate them.