Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 226 additions & 0 deletions audits/nightly-2026-05-19.md
Original file line number Diff line number Diff line change
@@ -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.