Spike: audio/video upload → whisperX SRT → Cardigan job (no caption file)

[mriechers]

What this is

A prototype / spike proving the "upload media, no caption file required" workflow end-to-end. Not for merge as-is — it's a reviewable vertical slice with an explicit build backlog (see SPIKE-NOTES.md).

The path it proves

raw MP4 → whisperX service → SRT → existing job pipeline → 4-phase metadata

• diarization service (diarization/app.py): new POST /transcribe — whisperX transcribe + align (no diarize), returns segments with text + duration + language. (The service already ran whisperX for /diarize but discarded the text; this surfaces it.)
• client (api/services/diarization_client.py): transcribe() mirroring diarize().
• utils (api/services/utils.py): segments_to_srt() — builds SRT from whisperX segments. Unit-tested (tests/api/test_segments_to_srt.py).
• upload router (api/routers/upload.py): POST /api/upload/media — accepts audio/video, calls the service, builds the SRT, writes it to transcripts/, and creates a job via the existing JobCreate path (dedup + Airtable SST auto-link).

Verified

Ran locally, prod-shaped (whisperX service in one venv, API/worker in another, over HTTP): raw MP4 → 16 segments → SRT → job → all 4 phases completed ($0.06). ruff clean; unit test passing. Full write-up in SPIKE-NOTES.md.

Key deployment finding

whisperX is already containerized in prod as cardigan-diarization (behind the diarization compose profile) — so "run whisperX in a container" is already solved; this extends that service.

Open build items (tracked in SPIKE-NOTES.md)

• Anti-hallucination thresholds in /transcribe (raw whisperX hallucinated cues over a silent b-roll gap; the validator caught it)
• Glossary wiring (base→project merge → whisper initial_prompt) — see Secrets resolver hardcodes macOS Keychain for local dev; team standard is 1Password #267-adjacent secrets note and glossary headers
• SRT download endpoint + frontend action
• wc-transcribe-style QC pass
• Media-ID extraction for full-date filenames (e.g. 6HNS20260619… → truncates to 6HNS2026)

🤖 Generated with Claude Code

[claude]

Code review

Three issues found in api/routers/upload.py. All three are in the spike code — noting them now so they're captured before the real build.

---

1. 2 GB upload buffered into RAM before the size check takes effect

cardigan/api/routers/upload.py

Lines 263 to 270 in b676636

	content = await file.read()
	if len(content) > MAX_MEDIA_FILE_SIZE:
	raise HTTPException(
	status_code=400,
	detail=f"File too large. Maximum size: {MAX_MEDIA_FILE_SIZE / 1024 / 1024 / 1024:.0f} GB",
	)

await file.read() with no argument materializes the entire request body into content before len(content) is checked. At a 2 GB cap, a single upload can exhaust server RAM — and the check fires only after the damage is done. Concurrent uploads multiply the exposure.

The fix is to stream the body to the temp file in bounded chunks, enforcing a running total:

MAX_CHUNK = 1 * 1024 * 1024  # 1 MB
with tempfile.NamedTemporaryFile(suffix=file_ext, delete=False) as tmp:
    tmp_path = tmp.name
    total = 0
    while chunk := await file.read(MAX_CHUNK):
        total += len(chunk)
        if total > MAX_MEDIA_FILE_SIZE:
            Path(tmp_path).unlink(missing_ok=True)
            raise HTTPException(status_code=400, detail="File too large. Maximum size: 2 GB")
        tmp.write(chunk)

This also eliminates the separate content buffer and lets the temp-file path be captured before any write (see issue 3).

---

2. SRT silently overwrites an existing transcript when the media ID is absent

cardigan/api/routers/upload.py

