diff --git a/backend/app/enrichment/describe.py b/backend/app/enrichment/describe.py index 8e4b166..8fcef9f 100644 --- a/backend/app/enrichment/describe.py +++ b/backend/app/enrichment/describe.py @@ -33,22 +33,27 @@ Progress = Callable[..., None] +# A description is a retrieval aid, not an essay — but see summarize.py for why this is +# also told to the model rather than left as a silent backstop: a limit it does not know +# about is one it writes past, and the truncation lands mid-sentence. +MAX_DESCRIPTION_CHARS = 6000 + SYSTEM_PROMPT = ( "You write descriptions for a media library. Someone will search this library " "later with half-remembered words, and your description is what has to match.\n\n" - "Write one paragraph saying what is actually in this item: who appears or speaks, " - "what is shown, the places, organisations, events and specific terms a person would " - "type. Name things explicitly rather than referring to them generally — 'a man in a " - "lab coat' helps nobody find anything; a name does.\n\n" + "Write what is actually in this item: who appears or speaks, what is shown, the " + "places, organisations, events and specific terms a person would type. Name things " + "explicitly rather than referring to them generally — 'a man in a lab coat' helps " + "nobody find anything; a name does.\n\n" + f"Keep the whole reply under {MAX_DESCRIPTION_CHARS} characters — a hard limit, not " + "a target, so finish the sentence you are on rather than trailing off as you " + "approach it. Most descriptions need nowhere near this much; write one paragraph " + "unless the material genuinely needs more than one.\n\n" "Describe, do not judge. Do not assess whether anything shown or said is true, and " "do not summarise the argument — another field does that. Never open with 'This " - "image' or 'The video shows'. No heading, no preamble, nothing after the paragraph." + "image' or 'The video shows'. No heading, no preamble, nothing after the text." ) -# A description is a retrieval aid, not an essay. The model is asked for one paragraph; -# this is the backstop for when it does not listen. -MAX_DESCRIPTION_CHARS = 2000 - def _prompt(asset: Asset, material: source.SourceMaterial) -> str: parts = [f"Filename: {asset.original_name or asset.name}"] diff --git a/backend/app/enrichment/generate_all.py b/backend/app/enrichment/generate_all.py new file mode 100644 index 0000000..4a9c25c --- /dev/null +++ b/backend/app/enrichment/generate_all.py @@ -0,0 +1,55 @@ +"""The generate-all job: summarise, describe and tag one asset in a single press. + +Three separate provider calls, run one after another — not the merged, single-prompt +version `docs/m6-ai-enrichment.md` sketches as a future cost optimisation ("one pass, +not three"). That would mean redesigning all three prompts around one shared +completion; this is the "press each button in turn" version turned into one job, which +is what a single "Generate all" control needs and costs nothing extra to get right. + +Order is summarize, describe, autotag — not the order the buttons sit in the UI. +`describe`'s own prompt already knows to skip repeating an existing summary ("A summary +already exists; do not repeat it"), so running summarize first is what lets that check +actually have something to check against. `autotag` reads both fields as context, so it +goes last regardless. +""" + +from __future__ import annotations + +import logging +from typing import Callable + +from sqlmodel import Session + +from app.enrichment import autotag, describe, summarize +from app.models.asset import Asset + +logger = logging.getLogger(__name__) + +Progress = Callable[..., None] + +_STEPS = ( + ("Summarising", summarize.run), + ("Describing", describe.run), + ("Suggesting tags", autotag.run), +) + + +def run(session: Session, asset: Asset, progress: Progress) -> str: + """Run summarize, describe and autotag over one asset, in turn. + + Stops at whichever step first raises — the same "one clear error" behaviour a + single-action job already has, rather than swallowing a failure to force the + remaining steps to run against material that step already showed is unusable. + """ + results = [] + total = len(_STEPS) + for index, (stage, step) in enumerate(_STEPS): + base = int(index * 100 / total) + + def inner(_stage: str, pct: int, detail: str = "", *, base=base) -> None: + progress(stage, base + pct // total, detail) + + progress(stage, base, "") + results.append(step(session, asset, inner)) + + return " · ".join(results) diff --git a/backend/app/enrichment/summarize.py b/backend/app/enrichment/summarize.py index 50846fd..3cf099b 100644 --- a/backend/app/enrichment/summarize.py +++ b/backend/app/enrichment/summarize.py @@ -26,22 +26,28 @@ Progress = Callable[..., None] +# Enough for several real paragraphs and not enough for an essay. Also told to the +# model in the prompt below, which is what stops the backstop below from being the +# thing that actually decides where a summary ends — a limit a model does not know +# about is a limit it will write past, and the truncation lands mid-sentence. +MAX_SUMMARY_CHARS = 6000 + SYSTEM_PROMPT = ( "You write summaries for a media library. The person reading yours is trying to " "find this file again later, sometimes years afterwards, often remembering only " "roughly what was in it.\n\n" - "Write one paragraph of plain prose. Lead with what the thing actually is, then " - "what it covers — the specific names, places, claims and terms someone would " - "search for. Prefer the concrete over the general.\n\n" + "Write plain prose. Lead with what the thing actually is, then what it covers — " + "the specific names, places, claims and terms someone would search for. Prefer " + "the concrete over the general.\n\n" + f"Keep the whole reply under {MAX_SUMMARY_CHARS} characters — a hard limit, not a " + "target, so finish the paragraph you are on rather than trailing off mid-sentence " + "as you approach it. Most summaries need nowhere near this much; write one " + "paragraph unless the material genuinely needs more than one.\n\n" "Never open with a phrase like 'This video' or 'The transcript shows'. Do not " "editorialise, do not assess whether anything said is true, and do not add a " - "preamble, a heading, or anything after the paragraph." + "preamble, a heading, or anything after the text." ) -# Enough for a real paragraph and not enough for an essay. The model is told to write -# one paragraph; this is the backstop for when it does not listen. -MAX_SUMMARY_CHARS = 2000 - def _prompt(asset: Asset, material: source.SourceMaterial) -> str: parts = [f"Filename: {asset.original_name or asset.name}"] diff --git a/backend/app/jobs/enrichment.py b/backend/app/jobs/enrichment.py index 22f54e5..e8646e6 100644 --- a/backend/app/jobs/enrichment.py +++ b/backend/app/jobs/enrichment.py @@ -24,6 +24,7 @@ from app.enrichment.describe import run as run_describe from app.enrichment.extract_text import TextExtractionError from app.enrichment.extract_text import run as run_extract_text +from app.enrichment.generate_all import run as run_generate_all from app.enrichment.source import NoSourceMaterial from app.enrichment.summarize import run as run_summarize from app.enrichment.transcribe import TranscriptionError @@ -45,6 +46,7 @@ KIND_DESCRIBE, KIND_EMBED, KIND_EXTRACT_TEXT, + KIND_GENERATE_ALL, KIND_SUMMARIZE, KIND_TRANSCRIBE, LIBRARY_KINDS, @@ -230,6 +232,8 @@ def _run_job(job_id: str) -> None: detail = run_autotag(session, asset, progress) elif job.kind == KIND_DESCRIBE: detail = run_describe(session, asset, progress) + elif job.kind == KIND_GENERATE_ALL: + detail = run_generate_all(session, asset, progress) elif job.kind == KIND_BULK_ENRICH: bulk = run_bulk(session, job.user_id, job.payload, progress) detail = f"{bulk.done} done" diff --git a/backend/app/models/job.py b/backend/app/models/job.py index 128696d..e549cf0 100644 --- a/backend/app/models/job.py +++ b/backend/app/models/job.py @@ -31,6 +31,10 @@ # the odd one out of the per-asset set: it calls no provider, costs nothing, and its # output is an input to the other three rather than something a user reads directly. KIND_EXTRACT_TEXT = "extract_text" +# The "Generate all" button: summarize, describe and autotag run one after another as a +# single job, rather than three activity rows the user has to watch separately. See +# `enrichment/generate_all.py` for why that order and not the button order. +KIND_GENERATE_ALL = "generate_all" # One action applied across a chosen set of assets. Which action, and which assets, # live in `EnrichmentJob.payload` — see there for why it is one job and not N. KIND_BULK_ENRICH = "bulk_enrich" @@ -44,6 +48,7 @@ KIND_AUTOTAG, KIND_EMBED, KIND_EXTRACT_TEXT, + KIND_GENERATE_ALL, } ) diff --git a/backend/app/routers/enrichment.py b/backend/app/routers/enrichment.py index 180a93b..da4ffbc 100644 --- a/backend/app/routers/enrichment.py +++ b/backend/app/routers/enrichment.py @@ -30,6 +30,7 @@ KIND_DESCRIBE, KIND_EMBED, KIND_EXTRACT_TEXT, + KIND_GENERATE_ALL, KIND_SUMMARIZE, ) from app.models.document import DocumentPage @@ -182,6 +183,41 @@ def start_describe( return DataResponse(data=KINDS["enrichment"].to_activity(job)) +@router.post( + "/{asset_id}/generate-all", response_model=DataResponse[ActivityJobRead], status_code=202 +) +def start_generate_all( + asset_id: str, + user: CurrentUser, + session: Session = Depends(get_session), +) -> DataResponse[ActivityJobRead]: + """Queue summarize, describe and autotag together — the one-button version of + pressing each in turn.""" + asset = _owned_asset(asset_id, user.id, session) + _require_provider(session, user.id) + + if not summarisable(asset): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "code": "not_summarisable", + "message": "This asset has no content to generate from", + }, + ) + + if enrichment_jobs.active_job(session, asset.id, KIND_GENERATE_ALL) is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "code": "already_running", + "message": "This asset is already generating metadata", + }, + ) + + job = enrichment_jobs.submit(session, asset, KIND_GENERATE_ALL) + return DataResponse(data=KINDS["enrichment"].to_activity(job)) + + class DocumentPageRead(BaseModel): model_config = ConfigDict(from_attributes=True) diff --git a/backend/app/services/assets.py b/backend/app/services/assets.py index 8625afe..a4b5f65 100644 --- a/backend/app/services/assets.py +++ b/backend/app/services/assets.py @@ -89,6 +89,7 @@ async def ingest_upload( _reindex(session, asset) _describe(session, storage, asset) + _chain_transcription(session, asset) return asset @@ -141,6 +142,47 @@ def _describe(session: Session, storage: LocalStorage, asset: Asset) -> None: _reindex(session, asset) +def _chain_transcription(session: Session, asset: Asset) -> None: + """Transcribe audio and video the moment they land, without being asked. + + Mirrors `jobs/enrichment.py::_chain_embedding`, one step earlier in the same + chain: a video that nobody transcribes has nothing for describe, summarise, + autotag or search to read, and asking a user to press Transcribe by hand before + any of that works is a step the pipeline does not need them for. + + Guarded on a Deepgram key actually being configured, for the same reason the + embedding chain is guarded on a provider: queueing unconditionally would put a + red "no Deepgram key" row in the activity feed after every single upload, which + trains people to ignore it. + + Imports `app.jobs.enrichment` lazily rather than at module load — that module's + import chain leads back through `app.enrichment.describe` to this one, and + importing it at the top of this file would be a cycle. + """ + from app.enrichment.transcribe import can_transcribe + from app.jobs import enrichment as enrichment_jobs + from app.models.job import KIND_TRANSCRIBE + from app.settings_store import load_deepgram_key + + if not can_transcribe(asset): + return + + try: + if not load_deepgram_key(session, asset.user_id): + logger.info( + "Not transcribing asset %s: no Deepgram key configured for this user", + asset.id, + ) + return + + if enrichment_jobs.active_job(session, asset.id, KIND_TRANSCRIBE) is not None: + return + + enrichment_jobs.submit(session, asset, KIND_TRANSCRIBE) + except Exception: # noqa: BLE001 - an upload that succeeded must stay succeeded + logger.warning("Could not queue transcription for asset %s", asset.id, exc_info=True) + + def _reindex(session: Session, asset: Asset) -> None: """Keep the keyword index in step with the row. diff --git a/backend/tests/test_generate_all.py b/backend/tests/test_generate_all.py new file mode 100644 index 0000000..c997e9f --- /dev/null +++ b/backend/tests/test_generate_all.py @@ -0,0 +1,203 @@ +"""The "Generate all" job: summarize, describe and autotag in one press. + +Three real provider calls in a row, stubbed at the same upstream boundary the other +enrichment tests use — this is the "press each button in turn" behaviour turned into +one job, not a new prompt design, so what is worth proving is the sequencing: each +step's output is visible to the ones after it, and a failure partway through stops the +rest rather than running them against material that step already showed was unusable. +""" + +import json + +import httpx +import pytest + +from app.enrichment import generate_all +from app.jobs import enrichment as enrichment_jobs +from app.models.asset import Asset +from app.models.job import EnrichmentJob, KIND_GENERATE_ALL +from app.providers import _upstream +from app.providers.base import ProviderError +from app.services import suggestions as suggestion_service + +from tests.test_summarize import _upload_image, add_transcript, configure_provider + +SUMMARY = "A long interview about neuroweapons, recorded in a studio." +DESCRIPTION = "James Giordano speaking to camera in a panelled studio, discussing DARPA." +SUGGESTIONS = {"title": "Giordano on Neuroweapons", "tags": ["James Giordano", "neuroweapons"]} + +# In call order: summarize's plain text, describe's plain text, autotag's JSON. +REPLIES = [SUMMARY, DESCRIPTION, json.dumps(SUGGESTIONS)] + + +@pytest.fixture(name="upstream") +def upstream_fixture(monkeypatch): + """Answer each of the three calls with its own canned reply, in order.""" + state = {"calls": [], "replies": list(REPLIES)} + + def fake_post_json(url, *, headers=None, json_body, timeout, label, **kwargs): + state["calls"].append({"url": url, "body": json_body}) + text = state["replies"][len(state["calls"]) - 1] + return httpx.Response( + 200, + json={ + "content": [{"type": "text", "text": text}], + "usage": {"input_tokens": 500, "output_tokens": 30}, + }, + ) + + monkeypatch.setattr(_upstream, "post_json", fake_post_json) + return state + + +def sent_prompt(state, call_index: int) -> str: + content = state["calls"][call_index]["body"]["messages"][0]["content"] + return "\n".join(b["text"] for b in content if b.get("type") == "text") + + +# ─── the module, directly ──────────────────────────────────────────────────── + + +def test_it_runs_all_three_and_writes_both_fields(library, session, upstream): + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + add_transcript(session, asset.id, ["Giordano on neuroweapons."]) + configure_provider(session) + + generate_all.run(session, asset, lambda *a, **k: None) + + session.refresh(asset) + assert asset.summary == SUMMARY + assert asset.description == DESCRIPTION + assert len(upstream["calls"]) == 3 + + suggestions = suggestion_service.pending_for(session, asset.id) + assert {s.value for s in suggestions} == { + "Giordano on Neuroweapons", + "James Giordano", + "neuroweapons", + } + + +def test_summarize_runs_before_describe_so_describe_can_avoid_repeating_it( + library, session, upstream +): + """`describe`'s own prompt knows to skip a summary that already exists. That only + means anything if the summary is written before describe reads the asset.""" + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + add_transcript(session, asset.id, ["Giordano on neuroweapons."]) + configure_provider(session) + + generate_all.run(session, asset, lambda *a, **k: None) + + describe_prompt = sent_prompt(upstream, 1) + assert SUMMARY in describe_prompt + assert "do not repeat it" in describe_prompt + + +def test_autotag_runs_last_and_sees_both_fields(library, session, upstream): + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + add_transcript(session, asset.id, ["Giordano on neuroweapons."]) + configure_provider(session) + + generate_all.run(session, asset, lambda *a, **k: None) + + autotag_prompt = sent_prompt(upstream, 2) + assert SUMMARY in autotag_prompt + assert DESCRIPTION in autotag_prompt + + +def test_a_failure_partway_through_stops_the_rest(library, session, monkeypatch): + """One clear error beats two of the three silently not happening.""" + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + add_transcript(session, asset.id, ["Giordano on neuroweapons."]) + configure_provider(session) + + def refuse(url, **kwargs): + return httpx.Response(401, json={"error": {"message": "credit balance is too low"}}) + + monkeypatch.setattr(_upstream, "post_json", refuse) + + with pytest.raises(ProviderError, match="credit balance is too low"): + generate_all.run(session, asset, lambda *a, **k: None) + + session.refresh(asset) + assert asset.summary is None + assert asset.description is None + + +# ─── the endpoint ──────────────────────────────────────────────────────────── + + +def test_generate_all_queues_a_job(library, session): + created = _upload_image(library) + configure_provider(session) + + response = library.post(f"/api/assets/{created['id']}/generate-all") + + assert response.status_code == 202 + assert response.json()["data"]["action"] == KIND_GENERATE_ALL + + +def test_without_a_provider_the_button_is_told_why(library, session): + created = _upload_image(library) + + response = library.post(f"/api/assets/{created['id']}/generate-all") + + assert response.status_code == 400 + assert response.json()["detail"]["code"] == "provider_unavailable" + + +def test_a_second_run_while_one_is_going_is_refused(library, session): + created = _upload_image(library) + configure_provider(session) + library.post(f"/api/assets/{created['id']}/generate-all") + + response = library.post(f"/api/assets/{created['id']}/generate-all") + + assert response.status_code == 409 + assert response.json()["detail"]["code"] == "already_running" + + +def test_someone_elses_asset_is_a_404(library, session): + configure_provider(session) + session.add( + Asset( + id="theirs", + user_id="a-different-user", + name="Theirs", + asset_type="image", + storage_key="k", + ) + ) + session.commit() + + assert library.post("/api/assets/theirs/generate-all").status_code == 404 + + +def test_authentication_is_required(client): + assert client.post("/api/assets/anything/generate-all").status_code == 401 + + +# ─── through the worker ────────────────────────────────────────────────────── + + +def test_the_worker_runs_it_end_to_end(library, session, monkeypatch, upstream): + created = _upload_image(library) + add_transcript(session, created["id"], ["Giordano on neuroweapons."]) + configure_provider(session) + + job = library.post(f"/api/assets/{created['id']}/generate-all").json()["data"] + + queue = enrichment_jobs.queue() + monkeypatch.setattr(queue, "engine", session.get_bind()) + enrichment_jobs._run_job(job["id"]) + + session.expire_all() + assert session.get(EnrichmentJob, job["id"]).status == "done" + asset = session.get(Asset, created["id"]) + assert asset.summary == SUMMARY + assert asset.description == DESCRIPTION diff --git a/backend/tests/test_transcripts.py b/backend/tests/test_transcripts.py index 8a811be..314f63d 100644 --- a/backend/tests/test_transcripts.py +++ b/backend/tests/test_transcripts.py @@ -64,6 +64,75 @@ def fake_transcribe_file(audio_path, api_key, *, model=deepgram.DEFAULT_MODEL, l return calls +def _upload_audio(client): + return client.post( + "/api/assets", + files=[("files", ("sample_audio.mp3", (FIXTURES / "sample_audio.mp3").read_bytes(), "audio/mpeg"))], + ).json()["created"][0] + + +# ─── auto-transcription on upload ──────────────────────────────────────────── + + +def test_uploading_a_video_with_a_key_configured_queues_its_own_transcription( + library, session +): + """Nothing else in the pipeline works on a video until it has a transcript, so + nobody should have to press Transcribe by hand first.""" + from app.settings_store import DEEPGRAM_API_KEY, set_setting + + set_setting(session, "user-under-test", DEEPGRAM_API_KEY, "dg-test-key") + + created = _upload_video(library) + + jobs = session.exec( + select(EnrichmentJob).where(EnrichmentJob.asset_id == created["id"]) + ).all() + assert len(jobs) == 1 + assert jobs[0].kind == KIND_TRANSCRIBE + assert jobs[0].status == "queued" + + +def test_uploading_audio_with_a_key_configured_also_queues_it(library, session): + """`can_transcribe` already treats audio and video the same; the auto-chain does + too rather than re-deriving its own narrower rule.""" + from app.settings_store import DEEPGRAM_API_KEY, set_setting + + set_setting(session, "user-under-test", DEEPGRAM_API_KEY, "dg-test-key") + + created = _upload_audio(library) + + jobs = session.exec( + select(EnrichmentJob).where(EnrichmentJob.asset_id == created["id"]) + ).all() + assert len(jobs) == 1 + assert jobs[0].kind == KIND_TRANSCRIBE + + +def test_uploading_a_video_without_a_key_queues_nothing(library, session): + """Queueing unconditionally would put a red "no Deepgram key" row in the activity + feed after every single upload, which trains people to ignore it.""" + created = _upload_video(library) + + jobs = session.exec( + select(EnrichmentJob).where(EnrichmentJob.asset_id == created["id"]) + ).all() + assert jobs == [] + + +def test_uploading_an_image_never_auto_transcribes(library, session): + from app.settings_store import DEEPGRAM_API_KEY, set_setting + + set_setting(session, "user-under-test", DEEPGRAM_API_KEY, "dg-test-key") + + created = _upload_image(library) + + jobs = session.exec( + select(EnrichmentJob).where(EnrichmentJob.asset_id == created["id"]) + ).all() + assert jobs == [] + + # ─── starting a job ────────────────────────────────────────────────────────── diff --git a/frontend/src/api/enrichment.ts b/frontend/src/api/enrichment.ts index bcd8f4d..2872f18 100644 --- a/frontend/src/api/enrichment.ts +++ b/frontend/src/api/enrichment.ts @@ -55,6 +55,13 @@ export const enrichmentApi = { .then((r) => r.data.data) }, + /** Summarise, describe and autotag together — the "Generate all" button. */ + generateAll(assetId: string): Promise { + return client + .post>(`/assets/${assetId}/generate-all`) + .then((r) => r.data.data) + }, + autotag(assetId: string): Promise { return client .post>(`/assets/${assetId}/autotag`) diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index df7b42d..ad15411 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -7,7 +7,9 @@ import { Mic, Pencil, ScanText, + Tags as TagsIcon, Trash2, + Wand2, X, } from 'lucide-react' import type { Asset } from '@/api/assets' @@ -246,6 +248,18 @@ export default function AssetDetail({ const details = (
+ {/* One button that runs describe, summarise and autotag in turn, for anyone who + would otherwise press the three icon buttons below one after another. */} + +
- +
+ + {/* Beside the label it writes into, rather than as its own row below the + box — describe fills this field, and that is what the icon says. */} + +