fix(ui): stop Phase 2 leak + Layer 2 eval tooling + docs refresh - #61
Conversation
Closes four orientation gaps in CLAUDE.md: a top-of-Architecture map that flags advisory/, experiments/, docs/, and tests/ground_truth/ as off-path; a Scripts-at-a-glance section covering scripts/ and apps/backend/scripts/; a pointer from the Phase 3 paragraph to SamplePlayback.tsx + sampleGenerationClient.ts; and a curl example plus allowlisted field paths on the csv_export.py module entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Oz <oz-agent@warp.dev>
Removes four classes of broken/placeholder content that made the Phase 2 recommendations read as engine output instead of producer-facing advice (audit finding #1). 1. Mix Chain card role text — `buildRoleSentence` used to fabricate "{stage phrase} by {verb}" by lowercasing the first letter of Gemini's `reason`. Because `reason` is a present-tense clause, every card produced ungrammatical splices like "Controls bass energy by ensures the extreme low-end mono…". Now renders the reason verbatim with a capitalized first letter and trailing period; HIGH-END cue suffix preserved as "(for …)". 2. Patch Framework `patchRole` — was a 7-key category-keyed fallback (`mapPatchRole`) that stamped duplicated placeholders ("Primary tone generator" on every SYNTHESIS card). Removed the field, the fallback, the JSX paragraph, and the contribution to the `inferProcessingGroup` text-concat. The category chip + per-card `whyThisWorks` already carry the bucket and explanation. 3. Patches/Mix Chain overlap — `buildPatchCards` now case-insensitively filters out devices that also appear in `mixAndMasterChain` so the Patches section stops re-listing chain devices. Synthetic fallbacks (Stereo Width, MIDI Clip Guide) are built from Phase 1 only and bypass the filter intentionally. 4. Interpretation Caution raw-JSON dump — the panel rendered `originalValue` verbatim, which for dropped recommendations is a JSON-dumped `AbletonRecommendation` (see `_stringify_warning_value` in `server_phase2.py`). Added `formatDroppedValue` helper that parses JSON-shaped values and renders a compact "device: X · parameter: Y · value: Z" summary. Non-JSON strings pass through; invalid JSON falls back to a truncated raw string. Also resolves the `$Saturator` symptom — Gemini's hallucinated `$`-prefixed device name now surfaces as a readable summary line instead of leaking through a raw JSON dump. 5. System Diagnostics dev-only leak — `validateNewFieldCoverage` emits "Phase 1 field 'X' is present… this warning is benign" coverage signals meant for the engine team, not producers. Added optional `audience?: 'dev' | 'user'` to `ValidationViolation`, marked `NEW_FIELD_UNCITED` as dev, and updated `Phase2ConsistencyReport` to filter dev-audience violations from the rendered table AND from the header counts (so the header doesn't read "5 warnings shown" above an empty table). The underlying `ValidationReport` still carries every violation for tests and offline analysis. Tripwire note: the backend grammar fix `_apply_phase2_grammar_fixes` in `server_phase2.py` only runs on the legacy `/api/phase2` endpoint, not the analysis-runs path the UI uses. After this change it becomes a redundant no-op against the new render shape; left untouched. Tests: 16 new unit cases across viewModel, UI rendering, validator, and consistency report. `npm run verify` passes (lint + 565 unit tests + build + 44 smoke tests). Live UI verified against saved run 855f1376… — all nine text-leak probes (`by ensures`, `by ducks`, `by generates`, `Primary tone generator`, `Texture and movement stage`, `Stereo placement stage`, raw `\$Saturator`, `this warning is benign`, "is present in the measurement payload but no Phase 2…") return false. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
# Conflicts: # apps/ui/src/components/AnalysisResults.tsx
Wires Layer 2 (torchcrepe) transcription into the Phase 1 evaluation harness behind --include-transcription, with a stepped-sine self-test that runs even when the corpus is empty and a transcriptionTracks slot in the manifest fixture. Adds new interactive tooling for the polyphonic-transcription spike: score_polyphonic_clip.py walks each clip/candidate pair and writes scorecards back into the report JSON, import_midi_to_ground_truth.py seeds ground-truth references, and summarize_midi_file emits richer diagnostic flags (note_clutter, octave_junk, dense_chords_unusable, sparse_likely_undertranscribed) to steer reviewer attention without bypassing the manual usefulness gates. Documents the corpus + scoring workflow and the distinction between the polyphonic_tracks/ research corpus and the transcription_tracks/ Layer 2 bench. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
slittycode
left a comment
slittycode
left a comment
There was a problem hiding this comment.
Verdict: COMMENT
Summary
Wires Layer 2 (torchcrepe) transcription evaluation into the Phase 1 harness behind --include-transcription, adds a score_polyphonic_clip.py interactive scorecard CLI, and extends summarize_midi_file with four new diagnostic flags (note_clutter, octave_junk, dense_chords_unusable, sparse_likely_undertranscribed). CI is green (Backend ✓, Frontend ✓). All changes are off the product path — offline eval tooling only. Phase boundaries are clean.
The PR description is misleading: the "fix(ui): stop Phase 2 leak", CLAUDE.md refresh, and LAYER2_EVALUATION.md bullets describe work already merged to main before this PR was opened. The only new content is commit 5f113b4. The unchecked "Manually confirm Phase 2 surface" checkbox is stale.
Findings
Worth considering
phase1_evaluation.py — The stepped-sine self-test passes noteMetrics.f1 >= 0.5. For a noise-free monophonic sine with clean onsets, a working torchcrepe should clear 0.75+. A 0.5 bar means the self-test passes even if the transcriber drops every other note. The intentional leniency (catching "model not running" vs. "model performing well") isn't documented at the threshold definition site, which will confuse the next person who looks at it and wonders why it's so low.
scripts/score_polyphonic_clip.py:37-40 — SCORECARD_FIELDS omits "notes", which makes _scorecard_is_complete correctly treat notes as optional. That intent isn't obvious from the code alone; a one-liner comment would close the loop.
PR description: trim the already-merged bullets and uncheck-then-delete the stale Phase 2 confirm item so the description matches what this PR actually adds.
Test results
Backend ✓ CI, Frontend ✓ CI. test_phase1_evaluation_transcription.py (333 lines) covers _match_notes, _compute_note_metrics, threshold direction semantics, harness integration via injected runner, and the MIDI import script end-to-end. test_polyphonic_evaluation.py adds 174 lines covering the four new flags and the scorecard CLI via subprocess. Fake runners prevent torchcrepe from loading in CI. Coverage is thorough.
Phase boundary check
Clean. No changes to analyze.py, any analyze_*.py module, server.py, or Phase 2/3 pipeline. No Phase 1 output fields added, removed, or renamed.
Generated by Claude Code
1 participant
Summary
--include-transcription, add an interactive scoring CLI (score_polyphonic_clip.py) for the polyphonic-transcription spike, and enrichsummarize_midi_filediagnostic flags.CLAUDE.mdwith repo-layout, scripts map, Phase 3 UI entry, and CSV recipe; document the corpus + scoring workflow inPOLYPHONIC_TRANSCRIPTION_SPIKE.md.Test plan
cd apps/backend && ./venv/bin/python -m unittest discover -s testspasses (coverstest_phase1_evaluation_transcription.pyand updatedtest_polyphonic_evaluation.py).cd apps/ui && npm run verifypasses (lint + unit + build + smoke)../venv/bin/python scripts/evaluate_phase1.py --include-transcriptionruns the stepped-sine self-test cleanly with an emptytranscription_tracks/corpus.🤖 Generated with Claude Code