fix: 2026-05-30 full-review fixes — staged-run lifecycle, contract drift, error boundary

[slittycode]

Summary

Fixes from the 2026-05-30 full bug + architecture review (audit f5b3870b, audits/full-review-2026-05-30.md). Two commits: a reliability/correctness cluster, then the review-driven follow-ups + the missing error-boundary coverage.

Commit 1 — staged-run lifecycle + chain-of-custody/contract fixes (e3e0f69a)

Staged-run lifecycle (P1/P2):

• Cancel now actually terminates later stages — pitch-note + MT3 subprocesses route through _run_streamed_subprocess so interrupt/delete can kill them (were orphaned, multi-GB RAM, until their 600s/1800s timeout), with a status guard so a killed orphan can't resurrect an interrupted row.
• Interpretation & measurement attempts terminalize on setup failure (were stranded running forever, no reaper).
• Frontend monitorAnalysisRun gained an overall wall-clock budget + per-poll deadline (was an unbounded poll loop).

Chain-of-custody / quick wins:

• Phase 1 parser carries the four prompt-citable reverb/vocal subfields it was silently dropping (broke the citation-existence check).
• normalizeKey no longer rewrites "major" → "majoror" (false key contradiction).
• Defined the missing temp-file-cache constants + imported timedelta (latent NameError).
• Added a React ErrorBoundary around the lazy AnalysisResults.

Commit 2 — review follow-ups + ErrorBoundary smoke test (e0e414f9)

A sequential 3-agent review of commit 1 returned ship-as-is; its P3/LOW follow-ups are bundled here:

• Backend orphan guard — _enqueue_requested_followups re-asserts measurement.status == 'completed', closing the cross-transaction interrupt window the review found.
• Faithful MT3 mock-timeout (_stream_text bytes→str coercion) so the timeout branch is actually exercised, not masked by a str in bytes TypeError.
• Bounded the last unbounded await (the stem-summary POST) via an exported raceWithDeadline helper.
• ErrorBoundary docstring corrected ("Try again" recovers a render-throw, not a chunk-load failure — React.lazy caches the rejected import).
• ErrorBoundary fallback smoke test — drives a mocked run to the results view, aborts the lazy AnalysisResults chunk, and asserts the fallback renders. Verified meaningful: it fails when the boundary is removed.

Verification

• Backend: 1069 tests green (unittest discover).
• Frontend: 716 unit tests green + tsc lint clean.
• New Playwright smoke spec passes (and fails correctly without the boundary).

Notes

No correctness or invariant violations introduced; the changes strictly harden the staged-run lifecycle and the chain of custody. One pre-existing hosted-only edge — a passive multi-client observer polling to the 90-min budget on interrupt — is now bounded by this work and left as an optional follow-up.

🤖 Generated with Claude Code

[slittycode]

Verdict: APPROVE (self-review — can't formally approve own PR)

Summary

Two-commit PR addressing the P1/P2 cluster from the 2026-05-30 full audit: orphaned subprocess lifecycle, interpretation/measurement stranded-running on setup failure, unbounded frontend poll loop, four parser-dropped Phase 1 fields breaking the citation-existence check, the normalizeKey "majoror" typo, and the latent NameError in the cache-eviction path. Every new logical branch has a test. Phase boundary invariants are clean. The author claims 1069 backend + 716 frontend unit tests green; I couldn't run them in this environment (no venv / node_modules installed), but reviewed all test code in the diff and it's correct.

Findings

Blocking: none.

Tracking only (not introduced or worsened by this PR):

• url_ingest.py:159 — SSRF DNS-rebinding TOCTOU is a P1 in any hosted/cloud deployment. Acknowledged in the audit and left open. Needs a tracking issue before hosted ships to cloud infra.
• analyze_core.py:252 — truePeak rounded to 1 decimal hides inter-sample overs in the (1.0, 1.05] band, defeating the loudness guardrail. Trivial fix (round(..., 6) to match the fast path).
• analyze_core.py:633 — PLR subtracts dB LUFS from linear true-peak. Systematic inflation, inverts the mixDoctor dynamics verdict on unmastered tracks.

Worth considering:

• _enqueue_requested_followups re-check: closing the cross-transaction interrupt race with a status re-read is the right call — the if updated: gate in the caller can't see that window. The comment explains it well.
• _execute_interpretation_attempt outer wrapper: the fail_interpretation_attempt call is also safe if the run is already interrupted (WHERE-clause guard makes it a no-op), so the wrapper is defensive in all cases, not just setup-failure cases. Not a bug.

Test results

Not executable in this environment. Reviewed all test code in the diff:

• StagedRunInterruptResurrectionTests (5 cases): assertion on DB state directly — correct, meaningful. Tests the measurement/pitch-note/MT3 resurrection paths and the cross-transaction enqueue race.
• TempFileCacheTests (3 cases): drives the previously-latent NameError path. Correct.
• MT3 timeout assertion extension (assertIn("timed out", ...)): verifies the timeout branch is actually reached after the bytes→str coercion fix rather than dying earlier on a TypeError. This is the right accompanying test.
• raceWithDeadline (3 cases): covers the abandoned-work unhandled-rejection case — the one most likely to be missing. Correct.
• analyzeAudio polling timeouts (2 cases): per-poll deadline and wall-clock budget with never-settling and negative-budget scenarios. Correct.
• ErrorBoundary unit + smoke: unit tests getDerivedStateFromError directly (correct for node-env per CLAUDE.md tripwire #5); Playwright smoke aborts the chunk, asserts fallback renders. PR says it fails without the boundary — that's the bar.
• normalizeKey 3-case suite: Maj abbreviation, Min abbreviation, genuine contradiction still detected. Third test is the critical one.
• Parser-drop regression guards: all four previously-dropped fields covered through both parse and citation-existence paths.

Phase boundary check

Clean. Parser changes (perBandRt60, preDelayMs, stemEnergyRatio, stemOtherCorrelation) restore fields already emitted by the backend and declared in measurement.ts — additive, no schema change. No Phase 2/3 code touches Phase 1 measurement values. analysis_runtime.py changes are pure state-machine guards.

---

Generated by Claude Code