Skip to content

Nightly audit: 1 issue(s) - #47

Merged
slittycode merged 3 commits into
mainfrom
claude/nightly-audit-2026-05-14
May 16, 2026
Merged

Nightly audit: 1 issue(s)#47
slittycode merged 3 commits into
mainfrom
claude/nightly-audit-2026-05-14

Conversation

@slittycode

Copy link
Copy Markdown
Owner

Nightly audit — 2026-05-14

Automated Phase-boundary + test-guardrail audit.

Result

Step Outcome
Unit suite (vitest run) 3 failed / 557 passed / 561 total
Playwright smoke (playwright test) 44 passed / 2 skipped / 0 failed
Phase-boundary scan No violations found
Phase 1 → Phase 2 contract Consumed read-only downstream — OK

Repo-structure note (task assumptions vs. reality)

The task brief assumes src/phase2/ and src/phase3/ directories and a root-level
npm ci / Essentia.js. The actual repo does not match:

  1. There is no root package.json. Frontend deps live in apps/ui; the setup
    script's npm ci was run there instead.
  2. Phase 1 DSP is Python/Essentia in apps/backend (analyze*.py), not
    Essentia.js. It is the ground-truth measurement layer.
  3. Phase 2 (interpretation) is apps/backend/server_phase2.py + Gemini, with the
    frontend guardrail in apps/ui/src/services/phase2Validator.ts.
  4. Phase 3 is not implemented (patchSmith.ts is still the open item in
    BACKLOG.md). Nothing to audit there.

The audit was performed against the real structure.

Failing tests

All 3 failures are in fixture-dependent live decision-gate comparators, all
introduced together in commit 94a26fbd (PR #18):

  1. apps/ui/tests/decision_gate.multi.live.test.ts:147
    decision-gate (multi-model) compares validator output across Gemini variants
  2. apps/ui/tests/decision_gate.real.live.test.ts:152
    decision-gate (multi-model, real track) ... on Vtss-CantCatchMe.mp3
  3. apps/ui/tests/decision_gate.stems.live.test.ts:168
    decision-gate (multi-model, stem-aware, real track) ... with stemAnalysis populated

Failure mode (all three): AssertionError: expected 0 to be greater than 0
at expect(ok.length).toBeGreaterThan(0). Each test loads pre-generated Gemini
snapshots (/tmp/decision_gate_<model>.json or apps/backend/.runtime/reports)
produced by live multi-model Gemini runs. With no snapshots present, ok.length
is 0 and the assertion fails.

Root cause: test-harness bug, not a product regression. Each file's own
docstring claims it "Skips when no snapshots are present, so the suite stays
clean elsewhere"
— but the code asserts toBeGreaterThan(0) instead of calling
it.skip / this.skip(). The intended skip-when-no-fixtures behavior was never
implemented.

Not a Phase-boundary violation. These tests exercise the boundary guardrail
(phase2Validator.ts) read-only; they don't cross it.

Note: npm run test:unit (scoped to tests/services) is green — these three
live tests sit at tests/ root and are only picked up by the unscoped npm test.

Phase-boundary scan — no violations

Checked every path where Phase 2 / interpretation touches Phase 1 / measurement
data:

  1. Storage layer (apps/backend/analysis_runtime.py) — measurement results
    live in the measurement_outputs table; interpretation results live in the
    separate interpretation_attempts table. _update_measurement_row is called
    only from the measurement stage (analysis_runtime.py:615,686,704) and
    progress updaters in server.py. The interpretation stage references the
    measurement via FK grounded_measurement_output_id — a read-only pointer,
    never a write-back.
  2. server_phase2.py:742 _normalize_measurement_result_for_gemini — builds
    dict(payload) / dict(stem_entry) copies and only renames spectral keys
    (spectralCentroidspectralCentroidMean, etc.) for prompt presentation so
    Gemini's field names match the validator contract. Operates on copies; the
    stored measurement is untouched. No recompute of values.
  3. apps/ui/src/services/analysisRunsClient.ts:184 projectPhase1FromRun
    returns { ...measurement, transcriptionDetail: pitchNote }: a new object that
    attaches the Layer-2 pitch/note result as an additive field. No measured DSP
    value is overwritten; no mutation of the source object.
  4. apps/ui/src/services/phase2Validator.ts — reads Phase 1 strictly
    read-only to check Phase 2 consistency. This is the boundary guardrail.

Phase 1 → Phase 2 contract — OK

Phase1Result (apps/ui/src/types/measurement.ts) flows into Phase 2 only as:
(a) read-only input to the Gemini prompt builder, and (b) read-only input to
validatePhase2Consistency. No downstream writer assigns to tonal-balance,
dynamics-envelope, stereo-image, loudness (LUFS/LRA/peak), or spectral
centroid/rolloff fields.

Needs human review

  1. decision_gate.*.live.test.ts skip logic. Decide intended behavior:
    either gate these behind an env flag (e.g. RUN_GEMINI_LIVE_SMOKE, as the
    smoke suite already does) or have them it.skip when no snapshots/fixtures are
    present — matching their own docstrings. Until then npm test is red on any
    machine without pre-generated Gemini snapshots, including CI.

Suggested next steps (no auto-fix applied)

  1. Fix the three live tests to skip cleanly when fixtures are absent, or move them
    under an env gate so the default npm test stays green.
  2. Optional: align npm test and npm run test:unit scope, or document that the
    nightly guardrail should run test:unit for the deterministic subset.

Generated by the asa-nightly-phase-1-invariant--test-guardrail scheduled task.
No source files were modified; no fixes were applied.

slittycode and others added 3 commits May 14, 2026 13:54
…rammar post-process

Implements the design audit's prescription end-to-end (findings #1#15 + N1/N2/N7/N9/N10
+ follow-ups). The product's chain-of-custody promise — every Phase 2 recommendation
traces back to the Phase 1 measurement that justifies it — is now visually first-class
on every card, not a 9px monospace footnote.

