wip: estado de trabajo pendiente antes de la vista grid (suite 230 verde)
This commit is contained in:
@@ -0,0 +1,308 @@
|
||||
# Discovery-Only Scrape Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Add one-channel and all-channel discovery jobs that register new YouTube videos as `pending` without downloading or processing their content.
|
||||
|
||||
**Architecture:** Extend the existing sequential `JobManager` with `opts.mode == "discover"`. Reuse `POST /api/scrape`, its SSE stream, and its job widget; add UI actions in Channels and Videos that pass either a channel ID or no channel ID. Keep the existing full scrape and processing endpoints unchanged.
|
||||
|
||||
**Tech Stack:** Python 3.10+, FastAPI, SQLite via the existing `Store`, yt-dlp flat discovery, Alpine.js CDN SPA, pytest.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Discovery-only jobs must not call `process_video()`.
|
||||
- Discovery-only jobs must not download transcripts, Markdown, audio, thumbnails, or avatars.
|
||||
- All HTTP requests must use the existing job queue and SSE event stream.
|
||||
- All database access must remain inside `Store` methods.
|
||||
- Preserve existing user changes in the dirty worktree and edit only the files listed below.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Make catalog insertion counts accurate
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/yt_scraper/store.py:266-283`
|
||||
- Test: `tests/test_store_platform.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: existing `Store.upsert_videos(refs: list[VideoRef])` callers.
|
||||
- Produces: the same integer return type, now equal to the number of distinct references that were not already present.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Add this test after the existing store filtering tests:
|
||||
|
||||
```python
|
||||
def test_upsert_videos_reports_only_new_rows(seeded_store):
|
||||
inserted = seeded_store.upsert_videos([
|
||||
VideoRef("v1", "UC1", "Updated title", "https://y/watch?v=v1", "20240101", 120),
|
||||
VideoRef("v4", "UC1", "New video", "https://y/watch?v=v4", "20240501", 180),
|
||||
])
|
||||
|
||||
assert inserted == 1
|
||||
assert seeded_store.get_video("v1").title == "Updated title"
|
||||
assert seeded_store.get_video("v4").status == "pending"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused test to verify it fails**
|
||||
|
||||
Run: `python -m pytest tests/test_store_platform.py::test_upsert_videos_reports_only_new_rows -q`
|
||||
|
||||
Expected: FAIL because SQLite's `ON CONFLICT DO UPDATE` currently makes `rowcount` nonzero for an existing row.
|
||||
|
||||
- [ ] **Step 3: Implement the minimal count fix**
|
||||
|
||||
Inside `Store.upsert_videos`, load the existing IDs for the incoming references before the loop, deduplicate the incoming IDs with a set, and increment `inserted` only when an incoming distinct ID was absent. Keep the current upsert SQL so existing rows still refresh title, upload date, and duration.
|
||||
|
||||
The essential implementation shape is:
|
||||
|
||||
```python
|
||||
incoming = {r.video_id for r in refs}
|
||||
with self._cursor() as cur:
|
||||
existing = set()
|
||||
if incoming:
|
||||
placeholders = ",".join("?" for _ in incoming)
|
||||
cur.execute(
|
||||
f"SELECT video_id FROM videos WHERE video_id IN ({placeholders})",
|
||||
list(incoming),
|
||||
)
|
||||
existing = {row["video_id"] for row in cur.fetchall()}
|
||||
for r in refs:
|
||||
# existing SQL remains here
|
||||
if r.video_id not in existing:
|
||||
inserted += 1
|
||||
existing.add(r.video_id)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the focused and regression tests**
|
||||
|
||||
Run: `python -m pytest tests/test_store_platform.py -q`
|
||||
|
||||
Expected: PASS for all store tests.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
Do not commit automatically in this workspace unless the user explicitly requests a commit. Leave the focused diff ready for review.
|
||||
|
||||
### Task 2: Add discovery-only job execution
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/yt_scraper/webapp/jobs.py:88-269`
|
||||
- Test: `tests/test_webapp_jobs.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `JobManager.enqueue(channel_id, opts)` with `opts={"mode": "discover"}` and optional `channel_id`.
|
||||
- Produces: queued jobs whose SSE events contain per-channel discovery counts and whose terminal event contains a summary.
|
||||
|
||||
- [ ] **Step 1: Write failing job tests**
|
||||
|
||||
Append tests using a fake `discover_channel` and a fake `process_video`:
|
||||
|
||||
```python
|
||||
def test_discovery_job_registers_new_videos_without_processing(tmp_path, monkeypatch):
|
||||
store = Store(tmp_path / "state.db")
|
||||
store.upsert_channel("UC1", "@alpha", "Alpha", 1)
|
||||
store.upsert_videos([
|
||||
VideoRef("old", "UC1", "Old", "https://y/watch?v=old", "20240101", 60)
|
||||
])
|
||||
store.mark_status("old", "done")
|
||||
store.create_job("discover-job", "UC1", {"mode": "discover"})
|
||||
calls = []
|
||||
|
||||
def fake_discover(url, sleep_subrequests=2.0):
|
||||
calls.append(url)
|
||||
return ("UC1", "Alpha", None, [
|
||||
VideoRef("new", "UC1", "New", "https://y/watch?v=new", "20240501", 90),
|
||||
VideoRef("old", "UC1", "Old", "https://y/watch?v=old", "20240101", 60),
|
||||
])
|
||||
|
||||
processed = []
|
||||
monkeypatch.setattr("yt_scraper.webapp.jobs.discover_channel", fake_discover)
|
||||
monkeypatch.setattr("yt_scraper.webapp.jobs.process_video", lambda *a, **k: processed.append(a))
|
||||
manager = JobManager(store, Config(database_path=str(tmp_path / "state.db"), output_dir=str(tmp_path / "markdown")))
|
||||
|
||||
manager._run_job("discover-job")
|
||||
|
||||
assert calls == ["https://www.youtube.com/@alpha/videos"]
|
||||
assert store.get_video("new").status == "pending"
|
||||
assert store.get_video("old").status == "done"
|
||||
assert processed == []
|
||||
assert store.get_job("discover-job").status == "done"
|
||||
assert any(event["event"] == "done" and event["data"]["new_videos"] == 1
|
||||
for event in manager.events_since("discover-job", 0))
|
||||
|
||||
|
||||
def test_all_channel_discovery_continues_after_one_error(tmp_path, monkeypatch):
|
||||
store = Store(tmp_path / "state.db")
|
||||
store.upsert_channel("UC1", "@one", "One", 0)
|
||||
store.upsert_channel("UC2", "@two", "Two", 0)
|
||||
store.create_job("discover-all", None, {"mode": "discover"})
|
||||
|
||||
def fake_discover(url, sleep_subrequests=2.0):
|
||||
if "@one" in url:
|
||||
raise RuntimeError("temporary failure")
|
||||
return ("UC2", "Two", None, [VideoRef("new2", "UC2", "New", "https://y/watch?v=new2")])
|
||||
|
||||
monkeypatch.setattr("yt_scraper.webapp.jobs.discover_channel", fake_discover)
|
||||
manager = JobManager(store, Config(database_path=str(tmp_path / "state.db"), output_dir=str(tmp_path / "markdown")))
|
||||
|
||||
manager._run_job("discover-all")
|
||||
|
||||
assert store.get_video("new2").status == "pending"
|
||||
assert store.get_job("discover-all").status == "done"
|
||||
done = [e for e in manager.events_since("discover-all", 0) if e["event"] == "done"][-1]
|
||||
assert done["data"]["errors"] == 1
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused tests to verify they fail**
|
||||
|
||||
Run: `python -m pytest tests/test_webapp_jobs.py::test_discovery_job_registers_new_videos_without_processing tests/test_webapp_jobs.py::test_all_channel_discovery_continues_after_one_error -q`
|
||||
|
||||
Expected: FAIL because `_run_job` currently routes both cases to `_run_channel`, which tries to process pending videos and cannot interpret an all-channel discovery job.
|
||||
|
||||
- [ ] **Step 3: Implement discovery dispatch and execution**
|
||||
|
||||
In `JobManager._run_job`, route `opts.get("mode") == "discover"` to a new `_run_discovery(job_id)` before the existing audio/video/channel branches.
|
||||
|
||||
Implement `_run_discovery` with this behavior:
|
||||
|
||||
```python
|
||||
def _run_discovery(self, job_id: str) -> None:
|
||||
job = self.store.get_job(job_id)
|
||||
if not job:
|
||||
return
|
||||
channels = ([self.store.get_channel(job.channel_id)] if job.channel_id
|
||||
else self.store.list_channels())
|
||||
channels = [c for c in channels if c]
|
||||
self.store.update_job(job_id, status="running", total=len(channels), completed=0)
|
||||
completed = 0
|
||||
totals = {"new_videos": 0, "known_videos": 0, "errors": 0}
|
||||
for channel in channels:
|
||||
if job_id in self._cancel:
|
||||
self.store.update_job(job_id, status="cancelled", completed=completed, finished=True)
|
||||
self._emit(job_id, "cancelled", {"completed": completed, "total": len(channels)})
|
||||
return
|
||||
try:
|
||||
# Build the stored handle/channel URL, call discover_channel, filter
|
||||
# using cfg.include_shorts/cfg.include_live, upsert the channel and refs.
|
||||
# Do not call deep_channel_avatar, cache helpers, or process_video.
|
||||
new_count = self.store.upsert_videos(refs)
|
||||
known_count = len({r.video_id for r in refs}) - new_count
|
||||
totals["new_videos"] += new_count
|
||||
totals["known_videos"] += known_count
|
||||
self._emit(job_id, "progress", {"channel_id": channel["channel_id"], "new_videos": new_count, "known_videos": known_count, "completed": completed + 1, "total": len(channels)})
|
||||
except Exception as exc:
|
||||
totals["errors"] += 1
|
||||
self._emit(job_id, "log", {"msg": f"discovery failed for {channel.get('name') or channel['channel_id']}: {exc}"})
|
||||
completed += 1
|
||||
self.store.update_job(job_id, completed=completed)
|
||||
self.store.update_job(job_id, status="done", completed=completed, finished=True)
|
||||
self._emit(job_id, "done", {"completed": completed, "total": len(channels), **totals})
|
||||
```
|
||||
|
||||
Use the existing `_resolve_channel_url` for each stored channel. Preserve the existing processing path untouched. For a single-channel exception, emit an error terminal event/status; for all-channel jobs, continue and report the error count as above.
|
||||
|
||||
- [ ] **Step 4: Run focused job tests**
|
||||
|
||||
Run: `python -m pytest tests/test_webapp_jobs.py -q`
|
||||
|
||||
Expected: PASS for both existing audio tests and the new discovery tests.
|
||||
|
||||
- [ ] **Step 5: Run the full Python suite**
|
||||
|
||||
Run: `python -m pytest tests/ -q`
|
||||
|
||||
Expected: all tests pass with no network access.
|
||||
|
||||
### Task 3: Expose the discovery mode through the API
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/yt_scraper/webapp/api.py:111-117`
|
||||
- Test: `tests/test_webapp_jobs.py` (job boundary coverage; no API test fixture exists in the repository).
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: JSON `{"channel_id": "UC...", "opts": {"mode": "discover"}}` or the same body without `channel_id` for all channels.
|
||||
- Produces: the existing `{"job_id": "..."}` response and a queued `JobManager` job.
|
||||
|
||||
- [ ] **Step 1: Add request-shape coverage at the job boundary**
|
||||
|
||||
Extend the discovery job tests to enqueue `manager.enqueue(None, {"mode": "discover"})` and verify that it completes as an all-channel job. This covers the exact payload shape the API forwards; the repository has no existing FastAPI router test fixture.
|
||||
|
||||
- [ ] **Step 2: Implement the minimal API change**
|
||||
|
||||
Keep the endpoint response unchanged. Normalize discovery options and permit a missing channel only for discovery:
|
||||
|
||||
```python
|
||||
opts = (payload or {}).get("opts", {}) or {}
|
||||
if opts.get("mode") == "discover":
|
||||
opts = {"mode": "discover"}
|
||||
job_id = jobs.enqueue(channel_id, opts)
|
||||
return {"job_id": job_id}
|
||||
```
|
||||
|
||||
Do not add a second synchronous discovery endpoint.
|
||||
|
||||
- [ ] **Step 3: Run the API/job regression tests**
|
||||
|
||||
Run: `python -m pytest tests/test_webapp_jobs.py tests/test_store_platform.py -q`
|
||||
|
||||
Expected: PASS.
|
||||
|
||||
### Task 4: Add one-channel and all-channel UI controls
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/yt_scraper/webapp/static/app.js:45-73, 758-815`
|
||||
- Modify: `src/yt_scraper/webapp/static/index.html:167-229, 231-319`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `POST /api/scrape` discovery jobs and SSE `progress`/`done` payloads.
|
||||
- Produces: buttons for channel-specific and all-channel discovery, with refresh and result feedback.
|
||||
|
||||
- [ ] **Step 1: Add the failing static contract checks**
|
||||
|
||||
Before editing, use a lightweight repository check to confirm the new labels and method are absent:
|
||||
|
||||
Run: `rg "startDiscovery|Investigar nuevos|Investigar todos" src/yt_scraper/webapp/static`
|
||||
|
||||
Expected: no matches.
|
||||
|
||||
- [ ] **Step 2: Add the Alpine discovery action**
|
||||
|
||||
Add `startDiscovery(channelId)` near `startScrape()`. It posts `{channel_id: channelId || null, opts: {mode: "discover"}}`, subscribes with kind `discovery`, and prevents duplicate active jobs. Extend the SSE `done` handler to parse the payload and show `N video(s) nuevo(s) encontrado(s)` for discovery jobs. Refresh channels, videos, and dashboard after completion.
|
||||
|
||||
- [ ] **Step 3: Add controls to Channels and Videos**
|
||||
|
||||
In Channels, add the global header button and one row button calling `startDiscovery(c.channel_id)`. In Videos, add a header action calling `startDiscovery(filters.channel || null)`. Disable each while `jobActive()` is true and while the channel list is empty. Preserve `.md`, `Process`, and `Audio` actions.
|
||||
|
||||
- [ ] **Step 4: Verify the static contract and inspect the rendered paths**
|
||||
|
||||
Run: `rg "startDiscovery|Investigar nuevos|Investigar todos|mode: \"discover\"" src/yt_scraper/webapp/static`
|
||||
|
||||
Expected: matches in `app.js` and `index.html` for the method, payload, and both scopes. Inspect the changed Alpine expressions to ensure the existing `app.js` script remains before Alpine in `index.html`.
|
||||
|
||||
### Task 5: Final verification and review
|
||||
|
||||
**Files:**
|
||||
- Review: `src/yt_scraper/store.py`
|
||||
- Review: `src/yt_scraper/webapp/jobs.py`
|
||||
- Review: `src/yt_scraper/webapp/api.py`
|
||||
- Review: `src/yt_scraper/webapp/static/app.js`
|
||||
- Review: `src/yt_scraper/webapp/static/index.html`
|
||||
|
||||
- [ ] **Step 1: Run the complete test suite**
|
||||
|
||||
Run: `python -m pytest tests/ -q`
|
||||
|
||||
Expected: all tests pass.
|
||||
|
||||
- [ ] **Step 2: Review the diff for scope and unintended downloads**
|
||||
|
||||
Run: `git diff -- src/yt_scraper/store.py src/yt_scraper/webapp/jobs.py src/yt_scraper/webapp/api.py src/yt_scraper/webapp/static/app.js src/yt_scraper/webapp/static/index.html tests/test_store_platform.py tests/test_webapp_jobs.py`
|
||||
|
||||
Confirm discovery code contains no calls to `process_video`, `cache_thumbnail`, `cache_channel_avatar`, `extract_video`, or audio routes.
|
||||
|
||||
- [ ] **Step 3: Check worktree status**
|
||||
|
||||
Run: `git status --short`
|
||||
|
||||
Confirm only intended feature files and the two planning documents are changed or untracked; do not modify or revert unrelated existing user changes.
|
||||
Reference in New Issue
Block a user