diff --git a/backend/app/enrichment/bulk.py b/backend/app/enrichment/bulk.py index 78ed12e..66449b0 100644 --- a/backend/app/enrichment/bulk.py +++ b/backend/app/enrichment/bulk.py @@ -19,7 +19,16 @@ from sqlmodel import Session -from app.enrichment import attribute, autotag, describe, embed, extract_text, summarize +from app.enrichment import ( + attribute, + autotag, + describe, + embed, + extract_text, + generate_all, + summarize, +) +from app.ingest.filetypes import display_name_from from app.models.asset import Asset from app.models.job import ( KIND_ATTRIBUTE, @@ -27,9 +36,11 @@ KIND_DESCRIBE, KIND_EMBED, KIND_EXTRACT_TEXT, + KIND_GENERATE_ALL, KIND_SUMMARIZE, ) from app.providers.base import ProviderUnavailable +from app.services import suggestions as suggestion_service logger = logging.getLogger(__name__) @@ -39,6 +50,26 @@ # run, but three in a row indicts the provider rather than the content. CONSECUTIVE_FAILURE_LIMIT = 3 +def _generate_and_apply(session: Session, asset: Asset, progress: Progress) -> str: + """Generate all, then keep what it proposed instead of leaving it for review. + + Single-asset "Generate all" stops at suggestions (FR 9.1.4): one asset's worth is + reviewable at a glance. Across a selection of dozens that review is the chore the + person pressed the button to avoid, so here the title and tags are applied as soon as + they exist. Reviewing happens afterwards, in the library, where a wrong tag is one + click to remove. Only blanks are filled, though: an existing summary or description + is kept, and so is a name that is no longer the one the file arrived with. + """ + # Still carrying the name it was ingested with — the filename, with or without its + # extension. Anything else was typed by someone (or generated earlier) and stays. + ingested = {asset.original_name, display_name_from(asset.original_name)} + detail = generate_all.run(session, asset, progress, only_missing=True) + applied = suggestion_service.accept_proposed( + session, asset, rename=asset.name in ingested + ) + return f"{detail} · {applied} applied" + + # What a selection may be put through. Deliberately not "any KIND_": transcription is # excluded because it is billed per minute of audio and a mis-click over two hundred # videos is an expensive way to learn that. @@ -55,6 +86,7 @@ # the mis-click that makes transcription too expensive to offer here costs only time. # It is also the action most worth having in bulk — documents arrive by the folder. KIND_EXTRACT_TEXT: extract_text.run, + KIND_GENERATE_ALL: _generate_and_apply, } # A selection has to be reviewable before it is run, and it is the thing standing between @@ -156,4 +188,5 @@ def inner(_stage, _pct, _detail="", *, stage=stage, pct=pct, detail=detail): KIND_AUTOTAG: "Suggesting tags", KIND_EMBED: "Embedding", KIND_EXTRACT_TEXT: "Reading text", + KIND_GENERATE_ALL: "Generating", } diff --git a/backend/app/enrichment/generate_all.py b/backend/app/enrichment/generate_all.py index 4a9c25c..002bbd1 100644 --- a/backend/app/enrichment/generate_all.py +++ b/backend/app/enrichment/generate_all.py @@ -34,16 +34,34 @@ ) -def run(session: Session, asset: Asset, progress: Progress) -> str: +def run( + session: Session, asset: Asset, progress: Progress, *, only_missing: bool = False +) -> 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. + + `only_missing` skips a summary or description the asset already has. The single-asset + button is a deliberate request for a fresh answer and overwrites; a run over a whole + selection must not quietly replace text someone wrote or generated earlier on the + strength of a stray Ctrl-A. """ + steps = [ + (stage, step) + for stage, step in _STEPS + if not ( + only_missing + and ( + (step is summarize.run and asset.summary) + or (step is describe.run and asset.description) + ) + ) + ] results = [] - total = len(_STEPS) - for index, (stage, step) in enumerate(_STEPS): + 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: diff --git a/backend/app/services/suggestions.py b/backend/app/services/suggestions.py index 8580e25..8fff142 100644 --- a/backend/app/services/suggestions.py +++ b/backend/app/services/suggestions.py @@ -134,13 +134,19 @@ def propose( return created -def accept(session: Session, asset: Asset, suggestion: Suggestion) -> Suggestion: +def accept( + session: Session, asset: Asset, suggestion: Suggestion, *, automatic: bool = False +) -> Suggestion: """Apply one suggestion for real. A title is written through `apply_metadata`, the *human* path, which stamps provenance "human" — because it is: the user read it and chose it. That also means a later enrichment run will not quietly replace a title they approved, which is the behaviour FR 8.1.3 is asking for. + + `automatic` is the one exception: a bulk "Generate all" applies its own proposals + without anyone reading them, so the title is stamped "ai" instead. Claiming "human" + there would make the provenance record say someone approved a name nobody saw. """ if suggestion.status != STATUS_PENDING: return suggestion @@ -154,7 +160,8 @@ def accept(session: Session, asset: Asset, suggestion: Suggestion) -> Suggestion if tag_service.attach(session, asset.id, tag.id): asset_service.reindex_ids(session, [asset.id]) elif suggestion.kind == KIND_TITLE: - asset_service.apply_metadata(session, asset, {"name": suggestion.value}) + write = asset_service.apply_ai_metadata if automatic else asset_service.apply_metadata + write(session, asset, {"name": suggestion.value}) elif suggestion.kind == KIND_ATTRIBUTION: decoded = decode_attribution(suggestion) if decoded: @@ -173,6 +180,28 @@ def accept(session: Session, asset: Asset, suggestion: Suggestion) -> Suggestion return suggestion +def accept_proposed(session: Session, asset: Asset, *, rename: bool = True) -> int: + """Apply every pending tag and title for this asset, unreviewed. Returns how many. + + With `rename` false the title is left pending rather than applied or discarded: the + caller knows the asset already has a name someone chose, and it is still there to + accept by hand. + + Attribution is left pending on purpose. A tag or a name is cheap to change back; a + credit line is a claim about who made something, backed by evidence a person is meant + to read before it is believed (M10). + """ + applied = 0 + for suggestion in pending_for(session, asset.id): + if suggestion.kind not in (KIND_TAG, KIND_TITLE): + continue + if suggestion.kind == KIND_TITLE and not rename: + continue + accept(session, asset, suggestion, automatic=True) + applied += 1 + return applied + + def reject(session: Session, suggestion: Suggestion) -> Suggestion: """Record a no, and keep it, so the next run does not ask again.""" if suggestion.status != STATUS_PENDING: diff --git a/backend/tests/test_bulk_enrich.py b/backend/tests/test_bulk_enrich.py index b673014..b5837c6 100644 --- a/backend/tests/test_bulk_enrich.py +++ b/backend/tests/test_bulk_enrich.py @@ -208,6 +208,91 @@ def reply(url, *, headers=None, json_body, timeout, label, **kwargs): assert session.exec(select(Tag)).all() == [] +def test_generate_all_over_a_selection_applies_the_title_and_tags( + library, session, monkeypatch, upstream +): + """The one bulk action that writes without review: reviewing dozens of suggestions + one asset at a time is the chore the button exists to remove.""" + from app.models.suggestion import Suggestion + from app.models.tag import Tag + + first = _upload_image(library) + second = _upload_image(library) + configure_provider(session) + + replies = iter( + ["A summary.", "A description.", '{"title": "A title", "tags": ["one", "two"]}'] * 2 + ) + + def reply(url, *, headers=None, json_body, timeout, label, **kwargs): + return httpx.Response( + 200, + json={ + "content": [{"type": "text", "text": next(replies)}], + "usage": {"input_tokens": 10, "output_tokens": 5}, + }, + ) + + monkeypatch.setattr(_upstream, "post_json", reply) + job = run_job(library, session, monkeypatch, "generate_all", [first["id"], second["id"]]) + + assert job.status == "done" + assert "2 done" in job.detail + assert {t.name for t in session.exec(select(Tag)).all()} == {"one", "two"} + + for created in (first, second): + asset = session.get(Asset, created["id"]) + assert asset.name == "A title" + assert asset.summary == "A summary." + # Nobody read it, so the record must not say somebody did. + assert json.loads(asset.field_provenance)["name"] == "ai" + + # Applied, not left waiting: nothing is pending, so nothing is asked about. + assert all(s.status == "accepted" for s in session.exec(select(Suggestion)).all()) + + +def test_generate_all_over_a_selection_only_fills_blanks(library, session, monkeypatch, upstream): + """A stray Ctrl-A must not replace text or a name somebody already settled on.""" + from app.models.tag import Tag + + created = _upload_image(library) + asset = session.get(Asset, created["id"]) + asset.name = "My own name" + asset.summary = "Mine." + asset.description = "Also mine." + session.add(asset) + session.commit() + configure_provider(session) + + prompts = [] + + def reply(url, *, headers=None, json_body, timeout, label, **kwargs): + prompts.append(json_body) + return httpx.Response( + 200, + json={ + "content": [{"type": "text", "text": '{"title": "A title", "tags": ["one"]}'}], + "usage": {"input_tokens": 10, "output_tokens": 5}, + }, + ) + + monkeypatch.setattr(_upstream, "post_json", reply) + job = run_job(library, session, monkeypatch, "generate_all", [created["id"]]) + + assert job.status == "done" + session.expire_all() + asset = session.get(Asset, created["id"]) + assert (asset.name, asset.summary, asset.description) == ( + "My own name", + "Mine.", + "Also mine.", + ) + # Only autotag ran, so the summary and description calls were never paid for. + assert len(prompts) == 1 + # Tags still land; the title stays a suggestion rather than being thrown away. + assert [t.name for t in session.exec(select(Tag)).all()] == ["one"] + + # ─── the endpoint ──────────────────────────────────────────────────────────── diff --git a/frontend/src/api/enrichment.ts b/frontend/src/api/enrichment.ts index 83fd339..b571b3a 100644 --- a/frontend/src/api/enrichment.ts +++ b/frontend/src/api/enrichment.ts @@ -39,9 +39,18 @@ export interface Suggestion { /** What a whole selection can be put through. Transcription is deliberately absent — * it is billed per minute of audio, and a mis-click over two hundred videos is an * expensive way to discover it was on the menu. Extracting text is here for the - * opposite reason: it calls nothing, and documents arrive by the folder. */ + * opposite reason: it calls nothing, and documents arrive by the folder. + * + * `generate_all` is the one that writes: over a selection it applies the title and tags + * it comes up with rather than leaving them as suggestions. */ export type BulkAction = - 'describe' | 'summarize' | 'autotag' | 'embed' | 'extract_text' | 'attribute' + | 'describe' + | 'summarize' + | 'autotag' + | 'embed' + | 'extract_text' + | 'attribute' + | 'generate_all' /** One readable chunk of a document, in the unit that document naturally has. */ export interface DocumentPage { diff --git a/frontend/src/components/SelectionBar.test.tsx b/frontend/src/components/SelectionBar.test.tsx index 91e85fb..db211b7 100644 --- a/frontend/src/components/SelectionBar.test.tsx +++ b/frontend/src/components/SelectionBar.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor, within } from '@testing-library/react' +import { act, render, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { MemoryRouter } from 'react-router-dom' import { AxiosError } from 'axios' @@ -112,6 +112,29 @@ describe('SelectionBar enrichment', () => { await waitFor(() => expect(bulk).toHaveBeenCalledWith('summarize', ['a1', 'a2'])) }) + it('generates all, then re-reads the selection when the run ends', async () => { + const bulk = vi.spyOn(enrichmentApi, 'bulk').mockResolvedValue(job()) + const refreshAsset = vi + .spyOn(useLibraryStore.getState(), 'refreshAsset') + .mockResolvedValue() + renderBar() + + await userEvent.click(screen.getByRole('button', { name: /generate all/i })) + await waitFor(() => expect(bulk).toHaveBeenCalledWith('generate_all', ['a1', 'a2'])) + + // The names and tags it applied are on the cards, which only know what they were + // last told — so nothing is re-read until the job is over. + // Each in its own act(): batched into one render the component would never see the + // job running, and so never see it finish. + await act(async () => useActivityStore.setState({ jobs: [job()] })) + expect(refreshAsset).not.toHaveBeenCalled() + await act(async () => useActivityStore.setState({ jobs: [job({ status: 'done' })] })) + + await waitFor(() => expect(refreshAsset).toHaveBeenCalledTimes(2)) + expect(refreshAsset).toHaveBeenCalledWith('a1') + expect(refreshAsset).toHaveBeenCalledWith('a2') + }) + it('offers embed, which M5 deferred', async () => { const bulk = vi.spyOn(enrichmentApi, 'bulk').mockResolvedValue(job()) renderBar() diff --git a/frontend/src/components/SelectionBar.tsx b/frontend/src/components/SelectionBar.tsx index 2fb7e39..6bb39a4 100644 --- a/frontend/src/components/SelectionBar.tsx +++ b/frontend/src/components/SelectionBar.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react' +import { useEffect, useMemo, useRef, useState } from 'react' import { Link } from 'react-router-dom' import { Eye, @@ -7,6 +7,7 @@ import { ScanText, Sparkles, Tags, + Wand2, WandSparkles, X, } from 'lucide-react' @@ -41,6 +42,9 @@ const ENRICHMENTS: Array<{ label: string icon: typeof Eye }> = [ + // First, and the only one here that changes names and tags without asking. It is the + // single-asset "Generate all" run over everything selected, minus the review step. + { action: 'generate_all', label: 'Generate all', icon: Wand2 }, { action: 'extract_text', label: 'Extract text', icon: ScanText }, { action: 'describe', label: 'Describe', icon: Eye }, { action: 'summarize', label: 'Summarise', icon: FileText }, @@ -69,6 +73,21 @@ export default function SelectionBar({ selected, assets, onSelectAll, onClear }: const ids = useMemo(() => [...selected], [selected]) + // Names and tags are on the cards, so unlike the other bulk actions this one leaves the + // grid stale when it finishes. Remembers which assets it was started over — the + // selection may have changed or been cleared by the time the job ends. + const refreshAsset = useLibraryStore((s) => s.refreshAsset) + const generatedIds = useRef(null) + const wasRunning = useRef(false) + useEffect(() => { + if (wasRunning.current && !bulkRunning && generatedIds.current) { + const finished = generatedIds.current + generatedIds.current = null + void Promise.all(finished.map((id) => refreshAsset(id))) + } + wasRunning.current = bulkRunning + }, [bulkRunning, refreshAsset]) + // In the order they were picked, not grid order: for a video the first is the start // frame and the second the end frame, and clicking them in that order is how anyone // would say so. A Set iterates in insertion order, which is exactly that. @@ -100,6 +119,7 @@ export default function SelectionBar({ selected, assets, onSelectAll, onClear }: setUnavailable(false) try { await enrichmentApi.bulk(action, ids) + generatedIds.current = action === 'generate_all' ? ids : null await refreshActivity() } catch (err) { const code = apiErrorCode(err)