diff --git a/audits/nightly-2026-05-19.md b/audits/nightly-2026-05-19.md new file mode 100644 index 00000000..691d6c0f --- /dev/null +++ b/audits/nightly-2026-05-19.md @@ -0,0 +1,226 @@ +## Nightly audit — 2026-05-19 + +Automated audit of the ASA codebase against the project rule that **Phase 1 DSP +measurements are ground truth** and Phase 2 / Phase 3 must never override or +write back to Phase 1 values. + +Run from worktree `claude/quizzical-jemison-22a55f` against `origin/main` +(`adb715ef`). Backend reachable on `127.0.0.1:8100` during the smoke run +(probed via `/openapi.json`); no separate backend was started by the +Playwright `webServer` block. + +### Headline counts + + 1. Unit tests: **428 total, 426 pass, 0 fail, 2 skipped** (vitest) + 2. Playwright: **46 total, 37 pass, 8 unexpected fail, 1 skipped, 0 flaky** + 3. Phase boundary violations: **0 confirmed**, **0 new review items** + +Total issues: **8** (0 unit, 8 e2e timeouts, 0 boundary). + +The three decision-gate suite failures noted in the 2026-05-16 audit no longer +appear — commit [`e9e97e1e`](https://github.com/slittycode/ableton-sonic-analyzer/commit/e9e97e1e) +("decision-gate skip, gerund exceptions, …") added skip handling so they +fall under the 2 skipped count instead of failing. The `estimatePlr` boundary +review item from 2026-05-16 §4 was resolved by commit +[`adb715ef`](https://github.com/slittycode/ableton-sonic-analyzer/commit/adb715ef) +("close estimatePlr audit item") with an inline comment documenting why the +fallback is not a competing source of truth — verified read-only in this +audit. + +--- + +### 1. Unit tests + +All 426 active vitest suites pass. The 2 skipped are the two decision-gate +multi-model comparators that are correctly gated on the presence of +`/tmp/decision_gate_*.json` snapshots. + +### 2. Playwright failures (8) + +All eight failures are timeouts on tests that share the same Vite dev server +(`webServer: reuseExistingServer: true`, `npm run dev:local`). The pass/fail +distribution per spec file: + + 1. `error-states.spec.ts` — 6 pass / 2 fail + 2. `file-validation.spec.ts` — 7 pass / 1 fail + 3. `phase2-degradation.spec.ts` — 4 pass / 0 fail + 4. `responsive-layout.spec.ts` — 5 pass / 0 fail + 5. `theme-shell.spec.ts` — 1 pass / 0 fail + 6. `ui-details.spec.ts` — 10 pass / 2 fail + 7. `upload-estimate-phase1.spec.ts` — 0 pass / 1 fail + 8. `upload-phase1-live.spec.ts` — 0 pass / 1 fail + 9. `upload-phase1-midi.spec.ts` — 3 pass / 1 fail + 10. `upload-phase1.spec.ts` — 1 pass / 0 fail + 11. `upload-phase2-live-gemini.spec.ts` — 1 skipped (gated on `RUN_GEMINI_LIVE_SMOKE`) + +Failing tests: + + 1. [error-states.spec.ts:339](apps/ui/tests/smoke/error-states.spec.ts:339) + "error banner dismiss button removes the error" — 90 s timeout. Fully + mocked path (`stubEstimateRoute` + `page.route('**/api/analysis-runs')` + fulfilled with HTTP 500). + 2. [error-states.spec.ts:464](apps/ui/tests/smoke/error-states.spec.ts:464) + "stop monitoring during interpretation preserves completed measurement + without showing an error banner" — 90 s timeout. Fully mocked. + 3. [file-validation.spec.ts:341](apps/ui/tests/smoke/file-validation.spec.ts:341) + "load demo track fetches demo.mp3 into the upload flow" — timeout + "while setting up page". Stubs `**/demo.mp3` and backend routes. + 4. [ui-details.spec.ts:359](apps/ui/tests/smoke/ui-details.spec.ts:359) + "top metric cards show TEMPO, KEY SIG, METER, CHARACTER after analysis" + — 90 s timeout. Mocked via `stubRoutes(page)`. + 5. [ui-details.spec.ts:395](apps/ui/tests/smoke/ui-details.spec.ts:395) + "header shows SonicAnalyzer brand" — 90 s timeout. The body is just + `page.goto('/', { waitUntil: 'networkidle' })` and + `expect(page.getByText('SonicAnalyzer')).toBeVisible()`; no upload, no + stubs, no backend dependency. + 6. [upload-estimate-phase1.spec.ts:13](apps/ui/tests/smoke/upload-estimate-phase1.spec.ts:13) + "upload shows estimate and local DSP processing copy before phase1 + completes" — 90 s timeout. **Same test that failed in the 2026-05-16 + audit; untouched since.** + 7. [upload-phase1-live.spec.ts:44](apps/ui/tests/smoke/upload-phase1-live.spec.ts:44) + "live backend phase1 renders results without connectivity errors" — + `page.waitForResponse` exceeded 35 s waiting for + `POST /api/analysis-runs`. The skip-gate + (`backendSupportsPhase1Routes`) did not skip the test, so a backend + was reachable but did not complete the silence.wav analysis in time. + This test is the only one in the suite that needs a real backend. + 8. [upload-phase1-midi.spec.ts:84](apps/ui/tests/smoke/upload-phase1-midi.spec.ts:84) + "phase1 dual-source session musician panel renders both blocks + simultaneously" — 90 s timeout. Mocked via `stubGeminiPhase2` and + explicit `**/api/analysis-runs/*` route stubs. + +**Suspected cause:** the seven mocked-path failures share `webServer.reuseExistingServer: true` ++ `await page.goto('/', { waitUntil: 'networkidle' })`. Under sequential load +against a single shared Vite dev server, the `networkidle` condition is known +to hang when Vite's HMR client keeps a long-poll connection open or when one +of the lazy-loaded chunks (e.g. `AnalysisResults.tsx`) is still being built on +first hit. Test #5 ("header shows brand") is the simplest possible page-load +assertion in the suite and still times out, which points at server/route +stability rather than at any production code change. Failures #1–6 and #8 all +do the same `page.goto('/', { waitUntil: 'networkidle' })` step. The seven +counterparts in the same files (e.g. the other 10 passing tests in +`ui-details.spec.ts`) succeed, so this is intermittent. + +The eighth failure (#7, `upload-phase1-live.spec.ts`) is independent — it +needs a backend that completes a real analysis pass on silence.wav within +35 s, which the host did not meet at run time. + +**Suggested next steps (do not auto-fix per audit policy):** + + 1. Swap `waitUntil: 'networkidle'` for `'domcontentloaded'` in the + smoke specs, or add an explicit selector wait, to remove Vite-HMR + long-poll sensitivity. The five `networkidle` waits are at + [error-states.spec.ts:340/465](apps/ui/tests/smoke/error-states.spec.ts:340), + [file-validation.spec.ts:350](apps/ui/tests/smoke/file-validation.spec.ts:350), + [ui-details.spec.ts:360/396](apps/ui/tests/smoke/ui-details.spec.ts:360), + [upload-estimate-phase1.spec.ts](apps/ui/tests/smoke/upload-estimate-phase1.spec.ts), + [upload-phase1-midi.spec.ts:84](apps/ui/tests/smoke/upload-phase1-midi.spec.ts:84) — confirm + against each file. + 2. Make `upload-phase1-live.spec.ts:44` either (a) gate on a + wall-clock budget appropriate for the local hardware, (b) inject a + pre-analyzed run id via the backend testing surface and skip the + analyze step, or (c) move it out of the default `test:smoke` glob + into the same live-only config as `upload-phase2-live-gemini.spec.ts`. + 3. Re-check [upload-estimate-phase1.spec.ts:13](apps/ui/tests/smoke/upload-estimate-phase1.spec.ts:13) + — it has now failed in two consecutive nightly runs (2026-05-16 and + 2026-05-19) with the same symptom. The 2026-05-16 audit suggested + looking at `git blame` on + [analysisRunsClient.ts](apps/ui/src/services/analysisRunsClient.ts) / + [analyzer.ts](apps/ui/src/services/analyzer.ts) for an interpretation-fetch + gating change; the recommendation still stands. + +### 3. Phase boundary audit — code paths checked + +Phase 2 / Phase 3 code surveyed for mutation, recomputation, or write-back +into Phase 1 authoritative fields (tonal balance, dynamics envelope, stereo +image, LUFS / LRA / peak, spectral centroid / rolloff): + + 1. [apps/backend/server_phase2.py](apps/backend/server_phase2.py) — read-only. + `_normalize_measurement_result_for_gemini` (line 742) builds a shallow + `dict(payload)` copy and, for each per-stem entry, a `dict(stem_entry)` + copy. Only spectral-field keys are renamed for prompt consistency; + values are not changed. `_build_descriptor_hooks` (line 832) reads + `measurement_result.get(...)` and emits a separate + `MEASUREMENT_DERIVED_DESCRIPTOR_HOOKS` JSON block — does not write + back. `_fix_grammar_in_record` (line 1037, modified in + [`89df12e1`](https://github.com/slittycode/ableton-sonic-analyzer/commit/89df12e1)) + mutates Phase 2 recommendation strings only; no Phase 1 path. + 2. [apps/backend/server_phase1.py](apps/backend/server_phase1.py) — read-only. + `_normalize_spectral_detail` (line 81) renames keys on a copy. + 3. [apps/backend/sample_generation.py](apps/backend/sample_generation.py), + [apps/backend/sample_theory.py](apps/backend/sample_theory.py), + [apps/backend/sample_synthesis.py](apps/backend/sample_synthesis.py), + [apps/backend/sample_drums.py](apps/backend/sample_drums.py), + [apps/backend/server_samples.py](apps/backend/server_samples.py) — Phase 3 audition-sample + code. Only `.get()` reads from the Phase 1 payload, all citations + declared explicitly via `phase1_fields=[...]` lists. No + recomputation of LUFS / spectral / dynamics fields. + 4. [apps/ui/src/services/phase2Validator.ts](apps/ui/src/services/phase2Validator.ts) — read-only. + All Phase 1 access via property reads (`phase1.bpm`, `phase1.key`, + `phase1.lufsIntegrated`, `phase1.spectralDetail?.spectralCentroidMean`, + etc.). The validators check Phase 2 against Phase 1, never the + reverse. The recent diff + ([`adb715ef..origin/main`](https://github.com/slittycode/ableton-sonic-analyzer)) + adds an `audience` field on violations and tags the `NEW_FIELD_UNCITED` + warning as `dev` — no field-write changes. + 5. [apps/ui/src/services/mixDoctor.ts](apps/ui/src/services/mixDoctor.ts) — read-only. + `estimatePlr` (line 112) and the newly inspected `estimateMonoCompatible` + (line 121) both early-return when the explicit Phase 1 field is + populated, and only derive locally for the advisory score; results + are not written back into the snapshot. `estimateMonoCompatible` + reads `phase1.monoCompatible` then falls back to + `phase1.stereoDetail.subBassMono` — both are pre-emitted Phase 1 + fields, not recomputations. + 6. [apps/ui/src/services/analysisRunsClient.ts](apps/ui/src/services/analysisRunsClient.ts), + [apps/ui/src/services/analyzer.ts](apps/ui/src/services/analyzer.ts), + [apps/ui/src/services/appliedRecommendations.ts](apps/ui/src/services/appliedRecommendations.ts), + [apps/ui/src/services/backendPhase1Client.ts](apps/ui/src/services/backendPhase1Client.ts) — + no Phase 1 mutations. `backendPhase1Client.ts` normalizes nullable + booleans (`monoCompatible`, `bpmAgreement`, `bpmDoubletime`) on the + way *in* from the wire envelope — that is the parsing boundary, not + a write-back from Phase 2/3. + 7. [apps/ui/src/services/sessionMusician/](apps/ui/src/services/sessionMusician/), + [apps/ui/src/services/midi/](apps/ui/src/services/midi/) — read-only consumption + of Phase 1 / interpretation outputs. + 8. [apps/ui/src/components/AnalysisResults.tsx](apps/ui/src/components/AnalysisResults.tsx), + [apps/ui/src/components/MeasurementDashboard.tsx](apps/ui/src/components/MeasurementDashboard.tsx), + [apps/ui/src/components/MixDoctorPanel.tsx](apps/ui/src/components/MixDoctorPanel.tsx) — + render-only consumption of LUFS, true peak, stereo stats, spectral + balance, centroid, and rolloff. No assignments to those fields anywhere + in `src/`. + +Strict regex `(phase1|measurementResult|measurement_result)\.[a-zA-Z_]+\s*=[^=]` +across `apps/ui/src` and `apps/backend/{server_phase2.py,sample_*.py,server_samples.py}` +returns **zero matches**. The only match for that pattern repo-wide is +[test_sample_generation.py:106](apps/backend/tests/test_sample_generation.py:106), +which is a test fixture mutating a local `phase1` dict before passing it to +the sample-generation harness — not production code. + +Phase 3 audition-sample generation (`sample_generation.py` etc.) is shipped; +the `patchSmith.ts` Phase 3 synth-patch generator referenced in +[BACKLOG.md](BACKLOG.md) has not landed yet — there is no `src/phase3/` +directory and nothing in the frontend to audit for that line. + +### 4. Needs human review + +None. The 2026-05-16 review item (`estimatePlr`) closed on +[`adb715ef`](https://github.com/slittycode/ableton-sonic-analyzer/commit/adb715ef); +the comment block at [mixDoctor.ts:102-111](apps/ui/src/services/mixDoctor.ts:102) +explains the byte-identity argument with the analyze_core.py reference. + +--- + +### What was *not* found + + 1. No Phase 1 field assignments anywhere in `apps/ui/src/` or the + Phase 2/3 backend modules. + 2. No mutations of objects returned from the backend `phase1` envelope. + 3. No backend code path that writes back into `measurement_result` / + `phase1` from the Phase 2 worker or Phase 3 sample generator. + 4. No drift between the citation contract (`phase1Fields`) and the + `collectPhase1FieldPaths` walker in `phase2Validator.ts`. + +The chain-of-custody invariant is intact at the source level. The eight +Playwright failures are independent of that invariant — seven are +`networkidle`-wait flakiness against a shared Vite dev server, one is a +real-backend timeout on `silence.wav` analysis.