# 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

### Findings

1. **[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.

### Verification

**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:
- `GET /` returns 200 with HTML containing "Download MP3".
- `POST /download` with a valid URL + mode streams a file.
- Invalid URL returns 400 JSON with `.message`.

**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.