Know your unknowns · During implementation
← All examples

Implementation notes

Instead of letting mid-build surprises vanish into the scrollback, ask the agent to log every deviation from the plan as it happens — so the surprises become inputs to your next attempt.

The prompt
“Keep an implementation-notes file as you build the export feature. If you hit an edge case that forces you to deviate from the plan, pick the conservative option, log it under ‘Deviations’, and keep going.”
What Claude produced
docs/notes/export-feature-implementation.md run started 14:02 · last write 16:51 · branch feat/review-export
11
entries
4
deviations
2
need your judgment
14:02
Plan-confirmed14:02

Step 1 — export job model and queue wiring, as planned

Added ExportJob in apps/api/src/exports/exportJob.ts, enqueued on the existing BullMQ media-tasks queue with a dedicated export job name. Migration 0121_export_jobs matches the plan's schema exactly (status enum, requested_by, review_id, format).

14:18
Plan-confirmed14:18

Step 2 — POST /api/v1/reviews/:id/export endpoint

Request validation reuses the zod schemas in apps/api/src/validation/ as the plan specified. format accepts mp4-burnin | csv | bundle. Returns 202 with a job id; polling endpoint deferred to step 6 per plan.

14:29
Discovery14:29

Review.duration_ms is denormalized and occasionally stale

Three fixture reviews have duration_ms that disagrees with the source asset's probe data (looks like re-uploads never refresh it). Export reads duration from MediaAsset.probe.duration instead — same approach playbackManifest.ts already takes. No plan change, just noting the trap for anyone who touches this later.

14:41
Deviation 1 of 414:41

Legacy annotations don't always have frame timestamps

What the plan said
Every annotation has a frame_ts; burn-in renderer can sort and place them all on the timeline.
What the code revealed
~12% of rows in annotations predate migration 0087 and have frame_ts = NULL — they were general review comments before per-frame annotations existed.
Conservative choice
Exclude null-timestamp annotations from the video burn-in; include them in the CSV sidecar with frame_ts left blank and a legacy_comment flag. Nothing is silently dropped from the export bundle.
Revisit
Could interpolate a timestamp from created_at offset against the review session — decide whether that's honest enough to show in the video.
15:05
Plan-confirmed15:05

Step 3 — annotation burn-in via ffmpeg drawtext/drawbox, as planned

Filtergraph builder in apps/worker/src/exports/burnin.ts. The plan's per-annotation overlay windows (enable='between(t,a,b)') work; verified against the 40-annotation stress fixture — render time 1.4x realtime on the worker instance class, inside the plan's 2x budget.

15:22
Deviation 2 of 415:22

The queue round-trips Date objects as strings

What the plan said
ExportJobPayload.requestedAt: Date, passed through to the worker and compared against the review's updated_at to detect stale exports.
What the code revealed
BullMQ JSON-serializes payloads into Redis, so the worker receives an ISO string, not a Date. The staleness comparison silently coerced and passed in unit tests, but failed on a redelivered job in the integration run.
Conservative choice
Typed the payload as wire format (requestedAt: string ISO-8601), parse once at the worker boundary in exportConsumer.ts. No custom serializer.
Revisit
Other media-tasks payloads likely have the same latent bug — a shared zod codec for job payloads would fix the class of problem.
15:48
Deviation 3 of 415:48

Zip streaming already exists — dropped the planned archiver dependency

What the plan said
Add archiver to build the export bundle (video + CSV + thumbnails) as a zip stream to S3.
What the code revealed
packages/media/src/zipStream.ts already does streaming zip with backpressure handling — it powers the bulk asset download feature and has been in production since March.
Conservative choice
Reused zipStream.ts as-is. One fewer dependency, and bundle exports inherit the same tested backpressure behavior as bulk downloads.
Revisit
zipStream lives in packages/media but is now used by two features — worth promoting to packages/shared in a cleanup pass.
16:07
Discovery16:07

A websocket progress channel convention already exists

The transcode pipeline emits media:progress:{assetId} events. Followed the same convention — exports emit export:progress:{jobId} — so the frontend can reuse useProgressChannel() from apps/web/src/hooks/ instead of the polling UI the plan sketched. Kept the polling endpoint too, as the fallback.

16:20
Deviation 4 of 416:20

Guest reviewers can view reviews but can't download assets

What the plan said
Export permission = review-level ACL: anyone who can open the review can export it.
What the code revealed
Workspace-level guest_reviewer role grants review access with can_download_assets = false — clients use it to let external stakeholders comment without taking the media offsite. A review-ACL-only check would let guests exfiltrate source video via export.
Conservative choice
Export returns 403 for guest_reviewer across all formats, mirroring the existing asset-download check in assetAccess.ts. Strictest defensible reading.
Revisit
Arguably guests should still get the CSV (annotations only, no media). That's a product call, not mine — logged below.
16:33
Todo for human16:33

Decide the guest-reviewer export policy

Current behavior (from Deviation 4): guests get 403 on every export format. If you want guests to export the annotation CSV without media, it's a ~10-line change in exportPolicy.ts plus one test — but it changes what “can't download assets” means to customers, so it should be your call. Blocking neither merge nor QA.

16:51
Todo for human16:51

Confirm the retention window for export bundles

The plan didn't specify a TTL for generated bundles. I put them under the exports/ S3 prefix with a 7-day lifecycle rule, matching share-link expiry — conservative, and nothing customer-visible promises longer. But if contracts or the data-retention policy say otherwise, the lifecycle rule in infra/s3.tf is the one line to change.