Lines 274 to 308 in b676636

	# Cheap duplicate check before the expensive transcription.
	media_id = extract_media_id(srt_name)
	existing = await database.find_jobs_by_media_id(media_id) if media_id else []
	if existing:
	return MediaUploadResponse(
	filename=filename,
	success=False,
	error=f"Already exists as job {existing[0].id} ({existing[0].status.value})",
	)
	# Transcribe via the WhisperX service (temp file because the service reads a path).
	client = DiarizationClient()
	try:
	if not await client.is_available():
	raise HTTPException(status_code=503, detail="Transcription service unavailable")
	with tempfile.NamedTemporaryFile(suffix=file_ext, delete=False) as tmp:
	tmp.write(content)
	tmp_path = tmp.name
	try:
	result = await client.transcribe(tmp_path)
	finally:
	Path(tmp_path).unlink(missing_ok=True)
	finally:
	await client.close()
	if not result or not result.get("segments"):
	return MediaUploadResponse(filename=filename, success=False, error="Transcription produced no segments")
	# Build + persist the SRT.
	srt_content = segments_to_srt(result["segments"])
	TRANSCRIPTS_DIR.mkdir(parents=True, exist_ok=True)
	srt_path = TRANSCRIPTS_DIR / srt_name
	srt_path.write_text(srt_content, encoding="utf-8")
	logger.info(f"Wrote generated SRT: {srt_path} ({len(result['segments'])} segments)")

When extract_media_id(srt_name) returns None (e.g. any filename containing a space like "My Interview Final.mp4"), the duplicate guard short-circuits:

existing = await database.find_jobs_by_media_id(media_id) if media_id else []

existing is always [], so the guard is skipped. Later, srt_path.write_text(...) runs with no existence check and unconditionally clobbers any file already at that path. Two uploads with the same stem both create jobs, but the second silently overwrites the first job's transcript on disk.

A simple guard before writing:

if srt_path.exists():
    raise HTTPException(status_code=409, detail=f"Transcript already exists: {srt_name}")

Or use open('x') mode to make it atomic:

try:
    srt_path.write_text(srt_content, encoding="utf-8")  # replace with open(..., 'x')
except FileExistsError:
    raise HTTPException(status_code=409, detail=f"Transcript already exists: {srt_name}")

---

3. Temp file leaked when tmp.write() raises (e.g. disk full)

cardigan/api/routers/upload.py

Lines 289 to 297 in b676636

	with tempfile.NamedTemporaryFile(suffix=file_ext, delete=False) as tmp:
	tmp.write(content)
	tmp_path = tmp.name
	try:
	result = await client.transcribe(tmp_path)
	finally:
	Path(tmp_path).unlink(missing_ok=True)
	finally:

with tempfile.NamedTemporaryFile(suffix=file_ext, delete=False) as tmp:
    tmp.write(content)
    tmp_path = tmp.name   # ← assigned AFTER the write
try:
    ...
finally:
    Path(tmp_path).unlink(missing_ok=True)   # ← NameError if write failed

If tmp.write(content) raises (disk full, I/O error), tmp_path is never assigned. The finally block then raises NameError, masking the original error and leaving the delete=False temp file on disk permanently. Moving tmp_path = tmp.name to before the write fixes it (and the streaming approach from issue 1 eliminates the problem entirely).

---

🤖 Generated with Claude Code

[mriechers]

@claude please review

[claude]

Claude finished @mriechers's task in 2m 28s —— View job

---

Code Review

• Gather context
• Read changed files
• Review all files

---

Verdict: solid spike. The path from raw media to publishable metadata is proven and the implementation is clean. Three issues were already flagged in the prior review against upload.py — those stand. I found additional items across the other files worth capturing before the real build.

---

diarization/app.py — new /transcribe endpoint

1. Same temp-file leak pattern as the prior review's Issue #3

Both /diarize (line 124–125) and /transcribe (line 221–222) share this pattern:

with tempfile.NamedTemporaryFile(suffix=suffix, delete=False) as tmp:
    tmp.write(await file.read())   # ← if this raises...
    tmp_path = tmp.name            # ← ...this never runs
# ...
finally:
    Path(tmp_path).unlink(missing_ok=True)  # ← NameError masks the original

Moving tmp_path = tmp.name to before the write (as in the upload.py fix) resolves it for both endpoints. Since this service is internal-only, it's lower urgency than the public router, but the same fix applies.

2. No file-size guard on the service

The service has no Content-Length / chunk-size check before materializing the entire upload into RAM. Because upload.py enforces the 2 GB limit before calling the service, this is currently safe — but if the service endpoint is ever called directly (e.g. curl during debugging, a future CLI client), there's no backstop. Worth a note in the build backlog.

