Skip to content

fix(jobs): atomic dot-path writes for file_urls/state_data (lost-update race) - #970

Merged
beveradb merged 2 commits into
mainfrom
feat/sess-20260831-1800-fix-file-urls-race
Aug 31, 2026
Merged

fix(jobs): atomic dot-path writes for file_urls/state_data (lost-update race)#970
beveradb merged 2 commits into
mainfrom
feat/sess-20260831-1800-fix-file-urls-race

Conversation

@beveradb

Copy link
Copy Markdown
Collaborator

Summary

  • JobManager.update_file_url and update_state_data copied the whole file_urls/state_data map and re-persisted it. The audio and screens workers run in parallel and both write these maps, so a write built from a stale snapshot silently dropped a sibling key set in between (a classic lost update).
  • Real symptom (job 5588e466, Keeno "Shelter from the Storm"): backing_vocals.flac was uploaded to GCS but missing from file_urls.stems (only the 4th-of-5 stem lost). The instrumental-review UI couldn't load the backing preview, and _analyze_backing_vocals early-returned → "Very few backing vocals detected (0%)" → the C1 mirror-skip forwarded /review/instrumental.
  • Rare — needs a precise interleaving (~1 in 4 parallel jobs). Pre-existing; unrelated to the timing-gate work.

Changes

  • update_file_url / update_state_data now write a single Firestore dot-path field (file_urls.<cat>.<type>, state_data.<key>) so siblings merge server-side — matching the existing update_processing_metadata pattern. All keys are plain snake_case (no dots), so no path is misread as nesting.
  • screens_worker._analyze_backing_vocals hardened: recovers a stem present at the conventional GCS path but missing from file_urls (re-registers it atomically), and stores a fallback analysis instead of a silent early-return.
  • Tests: unit tests asserting dot-path writes; emulator regression tests (test_file_urls_no_clobber.py) that reproduce the lost update via a stale snapshot — they fail against the old read-modify-write and pass against the fix. Updated one assertion in test_job_manager.py and the missing-stems worker test for the new behavior.
  • docs/LESSONS-LEARNED.md entry; version bump 0.216.0 → 0.216.1.

Testing

  • Full backend unit suite green (4216 passed; the only failures were emulator-connection env issues, verified green once the emulator was started).
  • Emulator suite green incl. new regression tests (127 passed); new tests confirmed to fail on the old impl and pass on the new.
  • CodeRabbit CLI review — No findings.

Follow-up (not in this PR)

  • Remediate the already-stuck job 5588e466 after deploy via POST /api/admin/jobs/{job_id}/regenerate-screens (the fixed screens worker recovers + re-registers the stem and recomputes the analysis).

Review

  • Local CodeRabbit review completed — no findings
  • Feedback addressed

@coderabbitai ignore


🤖 Generated with Claude Code

beveradb and others added 2 commits August 31, 2026 18:12
…te race)

update_file_url and update_state_data copied the whole map and re-persisted
it, so the parallel audio + screens workers could clobber each other's writes.
Real symptom: a backing_vocals stem uploaded to GCS but dropped from
file_urls.stems, so the instrumental-review UI couldn't load the backing
preview and _analyze_backing_vocals reported 0% backing.

- Write single Firestore dot-path fields (file_urls.<cat>.<type>,
  state_data.<key>) so siblings merge server-side, matching
  update_processing_metadata.
- _analyze_backing_vocals recovers a stem present at the conventional GCS
  path but missing from file_urls, and stores a fallback analysis instead of
  a silent early-return.
- Unit tests for dot-path writes + emulator regression tests that fail against
  the old read-modify-write and pass against the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…00-fix-file-urls-race

# Conflicts:
#	pyproject.toml
@beveradb
beveradb enabled auto-merge (squash) August 31, 2026 23:44
@beveradb
beveradb merged commit 9616342 into main Aug 31, 2026
22 checks passed
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