REVIEW

Append-only review log. Each task section is appended or updated in place; prior task history is preserved.

T-001 — FastAPI backend (`app.py`) + inline HTML/JS frontend (`requirements.txt`)

Verdict: PASS_WITH_NOTES

[minor] app.py:147 — On any failure inside download() (including yt-dlp errors), str(exc) is returned verbatim to the browser as the message field. yt-dlp exceptions can include local filesystem paths (the temp dir) or verbose internal detail. Not a required fix for this local/self-hosted tool, but consider mapping to a generic message (e.g. "The video could not be downloaded.") and logging the raw exception server-side instead, before this is exposed beyond local/dev use.
- Required fix: No.

No blocker or major findings.

Steps performed:

Re-read .ai/PLAN.md (Phase 1 spec for T-001) and diffed it against app.py / requirements.txt.
python -m py_compile app.py — succeeded.
Installed fastapi, uvicorn, httpx, python-multipart, yt-dlp into a scratch venv and exercised the app with starlette.testclient.TestClient, mocking yt_dlp.YoutubeDL to avoid live network calls:
- GET / → 200, body contains "Download MP3" and "Download Video".
- POST /download with a garbage (non-http) URL → 400 JSON {"message": ...}.
- POST /download with an empty URL → 400 JSON.
- POST /download missing the mode field (triggers RequestValidationError handler) → 400 JSON with .message.
- POST /download with a valid URL, mode mp3, mocked yt-dlp writing a fake file → 200, correct bytes streamed, Content-Disposition: attachment; filename="...", temp dir cleaned up via BackgroundTask.
- POST /download where mocked yt-dlp raises RuntimeError → 500 JSON {"message": "..."} (no server crash), temp dir cleaned up on the error path.
- Filename containing non-ASCII characters (e.g. "café mix 🎵.mp3") → correctly RFC 5987-encoded as filename*=utf-8''... by Starlette; no crash or mojibake.

Findings from verification: All acceptance criteria for T-001 hold:

Risks:

No live network test against real YouTube was performed (would require external network access and a real video URL); mocked yt-dlp calls confirm the FastAPI plumbing (routing, form validation, error handling, file streaming, temp-dir cleanup) is correct, but do not confirm yt-dlp/ffmpeg behavior against a live video. This risk carries into the Docker-level validation planned for T-002 (docker build + smoke test), where a live download should be attempted at least once.
Large/long-running downloads have no timeout or size cap — acceptable for v1 per ROADMAP.md (out of scope), but worth revisiting if this moves beyond local/dev use.

Verdict: PASS

[nit] Dockerfile — the container runs as root (no USER directive). Not required for a local/self-hosted v1 tool per ROADMAP.md, but worth adding a non-root user if this is ever exposed beyond a trusted local network.
- Required fix: No.

No blocker, major, or minor findings.

Steps performed:

Re-read .ai/PLAN.md (Phase 2 spec for T-002) and diffed it against Dockerfile / README.md; matches the planned Dockerfile contents and documented build/run/usage/size-note requirements.
docker build -t yt-dl-review . — exited 0.
docker run --rm -d -p 8091:8080 yt-dl-review — container started; curl http://localhost:8091/ returned HTTP 200 with both "Download MP3" and "Download Video" present in the body.
POST /download with an invalid URL against the running container → 400 JSON {"message": "..."}", confirming the containerized app behaves the same as the local dev checks from T-001; docker logs showed clean request handling with no crash/traceback.
Live end-to-end test (real network, real YouTube video https://www.youtube.com/watch?v=jNQXAC9IVRw, "Me at the zoo"):
- mode=mp3 → 200, content-type: audio/mpeg, valid ID3-tagged MP3 file returned (verified with file), Content-Disposition filename correctly derived from video title.
- mode=video → 200, content-type: video/mp4, valid ISO-Media MP4 file returned (verified with file).
- This closes the live-download risk flagged in the T-001 review — yt-dlp + ffmpeg inside the container work correctly end-to-end for both modes.
Cleaned up: stopped test container, removed the yt-dl-review test image, removed downloaded test files.

Findings from verification: All acceptance criteria for T-002 hold:

docker build -t yt-dl . exits 0.
docker run --rm -p 8080:8080 yt-dl starts the server.
curl http://localhost:8080/ returns HTML with both download buttons.
(Bonus, beyond stated AC) A real MP3 and a real video download both succeed end-to-end through the container.

Risks:

None outstanding. The live-download risk noted in the T-001 review has been verified and closed here.