fix(short): cerrar el bucle de edición y el desfase spec/vídeo en la subida
Build & Deploy ResearchOwl / build-and-push (push) Successful in 1m17s

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 <noreply@anthropic.com>
This commit is contained in:
ChemaVX
2026-08-05 15:46:06 +00:00
co-authored by Claude Fable 5
parent ec3e6d05c0
commit cc1f0cab93
5 changed files with 363 additions and 34 deletions
+12 -2
View File
@@ -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
+142 -14
View File
@@ -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,6 +540,18 @@ async def cmd_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE):
"🚫 Los Shorts están desactivados (`SHORTSMITH_ENABLED=false`).")
return
await _deliver_short(update.message, reporter, result, session)
except Exception as e:
logger.error("Short generation failed", error=str(e), exc_info=True)
await update.message.reply_text(f"❌ Short failed: {str(e)[:300]}")
finally:
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}"
@@ -543,26 +559,19 @@ async def cmd_short(update: Update, ctx: ContextTypes.DEFAULT_TYPE):
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(
await message.reply_video(
video=f,
filename=f"short_{session_id}.mp4",
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,
await _send_spec_file(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))
except Exception as e:
logger.error("Short generation failed", error=str(e), exc_info=True)
await update.message.reply_text(f"❌ Short failed: {str(e)[:300]}")
finally:
await db_conn.close()
await message.reply_text(_claims_message(result))
async def cmd_short_spec(update: Update, ctx: ContextTypes.DEFAULT_TYPE):
@@ -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 <tema>")
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))
+78 -7
View File
@@ -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)
+32 -1
View File
@@ -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
+89
View File
@@ -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)))