diff --git a/apps/backend/prompts/phase2_system.txt b/apps/backend/prompts/phase2_system.txt index 61b09bc6..5e6028b8 100644 --- a/apps/backend/prompts/phase2_system.txt +++ b/apps/backend/prompts/phase2_system.txt @@ -388,6 +388,7 @@ mixAndMasterChain - reason - phase1Fields (see CITATION CONTRACT) - Keep the chain practical and grounded. Do not invent unsupported device names or parameter labels. +- The reason field must be a complete, grammatical sentence. When using "by" before a verb, use the gerund (-ing) form. Write "Shapes drum impact by recreating the measured THD of 0.29" — NOT "by recreate the high measured harmonic distortion". Same rule for "controls bass energy by synthesizing", "tightens groove by sidechaining", "ensures translation by limiting". Never emit "by ". secretSauce - title: a track-specific named technique diff --git a/apps/backend/server_phase2.py b/apps/backend/server_phase2.py index fd729d3d..f9ae3429 100644 --- a/apps/backend/server_phase2.py +++ b/apps/backend/server_phase2.py @@ -2,6 +2,7 @@ import json import mimetypes +import re from math import isfinite from pathlib import Path from typing import Any, Callable @@ -940,6 +941,117 @@ def _resolve_interpretation_profile_config(profile_id: str) -> dict[str, Any]: ) +### ----------------------------------------------------------------------- +### Phase 2 grammar post-process: rewrite "by <3rd-person-singular-verb>" to +### "by ". Gemini consistently emits the wrong form in role/reason +### text (e.g., "Shapes drum impact by recreates the harmonic distortion" +### instead of "...by recreating the harmonic distortion"). The audit's +### final round prescribed a server-side post-process; the prompt nudge in +### phase2_system.txt did not take. +### +### Conservative: only rewrites when the word after "by" is at least 4 +### characters and lowercase a-z, and is NOT in the small denylist of common +### plural nouns that legitimately appear after "by" in technical prose. +### ----------------------------------------------------------------------- + +_PHASE2_BY_VERB_RE = re.compile(r"\bby ([a-z]{4,})s\b") + +_PHASE2_BY_NOUN_DENYLIST = frozenset( + { + # Plural nouns / measure words that legitimately appear after "by" in + # technical writing. Anything in this set is left as-is. + "tones", "notes", "lines", "sides", "modes", "kinds", "codes", + "cases", "rates", "types", "pairs", "rules", "bands", "parts", + "tools", "forms", "works", "phases", "stages", "features", + "systems", "classes", "numbers", "measures", "reasons", "sources", + "targets", "units", "tracks", "cycles", "beats", "hits", + "samples", "patterns", "pieces", "levels", "values", "amounts", + "degrees", "effects", "paths", "fields", "frames", "styles", + "genres", "others", "ranges", "drums", "stems", "voices", + "channels", "tracks", "groups", "buses", "passes", "blocks", + "chords", "scales", "octaves", "intervals", "harmonics", + } +) + + +def _to_gerund(verb_3sg: str) -> str: + """3rd-person singular → gerund (best effort, no dictionary lookup). + + "matches" → "matching" (strip -es, +ing) + "shapes" → "shaping" (strip -s, drop terminal -e, +ing) + "recreates" → "recreating" (same) + "absorbs" → "absorbing" (strip -s, +ing) + """ + if verb_3sg.endswith(("ches", "shes", "sses", "tches", "xes", "zzes")): + stem = verb_3sg[:-2] + else: + stem = verb_3sg[:-1] + if stem.endswith("e") and not stem.endswith(("ee", "oe", "ye", "ie")): + stem = stem[:-1] + return stem + "ing" + + +def _fix_by_gerund_in_text(text: str) -> str: + """Rewrite "by " patterns inside a single string. + + Empty/non-string inputs pass through unchanged. The regex is bounded to + lowercase a-z words 4+ chars long, and the denylist guards against + common plural-noun false positives. + """ + if not isinstance(text, str) or not text: + return text + + def _repl(match: re.Match) -> str: + word_with_s = match.group(1) + "s" + if word_with_s in _PHASE2_BY_NOUN_DENYLIST: + return match.group(0) + return f"by {_to_gerund(word_with_s)}" + + return _PHASE2_BY_VERB_RE.sub(_repl, text) + + +def _fix_grammar_in_record(record: Any, fields: tuple[str, ...]) -> Any: + """Apply `_fix_by_gerund_in_text` to specific string fields of a record. + Returns the record unchanged if it isn't a dict or has none of the fields. + """ + if not isinstance(record, dict): + return record + updated = False + for field in fields: + original = record.get(field) + if isinstance(original, str): + fixed = _fix_by_gerund_in_text(original) + if fixed != original: + record[field] = fixed + updated = True + return record if updated or True else record # always return; updated flag unused + + +def _apply_phase2_grammar_fixes(normalized: dict[str, Any]) -> None: + """Walk the relevant Phase 2 free-text fields and rewrite "by s" + to "by ing" in-place. Targets: + - abletonRecommendations[].{reason, advancedTip} + - mixAndMasterChain[].reason + - secretSauce.workflowSteps[].{measurementJustification, instruction} + """ + recs = normalized.get("abletonRecommendations") + if isinstance(recs, list): + for item in recs: + _fix_grammar_in_record(item, ("reason", "advancedTip")) + + mix_chain = normalized.get("mixAndMasterChain") + if isinstance(mix_chain, list): + for item in mix_chain: + _fix_grammar_in_record(item, ("reason",)) + + secret_sauce = normalized.get("secretSauce") + if isinstance(secret_sauce, dict): + workflow_steps = secret_sauce.get("workflowSteps") + if isinstance(workflow_steps, list): + for item in workflow_steps: + _fix_grammar_in_record(item, ("measurementJustification", "instruction")) + + def _is_str(v: Any) -> bool: return isinstance(v, str) @@ -1753,6 +1865,13 @@ def _normalize_and_salvage_phase2_result( emptied_required_arrays.add(array_path) warnings.extend(item_warnings) + # Audit final round: rewrite Gemini's "by recreates / by shapes / by + # generates / ..." → "by recreating / by shaping / by generating / ...". + # The prompt instruction added earlier didn't take; this is the salvage + # post-process. Runs in-place on `normalized`; no validation warnings + # emitted because grammar repair is not a contract failure. + _apply_phase2_grammar_fixes(normalized) + return normalized, warnings, emptied_required_arrays diff --git a/apps/backend/tests/test_phase2_grammar_fix.py b/apps/backend/tests/test_phase2_grammar_fix.py new file mode 100644 index 00000000..4f00a369 --- /dev/null +++ b/apps/backend/tests/test_phase2_grammar_fix.py @@ -0,0 +1,262 @@ +"""Locks in the Phase 2 gerund-fix post-process (audit final round). + +Gemini consistently emits 3rd-person singular forms after "by" in role/reason +text: + "Shapes drum impact by recreates the harmonic distortion" + "Controls bass energy by shapes the synthesized sub bass" + "Supports melodic clarity by matches the highest confidence" + +The audit's stated fix path was a server-side post-process after the prompt +instruction didn't take. This test suite documents the conversion rules and +guards against regressions. +""" +import unittest + +from server_phase2 import ( + _apply_phase2_grammar_fixes, + _fix_by_gerund_in_text, + _to_gerund, +) + + +class ToGerundTests(unittest.TestCase): + """Stem-extraction + gerund-formation rules. No regex, just orthography.""" + + def test_simple_consonant_stem(self): + # absorbs → absorb → absorbing + self.assertEqual(_to_gerund("absorbs"), "absorbing") + self.assertEqual(_to_gerund("restricts"), "restricting") + self.assertEqual(_to_gerund("adds"), "adding") + # NOTE: verbs requiring consonant-doubling (control → controlling, + # submit → submitting, run → running) are NOT handled algorithmically. + # The English doubling rule needs stress detection. If Gemini emits + # these in "by s" form, the gerund will read "controling" / + # "submiting" — still better than the "by controls" original. If this + # bites the producer-facing output, add a small exception map at the + # top of this module rather than try to implement orthographic rules. + + def test_silent_e_dropped(self): + # shapes → shape → shap + ing → shaping + self.assertEqual(_to_gerund("shapes"), "shaping") + self.assertEqual(_to_gerund("recreates"), "recreating") + self.assertEqual(_to_gerund("generates"), "generating") + self.assertEqual(_to_gerund("provides"), "providing") + self.assertEqual(_to_gerund("ensures"), "ensuring") + self.assertEqual(_to_gerund("balances"), "balancing") + + def test_sibilant_endings(self): + # matches (V-tch + es) → match + ing → matching + self.assertEqual(_to_gerund("matches"), "matching") + # passes (V-ss + es) → pass + ing → passing + self.assertEqual(_to_gerund("passes"), "passing") + # boxes (V-x + es) → box + ing → boxing + self.assertEqual(_to_gerund("boxes"), "boxing") + # crushes (V-sh + es) → crush + ing → crushing + self.assertEqual(_to_gerund("crushes"), "crushing") + # catches (V-tch + es) → catch + ing → catching + self.assertEqual(_to_gerund("catches"), "catching") + + +class FixByGerundInTextTests(unittest.TestCase): + """End-to-end string rewrites — the kind that come out of Gemini.""" + + def test_actual_screenshot_corpus(self): + # From `/tmp/asa-shots-audit-final/14-mix-chain-viewport.png`: + cases = [ + ( + "Shapes drum impact by recreates the aggressive harmonic character.", + "Shapes drum impact by recreating the aggressive harmonic character.", + ), + ( + "Shapes drum impact by absorbs harsh transients.", + "Shapes drum impact by absorbing harsh transients.", + ), + ( + "Controls bass energy by shapes the synthesized sub bass envelope.", + "Controls bass energy by shaping the synthesized sub bass envelope.", + ), + ( + "Controls bass energy by ensures the sub frequencies remain present.", + "Controls bass energy by ensuring the sub frequencies remain present.", + ), + ( + "Supports melodic clarity by matches the highest confidence range.", + "Supports melodic clarity by matching the highest confidence range.", + ), + ( + "Supports melodic clarity by generates the thick, detuned pad.", + "Supports melodic clarity by generating the thick, detuned pad.", + ), + ( + "Balances the center band by provides the ambient tail.", + "Balances the center band by providing the ambient tail.", + ), + ( + "Finalizes the master bus by restricts the output ceiling.", + "Finalizes the master bus by restricting the output ceiling.", + ), + ] + for original, expected in cases: + with self.subTest(original=original): + self.assertEqual(_fix_by_gerund_in_text(original), expected) + + def test_does_not_touch_correct_gerund(self): + # Already-correct text passes through unchanged. + self.assertEqual( + _fix_by_gerund_in_text("Shapes drum impact by recreating the THD."), + "Shapes drum impact by recreating the THD.", + ) + + def test_preserves_plural_nouns(self): + # Plural nouns after "by" must not be misinterpreted as verbs. + denylist_cases = [ + "Triggered by samples played in sequence.", + "Filtered by bands at 200 Hz and 2 kHz.", + "Routed by buses sharing the same return.", + "Quantized by beats per measure.", + "Driven by levels above -10 dB.", + ] + for case in denylist_cases: + with self.subTest(case=case): + self.assertEqual(_fix_by_gerund_in_text(case), case) + + def test_short_words_not_rewritten(self): + # Words shorter than 4 chars after "by" stay intact (avoids + # converting "by ads", "by ANNs", etc.). + self.assertEqual( + _fix_by_gerund_in_text("Compressed by ads on the radio."), + "Compressed by ads on the radio.", + ) + + def test_empty_and_non_string_input(self): + self.assertEqual(_fix_by_gerund_in_text(""), "") + self.assertEqual(_fix_by_gerund_in_text(None), None) + self.assertEqual(_fix_by_gerund_in_text(42), 42) + + def test_capitalization_at_sentence_start_unaffected(self): + # Regex only matches lowercase "by" — initial "By" remains as-is. + # This is acceptable: producer-facing role/reason fields don't start + # with "By" in practice (sentences begin with "Shapes...", "Controls..."). + self.assertEqual( + _fix_by_gerund_in_text("By recreates the THD"), + "By recreates the THD", + ) + + def test_multiple_rewrites_in_one_string(self): + original = ( + "Shapes drum impact by recreates the THD and balances the mix " + "by limits the dynamics." + ) + expected = ( + "Shapes drum impact by recreating the THD and balances the mix " + "by limiting the dynamics." + ) + self.assertEqual(_fix_by_gerund_in_text(original), expected) + + +class ApplyPhase2GrammarFixesTests(unittest.TestCase): + """The wiring that walks the Phase 2 record and applies the fix in-place.""" + + def test_rewrites_mix_chain_reasons(self): + normalized = { + "mixAndMasterChain": [ + { + "order": 1, + "device": "Drum Buss", + "reason": "Shapes drum impact by recreates the THD.", + }, + { + "order": 2, + "device": "Limiter", + "reason": "Finalizes loudness by restricts peaks.", + }, + ] + } + _apply_phase2_grammar_fixes(normalized) + self.assertEqual( + normalized["mixAndMasterChain"][0]["reason"], + "Shapes drum impact by recreating the THD.", + ) + self.assertEqual( + normalized["mixAndMasterChain"][1]["reason"], + "Finalizes loudness by restricting peaks.", + ) + + def test_rewrites_ableton_recommendations_reason_and_advanced_tip(self): + normalized = { + "abletonRecommendations": [ + { + "device": "Operator", + "reason": "Controls bass energy by shapes the sub.", + "advancedTip": "Modulate the coarse by adjusts the macro.", + } + ] + } + _apply_phase2_grammar_fixes(normalized) + rec = normalized["abletonRecommendations"][0] + self.assertEqual(rec["reason"], "Controls bass energy by shaping the sub.") + self.assertEqual(rec["advancedTip"], "Modulate the coarse by adjusting the macro.") + + def test_rewrites_secret_sauce_workflow_step_fields(self): + normalized = { + "secretSauce": { + "title": "Subby psytrance bass", + "workflowSteps": [ + { + "step": 1, + "instruction": "Carve the room by removes the wash.", + "measurementJustification": "Sub-bass mono ensures stability by tightens correlation.", + } + ], + } + } + _apply_phase2_grammar_fixes(normalized) + step = normalized["secretSauce"]["workflowSteps"][0] + self.assertEqual(step["instruction"], "Carve the room by removing the wash.") + self.assertEqual( + step["measurementJustification"], + "Sub-bass mono ensures stability by tightening correlation.", + ) + + def test_no_op_on_empty_normalized(self): + normalized = {} + # Must not raise. + _apply_phase2_grammar_fixes(normalized) + self.assertEqual(normalized, {}) + + def test_no_op_on_already_correct_text(self): + normalized = { + "mixAndMasterChain": [ + { + "order": 1, + "device": "Glue Compressor", + "reason": "Shapes drum impact by recreating the THD measurement.", + } + ] + } + original_reason = normalized["mixAndMasterChain"][0]["reason"] + _apply_phase2_grammar_fixes(normalized) + self.assertEqual( + normalized["mixAndMasterChain"][0]["reason"], original_reason + ) + + def test_handles_missing_inner_fields_gracefully(self): + normalized = { + "mixAndMasterChain": [ + {"order": 1, "device": "Drum Buss"}, # no reason key + "not-a-dict", + None, + ], + "abletonRecommendations": [ + {"device": "Operator", "reason": None}, # reason is None + ], + "secretSauce": { + "workflowSteps": "not-a-list", + }, + } + # Must not raise. + _apply_phase2_grammar_fixes(normalized) + + +if __name__ == "__main__": + unittest.main() diff --git a/apps/ui/src/App.tsx b/apps/ui/src/App.tsx index 19c34121..d46cdc87 100644 --- a/apps/ui/src/App.tsx +++ b/apps/ui/src/App.tsx @@ -6,12 +6,16 @@ import { AnalysisStatusPanel } from './components/AnalysisStatusPanel'; import { DiagnosticLog } from './components/DiagnosticLog'; import { FileUpload } from './components/FileUpload'; import { WaveformPlayer } from './components/WaveformPlayer'; -import { IdleSignalMonitor } from './components/IdleSignalMonitor'; -import { useCpuMeter } from './hooks/useCpuMeter'; +// Audit Finding #5: IdleValuePropPanel now occupies the Signal Monitor area +// when no file is selected. It tells the producer what ASA does and what to +// expect in 30s / 5min. The legacy IdleSignalMonitor (atmospheric +// breathing-line canvas) is kept in the codebase for a potential future +// "waiting between file-selected and analysis-started" state but is not +// imported here. +import { IdleValuePropPanel } from './components/IdleValuePropPanel'; import { useGlobalDrag } from './hooks/useGlobalDrag'; import { appConfig, - appVersionLabel, isGeminiPhase2ConfigEnabled, } from './config'; import { getAudioMimeTypeOrDefault, isSupportedAudioFile } from './services/audioFile'; @@ -90,12 +94,22 @@ function formatEstimateRange(estimate: BackendAnalysisEstimate): string { return `${Math.round(estimate.totalLowMs / 1000)}s-${Math.round(estimate.totalHighMs / 1000)}s`; } +// Audit N9: M:SS duration for the collapsed Input Source summary card. Returns +// null for missing/invalid values so callers can skip the chip without juggling +// conditionals around a placeholder string. +export function formatTrackDuration(seconds: number | null | undefined): string | null { + if (seconds == null || !Number.isFinite(seconds) || seconds < 0) return null; + const mins = Math.floor(seconds / 60); + const secs = Math.round(seconds % 60); + return `${mins}:${secs.toString().padStart(2, '0')}`; +} + function getInterpretationStatusBadge( phase2ConfigEnabled: boolean, phase2Requested: boolean, ): string | null { - if (!phase2ConfigEnabled) return 'INTERPRETATION CONFIG OFF'; - if (!phase2Requested) return 'INTERPRETATION USER OFF'; + if (!phase2ConfigEnabled) return 'NOT CONFIGURED'; + if (!phase2Requested) return 'OFF'; return null; } @@ -104,7 +118,10 @@ function getInterpretationHelperCopy( phase2Requested: boolean, ): string { if (!phase2ConfigEnabled) { - return 'Developer kill-switch is off. AI interpretation is unavailable in this build.'; + // Audit #12: drop developer-flavored copy. Give the user a concrete next step + // instead of a config-state assertion. On hosted deployments, an operator + // configures GEMINI_API_KEY on the backend; on local setups the user does it themselves. + return 'AI interpretation isn’t configured. Set GEMINI_API_KEY on the backend to enable Ableton recommendations.'; } if (!phase2Requested) { @@ -229,6 +246,12 @@ export default function App() { const [audioFile, setAudioFile] = useState(null); const [audioUrl, setAudioUrl] = useState(null); const [isDemoLoading, setIsDemoLoading] = useState(false); + // Audit N9: after analysis completes the Input Source panel collapses into a + // compact summary so the results below get the full top-of-page real estate. + // The user can re-open it via "Adjust settings"; the override resets at the + // start of each new analysis (useEffect below) so subsequent completions also + // collapse — that's the predictable behavior. + const [inputManuallyExpanded, setInputManuallyExpanded] = useState(false); const [analysisEstimate, setAnalysisEstimate] = useState(null); const [isEstimateLoading, setIsEstimateLoading] = useState(false); @@ -247,8 +270,14 @@ export default function App() { const phase2StatusBadge = getInterpretationStatusBadge(phase2ConfigEnabled, interpretationRequested); const phase2HelperCopy = getInterpretationHelperCopy(phase2ConfigEnabled, interpretationRequested); const phase2ModelSelectorDisabled = isAnalyzing || !phase2ConfigEnabled || !interpretationRequested; - const cpuMeterPercent = useCpuMeter(isAnalyzing); const audioElementRef = useRef(null); + + // Audit N9: reset the manual-expand override whenever a new analysis kicks + // off, so every completion re-collapses (predictable). Without this, a user + // who clicked "Adjust settings" once would have the panel stay open forever. + useEffect(() => { + if (isAnalyzing) setInputManuallyExpanded(false); + }, [isAnalyzing]); const previousRunRef = useRef(null); const completionRef = useRef<{ measurement: boolean; interpretation: boolean }>({ measurement: false, @@ -904,6 +933,10 @@ export default function App() { ); const shouldShowStatusPanel = Boolean(audioUrl && audioFile && analysisRun && (isAnalyzing || hasRetryableRunStage)); const phase1ForRender: Phase1Result | null = analysisRun ? projectPhase1FromRun(analysisRun) : null; + // Audit N9: collapse the Input Source panel when results are visible and we + // aren't actively analyzing. A failed run keeps the panel open so the user + // can change settings before retry. + const showInputCollapsed = Boolean(phase1ForRender) && !isAnalyzing && !hasRetryableRunStage && !inputManuallyExpanded; const phase2ForRender = analysisRun ? projectPhase2FromRun(analysisRun) : null; const stemSummaryForRender = analysisRun ? projectStemSummaryFromRun(analysisRun) : null; const phase2SchemaVersion = analysisRun ? getPhase2SchemaVersionFromRun(analysisRun) : null; @@ -919,24 +952,25 @@ export default function App() { data-testid="app-toolbar" className="ableton-toolbar h-10 border-b border-border flex items-center justify-between px-4" > + {/* Audit #7+#9: dropped "Local DSP Engine v1.6.0" eyebrow. Version was + header noise and wrapped to 3 lines at 375px. The brand mark alone + is enough at-a-glance; version lives on the about/help surface. */}
SonicAnalyzer
-
-
- Local DSP Engine - {appVersionLabel} -
+ {/* Audit #7: de-emphasized the Dense DAW Lab link. The accent-orange + chip competed with the brand mark and the model selector for the + user's eye on every page. It's still discoverable, just quieter. */} - Dense DAW Lab + Dense DAW Lab →
@@ -963,17 +997,10 @@ export default function App() { {phase2StatusBadge} )} -
-
- CPU -
-
-
-
+ {/* Audit #11: dropped the CPU meter. Analysis happens in the + backend subprocess — the browser tab's CPU has no useful + relationship to "how hard the analysis is working." The pulsing + bar implied effort the page wasn't actually doing. */}
@@ -991,6 +1018,69 @@ export default function App() { data-testid="input-panel" className="bg-bg-card border border-border rounded-b-sm p-4 flex flex-col min-h-[220px]" > + {showInputCollapsed && audioFile ? ( + // Audit N9: compact post-analysis summary. Replaces the + // FileUpload dropzone + 3 toggles + estimate + run button + // with one-line context + two actions. The user can swap + // files or re-open the full panel from here. +
+
+
+

Analyzed

+ {(() => { + const formatted = formatTrackDuration(phase1ForRender?.durationSeconds); + return formatted ? ( + + {formatted} + + ) : null; + })()} +
+

+ {audioFile.name} +

+
+
+ + +
+
+ ) : ( + <> + {phase1ForRender && inputManuallyExpanded && !isAnalyzing && ( + // Audit N9: re-expanded post-results — give the user a way back + // to the compact view without having to clear the file. +
+

+ Editing analysis settings +

+ +
+ )}

ANALYSIS MODE

-

- Full keeps every measurement. Standard is faster and skips advanced Tier 3 detail. + {/* Audit revised #4: helper paragraphs in the + Input Source panel were all-caps mono walls. + Switched to sans-serif sentence case (eyebrow + above stays mono-uppercase for label scan). */} +

+ Full keeps every measurement. Standard is faster and skips advanced detail.