From 5131f3ccf64d52087e8a0ecd18b1d023e2b9fbed Mon Sep 17 00:00:00 2001 From: Travis Herbranson Date: Mon, 25 May 2026 16:08:26 -0400 Subject: [PATCH] web+cli: edit-mode for source metadata + retry/re-run actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds per-record edit mode and retry to the source detail page, plus a CLI `second-brain retry ` for parity. Edit mode exposes domain (select), title, focus on /sources/{id}. URL stays read-only — it's the UNIQUE dedupe key and the embeddings identity tuple. Save goes through a new service helper `update_source_metadata` so the web route stays a thin wrapper. Retry exposes two modes, both available on any record: - full: status -> PENDING (re-pull + re-process) - extract: status -> TRANSCRIBED (re-extract only, requires an existing transcript; offered when source_type=video and a transcript is present, to avoid the expensive re-download path) Both modes clear claimed_by/claimed_at and error_message so the queue dance picks it up cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/second_brain/main.py | 42 +++ src/second_brain/sources_service.py | 112 ++++++++ src/second_brain/web/app.py | 141 ++++++++-- .../web/templates/source_detail.html | 92 ++++++- tests/test_source_edit_retry.py | 258 ++++++++++++++++++ 5 files changed, 615 insertions(+), 30 deletions(-) create mode 100644 tests/test_source_edit_retry.py diff --git a/src/second_brain/main.py b/src/second_brain/main.py index 29d870e..57854a1 100644 --- a/src/second_brain/main.py +++ b/src/second_brain/main.py @@ -4,6 +4,7 @@ second-brain CLI entry point. Commands: add — queue a source URL (video or article) process — run the full pipeline on pending/pulled/transcribed sources + retry — reset a source so the pipeline re-processes it serve — launch the FastAPI web UI compile — run the wiki compiler on accepted sources schedule — start the overnight scheduler @@ -307,6 +308,47 @@ def serve(host: str, port: int, reload: bool) -> None: ) +# --------------------------------------------------------------------------- +# retry +# --------------------------------------------------------------------------- + + +@cli.command() +@click.argument("source_id", type=int) +@click.option( + "--mode", + type=click.Choice(["full", "extract"]), + default="full", + show_default=True, + help="`full` resets to PENDING (re-pull + re-process); " + "`extract` resets to TRANSCRIBED (requires existing transcript).", +) +def retry(source_id: int, mode: str) -> None: + """Reset a source so the pipeline re-processes it. + + Clears the claim pair and error_message and rewinds status to the + appropriate stage. Parity with the dashboard re-run buttons. + """ + from second_brain.sources_service import reset_source_for_retry + + db = get_database() + try: + with db.session() as sess: + source = reset_source_for_retry( + sess, source_id=source_id, mode=mode + ) + new_status = source.status.value + url = source.url + except LookupError as exc: + click.echo(f"[retry] {exc}", err=True) + sys.exit(2) + except ValueError as exc: + click.echo(f"[retry] {exc}", err=True) + sys.exit(2) + + click.echo(f"[retry] id={source_id} status → {new_status} ({url})") + + # --------------------------------------------------------------------------- # compile # --------------------------------------------------------------------------- diff --git a/src/second_brain/sources_service.py b/src/second_brain/sources_service.py index ac59244..109e999 100644 --- a/src/second_brain/sources_service.py +++ b/src/second_brain/sources_service.py @@ -275,14 +275,126 @@ def add_playlist( return result +# --------------------------------------------------------------------------- +# Edit + retry helpers (shared by web routes and CLI) +# --------------------------------------------------------------------------- + + +# Fields the edit-mode form is allowed to mutate. URL is intentionally +# excluded — it's the UNIQUE dedupe key and editing it risks collisions +# plus breaks the embeddings 4-tuple identity (source_id is stable but +# semantic identity drifts). +EDITABLE_METADATA_FIELDS = ("domain", "title", "focus") + + +def update_source_metadata( + sess: Session, + *, + source_id: int, + domain: Optional[str] = None, + title: Optional[str] = None, + focus: Optional[str] = None, +) -> Source: + """Update editable metadata on a source row. + + Empty strings collapse to NULL for title/focus (matches `add_source` + semantics). Domain is required to stay in the configured set — + bumping the constraint is a separate decision. + + Raises ValueError on validation failures and LookupError if the row + is missing. Caller owns the transaction boundary. + """ + source = sess.query(Source).filter(Source.id == source_id).first() + if source is None: + raise LookupError(f"source id={source_id} not found") + + if domain is not None: + if domain not in DOMAINS: + raise ValueError( + f"unknown domain {domain!r}; valid: {', '.join(DOMAINS)}" + ) + source.domain = domain + + if title is not None: + source.title = title.strip() or None + + if focus is not None: + source.focus = focus.strip() or None + + source.updated_at = utcnow() + sess.flush() + return source + + +# Retry modes — keep the surface small. Anything else is a separate ask. +RETRY_FULL = "full" +RETRY_EXTRACT = "extract" +RETRY_MODES = (RETRY_FULL, RETRY_EXTRACT) + + +def reset_source_for_retry( + sess: Session, + *, + source_id: int, + mode: str = RETRY_FULL, +) -> Source: + """Reset a source so the pipeline re-processes it. + + Modes: + - "full": status → PENDING. Article pull or video download + + transcribe will happen again on the appropriate worker. + - "extract": status → TRANSCRIBED. Requires the row to already + carry a transcript (transcript_text). The dev scheduler will + re-run extraction without re-pulling the source. Cheap. + + Always clears `claimed_by` / `claimed_at` / `error_message` so the + queue dance can pick it up cleanly and stale errors don't bleed + into the UI. + + Raises ValueError on bad mode / missing transcript for extract mode, + and LookupError if the row is missing. + """ + if mode not in RETRY_MODES: + raise ValueError( + f"unknown retry mode {mode!r}; valid: {', '.join(RETRY_MODES)}" + ) + + source = sess.query(Source).filter(Source.id == source_id).first() + if source is None: + raise LookupError(f"source id={source_id} not found") + + if mode == RETRY_EXTRACT: + if not (source.transcript_text and source.transcript_text.strip()): + raise ValueError( + "extract-only retry requires an existing transcript; " + "use full retry instead" + ) + source.status = SourceStatus.TRANSCRIBED + else: + source.status = SourceStatus.PENDING + + source.claimed_by = None + source.claimed_at = None + source.error_message = None + source.updated_at = utcnow() + sess.flush() + return source + + __all__ = [ "AddResult", "DEFAULT_PLAYLIST_MAX_ITEMS", + "EDITABLE_METADATA_FIELDS", "PlaylistAddResult", + "RETRY_EXTRACT", + "RETRY_FULL", + "RETRY_MODES", "add_playlist", "add_source", "expand_youtube_playlist", "is_article_url", "is_youtube_playlist_url", + "reset_source_for_retry", + "update_source_metadata", "validate_url", ] diff --git a/src/second_brain/web/app.py b/src/second_brain/web/app.py index b6eff16..b8845ff 100644 --- a/src/second_brain/web/app.py +++ b/src/second_brain/web/app.py @@ -31,9 +31,14 @@ from second_brain.settings_store import ( ) from second_brain.sources_service import ( DEFAULT_PLAYLIST_MAX_ITEMS, + RETRY_EXTRACT, + RETRY_FULL, + RETRY_MODES, add_playlist, add_source, is_youtube_playlist_url, + reset_source_for_retry, + update_source_metadata, ) # --------------------------------------------------------------------------- @@ -120,16 +125,33 @@ async def index( ) -@app.get("/sources/{source_id}", response_class=HTMLResponse) -async def source_detail(request: Request, source_id: int): - """Source detail page with extraction results and accept/reject actions.""" +def _render_source_detail( + request: Request, + source_id: int, + *, + edit: bool = False, + flash: Optional[str] = None, + error: Optional[str] = None, + form_overrides: Optional[dict] = None, +) -> HTMLResponse: + """Single render path for /sources/{id} — used by GET + every HTMX + mutation endpoint so each action gets a consistent re-render. + + `form_overrides` lets a failed edit-save bounce show the user's + in-progress values instead of the row's stored values. + """ db = get_database() with db.session() as sess: source = sess.query(Source).filter(Source.id == source_id).first() if not source: raise HTTPException(status_code=404, detail="Source not found") source_data = _source_to_dict(source) - extraction_data = _extraction_to_dict(source.extraction) if source.extraction else None + extraction_data = ( + _extraction_to_dict(source.extraction) if source.extraction else None + ) + + if form_overrides: + source_data = {**source_data, **form_overrides} return templates.TemplateResponse( request, @@ -137,10 +159,25 @@ async def source_detail(request: Request, source_id: int): { "source": source_data, "extraction": extraction_data, + "edit_mode": edit, + "flash": flash, + "error": error, + "domains": DOMAINS, }, ) +@app.get("/sources/{source_id}", response_class=HTMLResponse) +async def source_detail(request: Request, source_id: int, edit: bool = False): + """Source detail page with extraction results and accept/reject actions. + + `?edit=1` opens the metadata editor inline. Editing exposes domain, + title, and focus — URL stays read-only since it's the UNIQUE dedupe + key for the queue and the embeddings identity tuple. + """ + return _render_source_detail(request, source_id, edit=edit) + + _IN_FLIGHT_STATUSES = [ SourceStatus.PENDING, SourceStatus.PULLED, @@ -192,17 +229,11 @@ async def accept_source(request: Request, source_id: int): if not source: raise HTTPException(status_code=404, detail="Source not found") source.status = SourceStatus.ACCEPTED - source_data = _source_to_dict(source) - extraction_data = _extraction_to_dict(source.extraction) if source.extraction else None - return templates.TemplateResponse( + return _render_source_detail( request, - "source_detail.html", - { - "source": source_data, - "extraction": extraction_data, - "flash": "Accepted — will be included in next wiki compile.", - }, + source_id, + flash="Accepted — will be included in next wiki compile.", ) @@ -215,18 +246,79 @@ async def reject_source(request: Request, source_id: int): if not source: raise HTTPException(status_code=404, detail="Source not found") source.status = SourceStatus.FAILED - source_data = _source_to_dict(source) - extraction_data = _extraction_to_dict(source.extraction) if source.extraction else None - return templates.TemplateResponse( - request, - "source_detail.html", - { - "source": source_data, - "extraction": extraction_data, - "flash": "Rejected.", - }, - ) + return _render_source_detail(request, source_id, flash="Rejected.") + + +@app.post("/sources/{source_id}/edit", response_class=HTMLResponse) +async def edit_source_metadata( + request: Request, + source_id: int, + domain: str = Form(...), + title: Optional[str] = Form(None), + focus: Optional[str] = Form(None), +): + """HTMX edit-mode save. Updates domain/title/focus only — URL is + intentionally not editable (see service-layer comment).""" + db = get_database() + try: + with db.session() as sess: + update_source_metadata( + sess, + source_id=source_id, + domain=domain, + title=title, + focus=focus, + ) + except LookupError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + except ValueError as exc: + # Bounce back to edit mode with the user's in-progress values so + # they don't have to retype them. + return _render_source_detail( + request, + source_id, + edit=True, + error=str(exc), + form_overrides={ + "domain": domain, + "title": title or "", + "focus": focus or "", + }, + ) + + return _render_source_detail(request, source_id, flash="Saved.") + + +@app.post("/sources/{source_id}/retry", response_class=HTMLResponse) +async def retry_source( + request: Request, + source_id: int, + mode: str = Form(RETRY_FULL), +): + """HTMX retry/re-run. `mode=full` → PENDING (re-pull + re-process). + `mode=extract` → TRANSCRIBED (re-extract only; requires an existing + transcript). Always clears the claim pair and error_message.""" + if mode not in RETRY_MODES: + raise HTTPException(status_code=400, detail=f"unknown mode {mode!r}") + + db = get_database() + try: + with db.session() as sess: + source = reset_source_for_retry( + sess, source_id=source_id, mode=mode + ) + new_status = source.status.value + except LookupError as exc: + raise HTTPException(status_code=404, detail=str(exc)) from exc + except ValueError as exc: + return _render_source_detail(request, source_id, error=str(exc)) + + if mode == RETRY_EXTRACT: + flash = f"Re-queued for extraction (status → {new_status})." + else: + flash = f"Re-queued from scratch (status → {new_status})." + return _render_source_detail(request, source_id, flash=flash) # --------------------------------------------------------------------------- @@ -286,6 +378,7 @@ def _source_to_dict(s: Source) -> dict: "focus": s.focus, "ingested_at": s.ingested_at.strftime("%Y-%m-%d %H:%M") if s.ingested_at else "", "has_extraction": s.extraction is not None, + "has_transcript": bool((s.transcript_text or "").strip()), "error_message": s.error_message, } diff --git a/src/second_brain/web/templates/source_detail.html b/src/second_brain/web/templates/source_detail.html index 389b93e..fa6cf59 100644 --- a/src/second_brain/web/templates/source_detail.html +++ b/src/second_brain/web/templates/source_detail.html @@ -11,9 +11,19 @@ Back to sources -

{{ source.title }}

- {{ source.url }} +
+
+

{{ source.title }}

+ {{ source.url }} +
+ {% if not edit_mode %} + + Edit + + {% endif %} +
{% if flash %} @@ -21,7 +31,54 @@ {{ flash }} {% endif %} +{% if error %} +
+ {{ error }} +
+{% endif %} +{% if edit_mode %} + +
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
+ + + Cancel + +
+
+{% else %}
{% set status_colors = { @@ -60,9 +117,9 @@
{% endif %} - -{% if source.status == 'analyzed' %} -
+ +
+ {% if source.status == 'analyzed' %} + {% endif %} + + + + + {% if source.has_transcript and source.source_type == 'video' %} + + + {% endif %}
{% endif %} diff --git a/tests/test_source_edit_retry.py b/tests/test_source_edit_retry.py new file mode 100644 index 0000000..c3177f6 --- /dev/null +++ b/tests/test_source_edit_retry.py @@ -0,0 +1,258 @@ +"""Tests for the source edit-metadata + retry-reset service helpers. + +Live-DB pattern matches `tests/test_claim.py` / `tests/test_playlist.py`: +both groups auto-skip when no petalbrain connection is configured. The +service layer is where edit/retry logic lives (the web routes + CLI are +thin wrappers around it), so this is where the meaningful coverage is. +""" + +from __future__ import annotations + +import os +import uuid + +import psycopg +import pytest + + +def _db_reachable() -> bool: + url = os.environ.get("SECOND_BRAIN_DATABASE_URL") or os.environ.get( + "HERBYLAB_DATABASE_URL" + ) + if not url: + return False + raw = url.replace("postgresql+psycopg://", "postgresql://", 1) + try: + with psycopg.connect(raw, connect_timeout=2): + return True + except Exception: + return False + + +pytestmark = pytest.mark.skipif( + not _db_reachable(), + reason="needs SECOND_BRAIN_DATABASE_URL pointed at a real petalbrain", +) + + +def _seed_source(db, **overrides): + """Insert a Source row and return (id, url). Caller cleans up by id.""" + from second_brain.models import Source, SourceStatus, SourceType, utcnow + + tag = uuid.uuid4().hex[:8] + defaults = dict( + url=f"https://example.test/edit-retry/{tag}", + title=f"seed-{tag}", + domain="development", + source_type=SourceType.ARTICLE, + status=SourceStatus.PENDING, + ingested_at=utcnow(), + ) + defaults.update(overrides) + with db.session() as sess: + src = Source(**defaults) + sess.add(src) + sess.flush() + return src.id, src.url + + +def _cleanup(db, source_id): + from second_brain.models import Source + + with db.session() as sess: + sess.query(Source).filter(Source.id == source_id).delete( + synchronize_session=False + ) + + +# --------------------------------------------------------------------------- +# update_source_metadata +# --------------------------------------------------------------------------- + + +def test_update_metadata_writes_domain_title_focus(): + from second_brain.database import Database, reset_database_singleton + from second_brain.models import Source + from second_brain.sources_service import update_source_metadata + + reset_database_singleton() + db = Database() + sid, _ = _seed_source(db) + try: + with db.session() as sess: + update_source_metadata( + sess, + source_id=sid, + domain="homelab", + title="new title", + focus="new focus directive", + ) + with db.session() as sess: + row = sess.query(Source).filter(Source.id == sid).first() + assert row.domain == "homelab" + assert row.title == "new title" + assert row.focus == "new focus directive" + assert row.updated_at is not None + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_update_metadata_blank_strings_become_null(): + from second_brain.database import Database, reset_database_singleton + from second_brain.models import Source + from second_brain.sources_service import update_source_metadata + + reset_database_singleton() + db = Database() + sid, _ = _seed_source(db) + try: + with db.session() as sess: + update_source_metadata( + sess, source_id=sid, domain="development", title=" ", focus="" + ) + with db.session() as sess: + row = sess.query(Source).filter(Source.id == sid).first() + assert row.title is None + assert row.focus is None + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_update_metadata_rejects_unknown_domain(): + from second_brain.database import Database, reset_database_singleton + from second_brain.sources_service import update_source_metadata + + reset_database_singleton() + db = Database() + sid, _ = _seed_source(db) + try: + with db.session() as sess: + with pytest.raises(ValueError, match="unknown domain"): + update_source_metadata(sess, source_id=sid, domain="not-a-domain") + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_update_metadata_missing_source_raises_lookup(): + from second_brain.database import Database, reset_database_singleton + from second_brain.sources_service import update_source_metadata + + reset_database_singleton() + db = Database() + try: + with db.session() as sess: + with pytest.raises(LookupError): + update_source_metadata( + sess, source_id=-99999, domain="development" + ) + finally: + reset_database_singleton() + + +# --------------------------------------------------------------------------- +# reset_source_for_retry +# --------------------------------------------------------------------------- + + +def test_retry_full_resets_to_pending_and_clears_claim(): + from second_brain.database import Database, reset_database_singleton + from second_brain.models import Source, SourceStatus, utcnow + from second_brain.sources_service import reset_source_for_retry + + reset_database_singleton() + db = Database() + sid, _ = _seed_source( + db, + status=SourceStatus.FAILED, + error_message="boom", + claimed_by="worker-x", + claimed_at=utcnow(), + ) + try: + with db.session() as sess: + reset_source_for_retry(sess, source_id=sid, mode="full") + with db.session() as sess: + row = sess.query(Source).filter(Source.id == sid).first() + assert row.status == SourceStatus.PENDING + assert row.claimed_by is None + assert row.claimed_at is None + assert row.error_message is None + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_retry_extract_sets_transcribed_when_transcript_present(): + from second_brain.database import Database, reset_database_singleton + from second_brain.models import Source, SourceStatus + from second_brain.sources_service import reset_source_for_retry + + reset_database_singleton() + db = Database() + sid, _ = _seed_source( + db, + status=SourceStatus.ANALYZED, + transcript_text="word " * 50, + error_message="prior failure", + ) + try: + with db.session() as sess: + reset_source_for_retry(sess, source_id=sid, mode="extract") + with db.session() as sess: + row = sess.query(Source).filter(Source.id == sid).first() + assert row.status == SourceStatus.TRANSCRIBED + assert row.error_message is None + assert row.transcript_text # transcript preserved + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_retry_extract_without_transcript_raises(): + from second_brain.database import Database, reset_database_singleton + from second_brain.models import SourceStatus + from second_brain.sources_service import reset_source_for_retry + + reset_database_singleton() + db = Database() + sid, _ = _seed_source(db, status=SourceStatus.PENDING, transcript_text=None) + try: + with db.session() as sess: + with pytest.raises(ValueError, match="requires an existing transcript"): + reset_source_for_retry(sess, source_id=sid, mode="extract") + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_retry_unknown_mode_raises(): + from second_brain.database import Database, reset_database_singleton + from second_brain.sources_service import reset_source_for_retry + + reset_database_singleton() + db = Database() + sid, _ = _seed_source(db) + try: + with db.session() as sess: + with pytest.raises(ValueError, match="unknown retry mode"): + reset_source_for_retry(sess, source_id=sid, mode="bogus") + finally: + _cleanup(db, sid) + reset_database_singleton() + + +def test_retry_missing_source_raises_lookup(): + from second_brain.database import Database, reset_database_singleton + from second_brain.sources_service import reset_source_for_retry + + reset_database_singleton() + db = Database() + try: + with db.session() as sess: + with pytest.raises(LookupError): + reset_source_for_retry(sess, source_id=-99999, mode="full") + finally: + reset_database_singleton()