From cc1f0cab93c1103d5e8a63b3aa420ea70cbf4677 Mon Sep 17 00:00:00 2001 From: ChemaVX Date: Wed, 5 Aug 2026 15:46:06 +0000 Subject: [PATCH] =?UTF-8?q?fix(short):=20cerrar=20el=20bucle=20de=20edici?= =?UTF-8?q?=C3=B3n=20y=20el=20desfase=20spec/v=C3=ADdeo=20en=20la=20subida?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dos huecos de la revisión: 1. /short_spec prometía re-renderizar el spec editado pero no existía el camino de vuelta. Ahora un .json adjunto se valida contra el contrato vivo (errores con su ruta verbatim), se re-comprueba el fundamento (la edición pudo meter una cifra nueva), se guarda como output nuevo y se renderiza. Sin LLM: este camino es gratis. La sesión sale del nombre del fichero (short_{id}_spec.json), que Telegram conserva al reenviar. 2. /upload_short podía subir un vídeo viejo con metadatos nuevos: produce guarda el spec ANTES de renderizar, así que un re-intento con render fallido deja en disco el MP4 de la vuelta anterior. Ahora se compara el mtime del vídeo con el created_at del spec y se niega (force lo salta). Co-Authored-By: Claude Fable 5 --- README.md | 14 ++- src/bot/bot.py | 176 ++++++++++++++++++++++++++++----- src/generator/short.py | 85 ++++++++++++++-- tests/test_bot_short_report.py | 33 ++++++- tests/test_short_producer.py | 89 +++++++++++++++++ 5 files changed, 363 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index a7e93fa..cff3cd4 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ OutputGenerator (Ollama) | `/finish` | Stop early, proceed to generation | | `/generate podcast\|blog\|report\|thread` | Generate output | | `/generate short_en` | Vertical Short: shot spec → grounding check → MP4 | -| `/short_spec` | Last shot spec as a JSON file, to hand-edit and re-render | +| `/short_spec` | Last shot spec as a JSON file; edit it and send it back to re-render free | | `/upload_short` | Upload the rendered Short to YouTube (private, for review) | | `/sources` | List all sources found | | `/cancel` | Cancel current research | @@ -84,6 +84,14 @@ Fallbacks hold throughout: if shortsmith is unreachable, the job errors, or the spec never validates, the spec JSON comes back as a file. The expensive part is the generation, not the render. +**Hand-editing loop:** `/short_spec` hands you the spec as +`short_{session_id}_spec.json`; edit it and send the file back to the bot. It +validates against the live contract (errors come back with their exact paths), +**re-runs the grounding check** — your edit may have introduced a new figure — +saves the edited spec as a new output, and renders. No LLM in that path: it is +free. The session comes from the filename, so it works even if the chat has +researched something else since. + Full spec of the phase: `docs/shortsmith-phase2-spec.md`. ## YouTube (`/upload_short`) @@ -92,7 +100,9 @@ Uploads `/data/shorts/{session_id}.mp4` to the channel with the title from the spec, a description carrying the article link and the sources the Short cites on screen, and tags derived from the topic. The YouTube URL is written back to the output row, so a second `/upload_short` on the same session refuses unless you -say `/upload_short force`. +say `/upload_short force`. It also refuses if the MP4 on disk is **older than +the latest saved spec** — that happens when a spec regeneration's render fails, +and uploading would put the new metadata on the old video. **Read this before setting it up.** Videos uploaded through `videos.insert` from an **unaudited API project** are [restricted to private viewing diff --git a/src/bot/bot.py b/src/bot/bot.py index b1dc966..5b799d1 100644 --- a/src/bot/bot.py +++ b/src/bot/bot.py @@ -5,6 +5,7 @@ Main user interface — all commands handled here import asyncio import json import os +import re import time from datetime import datetime, timedelta, timezone from pathlib import Path @@ -155,7 +156,8 @@ async def cmd_start(update: Update, ctx: ContextTypes.DEFAULT_TYPE): " Tipos: podcast|blog|report|thread\n" " Extended: podcast_extended|blog_extended|report_extended\n" "`/generate short_en` — Short vertical (vídeo) + informe de claims\n" - "`/short_spec` — Último shot spec como fichero JSON\n" + "`/short_spec` — Último shot spec como fichero JSON; edítalo y " + "mándamelo de vuelta para re-renderizar gratis\n" "`/upload_short` — Subir el Short a YouTube (privado, a revisar)\n" "`/sources` — List all sources found\n" "`/outputs` — List generated outputs\n" @@ -458,7 +460,9 @@ def _claims_message(result) -> str: if result.grounding: lines.append(result.grounding.summary()) else: - lines.append("⚠️ Sin comprobación de fundamento: no se llegó a escribir un spec.") + why = ("esta pasada no llegó a mirarlo" if result.spec + else "no se llegó a escribir un spec") + lines.append(f"⚠️ Sin comprobación de fundamento: {why}.") if result.render_warnings: lines.append("") @@ -536,27 +540,7 @@ async def cmd_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE): "🚫 Los Shorts están desactivados (`SHORTSMITH_ENABLED=false`).") return - if result.has_video: - await reporter.done("✅ Short renderizado") - caption = f"🎬 {result.title}" - if result.article_url: - caption += f"\n{result.article_url}" - caption += f"\n\n{session['topic']} · {result.duration_s:.0f}s" - with open(result.video_path, "rb") as f: - await update.message.reply_video( - video=f, - filename=f"short_{session_id}.mp4", - caption=caption[:1024], - supports_streaming=True, - write_timeout=180, - ) - else: - await reporter.done("⚠️ Short sin vídeo — te devuelvo el spec") - await _send_spec_file(update.message, result, session_id, - result.failure or "razón desconocida") - - # Informe de claims: SIEMPRE, y en su propio mensaje. - await update.message.reply_text(_claims_message(result)) + await _deliver_short(update.message, reporter, result, session) except Exception as e: logger.error("Short generation failed", error=str(e), exc_info=True) @@ -565,6 +549,31 @@ async def cmd_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE): await db_conn.close() +async def _deliver_short(message, reporter, result, session) -> None: + """El final común de `/generate short_en` y del re-render: vídeo si lo hay, + spec de vuelta si no, e informe de claims SIEMPRE, en su propio mensaje.""" + if result.has_video: + await reporter.done("✅ Short renderizado") + caption = f"🎬 {result.title}" + if result.article_url: + caption += f"\n{result.article_url}" + caption += f"\n\n{session['topic']} · {result.duration_s:.0f}s" + with open(result.video_path, "rb") as f: + await message.reply_video( + video=f, + filename=f"short_{session['id']}.mp4", + caption=caption[:1024], + supports_streaming=True, + write_timeout=180, + ) + else: + await reporter.done("⚠️ Short sin vídeo — te devuelvo el spec") + await _send_spec_file(message, result, session["id"], + result.failure or "razón desconocida") + + await message.reply_text(_claims_message(result)) + + async def cmd_short_spec(update: Update, ctx: ContextTypes.DEFAULT_TYPE): """Devuelve el último shot spec como fichero, para editarlo a mano y volver a renderizar sin pagar otra generación.""" @@ -594,7 +603,9 @@ async def cmd_short_spec(update: Update, ctx: ContextTypes.DEFAULT_TYPE): await update.message.reply_document( document=io.BytesIO(output["content"].encode("utf-8")), filename=f"short_{session['id']}_spec.json", - caption=f"🎬 Shot spec — {session['topic']}\n{created} UTC", + caption=f"🎬 Shot spec — {session['topic']}\n{created} UTC\n\n" + f"Edítalo y mándamelo de vuelta como fichero para " + f"re-renderizar sin pagar otra generación.", ) except Exception as e: logger.error("short_spec failed", error=str(e)) @@ -603,6 +614,92 @@ async def cmd_short_spec(update: Update, ctx: ContextTypes.DEFAULT_TYPE): await db_conn.close() +#: Tope de tamaño de un spec adjunto. El de Socorro son ~6 KB; 256 KB ya no es +#: un shot spec, es otra cosa que ha llegado aquí por accidente. +MAX_SPEC_FILE_BYTES = 256 * 1024 + +_SPEC_FILENAME = re.compile(r"short_(\d+)") + + +def _session_from_filename(name: Optional[str]) -> Optional[int]: + """La sesión que declara el nombre del fichero, si la declara. + + `/short_spec` nombra el fichero `short_{id}_spec.json` y Telegram conserva + el nombre al reenviarlo, así que el id del nombre manda sobre la sesión + activa: el spec editado es de ESA sesión aunque el chat haya investigado + otra cosa entre medias. + """ + match = _SPEC_FILENAME.search(name or "") + return int(match.group(1)) if match else None + + +async def handle_spec_document(update: Update, ctx: ContextTypes.DEFAULT_TYPE): + """Un `.json` adjunto es un shot spec editado: validar y re-renderizar. + + La vuelta de `/short_spec`. Sin LLM en este camino — renderizar de nuevo + es gratis, la parte cara fue la generación. + """ + if not is_authorized(update.effective_user.id): + return + doc = update.message.document + if not doc: + return + if (doc.file_size or 0) > MAX_SPEC_FILE_BYTES: + await update.message.reply_text( + "Ese fichero pesa demasiado para ser un shot spec.") + return + + db_conn = await get_db() + db = ResearchDB(db_conn) + try: + tg_file = await doc.get_file() + raw = bytes(await tg_file.download_as_bytearray()) + try: + spec = json.loads(raw.decode("utf-8")) + except (ValueError, UnicodeDecodeError) as e: + await update.message.reply_text( + f"No puedo leer ese JSON: {str(e)[:200]}") + return + if not isinstance(spec, dict) or "shots" not in spec: + await update.message.reply_text( + "Ese JSON no parece un shot spec (no tiene `shots`). El de " + "esta sesión te lo da /short_spec.") + return + + session = None + declared = _session_from_filename(doc.file_name) + if declared: + session = await db.get_session(declared) + if not session: + session = await _session_row(db_conn, update.effective_chat.id) + if not session: + await update.message.reply_text( + "No hay sesiones. Empieza con /research ") + return + + from src.generator.short import ShortProducer, ShortsDisabled + + reporter = ProgressReporter(update.message) + await reporter.start( + f"🎞 Re-renderizando spec editado — sesión #{session['id']}: " + f"{session['topic']}") + producer = ShortProducer(db, ContentProcessor(db, OllamaClient())) + try: + result = await producer.rerender(session["id"], spec, reporter.update) + except ShortsDisabled: + await reporter.done( + "🚫 Los Shorts están desactivados (`SHORTSMITH_ENABLED=false`).") + return + + await _deliver_short(update.message, reporter, result, session) + + except Exception as e: + logger.error("Spec re-render failed", error=str(e), exc_info=True) + await update.message.reply_text(f"❌ Re-render fallido: {str(e)[:300]}") + finally: + await db_conn.close() + + async def cmd_upload_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE): """`/upload_short [force]` — sube a YouTube el Short ya renderizado. @@ -658,6 +755,17 @@ async def cmd_upload_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE): f"`/generate short_en`.", parse_mode=ParseMode.MARKDOWN) return + if (_video_predates_spec(video_path.stat().st_mtime, + output["created_at"]) and not force): + await update.message.reply_text( + "⚠️ El vídeo en disco es ANTERIOR al último spec guardado: se " + "regeneró el spec pero el render no llegó a dejar vídeo nuevo. " + "Subirlo pondría metadatos nuevos a un vídeo viejo.\n\n" + "Re-renderiza con `/generate short_en` (o mándame el spec como " + "fichero `.json`). `/upload_short force` lo sube igualmente.", + parse_mode=ParseMode.MARKDOWN) + return + if output.get("published_url") and not force: await update.message.reply_text( f"Este Short ya está subido:\n{output['published_url']}\n\n" @@ -700,6 +808,23 @@ async def cmd_upload_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE): await db_conn.close() +#: Margen para relojes y redondeos del filesystem. Un render legítimo escribe +#: el MP4 ~1 minuto DESPUÉS de guardarse el spec; el caso malo (spec regenerado +#: con render fallido) deja un vídeo horas más viejo, no segundos. +_STALE_VIDEO_MARGIN_S = 5.0 + + +def _video_predates_spec(video_mtime: float, spec_created_at: float) -> bool: + """True si el MP4 en disco es anterior al último spec guardado. + + Pasa cuando `/generate short_en` se repite y el render falla: `produce` + guarda el spec ANTES de renderizar, así que en disco queda el vídeo de la + vuelta anterior. Subirlo con los metadatos del spec nuevo es un mismatch + silencioso — el vídeo dice una cosa y el título otra. + """ + return video_mtime + _STALE_VIDEO_MARGIN_S < spec_created_at + + def _upload_message(video, metadata: dict, article_url: Optional[str]) -> str: """El parte de la subida. Texto plano: lleva el título del modelo, y un Markdown desbalanceado haría que Telegram rechazara el mensaje que trae el @@ -1714,6 +1839,9 @@ def create_bot() -> Application: app.add_handler(CommandHandler("finish", cmd_finish)) app.add_handler(CommandHandler("generate", cmd_generate)) app.add_handler(CommandHandler("short_spec", cmd_short_spec)) + # Un .json adjunto es un spec editado que vuelve para re-renderizarse. + app.add_handler(MessageHandler(filters.Document.FileExtension("json"), + handle_spec_document)) app.add_handler(CommandHandler("upload_short", cmd_upload_short)) app.add_handler(CommandHandler("sources", cmd_sources)) app.add_handler(CommandHandler("outputs", cmd_outputs)) diff --git a/src/generator/short.py b/src/generator/short.py index 9c7a53a..61e5cab 100644 --- a/src/generator/short.py +++ b/src/generator/short.py @@ -31,6 +31,7 @@ from src.generator.shortsmith import ( ShortsmithClient, ShortsmithError, ShortsmithRejected, ShortsmithUnavailable, ) from src.generator.shortspec import ShortSpecWriter, SpecWriteFailed +from src.generator.spec_contract import SpecInvalid, editorial_notes, validate_spec from src.llm import get_anthropic_client logger = structlog.get_logger() @@ -217,6 +218,83 @@ class ShortProducer: logger.warning("No se pudo guardar el spec en outputs", error=str(e)) # 6. Render. + await self._render_guarded(result, session_id, progress_callback) + + if result.failure: + logger.warning("Short sin vídeo", session_id=session_id, why=result.failure) + logger.info("Short producido", session_id=session_id, + seconds=round(time.monotonic() - started, 1), + video=result.video_path, cost=round(result.cost_usd, 4)) + return result + + async def rerender(self, session_id: int, spec: dict, + progress_callback: Optional[Callable[[str], Any]] = None + ) -> ShortResult: + """Renderiza un spec editado a mano, sin pagar otra generación. + + Es la vuelta de `/short_spec`: el fichero sale, se retoca, y se manda + de nuevo. Cero LLM en este camino — se valida contra el contrato vivo, + se re-comprueba el fundamento (las cadenas han cambiado y el informe no + es decorativo) y se renderiza. El spec editado se guarda como output + nuevo ANTES del render, por la misma razón que en `produce` y por una + más: los metadatos de `/upload_short` salen del último spec guardado, y + tienen que describir el vídeo que de verdad se renderizó. + """ + if not settings.shortsmith_enabled: + raise ShortsDisabled( + "SHORTSMITH_ENABLED=false — el renderizador está apagado a propósito") + + session = await self.db.get_session(session_id) + if not session: + raise ValueError(f"Session {session_id} not found") + topic = session["topic"] + result = ShortResult(topic=topic, spec=spec) + + # 1. El contrato, en vivo — las mismas rutas verbatim que ve el modelo. + templates = await self.client.templates() + try: + validate_spec(spec, templates) + except SpecInvalid as e: + result.failure = ("El spec editado no pasa el contrato: " + + "; ".join(e.errors[:6])) + logger.warning("Rerender rechazado por el contrato", + session_id=session_id, errors=e.errors[:6]) + return result + result.notes = editorial_notes(spec) + + result.title = spec.get("meta", {}).get("title", topic) + result.duration_s = sum(s.get("duration", 0) for s in spec["shots"]) + result.article_url = await self.db.get_article_url(session_id) + + # 2. Fundamento, otra vez: la edición pudo meter una cifra nueva. + await _report(progress_callback, "🔍 Checking claims against sources…") + chunks = await self.processor.rag_chunks( + session_id, f"{topic} key facts figures dates quotes witnesses", + top_k=CONTEXT_CHUNKS) + if chunks: + result.grounding = check_grounding(spec, chunks) + else: + # Sesión purgada o sin procesar: se renderiza igual, pero el + # informe tiene que decir que esta vez no hubo contra qué mirar. + result.notes.append("sin chunks en la sesión: el fundamento del " + "spec editado NO se ha comprobado") + + # 3. Guardar antes de renderizar. + try: + await self.db.save_output(session_id, OutputType.SHORT_EN, + result.spec_json) + except Exception as e: + logger.warning("No se pudo guardar el spec editado", error=str(e)) + + await self._render_guarded(result, session_id, progress_callback) + logger.info("Short re-renderizado", session_id=session_id, + video=result.video_path, failure=result.failure) + return result + + async def _render_guarded(self, result: ShortResult, session_id: int, + progress_callback: Optional[Callable[[str], Any]] + ) -> None: + """`_render` con los fallos convertidos en `result.failure`.""" try: await self._render(result, session_id, progress_callback) except ShortsmithRejected as e: @@ -232,13 +310,6 @@ class ShortProducer: except OSError as e: result.failure = f"no se pudo guardar el vídeo: {e}" - if result.failure: - logger.warning("Short sin vídeo", session_id=session_id, why=result.failure) - logger.info("Short producido", session_id=session_id, - seconds=round(time.monotonic() - started, 1), - video=result.video_path, cost=round(result.cost_usd, 4)) - return result - async def _render(self, result: ShortResult, session_id: int, progress_callback: Optional[Callable[[str], Any]]) -> None: job_id = await self.client.render(result.spec) diff --git a/tests/test_bot_short_report.py b/tests/test_bot_short_report.py index 87928d4..caa6720 100644 --- a/tests/test_bot_short_report.py +++ b/tests/test_bot_short_report.py @@ -6,7 +6,7 @@ tiene test propio aparte del pipeline. """ import json -from src.bot.bot import _claims_message +from src.bot.bot import _claims_message, _session_from_filename, _video_predates_spec from src.generator.grounding import check_grounding from src.generator.short import ShortResult @@ -109,3 +109,34 @@ def test_upload_message_is_plain_text(): from src.bot.bot import _upload_message text = _upload_message(_uploaded(title="JAL 1628: *three* radars_"), {}, None) assert "*three*" in text and "radars_" in text + + +# --- guard de vídeo viejo en /upload_short ---------------------------------- + +class TestStaleVideoGuard: + """`produce` guarda el spec ANTES de renderizar: si un re-intento falla, + en disco queda el vídeo de la vuelta anterior y subirlo le pondría los + metadatos del spec nuevo a un vídeo viejo.""" + + def test_a_fresh_render_is_never_stale(self): + # El MP4 se escribe ~1 min después de guardarse el spec. + assert not _video_predates_spec(1000.0 + 60, 1000.0) + + def test_clock_jitter_does_not_cry_wolf(self): + assert not _video_predates_spec(1000.0 - 3, 1000.0) + + def test_a_video_hours_older_than_the_spec_is_flagged(self): + assert _video_predates_spec(1000.0 - 3600, 1000.0) + + +class TestSessionFromFilename: + """Telegram conserva el nombre del fichero al reenviarlo: el id que puso + /short_spec manda sobre la sesión activa del chat.""" + + def test_the_short_spec_filename_declares_its_session(self): + assert _session_from_filename("short_166_spec.json") == 166 + + def test_a_foreign_filename_falls_back_to_none(self): + assert _session_from_filename("myspec.json") is None + assert _session_from_filename("") is None + assert _session_from_filename(None) is None diff --git a/tests/test_short_producer.py b/tests/test_short_producer.py index 6da1994..ed0568c 100644 --- a/tests/test_short_producer.py +++ b/tests/test_short_producer.py @@ -361,3 +361,92 @@ async def test_the_youtube_url_never_passes_for_an_article_url(tmp_path): await conn.close() assert url == "https://theexclusionzone.com/x/" + + +# --- re-render de un spec editado (la vuelta de /short_spec) ---------------- + +async def _llm_prohibido(system, prompt): + raise AssertionError("el re-render no debe llamar al LLM: este camino es gratis") + + +@pytest.mark.asyncio +async def test_rerender_writes_the_mp4_without_touching_the_llm(tmp_path, monkeypatch): + from src.db.database import OutputType + + db = FakeDB(article_url="https://www.theexclusionzone.com/jal-1628/") + p = producer(tmp_path, monkeypatch, db=db, llm=_llm_prohibido) + + result = await p.rerender(153, json.loads(json.dumps(SPEC))) + + assert result.has_video + assert result.cost_usd == 0.0 + assert result.article_url == "https://www.theexclusionzone.com/jal-1628/" + # El spec editado queda guardado ANTES del render: /upload_short saca los + # metadatos del último spec y tienen que describir este vídeo. + assert db.saved and db.saved[-1][1] == OutputType.SHORT_EN + + +@pytest.mark.asyncio +async def test_rerender_rechecks_the_grounding_of_the_edited_strings(tmp_path, monkeypatch): + """La edición a mano puede meter una cifra nueva: el informe se rehace.""" + edited = json.loads(json.dumps(SPEC)) + edited["shots"][0]["props"]["headline"] = "41,000 FT" + p = producer(tmp_path, monkeypatch, llm=_llm_prohibido) + + result = await p.rerender(153, edited) + + assert result.has_video + assert [c.text for c in result.grounding.ungrounded] == ["41,000 FT"] + + +@pytest.mark.asyncio +async def test_an_edited_spec_that_breaks_the_contract_never_renders(tmp_path, monkeypatch): + """Los errores vuelven con su ruta verbatim, igual que al modelo.""" + broken = json.loads(json.dumps(SPEC)) + broken["shots"][0]["template"] = "no_existe" + client = FakeClient() + p = producer(tmp_path, monkeypatch, client=client, llm=_llm_prohibido) + + result = await p.rerender(153, broken) + + assert not result.has_video + assert "shots.0.template" in result.failure + assert client.rendered is None + # El spec editado se conserva para poder corregirlo y reenviarlo. + assert json.loads(result.spec_json)["shots"][0]["template"] == "no_existe" + + +@pytest.mark.asyncio +async def test_rerender_on_a_purged_session_says_it_could_not_check(tmp_path, monkeypatch): + """Sin chunks no hay contra qué mirar: se renderiza igual, avisando.""" + p = producer(tmp_path, monkeypatch, processor=FakeProcessor(chunks=[]), + llm=_llm_prohibido) + + result = await p.rerender(153, json.loads(json.dumps(SPEC))) + + assert result.has_video + assert result.grounding is None + assert any("NO se ha comprobado" in n for n in result.notes) + + +@pytest.mark.asyncio +async def test_rerender_failures_keep_the_spec_like_produce_does(tmp_path, monkeypatch): + p = producer(tmp_path, monkeypatch, client=FakeClient(fail_at="render"), + llm=_llm_prohibido) + + result = await p.rerender(153, json.loads(json.dumps(SPEC))) + + assert not result.has_video + assert "shortsmith no responde" in result.failure + assert json.loads(result.spec_json)["meta"]["id"] == "jal1628" + + +@pytest.mark.asyncio +async def test_rerender_respects_the_kill_switch(tmp_path, monkeypatch): + from src.generator.short import ShortsDisabled + + p = producer(tmp_path, monkeypatch, llm=_llm_prohibido) + monkeypatch.setattr(settings, "shortsmith_enabled", False) + + with pytest.raises(ShortsDisabled): + await p.rerender(153, json.loads(json.dumps(SPEC)))