3. load_model() uses deprecated @app.on_event("startup")

Pre-existing, not introduced by this PR, but since /transcribe is new code that relies on _model being loaded: worth a note to migrate to FastAPI's lifespan context manager when the service gets its first production hardening pass.

---

api/services/diarization_client.py — new transcribe() method

4. Timeout log message references the wrong constant name

Line 132:

logger.warning(f"Transcription timed out after {DIARIZE_TIMEOUT_SECONDS}s for {path.name}")

DIARIZE_TIMEOUT_SECONDS is the right value, but the name is misleading in a transcription context. Minor, but a reader scanning logs will see "DIARIZE timeout" for a transcription operation. Rename to SERVICE_TIMEOUT_SECONDS (or a new TRANSCRIBE_TIMEOUT_SECONDS) during the real build.

5. except (httpx.HTTPError, Exception) redundancy

Both diarize() and transcribe() (lines 95 and 131) have:

except (httpx.HTTPError, Exception) as e:

Exception already subsumes httpx.HTTPError — the union is a no-op. Either spell out the specific exceptions you want to handle distinctly, or just except Exception. This is a pre-existing pattern in diarize(); transcribe() copied it.

6. No context-manager support on DiarizationClient

The client creates an httpx.AsyncClient in __init__ and requires an explicit await client.close(). upload.py does this correctly in a finally block, but the API makes it easy to forget. Adding __aenter__/__aexit__ delegates would make it harder to leak connections in future callers.

---

api/services/utils.py — segments_to_srt()

7. generate_srt mutates caption index in-place

def generate_srt(captions: List[SRTCaption]) -> str:
    for i, caption in enumerate(captions, 1):
        caption.index = i   # ← mutates the dataclass
        ...

If the same SRTCaption list is passed to generate_srt twice, the second call will see indexes already modified by the first. This is only visible if a caller reuses a list (currently none do), but it's a subtle gotcha to fix before the SRT download endpoint reuses captions from a parsed SRT. Fix this →

---

tests/api/test_segments_to_srt.py

Tests are well-structured and the round-trip through parse_srt is a good pattern. Two gaps worth adding before shipping:

8. No test for empty input

segments_to_srt([]) should produce an empty string (or at least not raise). A one-liner test would confirm the contract:

def test_empty_input():
    assert segments_to_srt([]) == ""

9. No test for hour-boundary timecodes

PBS programs run 28–58 minutes; test that a segment starting at e.g. 3660.0 seconds formats as 01:01:00,000 correctly. The underlying ms_to_srt_timecode has a doctest for this, but segments_to_srt itself is not covered for the hours path.

---

api/routers/upload.py — prior review issues still open

The prior review raised three issues; none have been addressed (expected for a spike, just confirming):

#	Issue	Status
1	2 GB body buffered into RAM before size check	Still present, line 264–269
2	SRT silently overwrites existing file when media_id is None	Still present, line 307
3	Temp file leaked if tmp.write() raises	Still present, line 291–296

---

SPIKE-NOTES.md

Excellent documentation. The two ⚠️ findings (anti-hallucination thresholds, glossary not wired) and the layered glossary architecture decision are exactly the right things to capture before the real build. The repro steps are detailed enough to reproduce from cold.

One note: the SPIKE-NOTES.md hardcodes 6HNS20260619HoChunkLand.mp4 in the repro steps — fine for a spike doc, but that filename also surfaces the media-ID truncation bug you noted (6HNS20260619… → 6HNS2026). Worth a unit test in test_extract_media_id.py (if one exists) so the fix has coverage before the feature ships.

---

Summary

File	New Issues	Notes
diarization/app.py	3 (temp-file leak ×2, no size guard)	Same patterns as upload.py
diarization_client.py	3 (log message, except redundancy, no CM support)	Minor / naming
utils.py	1 (in-place mutation)	Low risk today, higher risk at SRT download
test_segments_to_srt.py	2 (missing edge cases)	Easy adds
upload.py	—	Covered in prior review
SPIKE-NOTES.md	—	Well-written

The spike is ready to inform the real build. I'd prioritize the three upload.py issues (#1 streaming, #2 overwrite guard, #3 temp file), then the diarization service temp-file leak (#1 above), before shipping to users.