What ships:

* Chain-of-custody (findings #2 + #3). New CitationBlock primitive renders a structured
  "GROUNDED IN" block above every Mix Chain / Patches / Sonic Element card with
  humanized labels (FIELD_LABELS map, ~50 entries + humanizeFieldPath fallback) and a
  ConfidenceBandBadge pill computed from the worst confidence among cited fields.
  Also retires GroundingBadgeList at the Track Layout site; segmentIndexes ride
  through a new extraRows prop.

* Recommendations-first IA (#1). MeasurementDashboard moved to the bottom of the
  results scroll; StickyNav's 9 measurement pills collapse to one trailing
  "Measurements" entry. Producers hit Style → Sonic → Mix Chain → Patches → Session
  before the measurement evidence.

* Header polish (#7, #9, #11). CPU meter removed (browser-tab CPU is misleading
  during backend analysis), "Local DSP Engine v1.6.0" eyebrow removed (resolves
  mobile 3-line wrap), Dense DAW Lab demoted from accent chip to quiet text link.

* Engineering vocab cleanup (#8 + N3/N4/N5/N8). New userLabels.ts service translates
  field paths to producer-readable labels at every render. Button labels renamed
  (Download data / Download report). FAMILY: NATIVE chip dropped from meta-badge rows.
  workflowStage prettified at the view-model layer ("Sound design" not "SOUND_DESIGN").
  AI Interpretation gated copy reworded ("AI interpretation isn't configured…" not
  "Developer kill-switch is off").

* Applied-recommendations tracker (#14 + #15). Per-card checkbox affordance + section-
  header "N of M applied" chip + localStorage persistence keyed by audio content
  SHA256. Producer can rename their file without losing their progress.

* Idle value-prop panel (#5). Replaces the 200px "NO SIGNAL DETECTED" canvas with
  a producer-readable explanation of what ASA does, with honest pacing copy
  (4–5 min Phase 2 wait, not 2–5 min).

* Patches group structure. Mirrors Mix Chain's emoji-eyebrow grouping (Drums / Bass /
  Synth / Master) so producers can jump to the bass patch without scanning 8 cards.

* Input Source collapse (N9). Post-analysis, the Input Source panel collapses to a
  compact summary with filename + duration + "Analyze new file" + "Adjust settings".
  Frees the top of the page for the results the user came for.

* AnalysisStatusPanel primary readout (#6). Stage diagnostic message promoted from
  9px footnote to the visual focus of the progress card. Pre-existing tone-aware
  fill (running / success / failed) preserved.

* Phase 2 failure mode (N1). Header subtitle derives from interpretation stage status
  (no more "PHASE COMPLETE" while INTERPRET still RUNNING/FAILED). StickyNav Phase 2
  pills render disabled with hover-reason when sections didn't populate. Retry button
  gated on error.retryable; non-retryable failures surface the error code inline.

* Misc audit follow-ups: BPM reconciled across exec card + Core Metrics tile (N2);
  Signal Monitor STANDBY canvas hidden when audio isn't playing, freeing ~160px (N7);
  StickyNav label "Device Chain" → "Sections" (N10, less ambiguous with Ableton's own
  effects-routing meaning); BASS group icon swapped from 🫧 → Lucide AudioWaveform
  (#13); toggle helper paragraphs switched from all-caps mono walls to sans-serif
  sentence case (#4 revised).

* Phase 2 grammar post-process (audit final round). The prompt instruction added
  earlier didn't take — Gemini still emits "by recreates / by absorbs / by shapes"
  3rd-person singular forms after "by" in role/reason text. Server-side
  _apply_phase2_grammar_fixes rewrites these to gerunds in-place on
  mixAndMasterChain[].reason, abletonRecommendations[].{reason,advancedTip}, and
  secretSauce.workflowSteps[].{instruction,measurementJustification}. Conservative
  regex (\bby \w{4,}s\b) + denylist guards against plural-noun false positives.

Test coverage:

* UI: 46 test files / 540 tests pass (was 39/422 before the audit). New service tests:
  userLabels (21), phase1Picker (25), citationBlock (12), appliedRecommendations (16),
  formatTrackDuration (12), interpretationSubtitle (10), workflowStagePrettifier (8),
  analysisStatusProgress (6), idleValuePropPanel + phase2NavReason. New DOM tests in
  analysisResultsUi.test.ts cover Track Layout citation, applied-checkbox flow, mix-
  chain citation rendering.

* Backend: 16 new unit tests in test_phase2_grammar_fix.py cover _to_gerund,
  _fix_by_gerund_in_text, _apply_phase2_grammar_fixes including the actual
  screenshot corpus (recreates → recreating, shapes → shaping, matches → matching,
  etc.). Full suite: 463 of 463 ASA tests pass; 1 unrelated pre-existing failure
  in tests.test_url_ingest predates this branch.

Visual verification: Playwright capture pass against a real 126s track confirmed
every surface (15 screenshots in /tmp/asa-shots-audit-final/). Phase 2 returned in
220s; localStorage round-trip verified on applied-checkbox toggles.

Documented limitations:

* The gerund rule is algorithmic — verbs requiring consonant doubling (control →
  controlling, submit → submitting) degrade to "controling" / "submiting". Still
  better than "by controls". Drop a hand-mapped exception into the module if
  observed in real output.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI surfaced 7 smoke test failures from this branch's audit changes that
hadn't been propagated to the smoke spec assertions:

* `tests/smoke/ui-details.spec.ts` × 3
  - `NO SIGNAL DETECTED` → `IdleValuePropPanel` (audit #5)
  - `JSON_DATA` / `REPORT_MD` button labels → `Download data` / `Download report`

* `tests/smoke/responsive-layout.spec.ts` × 4
  - `NO SIGNAL DETECTED` (×2) → `IdleValuePropPanel`
  - `CPU` text-presence checks removed; the two viewport-shape tests now
    assert just the model-selector responsive behavior (audit #11 retired
    the CPU meter; there's no element to assert)

* `tests/smoke/file-validation.spec.ts` × 1
  - `re-upload after results resets to file-selected state`: the test
    used `Remove File` (FileUpload component's affordance) to clear after
    results were visible. Post-N9 collapse, the Input Source panel
    replaces FileUpload with a compact summary card whose "↺ Analyze new
    file" button calls the same handleFileClear. Switched the test to
    target the new affordance.

Also updated the e2e exports spec for label consistency (not in the
failing CI job, but the same renames apply):

* `tests/e2e/phase1-exports.spec.ts`
  - `downloadTextArtifact(page, /JSON_DATA/i)` → `/Download data/i`
  - `downloadTextArtifact(page, /REPORT_MD/i)` → `/Download report/i`

Verified locally against the live stack: 45 of 46 smoke tests pass, 1
skipped (was unrelated). The previously-failing 7 are all green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 unit tests failing — all fixture-dependent live decision-gate
comparators that assert instead of skipping when Gemini snapshots
are absent. No Phase-boundary violations found.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@slittycode slittycode left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: COMMENT

Summary

Large mixed PR: a backend grammar post-processor for Phase 2 text, substantial frontend UX improvements (CitationBlock, applied-recommendations tracker, progress-card honesty fixes, nav reorder, idle panel), and the nightly audit report documenting 3 pre-existing test failures from PR #18. All new logic has test coverage. Phase boundary is clean. Phase 1 schema is untouched. The 3 failing tests predate this PR and are flagged correctly in the description — they're the only thing standing between npm test being green.

Findings

Should fix (not introduced by this PR, but blocks CI)

apps/ui/tests/decision_gate.*.live.test.ts (three files from PR #18) — the docstrings say "Skips when no snapshots are present" but the code unconditionally asserts expect(ok.length).toBeGreaterThan(0), which fails on any machine without pre-generated Gemini snapshots. npm test is red on CI until this is resolved. The fix is one of: (a) gate behind RUN_GEMINI_LIVE_SMOKE=true (matching the existing smoke pattern), or (b) add a guard that calls it.skip when the fixture directory is absent. Either is fine; the current state is not.

Worth considering

server_phase2.py_to_gerund doesn't handle consonant doubling (controls"controling" instead of "controlling"). Documented in the test comments. If Gemini starts emitting "by controls" the output gets worse, not better. A small exception dict at module level for the 5–10 most common affected verbs would close the gap without trying to implement English orthographic rules.

server_phase2.py:111return record if updated or True else record — the updated flag is computed but the or True makes it a no-op. Dead code; consider just return record.

appliedRecommendations.ts — no max-entries or TTL guard on the localStorage store. Orphaned records accumulate over many sessions. Low priority for V1 (acknowledged in the docstring), but if a producer runs ASA on 50 tracks, the key will hold 50 stale entries forever. A simple prune-to-N-most-recent on write would contain it.

Test results

Unit: 3 failed / 557 passed — all 3 failures are in decision_gate.*.live.test.ts (pre-existing, PR #18). npm run test:unit is green.
Playwright smoke: 44 passed / 2 skipped / 0 failed.

Phase boundary check

Clean.

  • _apply_phase2_grammar_fixes mutates the normalized Phase 2 dict in-place. Never touches Phase 1 data.
  • CitationBlock / phase1Picker.ts read Phase 1 via dotted-path traversal, strictly read-only.
  • appliedRecommendations.ts is a UI concern (localStorage); no contact with either measurement layer.
  • CONFIDENCE_PAIRS export from phase2Validator.ts is a correct refactor — single source of truth now shared by the picker and the validator.
  • Math.round(phase1.bpm) in MeasurementDashboard.tsx is display-only; the underlying phase1.bpm float is unchanged.

Generated by Claude Code

@slittycode
slittycode merged commit 7d509d2 into main May 16, 2026
@slittycode
slittycode deleted the claude/nightly-audit-2026-05-14 branch May 16, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant